From 89fcb6a81126f188520258e866a12902d8661b21 Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Tue, 17 Dec 2024 15:43:15 -0800 Subject: [PATCH] refactor(NodeService): Clean up code (#2023) --- .../dialog-guidance-authoring.module.ts | 4 +- .../embedded-authoring.module.ts | 4 +- .../wise5/services/gradingNodeService.ts | 26 ----- src/assets/wise5/services/nodeService.ts | 102 ++++-------------- .../wise5/services/studentNodeService.ts | 34 +++++- .../wise5/services/teacherNodeService.ts | 17 +++ 6 files changed, 73 insertions(+), 114 deletions(-) diff --git a/src/assets/wise5/components/dialogGuidance/dialog-guidance-authoring/dialog-guidance-authoring.module.ts b/src/assets/wise5/components/dialogGuidance/dialog-guidance-authoring/dialog-guidance-authoring.module.ts index 4103f054dee..d6119cc1183 100644 --- a/src/assets/wise5/components/dialogGuidance/dialog-guidance-authoring/dialog-guidance-authoring.module.ts +++ b/src/assets/wise5/components/dialogGuidance/dialog-guidance-authoring/dialog-guidance-authoring.module.ts @@ -12,7 +12,6 @@ import { EditComponentPrompt } from '../../../../../app/authoring-tool/edit-comp import { ProjectAssetService } from '../../../../../app/services/projectAssetService'; import { AnnotationService } from '../../../services/annotationService'; import { ConfigService } from '../../../services/configService'; -import { NodeService } from '../../../services/nodeService'; import { ProjectService } from '../../../services/projectService'; import { SessionService } from '../../../services/sessionService'; import { StudentDataService } from '../../../services/studentDataService'; @@ -24,6 +23,7 @@ import { ComputerAvatarService } from '../../../services/computerAvatarService'; import { DialogGuidanceService } from '../dialogGuidanceService'; import { FeedbackRuleHelpComponent } from '../../common/feedbackRule/feedback-rule-help/feedback-rule-help.component'; import { ComponentAuthoringModule } from '../../component-authoring.module'; +import { TeacherNodeService } from '../../../services/teacherNodeService'; @NgModule({ declarations: [ @@ -49,12 +49,12 @@ import { ComponentAuthoringModule } from '../../component-authoring.module'; ComputerAvatarService, ConfigService, DialogGuidanceService, - NodeService, ProjectAssetService, ProjectService, SessionService, StudentDataService, TagService, + TeacherNodeService, TeacherProjectService ], exports: [ diff --git a/src/assets/wise5/components/embedded/embedded-authoring/embedded-authoring.module.ts b/src/assets/wise5/components/embedded/embedded-authoring/embedded-authoring.module.ts index 68ba3674884..9aea421ce8f 100644 --- a/src/assets/wise5/components/embedded/embedded-authoring/embedded-authoring.module.ts +++ b/src/assets/wise5/components/embedded/embedded-authoring/embedded-authoring.module.ts @@ -12,7 +12,6 @@ import { EditComponentPrompt } from '../../../../../app/authoring-tool/edit-comp import { ProjectAssetService } from '../../../../../app/services/projectAssetService'; import { AnnotationService } from '../../../services/annotationService'; import { ConfigService } from '../../../services/configService'; -import { NodeService } from '../../../services/nodeService'; import { ProjectService } from '../../../services/projectService'; import { SessionService } from '../../../services/sessionService'; import { StudentAssetService } from '../../../services/studentAssetService'; @@ -22,6 +21,7 @@ import { TeacherProjectService } from '../../../services/teacherProjectService'; import { EmbeddedService } from '../embeddedService'; import { EmbeddedAuthoring } from './embedded-authoring.component'; import { ComponentAuthoringModule } from '../../component-authoring.module'; +import { TeacherNodeService } from '../../../services/teacherNodeService'; @NgModule({ declarations: [EmbeddedAuthoring, AuthorUrlParametersComponent], @@ -41,13 +41,13 @@ import { ComponentAuthoringModule } from '../../component-authoring.module'; AnnotationService, ConfigService, EmbeddedService, - NodeService, ProjectAssetService, ProjectService, SessionService, StudentAssetService, StudentDataService, TagService, + TeacherNodeService, TeacherProjectService ], exports: [EmbeddedAuthoring, EditComponentPrompt, AuthorUrlParametersComponent] diff --git a/src/assets/wise5/services/gradingNodeService.ts b/src/assets/wise5/services/gradingNodeService.ts index d4c83199cb4..4f568a11618 100644 --- a/src/assets/wise5/services/gradingNodeService.ts +++ b/src/assets/wise5/services/gradingNodeService.ts @@ -3,11 +3,6 @@ import { TeacherNodeService } from './teacherNodeService'; @Injectable() export class GradingNodeService extends TeacherNodeService { - /** - * Get the next node id in the project sequence that captures student work - * @param currentId (optional) - * @returns next node id - */ getNextNodeId(currentId = null): Promise { return super.getNextNodeId(currentId).then((nextNodeId: string) => { if (!nextNodeId) return null; @@ -17,31 +12,10 @@ export class GradingNodeService extends TeacherNodeService { }); } - /** - * Go to the next node that captures work - * @return a promise that will return the next node id - */ - goToNextNode(): Promise { - return this.getNextNodeId().then((nextNodeId: string) => { - if (nextNodeId) { - this.setCurrentNode(nextNodeId); - } - return nextNodeId; - }); - } - - /** - * Go to the previous node that captures work - */ goToPrevNode(): void { this.setCurrentNode(this.getPrevNodeId()); } - /** - * Get the previous node id in the project sequence that captures student work - * @param currentId (optional) - * @returns next node id - */ getPrevNodeId(currentId = null) { const prevNodeId = super.getPrevNodeId(currentId); if (!prevNodeId) return null; diff --git a/src/assets/wise5/services/nodeService.ts b/src/assets/wise5/services/nodeService.ts index 780f1261396..22f751b3ca0 100644 --- a/src/assets/wise5/services/nodeService.ts +++ b/src/assets/wise5/services/nodeService.ts @@ -9,7 +9,7 @@ import { ConstraintService } from './constraintService'; import { TransitionLogic } from '../common/TransitionLogic'; @Injectable() -export class NodeService { +export abstract class NodeService { private transitionResults = {}; private chooseTransitionPromises = {}; private nodeSubmitClickedSource: Subject = new Subject(); @@ -18,11 +18,11 @@ export class NodeService { public doneRenderingComponent$ = this.doneRenderingComponentSource.asObservable(); constructor( + protected dataService: DataService, protected dialog: MatDialog, protected configService: ConfigService, protected constraintService: ConstraintService, - protected projectService: ProjectService, - protected dataService: DataService + protected projectService: ProjectService ) {} setCurrentNode(nodeId: string): void { @@ -30,7 +30,7 @@ export class NodeService { } goToNextNode(): Promise { - return this.getNextNodeId().then((nextNodeId) => { + return this.getNextNodeId().then((nextNodeId: string) => { if (nextNodeId != null) { this.setCurrentNode(nextNodeId); } @@ -38,63 +38,13 @@ export class NodeService { }); } - /** - * This function should be implemented by the child service classes - */ - getNextNodeId(currentId?: string): Promise { - return null; - } + abstract getNextNodeId(currentId?: string): Promise; - goToPrevNode() { - const prevNodeId = this.getPrevNodeId(); - this.setCurrentNode(prevNodeId); + goToPrevNode(): void { + this.setCurrentNode(this.getPrevNodeId()); } - /** - * Get the previous node in the project sequence - * @param currentId (optional) - */ - getPrevNodeId(currentId?: string): string { - let prevNodeId = null; - const currentNodeId = currentId ?? this.dataService.getCurrentNodeId(); - if (currentNodeId) { - if (['author', 'classroomMonitor'].includes(this.configService.getMode())) { - const currentNodeOrder = this.projectService.getNodeOrderById(currentNodeId); - if (currentNodeOrder) { - const prevId = this.projectService.getNodeIdByOrder(currentNodeOrder - 1); - if (prevId) { - prevNodeId = this.projectService.isApplicationNode(prevId) - ? prevId - : this.getPrevNodeId(prevId); - } - } - } else { - // get all the nodes that transition to the current node - const nodeIdsByToNodeId = this.projectService - .getNodesByToNodeId(currentNodeId) - .map((node) => node.id); - if (nodeIdsByToNodeId.length === 1) { - // there is only one node that transitions to the current node - prevNodeId = nodeIdsByToNodeId[0]; - } else if (nodeIdsByToNodeId.length > 1) { - // there are multiple nodes that transition to the current node - - const stackHistory = this.dataService.getStackHistory(); - - // loop through the stack history node ids from newest to oldest - for (let s = stackHistory.length - 1; s >= 0; s--) { - const stackHistoryNodeId = stackHistory[s]; - if (nodeIdsByToNodeId.indexOf(stackHistoryNodeId) != -1) { - // we have found a node that we previously visited that transitions to the current node - prevNodeId = stackHistoryNodeId; - break; - } - } - } - } - } - return prevNodeId; - } + abstract getPrevNodeId(currentId?: string): string; /** * Close the current node (and open the current node's parent group) @@ -159,7 +109,7 @@ export class NodeService { * they last chose and not ask them again */ } else { - this.letUserChooseTransition(availableTransitions, nodeId, resolve); + this.letUserChooseTransition(availableTransitions, resolve); } } else { transitionResult = this.chooseTransitionAutomatically( @@ -184,28 +134,18 @@ export class NodeService { ); } - private letUserChooseTransition( - availableTransitions: any[], - nodeId: string, - resolve: (value: any) => void - ): void { - const paths = []; - for (const availableTransition of availableTransitions) { - const toNodeId = availableTransition.to; - const path = { - nodeId: toNodeId, - nodeTitle: this.projectService.getNodePositionAndTitle(toNodeId), - transition: availableTransition - }; - paths.push(path); - } - const dialogRef = this.dialog.open(ChooseBranchPathDialogComponent, { - data: paths, - disableClose: true - }); - dialogRef.afterClosed().subscribe((result) => { - resolve(result); - }); + private letUserChooseTransition(transitions: any[], resolve: (value: any) => void): void { + this.dialog + .open(ChooseBranchPathDialogComponent, { + data: transitions.map((transition) => ({ + nodeId: transition.to, + nodeTitle: this.projectService.getNodePositionAndTitle(transition.to), + transition: transition + })), + disableClose: true + }) + .afterClosed() + .subscribe((result) => resolve(result)); } private chooseTransitionAutomatically( diff --git a/src/assets/wise5/services/studentNodeService.ts b/src/assets/wise5/services/studentNodeService.ts index 585efdaaa56..5b1a7e32f18 100644 --- a/src/assets/wise5/services/studentNodeService.ts +++ b/src/assets/wise5/services/studentNodeService.ts @@ -13,14 +13,14 @@ import { TransitionLogic } from '../common/TransitionLogic'; @Injectable() export class StudentNodeService extends NodeService { constructor( + protected dataService: DataService, protected dialog: MatDialog, protected configService: ConfigService, protected constraintService: ConstraintService, private nodeStatusService: NodeStatusService, - protected projectService: ProjectService, - protected dataService: DataService + protected projectService: ProjectService ) { - super(dialog, configService, constraintService, projectService, dataService); + super(dataService, dialog, configService, constraintService, projectService); } setCurrentNode(nodeId: string): void { @@ -71,6 +71,34 @@ export class StudentNodeService extends NodeService { .join('
'); } + getPrevNodeId(currentId?: string): string { + let prevNodeId = null; + const currentNodeId = currentId ?? this.dataService.getCurrentNodeId(); + if (currentNodeId) { + // get all the nodes that transition to the current node + const nodeIdsByToNodeId = this.projectService + .getNodesByToNodeId(currentNodeId) + .map((node) => node.id); + if (nodeIdsByToNodeId.length === 1) { + // there is only one node that transitions to the current node + prevNodeId = nodeIdsByToNodeId[0]; + } else if (nodeIdsByToNodeId.length > 1) { + // there are multiple nodes that transition to the current node + const stackHistory = this.dataService.getStackHistory(); + // loop through the stack history node ids from newest to oldest + for (let s = stackHistory.length - 1; s >= 0; s--) { + const stackHistoryNodeId = stackHistory[s]; + if (nodeIdsByToNodeId.indexOf(stackHistoryNodeId) != -1) { + // we have found a node that we previously visited that transitions to the current node + prevNodeId = stackHistoryNodeId; + break; + } + } + } + } + return prevNodeId; + } + /** * Get the next node in the project sequence. We return a promise because in preview mode we allow * the user to specify which branch path they want to go to. In all other cases we will resolve diff --git a/src/assets/wise5/services/teacherNodeService.ts b/src/assets/wise5/services/teacherNodeService.ts index ad28a97fadf..5b3e1a0a70e 100644 --- a/src/assets/wise5/services/teacherNodeService.ts +++ b/src/assets/wise5/services/teacherNodeService.ts @@ -24,6 +24,23 @@ export class TeacherNodeService extends NodeService { this.starterStateResponseSource.next(args); } + getPrevNodeId(currentId?: string): string { + let prevNodeId = null; + const currentNodeId = currentId ?? this.dataService.getCurrentNodeId(); + if (currentNodeId) { + const currentNodeOrder = this.projectService.getNodeOrderById(currentNodeId); + if (currentNodeOrder) { + const prevId = this.projectService.getNodeIdByOrder(currentNodeOrder - 1); + if (prevId) { + prevNodeId = this.projectService.isApplicationNode(prevId) + ? prevId + : this.getPrevNodeId(prevId); + } + } + } + return prevNodeId; + } + getNextNodeId(currentId?: string): Promise { return new Promise((resolve, reject) => { let nextNodeId = null;