From 9c7a295f4a96324f9edd6c081aee16c285bac939 Mon Sep 17 00:00:00 2001 From: Anthony Shull Date: Tue, 11 Feb 2025 11:58:52 -0600 Subject: [PATCH] Planned disruptions logic (#2373) * date_time module and tests * docs * linting * some cleanup * some cleanup * remove broken tests and add new tests * this week test * make service rollover time configurable * split into two modules * more tests * 100% coverage * some docs * some docs * format * helper funcs * move test to helper func * docs * format * move private funcs to factory * format * alphabetize config * remove unused import * service range and tests * PR feedback * microsecond fidelity * microsecond fidelity * nil in range and docs * more tests * docs * move in_range? * call it date time range * docs * docs * one hundred percent * spec ref * format * tests * type spec * more date time tests * typo * some renaming * docs * naming and tests * remove log and allow error * add a mock for date_time * mocks, tests, format * docs * one hundred percent coverage * make test more clear * move test * docs * start work * logic looks solid * use helper func * tests * tests * tests * one hundred * dialyzer * format * specs * tests * spec error * change tests * type check * type check * pr feedback * make behaviour load consistent * remove livebook --- config/config.exs | 1 + config/test.exs | 1 + lib/alerts/repo.ex | 5 +- lib/alerts/repo/behaviour.ex | 12 +++ lib/dotcom/alerts.ex | 23 +++++ lib/dotcom/alerts/disruptions/subway.ex | 55 +++++++++++ lib/dotcom/routes.ex | 13 +++ lib/dotcom/system_status/alerts.ex | 10 +- lib/dotcom/utils/date_time.ex | 2 + .../dotcom/alerts/disruptions/subway_test.exs | 92 +++++++++++++++++++ test/dotcom/alerts_test.exs | 31 +++++++ test/dotcom/routes_test.exs | 12 +++ test/dotcom/system_status/subway_test.exs | 32 +++---- test/dotcom/utils/date_time_test.exs | 2 + test/dotcom/utils/service_date_time_test.exs | 2 + test/support/mocks.ex | 1 + 16 files changed, 270 insertions(+), 24 deletions(-) create mode 100644 lib/alerts/repo/behaviour.ex create mode 100644 lib/dotcom/alerts.ex create mode 100644 lib/dotcom/alerts/disruptions/subway.ex create mode 100644 lib/dotcom/routes.ex create mode 100644 test/dotcom/alerts/disruptions/subway_test.exs create mode 100644 test/dotcom/alerts_test.exs create mode 100644 test/dotcom/routes_test.exs diff --git a/config/config.exs b/config/config.exs index fb6b00fbc1..ea4b51c6bd 100644 --- a/config/config.exs +++ b/config/config.exs @@ -25,6 +25,7 @@ config :dotcom, :redix, Redix config :dotcom, :redix_pub_sub, Redix.PubSub config :dotcom, :repo_modules, + alerts: Alerts.Repo, predictions: Predictions.Repo, route_patterns: RoutePatterns.Repo, routes: Routes.Repo, diff --git a/config/test.exs b/config/test.exs index 71130b86fc..c655c44ffe 100644 --- a/config/test.exs +++ b/config/test.exs @@ -20,6 +20,7 @@ config :dotcom, :mbta_api_module, MBTA.Api.Mock config :dotcom, :location_service, LocationService.Mock config :dotcom, :repo_modules, + alerts: Alerts.Repo.Mock, predictions: Predictions.Repo.Mock, route_patterns: RoutePatterns.Repo.Mock, routes: Routes.Repo.Mock, diff --git a/lib/alerts/repo.ex b/lib/alerts/repo.ex index 1c88641a2a..d4808e5be1 100644 --- a/lib/alerts/repo.ex +++ b/lib/alerts/repo.ex @@ -1,6 +1,9 @@ defmodule Alerts.Repo do alias Alerts.{Alert, Banner, Priority} alias Alerts.Cache.Store + alias Alerts.Repo.Behaviour + + @behaviour Behaviour @spec all(DateTime.t()) :: [Alert.t()] def all(now) do @@ -17,7 +20,7 @@ defmodule Alerts.Repo do Store.alert(id) end - @spec by_route_ids([String.t()], DateTime.t()) :: [Alert.t()] + @impl Behaviour def by_route_ids([], _now) do [] end diff --git a/lib/alerts/repo/behaviour.ex b/lib/alerts/repo/behaviour.ex new file mode 100644 index 0000000000..95318b7313 --- /dev/null +++ b/lib/alerts/repo/behaviour.ex @@ -0,0 +1,12 @@ +defmodule Alerts.Repo.Behaviour do + @moduledoc """ + Behaviour for the Alerts repo. + """ + + alias Alerts.Alert + + @doc """ + Return all alerts for the given route ids. + """ + @callback by_route_ids([String.t()], DateTime.t()) :: [Alert.t()] +end diff --git a/lib/dotcom/alerts.ex b/lib/dotcom/alerts.ex new file mode 100644 index 0000000000..684cea804a --- /dev/null +++ b/lib/dotcom/alerts.ex @@ -0,0 +1,23 @@ +defmodule Dotcom.Alerts do + @moduledoc """ + A collection of functions that help to work with alerts in a unified way. + """ + + alias Alerts.Alert + + @service_impacting_effects [:delay, :shuttle, :suspension, :station_closure] + + @doc """ + Does the alert have an effect that is considered service-impacting? + """ + @spec service_impacting_alert?(Alert.t()) :: boolean() + def service_impacting_alert?(%Alert{effect: effect}) do + effect in @service_impacting_effects + end + + @doc """ + Returns a list of the alert effects that are considered service-impacting. + """ + @spec service_impacting_effects() :: [atom()] + def service_impacting_effects(), do: @service_impacting_effects +end diff --git a/lib/dotcom/alerts/disruptions/subway.ex b/lib/dotcom/alerts/disruptions/subway.ex new file mode 100644 index 0000000000..ccb77a7f95 --- /dev/null +++ b/lib/dotcom/alerts/disruptions/subway.ex @@ -0,0 +1,55 @@ +defmodule Dotcom.Alerts.Disruptions.Subway do + @moduledoc """ + Disruptions are alerts that have `service_impacting_effects` grouped by `service_range`. + """ + + import Dotcom.Alerts, only: [service_impacting_alert?: 1] + import Dotcom.Routes, only: [subway_route_ids: 0] + import Dotcom.Utils.ServiceDateTime, only: [service_range: 1] + + alias Alerts.Alert + alias Dotcom.Utils + + @alerts_repo Application.compile_env!(:dotcom, :repo_modules)[:alerts] + + @doc """ + Disruptions that occur any time after today's service range. + """ + @spec future_disruptions() :: %{Utils.ServiceDateTime.named_service_range() => [Alert.t()]} + def future_disruptions() do + disruption_groups() |> Map.take([:later_this_week, :next_week, :after_next_week]) + end + + @doc """ + Disruptions that occur during today's service range. + """ + @spec todays_disruptions() :: %{today: [Alert.t()]} + def todays_disruptions() do + disruption_groups() |> Map.take([:today]) + end + + # Groups all disruption alerts by service range. + # + # 1. Gets all alerts for subway routes. + # 2. Filters out non-service-impacting alerts + # 3. Groups them according to service range. + defp disruption_groups() do + subway_route_ids() + |> @alerts_repo.by_route_ids(Utils.DateTime.now()) + |> Enum.filter(&service_impacting_alert?/1) + |> Enum.reduce(%{}, &group_alerts/2) + end + + # Looks at every active period for an alert and groups that alert by service range. + # Alerts can overlap service ranges, in which case we want them to appear in both. + defp group_alerts(alert, groups) do + alert + |> Map.get(:active_period) + |> Enum.map(fn {start, stop} -> [service_range(start), service_range(stop)] end) + |> List.flatten() + |> Enum.uniq() + |> Enum.reduce(groups, fn service_range, groups -> + Map.update(groups, service_range, [alert], &(&1 ++ [alert])) + end) + end +end diff --git a/lib/dotcom/routes.ex b/lib/dotcom/routes.ex new file mode 100644 index 0000000000..4c2b8037ec --- /dev/null +++ b/lib/dotcom/routes.ex @@ -0,0 +1,13 @@ +defmodule Dotcom.Routes do + @moduledoc """ + A collection of functions that help to work with routes in a unified way. + """ + + @subway_route_ids ["Blue", "Green", "Orange", "Red"] + + @doc """ + Returns a list of route ids for all subway routes. + """ + @spec subway_route_ids() :: [String.t()] + def subway_route_ids(), do: @subway_route_ids +end diff --git a/lib/dotcom/system_status/alerts.ex b/lib/dotcom/system_status/alerts.ex index 0d5a4a863f..317bea478f 100644 --- a/lib/dotcom/system_status/alerts.ex +++ b/lib/dotcom/system_status/alerts.ex @@ -5,13 +5,9 @@ defmodule Dotcom.SystemStatus.Alerts do belong in the main `Alerts` module. """ + import Dotcom.Alerts, only: [service_impacting_alert?: 1] + @type service_effect_t :: :delay | :shuttle | :suspension | :station_closure - @service_effects [:delay, :shuttle, :suspension, :station_closure] - @doc """ - Returns a list of the alert effects that are considered - service-impacting. - """ - def service_effects(), do: @service_effects @doc """ Returns `true` if the alert is active at some point during the day @@ -92,7 +88,7 @@ defmodule Dotcom.SystemStatus.Alerts do } """ def filter_relevant(alerts) do - alerts |> Enum.filter(fn %{effect: effect} -> effect in @service_effects end) + alerts |> Enum.filter(&service_impacting_alert?/1) end # Returns true if the alert (as signified by the active_period_start provided) diff --git a/lib/dotcom/utils/date_time.ex b/lib/dotcom/utils/date_time.ex index 6703450fd7..9b38eaba0a 100644 --- a/lib/dotcom/utils/date_time.ex +++ b/lib/dotcom/utils/date_time.ex @@ -39,6 +39,8 @@ defmodule Dotcom.Utils.DateTime do Timex will return an error if the time occurs when we "spring-forward" in DST transitions. That is because one hour does not occur--02:00:00am to 03:00:00am. In that case, we set the time to 03:00:00am. + + If we are given something tha tis not a DateTime, AmbiguousDateTime, or an error tuple, we log the input and return `now`. """ @impl Behaviour def coerce_ambiguous_date_time(%DateTime{} = date_time), do: date_time diff --git a/test/dotcom/alerts/disruptions/subway_test.exs b/test/dotcom/alerts/disruptions/subway_test.exs new file mode 100644 index 0000000000..eb7d6601e7 --- /dev/null +++ b/test/dotcom/alerts/disruptions/subway_test.exs @@ -0,0 +1,92 @@ +defmodule Dotcom.Alerts.Disruptions.SubwayTest do + use ExUnit.Case + + import Dotcom.Alerts, only: [service_impacting_effects: 0] + import Dotcom.Alerts.Disruptions.Subway + + import Dotcom.Utils.ServiceDateTime, + only: [ + service_range_day: 0, + service_range_later_this_week: 0, + service_range_next_week: 0, + service_range_after_next_week: 0 + ] + + import Mox + + alias Test.Support.Factories + + setup :verify_on_exit! + + setup _ do + stub_with(Dotcom.Utils.DateTime.Mock, Dotcom.Utils.DateTime) + + :ok + end + + describe "future_disruptions/0" do + test "returns an empty map when there are no alerts" do + expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> + [] + end) + + # Exercise/Verify + assert %{} = future_disruptions() + end + + test "returns alerts for later this week, next week, and after next week" do + # Setup + alert_today = service_range_day() |> disruption_alert() + alert_later_this_week = service_range_later_this_week() |> disruption_alert() + alert_next_week = service_range_next_week() |> disruption_alert() + + {alert_after_next_week_start, _} = service_range_after_next_week() + + alert_after_next_week = + {alert_after_next_week_start, Timex.shift(alert_after_next_week_start, days: 1)} + |> disruption_alert() + + expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> + [alert_today, alert_later_this_week, alert_next_week, alert_after_next_week] + end) + + # Exercise/Verify + assert %{ + later_this_week: [^alert_later_this_week], + next_week: [^alert_next_week], + after_next_week: [^alert_after_next_week] + } = future_disruptions() + end + end + + describe "todays_disruptions/0" do + test "returns an empty map when there are no alerts" do + expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> + [] + end) + + # Exercise/Verify + assert %{} = todays_disruptions() + end + + test "returns alerts for today only" do + # Setup + alert_today = service_range_day() |> disruption_alert() + alert_next_week = service_range_next_week() |> disruption_alert() + + expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> + [alert_today, alert_next_week] + end) + + # Exercise/Verify + assert %{today: [^alert_today]} = todays_disruptions() + end + end + + defp disruption_alert(active_period) do + Factories.Alerts.Alert.build(:alert, + active_period: [active_period], + effect: service_impacting_effects() |> Faker.Util.pick() + ) + end +end diff --git a/test/dotcom/alerts_test.exs b/test/dotcom/alerts_test.exs new file mode 100644 index 0000000000..0750b0b990 --- /dev/null +++ b/test/dotcom/alerts_test.exs @@ -0,0 +1,31 @@ +defmodule Dotcom.AlertsTest do + use ExUnit.Case + + import Dotcom.Alerts + + describe "service_impacting_alert?/1" do + test "returns true if the alert has an effect that is considered service-impacting" do + # Setup + effect = service_impacting_effects() |> Faker.Util.pick() + alert = %Alerts.Alert{effect: effect} + + # Exercise/Verify + assert service_impacting_alert?(alert) + end + + test "returns false if the alert does not have an effect that is considered service-impacting" do + # Setup + alert = %Alerts.Alert{effect: :not_service_impacting} + + # Exercise/Verify + refute service_impacting_alert?(alert) + end + end + + describe "service_impacting_effects/0" do + test "returns a list of the alert effects as atoms" do + # Exercise/Verify + assert Enum.all?(service_impacting_effects(), &is_atom/1) + end + end +end diff --git a/test/dotcom/routes_test.exs b/test/dotcom/routes_test.exs new file mode 100644 index 0000000000..3f70d9f775 --- /dev/null +++ b/test/dotcom/routes_test.exs @@ -0,0 +1,12 @@ +defmodule Dotcom.RoutesTest do + use ExUnit.Case + + import Dotcom.Routes + + describe "subway_route_ids/0" do + test "returns a list of route ids" do + # Exercise/Verify + assert Enum.all?(subway_route_ids(), &is_binary/1) + end + end +end diff --git a/test/dotcom/system_status/subway_test.exs b/test/dotcom/system_status/subway_test.exs index 7156bcb010..e121273c03 100644 --- a/test/dotcom/system_status/subway_test.exs +++ b/test/dotcom/system_status/subway_test.exs @@ -2,7 +2,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do use ExUnit.Case, async: true doctest Dotcom.SystemStatus.Subway - alias Dotcom.SystemStatus.Alerts + alias Dotcom.Alerts alias Dotcom.SystemStatus.Subway alias Test.Support.Factories.Alerts.Alert alias Test.Support.Factories.Alerts.InformedEntity @@ -29,7 +29,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do # Setup affected_route_id = Faker.Util.pick(@lines_without_branches) time = time_today() - effect = Faker.Util.pick(Alerts.service_effects()) + effect = Faker.Util.pick(Alerts.service_impacting_effects()) alerts = [current_alert(route_id: affected_route_id, time: time, effect: effect)] # Exercise @@ -48,7 +48,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do # Setup affected_route_id = Faker.Util.pick(@lines_without_branches) time = time_today() - effect = Faker.Util.pick(Alerts.service_effects()) + effect = Faker.Util.pick(Alerts.service_impacting_effects()) alerts = [current_alert(route_id: affected_route_id, time: time, effect: effect)] # Exercise @@ -130,7 +130,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do affected_route_id = Faker.Util.pick(@lines_without_branches) time = time_today() - effect = Faker.Util.pick(Alerts.service_effects()) + effect = Faker.Util.pick(Alerts.service_impacting_effects()) alert_start_time = time_before(time) alerts = [ @@ -195,7 +195,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do # Sorted in reverse order in order to validate that the sorting # logic works [effect2, effect1] = - Faker.Util.sample_uniq(2, fn -> Faker.Util.pick(Alerts.service_effects()) end) + Faker.Util.sample_uniq(2, fn -> Faker.Util.pick(Alerts.service_impacting_effects()) end) |> Enum.sort(:desc) alerts = [ @@ -246,7 +246,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do time = time_today() - effect = Faker.Util.pick(Alerts.service_effects()) + effect = Faker.Util.pick(Alerts.service_impacting_effects()) alerts = [ current_alert(route_id: affected_route_id, time: time, effect: effect), @@ -256,7 +256,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do # Exercise groups = Subway.subway_status(alerts, time) - # Verify + # Verify multiples = groups |> status_entries_for(affected_route_id) @@ -272,7 +272,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do time = time_today() start_time = time_after(time) - effect = Faker.Util.pick(Alerts.service_effects()) + effect = Faker.Util.pick(Alerts.service_impacting_effects()) alerts = [ future_alert(route_id: affected_route_id, start_time: start_time, effect: effect), @@ -282,7 +282,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do # Exercise groups = Subway.subway_status(alerts, time) - # Verify + # Verify multiples = groups |> status_entries_for(affected_route_id) @@ -297,7 +297,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do # Setup time = time_today() - effect = Faker.Util.pick(Alerts.service_effects()) + effect = Faker.Util.pick(Alerts.service_impacting_effects()) alerts = GreenLine.branch_ids() @@ -322,7 +322,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do affected_branch_id = Faker.Util.pick(GreenLine.branch_ids()) time = time_today() - effect = Faker.Util.pick(Alerts.service_effects()) + effect = Faker.Util.pick(Alerts.service_impacting_effects()) alerts = [current_alert(route_id: affected_branch_id, effect: effect, time: time)] @@ -343,7 +343,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do affected_branch_id = Faker.Util.pick(GreenLine.branch_ids()) time = time_today() - effect = Faker.Util.pick(Alerts.service_effects()) + effect = Faker.Util.pick(Alerts.service_impacting_effects()) alerts = [current_alert(route_id: affected_branch_id, effect: effect, time: time)] @@ -366,7 +366,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do affected_branch_id = Faker.Util.pick(GreenLine.branch_ids()) time = time_today() - effect = Faker.Util.pick(Alerts.service_effects()) + effect = Faker.Util.pick(Alerts.service_impacting_effects()) alerts = [current_alert(route_id: affected_branch_id, effect: effect, time: time)] @@ -395,7 +395,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do time = time_today() [effect1, effect2] = - Faker.Util.sample_uniq(2, fn -> Faker.Util.pick(Alerts.service_effects()) end) + Faker.Util.sample_uniq(2, fn -> Faker.Util.pick(Alerts.service_impacting_effects()) end) alerts = [ current_alert(route_id: affected_branch_id1, effect: effect1, time: time), @@ -439,7 +439,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do test "shows Mattapan as a branch of Red if it has an alert" do # Setup time = time_today() - effect = Faker.Util.pick(Alerts.service_effects()) + effect = Faker.Util.pick(Alerts.service_impacting_effects()) alerts = [current_alert(route_id: "Mattapan", effect: effect, time: time)] @@ -562,7 +562,7 @@ defmodule Dotcom.SystemStatus.SubwayTest do # - effect (default behavior is to choose an effect at random) defp alert(opts) do route_id = opts |> Keyword.fetch!(:route_id) - effect = opts[:effect] || Faker.Util.pick(Alerts.service_effects()) + effect = opts[:effect] || Faker.Util.pick(Alerts.service_impacting_effects()) active_period = opts |> Keyword.fetch!(:active_period) Alert.build(:alert, diff --git a/test/dotcom/utils/date_time_test.exs b/test/dotcom/utils/date_time_test.exs index 5803cb0e2c..d044740ea1 100644 --- a/test/dotcom/utils/date_time_test.exs +++ b/test/dotcom/utils/date_time_test.exs @@ -8,6 +8,8 @@ defmodule Dotcom.Utils.DateTimeTest do @timezone Application.compile_env!(:dotcom, :timezone) + setup :verify_on_exit! + setup _ do stub_with(Dotcom.Utils.DateTime.Mock, Dotcom.Utils.DateTime) diff --git a/test/dotcom/utils/service_date_time_test.exs b/test/dotcom/utils/service_date_time_test.exs index d359f0a004..5d3a0b5690 100644 --- a/test/dotcom/utils/service_date_time_test.exs +++ b/test/dotcom/utils/service_date_time_test.exs @@ -7,6 +7,8 @@ defmodule Dotcom.Utils.ServiceDateTimeTest do import Test.Support.Generators.DateTime import Test.Support.Generators.ServiceDateTime + setup :verify_on_exit! + setup _ do stub_with(Dotcom.Utils.DateTime.Mock, Dotcom.Utils.DateTime) diff --git a/test/support/mocks.ex b/test/support/mocks.ex index 7de1066374..93b59322d8 100644 --- a/test/support/mocks.ex +++ b/test/support/mocks.ex @@ -19,6 +19,7 @@ Mox.defmock(Predictions.PubSub.Mock, for: [GenServer, Predictions.PubSub.Behavio Mox.defmock(Predictions.Store.Mock, for: Predictions.Store.Behaviour) # Repos +Mox.defmock(Alerts.Repo.Mock, for: Alerts.Repo.Behaviour) Mox.defmock(Predictions.Repo.Mock, for: Predictions.Repo.Behaviour) Mox.defmock(Routes.Repo.Mock, for: Routes.Repo.Behaviour) Mox.defmock(RoutePatterns.Repo.Mock, for: RoutePatterns.Repo.Behaviour)