From 1c6e0c7a9266065eff8f515108d6ccc54bed2874 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 7 Oct 2024 12:46:19 +0200 Subject: [PATCH] Display synced students count in auto-grading assignments (#6755) --- lms/static/scripts/frontend_apps/api-types.ts | 1 + .../components/dashboard/SyncGradesButton.tsx | 57 +++++++++++++++---- .../dashboard/test/SyncGradesButton-test.js | 33 ++++++++--- 3 files changed, 72 insertions(+), 19 deletions(-) diff --git a/lms/static/scripts/frontend_apps/api-types.ts b/lms/static/scripts/frontend_apps/api-types.ts index 56651228df..371218260c 100644 --- a/lms/static/scripts/frontend_apps/api-types.ts +++ b/lms/static/scripts/frontend_apps/api-types.ts @@ -312,6 +312,7 @@ export type StudentGradingSyncStatus = 'in_progress' | 'finished' | 'failed'; export type StudentGradingSync = { h_userid: string; status: StudentGradingSyncStatus; + grade: number; }; export type GradingSyncStatus = 'scheduled' | StudentGradingSyncStatus; diff --git a/lms/static/scripts/frontend_apps/components/dashboard/SyncGradesButton.tsx b/lms/static/scripts/frontend_apps/components/dashboard/SyncGradesButton.tsx index 0393f2d902..30813f01a2 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/SyncGradesButton.tsx +++ b/lms/static/scripts/frontend_apps/components/dashboard/SyncGradesButton.tsx @@ -1,9 +1,6 @@ -import { - Button, - LeaveIcon, - SpinnerCircleIcon, -} from '@hypothesis/frontend-shared'; -import { useCallback, useMemo } from 'preact/hooks'; +import { Button, LeaveIcon } from '@hypothesis/frontend-shared'; +import classnames from 'classnames'; +import { useCallback, useMemo, useState } from 'preact/hooks'; import { useParams } from 'wouter-preact'; import type { GradingSync, GradingSyncStatus } from '../../api-types'; @@ -58,6 +55,20 @@ export default function SyncGradesButton({ [lastSync], ); + const syncedStudentsCount = useMemo( + () => + lastSync.data?.grades.filter(s => s.status !== 'in_progress').length ?? 0, + [lastSync.data?.grades], + ); + const [totalStudentsToSync, setTotalStudentsToSync] = useState(); + const startSync = useCallback(() => { + // Right before starting, set amount of students that are going to be synced. + // This will prevent displaying a zero in the short interval between the + // list of students to sync is cleared, and the next sync check happens + setTotalStudentsToSync(studentsToSync?.length ?? 0); + updateSyncStatus('scheduled'); + }, [studentsToSync?.length, updateSyncStatus]); + const buttonContent = useMemo(() => { if (!studentsToSync || (lastSync.isLoading && !lastSync.data)) { return 'Loading...'; @@ -67,10 +78,26 @@ export default function SyncGradesButton({ lastSync.data && ['scheduled', 'in_progress'].includes(lastSync.data.status) ) { + // Use the amount set when a sync is started, but fall back to the amount + // of students from last sync, in case a sync was in progress when this + // was loaded + const totalStudentsBeingSynced = + totalStudentsToSync ?? lastSync.data.grades.length; + return ( <> Syncing grades - +
+
+ {syncedStudentsCount}/{totalStudentsBeingSynced} +
); } @@ -103,7 +130,14 @@ export default function SyncGradesButton({ } return 'Grades synced'; - }, [studentsToSync, lastSync.isLoading, lastSync.data, lastSync.error]); + }, [ + studentsToSync, + lastSync.isLoading, + lastSync.data, + lastSync.error, + totalStudentsToSync, + syncedStudentsCount, + ]); const buttonDisabled = lastSync.isLoading || @@ -112,10 +146,10 @@ export default function SyncGradesButton({ !studentsToSync || studentsToSync.length === 0; - const syncGrades = useCallback(async () => { - updateSyncStatus('scheduled'); + const syncGrades = useCallback(() => { + startSync(); - apiCall({ + return apiCall({ authToken: api.authToken, path: syncURL, method: 'POST', @@ -128,6 +162,7 @@ export default function SyncGradesButton({ }, [ api.authToken, onSyncScheduled, + startSync, studentsToSync, syncURL, updateSyncStatus, diff --git a/lms/static/scripts/frontend_apps/components/dashboard/test/SyncGradesButton-test.js b/lms/static/scripts/frontend_apps/components/dashboard/test/SyncGradesButton-test.js index 1df3700c25..f1e696979c 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/test/SyncGradesButton-test.js +++ b/lms/static/scripts/frontend_apps/components/dashboard/test/SyncGradesButton-test.js @@ -83,14 +83,31 @@ describe('SyncGradesButton', () => { }); }); - ['scheduled', 'in_progress'].forEach(status => { + [ + { + status: 'scheduled', + grades: [], + expectedCount: '0/0', + }, + { + status: 'in_progress', + grades: [ + { status: 'in_progress' }, + { status: 'in_progress' }, + { status: 'finished' }, + { status: 'in_progress' }, + { status: 'failed' }, + ], + expectedCount: '2/5', + }, + ].forEach(({ status, grades, expectedCount }) => { it('shows syncing text when grades are being synced', () => { const wrapper = createComponent(studentsToSync, { isLoading: false, - data: { status }, + data: { status, grades }, }); - assert.equal(buttonText(wrapper), 'Syncing grades'); + assert.equal(buttonText(wrapper), `Syncing grades${expectedCount}`); assert.isTrue(isButtonDisabled(wrapper)); }); }); @@ -98,7 +115,7 @@ describe('SyncGradesButton', () => { it('shows syncing errors and allows to retry', () => { const wrapper = createComponent(studentsToSync, { isLoading: false, - data: { status: 'failed' }, + data: { status: 'failed', grades: [] }, }); assert.equal(buttonText(wrapper), 'Error syncing. Click to retry'); @@ -125,7 +142,7 @@ describe('SyncGradesButton', () => { it('shows the amount of students to be synced when current status is "finished"', () => { const wrapper = createComponent(students, { isLoading: false, - data: { status: 'finished' }, + data: { status: 'finished', grades: [] }, }); assert.equal(buttonText(wrapper), `Sync ${expectedAmount} grades`); @@ -146,7 +163,7 @@ describe('SyncGradesButton', () => { it('shows grades synced when no students need to be synced', () => { const wrapper = createComponent([], { isLoading: false, - data: { status: 'finished' }, + data: { status: 'finished', grades: [] }, }); assert.equal(buttonText(wrapper), 'Grades synced'); @@ -156,7 +173,7 @@ describe('SyncGradesButton', () => { it('submits grades when the button is clicked, then calls onSyncScheduled', async () => { const wrapper = createComponent(studentsToSync, { isLoading: false, - data: { status: 'finished' }, + data: { status: 'finished', grades: [] }, mutate: sinon.stub(), }); await act(() => wrapper.find('Button').props().onClick()); @@ -180,7 +197,7 @@ describe('SyncGradesButton', () => { const mutate = sinon.stub(); const wrapper = createComponent(studentsToSync, { isLoading: false, - data: { status: 'finished' }, + data: { status: 'finished', grades: [] }, mutate, }); await act(() => wrapper.find('Button').props().onClick());