refactor(StudentDataService, AnnotationService, TeacherDataService): …

…simplify state retrieval logic using Array.findLast() (#2008)
WISE-Community · Nov 27, 2024 · 5a27618 · 5a27618
1 parent 75d12c1
commit 5a27618
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 122 deletions.
diff --git a/src/app/services/studentDataService.spec.ts b/src/app/services/studentDataService.spec.ts
@@ -4,23 +4,20 @@ import { StudentDataService } from '../../assets/wise5/services/studentDataServi
 import { ConfigService } from '../../assets/wise5/services/configService';
 import { AnnotationService } from '../../assets/wise5/services/annotationService';
 import { ProjectService } from '../../assets/wise5/services/projectService';
-import { MatDialogModule } from '@angular/material/dialog';
 import { StudentTeacherCommonServicesModule } from '../student-teacher-common-services.module';
 import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';
 
-let $rootScope;
 let http: HttpTestingController;
 let service: StudentDataService;
 let configService: ConfigService;
 let annotationService: AnnotationService;
 let projectService: ProjectService;
-
 describe('StudentDataService', () => {
   beforeEach(() => {
     TestBed.configureTestingModule({
-    imports: [MatDialogModule, StudentTeacherCommonServicesModule],
-    providers: [provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting()]
-});
+      imports: [StudentTeacherCommonServicesModule],
+      providers: [provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting()]
+    });
     http = TestBed.inject(HttpTestingController);
     service = TestBed.inject(StudentDataService);
     configService = TestBed.inject(ConfigService);
@@ -135,7 +132,6 @@ function shouldHandleSaveStudentWorkToServerSuccess() {
       ]
     };
     spyOn(configService, 'getMode').and.returnValue('preview');
-    spyOn($rootScope, '$broadcast');
     spyOn(service, 'updateNodeStatuses').and.callFake(() => {});
     service.saveToServerRequestCount = 1;
     service.handleSaveToServerSuccess(savedStudentDataResponse);
@@ -148,10 +144,6 @@ function shouldHandleSaveStudentWorkToServerSuccess() {
     expect(service.studentData.componentStates[2].serverSaveTime).toBeDefined();
     expect(service.studentData.componentStates[2].requestToken).toEqual(null);
     expect(service.studentData.componentStates[2].id).toEqual(3);
-    expect($rootScope.$broadcast).toHaveBeenCalledWith(
-      'studentWorkSavedToServer',
-      jasmine.any(Object)
-    );
     expect(service.saveToServerRequestCount).toEqual(0);
     expect(service.updateNodeStatuses).toHaveBeenCalled();
   });
@@ -191,7 +183,6 @@ function shouldHandleSaveEventsToServerSuccess() {
         createEvent(3, 'node1', 'component1', 'nodeEntered', 'c', 3000)
       ]
     };
-    spyOn($rootScope, '$broadcast');
     spyOn(service, 'updateNodeStatuses').and.callFake(() => {});
     service.saveToServerRequestCount = 1;
     service.handleSaveToServerSuccess(savedStudentDataResponse);

diff --git a/...sets/wise5/classroomMonitor/dataExport/strategies/OneWorkgroupPerRowDataExportStrategy.ts b/...sets/wise5/classroomMonitor/dataExport/strategies/OneWorkgroupPerRowDataExportStrategy.ts
@@ -246,14 +246,11 @@ export class OneWorkgroupPerRowDataExportStrategy extends AbstractDataExportStra
   }
 
   private getLatestBranchPathTakenEvent(workgroupId: number, nodeId: string): any {
-    const events = this.teacherDataService.getEventsByWorkgroupId(workgroupId);
-    for (let i = events.length - 1; i >= 0; i--) {
-      const event = events[i];
-      if (event.nodeId === nodeId && event.event === 'branchPathTaken') {
-        return event;
-      }
-    }
-    return null;
+    return (
+      this.teacherDataService
+        .getEventsByWorkgroupId(workgroupId)
+        .findLast((event) => event.nodeId === nodeId && event.event === 'branchPathTaken') ?? null
+    );
   }
 
   /**

diff --git a/src/assets/wise5/services/annotationService.ts b/src/assets/wise5/services/annotationService.ts
@@ -42,27 +42,17 @@ export class AnnotationService {
    * @returns the latest annotation that matches the params
    */
   getLatestAnnotation(params): any {
-    for (let a = this.annotations.length - 1; a >= 0; a--) {
-      const annotation = this.annotations[a];
-      let match = true;
-      if (annotation.nodeId !== params.nodeId) {
-        match = false;
-      }
-      if (match && annotation.componentId !== params.componentId) {
-        match = false;
-      }
-      if (match) {
-        if (params.type.constructor === Array) {
-          match = params.type.every((thisType) => annotation.type === thisType);
-        } else {
-          match = annotation.type === params.type;
-        }
-        if (match) {
-          return annotation;
-        }
-      }
-    }
-    return null;
+    return (
+      this.annotations.findLast((annotation) => {
+        return (
+          annotation.nodeId == params.nodeId &&
+          annotation.componentId == params.componentId &&
+          ((params.type.constructor === Array &&
+            params.type.every((thisType) => annotation.type === thisType)) ||
+            annotation.type === params.type)
+        );
+      }) ?? null
+    );
   }
 
   /**

diff --git a/src/assets/wise5/services/studentDataService.ts b/src/assets/wise5/services/studentDataService.ts
@@ -524,31 +524,24 @@ export class StudentDataService extends DataService {
    * are found
    */
   getLatestComponentStateByNodeIdAndComponentId(nodeId: string, componentId: string = null): any {
-    const componentStates = this.studentData.componentStates;
-    for (let c = componentStates.length - 1; c >= 0; c--) {
-      const componentState = componentStates[c];
-      if (componentState.nodeId === nodeId) {
-        if (componentId == null || componentState.componentId === componentId) {
-          return componentState;
-        }
-      }
-    }
-    return null;
+    return (
+      this.studentData.componentStates.findLast(
+        (componentState) =>
+          componentState.nodeId === nodeId &&
+          (componentId == null || componentState.componentId === componentId)
+      ) ?? null
+    );
   }
 
-  getLatestSubmitComponentState(nodeId, componentId) {
-    const componentStates = this.studentData.componentStates;
-    for (let c = componentStates.length - 1; c >= 0; c--) {
-      const componentState = componentStates[c];
-      if (
-        componentState.nodeId === nodeId &&
-        componentState.componentId === componentId &&
-        componentState.isSubmit
-      ) {
-        return componentState;
-      }
-    }
-    return null;
+  getLatestSubmitComponentState(nodeId: string, componentId: string): any {
+    return (
+      this.studentData.componentStates.findLast(
+        (componentState) =>
+          componentState.nodeId === nodeId &&
+          componentState.componentId === componentId &&
+          componentState.isSubmit
+      ) ?? null
+    );
   }
 
   getComponentStates(): any[] {
@@ -585,18 +578,14 @@ export class StudentDataService extends DataService {
    * @return the node id of the latest node entered event for an active node
    * that exists in the project
    */
-  getLatestNodeEnteredEventNodeIdWithExistingNode() {
-    const events = this.studentData.events;
-    for (let e = events.length - 1; e >= 0; e--) {
-      const event = events[e];
-      if (event.event == 'nodeEntered' && this.isNodeExistAndActive(event.nodeId)) {
-        return event.nodeId;
-      }
-    }
-    return null;
+  getLatestNodeEnteredEventNodeIdWithExistingNode(): string {
+    const event = this.studentData.events.findLast(
+      (event) => event.event === 'nodeEntered' && this.isNodeExistAndActive(event.nodeId)
+    );
+    return event?.nodeId ?? null;
   }
 
-  isNodeExistAndActive(nodeId) {
+  private isNodeExistAndActive(nodeId: string): boolean {
     return this.ProjectService.getNodeById(nodeId) != null && this.ProjectService.isActive(nodeId);
   }
 

diff --git a/src/assets/wise5/services/teacherDataService.ts b/src/assets/wise5/services/teacherDataService.ts
@@ -1,5 +1,3 @@
-'use strict';
-
 import { HttpClient, HttpParams } from '@angular/common/http';
 import { AnnotationService } from './annotationService';
 import { ConfigService } from './configService';
@@ -10,7 +8,6 @@ import { Observable, Subject, tap } from 'rxjs';
 import { DataService } from '../../../app/services/data.service';
 import { Node } from '../common/Node';
 import { compressToEncodedURIComponent } from 'lz-string';
-import { isMatchingPeriods } from '../common/period/period';
 import { getIntersectOfArrays } from '../common/array/array';
 import { serverSaveTimeComparator } from '../common/object/object';
 import { Annotation } from '../common/Annotation';
@@ -411,33 +408,26 @@ export class TeacherDataService extends DataService {
   }
 
   getLatestComponentStateByWorkgroupIdNodeIdAndComponentId(workgroupId, nodeId, componentId) {
-    const componentStates = this.getComponentStatesByWorkgroupIdAndNodeId(workgroupId, nodeId);
-    for (let c = componentStates.length - 1; c >= 0; c--) {
-      const componentState = componentStates[c];
-      if (this.isComponentStateMatchingNodeIdComponentId(componentState, nodeId, componentId)) {
-        return componentState;
-      }
-    }
-    return null;
-  }
-
-  isComponentStateMatchingNodeIdComponentId(componentState, nodeId, componentId) {
-    return componentState.nodeId === nodeId && componentState.componentId === componentId;
+    return (
+      this.getComponentStatesByWorkgroupIdAndNodeId(workgroupId, nodeId).findLast(
+        (componentState) =>
+          componentState.nodeId === nodeId && componentState.componentId === componentId
+      ) ?? null
+    );
   }
 
-  getLatestComponentStateByWorkgroupIdNodeId(workgroupId, nodeId) {
-    const componentStates = this.getComponentStatesByWorkgroupIdAndNodeId(workgroupId, nodeId);
-    for (let c = componentStates.length - 1; c >= 0; c--) {
-      const componentState = componentStates[c];
-      if (this.isComponentStateMatchingNodeId(componentState, nodeId)) {
-        return componentState;
-      }
-    }
-    return null;
+  getLatestComponentStateByWorkgroupIdNodeId(workgroupId: number, nodeId: string): any {
+    return (
+      this.getComponentStatesByWorkgroupIdAndNodeId(workgroupId, nodeId).findLast(
+        (componentState) => componentState.nodeId === nodeId
+      ) ?? null
+    );
   }
 
-  isComponentStateMatchingNodeId(componentState, nodeId) {
-    return componentState.nodeId === nodeId;
+  private getComponentStatesByWorkgroupIdAndNodeId(workgroupId: number, nodeId: string): any[] {
+    const componentStatesByWorkgroupId = this.getComponentStatesByWorkgroupId(workgroupId);
+    const componentStatesByNodeId = this.getComponentStatesByNodeId(nodeId);
+    return getIntersectOfArrays(componentStatesByWorkgroupId, componentStatesByNodeId);
   }
 
   /**
@@ -478,12 +468,6 @@ export class TeacherDataService extends DataService {
     return componentState.nodeId + '-' + componentState.componentId;
   }
 
-  getComponentStatesByWorkgroupIdAndNodeId(workgroupId, nodeId) {
-    const componentStatesByWorkgroupId = this.getComponentStatesByWorkgroupId(workgroupId);
-    const componentStatesByNodeId = this.getComponentStatesByNodeId(nodeId);
-    return getIntersectOfArrays(componentStatesByWorkgroupId, componentStatesByNodeId);
-  }
-
   getComponentStatesByWorkgroupIdAndComponentId(workgroupId, componentId) {
     const componentStatesByWorkgroupId = this.getComponentStatesByWorkgroupId(workgroupId);
     const componentStatesByComponentId = this.getComponentStatesByComponentId(componentId);
@@ -517,22 +501,6 @@ export class TeacherDataService extends DataService {
     return this.studentData.annotationsByNodeId[nodeId] || [];
   }
 
-  getAnnotationsByNodeIdAndComponentId(nodeId: string, componentId: string): any[] {
-    const annotationsByNodeId = this.getAnnotationsByNodeId(nodeId);
-    return annotationsByNodeId.filter((annotation: any) => annotation.componentId === componentId);
-  }
-
-  getAnnotationsByNodeIdAndPeriodId(nodeId, periodId) {
-    const annotationsByNodeId = this.studentData.annotationsByNodeId[nodeId];
-    if (annotationsByNodeId != null) {
-      return annotationsByNodeId.filter((annotation) => {
-        return isMatchingPeriods(annotation.periodId, periodId);
-      });
-    } else {
-      return [];
-    }
-  }
-
   setCurrentPeriod(period) {
     const previousPeriod = this.currentPeriod;
     this.currentPeriod = period;