front: simplify OP matching in updatePathStepsFromOperationalPoints() #9761
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emersion
force-pushed
the
emr/simplify-setup-itinerary-for-train-update
branch
3 times, most recently
from
c0dcdcf to
bed6c50
Compare
Trains created via NGE don't have a secondary_code field in their PathSteps passed to updatePathStepsFromOperationalPoints(). This can be checked by creating a train in NGE, setting a rolling stock for it, and then adding a console.log() right before the updatePathStepsFromOperationalPoints() call. The ch field is populated and the secondary_code field doesn't exist. This is because all PathItemLocation entries are piped into computeBasePathSteps(), which strips secondary_code regardless of whether the path steps are defined in terms of UIC (by OSRD) or trigram (by NGE). Signed-off-by: Simon Ser <[email protected]>
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 <[email protected]>
emersion
force-pushed
the
emr/simplify-setup-itinerary-for-train-update
branch
from
bed6c50 to
ea909bf
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We already have a
matchPathStepAndOp()function to compare aPathStepand aSuggestedOP. There are two differences:PathStep.chis set before comparing it withSuggestedOP.ch, but that's incorrect. On the editoast side thesecondary_codefield is defined asOption<String>:Noneaccepts OPs with any secondary code,""accepts only OPs without any secondary code. On the TypeScript side,undefinedaccepts OPs with any secondary code, and''accepts only OPs without any secondary code. However'ch' in stepis true for{ ch: undefined }, which compares SuggestedOP.ch with undefined even if an undefinedPathStep.chis supposed to match any OP secondary code.PathStep.secondary_code. However thePathSteps given toupdatePathStepsFromOperationalPoints()come straight up fromcomputeBasePathSteps(), which always leavessecondary_codeundefined. As a result we never hit this case.