fix(TripController): Preserve added trip stop order (#165)

* fix(TripController): Preserve added trip stop order * style(TripController): if/else block ordering * refactor(TripController): specify included representative_trip stops
mbta · Jul 11, 2024 · df4ba7d · df4ba7d
1 parent d8af57c
commit df4ba7d
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 9 deletions.
diff --git a/lib/mobile_app_backend_web/controllers/trip_controller.ex b/lib/mobile_app_backend_web/controllers/trip_controller.ex
@@ -3,7 +3,11 @@ defmodule MobileAppBackendWeb.TripController do
   use MobileAppBackendWeb, :controller
 
   def map(conn, %{"trip_id" => trip_id}) do
-    {:ok, %{data: trips, included: %{shapes: shapes_by_id, stops: stops_by_id}}} =
+    {:ok,
+     %{
+       data: trips,
+       included: %{shapes: shapes_by_id, route_patterns: route_patterns, trips: included_trips}
+     }} =
       Repository.trips(
         filter: [id: trip_id],
         include: [:shape, :stops, [route_pattern: [representative_trip: :stops]]],
@@ -18,8 +22,8 @@ defmodule MobileAppBackendWeb.TripController do
 
       stop_ids =
         if Enum.empty?(trip.stop_ids) do
-          # Fall back to route pattern stops for added trips
-          Map.keys(stops_by_id)
+          # Fall back to stops on the representative trip
+          resolve_representative_trip_stops(trip, route_patterns, included_trips)
         else
           trip.stop_ids
         end
@@ -35,4 +39,20 @@ defmodule MobileAppBackendWeb.TripController do
       })
     end
   end
+
+  defp resolve_representative_trip_stops(
+         %{route_pattern_id: route_pattern_id} = _trip,
+         route_patterns,
+         included_trips
+       )
+       when is_map_key(route_patterns, route_pattern_id) do
+    route_pattern = Map.fetch!(route_patterns, route_pattern_id)
+
+    representative_trip = Map.get(included_trips, route_pattern.representative_trip_id)
+    if is_nil(representative_trip), do: [], else: representative_trip.stop_ids
+  end
+
+  defp resolve_representative_trip_stops(_trip, _route_patterns, _included_trips) do
+    []
+  end
 end
diff --git a/test/mobile_app_backend_web/controllers/trip_controller_test.exs b/test/mobile_app_backend_web/controllers/trip_controller_test.exs
@@ -23,8 +23,7 @@ defmodule MobileAppBackendWeb.TripControllerTest do
           stop_ids: ["Harvard", "Nubian"]
         )
 
-      harvard = build(:stop, id: "Harvard")
-      nubian = build(:stop, id: "Nubian")
+      route_pattern = build(:route_pattern, %{id: "66-0-1"})
 
       shape = build(:shape, id: trip.shape_id, polyline: "66_shape_polyline")
 
@@ -34,7 +33,7 @@ defmodule MobileAppBackendWeb.TripControllerTest do
              |> Keyword.get(:filter)
              |> Keyword.get(:id) do
           "trip_id" ->
-            ok_response([trip], [shape, harvard, nubian])
+            ok_response([trip], [shape, route_pattern])
 
           _ ->
             ok_response([])
@@ -60,6 +59,9 @@ defmodule MobileAppBackendWeb.TripControllerTest do
 
     test "when trip found without related stops, falls back to route pattern stops",
          %{conn: conn} do
+      route_pattern =
+        build(:route_pattern, %{id: "66-0-1", representative_trip_id: "rep_trip"})
+
       trip =
         build(:trip,
           id: "trip_id",
@@ -70,8 +72,7 @@ defmodule MobileAppBackendWeb.TripControllerTest do
           stop_ids: []
         )
 
-      harvard = build(:stop, id: "Harvard")
-      nubian = build(:stop, id: "Nubian")
+      rep_trip = build(:trip, %{id: "rep_trip", stop_ids: ["Harvard", "Nubian"]})
 
       shape = build(:shape, id: trip.shape_id, polyline: "66_shape_polyline")
 
@@ -81,7 +82,7 @@ defmodule MobileAppBackendWeb.TripControllerTest do
              |> Keyword.get(:filter)
              |> Keyword.get(:id) do
           "trip_id" ->
-            ok_response([trip], [shape, harvard, nubian])
+            ok_response([trip], [shape, route_pattern, rep_trip])
 
           _ ->
             ok_response([])
@@ -105,6 +106,50 @@ defmodule MobileAppBackendWeb.TripControllerTest do
                response
     end
 
+    test "when trip found without related stops but route pattern doesn't resolve, returns empty list",
+         %{conn: conn} do
+      trip =
+        build(:trip,
+          id: "trip_id",
+          route_id: "66",
+          route_pattern_id: "66-0-1",
+          direction_id: "1",
+          shape_id: "66_shape",
+          stop_ids: []
+        )
+
+      shape = build(:shape, id: trip.shape_id, polyline: "66_shape_polyline")
+
+      RepositoryMock
+      |> expect(:trips, 1, fn params, _opts ->
+        case params
+             |> Keyword.get(:filter)
+             |> Keyword.get(:id) do
+          "trip_id" ->
+            ok_response([trip], [shape])
+
+          _ ->
+            ok_response([])
+        end
+      end)
+
+      conn =
+        get(conn, "/api/trip/map", %{"trip_id" => trip.id})
+
+      response = json_response(conn, 200)
+
+      assert %{
+               "shape_with_stops" => %{
+                 "direction_id" => "1",
+                 "route_id" => "66",
+                 "route_pattern_id" => "66-0-1",
+                 "shape" => %{"id" => "66_shape", "polyline" => "66_shape_polyline"},
+                 "stop_ids" => []
+               }
+             } =
+               response
+    end
+
     @tag capture_log: true
     test "when trip not found, 404 error",
          %{conn: conn} do