fix: apply filters to alerts cache correctly (#2145)

The alerts cache was missing a step that's present in the V3 API which
meant that filters were not being applied to the list of all alerts
correctly producing the wrong results.

This change adds a test suite to verify that filters work as expected
and adds the additional step of expanding built matchers so partial
matches are considered a match.

lib/screens/alerts/cache/filter.ex

@@ -56,7 +56,7 @@ defmodule Screens.Alerts.Cache.Filter do
     end
   end
 
-  def build_matchers(filter_opts) do
+  defp build_matchers(filter_opts) do
     filter_opts
     |> Enum.reduce([%{}], &build_matcher/2)
     |> reject_empty_matchers()
@@ -145,11 +145,44 @@ defmodule Screens.Alerts.Cache.Filter do
     |> Enum.any?(&matches?(alert, &1))
   end
 
-  defp matches?(alert, matcher) when is_map(matcher) do
-    Enum.all?(matcher, &matches?(alert, &1))
+  defp matches?(%{informed_entities: informed_entities}, matcher) when is_map(matcher) do
+    informed_entities
+    |> Enum.any?(fn ie ->
+      matcher
+      |> expand_matcher()
+      |> Enum.any?(fn matcher ->
+        ie
+        |> Map.intersect(matcher, &values_equal?/3)
+        |> Map.values()
+        |> Enum.all?()
+      end)
+    end)
+  end
+
+  defp expand_matcher(matcher) do
+    for route_type <- part_values(matcher, :route_type),
+        route <- part_values(matcher, :route),
+        stop <- part_values(matcher, :stop),
+        direction_id <- part_values(matcher, :direction_id) do
+      %{route_type: route_type, route: route, stop: stop, direction_id: direction_id}
+    end
+    |> Enum.map(fn expanded_matcher ->
+      Map.reject(expanded_matcher, &match?({_, :_}, &1))
+    end)
+    |> Enum.reject(fn expanded_matcher ->
+      expanded_matcher
+      |> Map.values()
+      |> Enum.all?(&is_nil/1)
+    end)
   end
 
-  defp matches?(alert, {key, value}) do
-    Enum.any?(alert.informed_entities, &(Map.get(&1, key) == value))
+  defp part_values(map, key) do
+    case Map.fetch(map, key) do
+      {:ok, nil} -> [nil]
+      {:ok, value} -> [value, nil]
+      :error -> [:_]
+    end
   end
+
+  defp values_equal?(_key, a, b), do: a == b
 end

test/screens/alerts/cache/filter_test.exs

@@ -3,69 +3,126 @@ defmodule Screens.Alerts.Cache.FilterTest do
 
   import Mox
 
+  alias Screens.Alerts.Alert
   alias Screens.Alerts.Cache.Filter
   alias Screens.Routes.Route
 
   setup :verify_on_exit!
 
-  describe "build_matchers/1" do
-    test "passes through empty filters" do
-      assert [] == Filter.build_matchers(%{})
+  describe "filter_by/2" do
+    setup do
+      alerts = [
+        alert("stop only", ie(stop: "stop-only")),
+        alert("route serving stop", ie(route: "serving-stop")),
+        alert("route only", ie(route: "route-only", route_type: 0)),
+        alert("stop and route", ie(stop: "stop-and-route-stop", route: "stop-and-route-route")),
+        alert("route only 2", ie(route: "route-only-2", route_type: 1)),
+        alert("route type only", ie(route_type: 0)),
+        alert(
+          "route in one direction",
+          ie(route: "route-in-direction", direction_id: 0, route_type: 3)
+        ),
+        alert(
+          "route in the other direction",
+          ie(route: "route-in-direction", direction_id: 1, route_type: 3)
+        ),
+        alert("route in either direction", ie(route: "route-in-direction", route_type: 3)),
+        alert("other activities 1", ie(route: "route-only", activities: ~w[USING_WHEELCHAIR])),
+        alert("other activities 2", ie(route: "route-only-3", activities: ~w[USING_WHEELCHAIR]))
+      ]
+
+      %{alerts: alerts}
     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})
+    test "filters for stop, including alerts impacting routes serving the stop", %{alerts: alerts} do
+      stub(Route.Mock, :serving_stop, fn _ -> {:ok, [%Route{id: "serving-stop"}]} end)
+
+      assert [%Alert{id: "stop only"}, %Alert{id: "route serving stop"}] =
+               Filter.filter_by(alerts, %{stops: ["stop-only"]})
     end
 
-    test "adds matchers for route_types" do
-      assert [%{route_type: 1}, %{route_type: 2}] == Filter.build_matchers(%{route_types: [1, 2]})
+    test "filters for route, including alerts impacting the entire route type", %{alerts: alerts} do
+      stub(Route.Mock, :by_id, fn
+        "route-only" -> {:ok, %Route{id: "route-only", type: :light_rail}}
+      end)
+
+      assert [%Alert{id: "route only"}, %Alert{id: "route type only"}] =
+               Filter.filter_by(alerts, %{routes: ["route-only"]})
     end
 
-    test "adds matchers for stops" do
-      stub(Route.Mock, :serving_stop, fn _ -> {:ok, []} end)
+    test "filters for stop and route combined", %{alerts: alerts} do
+      stub(Route.Mock, :by_id, fn
+        "stop-and-route-route" -> {:ok, %Route{id: "stop-and-route-route", type: :subway}}
+        "route-only-2" -> {:ok, %Route{id: "route-only", type: :subway}}
+      end)
+
+      stub(Route.Mock, :serving_stop, fn "stop-and-route-stop" ->
+        {:ok, [%Route{id: "stop-and-route-route"}]}
+      end)
 
-      assert [
-               %{stop: "place-pktrm"},
-               %{stop: "place-aport"}
-             ] = Filter.build_matchers(%{stops: ["place-pktrm", "place-aport"]})
+      assert [%Alert{id: "stop and route"}, %Alert{id: "route only 2"}] =
+               Filter.filter_by(alerts, %{
+                 routes: ["stop-and-route-route", "route-only-2"],
+                 stops: ["stop-and-route-stop"]
+               })
     end
 
-    test "merges stop matchers into other matchers" do
-      stub(Route.Mock, :serving_stop, fn _ -> {:ok, []} end)
+    test "filters for route_type", %{alerts: alerts} do
+      assert [%Alert{id: "route only"}, %Alert{id: "route type only"}] =
+               Filter.filter_by(alerts, %{route_types: [0]})
 
-      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"]})
+      assert [%Alert{id: "route only 2"}] = Filter.filter_by(alerts, %{route_types: [1]})
+
+      assert [%Alert{id: "route only"}, %Alert{id: "route only 2"}, %Alert{id: "route type only"}] =
+               Filter.filter_by(alerts, %{route_types: [0, 1]})
     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"}]}
+    test "filters for a specific direction_id", %{alerts: alerts} do
+      stub(Route.Mock, :by_id, fn
+        "route-in-direction" -> {:ok, %Route{id: "route-in-direction", type: :bus}}
       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"]})
+      assert [%Alert{id: "route in one direction"}, %Alert{id: "route in either direction"}] =
+               Filter.filter_by(alerts, %{routes: ["route-in-direction"], direction_id: 0})
+
+      assert [%Alert{id: "route in the other direction"}, %Alert{id: "route in either direction"}] =
+               Filter.filter_by(alerts, %{routes: ["route-in-direction"], direction_id: 1})
     end
 
-    test "expands route filters to include route types" do
+    test "filters on activities", %{alerts: alerts} do
       stub(Route.Mock, :by_id, fn
-        "Blue" -> {:ok, %Route{id: "Blue", type: :subway}}
-        "Green-E" -> {:ok, %Route{id: "Green-E", type: :light_rail}}
+        "route-only" -> {:ok, %Route{id: "route-only", type: :light_rail}}
+        "route-only-3" -> {:ok, %Route{id: "route-only-3", type: :light_rail}}
       end)
 
-      assert [
-               %{route: "Blue", route_type: 1},
-               %{route: "Green-E", route_type: 0}
-             ] = Filter.build_matchers(%{routes: ["Blue", "Green-E"]})
+      assert [%Alert{id: "other activities 1"}, %Alert{id: "other activities 2"}] =
+               Filter.filter_by(alerts, %{activities: ~w[USING_WHEELCHAIR]})
+
+      assert [%Alert{id: "other activities 1"}] =
+               Filter.filter_by(alerts, %{
+                 routes: ["route-only"],
+                 activities: ~w[USING_WHEELCHAIR]
+               })
+
+      assert [%Alert{id: "other activities 2"}] =
+               Filter.filter_by(alerts, %{
+                 routes: ["route-only-3"],
+                 activities: ~w[USING_WHEELCHAIR]
+               })
     end
   end
+
+  defp alert(id, ies) do
+    %Alert{id: id, informed_entities: List.wrap(ies)}
+  end
+
+  defp ie(opts) do
+    %{
+      stop: opts[:stop],
+      route: opts[:route],
+      route_type: opts[:route_type],
+      activities: opts[:activities] || ~w[BOARD EXIT RIDE],
+      direction_id: opts[:direction_id]
+    }
+  end
 end