From 012f998590985b948f01341830e7299cac4497f6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 8 Aug 2024 12:25:07 +0200 Subject: [PATCH] Add filters to dashboard assignment section (#6538) --- lms/static/scripts/frontend_apps/api-types.ts | 5 +- .../dashboard/AssignmentActivity.tsx | 53 ++++++- .../dashboard/test/AssignmentActivity-test.js | 144 ++++++++++++++++-- 3 files changed, 188 insertions(+), 14 deletions(-) diff --git a/lms/static/scripts/frontend_apps/api-types.ts b/lms/static/scripts/frontend_apps/api-types.ts index 97f51267b4..b336d15a0c 100644 --- a/lms/static/scripts/frontend_apps/api-types.ts +++ b/lms/static/scripts/frontend_apps/api-types.ts @@ -202,8 +202,11 @@ export type StudentsMetricsResponse = { students: StudentWithMetrics[]; }; -export type AssignmentWithMetrics = Assignment & { +export type AssignmentWithCourse = Assignment & { course: Course; +}; + +export type AssignmentWithMetrics = AssignmentWithCourse & { annotation_metrics: AnnotationMetrics; }; diff --git a/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx b/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx index 4eb5713e40..537c2feb7b 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx +++ b/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx @@ -1,14 +1,17 @@ import { useMemo } from 'preact/hooks'; -import { useParams } from 'wouter-preact'; +import { useLocation, useParams, useSearch } from 'wouter-preact'; import type { - AssignmentWithMetrics, + AssignmentWithCourse, StudentsMetricsResponse, } from '../../api-types'; import { useConfig } from '../../config'; import { urlPath, useAPIFetch } from '../../utils/api'; +import { useDashboardFilters } from '../../utils/dashboard/hooks'; +import { courseURL } from '../../utils/dashboard/navigation'; import { useDocumentTitle } from '../../utils/hooks'; -import { replaceURLParams } from '../../utils/url'; +import { recordToQueryString, replaceURLParams } from '../../utils/url'; +import DashboardActivityFilters from './DashboardActivityFilters'; import DashboardBreadcrumbs from './DashboardBreadcrumbs'; import FormattedDate from './FormattedDate'; import OrderableActivityTable from './OrderableActivityTable'; @@ -31,12 +34,20 @@ export default function AssignmentActivity() { assignmentId: string; organizationPublicId?: string; }>(); - const assignment = useAPIFetch( + + const { filters, updateFilters } = useDashboardFilters(); + const { studentIds } = filters; + const search = useSearch(); + const [, navigate] = useLocation(); + + const assignment = useAPIFetch( replaceURLParams(routes.assignment, { assignment_id: assignmentId }), ); + const students = useAPIFetch( routes.students_metrics, { + h_userid: studentIds, assignment_id: assignmentId, public_id: organizationPublicId, }, @@ -78,6 +89,40 @@ export default function AssignmentActivity() { {assignment.data && title} + {assignment.data && ( + + navigate( + recordToQueryString({ + student_id: studentIds, + assignment_id: assignmentId, + }), + ), + }} + assignments={{ + activeItem: assignment.data, + // When active assignment is cleared, navigate to its course page, + // but keep other query params intact + onClear: () => { + const query = search.length === 0 ? '' : `?${search}`; + navigate(`${courseURL(assignment.data!.course.id)}${query}`); + }, + }} + students={{ + selectedIds: studentIds, + onChange: studentIds => updateFilters({ studentIds }), + }} + onClearSelection={ + studentIds.length > 0 + ? () => updateFilters({ studentIds: [] }) + : undefined + } + /> + )} { }, }, ]; + const activeAssignment = { + id: 123, + title: 'The title', + course: { + id: 12, + title: 'The course', + }, + }; let fakeUseAPIFetch; + let fakeNavigate; + let fakeUseSearch; let fakeConfig; + let wrappers; beforeEach(() => { fakeUseAPIFetch = sinon.stub().callsFake(url => ({ isLoading: false, - data: url.endsWith('metrics') - ? { students } - : { - title: 'The title', - course: { - title: 'The course', - }, - }, + data: url.endsWith('metrics') ? { students } : activeAssignment, })); + fakeNavigate = sinon.stub(); + fakeUseSearch = sinon.stub().returns('current=query'); fakeConfig = { dashboard: { routes: { @@ -61,6 +68,8 @@ describe('AssignmentActivity', () => { }, }; + wrappers = []; + $imports.$mock(mockImportedComponents()); $imports.$restore({ // Do not mock FormattedDate, for consistency when checking @@ -71,19 +80,28 @@ describe('AssignmentActivity', () => { '../../utils/api': { useAPIFetch: fakeUseAPIFetch, }, + 'wouter-preact': { + useParams: sinon.stub().returns({ assignmentId: '123' }), + useSearch: fakeUseSearch, + useLocation: sinon.stub().returns(['', fakeNavigate]), + }, }); }); afterEach(() => { + wrappers.forEach(wrapper => wrapper.unmount()); $imports.$restore(); }); function createComponent() { - return mount( + const wrapper = mount( , ); + wrappers.push(wrapper); + + return wrapper; } it('shows loading indicators while data is loading', () => { @@ -171,6 +189,114 @@ describe('AssignmentActivity', () => { }); }); + context('when filters are set', () => { + function setCurrentURL(url) { + history.replaceState(null, '', url); + } + + beforeEach(() => { + setCurrentURL('?'); + }); + + it('initializes expected filters', () => { + setCurrentURL('?student_id=1&student_id=2'); + + const wrapper = createComponent(); + const filters = wrapper.find('DashboardActivityFilters'); + + // Active course and assignment are set from the route + assert.deepEqual( + filters.prop('courses').activeItem, + activeAssignment.course, + ); + assert.deepEqual( + filters.prop('assignments').activeItem, + activeAssignment, + ); + // Students are set from the query + assert.deepEqual(filters.prop('students').selectedIds, ['1', '2']); + + // Selected filters are propagated when loading assignment metrics + assert.calledWith(fakeUseAPIFetch.lastCall, sinon.match.string, { + h_userid: ['1', '2'], + assignment_id: '123', + public_id: undefined, + }); + }); + + [ + { query: '', expectedHasSelection: false }, + { query: '?foo=bar', expectedHasSelection: false }, + { query: '?student_id=3', expectedHasSelection: true }, + { query: '?student_id=1&student_id=3', expectedHasSelection: true }, + ].forEach(({ query, expectedHasSelection }) => { + it('has `onClearSelection` if at least one student is selected', () => { + setCurrentURL(query); + + const wrapper = createComponent(); + const filters = wrapper.find('DashboardActivityFilters'); + + assert.equal(!!filters.prop('onClearSelection'), expectedHasSelection); + }); + }); + + it('updates query when selected students change', () => { + const wrapper = createComponent(); + const filters = wrapper.find('DashboardActivityFilters'); + + act(() => filters.prop('students').onChange(['3', '7'])); + + assert.equal(location.search, '?student_id=3&student_id=7'); + }); + + it('clears selected students on clear selection', () => { + setCurrentURL('?foo=bar&student_id=8&student_id=20&student_id=32'); + + const wrapper = createComponent(); + const filters = wrapper.find('DashboardActivityFilters'); + + act(() => filters.props().onClearSelection()); + + assert.equal(location.search, '?foo=bar'); + }); + + it('navigates to home page preserving assignment and students when course is cleared', () => { + setCurrentURL('?student_id=8&student_id=20&student_id=32'); + + const wrapper = createComponent(); + const filters = wrapper.find('DashboardActivityFilters'); + + act(() => filters.prop('courses').onClear()); + + assert.calledWith( + fakeNavigate, + '?student_id=8&student_id=20&student_id=32&assignment_id=123', + ); + }); + + [ + { + currentSearch: 'current=query', + expectedDestination: '/courses/12?current=query', + }, + { + currentSearch: '', + expectedDestination: '/courses/12', + }, + ].forEach(({ currentSearch, expectedDestination }) => { + it('navigates to course preserving current query when selected assignment is cleared', () => { + fakeUseSearch.returns(currentSearch); + + const wrapper = createComponent(); + const filters = wrapper.find('DashboardActivityFilters'); + + act(() => filters.prop('assignments').onClear()); + + assert.calledWith(fakeNavigate, expectedDestination); + }); + }); + }); + it( 'should pass a11y checks', checkAccessibility({