-
Notifications
You must be signed in to change notification settings - Fork 13
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): Use Stop ID to query OTP #1799
Conversation
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.
Overall looks good. Just a few questions.
const stopIdEl = this.getById(ac._selectors.stop_id); | ||
stopIdEl.value = hit.stop.id; |
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.
Is there a reason to set a const here?
this.getById(ac._selectors.stop_id).value = hit.stop.id;
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.
No reason, just preference
Just so I know, are there no tests to write here? |
This reverts commit 1878cc0.
I had to revert this in release 2023.11.22.02 as it wasn't working well when the origin and destination were not stop IDs. It raises a GraphQL exception when I try to plan a trip between Fall River Historical Society, 451 Rock St, Fall River, MA, 02720, USA and Worcester locally, here's the response the query returns: %{
"data" => %{"plan" => nil},
"errors" => [
%{
"extensions" => %{"classification" => "DataFetchingException"},
"locations" => [%{"column" => 3, "line" => 2}],
"message" => "Exception while fetching data (/plan) : Missing mandatory id on FeedScopeId [Value cannot be null, empty or just whitespace: '']",
"path" => ["plan"]
}
]
} And when interacting with it on the front-end, I noticed if I edited the destination for a working trip plan (e.g. I've already pulled up itineraries) from a stop to an address, and regenerate the plan, the itineraries would appear to update, but are actually routing me to the initially selected stop. Not sure what's going on there. |
Summary of changes
This pull request allows us to query OTP using stop ids. This is supposed to remove walking directions to get to the initial stop if the user is already assumed at the stop.
Asana Ticket: Fix Trip planner origin and destinations
General checks
New UI, or substantial UI changes
New endpoints, or non-trivial changes to current endpoints