Skip to content
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.

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

front: leave ch code null when unspecified in XML import #9763

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

emersion
Copy link
Member

@emersion emersion commented Nov 18, 2024

An empty string will specifically select OPs without any secondary
code. However that's very unlikely that's what the user wants: in
the French infrastructure, all OPs have a secondary code. Instead,
use undefined to match any secondary code.

While at it, drop the empty string default for ciCode: this is
always guaranteed to be a string (because split() always returns
an array with at least one element) so it's dead code.

To test: edit an XML file to remove a ch code (e.g. replace SG/BV with SG), then import it.

Also includes a cleanup commit.

An empty string will specifically select OPs without any secondary
code. However that's very unlikely that's what the user wants: in
the French infrastructure, all OPs have a secondary code. Instead,
use undefined to match any secondary code.

While at it, drop the empty string default for ciCode: this is
always guaranteed to be a string (because split() always returns
an array with at least one element) so it's dead code.

Signed-off-by: Simon Ser <[email protected]>
This is unconditionally converted to a number later on (when turning
it into a UIC). We don't need to store this as a string.

Signed-off-by: Simon Ser <[email protected]>
@emersion emersion requested a review from a team as a code owner November 18, 2024 16:43
@RomainValls
Copy link
Contributor

RomainValls commented Nov 19, 2024

Does that mean we don't want the OP to appear in the pathSteps, but only in the "allWaypoints", in case the ch code isn't specified in the xml file ?
If yes, tested, lgtm 👍

this is before removing the ch code for la freissinouse :
Capture d’écran 2024-11-19 à 14 46 32

this is after removing it :
Capture d’écran 2024-11-19 à 14 51 05

@emersion
Copy link
Member Author

emersion commented Nov 21, 2024

Does that mean we don't want the OP to appear in the pathSteps, but only in the "allWaypoints", in case the ch code isn't specified in the xml file ?

This sounds like a separate, orthogonal bug: when secondary_code is left null in a PathItemLocation, the backend matches it with any OP's secondary code, but the times and stops table doesn't.

I believe this bug happens on the dev branch without this patch.

@kmer2016
Copy link
Contributor

This sounds like a separate, orthogonal bug: when secondary_code is left null in a PathItemLocation, the backend matches it with any OP's secondary code, but the times and stops table doesn't.

I believe this bug happens on the dev branch without this patch.

You're absolutely right, this bug is indeed already present on the dev branch. It seems this bug ticket track it

@emersion emersion added this pull request to the merge queue Nov 25, 2024
Merged via the queue into dev with commit ba69de2 Nov 25, 2024
27 checks passed
@emersion emersion deleted the emr/xml-import-missing-ch branch November 25, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants