From 22cded7f37aaafe96a67185a5b1dba37041f29a3 Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Wed, 27 Nov 2024 13:15:52 -0800 Subject: [PATCH] refactor(StudentDataService, VLEParentComponent): simplify logic for node retrieval --- src/app/services/studentDataService.spec.ts | 25 -------- .../wise5/services/studentDataService.ts | 22 ------- src/assets/wise5/vle/tsconfig.json | 19 +++++-- .../vle-parent/vle-parent.component.spec.ts | 57 ++++++++++++------- .../vle/vle-parent/vle-parent.component.ts | 36 ++++++++---- src/messages.xlf | 8 +-- 6 files changed, 81 insertions(+), 86 deletions(-) diff --git a/src/app/services/studentDataService.spec.ts b/src/app/services/studentDataService.spec.ts index 5e747fa8b66..7423a0faa7d 100644 --- a/src/app/services/studentDataService.spec.ts +++ b/src/app/services/studentDataService.spec.ts @@ -40,7 +40,6 @@ describe('StudentDataService', () => { shouldGetComponentStatesByNodeId(); shouldGetComponentStatesByNodeIdAndComponentId(); shouldGetEventsByNodeId(); - shouldGetLatestNodeEnteredEventNodeIdWithExistingNode(); shouldCalculateCanVisitNode(); shouldGetNodeStatusByNodeId(); shouldGetLatestComponentStatesByNodeId(); @@ -428,30 +427,6 @@ function shouldGetEventsByNodeId() { }); } -function shouldGetLatestNodeEnteredEventNodeIdWithExistingNode() { - it('should get latest node entered event node id with existing node', () => { - service.studentData = { - events: [ - createEvent(1, 'node1', 'component1', 'nodeEntered'), - createEvent(2, 'node1', 'component2', 'nodeEntered'), - createEvent(3, 'node2', 'component3', 'nodeEntered'), - createEvent(4, 'node3', 'component4', 'nodeEntered'), - createEvent(5, 'node1', 'component1', 'nodeEntered') - ] - }; - spyOn(projectService, 'getNodeById').and.callFake((nodeId) => { - return { - id: nodeId - }; - }); - spyOn(projectService, 'isActive').and.callFake((nodeId) => { - return nodeId === 'node1'; - }); - const nodeId = service.getLatestNodeEnteredEventNodeIdWithExistingNode(); - expect(nodeId).toEqual('node1'); - }); -} - function shouldCalculateCanVisitNode() { it('should calculate can visit node false', () => { service.nodeStatuses = { diff --git a/src/assets/wise5/services/studentDataService.ts b/src/assets/wise5/services/studentDataService.ts index 6732fa1dab6..ca7ad46413f 100644 --- a/src/assets/wise5/services/studentDataService.ts +++ b/src/assets/wise5/services/studentDataService.ts @@ -1,5 +1,3 @@ -'use strict'; - import { Injectable } from '@angular/core'; import { ConfigService } from './configService'; import { AnnotationService } from './annotationService'; @@ -569,26 +567,6 @@ export class StudentDataService extends DataService { return this.studentData.events.filter((event) => event.nodeId === nodeId); } - /** - * Get the node id of the latest node entered event for an active node that - * exists in the project. We need to check if the node exists in the project - * in case the node has been deleted from the project. We also need to check - * that the node is active in case the node has been moved to the inactive - * section of the project. - * @return the node id of the latest node entered event for an active node - * that exists in the project - */ - getLatestNodeEnteredEventNodeIdWithExistingNode(): string { - const event = this.studentData.events.findLast( - (event) => event.event === 'nodeEntered' && this.isNodeExistAndActive(event.nodeId) - ); - return event?.nodeId ?? null; - } - - private isNodeExistAndActive(nodeId: string): boolean { - return this.ProjectService.getNodeById(nodeId) != null && this.ProjectService.isActive(nodeId); - } - getTotalScore() { return this.AnnotationService.getTotalScore(this.studentData.annotations); } diff --git a/src/assets/wise5/vle/tsconfig.json b/src/assets/wise5/vle/tsconfig.json index f55890fff66..3a7b350453d 100644 --- a/src/assets/wise5/vle/tsconfig.json +++ b/src/assets/wise5/vle/tsconfig.json @@ -8,9 +8,18 @@ "moduleResolution": "node", "emitDecoratorMetadata": true, "experimentalDecorators": true, - "target": "es5", - "typeRoots": ["node_modules/@types"], - "lib": ["es2017", "dom"] + "target": "es2023", + "typeRoots": [ + "node_modules/@types" + ], + "lib": [ + "es2023", + "dom" + ] }, - "include": ["src/**/*.ts", "../services/projectService.ts", "**/*.ts"] -} + "include": [ + "src/**/*.ts", + "../services/projectService.ts", + "**/*.ts" + ] +} \ No newline at end of file diff --git a/src/assets/wise5/vle/vle-parent/vle-parent.component.spec.ts b/src/assets/wise5/vle/vle-parent/vle-parent.component.spec.ts index a6c68e657e9..7d76484bba3 100644 --- a/src/assets/wise5/vle/vle-parent/vle-parent.component.spec.ts +++ b/src/assets/wise5/vle/vle-parent/vle-parent.component.spec.ts @@ -1,8 +1,6 @@ -import { provideHttpClientTesting } from '@angular/common/http/testing'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { MatDialogModule } from '@angular/material/dialog'; -import { Router } from '@angular/router'; -import { RouterTestingModule } from '@angular/router/testing'; +import { provideRouter, Router, RouterModule } from '@angular/router'; import { BehaviorSubject } from 'rxjs'; import { StudentTeacherCommonServicesModule } from '../../../../app/student-teacher-common-services.module'; import { InitializeVLEService } from '../../services/initializeVLEService'; @@ -17,30 +15,31 @@ import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http' let component: VLEParentComponent; let fixture: ComponentFixture; let initializeVLEService: InitializeVLEService; +let dataService: StudentDataService; +let projectService: VLEProjectService; const nodeId1: string = 'node1'; let router: Router; const runId1: string = '1'; - describe('VLEParentComponent', () => { beforeEach(async () => { await TestBed.configureTestingModule({ - declarations: [VLEParentComponent], - imports: [MatDialogModule, - RouterTestingModule, - StudentTeacherCommonServicesModule], - providers: [ + declarations: [VLEParentComponent], + imports: [MatDialogModule, RouterModule, StudentTeacherCommonServicesModule], + providers: [ InitializeVLEService, PauseScreenService, ProjectService, StudentNotificationService, VLEProjectService, provideHttpClient(withInterceptorsFromDi()), - provideHttpClientTesting() - ] -}).compileComponents(); + provideRouter([]) + ] + }).compileComponents(); fixture = TestBed.createComponent(VLEParentComponent); component = fixture.componentInstance; initializeVLEService = TestBed.inject(InitializeVLEService); + dataService = TestBed.inject(StudentDataService); + projectService = TestBed.inject(VLEProjectService); router = TestBed.inject(Router); }); ngOnInit(); @@ -50,6 +49,7 @@ function ngOnInit() { describe('ngOnInit()', () => { initialize(); previewConstraints(); + initializeStudent(); }); } @@ -73,11 +73,27 @@ function expectInitialize(functionName: any, runId: string): void { function previewConstraints() { it('should set the starting node id when constraints are enabled', () => { setRouterUrl(`/preview/unit/${runId1}/${nodeId1}`); - expectSetCurrentNode(nodeId1); + expectSetCurrentNode(nodeId1, true); }); it('should set the starting node id when constraints are disabled', () => { setRouterUrl(`/preview/unit/${runId1}/${nodeId1}?constraints=false`); - expectSetCurrentNode(nodeId1); + expectSetCurrentNode(nodeId1, true); + }); +} + +function initializeStudent() { + it('should set the starting node id when there is no last NodeEntered event', () => { + setRouterUrl(`/unit/${runId1}`); + spyOn(dataService, 'getEvents').and.returnValue([]); + spyOn(projectService, 'getStartNodeId').and.returnValue('node2'); + expectSetCurrentNode('node2', false); + }); + it('should set the starting node id when there is last NodeEntered event', () => { + setRouterUrl(`/unit/${runId1}`); + spyOn(dataService, 'getEvents').and.returnValue([{ event: 'nodeEntered', nodeId: 'node32' }]); + spyOn(projectService, 'getNodeById').and.returnValue({}); + spyOn(projectService, 'isActive').and.returnValue(true); + expectSetCurrentNode('node32', false); }); } @@ -85,11 +101,14 @@ function setRouterUrl(url: string): void { spyOnProperty(router, 'url', 'get').and.returnValue(url); } -function expectSetCurrentNode(nodeId: string) { - spyOn(initializeVLEService, 'initializePreview').and.callFake(() => { - setInitialized(true); - return Promise.resolve(); - }); +function expectSetCurrentNode(nodeId: string, isPreview: boolean) { + spyOn(initializeVLEService, isPreview ? 'initializePreview' : 'initializeStudent').and.callFake( + () => { + setInitialized(true); + return Promise.resolve(); + } + ); + const setCurrentNodeIdSpy = spyOn(TestBed.inject(StudentDataService), 'setCurrentNodeByNodeId'); spyOn(router, 'navigate').and.callFake(() => { return Promise.resolve(true); diff --git a/src/assets/wise5/vle/vle-parent/vle-parent.component.ts b/src/assets/wise5/vle/vle-parent/vle-parent.component.ts index d851b22d4d2..15bc523d5f1 100644 --- a/src/assets/wise5/vle/vle-parent/vle-parent.component.ts +++ b/src/assets/wise5/vle/vle-parent/vle-parent.component.ts @@ -9,18 +9,18 @@ import { VLEProjectService } from '../vleProjectService'; }) export class VLEParentComponent implements OnInit { constructor( + private dataService: StudentDataService, private initializeVLEService: InitializeVLEService, private projectService: VLEProjectService, private route: ActivatedRoute, - private router: Router, - private studentDataService: StudentDataService + private router: Router ) {} ngOnInit(): void { this.initializeVLEService.initialized$.subscribe((initialized: boolean) => { if (initialized) { const startingNodeId = this.getStartingNodeId(); - this.studentDataService.setCurrentNodeByNodeId(startingNodeId); + this.dataService.setCurrentNodeByNodeId(startingNodeId); this.router.navigate([startingNodeId], { relativeTo: this.route.parent }); } }); @@ -34,13 +34,27 @@ export class VLEParentComponent implements OnInit { private getStartingNodeId(): string { const urlMatch = this.router.url.match(/unit\/[0-9]*\/([^?]*)/); - let nodeId = - urlMatch != null - ? urlMatch[1] - : this.studentDataService.getLatestNodeEnteredEventNodeIdWithExistingNode(); - if (nodeId == null) { - nodeId = this.projectService.getStartNodeId(); - } - return nodeId; + return urlMatch != null + ? urlMatch[1] + : (this.getLastNodeEnteredEvent()?.nodeId ?? this.projectService.getStartNodeId()); + } + + /** + * Get the last node entered event for an active node that exists in the project. + * We need to check if the node exists in the project in case the node has been deleted + * from the project. We also need to check that the node is active in case the node has been + * moved to the inactive section of the project. + * @return the last node entered event for an active node that exists in the project + */ + private getLastNodeEnteredEvent(): any { + return this.dataService + .getEvents() + .findLast( + (event) => event.event === 'nodeEntered' && this.isNodeExistAndActive(event.nodeId) + ); + } + + private isNodeExistAndActive(nodeId: string): boolean { + return this.projectService.getNodeById(nodeId) != null && this.projectService.isActive(nodeId); } } diff --git a/src/messages.xlf b/src/messages.xlf index 3b108413298..5d2fd0662a3 100644 --- a/src/messages.xlf +++ b/src/messages.xlf @@ -22022,28 +22022,28 @@ If this problem continues, let your teacher know and move on to the next activit Preview Student src/assets/wise5/services/studentDataService.ts - 78 + 76 StudentDataService.saveComponentEvent: component, category, event args must not be null src/assets/wise5/services/studentDataService.ts - 245 + 243 StudentDataService.saveComponentEvent: nodeId, componentId, componentType must not be null src/assets/wise5/services/studentDataService.ts - 255 + 253 StudentDataService.saveVLEEvent: category and event args must not be null src/assets/wise5/services/studentDataService.ts - 264 + 262