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

Readjust walking route departure and arrival time due to bus buffer time #274

Merged
merged 1 commit into from
Nov 2, 2019

Conversation

alanna-zhou
Copy link
Contributor

@alanna-zhou alanna-zhou commented Nov 2, 2019

Overview

Right now, there are two cases in which we display a walking route (note that we always show a walking route), and here are screenshots of this tragic issue we've been having with walking times:

1. A walking route we get solely from Graphhopper walking (when there are no other bus routes)

Here you can notice that the start time should actually be 11:56!

2. A walking route that we get from Graphhopper bus, which involves stuffing in a bus buffer time (a trick that we've been using so that we get more routes around the time that the client sends)

Once again, the start time should actually be 11:56!

Changes Made

There are a lot of entry points that I found that I could modify the bus routes. The way that I found that was most intuitive was in ParseRouteUtils. In RouteUtils, we first fetch routes with GraphhopperUtils, and then we parse them with parseRoute in parseRouteUtils. I thought that I should leave fetching to GraphhopperUtils and modifying the routes in parseRouteUtils since we have similar logic in there. What I did was check if the route.transfers attribute was equal to -1 (which means it is a walking route), and just overwrite its departure and arrival times in both the walking route and the walking route's direction's array (direction's start and end times) -- since we really don't know which bus buffer time (could be 20 min or 40 min difference) that was used to fetch the routes, as there's some complex logic used to merge bus routes afterwards.

I also refactored parseWalkingRoute to match my changes above so that other people know that these are exactly the same things getting modified -- it should be time and not date in the names we're using because the client is getting these with the keys named as arrivalTime and departureTime, not arrivalDate and departureDate!

Now that everything's fixed, here's how it looks on the dev server:

1. A lone walking route

2. A walking route with other bus routes

3. A walking route with arriveBy as true

Test Coverage

Pushed onto the dev server as of 11/2 noon, and everything looks fine just by going through the beta app. Will have it sit for a while and see if integration picks up anything funky -- but integration isn't testing this to begin with so I'm doubtful that this is necessary.

Next Steps

Going to talk to integration about testing departure and arrival times on walking routes.

Related PRs or Issues

Resolves Issue #261

src/utils/RouteUtils.js Outdated Show resolved Hide resolved
src/utils/ParseRouteUtils.js Outdated Show resolved Hide resolved
src/utils/ParseRouteUtils.js Outdated Show resolved Hide resolved
src/utils/ParseRouteUtils.js Outdated Show resolved Hide resolved
Copy link
Contributor

@Omarrasheed Omarrasheed left a comment

Choose a reason for hiding this comment

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

🚢

@alanna-zhou alanna-zhou merged commit bb4ab65 into master Nov 2, 2019
@meganle meganle deleted the alanna/fix-walking branch November 17, 2019 07:26
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.

4 participants