diff --git a/lib/dotcom/alerts/disruptions/subway.ex b/lib/dotcom/alerts/disruptions/subway.ex index 5025e4b252..17811f7d3b 100644 --- a/lib/dotcom/alerts/disruptions/subway.ex +++ b/lib/dotcom/alerts/disruptions/subway.ex @@ -9,6 +9,7 @@ defmodule Dotcom.Alerts.Disruptions.Subway do alias Alerts.Alert alias Dotcom.Utils + alias Dotcom.Utils.ServiceDateTime @alerts_repo Application.compile_env!(:dotcom, :repo_modules)[:alerts] @date_time_module Application.compile_env!(:dotcom, :date_time_module) @@ -38,6 +39,7 @@ defmodule Dotcom.Alerts.Disruptions.Subway do defp disruption_groups() do subway_route_ids() |> @alerts_repo.by_route_ids(@date_time_module.now()) + |> Enum.flat_map(&split_by_discontiguous_active_periods/1) |> Enum.filter(&service_impacting_alert?/1) |> Enum.reduce(%{}, &group_alerts/2) |> Enum.map(fn {group, alerts} -> @@ -46,6 +48,52 @@ defmodule Dotcom.Alerts.Disruptions.Subway do |> Enum.into(%{}) end + # Given an alert with multiple active periods, some of which may be + # contiguous with one another (next one starts the same day or the + # day after the previous one), combines the contiguous active + # periods, and then returns one alert per combined-contiguous active + # period + # + # For instance, if an alert has three active periods, {Sun, Mon}, + # {Mon, Tue}, and {Thu, Fri}, then {Sun, Mon} and {Mon, Tue} are + # combined into {Sun, Tue}, but {Thu, Fri} is kept separate. The + # result is then two alerts, both equivalent to the alert passed in, + # except that one has a single active period of {Sun, Tue}, and the + # other has a single active period of {Thu, Fri}. + defp split_by_discontiguous_active_periods(alert) do + alert.active_period + |> combine_contiguous_active_periods() + |> Enum.map(fn active_period -> + %Alerts.Alert{alert | active_period: [active_period]} + end) + end + + # Given a list of active periods, combines contiguous ones. See + # split_by_discontiguous_active_periods/1 for more detail. Active + # periods are considered contiguous if the start of the next one is + # the same day as the end of the previous one, or if it's the day + # after, so active periods of {Mon, Wed} and {Thu, Fri} would be + # combined into {Mon, Fri}, as would {Mon, Wed} and {Wed, Fri}, but + # {Mon, Tue} and {Thu, Fri} would be kept separate. + defp combine_contiguous_active_periods([active_period1, active_period2 | rest]) do + {start1, end1} = active_period1 + {start2, end2} = active_period2 + + if Date.before?( + ServiceDateTime.service_date(end1) |> Date.shift(day: 1), + ServiceDateTime.service_date(start2) + ) do + [active_period1 | combine_contiguous_active_periods([active_period2 | rest])] + else + combined_active_period = {start1, end2} + combine_contiguous_active_periods([combined_active_period | rest]) + end + end + + defp combine_contiguous_active_periods(active_periods) do + active_periods + 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 diff --git a/test/dotcom/alerts/disruptions/subway_test.exs b/test/dotcom/alerts/disruptions/subway_test.exs index 7b65751062..b0243531f7 100644 --- a/test/dotcom/alerts/disruptions/subway_test.exs +++ b/test/dotcom/alerts/disruptions/subway_test.exs @@ -14,7 +14,9 @@ defmodule Dotcom.Alerts.Disruptions.SubwayTest do import Mox + alias Dotcom.Utils.ServiceDateTime alias Test.Support.Factories + alias Test.Support.Generators setup :verify_on_exit! @@ -43,7 +45,7 @@ defmodule Dotcom.Alerts.Disruptions.SubwayTest do {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)} + {alert_after_next_week_start, DateTime.shift(alert_after_next_week_start, day: 1)} |> disruption_alert() expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> @@ -64,7 +66,7 @@ defmodule Dotcom.Alerts.Disruptions.SubwayTest do {alert_after_next_week_start, _} = service_range_after_next_week() long_alert = - [{alert_today_start, Timex.shift(alert_after_next_week_start, days: 1)}] + [{alert_today_start, DateTime.shift(alert_after_next_week_start, day: 1)}] |> disruption_alert() expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> @@ -81,18 +83,191 @@ defmodule Dotcom.Alerts.Disruptions.SubwayTest do test "handles alert with more than one active_period" do # Setup + long_alert = [service_range_this_week(), service_range_next_week()] |> disruption_alert() + + expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> + [long_alert] + end) + + # Exercise + disruptions = future_disruptions() + + # Verify + [alert_this_week] = disruptions.this_week + [alert_next_week] = disruptions.next_week + + assert alert_this_week.id == long_alert.id + assert alert_next_week.id == long_alert.id + end + + test "splits the active_periods of alerts with more than one into different buckets" do + # Setup + {beginning_of_next_week, end_of_next_week} = service_range_next_week() + + active_period_1_start = + Generators.DateTime.random_time_range_date_time( + {beginning_of_next_week, DateTime.shift(end_of_next_week, day: -2)} + ) + + active_period_1_end = + Generators.DateTime.random_time_range_date_time( + {active_period_1_start, DateTime.shift(end_of_next_week, day: -1)} + ) + + {beginning_of_week_after_next, _} = service_range_after_next_week() + + active_period_2_start = + Generators.DateTime.random_time_range_date_time({beginning_of_week_after_next, nil}) + + active_period_2_end = + Generators.DateTime.random_time_range_date_time({active_period_2_start, nil}) + long_alert = - [service_range_this_week(), service_range_next_week()] |> disruption_alert() + [ + {active_period_1_start, active_period_1_end}, + {active_period_2_start, active_period_2_end} + ] + |> disruption_alert() expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> [long_alert] end) - # Exercise/Verify - assert %{ - this_week: [^long_alert], - next_week: [^long_alert] - } = future_disruptions() + # Exercise + disruptions = future_disruptions() + + # Verify + [alert_next_week] = disruptions.next_week + assert alert_next_week.id == long_alert.id + assert alert_next_week.active_period == [{active_period_1_start, active_period_1_end}] + + [alert_after_next_week] = disruptions.after_next_week + assert alert_after_next_week.id == long_alert.id + assert alert_after_next_week.active_period == [{active_period_2_start, active_period_2_end}] + end + + test "combines consecutive active periods if the boundary is on the same day" do + # Setup + {beginning_of_week, end_of_week} = service_range_next_week() + + active_period_1_end = + Generators.DateTime.random_time_range_date_time( + {beginning_of_week, DateTime.shift(end_of_week, day: -2)} + ) + + active_period_2_start = + Generators.DateTime.random_time_range_date_time( + {active_period_1_end, ServiceDateTime.end_of_service_day(active_period_1_end)} + ) + + multi_active_period_alert = + [{beginning_of_week, active_period_1_end}, {active_period_2_start, end_of_week}] + |> disruption_alert() + + expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> + [multi_active_period_alert] + end) + + # Exercise + disruptions = future_disruptions() + + # Verify + assert Enum.count(disruptions.next_week) == 1 + [combined_active_period_alert] = disruptions.next_week + assert combined_active_period_alert.active_period == [{beginning_of_week, end_of_week}] + end + + test "combines consecutive active periods if the boundary is on consecutive days" do + # Setup + {beginning_of_week, end_of_week} = service_range_next_week() + + active_period_1_end = + Generators.DateTime.random_time_range_date_time( + {beginning_of_week, DateTime.shift(end_of_week, day: -2)} + ) + + active_period_2_start = + Generators.DateTime.random_time_range_date_time( + {active_period_1_end |> ServiceDateTime.beginning_of_next_service_day(), + active_period_1_end + |> ServiceDateTime.beginning_of_next_service_day() + |> ServiceDateTime.end_of_service_day()} + ) + + multi_active_period_alert = + [{beginning_of_week, active_period_1_end}, {active_period_2_start, end_of_week}] + |> disruption_alert() + + expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> + [multi_active_period_alert] + end) + + # Exercise + disruptions = future_disruptions() + + # Verify + assert Enum.count(disruptions.next_week) == 1 + [combined_active_period_alert] = disruptions.next_week + assert combined_active_period_alert.active_period == [{beginning_of_week, end_of_week}] + end + + test "combines consecutive active periods in the middle of the active period list" do + # Setup + {beginning_of_week, end_of_week} = service_range_next_week() + + active_period_0_start = + Generators.DateTime.random_time_range_date_time( + {beginning_of_week, DateTime.shift(beginning_of_week, day: 1)} + ) + + active_period_0_end = + Generators.DateTime.random_time_range_date_time( + {active_period_0_start, DateTime.shift(active_period_0_start, day: 1)} + ) + + active_period_1_start = + Generators.DateTime.random_time_range_date_time( + {DateTime.shift(active_period_0_end, day: 2), DateTime.shift(end_of_week, day: -2)} + ) + + active_period_1_end = + Generators.DateTime.random_time_range_date_time( + {active_period_1_start, DateTime.shift(end_of_week, day: -2)} + ) + + active_period_2_start = + Generators.DateTime.random_time_range_date_time( + {active_period_1_end, ServiceDateTime.end_of_service_day(active_period_1_end)} + ) + + active_period_2_end = end_of_week + + multi_active_period_alert = + [ + {active_period_0_start, active_period_0_end}, + {active_period_1_start, active_period_1_end}, + {active_period_2_start, active_period_2_end} + ] + |> disruption_alert() + + expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> + [multi_active_period_alert] + end) + + # Exercise + disruptions = future_disruptions() + + # Verify + assert Enum.count(disruptions.next_week) == 2 + [first_active_period_alert, combined_later_active_period_alert] = disruptions.next_week + + assert first_active_period_alert.active_period == [ + {active_period_0_start, active_period_0_end} + ] + + assert combined_later_active_period_alert.active_period == [ + {active_period_1_start, active_period_2_end} + ] end end @@ -137,17 +312,21 @@ defmodule Dotcom.Alerts.Disruptions.SubwayTest do [alert_today_and_beyond, alert_today_and_other_dates] end) - # Exercise/Verify - assert %{ - today: [^alert_today_and_beyond, ^alert_today_and_other_dates] - } = todays_disruptions() + # Exercise + disruptions = todays_disruptions() + + # Verify + assert disruptions.today |> Enum.map(& &1.id) == [ + alert_today_and_beyond.id, + alert_today_and_other_dates.id + ] end test "sorts alerts by start time" do # Setup {start, stop} = service_range_day() alert_today = disruption_alert({start, stop}) - alert_later = disruption_alert({Timex.shift(start, seconds: 1), stop}) + alert_later = disruption_alert({DateTime.shift(start, second: 1), stop}) expect(Alerts.Repo.Mock, :by_route_ids, fn _route_ids, _now -> [alert_later, alert_today] diff --git a/test/support/generators/date_time.ex b/test/support/generators/date_time.ex index cb738bf65d..6421de5cfd 100644 --- a/test/support/generators/date_time.ex +++ b/test/support/generators/date_time.ex @@ -30,7 +30,7 @@ defmodule Test.Support.Generators.DateTime do @doc "Generate a random date_time between the beginning and end of the time range." def time_range_date_time_generator({start, nil}) do - stop = Timex.shift(start, years: 10) + stop = Timex.shift(start, years: 10) |> @date_time_module.coerce_ambiguous_date_time() time_range_date_time_generator({start, stop}) end