Skip to content

Commit

Permalink
refactor(StudentDataService, VLEParentComponent): simplify logic for …
Browse files Browse the repository at this point in the history
…node retrieval (#2012)
  • Loading branch information
hirokiterashima authored Nov 27, 2024
1 parent 5a27618 commit 2cc9cea
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 86 deletions.
25 changes: 0 additions & 25 deletions src/app/services/studentDataService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ describe('StudentDataService', () => {
shouldGetComponentStatesByNodeId();
shouldGetComponentStatesByNodeIdAndComponentId();
shouldGetEventsByNodeId();
shouldGetLatestNodeEnteredEventNodeIdWithExistingNode();
shouldCalculateCanVisitNode();
shouldGetNodeStatusByNodeId();
shouldGetLatestComponentStatesByNodeId();
Expand Down Expand Up @@ -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 = {
Expand Down
22 changes: 0 additions & 22 deletions src/assets/wise5/services/studentDataService.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
'use strict';

import { Injectable } from '@angular/core';
import { ConfigService } from './configService';
import { AnnotationService } from './annotationService';
Expand Down Expand Up @@ -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);
}
Expand Down
19 changes: 14 additions & 5 deletions src/assets/wise5/vle/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
57 changes: 38 additions & 19 deletions src/assets/wise5/vle/vle-parent/vle-parent.component.spec.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -17,30 +15,31 @@ import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'
let component: VLEParentComponent;
let fixture: ComponentFixture<VLEParentComponent>;
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();
Expand All @@ -50,6 +49,7 @@ function ngOnInit() {
describe('ngOnInit()', () => {
initialize();
previewConstraints();
initializeStudent();
});
}

Expand All @@ -73,23 +73,42 @@ 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);
});
}

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);
Expand Down
36 changes: 25 additions & 11 deletions src/assets/wise5/vle/vle-parent/vle-parent.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
});
Expand All @@ -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);
}
}
8 changes: 4 additions & 4 deletions src/messages.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -22022,28 +22022,28 @@ If this problem continues, let your teacher know and move on to the next activit
<source>Preview Student</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/services/studentDataService.ts</context>
<context context-type="linenumber">78</context>
<context context-type="linenumber">76</context>
</context-group>
</trans-unit>
<trans-unit id="1967824796880640028" datatype="html">
<source>StudentDataService.saveComponentEvent: component, category, event args must not be null</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/services/studentDataService.ts</context>
<context context-type="linenumber">245</context>
<context context-type="linenumber">243</context>
</context-group>
</trans-unit>
<trans-unit id="944638387659785956" datatype="html">
<source>StudentDataService.saveComponentEvent: nodeId, componentId, componentType must not be null</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/services/studentDataService.ts</context>
<context context-type="linenumber">255</context>
<context context-type="linenumber">253</context>
</context-group>
</trans-unit>
<trans-unit id="4924640921903672943" datatype="html">
<source>StudentDataService.saveVLEEvent: category and event args must not be null</source>
<context-group purpose="location">
<context context-type="sourcefile">src/assets/wise5/services/studentDataService.ts</context>
<context context-type="linenumber">264</context>
<context context-type="linenumber">262</context>
</context-group>
</trans-unit>
<trans-unit id="3178064088723974543" datatype="html">
Expand Down

0 comments on commit 2cc9cea

Please sign in to comment.