refactor(TeacherDataService, DiscussionService, TeacherDiscussionServ…

…ice): simplify component state retrieval
WISE-Community · Nov 28, 2024 · 054f343 · 054f343
1 parent 4415fd1
commit 054f343
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 119 deletions.
diff --git a/src/app/services/discussionService.spec.ts b/src/app/services/discussionService.spec.ts
@@ -15,11 +15,7 @@ import { NotificationService } from '../../assets/wise5/services/notificationSer
 import { ClassroomStatusService } from '../../assets/wise5/services/classroomStatusService';
 import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';
 
-class MockTeacherDataService {
-  getComponentStatesByComponentIds() {
-    return [];
-  }
-}
+class MockTeacherDataService {}
 
 let service: DiscussionService;
 let http: HttpTestingController;
@@ -31,8 +27,8 @@ const runId = 1;
 describe('DiscussionService', () => {
   beforeEach(() => {
     TestBed.configureTestingModule({
-    imports: [],
-    providers: [
+      imports: [],
+      providers: [
         AchievementService,
         AnnotationService,
         ClassroomStatusService,
@@ -48,8 +44,8 @@ describe('DiscussionService', () => {
         TeacherWebSocketService,
         provideHttpClient(withInterceptorsFromDi()),
         provideHttpClientTesting()
-    ]
-});
+      ]
+    });
 
     http = TestBed.inject(HttpTestingController);
     service = TestBed.inject(DiscussionService);
@@ -184,29 +180,26 @@ function getClassmateResponsesFromComponents() {
 }
 
 function getClassmateResponses() {
-  it(
-    'should get classmate responses',
-    waitForAsync(() => {
-      const componentState1 = { studentData: { response: 'Hello World' } };
-      const annotation1 = { data: { action: 'Delete' } };
-      service
-        .getClassmateResponses(runId, periodId, nodeId, componentId)
-        .subscribe((response: any) => {
-          expect(response.studentWork).toEqual([componentState1]);
-          expect(response.annotations).toEqual([annotation1]);
-        });
-      http
-        .expectOne(
-          `/api/classmate/discussion/student-work/${runId}/${periodId}/${nodeId}/${componentId}`
-        )
-        .flush([componentState1]);
-      http
-        .expectOne(
-          `/api/classmate/discussion/annotations/${runId}/${periodId}/${nodeId}/${componentId}`
-        )
-        .flush([annotation1]);
-    })
-  );
+  it('should get classmate responses', waitForAsync(() => {
+    const componentState1 = { studentData: { response: 'Hello World' } };
+    const annotation1 = { data: { action: 'Delete' } };
+    service
+      .getClassmateResponses(runId, periodId, nodeId, componentId)
+      .subscribe((response: any) => {
+        expect(response.studentWork).toEqual([componentState1]);
+        expect(response.annotations).toEqual([annotation1]);
+      });
+    http
+      .expectOne(
+        `/api/classmate/discussion/student-work/${runId}/${periodId}/${nodeId}/${componentId}`
+      )
+      .flush([componentState1]);
+    http
+      .expectOne(
+        `/api/classmate/discussion/annotations/${runId}/${periodId}/${nodeId}/${componentId}`
+      )
+      .flush([annotation1]);
+  }));
 }
 
 function isTopLevelPost() {

diff --git a/src/app/services/teacherDiscussionService.spec.ts b/src/app/services/teacherDiscussionService.spec.ts
@@ -1,25 +1,16 @@
 import { DiscussionService } from '../../assets/wise5/components/discussion/discussionService';
 import { TestBed } from '@angular/core/testing';
-import { provideHttpClientTesting } from '@angular/common/http/testing';
 import { AnnotationService } from '../../assets/wise5/services/annotationService';
 import { ConfigService } from '../../assets/wise5/services/configService';
 import { ProjectService } from '../../assets/wise5/services/projectService';
 import { StudentAssetService } from '../../assets/wise5/services/studentAssetService';
-import { TagService } from '../../assets/wise5/services/tagService';
-import { SessionService } from '../../assets/wise5/services/sessionService';
 import { TeacherDataService } from '../../assets/wise5/services/teacherDataService';
 import { TeacherProjectService } from '../../assets/wise5/services/teacherProjectService';
-import { TeacherWebSocketService } from '../../assets/wise5/services/teacherWebSocketService';
-import { AchievementService } from '../../assets/wise5/services/achievementService';
-import { NotificationService } from '../../assets/wise5/services/notificationService';
-import { ClassroomStatusService } from '../../assets/wise5/services/classroomStatusService';
 import { TeacherDiscussionService } from '../../assets/wise5/components/discussion/teacherDiscussionService';
 import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';
 
 class MockTeacherDataService {
-  getComponentStatesByComponentIds() {
-    return [];
-  }
+  getComponentStatesByComponentId() {}
 }
 
 class ComponentState {
@@ -46,45 +37,30 @@ const nodeId = 'node1';
 describe('TeacherDiscussionService', () => {
   beforeEach(() => {
     TestBed.configureTestingModule({
-    imports: [],
-    providers: [
-        AchievementService,
+      providers: [
         AnnotationService,
-        ClassroomStatusService,
         ConfigService,
         DiscussionService,
-        NotificationService,
         ProjectService,
-        SessionService,
         StudentAssetService,
-        TagService,
         { provide: TeacherDataService, useClass: MockTeacherDataService },
         TeacherDiscussionService,
         TeacherProjectService,
-        TeacherWebSocketService,
-        provideHttpClient(withInterceptorsFromDi()),
-        provideHttpClientTesting()
-    ]
-});
-
+        provideHttpClient(withInterceptorsFromDi())
+      ]
+    });
     service = TestBed.inject(TeacherDiscussionService);
   });
-
   getPostAndAllRepliesWithComponentIdAndComponentStateId();
 });
 
 function getPostAndAllRepliesWithComponentIdAndComponentStateId() {
   it('should get post and all replies with component id and component state id', () => {
-    spyOn(TestBed.inject(TeacherDataService), 'getComponentStatesByComponentIds').and.callFake(
-      () => {
-        const componentStates = [
-          new ComponentState(1, nodeId, componentId, null, 'Hello', []),
-          new ComponentState(2, nodeId, componentId, 1, 'World', []),
-          new ComponentState(3, nodeId, componentId, null, 'OK', [])
-        ];
-        return componentStates;
-      }
-    );
+    spyOn(TestBed.inject(TeacherDataService), 'getComponentStatesByComponentId').and.returnValue([
+      new ComponentState(1, nodeId, componentId, null, 'Hello', []),
+      new ComponentState(2, nodeId, componentId, 1, 'World', []),
+      new ComponentState(3, nodeId, componentId, null, 'OK', [])
+    ]);
     const postAndAllReplies = service.getPostAndAllRepliesByComponentIds([componentId], 1);
     expect(postAndAllReplies.length).toEqual(2);
   });

diff --git a/...assets/wise5/components/discussion/discussion-show-work/discussion-show-work.component.ts b/...assets/wise5/components/discussion/discussion-show-work/discussion-show-work.component.ts
@@ -36,12 +36,13 @@ export class DiscussionShowWorkComponent extends ComponentShowWorkDirective {
     this.setStudentWork();
   }
 
-  setStudentWork(): void {
+  private setStudentWork(): void {
     const componentIds = this.getGradingComponentIds();
-    const componentStates = this.TeacherDiscussionService.getPostsAssociatedWithComponentIdsAndWorkgroupId(
-      componentIds,
-      this.workgroupId
-    );
+    const componentStates =
+      this.TeacherDiscussionService.getPostsAssociatedWithComponentIdsAndWorkgroupId(
+        componentIds,
+        this.workgroupId
+      );
     const annotations = this.getInappropriateFlagAnnotationsByComponentStates(componentStates);
     this.setClassResponses(componentStates, annotations);
   }
@@ -54,10 +55,11 @@ export class DiscussionShowWorkComponent extends ComponentShowWorkDirective {
   getInappropriateFlagAnnotationsByComponentStates(componentStates = []) {
     const annotations = [];
     for (const componentState of componentStates) {
-      const latestInappropriateFlagAnnotation = this.AnnotationService.getLatestAnnotationByStudentWorkIdAndType(
-        componentState.id,
-        'inappropriateFlag'
-      );
+      const latestInappropriateFlagAnnotation =
+        this.AnnotationService.getLatestAnnotationByStudentWorkIdAndType(
+          componentState.id,
+          'inappropriateFlag'
+        );
       if (latestInappropriateFlagAnnotation != null) {
         annotations.push(latestInappropriateFlagAnnotation);
       }
@@ -145,10 +147,11 @@ export class DiscussionShowWorkComponent extends ComponentShowWorkDirective {
       data
     );
     this.AnnotationService.saveAnnotation(annotation).then(() => {
-      const componentStates = this.TeacherDiscussionService.getPostsAssociatedWithComponentIdsAndWorkgroupId(
-        this.getGradingComponentIds(),
-        this.workgroupId
-      );
+      const componentStates =
+        this.TeacherDiscussionService.getPostsAssociatedWithComponentIdsAndWorkgroupId(
+          this.getGradingComponentIds(),
+          this.workgroupId
+        );
       const annotations = this.getInappropriateFlagAnnotationsByComponentStates(componentStates);
       this.setClassResponses(componentStates, annotations);
     });

diff --git a/src/assets/wise5/components/discussion/discussionService.ts b/src/assets/wise5/components/discussion/discussionService.ts
@@ -8,7 +8,10 @@ import { serverSaveTimeComparator } from '../../common/object/object';
 
 @Injectable()
 export class DiscussionService extends ComponentService {
-  constructor(protected http: HttpClient, protected ConfigService: ConfigService) {
+  constructor(
+    protected configService: ConfigService,
+    protected http: HttpClient
+  ) {
     super();
   }
 
@@ -216,10 +219,8 @@ export class DiscussionService extends ComponentService {
       if (componentState.studentData.isSubmit) {
         componentState.replies = [];
         this.setUsernames(componentState);
-        const latestInappropriateFlagAnnotation = this.getLatestInappropriateFlagAnnotationByStudentWorkId(
-          annotations,
-          componentState.id
-        );
+        const latestInappropriateFlagAnnotation =
+          this.getLatestInappropriateFlagAnnotationByStudentWorkId(annotations, componentState.id);
         if (isStudentMode) {
           if (
             latestInappropriateFlagAnnotation == null ||
@@ -244,17 +245,17 @@ export class DiscussionService extends ComponentService {
 
   setUsernames(componentState: any): void {
     const workgroupId = componentState.workgroupId;
-    const usernames = this.ConfigService.getUsernamesByWorkgroupId(workgroupId);
+    const usernames = this.configService.getUsernamesByWorkgroupId(workgroupId);
     if (usernames.length > 0) {
-      componentState.usernames = this.ConfigService.getUsernamesStringByWorkgroupId(workgroupId);
+      componentState.usernames = this.configService.getUsernamesStringByWorkgroupId(workgroupId);
     } else if (componentState.usernamesArray != null) {
       componentState.usernames = componentState.usernamesArray
         .map(function (obj) {
           return obj.name;
         })
         .join(', ');
     } else {
-      componentState.usernames = this.ConfigService.getUserIdsStringByWorkgroupId(workgroupId);
+      componentState.usernames = this.configService.getUserIdsStringByWorkgroupId(workgroupId);
     }
   }
 

diff --git a/src/assets/wise5/components/discussion/teacherDiscussionService.ts b/src/assets/wise5/components/discussion/teacherDiscussionService.ts
@@ -3,25 +3,22 @@ import { Injectable } from '@angular/core';
 import { ConfigService } from '../../services/configService';
 import { TeacherDataService } from '../../services/teacherDataService';
 import { DiscussionService } from './discussionService';
+import { getIntersectOfArrays } from '../../common/array/array';
 
 @Injectable()
 export class TeacherDiscussionService extends DiscussionService {
   constructor(
     protected http: HttpClient,
     protected configService: ConfigService,
-    protected teacherDataService: TeacherDataService
+    protected dataService: TeacherDataService
   ) {
-    super(http, configService);
+    super(configService, http);
   }
 
   getPostsAssociatedWithComponentIdsAndWorkgroupId(componentIds: string[], workgroupId: number) {
     let allPosts = [];
     const topLevelComponentStateIdsFound = [];
-    const componentStates = this.teacherDataService.getComponentStatesByWorkgroupIdAndComponentIds(
-      workgroupId,
-      componentIds
-    );
-    for (const componentState of componentStates) {
+    for (const componentState of this.getComponentStates(workgroupId, componentIds)) {
       const componentStateIdReplyingTo = componentState.studentData.componentStateIdReplyingTo;
       if (this.isTopLevelPost(componentState)) {
         if (
@@ -49,12 +46,20 @@ export class TeacherDiscussionService extends DiscussionService {
     return allPosts;
   }
 
+  private getComponentStates(workgroupId: number, componentIds: string[]): any[] {
+    const workgroupComponentStates = this.dataService.getComponentStatesByWorkgroupId(workgroupId);
+    const componentStates = componentIds.flatMap((componentId) =>
+      this.dataService.getComponentStatesByComponentId(componentId)
+    );
+    return getIntersectOfArrays(workgroupComponentStates, componentStates);
+  }
+
   getPostAndAllRepliesByComponentIds(componentIds: string[], componentStateId: number) {
     const postAndAllReplies = [];
-    const componentStatesForComponentIds = this.teacherDataService.getComponentStatesByComponentIds(
-      componentIds
+    const componentStates = componentIds.flatMap((componentId) =>
+      this.dataService.getComponentStatesByComponentId(componentId)
     );
-    for (const componentState of componentStatesForComponentIds) {
+    for (const componentState of componentStates) {
       if (componentState.id === componentStateId) {
         postAndAllReplies.push(componentState);
       } else {

diff --git a/src/assets/wise5/services/teacherDataService.ts b/src/assets/wise5/services/teacherDataService.ts
@@ -397,16 +397,6 @@ export class TeacherDataService extends DataService {
     return this.studentData.componentStatesByComponentId[componentId] || [];
   }
 
-  getComponentStatesByComponentIds(componentIds) {
-    let componentStatesByComponentId = [];
-    for (const componentId of componentIds) {
-      componentStatesByComponentId = componentStatesByComponentId.concat(
-        this.studentData.componentStatesByComponentId[componentId]
-      );
-    }
-    return componentStatesByComponentId;
-  }
-
   getLatestComponentStateByWorkgroupIdNodeIdAndComponentId(workgroupId, nodeId, componentId) {
     return (
       this.getComponentStatesByWorkgroupIdAndNodeId(workgroupId, nodeId).findLast(
@@ -436,17 +426,6 @@ export class TeacherDataService extends DataService {
     return getIntersectOfArrays(componentStatesByWorkgroupId, componentStatesByComponentId);
   }
 
-  getComponentStatesByWorkgroupIdAndComponentIds(workgroupId, componentIds) {
-    const componentStatesByWorkgroupId = this.getComponentStatesByWorkgroupId(workgroupId);
-    let componentStatesByComponentId = [];
-    for (const componentId of componentIds) {
-      componentStatesByComponentId = componentStatesByComponentId.concat(
-        this.getComponentStatesByComponentId(componentId)
-      );
-    }
-    return getIntersectOfArrays(componentStatesByWorkgroupId, componentStatesByComponentId);
-  }
-
   getEventsByWorkgroupId(workgroupId) {
     return this.studentData.eventsByWorkgroupId[workgroupId] || [];
   }

diff --git a/src/messages.xlf b/src/messages.xlf
@@ -17717,7 +17717,7 @@ Category Name: <x id="PH_1" equiv-text="categoryName"/></source>
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/discussion/discussionService.ts</context>
-          <context context-type="linenumber">16</context>
+          <context context-type="linenumber">19</context>
         </context-group>
       </trans-unit>
       <trans-unit id="f1affa088d785794802225dbc56ee2ff3d785f87" datatype="html">