From 8d0d8b29f6e422b52f96934a2dd45c8379244ed0 Mon Sep 17 00:00:00 2001 From: Alice Khoudli Date: Wed, 9 Oct 2024 18:42:55 +0200 Subject: [PATCH 1/4] front: fix waypoint removal in itinerary pathfinding module Signed-off-by: Alice Khoudli --- .../pathfinding/components/Itinerary/ModalSuggestedVias.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/front/src/modules/pathfinding/components/Itinerary/ModalSuggestedVias.tsx b/front/src/modules/pathfinding/components/Itinerary/ModalSuggestedVias.tsx index ec0d1f9830c..e2beb831d28 100644 --- a/front/src/modules/pathfinding/components/Itinerary/ModalSuggestedVias.tsx +++ b/front/src/modules/pathfinding/components/Itinerary/ModalSuggestedVias.tsx @@ -37,7 +37,8 @@ const ModalSuggestedVias = ({ suggestedVias }: ModalSuggestedViasProps) => { const removeViaFromPath = (op: SuggestedOP) => { const updatedPathSteps = compact(pathSteps).filter( (step) => - ('uic' in step && step.uic !== op.uic) || ('track' in step && step.track !== op.track) + ('uic' in step && (step.uic !== op.uic || step.ch !== op.ch)) || + ('track' in step && (step.track !== op.track || step.offset !== op.offsetOnTrack)) ); dispatch( updatePathSteps({ From 978bd77ec852603a80dae75e1b76952e1e2c37a0 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Thu, 10 Oct 2024 12:39:42 +0200 Subject: [PATCH 2/4] front: stop matching KP in matchPathStepAndOp() In many spots we need to match a PathStep with a SuggestedOP. Each spot is a special snowflake and needs its own little variation of the matching logic. formatSuggestedViasToRowVias() needs to match KP and PathStep.id, but that's undesirable for other spots. Extract the extra special matching near the spot it's needed. In following commits matchPathStepAndOp() will be re-used in more spots. No behavior change, only refactoring. Signed-off-by: Simon Ser --- front/src/modules/pathfinding/utils.ts | 10 +++------- front/src/modules/timesStops/helpers/utils.ts | 20 ++++++++++++++++--- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/front/src/modules/pathfinding/utils.ts b/front/src/modules/pathfinding/utils.ts index f57208a2bd9..8b121e4ee57 100644 --- a/front/src/modules/pathfinding/utils.ts +++ b/front/src/modules/pathfinding/utils.ts @@ -38,17 +38,13 @@ export const matchPathStepAndOp = (step: PathStep, op: SuggestedOP) => { if ('operational_point' in step) { return step.operational_point === op.opId; } - // We match the kp in case two OPs have the same uic+ch (can happen when the - // infra is imported) if ('uic' in step) { - return step.uic === op.uic && step.ch === op.ch && step.kp === op.kp; + return step.uic === op.uic && step.ch === op.ch; } if ('trigram' in step) { - return step.trigram === op.trigram && step.ch === op.ch && step.kp === op.kp; + return step.trigram === op.trigram && step.ch === op.ch; } - // TODO: we abuse the PathStep.id field here, the backend also sets it to an - // ID which has nothing to do with OPs - return step.id === op.opId; + return step.track === op.track && step.offset === op.offsetOnTrack; }; export const getPathfindingQuery = ({ diff --git a/front/src/modules/timesStops/helpers/utils.ts b/front/src/modules/timesStops/helpers/utils.ts index 0b301130b7e..8ab8dcb1f88 100644 --- a/front/src/modules/timesStops/helpers/utils.ts +++ b/front/src/modules/timesStops/helpers/utils.ts @@ -30,6 +30,20 @@ import { type TimesStopsInputRow, } from '../types'; +const matchPathStepAndOpWithKP = (step: PathStep, op: SuggestedOP) => { + if (!matchPathStepAndOp(step, op)) { + // TODO: we abuse the PathStep.id field here, the backend also sets it to an + // ID which has nothing to do with OPs + return step.id === op.opId; + } + // 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[], @@ -43,7 +57,7 @@ export const formatSuggestedViasToRowVias = ( // to move it to the first position const origin = pathSteps[0]; const originIndexInOps = origin - ? operationalPoints.findIndex((op) => matchPathStepAndOp(origin, op)) + ? operationalPoints.findIndex((op) => matchPathStepAndOpWithKP(origin, op)) : -1; if (originIndexInOps !== -1) { [formattedOps[0], formattedOps[originIndexInOps]] = [ @@ -55,7 +69,7 @@ export const formatSuggestedViasToRowVias = ( // Ditto: destination should be last const dest = pathSteps[pathSteps.length - 1]; const destIndexInOps = dest - ? operationalPoints.findIndex((op) => matchPathStepAndOp(dest, op)) + ? operationalPoints.findIndex((op) => matchPathStepAndOpWithKP(dest, op)) : -1; if (destIndexInOps !== -1) { const lastOpIndex = formattedOps.length - 1; @@ -66,7 +80,7 @@ export const formatSuggestedViasToRowVias = ( } return formattedOps.map((op, i) => { - const pathStep = pathSteps.find((step) => matchPathStepAndOp(step, op)); + const pathStep = pathSteps.find((step) => matchPathStepAndOpWithKP(step, op)); const { name } = pathStep || op; const objectToUse = tableType === TableType.Input ? pathStep : op; From d13058d33c182da0705408c4690bd43324e51f8f Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Thu, 10 Oct 2024 12:40:04 +0200 Subject: [PATCH 3/4] front: use matchPathStepAndOp() in ModalSuggestedVias The logic in matchPathStepAndOp() is more general. Signed-off-by: Simon Ser --- .../components/Itinerary/ModalSuggestedVias.tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/front/src/modules/pathfinding/components/Itinerary/ModalSuggestedVias.tsx b/front/src/modules/pathfinding/components/Itinerary/ModalSuggestedVias.tsx index e2beb831d28..64ac5390043 100644 --- a/front/src/modules/pathfinding/components/Itinerary/ModalSuggestedVias.tsx +++ b/front/src/modules/pathfinding/components/Itinerary/ModalSuggestedVias.tsx @@ -10,7 +10,7 @@ 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 { isVia, matchPathStepAndOp } from 'modules/pathfinding/utils'; import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types'; import { useAppDispatch } from 'store'; import { formatUicToCi } from 'utils/strings'; @@ -35,11 +35,7 @@ const ModalSuggestedVias = ({ suggestedVias }: ModalSuggestedViasProps) => { ); const removeViaFromPath = (op: SuggestedOP) => { - const updatedPathSteps = compact(pathSteps).filter( - (step) => - ('uic' in step && (step.uic !== op.uic || step.ch !== op.ch)) || - ('track' in step && (step.track !== op.track || step.offset !== op.offsetOnTrack)) - ); + const updatedPathSteps = compact(pathSteps).filter((step) => !matchPathStepAndOp(step, op)); dispatch( updatePathSteps({ pathSteps: updatedPathSteps, From bf9dc79d959ae2671de22b9d01f9076b1b7326cb Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Thu, 10 Oct 2024 12:40:40 +0200 Subject: [PATCH 4/4] front: use matchPathStepAndOp() in pathStepMatchesOp() The logic in matchPathStepAndOp() is more general. pathStepMatchesOp() performs some special checks on kp/name/opId so retain these. Also need to adapt the argument type for matchPathStepAndOp() to only pick the properties we really need. Signed-off-by: Simon Ser --- front/src/modules/pathfinding/utils.ts | 33 ++++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/front/src/modules/pathfinding/utils.ts b/front/src/modules/pathfinding/utils.ts index 8b121e4ee57..17c89b35815 100644 --- a/front/src/modules/pathfinding/utils.ts +++ b/front/src/modules/pathfinding/utils.ts @@ -34,7 +34,10 @@ export const formatSuggestedOperationalPoints = ( coordinates: getPointCoordinates(geometry, pathLength, op.position), })); -export const matchPathStepAndOp = (step: PathStep, op: SuggestedOP) => { +export const matchPathStepAndOp = ( + step: PathStep, + op: Pick +) => { if ('operational_point' in step) { return step.operational_point === op.opId; } @@ -155,15 +158,22 @@ export const upsertPathStepsInOPs = (ops: SuggestedOP[], pathSteps: PathStep[]): export const pathStepMatchesOp = ( pathStep: PathStep, - op: Pick, + op: Pick< + SuggestedOP, + 'opId' | 'uic' | 'ch' | 'trigram' | 'track' | 'offsetOnTrack' | 'name' | 'kp' + >, 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; +) => { + if (!matchPathStepAndOp(pathStep, op)) { + // TODO: we abuse the PathStep.id field here, the backend also sets it to an + // ID which has nothing to do with OPs + return pathStep.id === op.opId; + } + 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. @@ -175,6 +185,9 @@ export const pathStepMatchesOp = ( */ export const isVia = ( vias: PathStep[], - op: Pick, + op: Pick< + SuggestedOP, + 'opId' | 'uic' | 'ch' | 'trigram' | 'track' | 'offsetOnTrack' | 'name' | 'kp' + >, { withKP = false } = {} ) => vias.some((via) => pathStepMatchesOp(via, op, withKP));