-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat(TripPlanner): add itinerary tags #1883
Merged
Merged
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
- deprecate `TripPlan.Api` behaviour and the modules using it (`TripPlan.Api.MockPlanner` and `TripPlan.Api.OpenTripPlanner`) in favor of using the client (`OpenTripPlannerClient` and its behaviour `OpenTripPlannerClient.Behaviour`). - remove `TripPlan.plan/4` function
Tagging has been updated. This screenshot shows both (1) tagged itineraries shown before untagged and (2) tag prioritization (#2 is labelled just "least walking" instead of both "least walking" and "shortest trip") |
anthonyshull
approved these changes
Feb 14, 2024
thecristen
merged commit Feb 14, 2024
227af7a
into
feat/trip-planner-improvements
16 of 18 checks passed
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.
Summary of changes
Asana Ticket: Trip planner backend changes and Trip planner frontend changes
Lots of changes here!
Using the OpenTripPlannerClient
I've extracted OpenTripPlanner-touching code into a standalone open_trip_planner_client library, which I hope to move into our Github org and share with @mbta/mobile-app soon.
So far it primarily implements the
plan/3
function to query OTP'splan
, but under the hood it parses input arguments, dynamically constructs the GraphQL query, and handles very light parsing (mainly just error handling), while also applying tags (thanks mbta/mobile_app_backend#34 !).This PR updates our codebase to use the client's implementation. Half of the changes in this PR involved removing various bits of our code and adjusting others.
Mocking and test data generation
I added two long-recommend libraries,
Mox
andExMachina
, along withFaker
for generating data.The client library exposes a behavior,
OpenTripPlannerClient.Behaviour
, which we use with theMox
library to provide some mocked functionality in tests.Separately, we replace the
MockPlanner
functions with test factories created usingExMachina
andFaker
.The other half of the bulk of changes in this PR are from refactoring many tests.
Adding itinerary tags to the UI
Each itinerary should have at most one tag, and all tagged itineraries are shown before the rest.
General checks