Skip to content

Commit

Permalink
fix: apply filters to alerts cache correctly
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sloanelybutsurely committed Aug 19, 2024
1 parent 6f88363 commit 7de8900
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 4 deletions.
41 changes: 37 additions & 4 deletions lib/screens/alerts/cache/filter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
117 changes: 117 additions & 0 deletions test/screens/alerts/cache/filter_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ defmodule Screens.Alerts.Cache.FilterTest do

import Mox

alias Screens.Alerts.Alert
alias Screens.Alerts.Cache.Filter
alias Screens.Routes.Route

Expand Down Expand Up @@ -66,4 +67,120 @@ defmodule Screens.Alerts.Cache.FilterTest do
] = Filter.build_matchers(%{routes: ["Blue", "Green-E"]})
end
end

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("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 "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 "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 "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 [%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 "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 [%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 "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 [%Alert{id: "route in one direction"}] =
Filter.filter_by(alerts, %{routes: ["route-in-direction"], direction_id: 0})

assert [%Alert{id: "route in the other direction"}] =
Filter.filter_by(alerts, %{routes: ["route-in-direction"], direction_id: 1})
end

test "filters on activities", %{alerts: alerts} do
stub(Route.Mock, :by_id, fn
"route-only" -> {:ok, %Route{id: "route-only", type: :light_rail}}
"route-only-3" -> {:ok, %Route{id: "route-only-3", type: :light_rail}}
end)

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

0 comments on commit 7de8900

Please sign in to comment.