-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Filter flex trips by agency and route #6421
base: dev-2.x
Are you sure you want to change the base?
Filter flex trips by agency and route #6421
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6421 +/- ##
=============================================
+ Coverage 70.18% 70.19% +0.01%
- Complexity 18308 18348 +40
=============================================
Files 2082 2085 +3
Lines 77188 77287 +99
Branches 7828 7844 +16
=============================================
+ Hits 54175 54255 +80
- Misses 20247 20253 +6
- Partials 2766 2779 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1243956
to
fd9e9ef
Compare
63fdbe2
to
03f20e6
Compare
# Conflicts: # application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java
Flesh out tests
03f20e6
to
f5ac386
Compare
@eibakke This is now ready for review. |
application/src/ext-test/java/org/opentripplanner/ext/flex/template/ClosestTripTest.java
Outdated
Show resolved
Hide resolved
application/src/ext/java/org/opentripplanner/ext/flex/filter/FilterMapper.java
Show resolved
Hide resolved
application/src/main/java/org/opentripplanner/transit/model/filter/expr/ExpressionBuilder.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/opentripplanner/transit/model/filter/expr/NegationMatcher.java
Outdated
Show resolved
Hide resolved
public TripRequestBuilder withAgencies(FilterValues<FeedScopedId> agencies) { | ||
this.agencies = agencies; | ||
public TripRequestBuilder withIncludedAgencies(Collection<FeedScopedId> agencies) { | ||
this.includedAgencies = FilterValues.ofEmptyIsNothing("includedAgencies", agencies); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eibakke This is a different behaviour to what happens in the TransmodelAPI. Let's discuss this in the meeting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see now. We'll have to fix the Transmodel API to handle empty and unset arguments better. Right now it is not working as intended.
Summary
It introduces filtering of flex trips by agency and route.
To achieve this, the regular filter request is mapped to a flex-specific version of it and then the filter logic is implemented that way.
I'm not 100% sure I got the layers of abstraction correct. I would appreciate a design or pair programming session about this.
Issue
Closes #5463
Unit tests
Lots added
Documentation
Javadoc