Skip to content

front: handle unrecognized track in itinerary modal #16634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
theocrsb wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

front: handle unrecognized track in itinerary modal #16634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7f7158
front: handle unrecognized track in itinerary modal
theocrsb May 7, 2026
88e1116
front: persist unknown local track name
theocrsb May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions front/public/locales/en/operational-studies.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@
"moveLocationOnMap": "Move location on the map",
"movePointOnMap": "Move this waypoint using the map",
"next": "Next",
"noComputation": "there will be no computation for this train",
Copy link
Copy Markdown
Contributor

@SarahBellaha SarahBellaha May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"noComputation": "there will be no computation for this train",
"noComputation": "There will be no computation for this train.",

"opId": "Operational point identifier",
"opName": "Operational point name",
"opType": "Type",
Expand Down
1 change: 1 addition & 0 deletions front/public/locales/fr/operational-studies.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@
"moveLocationOnMap": "Déplacer le point remarquable sur la carte",
"movePointOnMap": "Déplacez ce point en utilisant la carte",
"next": "Suivant",
"noComputation": "il n'y aura pas de calcul pour ce train",
Copy link
Copy Markdown
Contributor

@SarahBellaha SarahBellaha May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"noComputation": "il n'y aura pas de calcul pour ce train",
"noComputation": "Il n'y aura pas de calcul pour ce train.",

"opId": "Identifiant de point",
"opName": "Nom du point remarquable",
"opType": "Type",
Expand Down
48 changes: 45 additions & 3 deletions ...ationalStudies/views/Scenario/components/ManageTrainSchedule/Itinerary/ItineraryModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {
PathItemLocation,
TrainCategory,
} from 'common/api/osrdEditoastApi';
import { osrdEditoastApi } from 'common/api/osrdEditoastApi';
import Banner from 'common/Banner';
import { computeBBoxViewport } from 'common/Map/WarpedMap/core/helpers';
import { useInfraID } from 'common/osrdContext';
Expand All @@ -35,7 +36,7 @@ import {
getPathSteps,
getRollingStockName,
} from 'reducers/osrdconf/operationalStudiesConf/selectors';
import type { PathStep, PathStepMetadata, PathStepV2 } from 'reducers/osrdconf/types';
import type { PathStep, PathStepMetadata, PathStepV2, TrainScheduleToEditData } from 'reducers/osrdconf/types';
import { useAppDispatch } from 'store';
import { addElementAtIndex } from 'utils/array';
import { Duration } from 'utils/duration';
Expand All @@ -61,6 +62,7 @@ type ItineraryModalProps = {
itineraryModalIsOpen: boolean;
setItineraryModalIsOpen: (isOpen: boolean) => void;
displayTrainScheduleManagement: string;
trainScheduleToEditData?: TrainScheduleToEditData;
Copy link
Copy Markdown
Contributor

@SarahBellaha SarahBellaha May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can only pass the trainScheduleId here

};

export type ItineraryModalFormState = {
Expand All @@ -75,6 +77,7 @@ const ItineraryModal = ({
itineraryModalIsOpen,
setItineraryModalIsOpen,
displayTrainScheduleManagement,
trainScheduleToEditData,
}: ItineraryModalProps) => {
const { t } = useTranslation('operational-studies', {
keyPrefix: 'manageTrainSchedule.itineraryModal',
Expand Down Expand Up @@ -153,6 +156,42 @@ const ItineraryModal = ({
const { launchPathfindingV2, pathProperties, pathfindingError } = usePathfindingV2();
const { convertFeatureClickToLocation } = useMapTrackSelection(infraId);

/**
* When a custom track name is added (one that doesn't exist in the OP parts),
* immediately update the train schedule's path via API to persist the local_track_name.
*/
const updateTrainSchedulePathTrackName = useCallback(
async (stepId: string, trackName: string) => {
if (!trainScheduleToEditData) return;

const { trainScheduleId } = trainScheduleToEditData;

// GET the current train schedule fresh from the DB to avoid overwriting anything
const { id: _, train_schedule_set_id: __, ...currentData } = await dispatch(
osrdEditoastApi.endpoints.getTrainSchedulesById.initiate(
{ id: trainScheduleId },
{ subscribe: false }
)
).unwrap();

// Only update the local_track_name on the matching path step
await dispatch(
osrdEditoastApi.endpoints.putTrainSchedulesById.initiate({
id: trainScheduleId,
trainSchedule: {
...currentData,
path: currentData.path.map((item) =>
item.id === stepId && item.location.type !== 'track_offset'
? { ...item, location: { ...item.location, local_track_name: trackName || undefined } }
Copy link
Copy Markdown
Contributor

@SarahBellaha SarahBellaha May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
? { ...item, location: { ...item.location, local_track_name: trackName || undefined } }
? { ...item, location: { ...item.location, local_track_name: trackName } }

Comment on lines +178 to +185
Copy link
Copy Markdown
Contributor

@SarahBellaha SarahBellaha May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this, you can sort of re-use a part of the updatePacedTrain helper by spliting it like :

export async function updateTrainSchedule(dispatch: AppDispatch, id: number, trainSchedule: TrainSchedule) {
  await dispatch(
    osrdEditoastApi.endpoints.putTrainSchedulesById.initiate({
      id,
      trainSchedule,
    })
  ).unwrap();
}

async function updatePacedTrain(dispatch: AppDispatch, id: number, trainSchedule: TrainSchedule) {
  if (trainSchedule.paced?.exceptions && trainSchedule.paced.exceptions.length > 0) {
    console.error(
      'updatePacedTrain: exceptions should not be included in the paced field. Use exception endpoints instead.'
    );
  }
  await updateTrainSchedule(dispatch, id, trainSchedule);
}

: item
),
},
})
).unwrap();
},
[dispatch, trainScheduleToEditData]
);

const applyOperationalPointToStep = (
stepId: string,
suggestion: OperationalPointSuggestion,
Expand Down Expand Up @@ -546,12 +585,12 @@ const ItineraryModal = ({
{rollingStockMessage && <Banner type="info" message={rollingStockMessage} />}
{hasInvalidPathStepDisplay && (
<div key={`invalid-op-${bannerWiggle}`}>
<Banner type="error" message={t('alertInvalidOP')} />
<Banner type="info" message={`${t('alertInvalidOP')} ${t('noComputation')}`} />
Copy link
Copy Markdown
Contributor

@SarahBellaha SarahBellaha May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Banner type="info" message={`${t('alertInvalidOP')} ${t('noComputation')}`} />
<Banner type="info" message={`${t('alertInvalidOP')}. ${t('noComputation')}`} />

You can either add the missing period between the two sentences here or, better, directly in the translation file :)

</div>
)}
{!hasInvalidPathStepDisplay && pathfindingError && (
<div key={`pathfinding-${bannerWiggle}`}>
<Banner type="error" message={pathfindingError} />
<Banner type="info" message={`${pathfindingError} ${t('noComputation')}`} />
Copy link
Copy Markdown
Contributor

@SarahBellaha SarahBellaha May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Banner type="info" message={`${pathfindingError} ${t('noComputation')}`} />
<Banner type="info" message={`${pathfindingError}. ${t('noComputation')}`} />

Same

</div>
)}
{submitAttempted &&
Expand Down Expand Up @@ -678,6 +717,9 @@ const ItineraryModal = ({
})
);
}}
onAddCustomTrackName={(trackName) => {
updateTrainSchedulePathTrackName(pathStep.id, trackName);
}}
onOpBlur={() => {
// If the user focuses out on an input with a valid op, we display the last valid op of this input (or empty)
const valueOnFocus = focusValueRef.current[pathStep.id];
Expand Down
55 changes: 40 additions & 15 deletions ...erationalStudies/views/Scenario/components/ManageTrainSchedule/Itinerary/PathStepItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useMemo } from 'react';
import { useEffect, useMemo, useState } from 'react';

import { ComboBox, Select, SegmentedControl } from '@osrd-project/ui-core';
import { ComboBox, SegmentedControl } from '@osrd-project/ui-core';
import {
AddedLocation,
AddLocation,
Expand Down Expand Up @@ -38,6 +38,7 @@ type PathStepProps = {
categoryColors: CategoryColors;
onOpInputChange: (value: string) => void;
onTrackNameChange: (trackName: string) => void;
onAddCustomTrackName?: (trackName: string) => void;
onOpFocus: () => void;
onOpBlur: () => void;
inputValue: string | undefined;
Expand Down Expand Up @@ -66,6 +67,7 @@ const PathStepItem = ({
categoryColors,
onOpInputChange,
onTrackNameChange,
onAddCustomTrackName,
onOpFocus,
onOpBlur,
inputValue,
Expand Down Expand Up @@ -110,11 +112,8 @@ const PathStepItem = ({
return (message += t('requestedPoint'));
}

const trackInfo =
location && location.local_track_name ? `, ${t('track')} ${location.local_track_name}` : '';

if (location?.operational_point.type === 'id') {
return (message += t('opId') + trackInfo);
return (message += t('opId'));
}

const secondaryCodeInfo =
Expand All @@ -130,7 +129,7 @@ const PathStepItem = ({
message += t('uic') + ' ' + location.operational_point.uic;
}

return message + secondaryCodeInfo + trackInfo;
return message + secondaryCodeInfo;
};

const selectedSecondaryCodeOption = useMemo(() => {
Expand Down Expand Up @@ -169,17 +168,29 @@ const PathStepItem = ({
return [{ label: '', id: '' }, ...sortedSuggestions];
}, [pathStepMetadata, selectedSecondaryCodeOption.id]);

const [filteredTrackSuggestions, setFilteredTrackSuggestions] = useState(trackNameSuggestions);

useEffect(() => {
setFilteredTrackSuggestions(trackNameSuggestions);
}, [trackNameSuggestions]);

const selectedTrackNameOption = useMemo(() => {
// When OP is invalid but has a local_track_name, show it
if (pathStepMetadata?.isInvalid && pathStepMetadata.localTrackName) {
return { label: pathStepMetadata.localTrackName, id: pathStepMetadata.localTrackName };
}

// No track should be selected if the path step is invalid or has no secondary code
// or is a step added by map click

if (!isOpRefMetadata(pathStepMetadata) || !pathStepMetadata.trackName) {
return EMPTY_OPTION;
}

return (
trackNameSuggestions.find((track) => track.label === pathStepMetadata.trackName) ||
EMPTY_OPTION
trackNameSuggestions.find((track) => track.label === pathStepMetadata.trackName) || {
label: pathStepMetadata.trackName,
id: pathStepMetadata.trackName,
}
);
}, [pathStepMetadata, trackNameSuggestions]);

Expand Down Expand Up @@ -293,6 +304,7 @@ const PathStepItem = ({
className={cx('path-step-wrapper', {
'is-placeholder': isTrailingPlaceHolder,
'map-selection-active': isMapSelectionMode,
'is-invalid': isInvalidAndIsEditing,
})}
>
<div
Expand Down Expand Up @@ -455,15 +467,28 @@ const PathStepItem = ({
<div
className={cx('track-name', {
invalid: isInvalidAndIsEditing,
'track-name-unknown':
isOpRefMetadata(pathStepMetadata) && pathStepMetadata.isTrackNameUnknown,
})}
>
<Select
<ComboBox
id={`pathStep-status-${pathStep.id}`}
value={selectedTrackNameOption}
options={trackNameSuggestions}
getOptionLabel={(option) => option.label}
getOptionValue={(option) => option.id}
onChange={(option) => onTrackNameChange(option?.label ?? '')}
suggestions={filteredTrackSuggestions}
getSuggestionLabel={(option) => option.label}
onChange={(e) => {
const input = e.target.value.toLowerCase();
setFilteredTrackSuggestions(
trackNameSuggestions.filter((s) => s.label.toLowerCase().includes(input))
);
}}
onSelectSuggestion={(option) => onTrackNameChange(option?.label ?? '')}
resetSuggestions={() => setFilteredTrackSuggestions(trackNameSuggestions)}
allowCustomValue
onAddCustomValue={(value) => {
onTrackNameChange(value);
onAddCustomTrackName?.(value);
}}
small
narrow
/>
Expand Down
22 changes: 13 additions & 9 deletions ...ies/views/Scenario/components/ManageTrainSchedule/Itinerary/hooks/usePathStepsMetadata.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,19 +105,22 @@ export const usePathStepsMetadata = (
const matchedOp = pathStepsOperationalPoints.get(pathStep.id)?.at(0);

const { local_track_name } = location;
const isValidLocalTrackName = local_track_name
? matchedOp?.parts.some((part) => {

if (!matchedOp) {
newPathStepsMetadataById.set(pathStep.id, {
isInvalid: true,
localTrackName: local_track_name ?? undefined,
});
return;
}

const isTrackNameUnknown = local_track_name
Copy link
Copy Markdown
Contributor

@SarahBellaha SarahBellaha May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this naming/logic combination a bit odd: isTrackNameUnknown is computed through a negated condition, which makes the code less intuitive to read.
I think it would be clearer to go back to:

const isValidLocalTrackName = local_track_name
          ? matchedOp?.parts.some((part) => {
              const track = trackSectionsById[part.track];
              if (!track) return false;
              return local_track_name === part.local_track_name;
            })
          : false;

Also, if local_track_name is undefined, i believe isValidLocalTrackName should be false rather than true.
You can then use it l.139 with a negation :)

? !matchedOp.parts.some((part) => {
const track = trackSectionsById[part.track];
if (!track) return false;
return local_track_name === part.local_track_name;
})
: true;

// If no op is found or if its local_track_name is invalid, it means the path step is invalid
if (!isValidLocalTrackName || !matchedOp) {
newPathStepsMetadataById.set(pathStep.id, { isInvalid: true });
return;
}
: false;

const parts: Extract<PathStepMetadata, { isInvalid: false; type: 'opRef' }>['parts'] =
matchedOp.parts.map((part) => ({
Expand All @@ -133,6 +136,7 @@ export const usePathStepsMetadata = (
uic: matchedOp.extensions?.identifier?.uic,
secondaryCode: matchedOp.extensions?.sncf?.ch,
trackName: local_track_name ?? undefined,
isTrackNameUnknown,
parts,
});
});
Expand Down
3 changes: 2 additions & 1 deletion ...tions/operationalStudies/views/Scenario/components/ManageTrainSchedule/Itinerary/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export const computePathStepCoordinates = (pathStepMetadata: PathStepMetadata) =
const foundPart = pathStepMetadata.parts.find(
(part) => part.trackName === pathStepMetadata.trackName
);
return foundPart ? [foundPart.coordinates] : [];
// If the track name is unknown (not matching any part), fall back to all parts coordinates
return foundPart ? [foundPart.coordinates] : pathStepMetadata.parts.map((p) => p.coordinates);
}
if (pathStepMetadata.secondaryCode) {
return pathStepMetadata.parts.map((part) => part.coordinates);
Expand Down
1 change: 1 addition & 0 deletions ...nalStudies/views/Scenario/components/ManageTrainSchedule/ManageTrainScheduleLeftPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ const ManageTrainScheduleLeftPanel = ({
itineraryModalIsOpen={itineraryModalIsOpen}
setItineraryModalIsOpen={setItineraryModalIsOpen}
displayTrainScheduleManagement={displayTrainScheduleManagement}
trainScheduleToEditData={trainScheduleToEditData}
/>
)}
<div
Expand Down
7 changes: 6 additions & 1 deletion front/src/reducers/osrdconf/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export type PathStepV2 = {
};

export type PathStepMetadata =
| { isInvalid: true }
| { isInvalid: true; localTrackName?: string }
| {
type: 'trackOffset';
isInvalid: false;
Expand All @@ -160,6 +160,11 @@ export type PathStepMetadata =
* we won't have access to the corresponding track name in the path step location
*/
trackName?: string;
/**
* True when the local_track_name from the location does not match any part of the matched OP.
* The step is still considered valid (OP was found), but the requested track is unknown.
*/
isTrackNameUnknown?: boolean;
/**
* Data of all parts (tracks) of the path step (name + ch)
*/
Expand Down
16 changes: 12 additions & 4 deletions front/src/styles/scss/applications/operationalStudies/_itineraryModal.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,11 @@
}
}

&.is-invalid {
// blackAlpha10
background-color: rgba(0, 0, 0, 0.1);
Comment on lines +297 to +298
Copy link
Copy Markdown
Contributor

@Akctarus Akctarus May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you use black10 ?

Suggested change
// blackAlpha10
background-color: rgba(0, 0, 0, 0.1);
background-color: var(--black10);

Copy link
Copy Markdown
Contributor Author

@theocrsb theocrsb May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Black10 is different. where we using transparency

Copy link
Copy Markdown
Contributor

@SarahBellaha SarahBellaha May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can totally use var(--black10) 👍
rgba(0, 0, 0, 0.1) means you are using the black color with 10% opacity

}

.path-step-counter:not(.is-trailing-placeholder):not(.is-only-step):hover {
opacity: 1;
background-color: var(--error60) !important;
Expand Down Expand Up @@ -407,8 +412,7 @@
}

&.invalid {
background-color: var(--error60);
border-color: var(--error30);
background-color: var(--info60);
}
&.is-only-step {
pointer-events: none;
Expand All @@ -423,7 +427,6 @@

&.invalid {
border-radius: 3px;
box-shadow: 0 0 0 1.5px rgba(255, 104, 104, 1);
}
}

Expand All @@ -433,6 +436,11 @@
border-radius: 4px;
}

.track-name-unknown .ui-combo-box {
border-radius: 3px;
box-shadow: 0 0 0 1.5px var(--grey50);
}

.map-interactions {
margin-left: 5px;
color: var(--primary60);
Expand Down Expand Up @@ -481,7 +489,7 @@
}

.invalid-step-message {
color: var(--error60);
color: var(--grey60);
padding-left: calc(51px - var(--path-step-wrapper-padding-left));
}
}
Expand Down
Loading