From 08cc1249bbfbe137dd2592ad3e2058f240116c01 Mon Sep 17 00:00:00 2001 From: Cora Grant <cgrant2@mbta.com> Date: Thu, 1 Aug 2024 10:14:13 -0400 Subject: [PATCH] fix: refactor of routes-serving-stop fetching 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. --- lib/screens/routes/route.ex | 2 +- lib/screens/v2/candidate_generator/dup/departures.ex | 6 +++++- .../candidate_generator/widgets/elevator_closures.ex | 9 ++++++++- .../v2/candidate_generator/dup/departures_test.exs | 12 ++++++------ 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/screens/routes/route.ex b/lib/screens/routes/route.ex index c366bfa6b..93b2111a0 100644 --- a/lib/screens/routes/route.ex +++ b/lib/screens/routes/route.ex @@ -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, diff --git a/lib/screens/v2/candidate_generator/dup/departures.ex b/lib/screens/v2/candidate_generator/dup/departures.ex index 0c8f8584c..1ae62cf89 100644 --- a/lib/screens/v2/candidate_generator/dup/departures.ex +++ b/lib/screens/v2/candidate_generator/dup/departures.ex @@ -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 diff --git a/lib/screens/v2/candidate_generator/widgets/elevator_closures.ex b/lib/screens/v2/candidate_generator/widgets/elevator_closures.ex index b7a53b2c3..354de3fde 100644 --- a/lib/screens/v2/candidate_generator/widgets/elevator_closures.ex +++ b/lib/screens/v2/candidate_generator/widgets/elevator_closures.ex @@ -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 @@ -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 diff --git a/test/screens/v2/candidate_generator/dup/departures_test.exs b/test/screens/v2/candidate_generator/dup/departures_test.exs index 8ed1f3832..a0ba24157 100644 --- a/test/screens/v2/candidate_generator/dup/departures_test.exs +++ b/test/screens/v2/candidate_generator/dup/departures_test.exs @@ -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 %{