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

refactor(StopPageRedesign): use route patterns #1759

Merged
merged 6 commits into from
Sep 29, 2023

Conversation

thecristen
Copy link
Collaborator

Summary of changes

Asana Ticket: Stops showing Headsign for Route Even if One Direction of Travel

Backend highlights

  • Adds /api/stop/:stop_id/route-patterns to get a JSON object of augmented route patterns grouped by headsign and route ID.
  • Replaces the /routes/by-stop/:stop_id with /routes/:route_ids that returns a JSON response containing a simple list of routes.
  • The polyline information continues to be computed in the backend, but is now part of the route pattern response as opposed to appending the routes response.

Frontend highlights

  • Adds a loader to the React Router's configuration to fetch the route patterns, which will be available from within child components via React Router's useLoaderData() hook. Disables revalidation because this data is not going to change.
  • Adds a helper function, departureInfoInRoutePatterns, to filter the list of departures against a list of route patterns.
  • Lots of restructuring:
    • <StopPageRedesign />
      • Get route patterns, and use the route IDs from that to fetch routes
      • Don't wait for routes to return a defined response, since they never will for stops that don't have any route patterns (South Attleboro)
    • <DeparturesAndMap />
      • Remove no longer needed routesWithPolylines prop
      • Get route patterns, use the included polylines
    • <StopPageDepartures />
      • Get route patterns, figure out route patterns for the filtered routes
    • <DepartureCard />
      • Add routePatternsByHeadsign prop
      • Refactors to render a <DepartureTimes /> per each headsign
      • Sets an onClick variable to pass to child
    • <DepartureTimes />
      • Adds headsign, isCR, and onClick props
      • Removes check of stop name against route's direction_destination as it should no longer be needed. Removes no longer needed route, directionId and stopName props.
      • Renames departuresForDirection prop to just departures (they were filtered by departureInfoInRoutePatterns earlier)

General checks

  • Are the changes organized into self-contained commits with descriptive and well-formatted commit messages? This is a good practice that can facilitate easier reviews.
  • Testing. Do the changes include relevant passing updates to tests? This includes updating screenshots. Preferably tests are run locally to verify that there are no test failures created by these changes, before opening a PR.
  • Tech debt. Have you checked for tech debt you can address in the area you're working in? This can be a good time to address small issues, or create Asana tickets for larger issues.

New endpoints, or non-trivial changes to current endpoints

  • Have we load-tested any new pages or internal API endpoints that will receive significant traffic? See load testing docs
  • If this change involves routes, does it work correctly with pertinent "unusual" routes such as the combined Green Line, Silver Line, Foxboro commuter rail, and single-direction bus routes like the 170?

@i0sea i0sea added the dev-blue Deploy to dev-blue label Sep 28, 2023
@i0sea i0sea temporarily deployed to dev-blue September 28, 2023 14:34 — with GitHub Actions Inactive
defp route_patterns_by_route_and_headsign(stop_id) do
stop_id
|> RoutePatterns.Repo.by_stop_id()
|> Enum.reject(&ends_at?(&1, stop_id))
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe there was a check for "is at last stop" on the JS side. Has that been removed since it's happening here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@i0sea Yep, it's been removed!

|> Map.put(:polylines, route_polylines(route, stop_id))
end)
@spec get(Plug.Conn.t(), map) :: Plug.Conn.t()
def get(conn, %{"route_ids" => route_ids} = _params) do
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: rename to get_by_route_ids for clarity

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@i0sea okay, changed

@thecristen thecristen temporarily deployed to dev-blue September 28, 2023 18:09 — with GitHub Actions Inactive
Copy link
Contributor

@i0sea i0sea left a comment

Choose a reason for hiding this comment

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

Not sure if this was occuring before these changes but am noticing routes that never stop at a given stop being listed with a headsign -> no upcoming trips. Example the 32 never stops here but is listed anyway. The oneway headsigns do appear to be dropped

Copy link
Contributor

@i0sea i0sea left a comment

Choose a reason for hiding this comment

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

Edit: noticing above issue on dev green as well so unrelated to these changes 🚀

@thecristen
Copy link
Collaborator Author

@i0sea good looking out -- looks like the 32 does stop there, but under one of the uncommon route variants.

Can also see a couple schedules from this route at that stop here:
image

@thecristen thecristen removed the dev-blue Deploy to dev-blue label Sep 29, 2023
@thecristen thecristen merged commit f0593d4 into master Sep 29, 2023
18 of 20 checks passed
@thecristen thecristen deleted the cbj/stop-page-with-route-patterns branch September 29, 2023 14:31
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.

2 participants