refactor(SummaryDisplay): Simplify data service initialization and im…

…prove subscription handling (#2001)
WISE-Community · Nov 25, 2024 · 6b9d87e · 6b9d87e
1 parent e20430e
commit 6b9d87e
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 59 deletions.
diff --git a/src/assets/wise5/directives/student-summary-display/student-summary-display.component.ts b/src/assets/wise5/directives/student-summary-display/student-summary-display.component.ts
@@ -5,47 +5,45 @@ import { ProjectService } from '../../services/projectService';
 import { SummaryService } from '../../components/summary/summaryService';
 import { SummaryDisplay } from '../summary-display/summary-display.component';
 import { StudentDataService } from '../../services/studentDataService';
+import { Subscription } from 'rxjs';
 
 @Component({
   selector: 'student-summary-display',
   templateUrl: '../summary-display/summary-display.component.html',
   styleUrls: ['../summary-display/summary-display.component.scss']
 })
 export class StudentSummaryDisplay extends SummaryDisplay {
+  private studentWorkSavedToServerSubscription: Subscription;
+
   constructor(
     protected annotationService: AnnotationService,
     protected configService: ConfigService,
+    protected dataService: StudentDataService,
     protected projectService: ProjectService,
-    private studentDataService: StudentDataService,
     protected summaryService: SummaryService
   ) {
-    super(annotationService, configService, projectService, summaryService);
+    super(annotationService, configService, dataService, projectService, summaryService);
   }
 
-  ngOnInit() {
+  ngOnInit(): void {
     super.ngOnInit();
     this.initializeChangeListeners();
   }
 
-  ngOnDestroy() {
+  ngOnDestroy(): void {
     this.studentWorkSavedToServerSubscription.unsubscribe();
   }
 
-  initializeDataService() {
-    this.dataService = this.studentDataService;
-  }
-
-  initializeChangeListeners() {
-    this.studentWorkSavedToServerSubscription = this.studentDataService.studentWorkSavedToServer$.subscribe(
-      (componentState) => {
+  private initializeChangeListeners(): void {
+    this.studentWorkSavedToServerSubscription =
+      this.dataService.studentWorkSavedToServer$.subscribe((componentState) => {
         if (
           this.doRender &&
           componentState.nodeId === this.nodeId &&
           componentState.componentId === this.componentId
         ) {
           this.renderDisplay();
         }
-      }
-    );
+      });
   }
 }
diff --git a/src/assets/wise5/directives/summary-display/summary-display.component.ts b/src/assets/wise5/directives/summary-display/summary-display.component.ts
@@ -1,6 +1,6 @@
 import * as Highcharts from 'highcharts';
 import { Component, Input, SimpleChanges } from '@angular/core';
-import { Observable, Subscription } from 'rxjs';
+import { Observable } from 'rxjs';
 import { AnnotationService } from '../../services/annotationService';
 import { ConfigService } from '../../services/configService';
 import { ProjectService } from '../../services/projectService';
@@ -9,6 +9,8 @@ import { of } from 'rxjs';
 import { tap } from 'rxjs/operators';
 import { copy } from '../../common/object/object';
 import { rgbToHex } from '../../common/color/color';
+import { DataService } from '../../../../app/services/data.service';
+import { StudentDataService } from '../../services/studentDataService';
 
 @Component({
   selector: 'summary-display',
@@ -40,7 +42,6 @@ export abstract class SummaryDisplay {
     correct: '#00C853',
     incorrect: '#C62828'
   };
-  dataService: any = null;
   defaultMaxScore: number = 5;
   hasCorrectness: boolean = false;
   Highcharts: typeof Highcharts = Highcharts;
@@ -50,7 +51,6 @@ export abstract class SummaryDisplay {
   otherComponent: any;
   otherComponentType: string;
   percentResponded: number;
-  studentWorkSavedToServerSubscription: Subscription;
   totalWorkgroups: number;
 
   @Input() nodeId: string;
@@ -68,14 +68,14 @@ export abstract class SummaryDisplay {
   constructor(
     protected annotationService: AnnotationService,
     protected configService: ConfigService,
+    protected dataService: DataService,
     protected projectService: ProjectService,
     protected summaryService: SummaryService
   ) {}
 
   ngOnInit() {
     this.setNumDummySamples();
     this.initializeOtherComponent();
-    this.initializeDataService();
     this.initializeCustomLabelColors();
     if (this.doRender) {
       this.renderDisplay();
@@ -99,10 +99,6 @@ export abstract class SummaryDisplay {
     }
   }
 
-  initializeDataService() {
-    // implemented by children
-  }
-
   initializeCustomLabelColors() {
     if (this.customLabelColors == null) {
       this.customLabelColors = [];
@@ -164,9 +160,9 @@ export abstract class SummaryDisplay {
     this.processComponentStates(componentStates);
   }
 
-  getResponseForSelf() {
+  private getResponseForSelf() {
     if (this.isVLEPreview() || this.isStudentRun()) {
-      return this.dataService.getLatestComponentStateByNodeIdAndComponentId(
+      return (this.dataService as StudentDataService).getLatestComponentStateByNodeIdAndComponentId(
         this.nodeId,
         this.componentId
       );
@@ -312,12 +308,11 @@ export abstract class SummaryDisplay {
     });
   }
 
-  getDummyStudentWorkForVLEPreview(nodeId: string, componentId: string): Observable<any> {
+  private getDummyStudentWorkForVLEPreview(nodeId: string, componentId: string): Observable<any> {
     const componentStates = this.createDummyComponentStates();
-    const componentState = this.dataService.getLatestComponentStateByNodeIdAndComponentId(
-      nodeId,
-      componentId
-    );
+    const componentState = (
+      this.dataService as StudentDataService
+    ).getLatestComponentStateByNodeIdAndComponentId(nodeId, componentId);
     if (componentState != null) {
       componentStates.push(componentState);
     }
@@ -341,15 +336,15 @@ export abstract class SummaryDisplay {
     return of(this.createDummyScoreAnnotations());
   }
 
-  createDummyComponentStates() {
+  private createDummyComponentStates() {
     const dummyComponentStates = [];
     for (let dummyCounter = 0; dummyCounter < this.numDummySamples; dummyCounter++) {
       dummyComponentStates.push(this.createDummyComponentState(this.otherComponent));
     }
     return dummyComponentStates;
   }
 
-  createDummyComponentState(component) {
+  private createDummyComponentState(component) {
     if (this.otherComponentType === 'MultipleChoice') {
       return this.createDummyMultipleChoiceComponentState(component);
     } else if (this.otherComponentType === 'Table') {
@@ -366,7 +361,7 @@ export abstract class SummaryDisplay {
     };
   }
 
-  createDummyTableComponentState(component) {
+  private createDummyTableComponentState(component) {
     if (this.isAuthoringPreview()) {
       return {
         studentData: {
@@ -391,12 +386,11 @@ export abstract class SummaryDisplay {
     ];
   }
 
-  getDummyTableDataSimilarToLatestComponentState() {
+  private getDummyTableDataSimilarToLatestComponentState(): any {
     let tableData = [];
-    const componentState = this.dataService.getLatestComponentStateByNodeIdAndComponentId(
-      this.nodeId,
-      this.componentId
-    );
+    const componentState = (
+      this.dataService as StudentDataService
+    ).getLatestComponentStateByNodeIdAndComponentId(this.nodeId, this.componentId);
     if (componentState != null) {
       tableData = copy(componentState.studentData.tableData);
       for (let r = 1; r < tableData.length; r++) {
@@ -406,15 +400,15 @@ export abstract class SummaryDisplay {
     return tableData;
   }
 
-  getRandomSimilarNumber(text) {
+  private getRandomSimilarNumber(text): number {
     return Math.ceil(this.convertToNumber(text) * Math.random());
   }
 
-  getRandomChoice(choices) {
+  private getRandomChoice(choices): any {
     return choices[Math.floor(Math.random() * choices.length)];
   }
 
-  createDummyScoreAnnotations() {
+  private createDummyScoreAnnotations(): any {
     const dummyScoreAnnotations = [];
     for (let dummyCounter = 0; dummyCounter < this.numDummySamples; dummyCounter++) {
       dummyScoreAnnotations.push(this.createDummyScoreAnnotation());

diff --git a/src/assets/wise5/directives/teacher-summary-display/teacher-summary-display.component.ts b/src/assets/wise5/directives/teacher-summary-display/teacher-summary-display.component.ts
@@ -15,14 +15,10 @@ export class TeacherSummaryDisplay extends SummaryDisplay {
   constructor(
     protected annotationService: AnnotationService,
     protected configService: ConfigService,
+    protected dataService: TeacherDataService,
     protected projectService: ProjectService,
-    protected summaryService: SummaryService,
-    private teacherDataService: TeacherDataService
+    protected summaryService: SummaryService
   ) {
-    super(annotationService, configService, projectService, summaryService);
-  }
-
-  initializeDataService() {
-    this.dataService = this.teacherDataService;
+    super(annotationService, configService, dataService, projectService, summaryService);
   }
 }
diff --git a/src/messages.xlf b/src/messages.xlf
@@ -19706,11 +19706,11 @@ Warning: This will delete all existing choices and buckets in this component.</s
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">576</context>
+          <context context-type="linenumber">570</context>
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">686</context>
+          <context context-type="linenumber">680</context>
         </context-group>
       </trans-unit>
       <trans-unit id="2496818939481806999" datatype="html">
@@ -19721,11 +19721,11 @@ Warning: This will delete all existing choices and buckets in this component.</s
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">576</context>
+          <context context-type="linenumber">570</context>
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">690</context>
+          <context context-type="linenumber">684</context>
         </context-group>
       </trans-unit>
       <trans-unit id="3309404570196522710" datatype="html">
@@ -21574,63 +21574,63 @@ If this problem continues, let your teacher know and move on to the next activit
         <source>The student will see a graph of their individual data here.</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">154</context>
+          <context context-type="linenumber">150</context>
         </context-group>
       </trans-unit>
       <trans-unit id="629070935308008546" datatype="html">
         <source>Your Response</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">710</context>
+          <context context-type="linenumber">704</context>
         </context-group>
       </trans-unit>
       <trans-unit id="610707389788117365" datatype="html">
         <source>Your Score</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">712</context>
+          <context context-type="linenumber">706</context>
         </context-group>
       </trans-unit>
       <trans-unit id="1159047178964534522" datatype="html">
         <source>Period Responses</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">719</context>
+          <context context-type="linenumber">713</context>
         </context-group>
       </trans-unit>
       <trans-unit id="6903542619561738551" datatype="html">
         <source>Period Scores</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">724</context>
+          <context context-type="linenumber">718</context>
         </context-group>
       </trans-unit>
       <trans-unit id="2142002953999044695" datatype="html">
         <source>Class Responses</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">733</context>
+          <context context-type="linenumber">727</context>
         </context-group>
       </trans-unit>
       <trans-unit id="489297649750722424" datatype="html">
         <source>Class Scores</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">738</context>
+          <context context-type="linenumber">732</context>
         </context-group>
       </trans-unit>
       <trans-unit id="4270887249744454870" datatype="html">
         <source><x id="PH" equiv-text="this.percentResponded"/>% Responded (<x id="PH_1" equiv-text="this.numResponses"/>/<x id="PH_2" equiv-text="this.totalWorkgroups"/>)</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">749</context>
+          <context context-type="linenumber">743</context>
         </context-group>
       </trans-unit>
       <trans-unit id="8177873832400820695" datatype="html">
         <source>Count</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/directives/summary-display/summary-display.component.ts</context>
-          <context context-type="linenumber">866</context>
+          <context context-type="linenumber">860</context>
         </context-group>
       </trans-unit>
       <trans-unit id="d53b401d14b43833857138029db823612e52f518" datatype="html">