Skip to content

Fix problem with submode filter - #4093

Merged
Bartosz-Kruba merged 2 commits into
opentripplanner:dev-2.xfrom
Skanetrafiken:submode_filter
Apr 20, 2022
Merged

Fix problem with submode filter#4093
Bartosz-Kruba merged 2 commits into
opentripplanner:dev-2.xfrom
Skanetrafiken:submode_filter

Conversation

@Bartosz-Kruba

@Bartosz-Kruba Bartosz-Kruba commented Apr 13, 2022

Copy link
Copy Markdown
Contributor

Summary

Ensure that Transmodel GraphQL UNKNOWN submode matches on nulls as well. This is needed to ensure that filter works with GTFS data.

Issue

GTFS trips does always have netex submode that is null. This means that no matter which submode filter we set, those trips will always be filtered out (there will be no match on null).

Unit tests

Created some tests for AllowedTransitMode class

Code style

Formatted with maven plugin

Documentation

No new documentation added.

@Bartosz-Kruba
Bartosz-Kruba requested a review from a team as a code owner April 13, 2022 08:49
@MikaelJarfors MikaelJarfors added the Skanetrafiken On skanetrafikens roadmap label Apr 13, 2022
- Ensure that UNKNOWN submode matches on null.
boolean submodeMatch =
subMode == null ||
subMode.equals(netexSubMode) ||
(subMode.equals("unknown") && netexSubMode == null);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you please move the string "unknown" into a constant?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@leonardehrenfried leonardehrenfried left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it not be really easy to add a test for AllowedTransitMode?

- Ensure that UNKNOWN submode matches on null.
@Bartosz-Kruba

Copy link
Copy Markdown
Contributor Author

Would it not be really easy to add a test for AllowedTransitMode?

I've added some tests now

@hannesj

hannesj commented Apr 19, 2022

Copy link
Copy Markdown
Contributor

Should we always allow for null submodes, even when a submode is set in the request, and not only when supplying unknown?

@Bartosz-Kruba
Bartosz-Kruba merged commit 94931c5 into opentripplanner:dev-2.x Apr 20, 2022
@Bartosz-Kruba
Bartosz-Kruba deleted the submode_filter branch April 20, 2022 08:07
t2gran pushed a commit that referenced this pull request Apr 20, 2022
@t2gran t2gran added this to the 2.2 milestone Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skanetrafiken On skanetrafikens roadmap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants