Refactor: Consolidate actions to modify nested steps

[kahboom]

Please describe the task that needs to be done

At the moment, there is a lot of repetitive code where we check if a step is nested, and if it is, we handle it differently from non-nested steps.

When deleting a step (stepService.ts):

deleteStep(UUID: string) {
    // check if the step being modified is nested
    const currentStepNested = this.nestedStepsStore.nestedSteps.find((ns) => ns.stepUuid === UUID);
    if (currentStepNested) {
      const parentStepIdx = this.findStepIdxWithUUID(
        currentStepNested.originStepUuid,
        this.integrationJsonStore.integrationJson.steps
      );

      // update the original parent step, without the child step
      const updatedParentStep = StepsService.filterStepWithBranches(
        this.integrationJsonStore.integrationJson.steps[parentStepIdx],
        (step: { UUID: string }) => step.UUID !== UUID
      );

      this.integrationJsonStore.deleteBranchStep(updatedParentStep, parentStepIdx);
    } else {
      // `deleteStep` requires the index to be from `integrationJson`, not `nodes`
      const stepsIndex = this.findStepIdxWithUUID(
        UUID,
        this.integrationJsonStore.integrationJson.steps
      );

      this.visualizationStore.deleteNode(stepsIndex);
      this.integrationJsonStore.deleteStep(stepsIndex);
    }
  }

We are checking if it's a nested step, we find the index of the root step, make a copy of that step, mutate it as needed, then replacing the whole step. Finally, we call the store and update the whole root step (this could also be an opportunity for performance testing and optimization). If the step is not nested, it has its own handling, sometimes calling a different store method, like below.

For updating step parameters it's a similar scenario (stepService.ts):

updateStepParameters(step: IStepProps, newValues: { [s: string]: unknown } | ArrayLike<unknown>) {
    let newStep: IStepProps = step;
    const newStepParameters = newStep.parameters?.slice();

    if (newStepParameters && newStepParameters.length > 0) {
      Object.entries(newValues).forEach(([key, value]) => {
        const paramIndex = newStepParameters.findIndex((p) => p.id === key);
        newStepParameters[paramIndex].value = value;
      });

      // check if the step being modified is nested
      if (this.nestedStepsStore.nestedSteps.some((ns) => ns.stepUuid === newStep.UUID)) {
        // use its path to replace only this part of the original step
        const currentStepNested = this.nestedStepsStore.nestedSteps.find(
          (ns) => ns.stepUuid === newStep.UUID
        );
        if (currentStepNested) {
          this.integrationJsonStore.replaceBranchStep(newStep, currentStepNested.pathToStep);
        }
      } else {
        const oldStepIdx = this.findStepIdxWithUUID(
          newStep.UUID,
          this.integrationJsonStore.integrationJson.steps
        );
        this.integrationJsonStore.replaceStep(newStep, oldStepIdx);
      }
    } else {
      return;
    }
  }

Suggested Solution

This repetitive code should be consolidated into a function or set of functions that a) check for the presence of a nested step, b) provide handling for the nested step, c) provide handling for the non-nested step. We need to evaluate how realistic this is to do, but if possible, it will likely help with separate concerns and optimizing only where necessary.

cc @igarashitm -- it's not a priority so I won't be working on this now, but just FYI as it could conflict with your refactoring depending on when you finish and when this starts.

[igarashitm]

Great point, totally agree 👍