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/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 } from 'modules/pathfinding/utils';
 import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
 import { useAppDispatch } from 'store';
 import { formatUicToCi } from 'utils/strings';
@@ -35,7 +34,7 @@ const ModalSuggestedVias = ({ suggestedVias }: ModalSuggestedViasProps) => {
   const removeViaFromPath = (op: SuggestedOP) => dispatch(removeVia(op));
 
   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}`}
@@ -98,7 +97,7 @@ const ModalSuggestedVias = ({ suggestedVias }: ModalSuggestedViasProps) => {
           {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

@@ -21,7 +21,6 @@ import type { PathfindingAction, PathfindingState } from 'modules/pathfinding/ty
 import {
   formatSuggestedOperationalPoints,
   getPathfindingQuery,
-  matchPathStepAndOp,
   upsertPathStepsInOPs,
 } from 'modules/pathfinding/utils';
 import { useStoreDataForRollingStockSelector } from 'modules/rollingStock/components/RollingStockSelector/useStoreDataForRollingStockSelector';
@@ -279,8 +278,8 @@ export const usePathfinding = (
       // We update existing pathsteps with coordinates, positionOnPath and kp corresponding to the new pathfinding result
       const updatedPathSteps: (PathStep | null)[] = pathSteps.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

@@ -155,23 +155,6 @@ export const pathStepMatchesOp = (
   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 { PathStep } from 'reducers/osrdconf/types';
 import { useAppDispatch } from 'store';
@@ -42,7 +41,7 @@ const createClearViaButton = ({
     allWaypoints &&
     rowIndex > 0 &&
     rowIndex < allWaypoints.length - 1 &&
-    isVia(pathSteps || [], rowData, { withKP: true }) &&
+    pathSteps.find((step) => step.id === rowData.pathStepId) &&
     (!isNil(rowData.stopFor) ||
       rowData.theoreticalMargin !== undefined ||
       rowData.arrival !== undefined ||
@@ -72,9 +71,7 @@ const TimesStopsInput = ({ allWaypoints, startTime, pathSteps }: 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 & { isWaypoint?: boolean })[],
   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/osrdConfCommon/index.ts

@@ -4,7 +4,6 @@ import { omit } from 'lodash';
 
 import type { ManageTrainSchedulePathProperties } from 'applications/operationalStudies/types';
 import { ArrivalTimeTypes, type StdcmStopTypes } from 'applications/stdcm/types';
-import { matchPathStepAndOp } from 'modules/pathfinding/utils';
 import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
 import { type InfraStateReducers, buildInfraStateReducers, infraState } from 'reducers/infra';
 import {
@@ -202,7 +201,7 @@ export function buildCommonConfReducers<S extends OsrdConfState>(): CommonConfRe
     removeVia(state: Draft<S>, action: PayloadAction<SuggestedOP>) {
       // Index takes count of the origin in the array
       state.pathSteps = state.pathSteps.filter(
-        (step) => !step || matchPathStepAndOp(step, action.payload)
+        (step) => !step || step.id !== action.payload.pathStepId
       );
       state.powerRestriction = [];
     },