wip: get stops from canonical route patterns

lib/dotcom_web/controllers/schedule/timetable_controller.ex

@@ -2,6 +2,7 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
   @moduledoc "Handles the Timetable tab for commuter rail routes."
   use DotcomWeb, :controller
   alias Plug.Conn
+  alias RoutePatterns.RoutePattern
   alias Routes.Route
   alias DotcomWeb.ScheduleView
 
@@ -11,7 +12,7 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
   plug(DotcomWeb.Plugs.DateInRating)
   plug(:tab_name)
   plug(:direction_id)
-  plug(:all_stops)
+  plug(:canonical_stops)
   plug(DotcomWeb.ScheduleController.RoutePdfs)
   plug(DotcomWeb.ScheduleController.Core)
   plug(:do_assign_trip_schedules)
@@ -58,7 +59,7 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
     %{
       trip_schedules: trip_schedules,
       all_stops: all_stops
-    } = build_timetable(conn.assigns.all_stops, timetable_schedules, direction_id)
+    } = build_timetable(conn.assigns.canonical_stops, timetable_schedules, direction_id)
 
     canonical_rps =
       RoutePatterns.Repo.by_route_id(route.id,
@@ -74,28 +75,6 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
 
     track_changes = track_changes(trip_schedules, canonical_stop_ids)
 
-    ## Logging to capture misordered stops on CR-Newburyport timetable ##
-    # First, is this code even present?
-    Logger.info("dotcom_web.schedule_controller.timetable_controller.assign_trip_schedules")
-
-    if route.id == "CR-Newburyport" && direction_id == 1 do
-      [last, next | _rest] = Enum.map(all_stops, & &1.name) |> Enum.reverse()
-
-      case {last, next} do
-        {"North Station", "Chelsea"} ->
-          # Log both good and bad results so we can quantify
-          Logger.info(
-            "dotcom_web.schedule_controller.timetable_controller.assign_trip_schedules stop_order=expected"
-          )
-
-        _ ->
-          # Show the offending last two stops
-          Logger.warning(
-            "dotcom_web.schedule_controller.timetable_controller.assign_trip_schedules stop_order=unexpected last=#{last} next=#{next}"
-          )
-      end
-    end
-
     conn
     |> assign(:timetable_schedules, timetable_schedules)
     |> assign(:header_schedules, header_schedules)
@@ -226,27 +205,42 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
     |> Enum.map(fn {train, stop, value} -> {{train, stop}, value} end)
   end
 
-  defp all_stops(%Conn{assigns: %{date_in_rating?: false}} = conn, _) do
-    conn
+  # Get canonical trip's stops, will be merged with schedule stops
+  # The advantage of using this is that stops returned for a trip ARE in order.
+  # Sometimes routes have more than one list of stops, so they need to be merged.
+  defp canonical_stops(
+         %{assigns: %{route: %{id: route_id}, direction_id: direction_id}} = conn,
+         _
+       ) do
+    canonical_stop_lists =
+      route_id
+      |> RoutePatterns.Repo.by_route_id(direction_id: direction_id, canonical: true)
+      |> Enum.map(&stops_for_route_pattern/1)
+      |> Enum.reduce(&merge_stop_lists(&1, &2, direction_id))
+
+    assign(conn, :canonical_stops, canonical_stop_lists)
   end
 
-  defp all_stops(conn, _) do
-    all_stops =
-      Stops.Repo.by_route(conn.assigns.route.id, conn.assigns.direction_id,
-        date: conn.assigns.date
-      )
+  defp canonical_stops(conn, _), do: conn
+
+  defp stops_for_route_pattern(%RoutePattern{representative_trip_id: trip_id}) do
+    Stops.Repo.by_trip(trip_id)
+    |> Enum.map(&Stops.Repo.get_parent/1)
+  end
 
-    case all_stops do
-      {:error, error} ->
-        :ok =
-          Logger.warning(
-            "module=#{__MODULE__} fun=all_stops error=#{inspect(error)} route=#{conn.assigns.route.id} direction_id=#{conn.assigns.direction_id} date=#{conn.assigns.date}"
-          )
+  defp stops_for_route_pattern(_), do: []
 
-        conn
+  defp merge_stop_lists(incoming_stops, base_stops, direction_id) do
+    if Enum.all?(incoming_stops, &Enum.member?(base_stops, &1)) do
+      base_stops
+    else
+      incoming_stops = Enum.reject(incoming_stops, &Enum.member?(base_stops, &1))
 
-      _ ->
-        assign(conn, :all_stops, all_stops)
+      if direction_id == 1 do
+        incoming_stops ++ base_stops
+      else
+        base_stops ++ incoming_stops
+      end
     end
   end
 
@@ -258,48 +252,18 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
           },
           required(:all_stops) => [Stops.Stop.t()]
         }
-  def build_timetable(all_stops, schedules, direction_id) do
+  def build_timetable(canonical_stops, schedules, _direction_id) do
     trip_schedules = Map.new(schedules, &trip_schedule(&1))
 
-    all_stops =
-      remove_unused_stops(all_stops, schedules)
-      |> Enum.sort_by(fn stop ->
-        {zone_to_sortable(stop, direction_id), trip_schedule_sequence_for_stop(stop, schedules)}
-      end)
+    adjusted_stops =
+      remove_unused_stops(canonical_stops, schedules)
 
     %{
       trip_schedules: trip_schedules,
-      all_stops: all_stops
+      all_stops: adjusted_stops
     }
   end
 
-  # Override North and South Station zones from 1A to 0
-  defp zone_to_sortable(%Stops.Stop{id: stop_id}, _direction_id)
-       when stop_id in ["place-sstat", "place-north"],
-       do: 0.0
-
-  defp zone_to_sortable(%Stops.Stop{zone: nil}, _direction_id), do: 0.0
-
-  defp zone_to_sortable(%Stops.Stop{zone: "1A"}, direction_id),
-    do: zone_to_sortable("0.5", direction_id)
-
-  defp zone_to_sortable(%Stops.Stop{zone: zone}, direction_id),
-    do: zone_to_sortable("#{zone}.0", direction_id)
-
-  defp zone_to_sortable(zone, 0) when is_binary(zone), do: String.to_float(zone)
-  defp zone_to_sortable(zone, 1) when is_binary(zone), do: -String.to_float(zone)
-  defp zone_to_sortable(_, _), do: 0.0
-
-  # translate each stop into a general stop_sequence value. a given stop will
-  # have a different value for stop_sequence depending on the other stops in the
-  # trip, so we summarize here by taking the maximum value
-  defp trip_schedule_sequence_for_stop(stop, schedules) do
-    schedules
-    |> Enum.filter(&(&1.stop == stop))
-    |> Enum.map(& &1.stop_sequence)
-    |> Enum.max(fn -> 0 end)
-  end
-
   @spec trip_schedule(Schedules.Schedule.t()) ::
           {{Schedules.Trip.id_t() | nil, Stops.Stop.id_t() | nil}, Schedules.Schedule.t()}
   defp trip_schedule(%Schedules.Schedule{trip: trip, stop: stop} = schedule)