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: hide CR timetable in the presence of special alerts #2301

Merged
merged 6 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
43 changes: 43 additions & 0 deletions lib/dotcom/timetable_blocking.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
defmodule Dotcom.TimetableBlocking do
@moduledoc """
Support for blocking the timetable in the presence of special alert text.
"""
alias Alerts.Alert
alias Routes.Route

def pdf_available_text, do: "View PDF Timetable on the MBTA website."
def no_pdf_text, do: "Schedule will be available soon on the MBTA website."

@doc """
Strips the timetable blocking text from an alert header (if present)
"""
@spec strip(String.t()) :: String.t()
def strip(header) when is_binary(header) do
String.replace_trailing(header, pdf_available_text(), "")
end

@doc """
Returns the alert blocking the timetable for a given route/date, or nil if there is no such alert.
"""
@spec blocking_alert([Alert.t()], Route.t(), Date.t()) :: Alert.t() | nil
def blocking_alert(alerts, %Route{} = route, %Date{} = date) when is_list(alerts) do
# we do the filtering in this order to do the cheap string comparison before
# the more complicated informed entity/time matching
alerts
|> Enum.filter(fn alert ->
String.ends_with?(alert.header, pdf_available_text()) or
String.ends_with?(alert.header, no_pdf_text())
end)
|> Alerts.Match.match(
%Alerts.InformedEntity{route: route.id},
date
)
|> List.first()
end

@doc "Returns true if the given alert is blocking and has a PDF."
@spec pdf?(Alert.t()) :: boolean
def pdf?(%Alert{} = alert) do
String.ends_with?(alert.header, pdf_available_text()) and is_binary(alert.url)
end
end
22 changes: 21 additions & 1 deletion lib/dotcom_web/controllers/schedule/timetable_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
plug(:direction_id)
plug(DotcomWeb.ScheduleController.RoutePdfs)
plug(DotcomWeb.ScheduleController.Core)
plug(:alert_blocks)
plug(:do_assign_trip_schedules)
plug(DotcomWeb.ScheduleController.Offset)
plug(DotcomWeb.ScheduleController.ScheduleError)
Expand Down Expand Up @@ -53,8 +54,27 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
Util.log_duration(__MODULE__, :assign_trip_schedules, [conn])
end

def alert_blocks(conn, _) do
assign(
conn,
:blocking_alert,
Dotcom.TimetableBlocking.blocking_alert(
conn.assigns.alerts,
conn.assigns.route,
conn.assigns.date
)
)
end

def assign_trip_schedules(
%{assigns: %{route: route, direction_id: direction_id, date_in_rating?: true}} = conn
%{
assigns: %{
route: route,
direction_id: direction_id,
blocking_alert: nil,
date_in_rating?: true
}
} = conn
) do
timetable_schedules = timetable_schedules(conn)
vehicle_schedules = vehicle_schedules(conn, timetable_schedules)
Expand Down
8 changes: 6 additions & 2 deletions lib/dotcom_web/templates/alert/_item.html.eex
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@
end %>
</div>
<div class="c-alert-item__content">
<%= replace_urls_with_links(@alert.header) %>
<%= replace_urls_with_links(Dotcom.TimetableBlocking.strip(@alert.header)) %>
<%= if @alert.url != nil && @alert.url != "" && !Map.get(assigns, :hide_url, false) do %>
<%= replace_urls_with_links(@alert.url) %>
<%= if Dotcom.TimetableBlocking.pdf?(@alert) do %>
<%= link "View PDF Timetable", to: @alert.url %>
<% else %>
<%= replace_urls_with_links(@alert.url) %>
<% end %>
<% end %>
</div>
</div>
Expand Down
9 changes: 6 additions & 3 deletions lib/dotcom_web/templates/schedule/_timetable.html.eex
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
<%= DotcomWeb.AlertView.group(alerts: @alerts, route: @route, date_time: @date_time, priority_filter: :high) %>
<%= DotcomWeb.AlertView.group(alerts: @alerts, route: @route, date_time: @date_time, priority_filter: :high, always_show: List.wrap(@blocking_alert)) %>
<%= render "_trip_view_filters.html", assigns %>

<div class="calendar-covered m-timetable">
<%= content_tag(:div, "", [class: "calendar-cover", hidden: !@show_date_select?]) %>
<%= if Enum.empty?(@header_schedules) do %>
<%= cond do %>
<% @blocking_alert != nil -> %>
<%= render "_timetable_blocked.html", formatted_date: @formatted_date, alert: @blocking_alert %>
<% Enum.empty?(@header_schedules) -> %>
<%= render "_empty.html", date: @date, direction: Routes.Route.direction_name(@route, @direction_id), conn: @conn, error: assigns[:schedule_error] %>
<% else %>
<% true -> %>
<% # only show scroll controllers if we have 2 or more schedules
should_show_scroll_controls? = match?([_, _ | _], @header_schedules)
%>
Expand Down
10 changes: 10 additions & 0 deletions lib/dotcom_web/templates/schedule/_timetable_blocked.html.eex
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<div class="callout schedule-empty">
The interactive timetable for <%= @formatted_date %> is temporarily unavailable due to a service change.
<div>
<%= if @alert.url do %>
<%= link "View PDF Timetable", to: @alert.url %>
<% else %>
PDF Upcoming
<% end %>
</div>
</div>
25 changes: 11 additions & 14 deletions lib/dotcom_web/views/alert_view.ex
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ defmodule DotcomWeb.AlertView do
alias Routes.Route
alias Stops.Stop

@type priority_filter :: :any | Alerts.Priority.priority_level()

@doc """

Used to render a group of alerts.
Expand All @@ -28,6 +30,7 @@ defmodule DotcomWeb.AlertView do
opts
|> Keyword.fetch!(:alerts)
|> Enum.filter(&filter_by_priority(priority_filter, &1))
|> then(&(Keyword.get(opts, :always_show, []) ++ &1))
|> deduplicate()

case {alerts, show_empty?} do
Expand All @@ -46,17 +49,8 @@ defmodule DotcomWeb.AlertView do
end
end

# Workaround handling duplicate Red Line alerts for JFK-Ashmont shuttle
defp deduplicate(alerts) do
alert_ids = Enum.map(alerts, & &1.id)
ashmont_shuttle_alert_ids = ["519314", "529291"]

if Enum.all?(ashmont_shuttle_alert_ids, &Enum.member?(alert_ids, &1)) do
# remove the second one
Enum.reject(alerts, &(&1.id == "529291"))
else
alerts
end
Enum.uniq_by(alerts, & &1.id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks!!

end

@spec no_alerts_message(map, boolean, atom) :: iolist
Expand Down Expand Up @@ -129,14 +123,17 @@ defmodule DotcomWeb.AlertView do
" at this time."
]

@spec filter_by_priority(boolean, Alert.t()) :: boolean
defp filter_by_priority(:any, _), do: true
@spec filter_by_priority(
priority_filter,
Alert.t()
) :: boolean
defp filter_by_priority(:any, _alert), do: true

defp filter_by_priority(priority_filter, %{priority: priority})
when priority_filter == priority,
when is_atom(priority_filter) and priority_filter == priority,
do: true

defp filter_by_priority(_, _), do: false
defp filter_by_priority(_filter, _alert), do: false

def effect_name(%{lifecycle: lifecycle} = alert)
when lifecycle in [:new, :unknown] do
Expand Down
58 changes: 58 additions & 0 deletions test/dotcom/timetable_blocking_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
defmodule Dotcom.TimetableBlockingTest do
@moduledoc false
use ExUnit.Case, async: true
alias Dotcom.TimetableBlocking

@date ~D[2025-01-01]
@route %Routes.Route{id: "CR-route"}

describe "blocking_alert/3" do
test "returns an alert if there's a specially tagged alert without a PDF" do
alert =
Alerts.Alert.new(
header: "No timetable. #{TimetableBlocking.no_pdf_text()}",
informed_entity: [%Alerts.InformedEntity{route: @route.id}],
active_period: [{~U[2025-01-01T00:00:00Z], nil}]
)

assert TimetableBlocking.blocking_alert([alert], @route, @date) == alert
end

test "returns an alert if there's a specially tagged alert with a PDF" do
alert =
Alerts.Alert.new(
header: "No timetable. #{TimetableBlocking.pdf_available_text()}",
informed_entity: [%Alerts.InformedEntity{route: @route.id}],
active_period: [{~U[2025-01-01T00:00:00Z], nil}],
url: "https://www.mbta.com/pdf-link"
)

assert TimetableBlocking.blocking_alert([alert], @route, @date) == alert
end

test "returns nil if there's no special alert" do
ie = %Alerts.InformedEntity{route: "CR-route"}
active_period = {~U[2025-01-01T00:00:00Z], ~U[2025-01-01T23:59:59Z]}

alerts = [
Alerts.Alert.new(
header: "Regular alert.",
informed_entity: [ie],
active_period: [active_period]
),
Alerts.Alert.new(
header: "Not active. #{TimetableBlocking.no_pdf_text()}",
informed_entity: [ie],
active_period: [{~U[2025-06-06T00:00:00Z], nil}]
),
Alerts.Alert.new(
header: "Different route. #{TimetableBlocking.no_pdf_text()}",
informed_entity: [%Alerts.InformedEntity{route: "bus"}],
active_period: [active_period]
)
]

assert TimetableBlocking.blocking_alert(alerts, @route, @date) == nil
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ defmodule DotcomWeb.ScheduleController.TimetableControllerTest do
use DotcomWeb.ConnCase, async: true
import DotcomWeb.ScheduleController.TimetableController
import Mox
alias Dotcom.TimetableBlocking
alias Routes.Route
alias Stops.Stop
alias Schedules.{Schedule, Trip}
Expand Down Expand Up @@ -333,4 +334,61 @@ defmodule DotcomWeb.ScheduleController.TimetableControllerTest do
)
end
end

describe "alert_blocks/2" do
setup %{conn: conn} do
conn =
conn
|> assign(:date, ~D[2025-01-01])
|> assign(:route, %Routes.Route{id: "CR-route"})

{:ok, %{conn: conn}}
end

test "assigns @blocking_alert if there's a specially tagged alert", %{conn: conn} do
alert =
Alerts.Alert.new(
header: "No timetable. #{TimetableBlocking.no_pdf_text()}",
informed_entity: [%Alerts.InformedEntity{route: conn.assigns.route.id}],
active_period: [{~U[2025-01-01T00:00:00Z], nil}]
)

conn =
conn
|> assign(:alerts, [alert])
|> alert_blocks([])

assert conn.assigns.blocking_alert == alert
end

test "assigns @blocking_alert to nil if there's no special alert", %{conn: conn} do
ie = %Alerts.InformedEntity{route: conn.assigns.route.id}
active_period = {~U[2025-01-01T00:00:00Z], ~U[2025-01-01T23:59:59Z]}

alerts = [
Alerts.Alert.new(
header: "Regular alert.",
informed_entity: [ie],
active_period: [active_period]
),
Alerts.Alert.new(
header: "Not active. #{TimetableBlocking.no_pdf_text()}",
informed_entity: [ie],
active_period: [{~U[2025-06-06T00:00:00Z], nil}]
),
Alerts.Alert.new(
header: "Different route. #{TimetableBlocking.no_pdf_text()}",
informed_entity: [%Alerts.InformedEntity{route: "bus"}],
active_period: [active_period]
)
]

conn =
conn
|> assign(:alerts, alerts)
|> alert_blocks([])

assert conn.assigns.blocking_alert == nil
end
end
end
Loading
Loading