refactor(NodeService): getNextNodeId() (#1470)

src/app/services/studentNodeService.spec.ts

@@ -5,11 +5,15 @@ import { HttpClientTestingModule } from '@angular/common/http/testing';
 import { MatDialog, MatDialogModule } from '@angular/material/dialog';
 import { NodeStatusService } from '../../assets/wise5/services/nodeStatusService';
 import { DataService } from './data.service';
+import { ProjectService } from '../../assets/wise5/services/projectService';
 
 let dataService: DataService;
 let dialog: MatDialog;
+const nodeId2 = 'node2';
 let nodeStatusService: NodeStatusService;
+let projectService: ProjectService;
 let service: StudentNodeService;
+
 describe('StudentNodeService', () => {
   beforeEach(() => {
     TestBed.configureTestingModule({
@@ -19,8 +23,10 @@ describe('StudentNodeService', () => {
     dialog = TestBed.inject(MatDialog);
     service = TestBed.inject(StudentNodeService);
     nodeStatusService = TestBed.inject(NodeStatusService);
+    projectService = TestBed.inject(ProjectService);
   });
   setCurrentNode();
+  getNextNodeId();
 });
 
 function setCurrentNode() {
@@ -57,3 +63,32 @@ function spyOnIsVisitable(isVisitable: boolean) {
     isVisitable: isVisitable
   });
 }
+
+function getNextNodeId() {
+  describe('getNextNodeId()', () => {
+    getNextNodeId_currentNodeHasTransition_getsNodeFromTransition();
+    getNextNodeId_previouslyBranched_getsNodeFromPreviousBranchPathTaken();
+  });
+}
+
+function getNextNodeId_previouslyBranched_getsNodeFromPreviousBranchPathTaken() {
+  describe('has previous branch path taken', () => {
+    it('gets the node id from the previous branch path taken', async () => {
+      spyOn(dataService, 'getBranchPathTakenEventsByNodeId').and.returnValue([
+        { data: { toNodeId: nodeId2 } }
+      ]);
+      expect(await service.getNextNodeId()).toEqual(nodeId2);
+    });
+  });
+}
+
+function getNextNodeId_currentNodeHasTransition_getsNodeFromTransition() {
+  describe('current node has a transition', () => {
+    it('gets the node id from the transition', async () => {
+      spyOn(projectService, 'getTransitionLogicByFromNodeId').and.returnValue({
+        transitions: [{ to: nodeId2 }]
+      });
+      expect(await service.getNextNodeId()).toEqual(nodeId2);
+    });
+  });
+}

src/app/services/teacherNodeService.spec.ts

@@ -0,0 +1,41 @@
+import { HttpClientTestingModule } from '@angular/common/http/testing';
+import { TestBed } from '@angular/core/testing';
+import { MatDialogModule } from '@angular/material/dialog';
+import { StudentTeacherCommonServicesModule } from '../student-teacher-common-services.module';
+import { DataService } from './data.service';
+import { ProjectService } from '../../assets/wise5/services/projectService';
+import { TeacherNodeService } from '../../assets/wise5/services/teacherNodeService';
+
+let dataService: DataService;
+const nodeId1 = 'node1';
+const nodeId2 = 'node2';
+let projectService: ProjectService;
+let service: TeacherNodeService;
+
+describe('TeacherNodeService', () => {
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      imports: [HttpClientTestingModule, MatDialogModule, StudentTeacherCommonServicesModule],
+      providers: [TeacherNodeService]
+    });
+    dataService = TestBed.inject(DataService);
+    projectService = TestBed.inject(ProjectService);
+    service = TestBed.inject(TeacherNodeService);
+  });
+  getNextNodeId();
+});
+
+function getNextNodeId() {
+  describe('getNextNodeId()', () => {
+    describe('the next node id is a step node', () => {
+      it('gets the next node id in the unit', async () => {
+        spyOn(dataService, 'getCurrentNodeId').and.returnValue(nodeId1);
+        spyOn(projectService, 'isApplicationNode').and.returnValue(true);
+        projectService.idToOrder = {};
+        projectService.idToOrder[nodeId1] = { order: 1 };
+        projectService.idToOrder[nodeId2] = { order: 2 };
+        expect(await service.getNextNodeId()).toEqual(nodeId2);
+      });
+    });
+  });
+}

src/assets/wise5/services/nodeService.ts

@@ -41,100 +41,10 @@ export class NodeService {
   }
 
   /**
-   * 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 the promise immediately.
-   * @param currentId (optional)
-   * @returns a promise that returns the next node id
+   * This function should be implemented by the child service classes
    */
   getNextNodeId(currentId?: string): Promise<any> {
-    const promise = new Promise((resolve, reject) => {
-      let nextNodeId = 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 nextId = this.ProjectService.getNodeIdByOrder(currentNodeOrder + 1);
-            if (nextId) {
-              nextNodeId = this.ProjectService.isApplicationNode(nextId)
-                ? nextId
-                : this.getNextNodeId(nextId);
-            }
-          }
-          resolve(nextNodeId);
-        } else {
-          const transitionLogic = this.ProjectService.getTransitionLogicByFromNodeId(currentNodeId);
-          const branchPathTakenEvents = this.DataService.getBranchPathTakenEventsByNodeId(
-            currentNodeId
-          );
-          if (
-            branchPathTakenEvents != null &&
-            branchPathTakenEvents.length > 0 &&
-            transitionLogic != null &&
-            transitionLogic.canChangePath != true
-          ) {
-            // the student has branched on this node before and they are not allowed to change paths
-            for (let b = branchPathTakenEvents.length - 1; b >= 0; b--) {
-              const branchPathTakenEvent = branchPathTakenEvents[b];
-              if (branchPathTakenEvent != null) {
-                const data = branchPathTakenEvent.data;
-                if (data != null) {
-                  const toNodeId = data.toNodeId;
-                  nextNodeId = toNodeId;
-                  resolve(nextNodeId);
-                  break;
-                }
-              }
-            }
-          } else {
-            // the student has not branched on this node before
-            if (transitionLogic != null) {
-              const transitions = transitionLogic.transitions;
-              if (transitions == null || transitions.length == 0) {
-                /*
-                 * this node does not have any transitions so we will
-                 * check if the parent group has transitions
-                 */
-                const parentGroupId = this.ProjectService.getParentGroupId(currentNodeId);
-                let parentHasTransitionLogic = false;
-                if (parentGroupId != null) {
-                  const parentTransitionLogic = this.ProjectService.getTransitionLogicByFromNodeId(
-                    parentGroupId
-                  );
-                  if (parentTransitionLogic != null) {
-                    parentHasTransitionLogic = true;
-                    this.chooseTransition(parentGroupId, parentTransitionLogic).then(
-                      (transition) => {
-                        if (transition != null) {
-                          const transitionToNodeId = transition.to;
-                          if (this.ProjectService.isGroupNode(transitionToNodeId)) {
-                            const startId = this.ProjectService.getGroupStartId(transitionToNodeId);
-                            if (startId == null || startId == '') {
-                              nextNodeId = transitionToNodeId;
-                            } else {
-                              nextNodeId = startId;
-                            }
-                          } else {
-                            nextNodeId = transitionToNodeId;
-                          }
-                        }
-                        resolve(nextNodeId);
-                      }
-                    );
-                  }
-                }
-              } else {
-                this.chooseTransition(currentNodeId, transitionLogic).then((transition) => {
-                  resolve(transition.to);
-                });
-              }
-            }
-          }
-        }
-      }
-    });
-    return promise;
+    return null;
   }
 
   /**

src/assets/wise5/services/studentNodeService.ts

@@ -8,6 +8,7 @@ import { ProjectService } from './projectService';
 import { NodeStatusService } from './nodeStatusService';
 import { DialogWithCloseComponent } from '../directives/dialog-with-close/dialog-with-close.component';
 import { Constraint } from '../../../app/domain/constraint';
+import { TransitionLogic } from '../common/TransitionLogic';
 
 @Injectable()
 export class StudentNodeService extends NodeService {
@@ -69,4 +70,60 @@ export class StudentNodeService extends NodeService {
       .filter((message) => message != '')
       .join('<br/>');
   }
+
+  /**
+   * 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
+   * the promise immediately.
+   * @param currentId (optional) the current node id
+   * @returns a promise that returns the next node id
+   */
+  getNextNodeId(currentId?: string): Promise<any> {
+    return new Promise((resolve, reject) => {
+      const currentNodeId = currentId ?? this.DataService.getCurrentNodeId();
+      const transitionLogic = this.ProjectService.getTransitionLogicByFromNodeId(currentNodeId);
+      const branchPathTakenEvents = this.DataService.getBranchPathTakenEventsByNodeId(
+        currentNodeId
+      );
+      if (this.hasPreviouslyBranchedAndCannotChange(branchPathTakenEvents, transitionLogic)) {
+        resolve(branchPathTakenEvents.at(-1).data.toNodeId);
+      } else {
+        this.resolveNextNodeIdFromTransition(resolve, currentNodeId);
+      }
+    });
+  }
+
+  private hasPreviouslyBranchedAndCannotChange(
+    branchPathTakenEvents: any[],
+    transitionLogic: TransitionLogic
+  ): boolean {
+    return branchPathTakenEvents.length > 0 && !transitionLogic.canChangePath;
+  }
+
+  private resolveNextNodeIdFromTransition(resolve: any, currentNodeId: string): void {
+    const transitionLogic = this.ProjectService.getTransitionLogicByFromNodeId(currentNodeId);
+    if (transitionLogic.transitions.length == 0) {
+      this.getNextNodeIdFromParent(resolve, currentNodeId);
+    } else {
+      this.chooseTransition(currentNodeId, transitionLogic).then((transition: any) => {
+        resolve(transition.to);
+      });
+    }
+  }
+
+  private getNextNodeIdFromParent(resolve: any, currentNodeId: string): void {
+    const parentGroupId = this.ProjectService.getParentGroupId(currentNodeId);
+    if (parentGroupId != null) {
+      const parentTransitionLogic = this.ProjectService.getTransitionLogicByFromNodeId(
+        parentGroupId
+      );
+      this.chooseTransition(parentGroupId, parentTransitionLogic).then((transition: any) => {
+        const transitionToNodeId = transition.to;
+        const startId = this.ProjectService.isGroupNode(transitionToNodeId)
+          ? this.ProjectService.getGroupStartId(transitionToNodeId)
+          : null;
+        resolve(startId == null || startId === '' ? transitionToNodeId : startId);
+      });
+    }
+  }
 }

src/assets/wise5/services/teacherNodeService.ts

@@ -22,4 +22,21 @@ export class TeacherNodeService extends NodeService {
   respondStarterState(args: any): void {
     this.starterStateResponseSource.next(args);
   }
+
+  getNextNodeId(currentId?: string): Promise<any> {
+    return new Promise((resolve, reject) => {
+      let nextNodeId = null;
+      const currentNodeId = currentId ?? this.DataService.getCurrentNodeId();
+      const currentNodeOrder = this.ProjectService.getNodeOrderById(currentNodeId);
+      if (currentNodeOrder) {
+        const nextId = this.ProjectService.getNodeIdByOrder(currentNodeOrder + 1);
+        if (nextId) {
+          nextNodeId = this.ProjectService.isApplicationNode(nextId)
+            ? nextId
+            : this.getNextNodeId(nextId);
+        }
+      }
+      resolve(nextNodeId);
+    });
+  }
 }