-
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
fix(SiteWeb.StopController): don't show route patterns for services that aren't running today #1762
Conversation
d8ca79b
to
b3e0b3b
Compare
b3e0b3b
to
35f272f
Compare
and parse it from the trip's relationships
semi-adapted from the V3 API's approach
- remove route patterns associated to a service that is not running today - except when it's a canonical service... let that route pattern through anyway
35f272f
to
d632823
Compare
"service" => [ | ||
%Item{ | ||
id: service_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.
This arraying of relationships is exactly what is happening in the line
parsing I am working on. Should we keep this behavior around?
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.
Yeah, I see what you mean.
Should we keep this behavior around?
Changing this (via removing list()
in the relevant part of the %JsonApi.Item{}
struct here) would require a baffling amount of downstream changes in all of our V3 API parsing. I don't see the benefit in changing it.
apps/services/lib/service.ex
Outdated
@spec parse_listed_dates([String.t()]) :: [NaiveDateTime.t()] | ||
defp parse_listed_dates(date_strings) do | ||
date_strings | ||
|> Enum.map(&Timex.parse(&1, "{YYYY}-{0M}-{D}")) |
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.
question: Does the {D}
need the leading zero?
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.
..yes. I will update
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.
Some minor questions but it looks good
Instead of using all route patterns from the whole rating, this PR adjusts the stop page's backend response for route patterns to remove those associated with services that aren't applicable today.
On the stop page, this removes headsigns associated with route patterns for this weekend's shuttle service, later disruptions, and so on.
General checks