refactor(NodeService): Clean up code (#2023)

Author: hirokiterashima

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: [

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]

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<string> {
     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<string> {
-    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;

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<any> = new Subject<any>();
@@ -18,83 +18,33 @@ 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 {
     this.dataService.setCurrentNodeByNodeId(nodeId);
   }
 
   goToNextNode(): Promise<string> {
-    return this.getNextNodeId().then((nextNodeId) => {
+    return this.getNextNodeId().then((nextNodeId: string) => {
       if (nextNodeId != null) {
         this.setCurrentNode(nextNodeId);
       }
       return nextNodeId;
     });
   }
 
-  /**
-   * This function should be implemented by the child service classes
-   */
-  getNextNodeId(currentId?: string): Promise<any> {
-    return null;
-  }
+  abstract getNextNodeId(currentId?: string): Promise<any>;
 
-  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(

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('<br/>');
   }
 
+  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

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<any> {
     return new Promise((resolve, reject) => {
       let nextNodeId = null;