Skip to content

Commit

Permalink
fix: refactor of routes-serving-stop fetching
Browse files Browse the repository at this point in the history
The version of this function prior to the refactor in 297c302 returned
only a list of routes, returning an empty list if an error occurred.
The moved/renamed version returns an `:ok` tuple or `:error`, consistent
with other data fetching functions, but the call sites were not updated
to reflect this, and no tests failed because the mocks for the function
still used the old return type (or, in the case of `ElevatorClosures`,
the module lacked any tests at all).

This keeps the return type of the fetching function the same, so errors
can be distinguished if needed, but changes the existing call sites to
perform the "default to empty list" logic themselves, so there should be
no change in behavior.
  • Loading branch information
digitalcora committed Aug 1, 2024
1 parent cf3e212 commit f6a1805
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 9 deletions.
2 changes: 1 addition & 1 deletion lib/screens/routes/route.ex
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ defmodule Screens.Routes.Route do
end

@doc "Fetches routes that serve the given stop."
@spec serving_stop(Stop.id()) :: {:ok, t()} | :error
@spec serving_stop(Stop.id()) :: {:ok, [t()]} | :error
def serving_stop(
stop_id,
get_json_fn \\ &V3Api.get_json/2,
Expand Down
6 changes: 5 additions & 1 deletion lib/screens/v2/candidate_generator/dup/departures.ex
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,11 @@ defmodule Screens.V2.CandidateGenerator.Dup.Departures do
) do
routes =
stop_ids
|> Enum.flat_map(&fetch_routes_serving_stop_fn.(&1))
|> Enum.map(&fetch_routes_serving_stop_fn.(&1))
|> Enum.flat_map(fn
{:ok, routes} -> routes
:error -> []
end)
|> Enum.uniq()

if route_ids == [] do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ defmodule Screens.V2.CandidateGenerator.Widgets.ElevatorClosures do
|> MapSet.new()
|> MapSet.put(home_parent_station_id)
|> Enum.map(fn station_id ->
{station_id, station_id |> Route.serving_stop() |> routes_to_icons()}
{station_id, station_id |> routes_serving_stop() |> routes_to_icons()}
end)
|> Enum.into(%{})
end
Expand All @@ -66,6 +66,13 @@ defmodule Screens.V2.CandidateGenerator.Widgets.ElevatorClosures do
end)
end

defp routes_serving_stop(stop_id) do
case Route.serving_stop(stop_id) do
{:ok, routes} -> routes
:error -> []
end
end

defp routes_to_icons(routes) do
routes
|> Enum.map(fn
Expand Down
12 changes: 6 additions & 6 deletions test/screens/v2/candidate_generator/dup/departures_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,12 @@ defmodule Screens.V2.CandidateGenerator.Dup.DeparturesTest do
fetch_vehicles_fn = fn _, _ -> [struct(Vehicle)] end

fetch_routes_serving_stop_fn = fn
"Boat" -> [%{id: "Ferry", type: :ferry}]
"place-A" -> [%{id: "Orange", type: :subway}, %{id: "Green", type: :light_rail}]
"bus-A" -> [%{id: "Bus A", type: :bus}]
"bus-B" -> [%{id: "Bus B", type: :bus}]
"place-overnight" -> [%{id: "Red", type: :subway}]
_ -> [%{id: "test", type: :test}]
"Boat" -> {:ok, [%{id: "Ferry", type: :ferry}]}
"place-A" -> {:ok, [%{id: "Orange", type: :subway}, %{id: "Green", type: :light_rail}]}
"bus-A" -> {:ok, [%{id: "Bus A", type: :bus}]}
"bus-B" -> {:ok, [%{id: "Bus B", type: :bus}]}
"place-overnight" -> {:ok, [%{id: "Red", type: :subway}]}
_ -> {:ok, [%{id: "test", type: :test}]}
end

%{
Expand Down

0 comments on commit f6a1805

Please sign in to comment.