From 51a942800388235948a0e31cf63ddf37a796439c Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Mon, 1 May 2023 17:01:58 -0700 Subject: [PATCH] refactor(TeacherProjectService): Move branch authoring code #1220 --- .../services/teacherProjectService.spec.ts | 20 --- ...ode-advanced-branch-authoring.component.ts | 169 ++++++++++++------ src/assets/wise5/services/projectService.ts | 8 +- .../wise5/services/teacherProjectService.ts | 103 +---------- src/messages.xlf | 14 +- 5 files changed, 125 insertions(+), 189 deletions(-) diff --git a/src/app/services/teacherProjectService.spec.ts b/src/app/services/teacherProjectService.spec.ts index 7318ab7731f..5af2915107a 100644 --- a/src/app/services/teacherProjectService.spec.ts +++ b/src/app/services/teacherProjectService.spec.ts @@ -42,7 +42,6 @@ describe('TeacherProjectService', () => { service.applicationNodes = []; }); registerNewProject(); - isNodeIdUsed(); isNodeIdToInsertTargetNotSpecified(); testDeleteComponent(); testDeleteTransition(); @@ -128,25 +127,6 @@ function registerNewProject() { }); } -function isNodeIdUsed() { - describe('isNodeIdUsed', () => { - beforeEach(() => { - service.setProject(demoProjectJSON); - }); - it('should find used node id in active nodes', () => { - expect(service.isNodeIdUsed('node1')).toEqual(true); - }); - - it('should find used node id in inactive nodes', () => { - expect(service.isNodeIdUsed('node789')).toEqual(true); - }); - - it('should not find used node id in active or inactive nodes', () => { - expect(service.isNodeIdUsed('nodedoesnotexist')).toEqual(false); - }); - }); -} - function isNodeIdToInsertTargetNotSpecified() { describe('isNodeIdToInsertTargetNotSpecified', () => { it('should return true for null, inactive nodes, steps, and activities', () => { diff --git a/src/assets/wise5/authoringTool/node/advanced/branch/node-advanced-branch-authoring.component.ts b/src/assets/wise5/authoringTool/node/advanced/branch/node-advanced-branch-authoring.component.ts index 530652c27b8..710a568b5cd 100644 --- a/src/assets/wise5/authoringTool/node/advanced/branch/node-advanced-branch-authoring.component.ts +++ b/src/assets/wise5/authoringTool/node/advanced/branch/node-advanced-branch-authoring.component.ts @@ -262,58 +262,30 @@ export class NodeAdvancedBranchAuthoringComponent implements OnInit { if (this.createBranchCriterion != null) { let nodeId = this.node.id; if (this.createBranchCriterion === 'workgroupId') { - this.ProjectService.setTransitionLogicField( - nodeId, - 'howToChooseAmongAvailablePaths', - 'workgroupId' - ); - this.ProjectService.setTransitionLogicField(nodeId, 'whenToChoosePath', 'enterNode'); - this.ProjectService.setTransitionLogicField(nodeId, 'canChangePath', false); - this.ProjectService.setTransitionLogicField(nodeId, 'maxPathsVisitable', 1); + this.setTransitionLogicField(nodeId, 'howToChooseAmongAvailablePaths', 'workgroupId'); + this.setTransitionLogicField(nodeId, 'whenToChoosePath', 'enterNode'); + this.setTransitionLogicField(nodeId, 'canChangePath', false); + this.setTransitionLogicField(nodeId, 'maxPathsVisitable', 1); } else if (this.createBranchCriterion === 'score') { - this.ProjectService.setTransitionLogicField( - nodeId, - 'howToChooseAmongAvailablePaths', - 'random' - ); - this.ProjectService.setTransitionLogicField( - nodeId, - 'whenToChoosePath', - 'studentDataChanged' - ); - this.ProjectService.setTransitionLogicField(nodeId, 'canChangePath', false); - this.ProjectService.setTransitionLogicField(nodeId, 'maxPathsVisitable', 1); + this.setTransitionLogicField(nodeId, 'howToChooseAmongAvailablePaths', 'random'); + this.setTransitionLogicField(nodeId, 'whenToChoosePath', 'studentDataChanged'); + this.setTransitionLogicField(nodeId, 'canChangePath', false); + this.setTransitionLogicField(nodeId, 'maxPathsVisitable', 1); } else if (this.createBranchCriterion === 'choiceChosen') { - this.ProjectService.setTransitionLogicField( - nodeId, - 'howToChooseAmongAvailablePaths', - 'random' - ); - this.ProjectService.setTransitionLogicField( - nodeId, - 'whenToChoosePath', - 'studentDataChanged' - ); - this.ProjectService.setTransitionLogicField(nodeId, 'canChangePath', false); - this.ProjectService.setTransitionLogicField(nodeId, 'maxPathsVisitable', 1); + this.setTransitionLogicField(nodeId, 'howToChooseAmongAvailablePaths', 'random'); + this.setTransitionLogicField(nodeId, 'whenToChoosePath', 'studentDataChanged'); + this.setTransitionLogicField(nodeId, 'canChangePath', false); + this.setTransitionLogicField(nodeId, 'maxPathsVisitable', 1); } else if (this.createBranchCriterion === 'random') { - this.ProjectService.setTransitionLogicField( - nodeId, - 'howToChooseAmongAvailablePaths', - 'random' - ); - this.ProjectService.setTransitionLogicField(nodeId, 'whenToChoosePath', 'enterNode'); - this.ProjectService.setTransitionLogicField(nodeId, 'canChangePath', false); - this.ProjectService.setTransitionLogicField(nodeId, 'maxPathsVisitable', 1); + this.setTransitionLogicField(nodeId, 'howToChooseAmongAvailablePaths', 'random'); + this.setTransitionLogicField(nodeId, 'whenToChoosePath', 'enterNode'); + this.setTransitionLogicField(nodeId, 'canChangePath', false); + this.setTransitionLogicField(nodeId, 'maxPathsVisitable', 1); } else if (this.createBranchCriterion === 'tag') { - this.ProjectService.setTransitionLogicField( - nodeId, - 'howToChooseAmongAvailablePaths', - 'tag' - ); - this.ProjectService.setTransitionLogicField(nodeId, 'whenToChoosePath', 'enterNode'); - this.ProjectService.setTransitionLogicField(nodeId, 'canChangePath', false); - this.ProjectService.setTransitionLogicField(nodeId, 'maxPathsVisitable', 1); + this.setTransitionLogicField(nodeId, 'howToChooseAmongAvailablePaths', 'tag'); + this.setTransitionLogicField(nodeId, 'whenToChoosePath', 'enterNode'); + this.setTransitionLogicField(nodeId, 'canChangePath', false); + this.setTransitionLogicField(nodeId, 'maxPathsVisitable', 1); } } this.createBranchUpdateTransitions(); @@ -569,9 +541,9 @@ export class NodeAdvancedBranchAuthoringComponent implements OnInit { if (item.checked) { let fromNodeId = this.nodeId; let toNodeId = firstNodeId; - this.ProjectService.addBranchPathTakenConstraints(nodeId, fromNodeId, toNodeId); + this.addBranchPathTakenConstraints(nodeId, fromNodeId, toNodeId); } else { - this.ProjectService.setTransition(nodeId, nodeIdAfter); + this.setTransition(nodeId, nodeIdAfter); } } @@ -583,7 +555,7 @@ export class NodeAdvancedBranchAuthoringComponent implements OnInit { // the branch path taken constraints will be from this node to the first node in the branch path const fromNodeId = this.nodeId; const toNodeId = firstNodeId; - this.ProjectService.addBranchPathTakenConstraints(itemNodeId, fromNodeId, toNodeId); + this.addBranchPathTakenConstraints(itemNodeId, fromNodeId, toNodeId); } this.ProjectService.calculateNodeNumbers(); this.saveProject(); @@ -662,12 +634,12 @@ export class NodeAdvancedBranchAuthoringComponent implements OnInit { * in the project. this may be different than the next step * if it was still the branch point. */ - this.ProjectService.setTransition(nodeId, nodeIdAfter); + this.setTransition(nodeId, nodeIdAfter); - this.ProjectService.setTransitionLogicField(nodeId, 'howToChooseAmongAvailablePaths', null); - this.ProjectService.setTransitionLogicField(nodeId, 'whenToChoosePath', null); - this.ProjectService.setTransitionLogicField(nodeId, 'canChangePath', null); - this.ProjectService.setTransitionLogicField(nodeId, 'maxPathsVisitable', null); + this.setTransitionLogicField(nodeId, 'howToChooseAmongAvailablePaths', null); + this.setTransitionLogicField(nodeId, 'whenToChoosePath', null); + this.setTransitionLogicField(nodeId, 'canChangePath', null); + this.setTransitionLogicField(nodeId, 'maxPathsVisitable', null); this.createBranchNumberOfBranches = 1; this.createBranchCriterion = null; @@ -723,7 +695,7 @@ export class NodeAdvancedBranchAuthoringComponent implements OnInit { * if it was still in the branch path. */ const nodeIdAfter = this.ProjectService.getNodeIdAfter(nodeId); - this.ProjectService.setTransition(nodeId, nodeIdAfter); + this.setTransition(nodeId, nodeIdAfter); } } const branchPathIndex = this.createBranchBranches.indexOf(branch); @@ -772,4 +744,87 @@ export class NodeAdvancedBranchAuthoringComponent implements OnInit { } this.saveProject(); } + + private setTransitionLogicField(nodeId: string, field: string, value: any): void { + const node = this.ProjectService.getNodeById(nodeId); + const transitionLogic = node.transitionLogic; + if (transitionLogic != null) { + transitionLogic[field] = value; + } + } + + private setTransition(fromNodeId: string, toNodeId: string): void { + const node = this.ProjectService.getNodeById(fromNodeId); + const transitionLogic = node.transitionLogic; + if (transitionLogic != null) { + let transitions = transitionLogic.transitions; + if (transitions == null || transitions.length == 0) { + transitionLogic.transitions = []; + const transition = {}; + transitionLogic.transitions.push(); + transitions = transitionLogic.transitions; + } + + if (transitions != null && transitions.length > 0) { + // get the first transition. we will assume there is only one transition. + const transition = transitions[0]; + if (transition != null) { + transition.to = toNodeId; + } + } + } + } + + /** + * Add branch path taken constraints to the node + * @param targetNodeId the node to add the constraints to + * @param fromNodeId the from node id of the branch path taken constraint + * @param toNodeId the to node id of the branch path taken constraint + */ + private addBranchPathTakenConstraints( + targetNodeId: string, + fromNodeId: string, + toNodeId: string + ): void { + const node = this.ProjectService.getNodeById(targetNodeId); + node.constraints.push( + this.createBranchPathTakenContraint( + targetNodeId, + fromNodeId, + toNodeId, + 'makeThisNodeNotVisible' + ) + ); + node.constraints.push( + this.createBranchPathTakenContraint( + targetNodeId, + fromNodeId, + toNodeId, + 'makeThisNodeNotVisitable' + ) + ); + } + + private createBranchPathTakenContraint( + targetNodeId: string, + fromNodeId: string, + toNodeId: string, + action: string + ): any { + return { + id: this.ProjectService.getNextAvailableConstraintIdForNodeId(targetNodeId), + action: action, + targetId: targetNodeId, + removalConditional: 'all', + removalCriteria: [ + { + name: 'branchPathTaken', + params: { + fromNodeId: fromNodeId, + toNodeId: toNodeId + } + } + ] + }; + } } diff --git a/src/assets/wise5/services/projectService.ts b/src/assets/wise5/services/projectService.ts index de535e1d744..29cd18df81f 100644 --- a/src/assets/wise5/services/projectService.ts +++ b/src/assets/wise5/services/projectService.ts @@ -225,9 +225,9 @@ export class ProjectService { return this.groupNodes.filter((node) => activeNodeIds.includes(node.id)); } - instantiateDefaults(): void { - this.project.nodes = this.project.nodes ? this.project.nodes : []; - this.project.inactiveNodes = this.project.inactiveNodes ? this.project.inactiveNodes : []; + private instantiateDefaults(): void { + this.project.nodes = this.project.nodes ?? []; + this.project.inactiveNodes = this.project.inactiveNodes ?? []; } private calculateNodeOrderOfProject(): void { @@ -727,7 +727,7 @@ export class ProjectService { return nodesByToNodeId; } - getInactiveNodes(): any { + getInactiveNodes(): any[] { return this.project.inactiveNodes; } diff --git a/src/assets/wise5/services/teacherProjectService.ts b/src/assets/wise5/services/teacherProjectService.ts index 90de2004a4b..efffbaa5911 100644 --- a/src/assets/wise5/services/teacherProjectService.ts +++ b/src/assets/wise5/services/teacherProjectService.ts @@ -371,65 +371,8 @@ export class TeacherProjectService extends ProjectService { } } - isInactive(nodeId) { - for (const inactiveNode of this.getInactiveNodes()) { - if (inactiveNode.id === nodeId) { - return true; - } - } - return false; - } - - /** - * Check if a node id is already being used in the project - * @param nodeId check if this node id is already being used in the project - * @return whether the node id is already being used in the project - */ - isNodeIdUsed(nodeId) { - for (const node of this.getNodes().concat(this.getInactiveNodes())) { - if (node.id === nodeId) { - return true; - } - } - return false; - } - - /** - * Set a field in the transition logic of a node - */ - setTransitionLogicField(nodeId, field, value) { - const node = this.getNodeById(nodeId); - const transitionLogic = node.transitionLogic; - if (transitionLogic != null) { - transitionLogic[field] = value; - } - } - - /** - * Set the transition to value of a node - * @param fromNodeId the from node - * @param toNodeId the to node - */ - setTransition(fromNodeId, toNodeId) { - const node = this.getNodeById(fromNodeId); - const transitionLogic = node.transitionLogic; - if (transitionLogic != null) { - let transitions = transitionLogic.transitions; - if (transitions == null || transitions.length == 0) { - transitionLogic.transitions = []; - const transition = {}; - transitionLogic.transitions.push(transition); - transitions = transitionLogic.transitions; - } - - if (transitions != null && transitions.length > 0) { - // get the first transition. we will assume there is only one transition. - const transition = transitions[0]; - if (transition != null) { - transition.to = toNodeId; - } - } - } + private isInactive(nodeId: string): boolean { + return this.getInactiveNodes().some((node) => node.id === nodeId); } /** @@ -447,48 +390,6 @@ export class TeacherProjectService extends ProjectService { } } - /** - * Add branch path taken constraints to the node - * @param targetNodeId the node to add the constraints to - * @param fromNodeId the from node id of the branch path taken constraint - * @param toNodeId the to node id of the branch path taken constraint - */ - addBranchPathTakenConstraints(targetNodeId, fromNodeId, toNodeId) { - const node = this.getNodeById(targetNodeId); - const makeThisNodeNotVisibleConstraint = { - id: this.getNextAvailableConstraintIdForNodeId(targetNodeId), - action: 'makeThisNodeNotVisible', - targetId: targetNodeId, - removalConditional: 'all', - removalCriteria: [ - { - name: 'branchPathTaken', - params: { - fromNodeId: fromNodeId, - toNodeId: toNodeId - } - } - ] - }; - node.constraints.push(makeThisNodeNotVisibleConstraint); - const makeThisNodeNotVisitableConstraint = { - id: this.getNextAvailableConstraintIdForNodeId(targetNodeId), - action: 'makeThisNodeNotVisitable', - targetId: targetNodeId, - removalConditional: 'all', - removalCriteria: [ - { - name: 'branchPathTaken', - params: { - fromNodeId: fromNodeId, - toNodeId: toNodeId - } - } - ] - }; - node.constraints.push(makeThisNodeNotVisitableConstraint); - } - getProjectRubric(): any { return this.project.rubric; } diff --git a/src/messages.xlf b/src/messages.xlf index fd9709f3f72..729c7b1a2bb 100644 --- a/src/messages.xlf +++ b/src/messages.xlf @@ -9531,7 +9531,7 @@ Click "Cancel" to keep the invalid JSON open so you can fix it.Are you sure you want to remove the branch? src/assets/wise5/authoringTool/node/advanced/branch/node-advanced-branch-authoring.component.ts - 645 + 617 @@ -18812,42 +18812,42 @@ If this problem continues, let your teacher know and move on to the next activit All steps after this one will not be visitable until src/assets/wise5/services/teacherProjectService.ts - 997 + 898 All steps after this one will not be visible until src/assets/wise5/services/teacherProjectService.ts - 1000 + 901 All other steps will not be visitable until src/assets/wise5/services/teacherProjectService.ts - 1003 + 904 All other steps will not be visible until src/assets/wise5/services/teacherProjectService.ts - 1006 + 907 This step will not be visitable until src/assets/wise5/services/teacherProjectService.ts - 1009 + 910 This step will not be visible until src/assets/wise5/services/teacherProjectService.ts - 1012 + 913