hotfix: reinstate earlier timetable sorting (#2005)

lib/dotcom_web/controllers/schedule/timetable_controller.ex

@@ -58,7 +58,7 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
     %{
       trip_schedules: trip_schedules,
       all_stops: all_stops
-    } = build_timetable(conn.assigns.all_stops, timetable_schedules)
+    } = build_timetable(conn.assigns.all_stops, timetable_schedules, direction_id)
 
     canonical_rps =
       RoutePatterns.Repo.by_route_id(route.id,
@@ -230,22 +230,43 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
 
   defp tab_name(conn, _), do: assign(conn, :tab, "timetable")
 
-  @spec build_timetable([Stops.Stop.t()], [Schedules.Schedule.t()]) :: %{
+  @spec build_timetable([Stops.Stop.t()], [Schedules.Schedule.t()], 0 | 1) :: %{
           required(:trip_schedules) => %{
             required({Schedules.Trip.id_t(), Stops.Stop.id_t()}) => Schedules.Schedule.t()
           },
           required(:all_stops) => [Stops.Stop.t()]
         }
-  def build_timetable(all_stops, schedules) do
+  def build_timetable(all_stops, schedules, direction_id) do
     trip_schedules = Map.new(schedules, &trip_schedule(&1))
-    all_stops = remove_unused_stops(all_stops, schedules)
+
+    all_stops =
+      remove_unused_stops(all_stops, schedules)
+      |> Enum.sort_by(fn stop ->
+        {zone_to_sortable(stop.zone, direction_id),
+         trip_schedule_sequence_for_stop(stop, schedules)}
+      end)
 
     %{
       trip_schedules: trip_schedules,
       all_stops: all_stops
     }
   end
 
+  defp zone_to_sortable("1A", _), do: 0
+  defp zone_to_sortable(zone, 0) when is_binary(zone), do: String.to_integer(zone)
+  defp zone_to_sortable(zone, 1) when is_binary(zone), do: -String.to_integer(zone)
+  defp zone_to_sortable(_, _), do: 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)

test/dotcom_web/controllers/schedule/timetable_controller_test.exs

@@ -70,9 +70,9 @@ defmodule DotcomWeb.ScheduleController.TimetableControllerTest do
     "shuttle-trip-4" => %{stop_name: "shuttle", stop_sequence: 4, trip_id: "trip-4"}
   }
 
-  describe "build_timetable/2" do
+  describe "build_timetable/3" do
     test "trip_schedules: a map from trip_id/stop_id to a schedule" do
-      %{trip_schedules: trip_schedules} = build_timetable(@stops, @schedules)
+      %{trip_schedules: trip_schedules} = build_timetable(@stops, @schedules, 0)
 
       for schedule <- @schedules do
         assert trip_schedules[{schedule.trip.id, schedule.stop.id}] == schedule
@@ -82,15 +82,15 @@ defmodule DotcomWeb.ScheduleController.TimetableControllerTest do
     end
 
     test "all_stops: list of the stops in the same order" do
-      %{all_stops: all_stops} = build_timetable(@stops, @schedules)
+      %{all_stops: all_stops} = build_timetable(@stops, @schedules, 0)
 
       assert all_stops == @stops
     end
 
     test "all_stops: if a stop isn't used, it's removed from the list" do
       schedules = Enum.take(@schedules, 1)
 
-      %{all_stops: all_stops} = build_timetable(@stops, schedules)
+      %{all_stops: all_stops} = build_timetable(@stops, schedules, 1)
       # other two stops were removed
       assert [%{id: "1"}] = all_stops
     end