fix(timetable): Restrict Upcoming Changes to service-impacting alerts

lib/dotcom/system_status/commuter_rail.ex

@@ -91,6 +91,38 @@ defmodule Dotcom.SystemStatus.CommuterRail do
     end
   end
 
+  @doc """
+  Returns upcoming alerts for the route given.
+  """
+  @spec commuter_rail_upcoming_changes(String.t()) :: [Alerts.Alert.t()]
+  def commuter_rail_upcoming_changes(id) do
+    [id]
+    |> @alerts_repo.by_route_ids(@date_time_module.now())
+    |> Enum.filter(fn alert ->
+      (service_impacting_alert?(alert) || alert.effect == :schedule_change) &&
+        future_alert?(alert)
+    end)
+  end
+
+  # Checks if the next active period for an alert is in the future.
+  # Excludes alerts that end today.
+  defp future_alert?(alert) do
+    case next_active_time(alert) do
+      {:future, _} ->
+        true
+
+      {:current, start_time} ->
+        {_, end_time} =
+          alert.active_period
+          |> Enum.find(fn {start, _} -> DateTime.compare(start, start_time) == :eq end)
+
+        Util.safe_time_compare(end_time, Util.end_of_service()) == :gt
+
+      _ ->
+        false
+    end
+  end
+
   # Groups the alerts given into train impacts (delays and
   # cancellations) versus service impacts (everything else). For
   # train impacts, add trip info (first departure time, train number,

lib/dotcom_web/controllers/schedule/timetable_controller.ex

@@ -6,8 +6,8 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
 
   require Logger
 
-  import Dotcom.Alerts.StartTime, only: [next_active_time: 1]
-  import Dotcom.SystemStatus.CommuterRail, only: [commuter_rail_route_status: 1]
+  import Dotcom.SystemStatus.CommuterRail,
+    only: [commuter_rail_route_status: 1, commuter_rail_upcoming_changes: 1]
 
   alias Dotcom.Timetables
   alias DotcomWeb.ScheduleView
@@ -52,8 +52,7 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
     |> assign(:meta_description, meta_description)
     |> assign(:direction_name, direction_name)
     |> assign(:formatted_date, formatted_date)
-    |> assign_cr_status()
-    |> assign_cr_upcoming()
+    |> assign_cr_info()
     |> assign_banner_alerts()
     |> put_view(ScheduleView)
     |> render("show.html", [])
@@ -68,43 +67,19 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
   defp station_type_name(%Route{type: 4}), do: ~t"docks"
   defp station_type_name(_route), do: ~t"stations"
 
-  defp assign_cr_status(%{assigns: %{route: route}} = conn) do
-    cr_status =
-      if Routes.Route.type_atom(route) == :commuter_rail do
-        commuter_rail_route_status(route.id)
-      end
-
-    conn
-    |> assign(:cr_status, cr_status)
-  end
-
-  defp assign_cr_upcoming(%{assigns: %{alerts: alerts, route: route}} = conn) do
-    cr_upcoming =
-      if Routes.Route.type_atom(route) == :commuter_rail do
-        alerts
-        |> Enum.filter(&future_alert?/1)
-      else
-        []
-      end
-
-    conn
-    |> assign(:cr_upcoming, cr_upcoming)
-  end
-
-  defp future_alert?(alert) do
-    case next_active_time(alert) do
-      {:future, _} ->
-        true
-
-      {:current, start_time} ->
-        {_, end_time} =
-          alert.active_period
-          |> Enum.find(fn {start, _} -> DateTime.compare(start, start_time) == :eq end)
-
-        Util.safe_time_compare(end_time, Util.end_of_service()) == :gt
-
-      _ ->
-        false
+  defp assign_cr_info(%{assigns: %{route: route}} = conn) do
+    if Routes.Route.type_atom(route) == :commuter_rail do
+      conn
+      |> assign(%{
+        cr_status: commuter_rail_route_status(route.id),
+        cr_upcoming: commuter_rail_upcoming_changes(route.id)
+      })
+    else
+      conn
+      |> assign(%{
+        cr_status: nil,
+        cr_upcoming: []
+      })
     end
   end
 

test/dotcom/system_status/commuter_rail_test.exs

@@ -1,7 +1,9 @@
 defmodule Dotcom.SystemStatus.CommuterRailTest do
   use ExUnit.Case
 
-  import Dotcom.SystemStatus.CommuterRail, only: [commuter_rail_route_status: 1]
+  import Dotcom.SystemStatus.CommuterRail,
+    only: [commuter_rail_route_status: 1, commuter_rail_upcoming_changes: 1]
+
   import Mox
   import Test.Support.Generators.DateTime, only: [random_time_range_date_time: 1]
 
@@ -701,4 +703,76 @@ defmodule Dotcom.SystemStatus.CommuterRailTest do
       assert delay.trip_info == :all
     end
   end
+
+  describe "commuter_rail_upcoming_changes/1" do
+    test "includes only schedule changes and service-impacting alerts" do
+      # SETUP
+      today = Util.now()
+      active_period = [{today, DateTime.shift(today, week: 2)}]
+
+      service_impacting_effects =
+        Dotcom.Alerts.service_impacting_effects()
+        |> Enum.map(fn item -> elem(item, 0) end)
+
+      non_service_impacting_effects = Alerts.Alert.all_types() -- service_impacting_effects
+
+      schedule_change =
+        Factories.Alerts.Alert.build(
+          :alert,
+          active_period: active_period,
+          effect: :schedule_change
+        )
+
+      service_impact =
+        Factories.Alerts.Alert.build(
+          :alert,
+          active_period: active_period,
+          effect: Faker.Util.pick(service_impacting_effects)
+        )
+
+      other_effect =
+        Factories.Alerts.Alert.build(
+          :alert,
+          active_period: active_period,
+          effect: Faker.Util.pick(non_service_impacting_effects -- [:schedule_change])
+        )
+
+      # EXERCISE
+      expect(Alerts.Repo.Mock, :by_route_ids, fn _, _ ->
+        [schedule_change, service_impact, other_effect]
+      end)
+
+      alerts = commuter_rail_upcoming_changes("foo")
+
+      # VERIFY
+      assert Enum.sort(alerts) == Enum.sort([schedule_change, service_impact])
+    end
+
+    test "excludes alerts that end today" do
+      # SETUP
+      today = Util.now()
+      later = DateTime.shift(today, day: 3)
+
+      alert1 =
+        Factories.Alerts.Alert.build(
+          :alert,
+          active_period: [{today, today}],
+          effect: :schedule_change
+        )
+
+      alert2 =
+        Factories.Alerts.Alert.build(
+          :alert,
+          active_period: [{today, later}],
+          effect: :schedule_change
+        )
+
+      # EXERCISE
+      expect(Alerts.Repo.Mock, :by_route_ids, fn _, _ -> [alert1, alert2] end)
+      alerts = commuter_rail_upcoming_changes("foo")
+
+      # VERIFY
+      assert alerts == [alert2]
+    end
+  end
 end