From 6b9d87e7a09087aba6448ba76864d63d8872c7df Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Mon, 25 Nov 2024 15:54:33 -0800 Subject: [PATCH] refactor(SummaryDisplay): Simplify data service initialization and improve subscription handling (#2001) --- .../student-summary-display.component.ts | 24 +++++----- .../summary-display.component.ts | 46 ++++++++----------- .../teacher-summary-display.component.ts | 10 ++-- src/messages.xlf | 26 +++++------ 4 files changed, 47 insertions(+), 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 index 53d69029c5c..e6769fdc384 100644 --- 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,6 +5,7 @@ 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', @@ -12,32 +13,30 @@ import { StudentDataService } from '../../services/studentDataService'; 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 && @@ -45,7 +44,6 @@ export class StudentSummaryDisplay extends SummaryDisplay { ) { 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 index 9a52697777b..806f3cd6fab 100644 --- 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,6 +68,7 @@ export abstract class SummaryDisplay { constructor( protected annotationService: AnnotationService, protected configService: ConfigService, + protected dataService: DataService, protected projectService: ProjectService, protected summaryService: SummaryService ) {} @@ -75,7 +76,6 @@ export abstract class SummaryDisplay { 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 { + private getDummyStudentWorkForVLEPreview(nodeId: string, componentId: string): Observable { 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,7 +336,7 @@ 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)); @@ -349,7 +344,7 @@ export abstract class SummaryDisplay { 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 index d7f38364768..278e4cae28a 100644 --- 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 index b46074d6576..0b82916f1a2 100644 --- a/src/messages.xlf +++ b/src/messages.xlf @@ -19706,11 +19706,11 @@ Warning: This will delete all existing choices and buckets in this component. src/assets/wise5/directives/summary-display/summary-display.component.ts - 576 + 570 src/assets/wise5/directives/summary-display/summary-display.component.ts - 686 + 680 @@ -19721,11 +19721,11 @@ Warning: This will delete all existing choices and buckets in this component. src/assets/wise5/directives/summary-display/summary-display.component.ts - 576 + 570 src/assets/wise5/directives/summary-display/summary-display.component.ts - 690 + 684 @@ -21574,63 +21574,63 @@ If this problem continues, let your teacher know and move on to the next activit The student will see a graph of their individual data here. src/assets/wise5/directives/summary-display/summary-display.component.ts - 154 + 150 Your Response src/assets/wise5/directives/summary-display/summary-display.component.ts - 710 + 704 Your Score src/assets/wise5/directives/summary-display/summary-display.component.ts - 712 + 706 Period Responses src/assets/wise5/directives/summary-display/summary-display.component.ts - 719 + 713 Period Scores src/assets/wise5/directives/summary-display/summary-display.component.ts - 724 + 718 Class Responses src/assets/wise5/directives/summary-display/summary-display.component.ts - 733 + 727 Class Scores src/assets/wise5/directives/summary-display/summary-display.component.ts - 738 + 732 % Responded (/) src/assets/wise5/directives/summary-display/summary-display.component.ts - 749 + 743 Count src/assets/wise5/directives/summary-display/summary-display.component.ts - 866 + 860