From bed6c500917e8c26a09ad0e50c93e2aa8af491ce Mon Sep 17 00:00:00 2001
From: Simon Ser <contact@emersion.fr>
Date: Mon, 18 Nov 2024 11:41:24 +0100
Subject: [PATCH] front: simplify OP matching in
 updatePathStepsFromOperationalPoints()

We already have a matchPathStepAndOp() function to compare a PathStep
and a SuggestedOP. There are two differences:

- The old code checks whether PathStep.ch is set before comparing
  it with SuggestedOP, but that's incorrect. On the editoast side
  the secondary_code field is defined as Option<String>: None
  accepts OPs with any secondary code, "" accepts only OPs without
  any secondary code. On the TypeScript side, undefined accepts OPs
  with any secondary code, and '' accepts only OPs without any
  secondary code. However `'ch' in step` is true for
  `{ ch: undefined }`, which compares SuggestedOP.ch with undefined
  even if an undefined PathStep.ch is supposed to match any OP
  secondary code.
- The old code checks PathStep.secondary_code. However the PathSteps
  given to updatePathStepsFromOperationalPoints() come straight up
  from computeBasePathSteps(), which always leaves secondary_code
  undefined. As a result we never hit this case.

Signed-off-by: Simon Ser <contact@emersion.fr>
---
 .../useSetupItineraryForTrainUpdate.spec.ts   |  9 ++----
 .../hooks/useSetupItineraryForTrainUpdate.ts  | 29 +++++--------------
 2 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/front/src/applications/operationalStudies/hooks/__tests__/useSetupItineraryForTrainUpdate.spec.ts b/front/src/applications/operationalStudies/hooks/__tests__/useSetupItineraryForTrainUpdate.spec.ts
index 9b7b93fcf81..3a803d30487 100644
--- a/front/src/applications/operationalStudies/hooks/__tests__/useSetupItineraryForTrainUpdate.spec.ts
+++ b/front/src/applications/operationalStudies/hooks/__tests__/useSetupItineraryForTrainUpdate.spec.ts
@@ -226,20 +226,20 @@ describe('updatePathStepsFrom', () => {
         {
           id: 'whatev-0',
           trigram: 'GE',
-          secondary_code: 'BV',
+          ch: 'BV',
           name: '87747006',
         },
         {
           id: 'whatev-1',
           trigram: 'GE',
-          secondary_code: 'P2',
+          ch: 'P2',
           name: '87747006',
           arrival: '15:00:00',
         },
         {
           id: 'who-0',
           trigram: 'VPE',
-          secondary_code: 'BV',
+          ch: 'BV',
           name: '87747337',
         },
       ];
@@ -262,7 +262,6 @@ describe('updatePathStepsFrom', () => {
           id: 'whatev-0',
           ch: 'BV',
           trigram: 'GE',
-          secondary_code: 'BV',
           name: 'Grenadille',
           kp: '130+538',
           positionOnPath: 0,
@@ -272,7 +271,6 @@ describe('updatePathStepsFrom', () => {
           id: 'whatev-1',
           ch: 'P2',
           trigram: 'GE',
-          secondary_code: 'P2',
           name: 'Grenadille',
           arrival: '15:00:00',
           kp: '129+952',
@@ -282,7 +280,6 @@ describe('updatePathStepsFrom', () => {
         {
           id: 'who-0',
           ch: 'BV',
-          secondary_code: 'BV',
           trigram: 'VPE',
           name: 'Voreppe',
           kp: '117+422',
diff --git a/front/src/applications/operationalStudies/hooks/useSetupItineraryForTrainUpdate.ts b/front/src/applications/operationalStudies/hooks/useSetupItineraryForTrainUpdate.ts
index 7775fa7629b..f3985256aba 100644
--- a/front/src/applications/operationalStudies/hooks/useSetupItineraryForTrainUpdate.ts
+++ b/front/src/applications/operationalStudies/hooks/useSetupItineraryForTrainUpdate.ts
@@ -14,7 +14,11 @@ import {
   type PathfindingResult,
 } from 'common/api/osrdEditoastApi';
 import { useOsrdConfActions, useOsrdConfSelectors } from 'common/osrdContext';
-import { formatSuggestedOperationalPoints, upsertPathStepsInOPs } from 'modules/pathfinding/utils';
+import {
+  formatSuggestedOperationalPoints,
+  matchPathStepAndOp,
+  upsertPathStepsInOPs,
+} from 'modules/pathfinding/utils';
 import { getSupportedElectrification, isThermal } from 'modules/rollingStock/helpers/electric';
 import { adjustConfWithTrainToModify } from 'modules/trainschedule/components/ManageTrainSchedule/helpers/adjustConfWithTrainToModify';
 import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
@@ -83,26 +87,9 @@ export function updatePathStepsFromOperationalPoints(
   stepsCoordinates: Position[]
 ) {
   const updatedPathSteps: PathStep[] = pathSteps.map((step, i) => {
-    const correspondingOp = suggestedOperationalPoints.find((suggestedOp) => {
-      if ('uic' in step) {
-        const condition = suggestedOp.uic === step.uic;
-        if ('ch' in step) {
-          return condition && suggestedOp.ch === step.ch;
-        }
-        // When importing train from open data or from files, secondary_code might not always exist
-        if (step.secondary_code) {
-          return condition && suggestedOp.ch === step.secondary_code;
-        }
-        return condition;
-      }
-      if ('trigram' in step) {
-        const condition = suggestedOp.trigram === step.trigram;
-        if (step.secondary_code) {
-          return condition && suggestedOp.ch === step.secondary_code;
-        }
-      }
-      return false;
-    });
+    const correspondingOp = suggestedOperationalPoints.find((suggestedOp) =>
+      matchPathStepAndOp(step, suggestedOp)
+    );
 
     const { kp, name, ch } = correspondingOp || step;