Skip to content

Commit

Permalink
Add button to sync grades
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Sep 13, 2024
1 parent 2e82ba5 commit dd90862
Show file tree
Hide file tree
Showing 6 changed files with 244 additions and 44 deletions.
7 changes: 7 additions & 0 deletions lms/static/scripts/frontend_apps/api-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,3 +278,10 @@ export type StudentsResponse = {
students: Student[];
pagination: Pagination;
};

/**
* Response for `/api/dashboard/assignments/{assignment_id}/grading/sync`
*/
export type GradingSync = {
status: 'scheduled' | 'in_progress' | 'finished' | 'failed';
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import FormattedDate from './FormattedDate';
import GradeIndicator from './GradeIndicator';
import type { OrderableActivityTableColumn } from './OrderableActivityTable';
import OrderableActivityTable from './OrderableActivityTable';
import SyncGradesButton from './SyncGradesButton';

type StudentsTableRow = {
lms_id: string;
Expand All @@ -33,7 +34,7 @@ type StudentsTableRow = {
*/
export default function AssignmentActivity() {
const { dashboard } = useConfig(['dashboard']);
const { routes } = dashboard;
const { routes, user } = dashboard;
const { assignmentId, organizationPublicId } = useParams<{
assignmentId: string;
organizationPublicId?: string;
Expand All @@ -58,13 +59,24 @@ export default function AssignmentActivity() {
},
);

const studentsToSync = useMemo(() => {
if (!autoGradingEnabled) {
return [];
}

// TODO Filter out students whose grades didn't change
return (students.data?.students ?? []).map(
({ h_userid, auto_grading_grade = 0 }) => ({
h_userid,
grade: auto_grading_grade,
}),
);
}, [autoGradingEnabled, students.data?.students]);
const rows: StudentsTableRow[] = useMemo(
() =>
(students.data?.students ?? []).map(
({ lms_id, display_name, auto_grading_grade, annotation_metrics }) => ({
lms_id,
display_name,
auto_grading_grade,
({ annotation_metrics, ...rest }) => ({
...rest,
...annotation_metrics,
}),
),
Expand Down Expand Up @@ -133,40 +145,45 @@ export default function AssignmentActivity() {
{assignment.data && title}
</h2>
</div>
{assignment.data && (
<DashboardActivityFilters
courses={{
activeItem: assignment.data.course,
// When the active course is cleared, navigate to home, but keep
// active assignment and students
onClear: () =>
navigate(
urlWithFilters(
{ studentIds, assignmentIds: [assignmentId] },
{ path: '' },
<div className="flex justify-between items-end gap-x-4">
{assignment.data && (
<DashboardActivityFilters
courses={{
activeItem: assignment.data.course,
// When the active course is cleared, navigate to home, but keep
// active assignment and students
onClear: () =>
navigate(
urlWithFilters(
{ studentIds, assignmentIds: [assignmentId] },
{ path: '' },
),
),
),
}}
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
}
/>
)}
}}
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
}
/>
)}
{autoGradingEnabled && !user.is_staff && (
<SyncGradesButton studentsToSync={studentsToSync} />
)}
</div>
<OrderableActivityTable
loading={students.isLoading}
title={assignment.isLoading ? 'Loading...' : title}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import { Button, SpinnerCircleIcon } from '@hypothesis/frontend-shared';
import { useCallback, useMemo, useState } from 'preact/hooks';
import { useParams } from 'wouter-preact';

import type { GradingSync } from '../../api-types';
import { useConfig } from '../../config';
import { apiCall, usePolledAPIFetch } from '../../utils/api';
import { replaceURLParams } from '../../utils/url';

export type SyncGradesButtonProps = {
/**
* List of students and their grades, which should be synced when the button
* is clicked.
*/
studentsToSync: Array<{ h_userid: string; grade: number }>;
};

export default function SyncGradesButton({
studentsToSync,
}: SyncGradesButtonProps) {
const { assignmentId } = useParams<{ assignmentId: string }>();
const { dashboard, api } = useConfig(['dashboard', 'api']);
const { routes } = dashboard;
const [syncing, setSyncing] = useState(false);

const syncUrl = useMemo(
() =>
replaceURLParams(routes.assignment_grades_sync ?? ':assignment_id', {
assignment_id: assignmentId,
}),
[assignmentId, routes.assignment_grades_sync],
);
const lastSync = usePolledAPIFetch<GradingSync>({
path: syncUrl,
// Keep polling as long as sync is in progress
shouldRetry: result =>
!!result.data &&
['scheduled', 'in_progress'].includes(result.data.status),
});

const buttonContent = useMemo(() => {
if (lastSync.isLoading) {
return 'Loading...';
}

if (
syncing ||
(lastSync.data &&
['scheduled', 'in_progress'].includes(lastSync.data.status))
) {
return (
<>
Syncing grades
<SpinnerCircleIcon className="ml-1.5" />
</>
);
}

if (lastSync.data?.status === 'failed') {
return 'Error syncing';
}

if (studentsToSync.length > 0) {
return `Sync ${studentsToSync.length} students`;
}

return 'Grades synced';
}, [lastSync.isLoading, lastSync.data, syncing, studentsToSync.length]);

const buttonDisabled = useMemo(
() => syncing || lastSync.isLoading || studentsToSync.length === 0,
[syncing, lastSync.isLoading, studentsToSync.length],
);

const syncGrades = useCallback(() => {
setSyncing(true);

apiCall({
authToken: api.authToken,
path: syncUrl,
method: 'POST',
data: {
grades: studentsToSync,
},
}).finally(() => setSyncing(false));
}, [api.authToken, studentsToSync, syncUrl]);

return (
<Button variant="primary" onClick={syncGrades} disabled={buttonDisabled}>
{buttonContent}
</Button>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ describe('AssignmentActivity', () => {
assignment: '/api/assignments/:assignment_id',
students_metrics: '/api/students/metrics',
},
user: {
is_staff: true,
},
},
};

Expand Down Expand Up @@ -390,6 +393,25 @@ describe('AssignmentActivity', () => {
});
},
);

[
{ isStaff: true, autoGradingEnabled: true, shouldShowButton: false },
{ isStaff: false, autoGradingEnabled: true, shouldShowButton: true },
{ isStaff: true, autoGradingEnabled: false, shouldShowButton: false },
{ isStaff: false, autoGradingEnabled: false, shouldShowButton: false },
].forEach(({ autoGradingEnabled, isStaff, shouldShowButton }) => {
it('shows sync button for non-staff users only', () => {
setUpFakeUseAPIFetch({
...activeAssignment,
auto_grading_config: autoGradingEnabled ? {} : null,
});
fakeConfig.dashboard.user.is_staff = isStaff;

const wrapper = createComponent();

assert.equal(wrapper.exists('SyncGradesButton'), shouldShowButton);
});
});
});

it(
Expand Down
3 changes: 3 additions & 0 deletions lms/static/scripts/frontend_apps/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,9 @@ export type DashboardRoutes = {
assignments: string;
/** Fetch list of students */
students: string;

/** Sync grades (POST) or check sync status (GET) */
assignment_grades_sync: string;
};

export type DashboardUser = {
Expand Down
70 changes: 64 additions & 6 deletions lms/static/scripts/frontend_apps/utils/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,69 @@ export function urlPath(strings: TemplateStringsArray, ...params: string[]) {
export function useAPIFetch<T = unknown>(
path: string | null,
params?: QueryParams,
): FetchResult<T> {
// We generate a URL-like key from the path and params, but we could generate
// 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 paramStr = recordToQueryString(params ?? {});
return useAPIFetchWithKey(path ? `${path}${paramStr}` : null, path, params);
}

type PolledAPIFetchOptions<T> = {
/** Path for API call */
path: string;
/** Query params for API call */
params?: QueryParams;
/** Determines if, based on the result, the request should be retried */
shouldRetry: (result: FetchResult<T>) => boolean;

/**
* Amount of ms after which a retry should happen, if shouldRetry() returns
* true.
* Defaults to 500ms.
*/
retryAfter?: number;
};

/**
* Hook that fetches data using authenticated API requests, allowing it to be
* retried periodically.
*/
export function usePolledAPIFetch<T = unknown>({
path,
params,
shouldRetry,
retryAfter = 500,
}: PolledAPIFetchOptions<T>): FetchResult<T> {
const [key, setKey] = useState(`${path}${recordToQueryString(params ?? {})}`);
const result = useAPIFetchWithKey<T>(key, path, params);

// Once we finish loading, check if request should be retried, and schedule a
// key change to enforce it
const timeout = useRef<ReturnType<typeof setTimeout> | null>(null);
if (!result.isLoading && shouldRetry(result)) {
timeout.current = setTimeout(
() => setKey(prev => `${prev}${Date.now()}`),
retryAfter,
);
}

useEffect(() => {
return () => {
if (timeout.current) {
clearTimeout(timeout.current);
}
};
}, []);

return result;
}

function useAPIFetchWithKey<T>(
key: string | null,
path: string | null,
params?: QueryParams,
): FetchResult<T> {
const {
api: { authToken },
Expand All @@ -206,12 +269,7 @@ export function useAPIFetch<T = unknown>(
})
: undefined;

// We generate a URL-like key from the path and params, but we could generate
// 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 paramStr = recordToQueryString(params ?? {});
return useFetch(path ? `${path}${paramStr}` : null, fetcher);
return useFetch(key, fetcher);
}

export type PaginatedFetchResult<T> = Omit<FetchResult<T>, 'mutate'> & {
Expand Down

0 comments on commit dd90862

Please sign in to comment.