Skip to content

Commit 88ef272

Browse files
committed
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]>
1 parent 844c6b3 commit 88ef272

File tree

5 files changed

+10
-47
lines changed

5 files changed

+10
-47
lines changed

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

+3-4
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import ModalBodySNCF from 'common/BootstrapSNCF/ModalSNCF/ModalBodySNCF';
99
import ModalFooterSNCF from 'common/BootstrapSNCF/ModalSNCF/ModalFooterSNCF';
1010
import ModalHeaderSNCF from 'common/BootstrapSNCF/ModalSNCF/ModalHeaderSNCF';
1111
import { useOsrdConfActions, useOsrdConfSelectors } from 'common/osrdContext';
12-
import { isVia, matchPathStepAndOp } from 'modules/pathfinding/utils';
1312
import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
1413
import type { PathStep } from 'reducers/osrdconf/types';
1514
import { useAppDispatch } from 'store';
@@ -36,12 +35,12 @@ const ModalSuggestedVias = ({ suggestedVias, launchPathfinding }: ModalSuggested
3635
);
3736

3837
const removeViaFromPath = (op: SuggestedOP) => {
39-
const newPathSteps = pathSteps.filter((step) => !matchPathStepAndOp(step, op));
38+
const newPathSteps = pathSteps.filter((step) => step.id !== op.pathStepId);
4039
launchPathfinding(newPathSteps);
4140
};
4241

4342
const formatOP = (op: SuggestedOP, idx: number, idxTrueVia: number) => {
44-
const isInVias = isVia(vias, op);
43+
const isInVias = vias.find((step) => step.id === op.pathStepId);
4544
return (
4645
<div
4746
key={`suggested-via-modal-${op.opId}-${idx}`}
@@ -104,7 +103,7 @@ const ModalSuggestedVias = ({ suggestedVias, launchPathfinding }: ModalSuggested
104103
{suggestedVias.map((via, idx) => {
105104
if (!isOriginOrDestination(via)) {
106105
// If name is undefined, we know the op/via has been added by clicking on map
107-
if (isVia(vias, via)) idxTrueVia += 1;
106+
if (vias.find((step) => step.id === via.pathStepId)) idxTrueVia += 1;
108107
return formatOP(via, idx, idxTrueVia);
109108
}
110109
return null;

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

+2-3
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import { useOsrdConfActions, useOsrdConfSelectors } from 'common/osrdContext';
1717
import {
1818
formatSuggestedOperationalPoints,
1919
getPathfindingQuery,
20-
matchPathStepAndOp,
2120
upsertPathStepsInOPs,
2221
} from 'modules/pathfinding/utils';
2322
import { useStoreDataForRollingStockSelector } from 'modules/rollingStock/components/RollingStockSelector/useStoreDataForRollingStockSelector';
@@ -122,8 +121,8 @@ const usePathfinding = (
122121
// We update existing pathsteps with coordinates, positionOnPath and kp corresponding to the new pathfinding result
123122
const updatedPathSteps: (PathStep | null)[] = pathStepsInput.map((step, i) => {
124123
if (!step) return step;
125-
const correspondingOp = suggestedOperationalPoints.find((suggestedOp) =>
126-
matchPathStepAndOp(step, suggestedOp)
124+
const correspondingOp = suggestedOperationalPoints.find(
125+
(suggestedOp) => step.id === suggestedOp.pathStepId
127126
);
128127

129128
const theoreticalMargin = i === 0 ? step.theoreticalMargin || '0%' : step.theoreticalMargin;

front/src/modules/pathfinding/utils.ts

-17
Original file line numberDiff line numberDiff line change
@@ -155,23 +155,6 @@ export const pathStepMatchesOp = (
155155
return true;
156156
};
157157

158-
/**
159-
* Check if a suggested operational point is a via.
160-
* Some OPs have same uic so we need to check also the ch (can be still not enough
161-
* probably because of imports problem).
162-
* If the vias has no uic, it has been added via map click and we know it has an id.
163-
* @param withKP - If true, we check the kp compatibility instead of the name.
164-
* It is used in the times and stops table to check if an operational point is a via.
165-
*/
166-
export const isVia = (
167-
vias: PathStep[],
168-
op: Pick<
169-
SuggestedOP,
170-
'pathStepId' | 'opId' | 'uic' | 'ch' | 'trigram' | 'track' | 'offsetOnTrack' | 'name' | 'kp'
171-
>,
172-
{ withKP = false } = {}
173-
) => vias.some((via) => pathStepMatchesOp(via, op, withKP));
174-
175158
export const isStation = (chCode: string): boolean =>
176159
chCode === 'BV' || chCode === '00' || chCode === '';
177160

front/src/modules/timesStops/TimesStopsInput.tsx

+2-5
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { useTranslation } from 'react-i18next';
77

88
import { useScenarioContext } from 'applications/operationalStudies/hooks/useScenarioContext';
99
import { useOsrdConfActions } from 'common/osrdContext';
10-
import { isVia, matchPathStepAndOp } from 'modules/pathfinding/utils';
1110
import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
1211
import type { PathStep } from 'reducers/osrdconf/types';
1312
import { useAppDispatch } from 'store';
@@ -42,7 +41,7 @@ const createClearViaButton = ({
4241
allWaypoints &&
4342
rowIndex > 0 &&
4443
rowIndex < allWaypoints.length - 1 &&
45-
isVia(pathSteps || [], rowData, { withKP: true }) &&
44+
pathSteps.find((step) => step.id === rowData.pathStepId) &&
4645
(!isNil(rowData.stopFor) ||
4746
rowData.theoreticalMargin !== undefined ||
4847
rowData.arrival !== undefined ||
@@ -72,9 +71,7 @@ const TimesStopsInput = ({ allWaypoints, startTime, pathSteps }: TimesStopsInput
7271
const { getTrackSectionsByIds, trackSectionsLoading } = useScenarioContext();
7372

7473
const clearPathStep = (rowData: TimesStopsInputRow) => {
75-
const index = pathSteps.findIndex(
76-
(step) => matchPathStepAndOp(step, rowData) && step.positionOnPath === rowData.positionOnPath
77-
);
74+
const index = pathSteps.findIndex((step) => step.id === rowData.pathStepId);
7875

7976
const updatedPathSteps = pathSteps.map((step, i) => {
8077
if (i === index) {

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

+3-18
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { keyColumn, createTextColumn } from 'react-datasheet-grid';
66

77
import type { ReceptionSignal } from 'common/api/osrdEditoastApi';
88
import type { IsoDurationString, TimeString } from 'common/types';
9-
import { matchPathStepAndOp } from 'modules/pathfinding/utils';
109
import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types';
1110
import type { PathStep } from 'reducers/osrdconf/types';
1211
import { NO_BREAK_SPACE } from 'utils/strings';
@@ -24,18 +23,6 @@ import {
2423
import { marginRegExValidation, MarginUnit } from '../consts';
2524
import { TableType, type TimeExtraDays, type TimesStopsInputRow } from '../types';
2625

27-
const matchPathStepAndOpWithKP = (step: PathStep, op: SuggestedOP) => {
28-
if (!matchPathStepAndOp(step, op)) {
29-
return step.id === op.pathStepId;
30-
}
31-
// We match the kp in case two OPs have the same uic+ch (can happen when the
32-
// infra is imported)
33-
if ('uic' in step || 'trigram' in step) {
34-
return step.kp === op.kp;
35-
}
36-
return true;
37-
};
38-
3926
export const formatSuggestedViasToRowVias = (
4027
operationalPoints: (SuggestedOP & { isWaypoint?: boolean })[],
4128
pathSteps: PathStep[],
@@ -49,7 +36,7 @@ export const formatSuggestedViasToRowVias = (
4936
// to move it to the first position
5037
const origin = pathSteps[0];
5138
const originIndexInOps = origin
52-
? operationalPoints.findIndex((op) => matchPathStepAndOpWithKP(origin, op))
39+
? operationalPoints.findIndex((op) => origin.id === op.pathStepId)
5340
: -1;
5441
if (originIndexInOps !== -1) {
5542
[formattedOps[0], formattedOps[originIndexInOps]] = [
@@ -60,9 +47,7 @@ export const formatSuggestedViasToRowVias = (
6047

6148
// Ditto: destination should be last
6249
const dest = pathSteps[pathSteps.length - 1];
63-
const destIndexInOps = dest
64-
? operationalPoints.findIndex((op) => matchPathStepAndOpWithKP(dest, op))
65-
: -1;
50+
const destIndexInOps = dest ? operationalPoints.findIndex((op) => dest.id === op.pathStepId) : -1;
6651
if (destIndexInOps !== -1) {
6752
const lastOpIndex = formattedOps.length - 1;
6853
[formattedOps[lastOpIndex], formattedOps[destIndexInOps]] = [
@@ -72,7 +57,7 @@ export const formatSuggestedViasToRowVias = (
7257
}
7358

7459
return formattedOps.map((op, i) => {
75-
const pathStep = pathSteps.find((step) => matchPathStepAndOpWithKP(step, op));
60+
const pathStep = pathSteps.find((step) => step.id === op.pathStepId);
7661
const { name } = pathStep || op;
7762
const objectToUse = tableType === TableType.Input ? pathStep : op;
7863

0 commit comments

Comments
 (0)