From f53e95206e6eed4aecf22831867acb5779569a64 Mon Sep 17 00:00:00 2001 From: sloane Date: Mon, 12 Aug 2024 14:54:08 -0400 Subject: [PATCH] feat: Expand stops filters to include routes at stops Adds `fetch_routes_for_stops/1` and uses that to expand a `stops:` filter similar to the V3 API. --- config/test.exs | 1 + lib/screens/alerts/cache/filter.ex | 61 +++++++++++++++++------ lib/screens/routes/route.ex | 2 +- test/screens/alerts/alert_test.exs | 10 +++- test/screens/alerts/cache/filter_test.exs | 57 +++++++++++++++++++++ test/support/mocks.ex | 1 + 6 files changed, 115 insertions(+), 17 deletions(-) create mode 100644 test/screens/alerts/cache/filter_test.exs create mode 100644 test/support/mocks.ex diff --git a/config/test.exs b/config/test.exs index 2c6e599b3..edd76db44 100644 --- a/config/test.exs +++ b/config/test.exs @@ -26,6 +26,7 @@ config :screens, blue_bikes_station_information_url: [:no_api_requests_allowed_during_testing], blue_bikes_station_status_url: [:no_api_requests_allowed_during_testing], blue_bikes_api_client: Screens.BlueBikes.FakeClient, + alerts_cache_filter_route_mod: Screens.Routes.Route.Mock, dup_headsign_replacements: %{ "Test 1" => "T1" }, diff --git a/lib/screens/alerts/cache/filter.ex b/lib/screens/alerts/cache/filter.ex index 6a5cbff56..8f8ddff73 100644 --- a/lib/screens/alerts/cache/filter.ex +++ b/lib/screens/alerts/cache/filter.ex @@ -2,6 +2,13 @@ defmodule Screens.Alerts.Cache.Filter do @moduledoc """ Logic to apply filters to a list of `Screens.Alerts.Alert` structs. """ + + @route_mod Application.compile_env( + :screens, + :alerts_cache_filter_route_mod, + Screens.Routes.Route + ) + @default_activities ~w[BOARD EXIT RIDE] @type filter_opts() :: %{ @@ -16,20 +23,22 @@ defmodule Screens.Alerts.Cache.Filter do def filter_by(alerts, filter_opts) when filter_opts == %{}, do: alerts def filter_by(alerts, filter_opts) do - filter_opts = Map.put_new(filter_opts, :activities, @default_activities) + {activities, filter_opts} = Map.pop(filter_opts, :activities, @default_activities) alerts |> filter(filter_opts) - |> filter_by_informed_entity_activity(filter_opts) + |> filter_by_informed_entity_activity(activities) end + defp filter(alerts, filter_opts) when filter_opts == %{}, do: alerts + defp filter(alerts, filter_opts) do filter_opts |> build_matchers() |> apply_matchers(alerts) end - defp filter_by_informed_entity_activity(alerts, %{activities: values}) do + defp filter_by_informed_entity_activity(alerts, values) do values = MapSet.new(values) if MapSet.member?(values, "ALL") do @@ -49,15 +58,20 @@ defmodule Screens.Alerts.Cache.Filter do end end - defp filter_by_informed_entity_activity(alerts, filter_opts) do - filter_opts = Map.put(filter_opts, :activities, @default_activities) - - filter_by_informed_entity_activity(alerts, filter_opts) - end - - defp build_matchers(filter_opts) do + def build_matchers(filter_opts) do filter_opts |> Enum.reduce([%{}], &build_matcher/2) + |> reject_empty_matchers() + |> Enum.uniq() + end + + defp reject_empty_matchers(matchers) do + matchers + |> Enum.reject(fn matcher -> + matcher + |> Map.values() + |> Enum.all?(&is_nil/1) + end) end defp apply_matchers(matchers, alerts) do @@ -78,12 +92,29 @@ defmodule Screens.Alerts.Cache.Filter do end defp build_matcher({:stops, values}, acc) when is_list(values) do - matchers_for_values(acc, :stop, values) - end + routes = + values + |> Enum.flat_map(fn stop_id -> + {:ok, routes} = @route_mod.serving_stop(stop_id) + routes + end) + + route_matchers = + for %{id: route_id} <- routes, + stop_id <- [nil | values] do + %{route: route_id, stop: stop_id} + end - defp build_matcher({:activities, values}, acc) when is_list(values) do - # activities are filtered later, no need to add matchers - acc + stop_matchers = + for stop_id <- [nil | values] do + %{stop: stop_id} + end + + for matcher_list <- [route_matchers, stop_matchers], + merge <- matcher_list, + matcher <- acc do + Map.merge(matcher, merge) + end end defp matchers_for_values(acc, key, values) do diff --git a/lib/screens/routes/route.ex b/lib/screens/routes/route.ex index 93b2111a0..c61bdad58 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 + @callback serving_stop(Stop.id()) :: {:ok, [t()]} | :error def serving_stop( stop_id, get_json_fn \\ &V3Api.get_json/2, diff --git a/test/screens/alerts/alert_test.exs b/test/screens/alerts/alert_test.exs index ac77a98db..ba702fecf 100644 --- a/test/screens/alerts/alert_test.exs +++ b/test/screens/alerts/alert_test.exs @@ -1,7 +1,10 @@ defmodule Screens.Alerts.AlertTest do use ExUnit.Case, async: true + import Mox + alias Screens.Alerts.Alert + alias Screens.Routes.Route defp alert_json(id) do %{ @@ -178,11 +181,16 @@ defmodule Screens.Alerts.AlertTest do [alerts: alerts, get_all_alerts: fn -> alerts end] end - test "returns all of the alerts", %{alerts: alerts, get_all_alerts: get_all_alerts} do + test "returns all of the alerts matching the default activities", %{ + alerts: alerts, + get_all_alerts: get_all_alerts + } do assert {:ok, alerts} == Alert.fetch_from_cache([], get_all_alerts) end test "filters by stops", %{get_all_alerts: get_all_alerts} do + stub(Route.Mock, :serving_stop, fn _ -> {:ok, []} end) + assert {:ok, [%Alert{id: "stop: A" <> _}]} = Alert.fetch_from_cache([stop_id: "A"], get_all_alerts) diff --git a/test/screens/alerts/cache/filter_test.exs b/test/screens/alerts/cache/filter_test.exs new file mode 100644 index 000000000..1f9e75208 --- /dev/null +++ b/test/screens/alerts/cache/filter_test.exs @@ -0,0 +1,57 @@ +defmodule Screens.Alerts.Cache.FilterTest do + use ExUnit.Case, async: true + + import Mox + + alias Screens.Alerts.Cache.Filter + alias Screens.Routes.Route + + describe "build_matchers/1" do + test "passes through empty filters" do + assert [] == Filter.build_matchers(%{}) + end + + test "adds matchers for direction_id" do + assert [%{direction_id: 0}] == Filter.build_matchers(%{direction_id: 0}) + assert [%{direction_id: 1}] == Filter.build_matchers(%{direction_id: 1}) + end + + test "adds matchers for route_types" do + assert [%{route_type: 1}, %{route_type: 2}] == Filter.build_matchers(%{route_types: [1, 2]}) + end + + test "adds matchers for stops" do + stub(Route.Mock, :serving_stop, fn _ -> {:ok, []} end) + + assert [ + %{stop: "place-pktrm"}, + %{stop: "place-aport"} + ] = Filter.build_matchers(%{stops: ["place-pktrm", "place-aport"]}) + end + + test "merges stop matchers into other matchers" do + stub(Route.Mock, :serving_stop, fn _ -> {:ok, []} end) + + assert [ + %{stop: nil, direction_id: 0}, + %{stop: "place-pktrm", direction_id: 0}, + %{stop: "place-aport", direction_id: 0} + ] == Filter.build_matchers(%{direction_id: 0, stops: ["place-pktrm", "place-aport"]}) + end + + test "adds matchers for routes at the stops" do + stub(Route.Mock, :serving_stop, fn + "place-aport" -> + {:ok, [%Route{id: "Blue"}, %Route{id: "743"}]} + end) + + assert [ + %{stop: nil, route: "Blue"}, + %{stop: "place-aport", route: "Blue"}, + %{stop: nil, route: "743"}, + %{stop: "place-aport", route: "743"}, + %{stop: "place-aport"} + ] == Filter.build_matchers(%{stops: ["place-aport"]}) + end + end +end diff --git a/test/support/mocks.ex b/test/support/mocks.ex new file mode 100644 index 000000000..552fa1c40 --- /dev/null +++ b/test/support/mocks.ex @@ -0,0 +1 @@ +Mox.defmock(Screens.Routes.Route.Mock, for: Screens.Routes.Route)