Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add module that retrieves system-status relevant alerts #2327

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions lib/dotcom/system_status.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
defmodule Dotcom.SystemStatus do
@moduledoc """
Parent module for the system status feature
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: can you describe the system status feature a bit for the benefit of future readers? :)

"""

alias Dotcom.SystemStatus

@doc """
Returns a list of alerts that satisfy the following criteria:
- They are for one of the subway or trolley lines (including Mattapan), and
- They are either currently active, or will be later today
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make this more straightforward by saying that the alert is active today... if it is currently active, it will necessarily be active today.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it the way I did because "active today" sort of implies that it would also include alerts that were active earlier today, but have been closed out. I guess I could write it as

- They are active today, and not in the past

Or maybe

- They will be active at some point later today (possibly including now)

"""
def subway_alerts_for_today() do
subway_alerts_for_today(Timex.now())
end

defp subway_alerts_for_today(now) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename this function to subway_alerts_for_day. That would make more sense with the function above. It's also confusing to name this argument now as it can be any time.

[
"Red",
"Orange",
"Blue",
"Green-B",
"Green-C",
"Green-D",
"Green-E",
"Mattapan"
]
|> Alerts.Repo.by_route_ids(now)
|> SystemStatus.Alerts.for_today(now)
|> SystemStatus.Alerts.filter_relevant()
end
end
103 changes: 103 additions & 0 deletions lib/dotcom/system_status/alerts.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
defmodule Dotcom.SystemStatus.Alerts do
@moduledoc """
A utility module intended to filter alerts for the system status feature,
relying on some specific criteria that are specific enough that they don't
belong in the main `Alerts` module.
"""
@relevant_effects [:delay, :shuttle, :suspension, :station_closure]

defp has_started?(active_period_start, now) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing with all of these. now should be a more generic date_time or something.

now |> Timex.end_of_day() |> Timex.after?(active_period_start)
end

defp has_not_ended?(nil, _now) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We put all public functions higher in the module than all private functions. I generally try to alphabetize them in order as well.

true
end

defp has_not_ended?(active_period_end, now) do
now |> Timex.before?(active_period_end)
end

@doc """
Checks to see whether an alert is active at some point later today, possibly including
`now`.

Returns `true` if
- The alert is currently active
- The alert will become active later in the day

## Example (Currently Active)
iex> now = Timex.to_datetime(~N[2025-01-05 14:00:00], "America/New_York")
iex> one_hour_ago = Timex.to_datetime(~N[2025-01-05 13:00:00], "America/New_York")
iex> one_hour_from_now = Timex.to_datetime(~N[2025-01-05 15:00:00], "America/New_York")
iex> Dotcom.SystemStatus.Alerts.active_today?(
...> %Alerts.Alert{active_period: [{one_hour_ago, one_hour_from_now}]},
...> now
...> )
true

## Example (Active Later Today)
iex> now = Timex.to_datetime(~N[2025-01-05 14:00:00], "America/New_York")
iex> one_hour_from_now = Timex.to_datetime(~N[2025-01-05 15:00:00], "America/New_York")
iex> one_day_from_now = Timex.to_datetime(~N[2025-01-06 14:00:00], "America/New_York")
iex> Dotcom.SystemStatus.Alerts.active_today?(
...> %Alerts.Alert{active_period: [{one_hour_from_now, one_day_from_now}]},
...> now
...> )
true

Returns `false` if the alert is not currently active, and either
- Was only active in the past (even if earlier today)
- Will next become active after the end of the day today

## Example (Expired)
iex> now = Timex.to_datetime(~N[2025-01-05 14:00:00], "America/New_York")
iex> two_hours_ago = Timex.to_datetime(~N[2025-01-05 12:00:00], "America/New_York")
iex> one_hour_ago = Timex.to_datetime(~N[2025-01-05 13:00:00], "America/New_York")
iex> Dotcom.SystemStatus.Alerts.active_today?(
...> %Alerts.Alert{active_period: [{two_hours_ago, one_hour_ago}]},
...> now
...> )
false

## Example (Not Active Until Tomorrow)
iex> now = Timex.to_datetime(~N[2025-01-05 14:00:00], "America/New_York")
iex> one_day_from_now = Timex.to_datetime(~N[2025-01-06 14:00:00], "America/New_York")
iex> two_days_from_now = Timex.to_datetime(~N[2025-01-07 14:00:00], "America/New_York")
iex> Dotcom.SystemStatus.Alerts.active_today?(
...> %Alerts.Alert{active_period: [{one_day_from_now, two_days_from_now}]},
...> now
...> )
false
"""
def active_today?(alert, now) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, I would name this active_on_day? since it can take any time.

Enum.any?(alert.active_period, fn {active_period_start, active_period_end} ->
has_started?(active_period_start, now) && has_not_ended?(active_period_end, now)
end)
end

@doc """
Given a list of alerts, filters only the ones that are active today, as defined in `&active_today?/2`.
See that function for details
"""
def for_today(alerts, now) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for_day

Enum.filter(alerts, &active_today?(&1, now))
end

@doc """
Given a list of alerts, returns only the alerts whose effects are one of
`[:delay, :shuttle, :suspension, :station_closure]`.

## Examples
iex> alerts = [
...> %Alerts.Alert{id: "include_this", effect: :delay},
...> %Alerts.Alert{id: "exclude_this", effect: :escalator_closure}
...> ]
iex> Dotcom.SystemStatus.Alerts.filter_relevant(alerts) |> Enum.map(& &1.id)
["include_this"]
}
"""
def filter_relevant(alerts) do
alerts |> Enum.filter(fn %{effect: effect} -> effect in @relevant_effects end)
end
end
5 changes: 5 additions & 0 deletions lib/dotcom_web/live/admin.ex
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ defmodule DotcomWeb.Live.Admin do
url: Helpers.live_path(socket, DotcomWeb.Live.TripPlanner),
title: "Trip Planner Preview",
description: "WIP on the trip planner rewrite."
},
%{
url: Helpers.live_path(socket, DotcomWeb.Live.SystemStatus),
title: "System Status Widget Preview",
description: "WIP on the system status widget."
}
]
)}
Expand Down
35 changes: 35 additions & 0 deletions lib/dotcom_web/live/system_status.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
defmodule DotcomWeb.Live.SystemStatus do
@moduledoc """
A temporary LiveView for showing off the system status widget until we
put it into the homepage (and elsewhere).
"""

alias Dotcom.SystemStatus
use DotcomWeb, :live_view

def render(assigns) do
assigns =
assigns
|> assign(:alerts, SystemStatus.subway_alerts_for_today())

~H"""
<div class="flex flex-col gap-2">
<.alert :for={alert <- @alerts} alert={alert} />
</div>
"""
end

defp alert(assigns) do
~H"""
<details class="border border-gray-lighter p-2">
<summary>
<span class="font-bold">{@alert.severity} {@alert.effect}:</span> {@alert.header}
</summary>
<details>
<summary>Raw alert</summary>
<pre>{inspect(@alert, pretty: true)}</pre>
</details>
</details>
"""
end
end
9 changes: 9 additions & 0 deletions lib/dotcom_web/router.ex
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,15 @@ defmodule DotcomWeb.Router do
end
end

scope "/preview", DotcomWeb do
import Phoenix.LiveView.Router
pipe_through([:browser, :browser_live, :basic_auth_readonly])

live_session :system_status, layout: {DotcomWeb.LayoutView, :preview} do
live "/system-status", Live.SystemStatus
end
end

scope "/api", DotcomWeb do
pipe_through([:secure, :browser])

Expand Down
178 changes: 178 additions & 0 deletions test/dotcom/system_status/alerts_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
defmodule Dotcom.SystemStatus.AlertsTest do
use ExUnit.Case, async: true
doctest Dotcom.SystemStatus.Alerts
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: so cool to add this (honestly I forgot about this feature)


import Test.Support.Factories.Alerts.Alert

alias Dotcom.SystemStatus.Alerts

defp local_datetime(naive) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually put private functions at the bottom of the file, though, tbh, not sure we need this. See below.

DateTime.from_naive!(naive, "America/New_York")
end

describe "active_today?/2" do
test "returns true if the alert is currently active" do
assert Alerts.active_today?(
build(:alert,
active_period: [
{
local_datetime(~N[2025-01-09 12:00:00]),
local_datetime(~N[2025-01-09 20:00:00])
Comment on lines +19 to +20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than hardcoding these, you can use Faker: https://hexdocs.pm/faker/Faker.DateTime.html. It's a good way to catch subtle bugs that otherwise wouldn't occur because of the values chosen.

To me, this is much easier to understand as well.

# Setup
start = Timex.now()
end = Timex.shift(start, hours: 24)

alert = build(:alert, active_period: [{start, end}]

time = Faker.DateTime.between(start, end)

# Exercise/Verify
assert Alerts.active_for_day?(alert, time)

}
]
),
local_datetime(~N[2025-01-09 13:00:00])
)
end

test "returns false if the alert starts after end-of-service" do
refute Alerts.active_today?(
build(:alert,
active_period: [
{
local_datetime(~N[2025-01-10 12:00:00]),
local_datetime(~N[2025-01-10 20:00:00])
}
]
),
local_datetime(~N[2025-01-09 13:00:00])
)
end

test "returns true if the alert starts later, but before end-of-service" do
assert Alerts.active_today?(
build(:alert,
active_period: [
{
local_datetime(~N[2025-01-09 20:00:00]),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious what happens if it starts at ~N[2025-01-10 01:00:00], which is technically the same service day (1/9) as the date being checked against, but is technically the next calendar day...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UGH

Yeah I admit that I was busy pretending that end-of-service was midnight, but we should probably handle this correctly, shouldn't we.

local_datetime(~N[2025-01-10 20:00:00])
}
]
),
local_datetime(~N[2025-01-09 13:00:00])
)
end

test "returns false if the alert has already ended" do
refute Alerts.active_today?(
build(:alert,
active_period: [
{
local_datetime(~N[2025-01-09 10:00:00]),
local_datetime(~N[2025-01-09 12:00:00])
}
]
),
local_datetime(~N[2025-01-09 13:00:00])
)
end

test "returns true if the alert has no end time" do
alert =
build(:alert,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file might read a bit cleaner with a helper function to generate an alert from a given active_period start and/or stop 😅

active_period: [
{
local_datetime(~N[2025-01-09 10:00:00]),
nil
}
]
)

assert Alerts.active_today?(
alert,
local_datetime(~N[2025-01-09 13:00:00])
)
end

test "returns false if the alert has no end time but hasn't started yet" do
refute Alerts.active_today?(
build(:alert,
active_period: [
{
local_datetime(~N[2025-01-10 10:00:00]),
nil
}
]
),
local_datetime(~N[2025-01-09 13:00:00])
)
end

test "returns true if a later part of the alert's active period is active" do
assert Alerts.active_today?(
build(:alert,
active_period: [
{
local_datetime(~N[2025-01-08 10:00:00]),
local_datetime(~N[2025-01-08 12:00:00])
},
{
local_datetime(~N[2025-01-09 10:00:00]),
local_datetime(~N[2025-01-09 12:00:00])
}
]
),
local_datetime(~N[2025-01-09 11:00:00])
)
end
end

describe "for_today/2" do
test "includes alerts that are active today" do
alert1 =
build(:alert,
active_period: [
{
local_datetime(~N[2025-01-09 12:00:00]),
local_datetime(~N[2025-01-09 20:00:00])
}
]
)

alert2 =
build(:alert,
active_period: [
{
local_datetime(~N[2025-01-10 12:00:00]),
local_datetime(~N[2025-01-10 20:00:00])
}
]
)

assert Alerts.for_today(
[alert1, alert2],
local_datetime(~N[2025-01-09 13:00:00])
) == [alert1]
end
end

describe "filter_relevant/1" do
test "includes an alert if its effect is :delay" do
alert = build(:alert, effect: :delay)
assert Alerts.filter_relevant([alert]) == [alert]
end

test "includes an alert if its effect is :shuttle" do
alert = build(:alert, effect: :shuttle)
assert Alerts.filter_relevant([alert]) == [alert]
end

test "includes an alert if its effect is :suspension" do
alert = build(:alert, effect: :suspension)
assert Alerts.filter_relevant([alert]) == [alert]
end

test "includes an alert if its effect is :station_closure" do
alert = build(:alert, effect: :station_closure)
assert Alerts.filter_relevant([alert]) == [alert]
end

test "does not include alerts with other effects" do
assert Alerts.filter_relevant([
build(:alert, effect: :policy_change),
build(:alert, effect: :extra_service),
build(:alert, effect: :stop_closure)
]) == []
end
end
end
Loading