Hotfix: forgot to add service change to diversions (#2474)

* add service change tod diversions page

* docs

* docs

* docs

* add time based tests

* add time based tests

Author: anthonyshull

lib/alerts/alert.ex

@@ -39,14 +39,6 @@ defmodule Alerts.Alert do
     :unknown | @ongoing_effects
   ]
 
-  @diversion_effects [
-    :detour,
-    :shuttle,
-    :stop_closure,
-    :station_closure,
-    :suspension
-  ]
-
   @lifecycles [:new, :ongoing, :ongoing_upcoming, :unknown, :upcoming]
 
   defstruct id: "",
@@ -291,15 +283,6 @@ defmodule Alerts.Alert do
 
   def high_severity_or_high_priority?(_), do: false
 
-  @spec diversion?(t) :: boolean()
-  def diversion?(alert) do
-    alert.effect in @diversion_effects &&
-      alert.active_period
-      |> List.first()
-      |> Kernel.elem(0)
-      |> Timex.after?(alert.created_at)
-  end
-
   @spec municipality(t) :: String.t() | nil
   def municipality(alert) do
     alert

lib/alerts/repo.ex

@@ -1,4 +1,6 @@
 defmodule Alerts.Repo do
+  import Dotcom.Alerts, only: [diversion_alert?: 1]
+
   alias Alerts.{Alert, Banner, Priority}
   alias Alerts.Cache.Store
   alias Alerts.Repo.Behaviour
@@ -33,15 +35,15 @@ defmodule Alerts.Repo do
   end
 
   @doc """
-  Get alerts that are diversion types: shuttle, station_closure, suspension.
+  Get alerts that are diversions: detour, service change, shuttle, station closure, stop closure, or suspension.
 
-  We sort them so that earlier alerts are displaed first.
+  Sort them so that earlier alerts are displaed first.
   """
   @spec diversions_by_route_ids([String.t()], DateTime.t()) :: [Alert.t()]
   def diversions_by_route_ids(route_ids, now) do
     route_ids
     |> by_route_ids(now)
-    |> Enum.filter(&Alert.diversion?/1)
+    |> Enum.filter(&diversion_alert?/1)
     |> Enum.sort(fn a, b ->
       first = a.active_period |> List.first() |> elem(0)
       second = b.active_period |> List.first() |> elem(0)

lib/dotcom/alerts.ex

@@ -8,9 +8,22 @@ defmodule Dotcom.Alerts do
 
   @stops_repo_module Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
+  @type diversion_effect_t() ::
+          :detour | :service_change | :shuttle | :station_closure | :stop_closure | :suspension
+
   @typedoc "Service alerts which typically impact rider experience."
   @type service_effect_t() :: :delay | :service_change | :shuttle | :suspension | :station_closure
 
+  # A keyword list of effects and the severity level necessary to make an alert a 'diversion.'
+  @diversion_effects [
+    detour: 1,
+    service_change: 3,
+    shuttle: 1,
+    station_closure: 1,
+    stop_closure: 1,
+    suspension: 1
+  ]
+
   # A keyword list of effects and the severity level necessary to make an alert 'service impacting.'
   @service_impacting_effects [
     delay: 1,
@@ -35,6 +48,22 @@ defmodule Dotcom.Alerts do
     |> Enum.sort_by(& &1.name)
   end
 
+  @doc """
+  Does the alert have an effect/severity that is considered a diversion?
+  And, was it planned?
+  """
+  @spec diversion_alert?(Alert.t()) :: boolean()
+  def diversion_alert?(alert) do
+    effects_match?(@diversion_effects, alert) &&
+      planned_alert?(alert)
+  end
+
+  @doc """
+  Returns a keyword list of the alert effects that are considered service-impacting and their severity levels.
+  """
+  @spec diversion_effects() :: [{diversion_effect_t(), integer()}]
+  def diversion_effects(), do: @diversion_effects
+
   @doc """
   Does the alert match a group of effects/severities?
   """
@@ -86,6 +115,14 @@ defmodule Dotcom.Alerts do
     effect == alert.effect && alert.severity >= severity
   end
 
+  # We consider an alert planned if the first active period starts after the alert was created.
+  defp planned_alert?(alert) do
+    alert.active_period
+    |> List.first()
+    |> Kernel.elem(0)
+    |> Timex.after?(alert.created_at)
+  end
+
   # Take an alert and return the start time of the first active period.
   # Return nil if there is no active period.
   defp sort_by_start_time_mapper(alert) do

test/alerts/alerts_test.exs

@@ -7,7 +7,6 @@ defmodule AlertsTest do
 
   alias Alerts.Alert
   alias Alerts.InformedEntity
-  alias Test.Support.Factories
 
   setup :verify_on_exit!
 
@@ -148,48 +147,6 @@ defmodule AlertsTest do
     end
   end
 
-  describe "diversion?/1" do
-    test "returns true when created at is before active period" do
-      created_at = Timex.now()
-      active_period = [{Timex.shift(created_at, days: 1), Timex.shift(created_at, days: 2)}]
-
-      alert =
-        Factories.Alerts.Alert.build(:alert,
-          active_period: active_period,
-          created_at: created_at,
-          effect: :shuttle
-        )
-
-      assert diversion?(alert)
-    end
-
-    test "returns false when created at is after active period" do
-      created_at = Timex.now()
-      active_period = [{Timex.shift(created_at, days: -2), Timex.shift(created_at, days: -1)}]
-
-      alert =
-        Factories.Alerts.Alert.build(:alert,
-          active_period: active_period,
-          created_at: created_at,
-          effect: :shuttle
-        )
-
-      refute diversion?(alert)
-    end
-
-    test "returns true for certain effects" do
-      assert diversion?(Factories.Alerts.Alert.build(:alert, effect: :shuttle))
-      assert diversion?(Factories.Alerts.Alert.build(:alert, effect: :stop_closure))
-      assert diversion?(Factories.Alerts.Alert.build(:alert, effect: :station_closure))
-    end
-
-    test "returns false for other effects" do
-      refute diversion?(Factories.Alerts.Alert.build(:alert, effect: :access_issue))
-      refute diversion?(Factories.Alerts.Alert.build(:alert, effect: :amber_alert))
-      refute diversion?(Factories.Alerts.Alert.build(:alert, effect: :delay))
-    end
-  end
-
   describe "municipality/1" do
     test "gets municipality from an alert's stops" do
       alert_with_muni = Alert.new(informed_entity: [%InformedEntity{stop: "some-stop"}])

test/alerts/repo_test.exs

@@ -66,8 +66,8 @@ defmodule Alerts.RepoTest do
   describe "diversions_by_route_id/2" do
     test "returns only diversions for the given route_ids" do
       # Setup
-      diversion = Factories.Alerts.Alert.build(:alert, effect: :shuttle)
-      non_diversion = Factories.Alerts.Alert.build(:alert, effect: :delay)
+      diversion = Factories.Alerts.Alert.build(:alert, effect: :service_change, severity: 3)
+      non_diversion = Factories.Alerts.Alert.build(:alert, effect: :delay, severity: 1)
 
       Store.update([diversion, non_diversion], nil)
 

test/dotcom/alerts_test.exs

@@ -7,7 +7,6 @@ defmodule Dotcom.AlertsTest do
   import Test.Support.Generators.DateTime,
     only: [random_date_time: 0, random_time_range_date_time: 1]
 
-  alias Alerts.Alert
   alias Test.Support.Factories
 
   setup :verify_on_exit!
@@ -24,6 +23,69 @@ defmodule Dotcom.AlertsTest do
     :ok
   end
 
+  describe "diversion_alert?/1" do
+    test "returns true if the alert has an effect that is considered a diversion" do
+      # Setup
+      {effect, severity} = diversion_effects() |> Faker.Util.pick()
+      alert = Factories.Alerts.Alert.build(:alert, effect: effect, severity: severity)
+
+      # Exercise/Verify
+      assert diversion_alert?(alert)
+    end
+
+    test "returns false if the alert does not have an effect that is considered a diversion" do
+      # Setup
+      alert = Factories.Alerts.Alert.build(:alert, effect: :not_a_diversion)
+
+      # Exercise/Verify
+      refute diversion_alert?(alert)
+    end
+
+    test "returns true when created at is before active period" do
+      # Setup
+      created_at = Timex.now()
+      active_period = [{Timex.shift(created_at, days: 1), Timex.shift(created_at, days: 2)}]
+
+      {effect, severity} = diversion_effects() |> Faker.Util.pick()
+
+      alert =
+        Factories.Alerts.Alert.build(:alert,
+          active_period: active_period,
+          created_at: created_at,
+          effect: effect,
+          severity: severity
+        )
+
+      # Exercise/Verify
+      assert diversion_alert?(alert)
+    end
+
+    test "returns false when created at is after active period" do
+      created_at = Timex.now()
+      active_period = [{Timex.shift(created_at, days: -2), Timex.shift(created_at, days: -1)}]
+
+      {effect, severity} = diversion_effects() |> Faker.Util.pick()
+
+      alert =
+        Factories.Alerts.Alert.build(:alert,
+          active_period: active_period,
+          created_at: created_at,
+          effect: effect,
+          severity: severity
+        )
+
+      # Exercise/Verify
+      refute diversion_alert?(alert)
+    end
+  end
+
+  describe "diversion_effects/0" do
+    test "returns a list of the alert effects as keywords" do
+      # Exercise/Verify
+      assert Keyword.keyword?(diversion_effects())
+    end
+  end
+
   describe "affected_stations/1" do
     test "returns a list of stations that are affected by the alert" do
       # Setup
@@ -52,15 +114,15 @@ defmodule Dotcom.AlertsTest do
     test "returns true if the alert has an effect that is considered service-impacting" do
       # Setup
       {effect, severity} = service_impacting_effects() |> Faker.Util.pick()
-      alert = %Alert{effect: effect, severity: severity}
+      alert = Factories.Alerts.Alert.build(:alert, effect: effect, severity: severity)
 
       # 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 = %Alert{effect: :not_service_impacting}
+      alert = Factories.Alerts.Alert.build(:alert, effect: :not_service_impacting)
 
       # Exercise/Verify
       refute service_impacting_alert?(alert)