Skip to content

Commit

Permalink
Create hook to track selected dashboard filters in query string
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Jul 25, 2024
1 parent 2531350 commit 2e52aca
Show file tree
Hide file tree
Showing 8 changed files with 250 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import { useConfig } from '../../config';
import { useAPIFetch } from '../../utils/api';

export type DashboardActivityFiltersProps = {
selectedCourses: Course[];
onCoursesChange: (newCourses: Course[]) => void;
selectedAssignments: Assignment[];
onAssignmentsChange: (newAssignments: Assignment[]) => void;
selectedStudents: Student[];
onStudentsChange: (newStudents: Student[]) => void;
selectedCourses: string[];
onCoursesChange: (newCourses: string[]) => void;
selectedAssignments: string[];
onAssignmentsChange: (newAssignments: string[]) => void;
selectedStudents: string[];
onStudentsChange: (newStudents: string[]) => void;
};

/**
Expand Down Expand Up @@ -52,7 +52,8 @@ export default function DashboardActivityFilters({
) : selectedCourses.length === 0 ? (
<>All courses</>
) : selectedCourses.length === 1 ? (
selectedCourses[0].title
courses.data?.courses.find(c => `${c.id}` === selectedCourses[0])
?.title
) : (
<>{selectedCourses.length} courses</>
)
Expand All @@ -61,7 +62,7 @@ export default function DashboardActivityFilters({
>
<MultiSelect.Option value={undefined}>All courses</MultiSelect.Option>
{courses.data?.courses.map(course => (
<MultiSelect.Option key={course.id} value={course}>
<MultiSelect.Option key={course.id} value={`${course.id}`}>
{course.title}
</MultiSelect.Option>
))}
Expand All @@ -77,7 +78,9 @@ export default function DashboardActivityFilters({
) : selectedAssignments.length === 0 ? (
<>All assignments</>
) : selectedAssignments.length === 1 ? (
selectedAssignments[0].title
assignments.data?.assignments.find(
a => `${a.id}` === selectedAssignments[0],
)?.title
) : (
<>{selectedAssignments.length} assignments</>
)
Expand All @@ -88,7 +91,7 @@ export default function DashboardActivityFilters({
All assignments
</MultiSelect.Option>
{assignments.data?.assignments.map(assignment => (
<MultiSelect.Option key={assignment.id} value={assignment}>
<MultiSelect.Option key={assignment.id} value={`${assignment.id}`}>
{assignment.title}
</MultiSelect.Option>
))}
Expand All @@ -104,7 +107,9 @@ export default function DashboardActivityFilters({
) : selectedStudents.length === 0 ? (
<>All students</>
) : selectedStudents.length === 1 ? (
selectedStudents[0].display_name
students.data?.students.find(
s => s.h_userid === selectedStudents[0],
)?.display_name
) : (
<>{selectedStudents.length} students</>
)
Expand All @@ -113,7 +118,7 @@ export default function DashboardActivityFilters({
>
<MultiSelect.Option value={undefined}>All students</MultiSelect.Option>
{studentsWithName?.map(student => (
<MultiSelect.Option key={student.lms_id} value={student}>
<MultiSelect.Option key={student.lms_id} value={student.h_userid}>
{student.display_name}
</MultiSelect.Option>
))}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import { Link } from '@hypothesis/frontend-shared';
import { useMemo, useState } from 'preact/hooks';
import { useMemo } from 'preact/hooks';
import { Link as RouterLink, useParams } from 'wouter-preact';

import type {
Assignment,
Course,
CoursesResponse,
Student,
} from '../../api-types';
import type { CoursesResponse } from '../../api-types';
import { useConfig } from '../../config';
import { urlPath, useAPIFetch } from '../../utils/api';
import { useDashboardQueryFilters } from '../../utils/dashboard/hooks';
import { useDocumentTitle } from '../../utils/hooks';
import { replaceURLParams } from '../../utils/url';
import DashboardActivityFilters from './DashboardActivityFilters';
Expand Down Expand Up @@ -37,23 +33,8 @@ export default function OrganizationActivity() {

useDocumentTitle('All courses');

const [selectedStudents, setSelectedStudents] = useState<Student[]>([]);
const [selectedAssignments, setSelectedAssignments] = useState<Assignment[]>(
[],
);
const [selectedCourses, setSelectedCourses] = useState<Course[]>([]);
const studentIds = useMemo(
() => selectedStudents.map(s => s.h_userid),
[selectedStudents],
);
const assignmentIds = useMemo(
() => selectedAssignments.map(a => `${a.id}`),
[selectedAssignments],
);
const courseIds = useMemo(
() => selectedCourses.map(c => `${c.id}`),
[selectedCourses],
);
const { filters, updateFilters } = useDashboardQueryFilters();
const { courseIds, assignmentIds, studentIds } = filters;

const courses = useAPIFetch<CoursesResponse>(
replaceURLParams(routes.organization_courses, {
Expand All @@ -79,12 +60,12 @@ export default function OrganizationActivity() {
<div className="flex flex-col gap-y-5">
<h2 className="text-lg text-brand font-semibold">All courses</h2>
<DashboardActivityFilters
selectedStudents={selectedStudents}
onStudentsChange={setSelectedStudents}
selectedAssignments={selectedAssignments}
onAssignmentsChange={setSelectedAssignments}
selectedCourses={selectedCourses}
onCoursesChange={setSelectedCourses}
selectedStudents={studentIds}
onStudentsChange={studentIds => updateFilters({ studentIds })}
selectedAssignments={assignmentIds}
onAssignmentsChange={assignmentIds => updateFilters({ assignmentIds })}
selectedCourses={courseIds}
onCoursesChange={courseIds => updateFilters({ courseIds })}
/>
<OrderableActivityTable
loading={courses.isLoading}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ import { MultiSelect } from '@hypothesis/frontend-shared';
import {
checkAccessibility,
mockImportedComponents,
waitFor,
} from '@hypothesis/frontend-testing';
import { mount } from 'enzyme';
import sinon from 'sinon';

import { Config } from '../../../config';
import { urlPath } from '../../../utils/api';
import DashboardActivityFilters, {
$imports,
} from '../DashboardActivityFilters';
Expand Down Expand Up @@ -35,17 +37,20 @@ describe('DashboardActivityFilters', () => {
const studentsWithName = [
{
lms_id: '1',
h_userid: 'acct:[email protected]',
display_name: 'First student',
},
{
lms_id: '2',
h_userid: 'acct:[email protected]',
display_name: 'Second student',
},
];
const students = [
...studentsWithName,
{
lms_id: '3',
h_userid: 'acct:[email protected]',
display_name: '', // Student with an empty name won't be displayed
},
];
Expand Down Expand Up @@ -204,33 +209,85 @@ describe('DashboardActivityFilters', () => {
[
{
id: 'courses-select',
selection: courses.map(c => `${c.id}`),
getExpectedCallback: () => onCoursesChange,
},
{
id: 'assignments-select',
selection: assignments.map(a => `${a.id}`),
getExpectedCallback: () => onAssignmentsChange,
},
{
id: 'students-select',
selection: studentsWithName.map(s => s.h_userid),
getExpectedCallback: () => onStudentsChange,
},
].forEach(({ id, getExpectedCallback }) => {
].forEach(({ id, selection, getExpectedCallback }) => {
it('invokes corresponding change callback', () => {
const wrapper = createComponent();
const select = getSelect(wrapper, id);

select.props().onChange();
assert.called(getExpectedCallback());
select.props().onChange(selection);
assert.calledWith(getExpectedCallback(), selection);
});
});

[
{
initialQueryString: `?course_id=${courses[0].id}&assignment_id=${assignments[0].id}`,
expectedCourses: [`${courses[0].id}`],
expectedAssignments: [`${assignments[0].id}`],
expectedStudents: [],
},
{
initialQueryString: `?course_id=${courses[0].id}&course_id=${courses[1].id}`,
expectedCourses: courses.map(c => `${c.id}`),
expectedAssignments: [],
expectedStudents: [],
},
{
initialQueryString: urlPath`?h_userid=${studentsWithName[0].h_userid}&h_userid=${studentsWithName[1].h_userid}&assignment_id=${assignments[1].id}`,
expectedCourses: [],
expectedAssignments: [`${assignments[1].id}`],
expectedStudents: studentsWithName.map(s => s.h_userid),
},
{
initialQueryString: urlPath`?h_userid=${studentsWithName[1].h_userid}`,
expectedCourses: [],
expectedAssignments: [],
expectedStudents: [studentsWithName[1].h_userid],
},
].forEach(
({
initialQueryString,
expectedCourses,
expectedAssignments,
expectedStudents,
}) => {
it.skip('should initialize selection based on query string', async () => {
history.replaceState(null, '', initialQueryString);

createComponent();

// Callbacks will be invoked only after corresponding courses,
// assignments and students have been asynchronously loaded via
// useAPIFetch, so we need to wait for it.
await waitFor(() => onCoursesChange.called);

assert.calledWith(onCoursesChange, expectedCourses);
assert.calledWith(onAssignmentsChange, expectedAssignments);
assert.calledWith(onStudentsChange, expectedStudents);
});
},
);

context('when items are selected', () => {
[0, 1].forEach(index => {
it('shows item name when only one is selected', () => {
const wrapper = createComponent({
selectedCourses: [courses[index]],
selectedAssignments: [assignments[index]],
selectedStudents: [students[index]],
selectedCourses: [`${courses[index].id}`],
selectedAssignments: [`${assignments[index].id}`],
selectedStudents: [students[index].h_userid],
});

assert.equal(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,27 +164,24 @@ describe('OrganizationActivity', () => {

// Every time the filters callbacks are invoked, the component will
// re-render and re-fetch metrics with updated query.
updateFilter('onStudentsChange', [
{ h_userid: '123' },
{ h_userid: '456' },
]);
updateFilter('onStudentsChange', ['123', '456']);
assertCoursesFetched({
h_userid: ['123', '456'],
assignment_id: [],
course_id: [],
});

updateFilter('onAssignmentsChange', [{ id: 1 }, { id: 2 }]);
updateFilter('onAssignmentsChange', ['1', '2']);
assertCoursesFetched({
h_userid: ['123', '456'],
h_userid: [],
assignment_id: ['1', '2'],
course_id: [],
});

updateFilter('onCoursesChange', [{ id: 3 }, { id: 8 }, { id: 9 }]);
updateFilter('onCoursesChange', ['3', '8', '9']);
assertCoursesFetched({
h_userid: ['123', '456'],
assignment_id: ['1', '2'],
h_userid: [],
assignment_id: [],
course_id: ['3', '8', '9'],
});
});
Expand Down
9 changes: 3 additions & 6 deletions lms/static/scripts/frontend_apps/utils/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useConfig } from '../config';
import { APIError } from '../errors';
import { useFetch } from './fetch';
import type { FetchResult, Fetcher } from './fetch';
import { recordToSearchParams } from './url';
import { recordToQueryStringFragment } from './url';

/**
* Parameters for an API call that will refresh an expired access token.
Expand Down Expand Up @@ -111,9 +111,7 @@ export async function apiCall<Result = unknown>(
headers['Content-Type'] = 'application/json; charset=UTF-8';
}

const queryString = recordToSearchParams(params ?? {}).toString();
const query = queryString.length > 0 ? `?${queryString}` : '';

const query = recordToQueryStringFragment(params ?? {});
const defaultMethod = data === undefined ? 'GET' : 'POST';
const result = await fetch(path + query, {
method: method ?? defaultMethod,
Expand Down Expand Up @@ -208,7 +206,6 @@ export function useAPIFetch<T = unknown>(
// something simpler, as long as it encodes the same information. The auth
// token is not included in the key, as we assume currently that it does not
// change the result.
const queryString = recordToSearchParams(params ?? {}).toString();
const paramStr = queryString.length > 0 ? `?${queryString}` : '';
const paramStr = recordToQueryStringFragment(params ?? {});
return useFetch(path ? `${path}${paramStr}` : null, fetcher);
}
54 changes: 54 additions & 0 deletions lms/static/scripts/frontend_apps/utils/dashboard/hooks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { useCallback, useMemo } from 'preact/hooks';
import { useLocation, useSearch } from 'wouter-preact';

import { queryStringToRecord, recordToQueryStringFragment } from '../url';

export type DashboardFilters = {
studentIds: string[];
assignmentIds: string[];
courseIds: string[];
};

export type UseDashboardQueryFilters = {
filters: DashboardFilters;
updateFilters: (filtersToUpdate: Partial<DashboardFilters>) => void;
};

/**
* Allows to track dashboard filters as a piece of state synced in the query string
*/
export function useDashboardQueryFilters(): UseDashboardQueryFilters {
const search = useSearch();
const queryParams = useMemo(() => queryStringToRecord(search), [search]);
const filters = useMemo(() => {
const { student_id = [], course_id = [], assignment_id = [] } = queryParams;
const courseIds = Array.isArray(course_id) ? course_id : [course_id];
const assignmentIds = Array.isArray(assignment_id)
? assignment_id
: [assignment_id];
const studentIds = Array.isArray(student_id) ? student_id : [student_id];

return { courseIds, assignmentIds, studentIds };
}, [queryParams]);

const [, navigate] = useLocation();
const updateFilters = useCallback(
({ courseIds, assignmentIds, studentIds }: Partial<DashboardFilters>) => {
const newQueryParams = { ...queryParams };
if (courseIds) {
newQueryParams.course_id = courseIds;
}
if (assignmentIds) {
newQueryParams.assignment_id = assignmentIds;
}
if (studentIds) {
newQueryParams.student_id = studentIds;
}

navigate(recordToQueryStringFragment(newQueryParams), { replace: true });
},
[navigate, queryParams],
);

return { filters, updateFilters };
}
Loading

0 comments on commit 2e52aca

Please sign in to comment.