From 50e6de42fd10abfb397b8af7d4e89f24b34c64e4 Mon Sep 17 00:00:00 2001 From: Jon Zimbel Date: Fri, 6 Mar 2026 14:06:48 -0500 Subject: [PATCH 1/5] test: Only merge activities on computed parent station informed entities, not original child stop i.e.s --- apps/state/test/state/alert/hooks_test.exs | 558 +++++++++++++++++++-- 1 file changed, 530 insertions(+), 28 deletions(-) diff --git a/apps/state/test/state/alert/hooks_test.exs b/apps/state/test/state/alert/hooks_test.exs index 456b593b3..8ed418112 100644 --- a/apps/state/test/state/alert/hooks_test.exs +++ b/apps/state/test/state/alert/hooks_test.exs @@ -1,34 +1,536 @@ defmodule State.Alert.HooksTest do @moduledoc false - use ExUnit.Case, async: true + use ExUnit.Case import State.Alert.Hooks + alias Model.Alert - @alert %Model.Alert{ - id: "alert1", - informed_entity: [ - %{ - route_type: 3, - route: "1", - direction_id: 0, - stop: "place-cool", - activities: ["BOARD", "RIDE"] - }, - %{ - route_type: 3, - route: "1", - direction_id: 0, - stop: "place-cool", - activities: ["EXIT"] - } - ], - severity: 1 - } - - test "pre_insert_hook/1 merges informed entities" do - [alert] = pre_insert_hook(@alert) - ie = Map.get(alert, :informed_entity) - assert is_list(ie) - assert length(ie) == 1 - assert length(ie |> hd |> Map.get(:activities)) == 3 + setup do + State.Stop.new_state([]) + State.Trip.new_state([]) + State.Route.new_state([]) + :ok + end + + @spec get_computed_informed_entities([Alert.informed_entity()]) :: + %{ + added: [Alert.informed_entity()], + removed: [Alert.informed_entity()], + preserved: [Alert.informed_entity()] + } + defp get_computed_informed_entities(informed_entities) when is_list(informed_entities) do + get_computed_informed_entities(%Alert{id: "alert1", informed_entity: informed_entities}) + end + + @spec get_computed_informed_entities(Alert.t()) :: %{ + added: [Alert.informed_entity()], + removed: [Alert.informed_entity()], + preserved: [Alert.informed_entity()] + } + defp get_computed_informed_entities(%Alert{} = alert) do + informed_entities = alert.informed_entity + + assert [%Alert{informed_entity: new_informed_entities}] = pre_insert_hook(alert) + + new_informed_entities = MapSet.new(new_informed_entities) + informed_entities = MapSet.new(informed_entities) + + [ + added: MapSet.difference(new_informed_entities, informed_entities), + removed: MapSet.difference(informed_entities, new_informed_entities), + preserved: MapSet.intersection(new_informed_entities, informed_entities) + ] + |> Map.new(fn {k, ies} -> {k, normalize(ies)} end) + end + + defp normalize(ies) do + ies + |> Enum.map(fn ie -> Map.replace_lazy(ie, :activities, &Enum.sort/1) end) + |> Enum.sort() + end + + describe "pre_insert_hook/1" do + test "adds informed entities for parent stations" do + State.Stop.new_state([ + %Model.Stop{id: "child-stop1", parent_station: "parent-stationA"}, + %Model.Stop{id: "child-stop2", parent_station: "parent-stationB"}, + %Model.Stop{id: "child-stop3", parent_station: "parent-stationB"}, + %Model.Stop{id: "parentless-stop"} + ]) + + informed_entities = + [ + %{stop: "child-stop1"}, + %{stop: "child-stop2"}, + %{stop: "child-stop3"}, + %{stop: "parentless-stop"} + ] + |> normalize() + + assert %{ + preserved: ^informed_entities, + added: [%{stop: "parent-stationA"}, %{stop: "parent-stationB"}] + } = get_computed_informed_entities(informed_entities) + end + + test "merges child informed entities' activities for parent station informed entities," <> + " and does *not* merge activities for pre-existing child stop informed entities" do + State.Stop.new_state([ + %Model.Stop{id: "child-stop1", parent_station: "parent-stationA"}, + %Model.Stop{id: "child-stop2", parent_station: "parent-stationA"}, + %Model.Stop{id: "child-stop3", parent_station: "parent-stationB"}, + %Model.Stop{id: "child-stop4", parent_station: "parent-stationB"}, + %Model.Stop{id: "child-stop5", parent_station: "parent-stationC"}, + %Model.Stop{id: "parentless-stop"} + ]) + + informed_entities = + [ + %{stop: "child-stop1", activities: ["BOARD", "EXIT", "USING_WHEELCHAIR"]}, + %{stop: "child-stop2", activities: ["BOARD", "EXIT", "RIDE"]}, + %{stop: "child-stop3", activities: ["BOARD"], route: "route1", trip: "trip1"}, + %{stop: "child-stop4", activities: ["EXIT"], route: "route1", trip: "trip1"}, + %{stop: "child-stop4", activities: ["RIDE"], route: "route1", trip: "trip2"}, + %{stop: "child-stop4", activities: ["RIDE"], route: "route2", trip: "trip1"}, + %{stop: "child-stop5", activities: ["USING_ESCALATOR"]}, + %{stop: "parentless-stop", activities: ["BRINGING_BIKE", "STORE_BIKE"]} + ] + |> normalize() + + assert %{ + preserved: ^informed_entities, + added: [ + %{ + stop: "parent-stationA", + activities: ["BOARD", "EXIT", "RIDE", "USING_WHEELCHAIR"] + }, + %{stop: "parent-stationC", activities: ["USING_ESCALATOR"]}, + %{ + stop: "parent-stationB", + activities: ["BOARD", "EXIT"], + route: "route1", + trip: "trip1" + }, + %{stop: "parent-stationB", activities: ["RIDE"], route: "route1", trip: "trip2"}, + %{stop: "parent-stationB", activities: ["RIDE"], route: "route2", trip: "trip1"} + ] + } = get_computed_informed_entities(informed_entities) + end + + test "adds informed entities for alternate trips" do + State.Trip.new_state([ + %Model.Trip{id: "trip1", alternate_route: false, route_id: "main-route"}, + %Model.Trip{id: "trip1", alternate_route: true, route_id: "alt-route1"}, + %Model.Trip{id: "trip1", alternate_route: true, route_id: "alt-route2"}, + %Model.Trip{id: "trip1", alternate_route: true, route_id: "alt-routeX", direction_id: 1} + ]) + + State.Route.new_state([ + %Model.Route{id: "main-route", type: 3}, + %Model.Route{id: "alt-route1", type: 3}, + %Model.Route{id: "alt-route2", type: 3} + ]) + + informed_entities = + [ + %{ + stop: "bus-stop1", + trip: "trip1", + route: "main-route", + direction_id: 0, + activities: ["BOARD", "EXIT"] + }, + %{ + stop: "bus-stop2", + trip: "trip1", + route: "a-different-route", + direction_id: 0, + activities: ["BOARD"] + }, + %{stop: "bus-stop3", trip: "trip2", direction_id: 1} + ] + |> normalize() + + assert %{ + preserved: [ + %{stop: "bus-stop3", trip: "trip2", direction_id: 1}, + %{ + stop: "bus-stop2", + trip: "trip1", + route: "a-different-route", + direction_id: 0, + activities: ["BOARD"] + } + ], + added: [ + %{ + stop: "bus-stop2", + trip: "trip1", + route: "alt-routeX", + direction_id: 1, + activities: ["BOARD"] + }, + %{ + stop: "bus-stop1", + trip: "trip1", + route: "alt-routeX", + direction_id: 1, + activities: ["BOARD", "EXIT"] + }, + %{ + stop: "bus-stop2", + route: "alt-route1", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD"] + }, + %{ + stop: "bus-stop2", + route: "alt-route2", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD"] + }, + %{ + stop: "bus-stop2", + route: "main-route", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD"] + }, + %{ + stop: "bus-stop1", + route: "alt-route1", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD", "EXIT"] + }, + %{ + stop: "bus-stop1", + route: "alt-route2", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD", "EXIT"] + }, + %{ + stop: "bus-stop1", + route: "main-route", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD", "EXIT"] + } + ], + removed: [ + %{ + stop: "bus-stop1", + trip: "trip1", + route: "main-route", + direction_id: 0, + activities: ["BOARD", "EXIT"] + } + ] + } = get_computed_informed_entities(informed_entities) + end + + test "adds Cartesian product of computed informed entities for parent stations X alternate trips" do + State.Stop.new_state([ + %Model.Stop{id: "busway-berth1", parent_station: "parent-stationA"}, + %Model.Stop{id: "busway-berth2", parent_station: "parent-stationA"} + ]) + + State.Trip.new_state([ + %Model.Trip{id: "trip1", alternate_route: false, route_id: "main-route"}, + %Model.Trip{id: "trip1", alternate_route: true, route_id: "alt-route"} + ]) + + State.Route.new_state([ + %Model.Route{id: "main-route", type: 3}, + %Model.Route{id: "alt-route", type: 3} + ]) + + informed_entities = + [ + %{ + stop: "busway-berth1", + trip: "trip1", + route: "main-route", + direction_id: 0, + activities: ["BOARD", "EXIT"] + }, + %{ + stop: "busway-berth2", + trip: "trip1", + route: "main-route", + direction_id: 0, + activities: ["BOARD", "USING_WHEELCHAIR"] + } + ] + |> normalize() + + assert %{ + preserved: [], + added: [ + %{ + stop: "busway-berth1", + route: "alt-route", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD", "EXIT"] + }, + %{ + stop: "busway-berth1", + route: "main-route", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD", "EXIT"] + }, + %{ + stop: "parent-stationA", + route: "alt-route", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD", "EXIT", "USING_WHEELCHAIR"] + }, + %{ + stop: "parent-stationA", + route: "main-route", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD", "EXIT", "USING_WHEELCHAIR"] + }, + %{ + stop: "busway-berth2", + route: "alt-route", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD", "USING_WHEELCHAIR"] + }, + %{ + stop: "busway-berth2", + route: "main-route", + direction_id: nil, + route_type: 3, + trip: "trip1", + activities: ["BOARD", "USING_WHEELCHAIR"] + } + ], + removed: [ + %{ + stop: "busway-berth1", + route: "main-route", + direction_id: 0, + trip: "trip1", + activities: ["BOARD", "EXIT"] + }, + %{ + stop: "busway-berth2", + route: "main-route", + direction_id: 0, + trip: "trip1", + activities: ["BOARD", "USING_WHEELCHAIR"] + } + ] + } = get_computed_informed_entities(informed_entities) + end + + test "handles specific case from bugfix ticket" do + # https://app.asana.com/1/15492006741476/project/584764604969369/task/1213450825693783?focus=true + + State.Stop.new_state([ + %Model.Stop{id: "BNT-0000", parent_station: "place-north"}, + %Model.Stop{id: "WR-0045-S", parent_station: "place-mlmnl"}, + %Model.Stop{id: "WR-0053-S", parent_station: "place-ogmnl"} + ]) + + assert [alert] = Parse.Alerts.parse(alerts_enhanced_json_excerpt()) + + informed_entities = normalize(alert.informed_entity) + + expected_new_informed_entities = + [ + %{ + stop: "place-mlmnl", + activities: ["BOARD", "EXIT", "RIDE"], + route: "CR-Haverhill", + direction_id: 1, + route_type: 2 + }, + %{ + stop: "place-mlmnl", + activities: ["BOARD", "EXIT", "RIDE"], + route: "CR-Haverhill", + direction_id: 0, + route_type: 2 + }, + %{ + stop: "place-north", + activities: ["BOARD", "EXIT", "RIDE"], + route: "CR-Haverhill", + direction_id: 1, + route_type: 2 + }, + %{ + stop: "place-north", + activities: ["BOARD", "EXIT", "RIDE"], + route: "CR-Haverhill", + direction_id: 0, + route_type: 2 + }, + %{ + stop: "place-ogmnl", + activities: ["BOARD", "EXIT", "RIDE"], + route: "CR-Haverhill", + direction_id: 1, + route_type: 2 + }, + %{ + stop: "place-ogmnl", + activities: ["BOARD", "EXIT", "RIDE"], + route: "CR-Haverhill", + direction_id: 0, + route_type: 2 + } + ] + |> normalize() + + assert %{ + preserved: ^informed_entities, + added: ^expected_new_informed_entities + } = get_computed_informed_entities(alert) + end + end + + defp alerts_enhanced_json_excerpt do + ~S""" + { + "entity": [ + { + "id": "1000217", + "alert": { + "cause": "UNKNOWN_CAUSE", + "effect": "REDUCED_SERVICE", + "severity": 7, + "active_period": [ + { + "start": 1772415300, + "end": 1772697600 + } + ], + "duration_certainty": "KNOWN", + "cause_detail": "UNKNOWN_CAUSE", + "effect_detail": "SUSPENSION", + "informed_entity": [ + { + "direction_id": 1, + "stop_id": "BNT-0000", + "route_id": "CR-Haverhill", + "route_type": 2, + "agency_id": "1", + "activities": [ + "EXIT", + "RIDE" + ] + }, + { + "direction_id": 1, + "stop_id": "WR-0045-S", + "route_id": "CR-Haverhill", + "route_type": 2, + "agency_id": "1", + "activities": [ + "BOARD", + "EXIT", + "RIDE" + ] + }, + { + "direction_id": 1, + "stop_id": "WR-0053-S", + "route_id": "CR-Haverhill", + "route_type": 2, + "agency_id": "1", + "activities": [ + "BOARD", + "RIDE" + ] + }, + { + "direction_id": 0, + "stop_id": "BNT-0000", + "route_id": "CR-Haverhill", + "route_type": 2, + "agency_id": "1", + "activities": [ + "BOARD", + "RIDE" + ] + }, + { + "direction_id": 0, + "stop_id": "WR-0045-S", + "route_id": "CR-Haverhill", + "route_type": 2, + "agency_id": "1", + "activities": [ + "BOARD", + "EXIT", + "RIDE" + ] + }, + { + "direction_id": 0, + "stop_id": "WR-0053-S", + "route_id": "CR-Haverhill", + "route_type": 2, + "agency_id": "1", + "activities": [ + "EXIT", + "RIDE" + ] + } + ], + "last_modified_timestamp": 1772415513, + "severity_level": "SEVERE", + "header_text": { + "translation": [ + { + "text": "Haverhill Line: service suspended between North Station and Oak Grove today.", + "language": "en" + } + ] + }, + "description_text": { + "translation": [ + { + "text": "Affected stations:\r\nNorth Station\r\nMalden Center\r\nOak Grove", + "language": "en" + } + ] + }, + "service_effect_text": { + "translation": [ + { + "text": "Suspension of service on Haverhill Line", + "language": "en" + } + ] + }, + "created_timestamp": 1772415356, + "alert_lifecycle": "NEW" + } + } + ] + } + """ end end From c0c0409a7a8ed1545ae2c3881e2656f76566b15c Mon Sep 17 00:00:00 2001 From: Jon Zimbel Date: Fri, 6 Mar 2026 14:07:00 -0500 Subject: [PATCH 2/5] fix: Only merge activities on computed parent station informed entities, not original child stop i.e.s --- apps/state/lib/state/alert/hooks.ex | 98 +++++++++++++++-------------- 1 file changed, 51 insertions(+), 47 deletions(-) diff --git a/apps/state/lib/state/alert/hooks.ex b/apps/state/lib/state/alert/hooks.ex index 017c4c6e5..9a2ffe1aa 100644 --- a/apps/state/lib/state/alert/hooks.ex +++ b/apps/state/lib/state/alert/hooks.ex @@ -6,42 +6,72 @@ defmodule State.Alert.Hooks do @spec pre_insert_hook(Alert.t()) :: [Alert.t()] def pre_insert_hook(alert) do - entities = entities_with_parents(alert.informed_entity) + entities = add_computed_entities(alert.informed_entity) [ %{alert | informed_entity: entities} ] end - defp entities_with_parents(entities) do + defp add_computed_entities(entities) do entities - |> Stream.flat_map(&include_entity_parent_stop/1) - |> Stream.flat_map(&include_entity_alternate_trips/1) - |> Enum.group_by(&get_key/1) - |> Stream.flat_map(&merge_entities/1) + |> Stream.concat(get_parent_station_entities(entities)) + |> Enum.flat_map(&include_alternate_trip_entities/1) |> Enum.uniq() end - defp get_key(%{} = ie), do: Map.take(ie, ~w(route stop trip)a) + @spec get_parent_station_entities([Alert.informed_entity()]) :: [Alert.informed_entity()] + defp get_parent_station_entities(entities) do + entities + |> Stream.map(&get_parent_station_entity/1) + |> Stream.reject(&is_nil/1) + |> Enum.group_by(&get_key/1) + |> Enum.flat_map(&merge_parent_entity_activities/1) + end + + defp get_parent_station_entity(%{stop: stop_id} = entity) when is_binary(stop_id) do + case State.Stop.by_id(stop_id) do + %{parent_station: station} when is_binary(station) -> + %{entity | stop: station} + + _ -> + nil + end + end - defp merge_entities({_key, entities}) do - activities = merge_activities(entities) + defp get_parent_station_entity(_entity) do + nil + end + + defp merge_parent_entity_activities({_key, parent_entities}) do + merged_activities = + parent_entities + |> Enum.map(&MapSet.new(&1[:activities] || [])) + |> Enum.reduce(&MapSet.union/2) + |> Enum.sort() - if MapSet.size(activities) == 0 do - entities + if merged_activities == [] do + parent_entities else - result = MapSet.to_list(activities) - for entity <- entities, do: Map.put(entity, :activities, result) + parent_entities + |> Enum.map(&Map.put(&1, :activities, merged_activities)) + |> Enum.uniq() end end - defp merge_activities(entities) when is_list(entities) do - Enum.reduce(entities, MapSet.new(), fn ie, acc -> - case Map.get(ie, :activities) do - [_ | _] = activities -> MapSet.union(acc, MapSet.new(activities)) - _ -> acc - end - end) + @spec include_alternate_trip_entities(Alert.informed_entity()) :: [Alert.informed_entity()] + defp include_alternate_trip_entities(%{trip: trip_id} = entity) when is_binary(trip_id) do + case all_route_entities(entity) do + [] -> + [entity] + + entities -> + entities + end + end + + defp include_alternate_trip_entities(entity) do + [entity] end defp all_route_entities(%{trip: trip_id} = entity) when is_binary(trip_id) do @@ -77,31 +107,5 @@ defmodule State.Alert.Hooks do end end - defp include_entity_alternate_trips(%{trip: trip_id} = entity) when is_binary(trip_id) do - case all_route_entities(entity) do - [] -> - [entity] - - entities -> - entities - end - end - - defp include_entity_alternate_trips(entity) do - [entity] - end - - defp include_entity_parent_stop(%{stop: stop_id} = entity) when is_binary(stop_id) do - case State.Stop.by_id(stop_id) do - %{parent_station: station} when is_binary(station) -> - [entity, %{entity | stop: station}] - - _ -> - [entity] - end - end - - defp include_entity_parent_stop(entity) do - [entity] - end + defp get_key(%{} = ie), do: Map.take(ie, ~w(route stop trip)a) end From 6aa47f65f2610621027fafa08debf211a0a57c48 Mon Sep 17 00:00:00 2001 From: Jon Zimbel Date: Fri, 6 Mar 2026 14:23:11 -0500 Subject: [PATCH 3/5] tweak(test): Check grouping logic --- apps/state/test/state/alert/hooks_test.exs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/state/test/state/alert/hooks_test.exs b/apps/state/test/state/alert/hooks_test.exs index 8ed418112..cbfc55bc4 100644 --- a/apps/state/test/state/alert/hooks_test.exs +++ b/apps/state/test/state/alert/hooks_test.exs @@ -85,8 +85,13 @@ defmodule State.Alert.HooksTest do informed_entities = [ - %{stop: "child-stop1", activities: ["BOARD", "EXIT", "USING_WHEELCHAIR"]}, - %{stop: "child-stop2", activities: ["BOARD", "EXIT", "RIDE"]}, + %{ + stop: "child-stop1", + route: "route1", + activities: ["BOARD", "EXIT", "USING_WHEELCHAIR"] + }, + %{stop: "child-stop1", route: "route2", activities: ["BOARD"]}, + %{stop: "child-stop2", route: "route1", activities: ["BOARD", "EXIT", "RIDE"]}, %{stop: "child-stop3", activities: ["BOARD"], route: "route1", trip: "trip1"}, %{stop: "child-stop4", activities: ["EXIT"], route: "route1", trip: "trip1"}, %{stop: "child-stop4", activities: ["RIDE"], route: "route1", trip: "trip2"}, @@ -99,11 +104,13 @@ defmodule State.Alert.HooksTest do assert %{ preserved: ^informed_entities, added: [ + %{stop: "parent-stationC", activities: ["USING_ESCALATOR"]}, + %{stop: "parent-stationA", route: "route2", activities: ["BOARD"]}, %{ stop: "parent-stationA", + route: "route1", activities: ["BOARD", "EXIT", "RIDE", "USING_WHEELCHAIR"] }, - %{stop: "parent-stationC", activities: ["USING_ESCALATOR"]}, %{ stop: "parent-stationB", activities: ["BOARD", "EXIT"], From d667db74df528999a6dd10468f333753a347ed17 Mon Sep 17 00:00:00 2001 From: Jon Zimbel Date: Fri, 6 Mar 2026 14:40:35 -0500 Subject: [PATCH 4/5] Clean up test module --- apps/state/test/state/alert/hooks_test.exs | 41 ++++++++++------------ 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/apps/state/test/state/alert/hooks_test.exs b/apps/state/test/state/alert/hooks_test.exs index cbfc55bc4..f9e093e25 100644 --- a/apps/state/test/state/alert/hooks_test.exs +++ b/apps/state/test/state/alert/hooks_test.exs @@ -11,33 +11,28 @@ defmodule State.Alert.HooksTest do :ok end - @spec get_computed_informed_entities([Alert.informed_entity()]) :: - %{ - added: [Alert.informed_entity()], - removed: [Alert.informed_entity()], - preserved: [Alert.informed_entity()] - } - defp get_computed_informed_entities(informed_entities) when is_list(informed_entities) do - get_computed_informed_entities(%Alert{id: "alert1", informed_entity: informed_entities}) - end - - @spec get_computed_informed_entities(Alert.t()) :: %{ + @type hook_result :: %{ added: [Alert.informed_entity()], removed: [Alert.informed_entity()], preserved: [Alert.informed_entity()] } - defp get_computed_informed_entities(%Alert{} = alert) do - informed_entities = alert.informed_entity + @spec apply_hook([Alert.informed_entity()]) :: hook_result + @spec apply_hook(Alert.t()) :: hook_result + defp apply_hook(informed_entities) when is_list(informed_entities) do + apply_hook(%Alert{id: "alert1", informed_entity: informed_entities}) + end + + defp apply_hook(%Alert{} = alert) do assert [%Alert{informed_entity: new_informed_entities}] = pre_insert_hook(alert) - new_informed_entities = MapSet.new(new_informed_entities) - informed_entities = MapSet.new(informed_entities) + old = MapSet.new(alert.informed_entity) + new = MapSet.new(new_informed_entities) [ - added: MapSet.difference(new_informed_entities, informed_entities), - removed: MapSet.difference(informed_entities, new_informed_entities), - preserved: MapSet.intersection(new_informed_entities, informed_entities) + added: MapSet.difference(new, old), + removed: MapSet.difference(old, new), + preserved: MapSet.intersection(old, new) ] |> Map.new(fn {k, ies} -> {k, normalize(ies)} end) end @@ -69,7 +64,7 @@ defmodule State.Alert.HooksTest do assert %{ preserved: ^informed_entities, added: [%{stop: "parent-stationA"}, %{stop: "parent-stationB"}] - } = get_computed_informed_entities(informed_entities) + } = apply_hook(informed_entities) end test "merges child informed entities' activities for parent station informed entities," <> @@ -120,7 +115,7 @@ defmodule State.Alert.HooksTest do %{stop: "parent-stationB", activities: ["RIDE"], route: "route1", trip: "trip2"}, %{stop: "parent-stationB", activities: ["RIDE"], route: "route2", trip: "trip1"} ] - } = get_computed_informed_entities(informed_entities) + } = apply_hook(informed_entities) end test "adds informed entities for alternate trips" do @@ -241,7 +236,7 @@ defmodule State.Alert.HooksTest do activities: ["BOARD", "EXIT"] } ] - } = get_computed_informed_entities(informed_entities) + } = apply_hook(informed_entities) end test "adds Cartesian product of computed informed entities for parent stations X alternate trips" do @@ -347,7 +342,7 @@ defmodule State.Alert.HooksTest do activities: ["BOARD", "USING_WHEELCHAIR"] } ] - } = get_computed_informed_entities(informed_entities) + } = apply_hook(informed_entities) end test "handles specific case from bugfix ticket" do @@ -413,7 +408,7 @@ defmodule State.Alert.HooksTest do assert %{ preserved: ^informed_entities, added: ^expected_new_informed_entities - } = get_computed_informed_entities(alert) + } = apply_hook(alert) end end From 330139f821e190f23d65bd92dfc5ea1da3302717 Mon Sep 17 00:00:00 2001 From: Jon Zimbel Date: Wed, 11 Mar 2026 15:21:26 -0400 Subject: [PATCH 5/5] fix: Address review comments --- apps/state/lib/state/alert/hooks.ex | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/apps/state/lib/state/alert/hooks.ex b/apps/state/lib/state/alert/hooks.ex index 9a2ffe1aa..a4ca3edea 100644 --- a/apps/state/lib/state/alert/hooks.ex +++ b/apps/state/lib/state/alert/hooks.ex @@ -20,12 +20,20 @@ defmodule State.Alert.Hooks do |> Enum.uniq() end + # Parent station informed entities that share values for these keys + # will each have their `activities` replaced with the union of all of + # the group's `activities` lists: + # [%{activities: ["BOARD"]}, %{activities: ["BOARD", "EXIT"]}] + # => + # [%{activities: ["BOARD", "EXIT"]}, %{activities: ["BOARD", "EXIT"]}] + @parent_station_entity_activity_merge_criteria [:route, :stop, :trip] + @spec get_parent_station_entities([Alert.informed_entity()]) :: [Alert.informed_entity()] defp get_parent_station_entities(entities) do entities |> Stream.map(&get_parent_station_entity/1) |> Stream.reject(&is_nil/1) - |> Enum.group_by(&get_key/1) + |> Enum.group_by(&Map.take(&1, @parent_station_entity_activity_merge_criteria)) |> Enum.flat_map(&merge_parent_entity_activities/1) end @@ -46,8 +54,8 @@ defmodule State.Alert.Hooks do defp merge_parent_entity_activities({_key, parent_entities}) do merged_activities = parent_entities - |> Enum.map(&MapSet.new(&1[:activities] || [])) - |> Enum.reduce(&MapSet.union/2) + |> Enum.flat_map(&(&1[:activities] || [])) + |> Enum.uniq() |> Enum.sort() if merged_activities == [] do @@ -106,6 +114,4 @@ defmodule State.Alert.Hooks do trip_entities end end - - defp get_key(%{} = ie), do: Map.take(ie, ~w(route stop trip)a) end