front: refactor upsertViaFromSuggestedOP

- using lodash pick to simplify the object construction
- no need to lookup twice to find the corresponding pathStep
- we don’t need to create copies of the pathSteps array since we are using immer to handle immutability for us
- extracting the reducer logic in buildPathStepFromSuggestedOp to be reused

front/src/modules/pathfinding/utils.ts

@@ -157,6 +157,14 @@ export const upsertPathStepsInOPs = (ops: SuggestedOP[], pathSteps: PathStep[]):
   return updatedOPs;
 };
 
+export const pathStepMatchesOp = (pathStep: PathStep, op: SuggestedOP, withKP = false) =>
+  ('uic' in pathStep &&
+    'ch' in pathStep &&
+    pathStep.uic === op.uic &&
+    pathStep.ch === op.ch &&
+    (withKP ? pathStep.kp === op.kp : pathStep.name === op.name)) ||
+  pathStep.id === op.opId;
+
 /**
  * Check if a suggested operational point is a via.
  * Some OPs have same uic so we need to check also the ch (can be still not enough
@@ -166,12 +174,4 @@ export const upsertPathStepsInOPs = (ops: SuggestedOP[], pathSteps: PathStep[]):
  * It is used in the times and stops table to check if an operational point is a via.
  */
 export const isVia = (vias: PathStep[], op: SuggestedOP, withKP = false) =>
-  vias.some(
-    (via) =>
-      ('uic' in via &&
-        'ch' in via &&
-        via.uic === op.uic &&
-        via.ch === op.ch &&
-        (withKP ? via.kp === op.kp : via.name === op.name)) ||
-      via.id === op.opId
-  );
+  vias.some((via) => pathStepMatchesOp(via, op, withKP));

front/src/reducers/osrdconf/helpers.ts

@@ -1,8 +1,11 @@
 import { feature, point } from '@turf/helpers';
-import { last } from 'lodash';
+import { compact, last, pick } from 'lodash';
+import nextId from 'react-id-generator';
 
 import { calculateDistanceAlongTrack } from 'applications/editor/tools/utils';
 import type { ManageTrainSchedulePathProperties } from 'applications/operationalStudies/types';
+import { pathStepMatchesOp } from 'modules/pathfinding/utils';
+import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
 import { addElementAtIndex, replaceElementAtIndex } from 'utils/array';
 import { formatIsoDate } from 'utils/date';
 import { sec2time, time2sec } from 'utils/timeManipulation';
@@ -142,3 +145,47 @@ export const updateDestinationPathStep = (
   destination: Partial<PathStep> | null,
   replaceCompletely: boolean = false
 ) => updatePathStepAtIndex(pathSteps, pathSteps.length - 1, destination, replaceCompletely);
+
+/**
+ * modifies the array statePathSteps in place
+ */
+export function upsertPathStep(statePathSteps: (PathStep | null)[], op: SuggestedOP) {
+  // We know that, at this point, origin and destination are defined because pathfinding has been done
+  const cleanPathSteps = compact(statePathSteps);
+
+  let newVia = {
+    ...pick(op, [
+      'coordinates',
+      'positionOnPath',
+      'name',
+      'ch',
+      'kp',
+      'stopFor',
+      'arrival',
+      'locked',
+      'deleted',
+      'onStopSignal',
+      'theoreticalMargin',
+    ]),
+    id: nextId(),
+    ...(op.uic
+      ? { uic: op.uic }
+      : {
+          track: op.track,
+          offset: op.offsetOnTrack,
+        }),
+  } as PathStep;
+
+  const stepIndex = cleanPathSteps.findIndex((step) => pathStepMatchesOp(step, op));
+  if (stepIndex >= 0) {
+    // Because of import issues, there can be multiple ops with same position on path
+    // To avoid updating the wrong one, we need to find the one that matches the payload
+    newVia = { ...newVia, id: cleanPathSteps[stepIndex].id }; // We don't need to change the id of the updated via
+    statePathSteps[stepIndex] = newVia;
+  } else {
+    // Because of import issues, there can be multiple ops at position 0
+    // To avoid inserting a new via before the origin we need to check if the index is 0
+    const index = cleanPathSteps.findIndex((step) => step.positionOnPath! >= op.positionOnPath);
+    statePathSteps.splice(index || 1, 0, newVia);
+  }
+}

front/src/reducers/osrdconf/osrdConfCommon/index.ts

@@ -1,18 +1,17 @@
 import type { CaseReducer, PayloadAction, PrepareAction } from '@reduxjs/toolkit';
 import type { Draft } from 'immer';
-import { compact, omit } from 'lodash';
-import nextId from 'react-id-generator';
+import { omit } from 'lodash';
 
 import type { ManageTrainSchedulePathProperties } from 'applications/operationalStudies/types';
 import { ArrivalTimeTypes } from 'applications/stdcmV2/types';
-import { isVia } from 'modules/pathfinding/utils';
 import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
 import { type InfraStateReducers, buildInfraStateReducers, infraState } from 'reducers/infra';
 import {
   computeLinkedOriginTimes,
   insertViaFromMap,
   updateDestinationPathStep,
   updateOriginPathStep,
+  upsertPathStep,
 } from 'reducers/osrdconf/helpers';
 import type {
   OperationalStudiesConfSlice,
@@ -22,7 +21,7 @@ import type { OperationalStudiesConfSelectors } from 'reducers/osrdconf/operatio
 import type { StdcmConfSlice, StdcmConfSliceActions } from 'reducers/osrdconf/stdcmConf';
 import type { StdcmConfSelectors } from 'reducers/osrdconf/stdcmConf/selectors';
 import type { OsrdConfState, PathStep } from 'reducers/osrdconf/types';
-import { addElementAtIndex, removeElementAtIndex, replaceElementAtIndex } from 'utils/array';
+import { removeElementAtIndex } from 'utils/array';
 import { formatIsoDate } from 'utils/date';
 import type { ArrayElement } from 'utils/types';
 
@@ -288,53 +287,7 @@ export function buildCommonConfReducers<S extends OsrdConfState>(): CommonConfRe
     // Use this action to transform an op to via from times and stop table or
     // from the suggested via modal
     upsertViaFromSuggestedOP(state: Draft<S>, action: PayloadAction<SuggestedOP>) {
-      // We know that, at this point, origin and destination are defined because pathfinding has been done
-      const pathSteps = compact(state.pathSteps);
-
-      let newVia: PathStep = {
-        coordinates: action.payload.coordinates,
-        id: nextId(),
-        positionOnPath: action.payload.positionOnPath,
-        name: action.payload.name,
-        ch: action.payload.ch,
-        kp: action.payload.kp,
-        stopFor: action.payload.stopFor,
-        arrival: action.payload.arrival,
-        locked: action.payload.locked,
-        deleted: action.payload.deleted,
-        onStopSignal: action.payload.onStopSignal,
-        theoreticalMargin: action.payload.theoreticalMargin,
-        ...(action.payload.uic
-          ? { uic: action.payload.uic }
-          : {
-              track: action.payload.track,
-              offset: action.payload.offsetOnTrack,
-            }),
-      };
-
-      const isInVias = isVia(pathSteps, action.payload);
-      if (isInVias) {
-        // Because of import issues, there can be multiple ops with same position on path
-        // To avoid updating the wrong one, we need to find the one that matches the payload
-        const stepIndex = pathSteps.findIndex(
-          (step) =>
-            ('uic' in step &&
-              'ch' in step &&
-              step.uic === action.payload.uic &&
-              step.ch === action.payload.ch &&
-              step.name === action.payload.name) ||
-            step.id === action.payload.opId
-        );
-        newVia = { ...newVia, id: pathSteps[stepIndex].id }; // We don't need to change the id of the updated via
-        state.pathSteps = replaceElementAtIndex(state.pathSteps, stepIndex, newVia);
-      } else {
-        const index = pathSteps.findIndex(
-          (step) => step.positionOnPath! >= action.payload.positionOnPath
-        );
-        // Because of import issues, there can be multiple ops at position 0
-        // To avoid inserting a new via before the origin we need to check if the index is 0
-        state.pathSteps = addElementAtIndex(state.pathSteps, index || 1, newVia);
-      }
+      upsertPathStep(state.pathSteps, action.payload);
     },
     updateRollingStockComfort(state: Draft<S>, action: PayloadAction<S['rollingStockComfort']>) {
       state.rollingStockComfort = action.payload;