feat(gradebook): import did-you-mean, conflict change-matrix, loading cues, and copy polish

client/app/bundles/course/gradebook/__tests__/ManageExternalAssessmentsButton.test.tsx

@@ -1,16 +1,25 @@
 import { fireEvent, render, screen } from 'test-utils';
+
 import ManageExternalAssessmentsButton from '../components/manage/ManageExternalAssessmentsButton';
 
 const state = {
   gradebook: {
-    categories: [], tabs: [], students: [], submissions: [], assessments: [],
-    gamificationEnabled: false, weightedViewEnabled: false, canManageWeights: true,
+    categories: [],
+    tabs: [],
+    students: [],
+    submissions: [],
+    assessments: [],
+    gamificationEnabled: false,
+    weightedViewEnabled: false,
+    canManageWeights: true,
   },
 };
 
 it('opens the panel on click', async () => {
   render(<ManageExternalAssessmentsButton />, { state });
-  fireEvent.click(await screen.findByRole('button', { name: 'Manage external assessments' }));
+  fireEvent.click(
+    await screen.findByRole('button', { name: 'Manage external assessments' }),
+  );
   expect(await screen.findByText('External assessments')).toBeVisible();
 });
 

client/app/bundles/course/gradebook/__tests__/buildTemplate.test.ts

@@ -1,53 +1,62 @@
-import { buildTemplateCsv, identifierHeader } from '../components/import/buildTemplate';
+import {
+  buildTemplateCsv,
+  identifierHeader,
+} from '../components/import/buildTemplate';
 
 describe('identifierHeader', () => {
   it('maps mode to the concrete header', () => {
-    expect(identifierHeader('student_id')).toBe('External ID');
+    expect(identifierHeader('external_id')).toBe('External ID');
     expect(identifierHeader('email')).toBe('Email');
   });
 });
 
 describe('buildTemplateCsv', () => {
   const components = [{ name: 'Midterm', weightage: 30, maximumGrade: 50 }];
 
-  it('uses the External ID header in student_id mode', () => {
-    expect(buildTemplateCsv(components, 'student_id')).toBe('External ID,Midterm\n');
+  it('uses the External ID header in external_id mode', () => {
+    expect(buildTemplateCsv(components, 'external_id')).toBe(
+      'External ID,Midterm\n',
+    );
   });
 
   it('uses the Email header in email mode', () => {
     expect(buildTemplateCsv(components, 'email')).toBe('Email,Midterm\n');
   });
 
   it('quotes a component name containing a comma', () => {
-    const csv = buildTemplateCsv([
-      { name: 'Lab, week 1', weightage: 10, maximumGrade: 20 },
-    ], 'student_id');
+    const csv = buildTemplateCsv(
+      [{ name: 'Lab, week 1', weightage: 10, maximumGrade: 20 }],
+      'external_id',
+    );
     expect(csv.split('\n')[0]).toBe('External ID,"Lab, week 1"');
   });
 
-  it('returns "External ID\\n" for empty components array in student_id mode', () => {
-    expect(buildTemplateCsv([], 'student_id')).toBe('External ID\n');
+  it('returns "External ID\\n" for empty components array in external_id mode', () => {
+    expect(buildTemplateCsv([], 'external_id')).toBe('External ID\n');
   });
 
   it('quotes a component name containing a double-quote', () => {
-    const csv = buildTemplateCsv([
-      { name: 'My "Best" Quiz', weightage: 10, maximumGrade: 20 },
-    ], 'student_id');
+    const csv = buildTemplateCsv(
+      [{ name: 'My "Best" Quiz', weightage: 10, maximumGrade: 20 }],
+      'external_id',
+    );
     expect(csv.split('\n')[0]).toBe('External ID,"My ""Best"" Quiz"');
   });
 
   it('quotes a component name containing a newline', () => {
-    const csv = buildTemplateCsv([
-      { name: 'Lab\nWeek1', weightage: 10, maximumGrade: 20 },
-    ], 'student_id');
+    const csv = buildTemplateCsv(
+      [{ name: 'Lab\nWeek1', weightage: 10, maximumGrade: 20 }],
+      'external_id',
+    );
     // The quoted cell spans two lines; verify the full header row content.
     expect(csv.startsWith('External ID,"Lab\nWeek1"')).toBe(true);
   });
 
   it('always ends with exactly one newline', () => {
-    const csv = buildTemplateCsv([
-      { name: 'A', weightage: 0, maximumGrade: 100 },
-    ], 'student_id');
+    const csv = buildTemplateCsv(
+      [{ name: 'A', weightage: 0, maximumGrade: 100 }],
+      'external_id',
+    );
     expect(csv.endsWith('\n')).toBe(true);
     expect(csv.split('\n')).toHaveLength(2); // header line + empty string after trailing \n
   });

client/app/bundles/course/gradebook/components/GradebookTable.tsx

@@ -738,24 +738,24 @@ const GradebookTable = ({
 
   const toolbarWithLabel = toolbar
     ? {
-      ...toolbar,
-      buttons: toolbarAction
-        ? [
-          ...(toolbar.buttons ?? []),
-          cloneElement(toolbarAction, { key: 'toolbar-action' }),
-        ]
-        : toolbar.buttons,
-      ...(toolbar.columnPicker && {
-        columnPicker: {
-          ...toolbar.columnPicker,
-          directExportLabel,
-          directExportTooltip:
-            selectedCount === 0
-              ? t(translations.exportAllTooltip)
-              : undefined,
-        },
-      }),
-    }
+        ...toolbar,
+        buttons: toolbarAction
+          ? [
+              ...(toolbar.buttons ?? []),
+              cloneElement(toolbarAction, { key: 'toolbar-action' }),
+            ]
+          : toolbar.buttons,
+        ...(toolbar.columnPicker && {
+          columnPicker: {
+            ...toolbar.columnPicker,
+            directExportLabel,
+            directExportTooltip:
+              selectedCount === 0
+                ? t(translations.exportAllTooltip)
+                : undefined,
+          },
+        }),
+      }
     : toolbar;
 
   const totalWidth = useMemo(

client/app/bundles/course/gradebook/components/OutOfRangeAlert.tsx

@@ -31,7 +31,11 @@ const formatAssessmentNames = (names: string[]): string => {
   return `${names.slice(0, -1).join(', ')} and ${names[names.length - 1]}`;
 };
 
-const OutOfRangeAlert: FC<Props> = ({ gradeCount, assessmentNames, weightedViewEnabled }) => {
+const OutOfRangeAlert: FC<Props> = ({
+  gradeCount,
+  assessmentNames,
+  weightedViewEnabled,
+}) => {
   const { t } = useTranslation();
   if (gradeCount === 0) return null;
   if (weightedViewEnabled) {
@@ -43,18 +47,17 @@ const OutOfRangeAlert: FC<Props> = ({ gradeCount, assessmentNames, weightedViewE
           assessmentNames: formatAssessmentNames(assessmentNames),
         })}
       </Alert>
-    )
-  } else {
-    return (
-      <Alert severity="warning" sx={{ mx: 2, my: 1 }}>
-        {t(translations.warning, {
-          gradeCount,
-          assessmentCount: assessmentNames.length,
-          assessmentNames: formatAssessmentNames(assessmentNames),
-        })}
-      </Alert>
     );
   }
+  return (
+    <Alert severity="warning" sx={{ mx: 2, my: 1 }}>
+      {t(translations.warning, {
+        gradeCount,
+        assessmentCount: assessmentNames.length,
+        assessmentNames: formatAssessmentNames(assessmentNames),
+      })}
+    </Alert>
+  );
 };
 
 export default OutOfRangeAlert;

client/app/bundles/course/gradebook/components/import/ExternalGradeConflictPrompt.tsx

@@ -1,5 +1,6 @@
 import { FC } from 'react';
 import { defineMessages } from 'react-intl';
+import { LoadingButton } from '@mui/lab';
 import {
   Box,
   Button,
@@ -8,8 +9,9 @@ import {
   DialogContent,
   DialogContentText,
   DialogTitle,
+  Typography,
 } from '@mui/material';
-import type { ImportConflict } from 'types/course/gradebook';
+import type { ConflictRow } from 'types/course/gradebook';
 
 import useTranslation from 'lib/hooks/useTranslation';
 
@@ -23,7 +25,7 @@ const translations = defineMessages({
   body: {
     id: 'course.gradebook.ExternalGradeConflictPrompt.body',
     defaultMessage:
-      'These students already have a grade for these components. Keep their existing grades, or replace them with the values from your file? New students and blank cells are unaffected.',
+      'Some students already have grades for these components that differ from the values in your file. Replace will overwrite the existing grades with the values from your file. Keep Existing will leave the existing grades unchanged.',
   },
   goBack: {
     id: 'course.gradebook.ExternalGradeConflictPrompt.goBack',
@@ -37,49 +39,87 @@ const translations = defineMessages({
     id: 'course.gradebook.ExternalGradeConflictPrompt.replace',
     defaultMessage: 'Replace',
   },
+  changesSummary: {
+    id: 'course.gradebook.ExternalGradeConflictPrompt.changesSummary',
+    defaultMessage: '{changed} of {total} rows have changes',
+  },
 });
 
 interface Props {
   open: boolean;
-  conflicts: ImportConflict[];
+  rows: ConflictRow[];
+  componentNames: string[];
+  identifierLabel: string;
+  totalRows: number;
   disabled?: boolean;
+  keepLoading?: boolean;
+  replaceLoading?: boolean;
   onKeepExisting: () => void;
   onReplaceAll: () => void;
   onCancel: () => void;
 }
 
 const ExternalGradeConflictPrompt: FC<Props> = ({
   open,
-  conflicts,
+  rows,
+  componentNames,
+  identifierLabel,
+  totalRows,
   disabled = false,
+  keepLoading = false,
+  replaceLoading = false,
   onKeepExisting,
   onReplaceAll,
   onCancel,
 }) => {
   const { t } = useTranslation();
   return (
-    <Dialog disableEscapeKeyDown maxWidth="md" onClose={onCancel} open={open}>
+    <Dialog
+      disableEscapeKeyDown
+      maxWidth="md"
+      onClose={(_event, reason) => {
+        if (reason === 'backdropClick') return;
+        onCancel();
+      }}
+      open={open}
+    >
       <DialogTitle>{t(translations.title)}</DialogTitle>
       <DialogContent>
         <DialogContentText>{t(translations.body)}</DialogContentText>
+        <Typography sx={{ mt: 2 }} variant="subtitle2">
+          {t(translations.changesSummary, {
+            changed: rows.length,
+            total: totalRows,
+          })}
+        </Typography>
         <Box sx={{ maxHeight: 320, mt: 2, overflow: 'auto' }}>
-          <ExternalGradeConflictTable rows={conflicts} />
+          <ExternalGradeConflictTable
+            componentNames={componentNames}
+            identifierLabel={identifierLabel}
+            rows={rows}
+          />
         </Box>
       </DialogContent>
       <DialogActions>
         <Button disabled={disabled} onClick={onCancel} variant="outlined">
           {t(translations.goBack)}
         </Button>
-        <Button
+        <LoadingButton
           disabled={disabled}
+          loading={keepLoading}
           onClick={onKeepExisting}
           variant="contained"
         >
           {t(translations.keepExisting)}
-        </Button>
-        <Button disabled={disabled} onClick={onReplaceAll} variant="contained">
+        </LoadingButton>
+        <LoadingButton
+          disabled={disabled}
+          loading={replaceLoading}
+          onClick={onReplaceAll}
+          variant="contained"
+        >
           {t(translations.replace)}
-        </Button>
+        </LoadingButton>
       </DialogActions>
     </Dialog>
   );