[#13454] Refactor instructor search page to remove unnecessary API calls

src/web/app/pages-instructor/instructor-search-page/__snapshots__/instructor-search-page.component.spec.ts.snap

@@ -5,6 +5,7 @@ exports[`InstructorSearchPageComponent > should snap with a search key 1`] = `
   courseService={[Function Object]}
   instructorService={[Function Object]}
   isSearching="false"
+  privilegeCache={[Function Map]}
   searchParams={[Function Object]}
   searchService={[Function _SearchService]}
   searchString=""
@@ -61,6 +62,7 @@ exports[`InstructorSearchPageComponent > should snap with a student table 1`] =
   courseService={[Function Object]}
   instructorService={[Function Object]}
   isSearching="false"
+  privilegeCache={[Function Map]}
   searchParams={[Function Object]}
   searchService={[Function _SearchService]}
   searchString=""
@@ -520,6 +522,7 @@ exports[`InstructorSearchPageComponent > should snap with default fields 1`] = `
   courseService={[Function Object]}
   instructorService={[Function Object]}
   isSearching="false"
+  privilegeCache={[Function Map]}
   searchParams={[Function Object]}
   searchService={[Function _SearchService]}
   searchString=""

src/web/app/pages-instructor/instructor-search-page/instructor-search-page.component.spec.ts

@@ -278,12 +278,23 @@ describe('InstructorSearchPageComponent', () => {
     ).toEqual(students.filter((s: Student) => s.sectionName === students[0].sectionName).length);
   });
 
-  it('should execute GET when fetching privileges', () => {
-    component.getPrivileges(coursesWithStudents);
+  it('should call loadInstructorPrivilege once per unique courseId', () => {
+    component.getPrivileges(coursesWithStudents).subscribe(() => {
+      // 2 unique courses: CS3281 and CS3282
+      expect(mockInstructorService.loadInstructorPrivilege).toHaveBeenCalledTimes(2);
+      expect(mockInstructorService.loadInstructorPrivilege).toHaveBeenCalledWith({ courseId: 'CS3281' });
+      expect(mockInstructorService.loadInstructorPrivilege).toHaveBeenCalledWith({ courseId: 'CS3282' });
+    });
+  });
+
+  it('should use cached privileges on subsequent calls', () => {
+    component.getPrivileges(coursesWithStudents).subscribe(() => {
+      mockInstructorService.loadInstructorPrivilege.mockClear();
 
-    for (const course of coursesWithStudents) {
-      expect(mockInstructorService.loadInstructorPrivilege).toHaveBeenCalledWith({ courseId: course.courseId });
-    }
+      component.getPrivileges(coursesWithStudents).subscribe(() => {
+        expect(mockInstructorService.loadInstructorPrivilege).not.toHaveBeenCalled();
+      });
+    });
   });
 
   it('should combine privileges and course data correctly', () => {
@@ -295,59 +306,39 @@ describe('InstructorSearchPageComponent', () => {
       canViewSession: true,
       canSubmitSession: true,
     };
-    const mockPrivilegesArray: InstructorPrivilege[] = [
-      {
-        privileges: {
-          courseLevel: basePrivilege,
-          sectionLevel: {},
-          sessionLevel: {},
-        },
-      },
-      {
-        privileges: {
-          courseLevel: {
-            ...basePrivilege,
-            canModifyStudent: true,
+    const privilegeMap: Map<string, InstructorPrivilege> = new Map([
+      [
+        'CS3281',
+        {
+          privileges: {
+            courseLevel: basePrivilege,
+            sectionLevel: {},
+            sessionLevel: {},
           },
-          sectionLevel: {},
-          sessionLevel: {},
         },
-      },
-      {
-        privileges: {
-          courseLevel: {
-            ...basePrivilege,
-            canModifyStudent: false,
-          },
-          sectionLevel: {},
-          sessionLevel: {},
-        },
-      },
-      {
-        privileges: {
-          courseLevel: {
-            ...basePrivilege,
-            canModifyStudent: false,
+      ],
+      [
+        'CS3282',
+        {
+          privileges: {
+            courseLevel: {
+              ...basePrivilege,
+              canModifyStudent: false,
+            },
+            sectionLevel: {},
+            sessionLevel: {},
           },
-          sectionLevel: {},
-          sessionLevel: {},
         },
-      },
-    ];
-    component.combinePrivileges([coursesWithStudents, mockPrivilegesArray]);
-
-    const course1Student1: StudentListRowModel = coursesWithStudents[0].students[0];
-    expect(course1Student1.isAllowedToModifyStudent).toEqual(true);
-
-    const course1Student2: StudentListRowModel = coursesWithStudents[0].students[1];
-    expect(course1Student2.isAllowedToModifyStudent).toEqual(true);
-
-    const course1Student3: StudentListRowModel = coursesWithStudents[0].students[2];
-    expect(course1Student3.isAllowedToModifyStudent).toEqual(false);
+      ],
+    ]);
+    component.combinePrivileges(coursesWithStudents, privilegeMap);
 
-    const course2Student1: StudentListRowModel = coursesWithStudents[1].students[0];
-    expect(course2Student1.isAllowedToModifyStudent).toEqual(false);
+    // CS3281 has courseLevel canModifyStudent: true
+    expect(coursesWithStudents[0].students[0].isAllowedToModifyStudent).toEqual(true);
+    expect(coursesWithStudents[0].students[1].isAllowedToModifyStudent).toEqual(true);
+    expect(coursesWithStudents[0].students[2].isAllowedToModifyStudent).toEqual(true);
 
-    expect(mockPrivilegesArray.length).toEqual(0);
+    // CS3282 has courseLevel canModifyStudent: false
+    expect(coursesWithStudents[1].students[0].isAllowedToModifyStudent).toEqual(false);
   });
 });

src/web/app/pages-instructor/instructor-search-page/instructor-search-page.component.ts

@@ -32,6 +32,7 @@ export class InstructorSearchPageComponent implements OnInit {
   private courseService = inject(CourseService);
   private studentService = inject(StudentService);
   private visibleCourseIds$: Observable<string[]> = of([]);
+  private privilegeCache: Map<string, InstructorPrivilege> = new Map();
 
   searchParams: SearchParams = {
     searchKey: '',
@@ -58,16 +59,18 @@ export class InstructorSearchPageComponent implements OnInit {
         mergeMap((courseIds: string[]) => this.searchService.searchInstructor(this.searchParams.searchKey, courseIds)),
         map((res: InstructorSearchResult) => this.getCoursesWithStudents(res.students)),
         mergeMap((coursesWithStudents: SearchStudentsListRowTable[]) =>
-          forkJoin([of(coursesWithStudents), this.getPrivileges(coursesWithStudents)]),
+          this.getPrivileges(coursesWithStudents).pipe(
+            map((privilegeMap: Map<string, InstructorPrivilege>) =>
+              this.combinePrivileges(coursesWithStudents, privilegeMap),
+            ),
+          ),
         ),
-        map((res: [SearchStudentsListRowTable[], InstructorPrivilege[]]) => this.combinePrivileges(res)),
         finalize(() => {
           this.isSearching = false;
         }),
       )
       .subscribe({
-        next: (resp: TransformedInstructorSearchResult) => {
-          const searchStudentsTable: SearchStudentsListRowTable[] = resp.searchStudentTables;
+        next: (searchStudentsTable: SearchStudentsListRowTable[]) => {
           const hasStudents = !!searchStudentsTable?.length;
 
           if (hasStudents) {
@@ -105,51 +108,48 @@ export class InstructorSearchPageComponent implements OnInit {
     return coursesWithStudents;
   }
 
-  getPrivileges(coursesWithStudents: SearchStudentsListRowTable[]): Observable<InstructorPrivilege[]> {
-    if (coursesWithStudents.length === 0) {
-      return of([]);
+  getPrivileges(coursesWithStudents: SearchStudentsListRowTable[]): Observable<Map<string, InstructorPrivilege>> {
+    const courseIds: string[] = Array.from(
+      new Set(coursesWithStudents.map((c: SearchStudentsListRowTable) => c.courseId)),
+    );
+    const uncachedIds: string[] = courseIds.filter((id: string) => !this.privilegeCache.has(id));
+
+    if (uncachedIds.length === 0) {
+      return of(new Map(this.privilegeCache));
     }
-    const privileges: Observable<InstructorPrivilege>[] = [];
-    coursesWithStudents.forEach((course: SearchStudentsListRowTable) => {
-      const sectionToPrivileges: Record<string, Observable<InstructorPrivilege>> = {};
-      Array.from(
-        new Set(course.students.map((studentModel: StudentListRowModel) => studentModel.student.sectionName)),
-      ).forEach((section: string) => {
-        sectionToPrivileges[section] = this.instructorService.loadInstructorPrivilege({ courseId: course.courseId });
-      });
-      course.students.forEach((studentModel: StudentListRowModel) =>
-        privileges.push(sectionToPrivileges[studentModel.student.sectionName]),
-      );
+
+    const requests: Record<string, Observable<InstructorPrivilege>> = {};
+    uncachedIds.forEach((id: string) => {
+      requests[id] = this.instructorService.loadInstructorPrivilege({ courseId: id });
     });
-    return forkJoin(privileges);
+
+    return forkJoin(requests).pipe(
+      map((results: Record<string, InstructorPrivilege>) => {
+        Object.entries(results).forEach(([courseId, privilege]: [string, InstructorPrivilege]) => {
+          this.privilegeCache.set(courseId, privilege);
+        });
+        return new Map(this.privilegeCache);
+      }),
+    );
   }
 
-  combinePrivileges([coursesWithStudents, privileges]: [
-    SearchStudentsListRowTable[],
-    InstructorPrivilege[],
-  ]): TransformedInstructorSearchResult {
-    /**
-     * Pop the privilege objects one at a time and attach them to the results. This is possible
-     * because `forkJoin` guarantees that the `InstructorPrivilege` results are returned in the
-     * same order the requests were made.
-     */
+  combinePrivileges(
+    coursesWithStudents: SearchStudentsListRowTable[],
+    privilegeMap: Map<string, InstructorPrivilege>,
+  ): SearchStudentsListRowTable[] {
     for (const course of coursesWithStudents) {
+      const privilege: InstructorPrivilege | undefined = privilegeMap.get(course.courseId);
+      if (!privilege) {
+        continue;
+      }
+      const courseLevel: InstructorPermissionSet = privilege.privileges.courseLevel;
       for (const studentModel of course.students) {
-        const privilege: InstructorPrivilege | undefined = privileges.shift();
-        if (!privilege) {
-          continue;
-        }
         const sectionId: string = studentModel.student.sectionId;
-        const courseLevel: InstructorPermissionSet = privilege.privileges.courseLevel;
         const sectionLevel: InstructorPermissionSet = privilege.privileges.sectionLevel[sectionId] || courseLevel;
-
         studentModel.isAllowedToModifyStudent = sectionLevel.canModifyStudent;
       }
     }
-
-    return {
-      searchStudentTables: coursesWithStudents,
-    };
+    return coursesWithStudents;
   }
 
   /**
@@ -185,7 +185,3 @@ export class InstructorSearchPageComponent implements OnInit {
       .pipe(map((courses) => Array.from(new Set(courses.courses.map((course) => course.courseId)))));
   }
 }
-
-interface TransformedInstructorSearchResult {
-  searchStudentTables: SearchStudentsListRowTable[];
-}