From 0a8128e8b4eb7a9e058d56b5e70b9cf97c82d09b Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Fri, 13 Dec 2024 10:42:27 -0500 Subject: [PATCH] refactor(TransitDetail): use trip struct (#2260) * refactor(TransitDetail): use trip struct * cleanup: move route guard functions * refactor: better transit leg route/headsign text * format * update test to use trip headsigns * fix: improve flaky headsign tests * deps: bump open_trip_planner_client * feat: OTP gets trip direction_id * Update test/dotcom_web/live/trip_planner_test.exs Co-authored-by: Josh Larson * order the test itinerary start times --------- Co-authored-by: Josh Larson --- lib/dotcom/trip_plan/alerts.ex | 2 +- lib/dotcom/trip_plan/leg.ex | 4 +- lib/dotcom/trip_plan/parser.ex | 18 ++- lib/dotcom/trip_plan/related_link.ex | 8 +- lib/dotcom/trip_plan/transit_detail.ex | 4 +- lib/dotcom_web/components/route_symbols.ex | 18 +-- .../trip_planner/itinerary_detail.ex | 2 +- .../components/trip_planner/transit_leg.ex | 66 ++++++++- .../trip_planner_results_section.ex | 37 ++--- lib/routes/route.ex | 6 + mix.exs | 2 +- mix.lock | 2 +- .../trip_plan/itinerary_row_list_test.exs | 4 +- test/dotcom/trip_plan/itinerary_test.exs | 4 +- test/dotcom/trip_plan/leg_test.exs | 2 +- test/dotcom/trip_plan/related_link_test.exs | 2 - test/dotcom_web/live/trip_planner_test.exs | 132 +++++++++--------- test/dotcom_web/views/trip_plan_view_test.exs | 12 +- test/fares/fares_test.exs | 2 +- 19 files changed, 194 insertions(+), 133 deletions(-) diff --git a/lib/dotcom/trip_plan/alerts.ex b/lib/dotcom/trip_plan/alerts.ex index 6c0a033547..b4c0294002 100644 --- a/lib/dotcom/trip_plan/alerts.ex +++ b/lib/dotcom/trip_plan/alerts.ex @@ -42,7 +42,7 @@ defmodule Dotcom.TripPlan.Alerts do end end - defp mode_entities(%TransitDetail{route: route, trip_id: trip_id}) do + defp mode_entities(%TransitDetail{route: route, trip: %{id: trip_id}}) do trip = if is_nil(route.external_agency_name) do Schedules.Repo.trip(trip_id) diff --git a/lib/dotcom/trip_plan/leg.ex b/lib/dotcom/trip_plan/leg.ex index a814d15802..bc1796baf5 100644 --- a/lib/dotcom/trip_plan/leg.ex +++ b/lib/dotcom/trip_plan/leg.ex @@ -41,12 +41,12 @@ defmodule Dotcom.TripPlan.Leg do @doc "Returns the trip ID for the leg, if present" @spec trip_id(t) :: {:ok, Schedules.Trip.id_t()} | :error - def trip_id(%__MODULE__{mode: %TransitDetail{trip_id: trip_id}}), do: {:ok, trip_id} + def trip_id(%__MODULE__{mode: %TransitDetail{trip: %{id: trip_id}}}), do: {:ok, trip_id} def trip_id(%__MODULE__{}), do: :error @spec route_trip_ids(t) :: {:ok, {Routes.Route.id_t(), Schedules.Trip.id_t()}} | :error def route_trip_ids(%__MODULE__{mode: %TransitDetail{} = mode}) do - {:ok, {mode.route.id, mode.trip_id}} + {:ok, {mode.route.id, mode.trip.id}} end def route_trip_ids(%__MODULE__{}) do diff --git a/lib/dotcom/trip_plan/parser.ex b/lib/dotcom/trip_plan/parser.ex index 0db50e7f07..146296e340 100644 --- a/lib/dotcom/trip_plan/parser.ex +++ b/lib/dotcom/trip_plan/parser.ex @@ -103,7 +103,7 @@ defmodule Dotcom.TripPlan.Parser do mode: mode, intermediate_stops: Enum.map(stops, &build_stop/1), route: build_route(route, agency_name), - trip_id: id_from_gtfs(trip.gtfs_id) + trip: build_trip(trip) } end @@ -132,6 +132,22 @@ defmodule Dotcom.TripPlan.Parser do } end + defp build_trip(%Schema.Trip{ + direction_id: direction_id, + gtfs_id: gtfs_id, + trip_headsign: headsign, + trip_short_name: name + }) do + id = id_from_gtfs(gtfs_id) + + %Schedules.Trip{ + direction_id: if(direction_id in ["0", "1"], do: String.to_integer(direction_id)), + id: id, + name: name, + headsign: headsign + } + end + defp route_name("Logan Express", _short_name, long_name), do: "#{long_name} Logan Express" defp route_name("Massport", short_name, long_name) do diff --git a/lib/dotcom/trip_plan/related_link.ex b/lib/dotcom/trip_plan/related_link.ex index e74a8a3c8f..bc728f050b 100644 --- a/lib/dotcom/trip_plan/related_link.ex +++ b/lib/dotcom/trip_plan/related_link.ex @@ -77,9 +77,9 @@ defmodule Dotcom.TripPlan.RelatedLink do defp route_links(itinerary) do not_shuttle? = fn route -> route.description !== :rail_replacement_bus end - for %Leg{mode: %TransitDetail{route: route, trip_id: trip_id}} <- itinerary.legs, + for %Leg{mode: %TransitDetail{route: route, trip: trip}} <- itinerary.legs, not_shuttle?.(route) do - route_link(route, trip_id, itinerary) + route_link(route, if(trip, do: trip.id), itinerary) end end @@ -104,10 +104,10 @@ defmodule Dotcom.TripPlan.RelatedLink do new("Massport schedules", url, :massport_shuttle) end - defp route_link(route, trip_id, itinerary) do + defp route_link(route, _, itinerary) do icon_name = Route.icon_atom(route) date = Date.to_iso8601(itinerary.start) - url = schedule_path(DotcomWeb.Endpoint, :show, route, date: date, trip: trip_id) + url = schedule_path(DotcomWeb.Endpoint, :show, route, date: date) route |> link_text() diff --git a/lib/dotcom/trip_plan/transit_detail.ex b/lib/dotcom/trip_plan/transit_detail.ex index 536b2d7c9c..f036fc8ef4 100644 --- a/lib/dotcom/trip_plan/transit_detail.ex +++ b/lib/dotcom/trip_plan/transit_detail.ex @@ -14,14 +14,14 @@ defmodule Dotcom.TripPlan.TransitDetail do }, mode: :TRANSIT, route: nil, - trip_id: "", + trip: nil, intermediate_stops: [] @type t :: %__MODULE__{ fares: fares, mode: Leg.mode(), route: Routes.Route.t(), - trip_id: Schedules.Trip.id_t(), + trip: Schedules.Trip.t(), intermediate_stops: [Stops.Stop.t()] } diff --git a/lib/dotcom_web/components/route_symbols.ex b/lib/dotcom_web/components/route_symbols.ex index 3d922fc42e..1100502b9f 100644 --- a/lib/dotcom_web/components/route_symbols.ex +++ b/lib/dotcom_web/components/route_symbols.ex @@ -6,19 +6,13 @@ defmodule DotcomWeb.Components.RouteSymbols do use Phoenix.Component import MbtaMetro.Components.Icon + import Routes.Route, only: [is_external?: 1, is_shuttle?: 1] alias Routes.Route @logan_express_icon_names Route.logan_express_icon_names() @massport_icon_names Route.massport_icon_names() - defguardp is_external_route?(assigns) when not is_nil(assigns.route.external_agency_name) - - defguardp is_shuttle_route?(assigns) - when assigns.route.type == 3 and - assigns.route.description == :rail_replacement_bus and - not is_external_route?(assigns) - variant( :size, [ @@ -55,8 +49,8 @@ defmodule DotcomWeb.Components.RouteSymbols do <.route_symbol route={%Routes.Route{type: 3}} data-toggle="tooltip" /> ``` """ - def route_symbol(%{route: %Route{description: description, type: 3}} = assigns) - when not is_external_route?(assigns) and not is_shuttle_route?(assigns) and + def route_symbol(%{route: %Route{description: description, type: 3} = route} = assigns) + when not is_external?(route) and not is_shuttle?(route) and description != :rapid_transit do route_class = if(Routes.Route.silver_line?(assigns.route), @@ -88,8 +82,8 @@ defmodule DotcomWeb.Components.RouteSymbols do attr(:rest, :global) attr(:route, Routes.Route, required: true) - def route_icon(%{route: %Route{name: shuttle_name}} = assigns) - when is_shuttle_route?(assigns) do + def route_icon(%{route: %Route{name: shuttle_name} = route} = assigns) + when is_shuttle?(route) do route_class = shuttle_name |> String.replace(" Shuttle", "") @@ -110,7 +104,7 @@ defmodule DotcomWeb.Components.RouteSymbols do """ end - def route_icon(assigns) when not is_external_route?(assigns) do + def route_icon(%{route: route} = assigns) when not is_external?(route) do assigns = assigns |> assign_new(:icon_name, fn %{route: route, size: size} -> diff --git a/lib/dotcom_web/components/trip_planner/itinerary_detail.ex b/lib/dotcom_web/components/trip_planner/itinerary_detail.ex index ff7cd75721..a62e6eb92d 100644 --- a/lib/dotcom_web/components/trip_planner/itinerary_detail.ex +++ b/lib/dotcom_web/components/trip_planner/itinerary_detail.ex @@ -39,7 +39,7 @@ defmodule DotcomWeb.Components.TripPlanner.ItineraryDetail do ~H"""
1}>

Depart at

-
+
<.depart_at_button :for={{itinerary, index} <- Enum.with_index(@itineraries)} active={@selected_itinerary_detail_index == index} diff --git a/lib/dotcom_web/components/trip_planner/transit_leg.ex b/lib/dotcom_web/components/trip_planner/transit_leg.ex index 4e6107288a..13d03f5ba0 100644 --- a/lib/dotcom_web/components/trip_planner/transit_leg.ex +++ b/lib/dotcom_web/components/trip_planner/transit_leg.ex @@ -9,6 +9,7 @@ defmodule DotcomWeb.Components.TripPlanner.TransitLeg do import DotcomWeb.Components.RouteSymbols, only: [route_symbol: 1] import DotcomWeb.Components.TripPlanner.Place import MbtaMetro.Components.Icon, only: [icon: 1] + import Routes.Route, only: [is_external?: 1, is_shuttle?: 1] alias Dotcom.TripPlan.TransitDetail alias Routes.Route @@ -74,31 +75,67 @@ defmodule DotcomWeb.Components.TripPlanner.TransitLeg do defp leg_line_class(_), do: "" defp leg_summary(assigns) do - assigns = assign(assigns, :stops_count, Enum.count(assigns.leg.mode.intermediate_stops) + 1) + assigns = + assigns + |> assign(:stops_count, Enum.count(assigns.leg.mode.intermediate_stops) + 1) + |> assign(:headsign, headsign(assigns.leg.mode)) ~H"""
<.route_symbol route={@leg.mode.route} /> - {@leg.mode.trip_id} + {@headsign}
- Ride the {route_name(@leg.mode.route)} + <.ride_message mode={@leg.mode} /> {@stops_count} {Inflex.inflect("stop", @stops_count)}
""" end + # Massport trips might not have headsigns, so use the route names instead + defp headsign(%{route: %Route{} = route}) when is_external?(route) do + route_name(route) + end + + defp headsign(%{trip: %{headsign: headsign}}) when not is_nil(headsign) do + headsign + end + + defp headsign(_), do: nil + + defp ride_message(%{mode: %{route: %Route{} = route}} = assigns) + when is_external?(route) do + assigns = + assigns + |> assign(:vehicle_name, vehicle_name(route)) + + ~H""" + Ride the {@vehicle_name} + """ + end + + defp ride_message(%{mode: %{route: route, trip: trip}} = assigns) do + assigns = + assigns + |> assign(:route_name, route_name(route)) + |> assign(:train_number, train_number(trip)) + |> assign(:vehicle_name, vehicle_name(route)) + + ~H""" + Ride the {@route_name} {if @train_number, do: "Train #{@train_number}", else: @vehicle_name} + """ + end + # Returns the name of the route for the given row. # If there is an external agency name, we use the long name. # If it is a bus, we use the short name. # For all others, we use the long name. - defp route_name(%Route{external_agency_name: agency, long_name: long_name}) - when is_binary(agency) and is_binary(long_name), - do: long_name + defp route_name(%Route{} = route) when is_external?(route), + do: route.long_name defp route_name(%Route{name: name, type: 3}) when is_binary(name), - do: "#{name} bus" + do: name defp route_name(%Route{long_name: long_name}) when is_binary(long_name), @@ -107,6 +144,21 @@ defmodule DotcomWeb.Components.TripPlanner.TransitLeg do defp route_name(%Route{name: name}), do: name defp route_name(_), do: nil + defp train_number(%Schedules.Trip{name: name}) when not is_nil(name), do: name + defp train_number(_), do: nil + + defp vehicle_name(%Route{} = route) when is_shuttle?(route) do + "shuttle bus" + end + + defp vehicle_name(%Route{} = route) do + route + |> Route.vehicle_name() + |> String.downcase() + end + + defp vehicle_name(nil), do: nil + defp leg_details(assigns) do ~H"""
    diff --git a/lib/dotcom_web/components/trip_planner/trip_planner_results_section.ex b/lib/dotcom_web/components/trip_planner/trip_planner_results_section.ex index 0021e35906..240a526cc5 100644 --- a/lib/dotcom_web/components/trip_planner/trip_planner_results_section.ex +++ b/lib/dotcom_web/components/trip_planner/trip_planner_results_section.ex @@ -97,26 +97,27 @@ defmodule DotcomWeb.Components.TripPlanner.TripPlannerResultsSection do }} } = assigns ) do - %{itineraries: itineraries, summary: summary} = results |> Enum.at(itinerary_group_index) - - assigns = - assigns - |> assign(:itineraries, itineraries) - |> assign(:summary, summary) - |> assign(:itinerary_index, itinerary_index) + with %{itineraries: itineraries, summary: summary} <- + results |> Enum.at(itinerary_group_index) do + assigns = + assigns + |> assign(:itineraries, itineraries) + |> assign(:summary, summary) + |> assign(:itinerary_index, itinerary_index) + + ~H""" +
    +
    + <.itinerary_summary summary={@summary} /> +
    - ~H""" -
    -
    - <.itinerary_summary summary={@summary} /> + <.itinerary_detail + itineraries={@itineraries} + selected_itinerary_detail_index={@itinerary_index} + />
    - - <.itinerary_detail - itineraries={@itineraries} - selected_itinerary_detail_index={@itinerary_index} - /> -
    - """ + """ + end end defp itinerary_panel(assigns) do diff --git a/lib/routes/route.ex b/lib/routes/route.ex index 9b337e8b67..16097c7349 100644 --- a/lib/routes/route.ex +++ b/lib/routes/route.ex @@ -92,6 +92,12 @@ defmodule Routes.Route do @silver_line ~w(741 742 743 746 749 751) @silver_line_set MapSet.new(@silver_line) + defguard is_external?(route) when not is_nil(route.external_agency_name) + + defguard is_shuttle?(route) + when route.type == 3 and route.description == :rail_replacement_bus and + not is_external?(route) + def logan_express_icon_names, do: @logan_express_icon_names def massport_icon_names, do: @massport_icon_names diff --git a/mix.exs b/mix.exs index 1fc0a88015..d212581ddb 100644 --- a/mix.exs +++ b/mix.exs @@ -113,7 +113,7 @@ defmodule DotCom.Mixfile do {:nebulex_redis_adapter, "2.4.1"}, { :open_trip_planner_client, - [github: "mbta/open_trip_planner_client", tag: "v0.11.2"] + [github: "mbta/open_trip_planner_client", tag: "v0.11.3"] }, {:parallel_stream, "1.1.0"}, # latest version 1.7.14 diff --git a/mix.lock b/mix.lock index d9c6680621..910b127976 100644 --- a/mix.lock +++ b/mix.lock @@ -72,7 +72,7 @@ "nimble_ownership": {:hex, :nimble_ownership, "1.0.0", "3f87744d42c21b2042a0aa1d48c83c77e6dd9dd357e425a038dd4b49ba8b79a1", [:mix], [], "hexpm", "7c16cc74f4e952464220a73055b557a273e8b1b7ace8489ec9d86e9ad56cb2cc"}, "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"}, "nimble_pool": {:hex, :nimble_pool, "1.1.0", "bf9c29fbdcba3564a8b800d1eeb5a3c58f36e1e11d7b7fb2e084a643f645f06b", [:mix], [], "hexpm", "af2e4e6b34197db81f7aad230c1118eac993acc0dae6bc83bac0126d4ae0813a"}, - "open_trip_planner_client": {:git, "https://github.com/mbta/open_trip_planner_client.git", "3bf74242bda2c4174bbbc48363decd864562d0e9", [tag: "v0.11.2"]}, + "open_trip_planner_client": {:git, "https://github.com/mbta/open_trip_planner_client.git", "b127ccfcc7cdfc40bc91e30c55d41754426a7cc9", [tag: "v0.11.3"]}, "parallel_stream": {:hex, :parallel_stream, "1.1.0", "f52f73eb344bc22de335992377413138405796e0d0ad99d995d9977ac29f1ca9", [:mix], [], "hexpm", "684fd19191aedfaf387bbabbeb8ff3c752f0220c8112eb907d797f4592d6e871"}, "parse_trans": {:hex, :parse_trans, "3.4.1", "6e6aa8167cb44cc8f39441d05193be6e6f4e7c2946cb2759f015f8c56b76e5ff", [:rebar3], [], "hexpm", "620a406ce75dada827b82e453c19cf06776be266f5a67cff34e1ef2cbb60e49a"}, "phoenix": {:hex, :phoenix, "1.7.14", "a7d0b3f1bc95987044ddada111e77bd7f75646a08518942c72a8440278ae7825", [:mix], [{:castore, ">= 0.0.0", [hex: :castore, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.1", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.7", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:websock_adapter, "~> 0.5.3", [hex: :websock_adapter, repo: "hexpm", optional: false]}], "hexpm", "c7859bc56cc5dfef19ecfc240775dae358cbaa530231118a9e014df392ace61a"}, diff --git a/test/dotcom/trip_plan/itinerary_row_list_test.exs b/test/dotcom/trip_plan/itinerary_row_list_test.exs index 42c7ae4bf7..9638bbb5bb 100644 --- a/test/dotcom/trip_plan/itinerary_row_list_test.exs +++ b/test/dotcom/trip_plan/itinerary_row_list_test.exs @@ -179,7 +179,7 @@ defmodule Dotcom.TripPlan.ItineraryRowListTest do to: %NamedPosition{stop: %Stops.Stop{id: "place-pktrm"}, name: "Park Street"}, mode: %TransitDetail{ route: %Routes.Route{id: "Green-C"}, - trip_id: "Green-1", + trip: %Schedules.Trip{id: "Green-1"}, intermediate_stops: [] } } @@ -248,7 +248,7 @@ defmodule Dotcom.TripPlan.ItineraryRowListTest do to: %NamedPosition{stop: %Stops.Stop{id: "place-kencl"}, name: "Kenmore"}, mode: %TransitDetail{ route: %Routes.Route{id: "Green-C"}, - trip_id: "Green-1", + trip: %Schedules.Trip{id: "Green-1"}, intermediate_stops: [] } } diff --git a/test/dotcom/trip_plan/itinerary_test.exs b/test/dotcom/trip_plan/itinerary_test.exs index 6d642ef9fe..835e0def0c 100644 --- a/test/dotcom/trip_plan/itinerary_test.exs +++ b/test/dotcom/trip_plan/itinerary_test.exs @@ -54,7 +54,7 @@ defmodule Dotcom.TripPlan.ItineraryTest do test_calculated_ids = Enum.flat_map(itinerary, fn leg -> case leg.mode do - %TransitDetail{trip_id: trip_id} -> [trip_id] + %TransitDetail{trip: %{id: trip_id}} -> [trip_id] _ -> [] end end) @@ -68,7 +68,7 @@ defmodule Dotcom.TripPlan.ItineraryTest do test_calculated_ids = Enum.flat_map(itinerary.legs, fn leg -> case leg.mode do - %TransitDetail{} = td -> [{td.route.id, td.trip_id}] + %TransitDetail{} = td -> [{td.route.id, td.trip.id}] _ -> [] end end) diff --git a/test/dotcom/trip_plan/leg_test.exs b/test/dotcom/trip_plan/leg_test.exs index 92ad19195f..ad75bce9dd 100644 --- a/test/dotcom/trip_plan/leg_test.exs +++ b/test/dotcom/trip_plan/leg_test.exs @@ -48,7 +48,7 @@ defmodule Dotcom.TripPlan.LegTest do describe "trip_id/1" do test "returns {:ok, id} for a transit leg", context do - trip_id = context.transit_leg.mode.trip_id + trip_id = context.transit_leg.mode.trip.id assert {:ok, ^trip_id} = trip_id(context.transit_leg) end diff --git a/test/dotcom/trip_plan/related_link_test.exs b/test/dotcom/trip_plan/related_link_test.exs index 21f89a5a97..ea5ac96a94 100644 --- a/test/dotcom/trip_plan/related_link_test.exs +++ b/test/dotcom/trip_plan/related_link_test.exs @@ -35,10 +35,8 @@ defmodule Dotcom.TripPlan.RelatedLinkTest do fn id -> %Stops.Stop{id: id} end ) - [trip_id] = Itinerary.trip_ids(itinerary) assert [route_link, fare_link] = links_for_itinerary(itinerary) assert url(route_link) =~ Timex.format!(itinerary.start, "date={ISOdate}") - assert url(route_link) =~ ~s(trip=#{trip_id}) |> URI.encode() assert fare_link.text == "View fare information" if route.type == 3 do diff --git a/test/dotcom_web/live/trip_planner_test.exs b/test/dotcom_web/live/trip_planner_test.exs index a27327cbe2..091e2e8607 100644 --- a/test/dotcom_web/live/trip_planner_test.exs +++ b/test/dotcom_web/live/trip_planner_test.exs @@ -4,6 +4,7 @@ defmodule DotcomWeb.Live.TripPlannerTest do import Mox import Phoenix.LiveViewTest + alias OpenTripPlannerClient.Test.Support.Factory alias Test.Support.Factories.TripPlanner.TripPlanner, as: TripPlannerFactory setup :verify_on_exit! @@ -20,19 +21,36 @@ defmodule DotcomWeb.Live.TripPlannerTest do stub_otp_results(itineraries) end - defp update_trip_id(itinerary, trip_id) do - updated_transit_leg = - itinerary.legs - |> Enum.at(1) - |> update_in([:trip, :gtfs_id], fn _ -> "ma_mbta_us:#{trip_id}" end) + # For a list of headsigns, create a bunch of itineraries that would be grouped + # by the Trip Planner's logic + defp grouped_itineraries_from_headsigns([initial_headsign | _] = headsigns) do + # Only MBTA transit legs show the headsigns right now, so ensure the + # generated legs are MBTA-only + base_leg = + Factory.build(:transit_leg, %{ + agency: Factory.build(:agency, %{name: "MBTA"}), + route: + Factory.build(:route, %{gtfs_id: "mbta-ma-us:internal", type: Faker.Util.pick(0..4)}), + trip: + Factory.build(:trip, %{ + direction_id: Faker.Util.pick(["0", "1"]), + trip_headsign: initial_headsign + }) + }) + + base_itinerary = Factory.build(:itinerary, legs: [base_leg]) + + headsigns + |> Enum.with_index() + |> Enum.map(fn {headsign, index} -> + leg = update_in(base_leg, [:trip, :trip_headsign], fn _ -> headsign end) - itinerary - |> Map.update!(:legs, &List.replace_at(&1, 1, updated_transit_leg)) - end - - defp update_start_time(itinerary, start_time) do - itinerary - |> Map.put(:start, DateTime.new!(Date.utc_today(), start_time, "America/New_York")) + %{ + base_itinerary + | legs: [leg], + start: Timex.shift(base_itinerary.start, minutes: 10 * index) + } + end) end test "Preview version behind basic auth", %{conn: conn} do @@ -199,26 +217,15 @@ defmodule DotcomWeb.Live.TripPlannerTest do conn: conn, params: params } do - trip_datetime_1 = Faker.DateTime.forward(2) - trip_time_1 = trip_datetime_1 |> DateTime.to_time() - trip_id_1 = Faker.UUID.v4() - - trip_datetime_2 = trip_datetime_1 |> DateTime.shift(hour: 1) - trip_time_2 = trip_datetime_2 |> DateTime.to_time() - trip_time_display_2 = trip_time_2 |> Timex.format!("%-I:%M", :strftime) - trip_id_2 = Faker.UUID.v4() - - base_itinerary = TripPlannerFactory.build(:otp_itinerary) - - # Right now, the headsign (which is what we actually want to - # show) is not available from OTP client, but we're rendering - # the trip ID in its place. Once the headsign is available, we - # should update these updates and the assertions below to use - # the headsign instead of the trip ID. - stub_otp_results([ - base_itinerary |> update_trip_id(trip_id_1) |> update_start_time(trip_time_1), - base_itinerary |> update_trip_id(trip_id_2) |> update_start_time(trip_time_2) - ]) + trip_headsign_1 = "Headsign:#{Faker.App.name()}" + trip_headsign_2 = "Headsign:#{Faker.App.name()}" + + expect(OpenTripPlannerClient.Mock, :plan, fn _ -> + {:ok, + %OpenTripPlannerClient.Plan{ + itineraries: grouped_itineraries_from_headsigns([trip_headsign_1, trip_headsign_2]) + }} + end) {:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}") @@ -226,27 +233,25 @@ defmodule DotcomWeb.Live.TripPlannerTest do view |> element("button", "Details") |> render_click() - assert render_async(view) =~ trip_id_1 - refute render_async(view) =~ trip_id_2 + assert render_async(view) =~ trip_headsign_1 + refute render_async(view) =~ trip_headsign_2 - view |> element("button", trip_time_display_2) |> render_click() + view |> element("#itinerary-detail-departure-times button:last-child") |> render_click() - assert render_async(view) =~ trip_id_2 - refute render_async(view) =~ trip_id_1 + assert render_async(view) =~ trip_headsign_2 + refute render_async(view) =~ trip_headsign_1 end test "'Depart At' buttons don't appear if there would only be one", %{ conn: conn, params: params } do - trip_time_1 = Faker.DateTime.forward(2) |> DateTime.to_time() - trip_time_display_1 = trip_time_1 |> Timex.format!("%-I:%M", :strftime) - - base_itinerary = TripPlannerFactory.build(:otp_itinerary) - - stub_otp_results([ - base_itinerary |> update_start_time(trip_time_1) - ]) + expect(OpenTripPlannerClient.Mock, :plan, fn _ -> + {:ok, + %OpenTripPlannerClient.Plan{ + itineraries: TripPlannerFactory.build_list(1, :otp_itinerary) + }} + end) {:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}") @@ -254,45 +259,34 @@ defmodule DotcomWeb.Live.TripPlannerTest do view |> element("button", "Details") |> render_click() - refute view |> element("button", trip_time_display_1) |> has_element?() + refute view |> element("#itinerary-detail-departure-times") |> has_element?() end test "'Depart At' button state is not preserved when leaving details view", %{ conn: conn, params: params } do - trip_datetime_1 = Faker.DateTime.forward(2) - trip_time_1 = trip_datetime_1 |> DateTime.to_time() - trip_id_1 = Faker.UUID.v4() - - trip_datetime_2 = trip_datetime_1 |> DateTime.shift(hour: 1) - trip_time_2 = trip_datetime_2 |> DateTime.to_time() - trip_time_display_2 = trip_time_2 |> Timex.format!("%-I:%M", :strftime) - trip_id_2 = Faker.UUID.v4() - - base_itinerary = TripPlannerFactory.build(:otp_itinerary) - - # Right now, the headsign (which is what we actually want to - # show) is not available from OTP client, but we're rendering - # the trip ID in its place. Once the headsign is available, we - # should update these updates and the assertions below to use - # the headsign instead of the trip ID. - stub_otp_results([ - base_itinerary |> update_trip_id(trip_id_1) |> update_start_time(trip_time_1), - base_itinerary |> update_trip_id(trip_id_2) |> update_start_time(trip_time_2) - ]) + trip_headsign_1 = "Headsign:#{Faker.App.name()}" + trip_headsign_2 = "Headsign:#{Faker.App.name()}" + + expect(OpenTripPlannerClient.Mock, :plan, fn _ -> + {:ok, + %OpenTripPlannerClient.Plan{ + itineraries: grouped_itineraries_from_headsigns([trip_headsign_1, trip_headsign_2]) + }} + end) {:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}") render_async(view) view |> element("button", "Details") |> render_click() - view |> element("button", trip_time_display_2) |> render_click() + view |> element("#itinerary-detail-departure-times button:last-child") |> render_click() view |> element("button", "View All Options") |> render_click() view |> element("button", "Details") |> render_click() - assert render_async(view) =~ trip_id_1 - refute render_async(view) =~ trip_id_2 + assert render_async(view) =~ trip_headsign_1 + refute render_async(view) =~ trip_headsign_2 end test "displays error message from the Open Trip Planner client", %{conn: conn, params: params} do diff --git a/test/dotcom_web/views/trip_plan_view_test.exs b/test/dotcom_web/views/trip_plan_view_test.exs index dbbdabe74c..9018c5c8e2 100644 --- a/test/dotcom_web/views/trip_plan_view_test.exs +++ b/test/dotcom_web/views/trip_plan_view_test.exs @@ -925,7 +925,7 @@ closest arrival to 12:00 AM, Thursday, January 1st." fares: @fares, intermediate_stops: [%Stops.Stop{id: "70071"}, %Stops.Stop{id: "70073"}], route: %Routes.Route{id: "Red"}, - trip_id: "43870769C0" + trip: %Schedules.Trip{id: "43870769C0"} }, to: %NamedPosition{ latitude: 42.356395, @@ -1073,7 +1073,7 @@ closest arrival to 12:00 AM, Thursday, January 1st." fares: @fares, intermediate_stops: [%Stops.Stop{id: "70071"}, %Stops.Stop{id: "70073"}], route: %Routes.Route{id: "Red"}, - trip_id: "43870769C0" + trip: %Schedules.Trip{id: "43870769C0"} }, to: %NamedPosition{ latitude: 42.356395, @@ -1099,7 +1099,7 @@ closest arrival to 12:00 AM, Thursday, January 1st." %Stops.Stop{id: "80"} ], route: %Routes.Route{id: "1"}, - trip_id: "44170977" + trip: %Schedules.Trip{id: "44170977"} }, to: %NamedPosition{ latitude: 42.342478, @@ -1119,7 +1119,7 @@ closest arrival to 12:00 AM, Thursday, January 1st." fares: @fares, intermediate_stops: [%Stops.Stop{id: "70071"}, %Stops.Stop{id: "70073"}], route: %Routes.Route{id: "Red"}, - trip_id: "43870769C0" + trip: %Schedules.Trip{id: "43870769C0"} }, to: %NamedPosition{ latitude: 42.356395, @@ -1343,7 +1343,7 @@ closest arrival to 12:00 AM, Thursday, January 1st." }, intermediate_stops: [%Stops.Stop{id: "70071"}, %Stops.Stop{id: "70073"}], route: %Routes.Route{id: "Red"}, - trip_id: "43870769C0" + trip: %Schedules.Trip{id: "43870769C0"} }, to: %NamedPosition{ latitude: 42.356395, @@ -1528,7 +1528,7 @@ closest arrival to 12:00 AM, Thursday, January 1st." %Stops.Stop{id: "74616"} ], route: %Routes.Route{id: "741"}, - trip_id: "44812009" + trip: %Schedules.Trip{id: "44812009"} }, to: %NamedPosition{ latitude: 42.352271, diff --git a/test/fares/fares_test.exs b/test/fares/fares_test.exs index 5182ac405a..a5605cb76b 100644 --- a/test/fares/fares_test.exs +++ b/test/fares/fares_test.exs @@ -245,7 +245,7 @@ defmodule FaresTest do }, intermediate_stops: [%Stops.Stop{id: "70071"}, %Stops.Stop{id: "70073"}], route: %Routes.Route{id: "Red"}, - trip_id: "43870769C0" + trip: %Schedules.Trip{id: "43870769C0"} }, to: %NamedPosition{ latitude: 42.356395,