refactor(TripPlan.ItineraryGroups): improve handling for "arrive by" selection

lib/dotcom/trip_plan/itinerary_group.ex

@@ -0,0 +1,74 @@
+defmodule Dotcom.TripPlan.ItineraryGroup do
+  @moduledoc """
+  A single group of related itineraries
+  """
+
+  alias Dotcom.TripPlan.Itinerary
+
+  defstruct [:itineraries, :representative_index, :representative_time, :summary]
+
+  @type summarized_leg :: %{
+          routes: [Routes.Route.t()],
+          walk_minutes: non_neg_integer()
+        }
+
+  @type summary :: %{
+          accessible?: boolean() | nil,
+          duration: non_neg_integer(),
+          start: DateTime.t(),
+          stop: DateTime.t(),
+          summarized_legs: [summarized_leg()],
+          tag: String.t(),
+          total_cost: non_neg_integer(),
+          walk_distance: float()
+        }
+
+  @type t :: %__MODULE__{
+          itineraries: [Itinerary.t()],
+          representative_index: non_neg_integer(),
+          representative_time: :start | :stop,
+          summary: summary()
+        }
+
+  @doc """
+  List of either start times or stop times for this group
+  """
+  @spec options_text(t()) :: String.t() | nil
+  def options_text(%__MODULE__{itineraries: []}), do: nil
+  def options_text(%__MODULE__{itineraries: [_single]}), do: nil
+
+  def options_text(
+        %__MODULE__{
+          representative_index: representative_index,
+          representative_time: start_or_stop
+        } =
+          group
+      ) do
+    {_, other_times} =
+      group
+      |> all_times()
+      |> List.pop_at(representative_index)
+
+    phrase = options_phrase(start_or_stop == :stop, Enum.count(other_times))
+
+    formatted_times =
+      other_times
+      |> Enum.map(&Timex.format!(&1, "%-I:%M", :strftime))
+      |> Enum.join(", ")
+
+    "Similar #{phrase} #{formatted_times}"
+  end
+
+  defp options_phrase(true, 1), do: "trip arrives by"
+  defp options_phrase(true, _), do: "trips arrive by"
+  defp options_phrase(false, 1), do: "trip departs at"
+  defp options_phrase(_, _), do: "trips depart at"
+
+  @doc """
+  List of either start times or stop times for this group
+  """
+  @spec all_times(t()) :: [DateTime.t()]
+  def all_times(%__MODULE__{itineraries: itineraries, representative_time: start_or_stop}) do
+    Enum.map(itineraries, &Map.get(&1, start_or_stop))
+  end
+end

lib/dotcom/trip_plan/itinerary_groups.ex

@@ -8,48 +8,34 @@ defmodule Dotcom.TripPlan.ItineraryGroups do
 
   import DotcomWeb.TripPlanView, only: [get_one_way_total_by_type: 2]
 
-  alias Dotcom.TripPlan.{Itinerary, Leg, PersonalDetail, TransitDetail}
+  alias Dotcom.TripPlan.{Itinerary, ItineraryGroup, Leg, PersonalDetail, TransitDetail}
   alias OpenTripPlannerClient.ItineraryTag
 
   @short_walk_threshold_minutes 5
-  @max_per_group 5
-
-  @type summarized_leg :: %{
-          routes: [Routes.Route.t()],
-          walk_minutes: non_neg_integer()
-        }
-  @type summary :: %{
-          accessible?: boolean() | nil,
-          duration: non_neg_integer(),
-          first_start: DateTime.t(),
-          first_stop: DateTime.t(),
-          next_starts: [DateTime.t()],
-          summarized_legs: [summarized_leg()],
-          tag: String.t(),
-          total_cost: non_neg_integer(),
-          walk_distance: float()
-        }
-
-  @spec from_itineraries([Itinerary.t()]) :: [
-          %{itineraries: [Itinerary.t()], summary: summary()}
-        ]
-  def from_itineraries(itineraries) do
+  @max_per_group 4
+
+  @doc """
+  From a large list of itineraries, collect them into 5 groups of at most
+  #{@max_per_group} itineraries each, sorting the groups in favor of tagged
+  groups first
+  """
+  @spec from_itineraries([Itinerary.t()], boolean()) :: [ItineraryGroup.t()]
+  def from_itineraries(itineraries, take_from_end? \\ false) do
     itineraries
     |> Enum.group_by(&unique_legs_to_hash/1)
     |> Enum.map(&drop_hash/1)
     |> Enum.reject(&Enum.empty?/1)
-    |> Enum.take(5)
-    |> Enum.map(&limit_itinerary_count/1)
-    |> Enum.map(&to_summarized_group/1)
+    |> Enum.map(&limit_itinerary_count(&1, take_from_end?))
+    |> Enum.map(&to_group(&1, take_from_end?))
     |> Enum.sort_by(fn
-      %{itineraries: [%{tag: tag} | _] = _} ->
+      %ItineraryGroup{summary: %{tag: tag}} ->
         Enum.find_index(ItineraryTag.tag_priority_order(), &(&1 == tag))
-
-      _ ->
-        -1
     end)
+    |> Enum.take(5)
   end
 
+  def max_per_group, do: @max_per_group
+
   defp unique_legs_to_hash(%Itinerary{legs: legs}) do
     legs
     |> Enum.reject(&short_walking_leg?/1)
@@ -69,22 +55,44 @@ defmodule Dotcom.TripPlan.ItineraryGroups do
     grouped_itineraries
   end
 
-  defp to_summarized_group(grouped_itineraries) do
-    %{
-      itineraries: ItineraryTag.sort_tagged(grouped_itineraries),
-      summary: summary(grouped_itineraries)
-    }
+  defp limit_itinerary_count(itineraries, take_from_end?) do
+    if take_from_end? do
+      Enum.take(itineraries, -@max_per_group)
+    else
+      Enum.take(itineraries, @max_per_group)
+    end
   end
 
-  defp limit_itinerary_count(itineraries) do
-    Enum.take(itineraries, @max_per_group)
+  # Summarize a group of chronologically-sorted itineraries, noting the
+  # - representative_index: Which itinerary should represent the group? It will
+  #   be either the first or final one.
+  # - representative_time: An itinerary departs at the :start time and arrives
+  #   by the :stop time. This denotes which of those is relevant to the group.
+  defp to_group(grouped_itineraries, take_from_end?) do
+    representative_index = if(take_from_end?, do: Enum.count(grouped_itineraries) - 1, else: 0)
+
+    summary =
+      grouped_itineraries
+      |> Enum.at(representative_index)
+      |> to_summary(grouped_itineraries)
+
+    %ItineraryGroup{
+      # TODO: use second arg to sort by end time instead of start time
+      itineraries: ItineraryTag.sort_tagged(grouped_itineraries),
+      representative_index: representative_index,
+      representative_time: if(take_from_end?, do: :stop, else: :start),
+      summary: summary
+    }
   end
 
-  defp summary(itineraries) do
-    itineraries
-    |> Enum.map(&to_map_with_fare/1)
-    |> to_summary()
-    |> Map.put(:summarized_legs, to_summarized_legs(itineraries))
+  @doc """
+  The itinerary summary additionally includes a fare and summarized legs
+  """
+  @spec to_summary(Itinerary.t(), [Itinerary.t()]) :: map()
+  def to_summary(representative_itinerary, grouped_itineraries) do
+    representative_itinerary
+    |> to_map_with_fare()
+    |> Map.put(:summarized_legs, to_summarized_legs(grouped_itineraries))
   end
 
   defp to_summarized_legs(itineraries) do
@@ -109,7 +117,7 @@ defmodule Dotcom.TripPlan.ItineraryGroups do
 
   defp to_map_with_fare(itinerary) do
     itinerary
-    |> Map.take([:start, :stop, :tag, :duration, :accessible?, :walk_distance])
+    |> Map.take([:accessible?, :duration, :start, :stop, :tag, :walk_distance])
     |> Map.put(
       :total_cost,
       get_one_way_total_by_type(itinerary, :highest_one_way_fare)
@@ -126,34 +134,6 @@ defmodule Dotcom.TripPlan.ItineraryGroups do
     |> Enum.map(fn {leg, _} -> leg end)
   end
 
-  defp to_summary(itinerary_maps) do
-    # for most of the summary we can reflect the first itinerary
-    [
-      %{
-        tag: tag,
-        accessible?: accessible,
-        total_cost: total_cost,
-        duration: duration,
-        walk_distance: walk_distance,
-        stop: first_stop
-      }
-      | _
-    ] = itinerary_maps
-
-    [first_start | next_starts] = Enum.map(itinerary_maps, & &1.start)
-
-    %{
-      first_start: first_start,
-      first_stop: first_stop,
-      next_starts: next_starts,
-      tag: if(tag, do: Atom.to_string(tag) |> String.replace("_", " ")),
-      duration: duration,
-      accessible?: accessible,
-      walk_distance: walk_distance,
-      total_cost: total_cost
-    }
-  end
-
   defp summarize_legs(%{walk_minutes: new}, %{walk_minutes: old} = summary) do
     %{summary | walk_minutes: new + old}
   end

lib/dotcom_web/components/trip_planner/itinerary_detail.ex

@@ -14,48 +14,6 @@ defmodule DotcomWeb.Components.TripPlanner.ItineraryDetail do
   alias Dotcom.TripPlan.Alerts
 
   def itinerary_detail(assigns) do
-    itinerary_group =
-      Enum.at(assigns.results.itinerary_groups, assigns.results.itinerary_group_selection || 0)
-
-    itinerary = Enum.at(itinerary_group.itineraries, assigns.results.itinerary_selection || 0)
-
-    assigns = %{
-      itinerary: itinerary,
-      itinerary_selection: assigns.results.itinerary_selection,
-      itineraries: itinerary_group.itineraries
-    }
-
-    ~H"""
-    <div>
-      <.depart_at_buttons itineraries={@itineraries} itinerary_selection={@itinerary_selection} />
-      <.specific_itinerary_detail itinerary={@itinerary} />
-    </div>
-    """
-  end
-
-  defp depart_at_buttons(assigns) do
-    ~H"""
-    <div :if={Enum.count(@itineraries) > 1}>
-      <hr class="border-gray-lighter" />
-      <p class="text-sm mb-2 mt-3">Depart at</p>
-      <div id="itinerary-detail-departure-times" class="flex flex-wrap gap-2">
-        <.button
-          :for={{itinerary, index} <- Enum.with_index(@itineraries)}
-          type="button"
-          class={if(@itinerary_selection == index, do: "bg-brand-primary-lightest")}
-          size="small"
-          variant="secondary"
-          phx-click="select_itinerary"
-          phx-value-index={index}
-        >
-          {Timex.format!(itinerary.start, "%-I:%M%p", :strftime) |> String.downcase()}
-        </.button>
-      </div>
-    </div>
-    """
-  end
-
-  defp specific_itinerary_detail(assigns) do
     assigns =
       assigns
       |> assign_new(:alerts, fn -> Alerts.from_itinerary(assigns.itinerary) end)

lib/dotcom_web/components/trip_planner/itinerary_summary.ex

@@ -5,12 +5,14 @@ defmodule DotcomWeb.Components.TripPlanner.ItinerarySummary do
 
   use DotcomWeb, :component
 
+  attr :summary, :map, required: true
+
   def itinerary_summary(assigns) do
     ~H"""
     <div>
       <div class="flex flex-row mb-3 font-bold text-lg justify-between">
         <div>
-          {format_datetime_full(@summary.first_start)} - {format_datetime_full(@summary.first_stop)}
+          {format_datetime_full(@summary.start)} - {format_datetime_full(@summary.stop)}
         </div>
         <div>
           {@summary.duration} min