Include students without name in students dropdown (#6521)

hypothesis · Aug 6, 2024 · b471df8 · b471df8
1 parent c05a73c
commit b471df8
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 9 deletions.
diff --git a/lms/static/scripts/frontend_apps/components/dashboard/DashboardActivityFilters.tsx b/lms/static/scripts/frontend_apps/components/dashboard/DashboardActivityFilters.tsx
@@ -8,6 +8,7 @@ import { useMemo } from 'preact/hooks';
 import type {
   AssignmentsResponse,
   CoursesResponse,
+  Student,
   StudentsResponse,
 } from '../../api-types';
 import { useConfig } from '../../config';
@@ -23,6 +24,8 @@ export type DashboardActivityFiltersProps = {
   onClearSelection?: () => void;
 };
 
+type FiltersStudent = Student & { has_display_name: boolean };
+
 /**
  * Renders drop-downs to select courses, assignments and/or students, used to
  * filter dashboard activity metrics.
@@ -58,8 +61,15 @@ export default function DashboardActivityFilters({
     course_id: selectedCourseIds,
     public_id: dashboard.organization_public_id,
   });
-  const studentsWithName = useMemo(
-    () => students.data?.students.filter(s => !!s.display_name),
+  const studentsWithFallbackName: FiltersStudent[] | undefined = useMemo(
+    () =>
+      students.data?.students.map(({ display_name, ...s }) => ({
+        ...s,
+        display_name:
+          display_name ??
+          `Student name unavailable (ID: ${s.lms_id.substring(0, 5)})`,
+        has_display_name: !!display_name,
+      })),
     [students.data?.students],
   );
 
@@ -134,7 +144,7 @@ export default function DashboardActivityFilters({
           ) : selectedStudentIds.length === 0 ? (
             <>All students</>
           ) : selectedStudentIds.length === 1 ? (
-            students.data?.students.find(
+            studentsWithFallbackName?.find(
               s => s.h_userid === selectedStudentIds[0],
             )?.display_name
           ) : (
@@ -144,9 +154,19 @@ export default function DashboardActivityFilters({
         data-testid="students-select"
       >
         <MultiSelect.Option value={undefined}>All students</MultiSelect.Option>
-        {studentsWithName?.map(student => (
+        {studentsWithFallbackName?.map(student => (
           <MultiSelect.Option key={student.lms_id} value={student.h_userid}>
-            {student.display_name}
+            <span
+              className={student.has_display_name ? undefined : 'italic'}
+              title={
+                student.has_display_name
+                  ? undefined
+                  : `User ID: ${student.lms_id}`
+              }
+              data-testid="option-content-wrapper"
+            >
+              {student.display_name}
+            </span>
           </MultiSelect.Option>
         ))}
       </MultiSelect>

diff --git a/lms/static/scripts/frontend_apps/components/dashboard/test/DashboardActivityFilters-test.js b/lms/static/scripts/frontend_apps/components/dashboard/test/DashboardActivityFilters-test.js
@@ -47,9 +47,9 @@ describe('DashboardActivityFilters', () => {
   const students = [
     ...studentsWithName,
     {
-      lms_id: '3',
+      lms_id: '123456789',
       h_userid: 'acct:[email protected]',
-      display_name: '', // Student with an empty name won't be displayed
+      display_name: null, // Student with an empty name won't be displayed
     },
   ];
 
@@ -178,9 +178,11 @@ describe('DashboardActivityFilters', () => {
       expectedOptions: [
         'All students',
         ...studentsWithName.map(s => s.display_name),
+        'Student name unavailable (ID: 12345)',
       ],
+      expectedTitles: [undefined, undefined, undefined, 'User ID: 123456789'],
     },
-  ].forEach(({ id, expectedOptions }) => {
+  ].forEach(({ id, expectedOptions, expectedTitles = [] }) => {
     it('renders corresponding options', () => {
       const wrapper = createComponent();
       const select = getSelect(wrapper, id);
@@ -189,6 +191,13 @@ describe('DashboardActivityFilters', () => {
       assert.equal(options.length, expectedOptions.length);
       options.forEach((option, index) => {
         assert.equal(option.text(), expectedOptions[index]);
+
+        if (expectedTitles[index]) {
+          assert.equal(
+            option.find('[data-testid="option-content-wrapper"]').prop('title'),
+            expectedTitles[index],
+          );
+        }
       });
     });
   });
@@ -302,7 +311,7 @@ describe('DashboardActivityFilters', () => {
       {
         props: {
           selectedAssignmentIds: [...assignments],
-          selectedStudentIds: [...studentsWithName],
+          selectedStudentIds: [...students],
         },
         shouldRenderClearButton: false,
       },