fix: implement fixes based on comments

Author: LWS49

.circleci/config.yml

@@ -119,6 +119,7 @@ commands:
           command: yarn build:test
           environment:
             AVAILABLE_CPUS: 4
+            NODE_OPTIONS: --max-old-space-size=4096
 
       - save_cache:
           paths:

app/controllers/components/course/gradebook_component.rb

@@ -13,7 +13,6 @@ def sidebar_items
       {
         key: self.class.key,
         icon: :gradebook,
-        title: I18n.t('course.gradebook.component.sidebar_title'),
         type: :normal,
         weight: 9,
         path: course_gradebook_path(current_course)

app/controllers/course/gradebook_controller.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 class Course::GradebookController < Course::ComponentController
   before_action :authorize_read_gradebook!
+  before_action :preload_levels, only: [:index]
 
   def index
     respond_to do |format|
@@ -34,9 +35,8 @@ def fetch_categories_and_tabs
   end
 
   def fetch_students
-    current_course.levels.to_a
     current_course.course_users.students.without_phantom_users.
-      calculated(:experience_points).includes(:user).to_a
+      calculated(:experience_points).includes(user: :emails).to_a
   end
 
   def fetch_published_assessments
@@ -46,4 +46,11 @@ def fetch_published_assessments
       joins(tab: :category).
       reorder('course_assessment_categories.weight, course_assessment_tabs.weight, course_assessments.id')
   end
+
+  # Pre-loads course levels to avoid N+1 queries when each course_user.level_number is rendered.
+  # level_number is derived, not an association (it buckets EXP against course.levels), so it
+  # can't be added to fetch_students' includes. See Course::LevelsConcern#level_for.
+  def preload_levels
+    current_course.levels.to_a
+  end
 end

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

@@ -701,7 +701,9 @@ describe('GradebookTable', () => {
         const user = userEvent.setup();
         renderGrades(mixedStudents, mixedSubmissions);
         await screen.findByText('Alice');
-        await user.click(screen.getByRole('button', { name: /quiz 1/i }));
+        // Grade columns sort desc-first, so click twice to reach ascending.
+        await user.click(screen.getByRole('button', { name: /quiz 1/i })); // desc
+        await user.click(screen.getByRole('button', { name: /quiz 1/i })); // asc
         // Ascending: Bob(3), Alice(8), then the two missing rows last.
         await waitFor(() => expectInOrder(['Bob', 'Alice']));
         expectInOrder(['Alice', 'Carol']);
@@ -712,7 +714,7 @@ describe('GradebookTable', () => {
         const user = userEvent.setup();
         renderGrades(mixedStudents, mixedSubmissions);
         await screen.findByText('Alice');
-        await user.click(screen.getByRole('button', { name: /quiz 1/i })); // asc
+        // Grade columns sort desc-first, so a single click yields descending.
         await user.click(screen.getByRole('button', { name: /quiz 1/i })); // desc
         // Descending: Alice(8), Bob(3), then the two missing rows still last.
         await waitFor(() => expectInOrder(['Alice', 'Bob']));
@@ -730,7 +732,9 @@ describe('GradebookTable', () => {
           ],
         );
         await screen.findByText('Alice');
-        await user.click(screen.getByRole('button', { name: /quiz 1/i }));
+        // Grade columns sort desc-first, so click twice to reach ascending.
+        await user.click(screen.getByRole('button', { name: /quiz 1/i })); // desc
+        await user.click(screen.getByRole('button', { name: /quiz 1/i })); // asc
         // Numeric ascending: 9 (Alice) before 10 (Bob). Lexical would reverse this.
         await waitFor(() => expectInOrder(['Alice', 'Bob']));
       });

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

@@ -212,26 +212,26 @@ const GradebookTable = ({
   const { t } = useTranslation();
 
   const submissionsByStudent = useMemo(() => {
-    const map = new Map<number, SubmissionData[]>();
+    const map = new Map<number, Map<number, SubmissionData>>();
     submissions.forEach((s) => {
-      const existing = map.get(s.studentId);
-      if (existing) {
-        existing.push(s);
-      } else {
-        map.set(s.studentId, [s]);
+      let byAssessment = map.get(s.studentId);
+      if (!byAssessment) {
+        byAssessment = new Map<number, SubmissionData>();
+        map.set(s.studentId, byAssessment);
       }
+      byAssessment.set(s.assessmentId, s);
     });
     return map;
   }, [submissions]);
 
   const rows = useMemo<GradebookRow[]>(
     () =>
       students.map((student) => {
-        const subs = submissionsByStudent.get(student.id) ?? [];
+        const subs = submissionsByStudent.get(student.id);
         const grades: Partial<Record<number, number | null>> = {};
         const submissionIds: Partial<Record<number, number>> = {};
         assessments.forEach((a) => {
-          const sub = subs.find((s) => s.assessmentId === a.id);
+          const sub = subs?.get(a.id);
           if (sub != null) {
             grades[a.id] = sub.grade;
             submissionIds[a.id] = sub.submissionId;
@@ -321,7 +321,6 @@ const GradebookTable = ({
         sortable: true,
         sortProps: {
           undefinedPriority: 'last',
-          descFirst: false,
           sort: (a, b) => {
             const aGrade = a.grades[asn.id];
             const bGrade = b.grades[asn.id];

client/app/bundles/course/statistics/pages/StatisticsIndex/index.tsx

@@ -115,10 +115,7 @@ const StatisticsIndex: FC = () => {
 
         return (
           <Page title={t(translations.statistics)} unpadded>
-            <Box
-              className="max-w-full"
-              sx={{ borderBottom: 1, borderColor: 'divider' }}
-            >
+            <Box className="max-w-full border-b border-divider">
               <Tabs
                 aria-label="Statistics Index Tabs"
                 onChange={(_, value) => {

client/app/lib/components/table/TanStackTableBuilder/columnsBuilder.ts

@@ -61,9 +61,6 @@ const buildTanStackColumns = <D extends Data>(
               column.sortProps!.sort!(rowA.original, rowB.original)
           : 'alphanumeric',
         sortUndefined: column.sortProps?.undefinedPriority ?? false,
-        ...(column.sortProps?.descFirst !== undefined && {
-          sortDescFirst: column.sortProps.descFirst,
-        }),
         filterFn:
           column.filterProps?.shouldInclude &&
           Object.assign(

client/app/lib/components/table/__tests__/useTanStackTableBuilder.test.tsx

@@ -1049,17 +1049,12 @@ describe('useTanStackTableBuilder sort reconciliation', () => {
               cell: (r) => r.label,
               sortable: true,
             },
-            // descFirst:false so the first click sorts score ascending → Bob(1),
-            // Alice(9). TanStack otherwise derives desc-first from the numeric
-            // first value, yielding Alice,Bob — which is identical to the
-            // label-asc reset order and would silently defeat this test.
             {
               id: 'score',
               of: 'score',
               title: 'Score',
               cell: (r) => r.score,
               sortable: true,
-              sortProps: { descFirst: false },
             },
           ]
         : [
@@ -1086,13 +1081,20 @@ describe('useTanStackTableBuilder sort reconciliation', () => {
     // Initial sort is label asc → Alice, Bob.
     expect(names(result)).toEqual(['Alice', 'Bob']);
 
-    // User sorts by the score column (asc) → Bob (1), Alice (9).
-    act(() => {
-      const scoreHeader = result.current.header!.headers[1];
-      result.current.header!.forEach(scoreHeader, 1).sorting!.onClickSort!(
-        {} as never,
-      );
-    });
+    // Score is numeric, so TanStack sorts it desc-first. The first click gives
+    // desc (Alice 9, Bob 1) — identical to the label-asc reset order — so click
+    // again to reach asc (Bob 1, Alice 9), an order distinct from the reset that
+    // proves the score column is the active sort before it is removed.
+    const clickScoreSort = (): void => {
+      act(() => {
+        const scoreHeader = result.current.header!.headers[1];
+        result.current.header!.forEach(scoreHeader, 1).sorting!.onClickSort!(
+          {} as never,
+        );
+      });
+    };
+    clickScoreSort(); // desc → Alice, Bob
+    clickScoreSort(); // asc → Bob, Alice
     expect(names(result)).toEqual(['Bob', 'Alice']);
 
     // Score column disappears (gamification turned off) → sort resets to label asc.

client/app/lib/components/table/__tests__/utils.test.ts

@@ -9,18 +9,6 @@ const mockDownloadFile = downloadFile as jest.Mock;
 describe('downloadCsv', () => {
   beforeEach(() => mockDownloadFile.mockClear());
 
-  it('prepends a UTF-8 BOM so Excel detects UTF-8 encoding', () => {
-    downloadCsv('a,b\n1,2');
-    const content: string = mockDownloadFile.mock.calls[0][1];
-    expect(content.charCodeAt(0)).toBe(0xfeff);
-  });
-
-  it('preserves em dash characters after the BOM', () => {
-    downloadCsv('Name,Score\nAlice — Test,10');
-    const content: string = mockDownloadFile.mock.calls[0][1];
-    expect(content).toContain('—');
-  });
-
   it('uses the provided filename with .csv extension', () => {
     downloadCsv('a,b', 'my-file');
     expect(mockDownloadFile.mock.calls[0][2]).toBe('my-file.csv');

client/app/lib/components/table/builder/ColumnTemplate.ts

@@ -17,7 +17,6 @@ interface SearchingProps<D> {
 interface SortingProps<D> {
   sort?: (datumA: D, datumB: D) => number;
   undefinedPriority?: false | 'first' | 'last';
-  descFirst?: boolean;
 }
 
 interface ColumnTemplate<D extends Data> {