-
Notifications
You must be signed in to change notification settings - Fork 2
CR Schedule View: Display basic timetable #1340
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
base: master
Are you sure you want to change the base?
Conversation
0f40d33 to
d165d90
Compare
|
Re: changes to add the link from the view disruption page - we don't really have any existing tests as far as I can tell for the display of the view trainsformer export details section, so I'm not sure how to best test my change. Suggestions welcome. |
f24a841 to
7a617bd
Compare
| </th> | ||
| </tr> | ||
| <% end %> | ||
| </thead> |
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.
nice! I always forget the thead / tbody tags
| assert response =~ "SPRING2025-SOUTHSS-Weekend-66" | ||
| assert response =~ "CR-Foxboro" | ||
| assert response =~ "Back Bay" | ||
| assert response =~ "10:50:00" |
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.
could you also test clicking the route / direction here, or would that be better in an integration test? would it make more sense to test an export with more than one route?
| Enum.reduce(all_schedules, [], fn trip_data, stop_ids -> | ||
| unseen_stop_ids = | ||
| trip_data.stop_times | ||
| |> Enum.map(& &1.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.
You could do this in a single pass of stop_times with Enum.reduce. Also, would it be worth making stop IDs ito a MapSet here?
| all_schedules = | ||
| schedule_data | ||
| |> Map.get(service_id) | ||
| |> Enum.map(fn {_trip_id, trip_data} -> trip_data end) |
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.
This map and reduce could also be combined into a single pass. I'm not sure if mix and/or BEAM do this by default, in which case it's just a matter of taste
| train_numbers = Enum.map(all_schedules, & &1.short_name) | ||
|
|
||
| # Combine stop names and stop times into rows | ||
| stop_times_by_stop = |
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.
could you utilize Enum.group_by/3 here?
Summary of changes
Asana Ticket: part of 🏹🚆 Schedule View
This change handles the basics of rendering the schedule into a tabular timetable view, while also allowing toggling between different routes and directions within the same export. The page is still not linked directly from the Trainsformer export section, but you can manually navigate to
/trainsformer_exports/[export ID]/timetablein your browser to look at the rendered result.Note that I am aware that the ordering of the stops when not all trips have the same stopping pattern isn't great. I think improving that will be for a followup PR / subtask.
Reviewer Checklist