Skip to content

Commit

Permalink
Make DashboardActivityFilters take query string into consideration
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Jul 24, 2024
1 parent 028562a commit b2bb84c
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { MultiSelect } from '@hypothesis/frontend-shared';
import { useMemo } from 'preact/hooks';
import { useCallback, useEffect, useMemo } from 'preact/hooks';
import { useLocation, useSearch } from 'wouter-preact';

import type { Assignment, Course, Student } from '../../api-types';
import { useConfig } from '../../config';
import { useAPIFetch } from '../../utils/api';
import {
queryStringToRecord,
recordToQueryStringFragment,
} from '../../utils/url';

export type DashboardActivityFiltersProps = {
selectedCourses: Course[];
Expand All @@ -26,9 +31,23 @@ export default function DashboardActivityFilters({
selectedStudents,
onStudentsChange,
}: DashboardActivityFiltersProps) {
const search = useSearch();
const queryParams = useMemo(() => queryStringToRecord(search), [search]);
const [, navigate] = useLocation();
const { dashboard } = useConfig(['dashboard']);
const { routes } = dashboard;

/**
* Used to persist selected filters in the query string
*/
const updateQueryString = useCallback(
(paramsToUpdate: Record<string, string[]>) => {
const newQueryParams = { ...queryParams, ...paramsToUpdate };
navigate(recordToQueryStringFragment(newQueryParams));
},
[navigate, queryParams],
);

const courses = useAPIFetch<{ courses: Course[] }>(routes.courses);
const assignments = useAPIFetch<{ assignments: Assignment[] }>(
routes.assignments,
Expand All @@ -39,12 +58,65 @@ export default function DashboardActivityFilters({
[students.data?.students],
);

// Try to initialize selection based on query parameters
useEffect(() => {
const { course_id = [] } = queryParams;
const courseIds = Array.isArray(course_id) ? course_id : [course_id];
if (selectedCourses.length > 0 || courseIds.length === 0) {
return;
}

if (courseIds.length > 0 && courses.data) {
onCoursesChange(
courses.data.courses.filter(c => courseIds.includes(`${c.id}`)),
);
}
}, [courses.data, onCoursesChange, queryParams, selectedCourses.length]);
useEffect(() => {
const { assignment_id = [] } = queryParams;
const assignmentIds = Array.isArray(assignment_id)
? assignment_id
: [assignment_id];
if (selectedAssignments.length > 0 || assignmentIds.length === 0) {
return;
}

if (assignmentIds.length > 0 && assignments.data) {
onAssignmentsChange(
assignments.data.assignments.filter(a =>
assignmentIds.includes(`${a.id}`),
),
);
}
}, [
assignments.data,
onAssignmentsChange,
queryParams,
selectedAssignments.length,
]);
useEffect(() => {
const { h_userid = [] } = queryParams;
const studentIds = Array.isArray(h_userid) ? h_userid : [h_userid];
if (selectedStudents.length > 0 || studentIds.length === 0) {
return;
}

if (studentIds.length > 0 && students.data) {
onStudentsChange(
students.data.students.filter(s => studentIds.includes(s.h_userid)),
);
}
}, [onStudentsChange, queryParams, selectedStudents.length, students.data]);

return (
<div className="flex gap-2 md:w-1/2">
<MultiSelect
disabled={courses.isLoading}
value={selectedCourses}
onChange={onCoursesChange}
onChange={newCourses => {
updateQueryString({ course_id: newCourses.map(c => `${c.id}`) });
onCoursesChange(newCourses);
}}
aria-label="Select courses"
buttonContent={
courses.isLoading ? (
Expand All @@ -69,7 +141,12 @@ export default function DashboardActivityFilters({
<MultiSelect
disabled={assignments.isLoading}
value={selectedAssignments}
onChange={onAssignmentsChange}
onChange={newAssignments => {
updateQueryString({
assignment_id: newAssignments.map(a => `${a.id}`),
});
onAssignmentsChange(newAssignments);
}}
aria-label="Select assignments"
buttonContent={
assignments.isLoading ? (
Expand All @@ -96,7 +173,10 @@ export default function DashboardActivityFilters({
<MultiSelect
disabled={students.isLoading}
value={selectedStudents}
onChange={onStudentsChange}
onChange={newStudents => {
updateQueryString({ h_userid: newStudents.map(s => s.h_userid) });
onStudentsChange(newStudents);
}}
aria-label="Select students"
buttonContent={
students.isLoading ? (
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);
}
57 changes: 56 additions & 1 deletion lms/static/scripts/frontend_apps/utils/test/url-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { recordToSearchParams, replaceURLParams } from '../url';
import {
queryStringToRecord,
recordToQueryStringFragment,
recordToSearchParams,
replaceURLParams,
} from '../url';

describe('replaceURLParams', () => {
it('should replace params in URLs', () => {
Expand Down Expand Up @@ -47,3 +52,53 @@ describe('recordToSearchParams', () => {
assert.equal(result.toString(), 'foo=bar&baz=1&baz=2&baz=3');
});
});

describe('recordToQueryStringFragment', () => {
[
{
params: {
foo: 'bar',
baz: ['1', '2', '3'],
},
expectedResult: '?foo=bar&baz=1&baz=2&baz=3',
},
{
params: {},
expectedResult: '',
},
{
params: { foo: [], bar: [] },
expectedResult: '',
},
].forEach(({ params, expectedResult }) => {
it('parses provided record and appends entries', () => {
const result = recordToQueryStringFragment(params);
assert.equal(result, expectedResult);
});
});
});

describe('queryStringToRecord', () => {
[
{
queryString: '?foo=bar&baz=1&baz=2&baz=3',
expectedResult: {
foo: 'bar',
baz: ['1', '2', '3'],
},
},
{
queryString: '',
expectedResult: {},
},
{
queryString: '?',
expectedResult: {},
},
].forEach(({ queryString, expectedResult }) => {
it('parses provided record and appends entries', () => {
const result = queryStringToRecord(queryString);
assert.deepEqual(result, expectedResult);
});
});
});
40 changes: 40 additions & 0 deletions lms/static/scripts/frontend_apps/utils/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,43 @@ export function recordToSearchParams(

return queryParams;
}

/**
* Converts a record into a query string fragment.
* The result is prefixed with a question mark (`?`) if it's not empty.
*
* Examples:
* * {} -> ''
* * { foo: [] } -> ''
* * { foo: 'bar' } -> '?foo=bar'
* * { foo: 'bar', something: ['hello', 'world'] } -> '?foo=bar&something=hello&something=world'
*/
export function recordToQueryStringFragment(
params: Record<string, string | string[]>,
): string {
const queryString = recordToSearchParams(params).toString();
return queryString.length > 0 ? `?${queryString}` : '';
}

/**
* Converts provided query string into a record object.
* Parameters that appear more than once will be converted to an array.
*/
export function queryStringToRecord(
queryString: string,
): Record<string, string | string[]> {
const queryParams = new URLSearchParams(queryString);
const params: Record<string, string | string[]> = {};

queryParams.forEach((value, name) => {
if (!params[name]) {
params[name] = value;
} else if (Array.isArray(params[name])) {
(params[name] as string[]).push(value);
} else {
params[name] = [params[name] as string, value];
}
});

return params;
}

0 comments on commit b2bb84c

Please sign in to comment.