From 5ab18814ce2a45d75b9fd1b65bbac48436c1cc5e Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Tue, 3 Dec 2024 12:09:57 -0500 Subject: [PATCH] feat: Show "Depart at" buttons and show the correct itinerary on click (#2238) * prefactor: Convert <.itinerary_detail /> into a live component And change its input from `itinerary` to `itineraries` so that it can be responsible for choosing which itinerary to display * feat: Show "Depart at" buttons and show the correct itinerary on click * style: Style depart-at buttons correctly (entirely with Tailwind) * refactor: Extract <.depart_at_button /> as a sub-component * style: Format the "Depart at" text correctly * cleanup: Remove some useless assigns * tests: Add tests that validate itinerary-toggling behavior * refactor/cleanup: Rename selected_trip_index to selected_itinerary_index * fix: Remove TODO tag, which was causing Credo to fail * refactor: Convert itinerary_details to a function component * tests: Co-locate exlicit testing values * cleanup: Make test helper functions private * fix: Don't preserve selected_itinerary_detail_index when hiding and re-showing an itinerary * tests: Use existing factory for otp_itineraries rather than calling limit_route_types each time * cleanup: Use faker for trip times and trip ID's in tests --- assets/tailwind.config.js | 3 +- .../trip_planner_results_section.ex | 24 +++- .../trip_planner/itinerary_detail.ex | 64 ++++++++- test/dotcom_web/live/trip_planner_test.exs | 123 ++++++++++++++++-- .../factories/trip_planner/trip_planner.ex | 5 + 5 files changed, 196 insertions(+), 23 deletions(-) diff --git a/assets/tailwind.config.js b/assets/tailwind.config.js index be9b51dea7..e12c5e97f2 100644 --- a/assets/tailwind.config.js +++ b/assets/tailwind.config.js @@ -42,7 +42,8 @@ module.exports = { }, "brand-primary": { DEFAULT: "#165c96", - darkest: "#0b2f4c" + darkest: "#0b2f4c", + lightest: "#cee0f4" }, "logan-express": { BB: "#f16823", diff --git a/lib/dotcom_web/components/live_components/trip_planner_results_section.ex b/lib/dotcom_web/components/live_components/trip_planner_results_section.ex index 744336336d..321f041078 100644 --- a/lib/dotcom_web/components/live_components/trip_planner_results_section.ex +++ b/lib/dotcom_web/components/live_components/trip_planner_results_section.ex @@ -10,7 +10,10 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerResultsSection do @impl true def mount(socket) do - {:ok, socket |> assign(:expanded_itinerary_index, nil)} + {:ok, + socket + |> assign(:expanded_itinerary_index, nil) + |> assign(:selected_itinerary_detail_index, 0)} end @impl true @@ -64,6 +67,7 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerResultsSection do <.itinerary_panel results={results} details_index={@expanded_itinerary_index} + selected_itinerary_detail_index={@selected_itinerary_detail_index} target={@myself} /> @@ -117,7 +121,11 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerResultsSection do <.itinerary_summary summary={@summary} /> - <.itinerary_detail :for={itinerary <- @itineraries} itinerary={itinerary} /> + <.itinerary_detail + itineraries={@itineraries} + selected_itinerary_detail_index={@selected_itinerary_detail_index} + target={@target} + /> """ end @@ -141,7 +149,17 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerResultsSection do _ -> nil end - {:noreply, socket |> assign(:expanded_itinerary_index, index)} + {:noreply, + socket + |> assign(:expanded_itinerary_index, index) + |> assign(:selected_itinerary_detail_index, 0)} + end + + @impl true + def handle_event("set_selected_itinerary_detail_index", %{"trip-index" => index_str}, socket) do + {index, ""} = Integer.parse(index_str) + + {:noreply, socket |> assign(:selected_itinerary_detail_index, index)} end defp format_datetime_short(datetime) do diff --git a/lib/dotcom_web/components/trip_planner/itinerary_detail.ex b/lib/dotcom_web/components/trip_planner/itinerary_detail.ex index ae240f1ce7..0cef0b9786 100644 --- a/lib/dotcom_web/components/trip_planner/itinerary_detail.ex +++ b/lib/dotcom_web/components/trip_planner/itinerary_detail.ex @@ -1,14 +1,66 @@ defmodule DotcomWeb.Components.TripPlanner.ItineraryDetail do @moduledoc """ - A component to render an itinerary in detail.. + The section of the trip planner page that shows the map and + the summary or details panel """ + use DotcomWeb, :component import DotcomWeb.Components.TripPlanner.Leg alias Dotcom.TripPlan.PersonalDetail - def itinerary_detail(assigns) do + def itinerary_detail( + %{ + itineraries: itineraries, + selected_itinerary_detail_index: selected_itinerary_detail_index + } = assigns + ) do + assigns = + assign(assigns, :selected_itinerary, Enum.at(itineraries, selected_itinerary_detail_index)) + + ~H""" +
+

Depart at

+
+ <.depart_at_button + :for={{itinerary, index} <- Enum.with_index(@itineraries)} + active={@selected_itinerary_detail_index == index} + phx-click="set_selected_itinerary_detail_index" + phx-value-trip-index={index} + phx-target={@target} + > + <%= Timex.format!(itinerary.start, "%-I:%M%p", :strftime) %> + +
+ <.specific_itinerary_detail itinerary={@selected_itinerary} /> +
+ """ + end + + attr :active, :boolean + attr :rest, :global + slot :inner_block + + defp depart_at_button(%{active: active} = assigns) do + background_class = if active, do: "bg-brand-primary-lightest", else: "bg-transparent" + assigns = assign(assigns, :background_class, background_class) + + ~H""" + + """ + end + + defp specific_itinerary_detail(assigns) do assigns = assign( assigns, @@ -19,11 +71,11 @@ defmodule DotcomWeb.Components.TripPlanner.ItineraryDetail do ) ~H""" -
- +
+
Depart at <%= Timex.format!(@itinerary.start, "%-I:%M%p", :strftime) %> <.route_symbol :for={route <- @all_routes} route={route} class="ml-2" /> -
+
<.leg start_time={leg.start} @@ -35,7 +87,7 @@ defmodule DotcomWeb.Components.TripPlanner.ItineraryDetail do realtime_state={leg.realtime_state} />
-
+ """ end end diff --git a/test/dotcom_web/live/trip_planner_test.exs b/test/dotcom_web/live/trip_planner_test.exs index d01b899ebb..2d21d4a3db 100644 --- a/test/dotcom_web/live/trip_planner_test.exs +++ b/test/dotcom_web/live/trip_planner_test.exs @@ -4,8 +4,33 @@ defmodule DotcomWeb.Live.TripPlannerTest do import Mox import Phoenix.LiveViewTest + alias Test.Support.Factories.TripPlanner.TripPlanner, as: TripPlannerFactory + setup :verify_on_exit! + defp stub_otp_results(itineraries) do + expect(OpenTripPlannerClient.Mock, :plan, fn _ -> + {:ok, %OpenTripPlannerClient.Plan{itineraries: itineraries}} + end) + end + + defp stub_populated_otp_results() do + itineraries = TripPlannerFactory.build_list(3, :otp_itinerary) + + stub_otp_results(itineraries) + end + + defp update_trip_details(itinerary, trip_id: trip_id, start_time: start_time) do + updated_transit_leg = + itinerary.legs + |> Enum.at(1) + |> update_in([:trip, :gtfs_id], fn _ -> "ma_mbta_us:#{trip_id}" end) + + itinerary + |> Map.update!(:legs, &List.replace_at(&1, 1, updated_transit_leg)) + |> Map.put(:start, DateTime.new!(Date.utc_today(), start_time, "America/New_York")) + end + test "Preview version behind basic auth", %{conn: conn} do conn = get(conn, ~p"/preview/trip-planner") @@ -96,9 +121,7 @@ defmodule DotcomWeb.Live.TripPlannerTest do } } - expect(OpenTripPlannerClient.Mock, :plan, fn _ -> - {:ok, %OpenTripPlannerClient.Plan{itineraries: []}} - end) + stub_otp_results([]) {:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}") @@ -115,16 +138,6 @@ defmodule DotcomWeb.Live.TripPlannerTest do Test.Support.Factories.Stops.Stop.build(:stop) end) - # Uhhh the OTP factory will generate with any route_type value but our - # parsing will break with unexpected route types - itineraries = - OpenTripPlannerClient.Test.Support.Factory.build_list(3, :itinerary) - |> Enum.map(&Test.Support.Factories.TripPlanner.TripPlanner.limit_route_types/1) - - expect(OpenTripPlannerClient.Mock, :plan, fn _ -> - {:ok, %OpenTripPlannerClient.Plan{itineraries: itineraries}} - end) - %{ conn: put_req_header( @@ -144,12 +157,16 @@ defmodule DotcomWeb.Live.TripPlannerTest do end test "starts out with no 'View All Options' button", %{conn: conn, params: params} do + stub_populated_otp_results() + {:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}") refute render_async(view) =~ "View All Options" end test "clicking 'Details' button opens details view", %{conn: conn, params: params} do + stub_populated_otp_results() + {:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}") render_async(view) @@ -162,6 +179,8 @@ defmodule DotcomWeb.Live.TripPlannerTest do conn: conn, params: params } do + stub_populated_otp_results() + {:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}") render_async(view) @@ -171,5 +190,83 @@ defmodule DotcomWeb.Live.TripPlannerTest do refute render_async(view) =~ "View All Options" end + + test "'Depart At' buttons toggle which itinerary to show", %{ + 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([ + update_trip_details(base_itinerary, trip_id: trip_id_1, start_time: trip_time_1), + update_trip_details(base_itinerary, trip_id: trip_id_2, start_time: trip_time_2) + ]) + + {:ok, view, _html} = live(conn, ~p"/preview/trip-planner?#{params}") + + render_async(view) + + view |> element("button", "Details") |> render_click() + + assert render_async(view) =~ trip_id_1 + refute render_async(view) =~ trip_id_2 + + view |> element("button", trip_time_display_2) |> render_click() + + assert render_async(view) =~ trip_id_2 + refute render_async(view) =~ trip_id_1 + 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([ + update_trip_details(base_itinerary, trip_id: trip_id_1, start_time: trip_time_1), + update_trip_details(base_itinerary, trip_id: trip_id_2, start_time: trip_time_2) + ]) + + {: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("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 + end end end diff --git a/test/support/factories/trip_planner/trip_planner.ex b/test/support/factories/trip_planner/trip_planner.ex index 8c8858cd79..bfcfe75ff6 100644 --- a/test/support/factories/trip_planner/trip_planner.ex +++ b/test/support/factories/trip_planner/trip_planner.ex @@ -13,6 +13,11 @@ defmodule Test.Support.Factories.TripPlanner.TripPlanner do |> Parser.parse() end + def otp_itinerary_factory do + Factory.build(:itinerary) + |> limit_route_types() + end + def leg_factory do [:walking_leg, :transit_leg] |> Faker.Util.pick()