front: match PathStep and SuggestedOP with pathStepId

Matching via uic/trigram/ch/etc is unreliable because:

- Two OPs can have the same trigram/ch.
- The same OP could appear multiple times at different spots in a
  path.

Moreover, we had multiple variants of the matching with slight
differences. Some of the matching logic got adjusted to account
for specific bugs, while others were left alone.

Each path step can be identified with a unique ID stored in
PathStep.id. Use that instead.

Signed-off-by: Simon Ser <[email protected]>

Author: emersion

front/src/applications/operationalStudies/hooks/useSetupItineraryForTrainUpdate.ts

@@ -13,7 +13,7 @@ import {
 } from 'common/api/osrdEditoastApi';
 import { useOsrdConfActions } from 'common/osrdContext';
 import buildOpSearchQuery from 'modules/operationalPoint/helpers/buildOpSearchQuery';
-import { formatSuggestedOperationalPoints, matchPathStepAndOp } from 'modules/pathfinding/utils';
+import { formatSuggestedOperationalPoints } from 'modules/pathfinding/utils';
 import { getSupportedElectrification, isThermal } from 'modules/rollingStock/helpers/electric';
 import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
 import computeBasePathStep from 'modules/trainschedule/helpers/computeBasePathStep';
@@ -39,8 +39,8 @@ export function updatePathStepsFromOperationalPoints(
   stepsCoordinates: Position[]
 ) {
   const updatedPathSteps: PathStep[] = pathSteps.map((step, i) => {
-    const correspondingOp = suggestedOperationalPoints.find((suggestedOp) =>
-      matchPathStepAndOp(step, suggestedOp)
+    const correspondingOp = suggestedOperationalPoints.find(
+      (suggestedOp) => step.id === suggestedOp.pathStepId
     );
 
     const { kp, name } = correspondingOp || step;

front/src/modules/pathfinding/components/Itinerary/ModalSuggestedVias.tsx

@@ -9,7 +9,6 @@ import ModalBodySNCF from 'common/BootstrapSNCF/ModalSNCF/ModalBodySNCF';
 import ModalFooterSNCF from 'common/BootstrapSNCF/ModalSNCF/ModalFooterSNCF';
 import ModalHeaderSNCF from 'common/BootstrapSNCF/ModalSNCF/ModalHeaderSNCF';
 import { useOsrdConfActions, useOsrdConfSelectors } from 'common/osrdContext';
-import { isVia, matchPathStepAndOp } from 'modules/pathfinding/utils';
 import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
 import type { OperationalStudiesConfSliceActions } from 'reducers/osrdconf/operationalStudiesConf';
 import type { PathStep } from 'reducers/osrdconf/types';
@@ -37,12 +36,12 @@ const ModalSuggestedVias = ({ suggestedVias, launchPathfinding }: ModalSuggested
   );
 
   const removeViaFromPath = (op: SuggestedOP) => {
-    const newPathSteps = pathSteps.filter((step) => !matchPathStepAndOp(step!, op));
+    const newPathSteps = pathSteps.filter((step) => step!.id !== op.pathStepId);
     launchPathfinding(newPathSteps);
   };
 
   const formatOP = (op: SuggestedOP, idx: number, idxTrueVia: number) => {
-    const isInVias = isVia(vias, op);
+    const isInVias = vias.find((step) => step.id === op.pathStepId);
     return (
       <div
         key={`suggested-via-modal-${op.opId}-${idx}`}
@@ -105,7 +104,7 @@ const ModalSuggestedVias = ({ suggestedVias, launchPathfinding }: ModalSuggested
           {suggestedVias.map((via, idx) => {
             if (!isOriginOrDestination(via)) {
               // If name is undefined, we know the op/via has been added by clicking on map
-              if (isVia(vias, via)) idxTrueVia += 1;
+              if (vias.find((step) => step.id === via.pathStepId)) idxTrueVia += 1;
               return formatOP(via, idx, idxTrueVia);
             }
             return null;

front/src/modules/pathfinding/hooks/usePathfinding.ts

@@ -14,11 +14,7 @@ import type {
 } from 'common/api/osrdEditoastApi';
 import { osrdEditoastApi } from 'common/api/osrdEditoastApi';
 import { useOsrdConfActions, useOsrdConfSelectors } from 'common/osrdContext';
-import {
-  formatSuggestedOperationalPoints,
-  getPathfindingQuery,
-  matchPathStepAndOp,
-} from 'modules/pathfinding/utils';
+import { formatSuggestedOperationalPoints, getPathfindingQuery } from 'modules/pathfinding/utils';
 import { useStoreDataForRollingStockSelector } from 'modules/rollingStock/components/RollingStockSelector/useStoreDataForRollingStockSelector';
 import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
 import { setFailure, setWarning } from 'reducers/main';
@@ -126,8 +122,8 @@ const usePathfinding = (
     // We update existing pathsteps with coordinates, positionOnPath and kp corresponding to the new pathfinding result
     const updatedPathSteps: (PathStep | null)[] = pathStepsInput.map((step, i) => {
       if (!step) return step;
-      const correspondingOp = suggestedOperationalPoints.find((suggestedOp) =>
-        matchPathStepAndOp(step, suggestedOp)
+      const correspondingOp = suggestedOperationalPoints.find(
+        (suggestedOp) => step.id === suggestedOp.pathStepId
       );
 
       const theoreticalMargin = i === 0 ? step.theoreticalMargin || '0%' : step.theoreticalMargin;

front/src/modules/pathfinding/utils.ts

@@ -16,7 +16,7 @@ import { getPointCoordinates } from 'utils/geometry';
 
 import getStepLocation from './helpers/getStepLocation';
 
-export const matchPathStepAndOp = (
+const matchPathStepAndOp = (
   step: PathStep,
   op: Pick<SuggestedOP, 'opId' | 'uic' | 'ch' | 'trigram' | 'track' | 'offsetOnTrack'>
 ) => {
@@ -143,40 +143,6 @@ export const upsertPathStepsInOPs = (ops: SuggestedOP[], pathSteps: PathStep[]):
   return updatedOPs;
 };
 
-export const pathStepMatchesOp = (
-  pathStep: PathStep,
-  op: Pick<
-    SuggestedOP,
-    'pathStepId' | 'opId' | 'uic' | 'ch' | 'trigram' | 'track' | 'offsetOnTrack' | 'name' | 'kp'
-  >,
-  withKP = false
-) => {
-  if (!matchPathStepAndOp(pathStep, op)) {
-    return pathStep.id === op.pathStepId;
-  }
-  if ('uic' in pathStep) {
-    return withKP ? pathStep.kp === op.kp : pathStep.name === op.name;
-  }
-  return true;
-};
-
-/**
- * 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
- * probably because of imports problem).
- * If the vias has no uic, it has been added via map click and we know it has an id.
- * @param withKP - If true, we check the kp compatibility instead of the name.
- * It is used in the times and stops table to check if an operational point is a via.
- */
-export const isVia = (
-  vias: PathStep[],
-  op: Pick<
-    SuggestedOP,
-    'pathStepId' | 'opId' | 'uic' | 'ch' | 'trigram' | 'track' | 'offsetOnTrack' | 'name' | 'kp'
-  >,
-  { withKP = false } = {}
-) => vias.some((via) => pathStepMatchesOp(via, op, withKP));
-
 export const isStation = (chCode: string): boolean =>
   chCode === 'BV' || chCode === '00' || chCode === '';
 

front/src/modules/timesStops/TimesStopsInput.tsx

@@ -7,7 +7,6 @@ import { useTranslation } from 'react-i18next';
 
 import { useScenarioContext } from 'applications/operationalStudies/hooks/useScenarioContext';
 import { useOsrdConfActions } from 'common/osrdContext';
-import { isVia, matchPathStepAndOp } from 'modules/pathfinding/utils';
 import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
 import type { OperationalStudiesConfSliceActions } from 'reducers/osrdconf/operationalStudiesConf';
 import type { PathStep } from 'reducers/osrdconf/types';
@@ -43,7 +42,7 @@ const createClearViaButton = ({
     pathStepsAndSuggestedOPs &&
     rowIndex > 0 &&
     rowIndex < pathStepsAndSuggestedOPs.length - 1 &&
-    isVia(pathSteps || [], rowData, { withKP: true }) &&
+    pathSteps.find((step) => step.id === rowData.pathStepId) &&
     (!isNil(rowData.stopFor) ||
       rowData.theoreticalMargin !== undefined ||
       rowData.arrival !== undefined ||
@@ -78,9 +77,7 @@ const TimesStopsInput = ({
   const { getTrackSectionsByIds, trackSectionsLoading } = useScenarioContext();
 
   const clearPathStep = (rowData: TimesStopsInputRow) => {
-    const index = pathSteps.findIndex(
-      (step) => matchPathStepAndOp(step, rowData) && step.positionOnPath === rowData.positionOnPath
-    );
+    const index = pathSteps.findIndex((step) => step.id === rowData.pathStepId);
 
     const updatedPathSteps = pathSteps.map((step, i) => {
       if (i === index) {

front/src/modules/timesStops/helpers/utils.ts

@@ -6,7 +6,6 @@ import { keyColumn, createTextColumn } from 'react-datasheet-grid';
 
 import type { ReceptionSignal } from 'common/api/osrdEditoastApi';
 import type { IsoDurationString, TimeString } from 'common/types';
-import { matchPathStepAndOp } from 'modules/pathfinding/utils';
 import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
 import type { PathStep } from 'reducers/osrdconf/types';
 import { NO_BREAK_SPACE } from 'utils/strings';
@@ -24,18 +23,6 @@ import {
 import { marginRegExValidation, MarginUnit } from '../consts';
 import { TableType, type TimeExtraDays, type TimesStopsInputRow } from '../types';
 
-const matchPathStepAndOpWithKP = (step: PathStep, op: SuggestedOP) => {
-  if (!matchPathStepAndOp(step, op)) {
-    return step.id === op.pathStepId;
-  }
-  // We match the kp in case two OPs have the same uic+ch (can happen when the
-  // infra is imported)
-  if ('uic' in step || 'trigram' in step) {
-    return step.kp === op.kp;
-  }
-  return true;
-};
-
 export const formatSuggestedViasToRowVias = (
   operationalPoints: SuggestedOP[],
   pathSteps: PathStep[],
@@ -49,7 +36,7 @@ export const formatSuggestedViasToRowVias = (
   // to move it to the first position
   const origin = pathSteps[0];
   const originIndexInOps = origin
-    ? operationalPoints.findIndex((op) => matchPathStepAndOpWithKP(origin, op))
+    ? operationalPoints.findIndex((op) => origin.id === op.pathStepId)
     : -1;
   if (originIndexInOps !== -1) {
     [formattedOps[0], formattedOps[originIndexInOps]] = [
@@ -60,9 +47,7 @@ export const formatSuggestedViasToRowVias = (
 
   // Ditto: destination should be last
   const dest = pathSteps[pathSteps.length - 1];
-  const destIndexInOps = dest
-    ? operationalPoints.findIndex((op) => matchPathStepAndOpWithKP(dest, op))
-    : -1;
+  const destIndexInOps = dest ? operationalPoints.findIndex((op) => dest.id === op.pathStepId) : -1;
   if (destIndexInOps !== -1) {
     const lastOpIndex = formattedOps.length - 1;
     [formattedOps[lastOpIndex], formattedOps[destIndexInOps]] = [
@@ -72,7 +57,7 @@ export const formatSuggestedViasToRowVias = (
   }
 
   return formattedOps.map((op, i) => {
-    const pathStep = pathSteps.find((step) => matchPathStepAndOpWithKP(step, op));
+    const pathStep = pathSteps.find((step) => step.id === op.pathStepId);
     const { name } = pathStep || op;
     const objectToUse = tableType === TableType.Input ? pathStep : op;
 

front/src/reducers/osrdconf/helpers.ts

@@ -4,7 +4,6 @@ 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 } from 'utils/array';
 
@@ -76,7 +75,7 @@ export function upsertPathStep(statePathSteps: (PathStep | null)[], op: Suggeste
         }),
   };
 
-  const stepIndex = cleanPathSteps.findIndex((step) => pathStepMatchesOp(step, op));
+  const stepIndex = cleanPathSteps.findIndex((step) => step.id === op.pathStepId);
   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