fix: Allow Hastus import to work when a block has as-directeds and re…

…gular trips (#2736) * fix: Allow Hastus import to work when a block has as-directeds and regular trips * fix: add :id to as directeds vehicles have issues without this * fix: parse :id on frontend * fix: reorder logic for WAD * wip! convert `isTripData` => `isAsDirectedData` * fix: add :id to as directed FE tests
mbta · Aug 14, 2024 · 8ff3d78 · 8ff3d78
1 parent 5a65a12
commit 8ff3d78
Show file tree

Hide file tree

Showing 10 changed files with 260 additions and 53 deletions.
diff --git a/assets/src/components/propertiesPanel/minischedule.tsx b/assets/src/components/propertiesPanel/minischedule.tsx
@@ -544,29 +544,27 @@ const TripSchedule = ({
           activeStatus={layoverActiveStatus}
         />
       ) : null}
-      {isTrip(trip) ? (
-        isDeadhead(trip) ? (
-          <DeadheadTrip
-            trip={trip}
-            status={sequenceToNonRevenueStatus(sequence)}
-            timeBasedStyle={deadheadTimeBasedStyle}
-            activeStatus={deadheadActiveStatus}
-            overloadOffset={overloadOffset}
-          />
-        ) : (
-          <RevenueTrip
-            trip={trip}
-            timeBasedStyle={onRouteTimeBasedStyle}
-            activeStatus={onRouteActiveStatus}
-            overloadOffset={overloadOffset}
-          />
-        )
-      ) : (
+      {isAsDirected(trip) ? (
         <AsDirectedRow
           asDirected={trip}
           timeBasedStyle={onRouteTimeBasedStyle}
           overloadOffset={overloadOffset}
         />
+      ) : isDeadhead(trip) ? (
+        <DeadheadTrip
+          trip={trip}
+          status={sequenceToNonRevenueStatus(sequence)}
+          timeBasedStyle={deadheadTimeBasedStyle}
+          activeStatus={deadheadActiveStatus}
+          overloadOffset={overloadOffset}
+        />
+      ) : (
+        <RevenueTrip
+          trip={trip}
+          timeBasedStyle={onRouteTimeBasedStyle}
+          activeStatus={onRouteActiveStatus}
+          overloadOffset={overloadOffset}
+        />
       )}
     </>
   )
@@ -839,7 +837,10 @@ const isBreak = (activity: Piece | Break): activity is Break =>
   Object.prototype.hasOwnProperty.call(activity, "breakType")
 
 const isTrip = (trip: Trip | AsDirected): trip is Trip =>
-  Object.prototype.hasOwnProperty.call(trip, "id")
+  !Object.prototype.hasOwnProperty.call(trip, "kind")
+
+const isAsDirected = (trip: Trip | AsDirected): trip is AsDirected =>
+  Object.prototype.hasOwnProperty.call(trip, "kind")
 
 const isDeadhead = (trip: Trip | AsDirected): boolean =>
   isTrip(trip) && trip.routeId == null

diff --git a/assets/src/minischedule.d.ts b/assets/src/minischedule.d.ts
@@ -21,6 +21,7 @@ export interface Break {
 }
 
 export interface AsDirected {
+  id: TripId | null
   kind: "wad" | "rad"
   startTime: Time
   endTime: Time

diff --git a/assets/src/models/minischeduleData.ts b/assets/src/models/minischeduleData.ts
@@ -23,6 +23,7 @@ interface BlockData {
 }
 
 interface AsDirectedData {
+  id: TripId | null
   kind: "wad" | "rad"
   start_time: Time
   end_time: Time
@@ -93,7 +94,7 @@ const pieceFromData = (pieceData: PieceData): Piece => ({
   startTime: pieceData.start_time,
   startPlace: pieceData.start_place,
   trips: pieceData.trips.map((data) =>
-    isTripData(data) ? tripFromData(data) : asDirectedFromData(data)
+    isAsDirectedData(data) ? asDirectedFromData(data) : tripFromData(data)
   ),
   endTime: pieceData.end_time,
   endPlace: pieceData.end_place,
@@ -122,10 +123,12 @@ const tripFromData = (tripData: TripData): Trip => ({
 })
 
 const asDirectedFromData = (asDirectedData: AsDirectedData): AsDirected => ({
+  id: asDirectedData.id,
   kind: asDirectedData.kind,
   startTime: asDirectedData.start_time,
   endTime: asDirectedData.end_time,
 })
 
-const isTripData = (data: TripData | AsDirectedData): data is TripData =>
-  Object.prototype.hasOwnProperty.call(data, "id")
+const isAsDirectedData = (
+  data: TripData | AsDirectedData
+): data is AsDirectedData => Object.prototype.hasOwnProperty.call(data, "kind")
diff --git a/assets/tests/api.test.ts b/assets/tests/api.test.ts
@@ -885,6 +885,7 @@ describe("fetchScheduleBlock", () => {
                 end_place: "end place",
               },
               {
+                id: "as-directed",
                 kind: "rad",
                 start_time: 567,
                 end_time: 1000,
@@ -923,6 +924,7 @@ describe("fetchScheduleBlock", () => {
                 endPlace: "end place",
               },
               {
+                id: "as-directed",
                 kind: "rad",
                 startTime: 567,
                 endTime: 1000,

diff --git a/assets/tests/components/propertiesPanel/minischedule.test.tsx b/assets/tests/components/propertiesPanel/minischedule.test.tsx
@@ -116,6 +116,7 @@ const asDirectedPiece: Piece = {
   startPlace: "place",
   trips: [
     {
+      id: "asDirected1",
       kind: "rad",
       startTime: 16200,
       endTime: 44400,

diff --git a/lib/schedule/as_directed.ex b/lib/schedule/as_directed.ex
@@ -1,9 +1,11 @@
 defmodule Schedule.AsDirected do
   @moduledoc false
 
+  alias Schedule.Trip
   alias Schedule.Hastus.Place
 
   @type t :: %__MODULE__{
+          id: Trip.id() | nil,
           kind: :wad | :rad,
           start_time: Util.Time.time_of_day(),
           end_time: Util.Time.time_of_day(),
@@ -12,6 +14,7 @@ defmodule Schedule.AsDirected do
         }
 
   @enforce_keys [
+    :id,
     :kind,
     :start_time,
     :end_time,
@@ -22,6 +25,7 @@ defmodule Schedule.AsDirected do
   @derive Jason.Encoder
 
   defstruct [
+    :id,
     :kind,
     :start_time,
     :end_time,

diff --git a/lib/schedule/hastus/activity.ex b/lib/schedule/hastus/activity.ex
@@ -151,20 +151,13 @@ defmodule Schedule.Hastus.Activity do
       end)
     end
 
-    as_directeds_and_schedule_trips =
-      if operator_is_as_directed?(activity) do
-        [as_directed_from_trips(trips_in_piece)]
-      else
-        Enum.map(trips_in_piece, &Map.fetch!(schedule_trips_by_id, &1.trip_id))
-      end
-
     %Piece{
       schedule_id: activity.schedule_id,
       run_id: activity.run_id,
       block_id: block_id_from_trips(trips_in_piece),
       start_time: activity.start_time,
       start_place: activity.start_place,
-      trips: as_directeds_and_schedule_trips,
+      trips: as_directeds_and_schedule_trips_from_trips(trips_in_piece, schedule_trips_by_id),
       end_time: activity.end_time,
       end_place: activity.end_place,
       start_mid_route?:
@@ -175,29 +168,79 @@ defmodule Schedule.Hastus.Activity do
 
   defp operator_activity_to_piece(activity, _, _, _), do: activity
 
-  @spec as_directed_from_trips([Hastus.Trip.t()]) :: AsDirected.t()
-  defp as_directed_from_trips(trips_in_piece) do
-    [
-      %Hastus.Trip{route_id: nil} = _pullout,
-      as_directed_trip,
-      %Hastus.Trip{route_id: nil} = _pull_back
-    ] = trips_in_piece
-
-    kind =
-      case as_directed_trip.route_id do
-        "rad" -> :rad
-        "wad" -> :wad
-      end
+  defp as_directeds_and_schedule_trips_from_trips(trips, schedule_trips_by_id) do
+    trips
+    |> Enum.map(&convert_as_directed/1)
+    |> strip_surrounding_pulls_from_as_directeds()
+    |> lookup_schedule_trips(schedule_trips_by_id)
+  end
+
+  defp convert_as_directed(%Hastus.Trip{route_id: "wad"} = trip) do
+    as_directed(trip, :wad)
+  end
+
+  defp convert_as_directed(%Hastus.Trip{route_id: "rad"} = trip) do
+    as_directed(trip, :rad)
+  end
+
+  defp convert_as_directed(trip) do
+    trip
+  end
 
+  defp as_directed(trip, kind) do
     %AsDirected{
+      id: trip.trip_id,
       kind: kind,
-      start_time: as_directed_trip.start_time,
-      end_time: as_directed_trip.end_time,
-      start_place: as_directed_trip.start_place,
-      end_place: as_directed_trip.end_place
+      start_time: trip.start_time,
+      end_time: trip.end_time,
+      start_place: trip.start_place,
+      end_place: trip.end_place
     }
   end
 
+  defp strip_surrounding_pulls_from_as_directeds([]) do
+    []
+  end
+
+  defp strip_surrounding_pulls_from_as_directeds([
+         %Hastus.Trip{route_id: nil} = _pullout,
+         %Schedule.AsDirected{} = as_directed_trip,
+         %Hastus.Trip{route_id: nil} = _pull_back
+         | rest
+       ]) do
+    [
+      as_directed_trip | strip_surrounding_pulls_from_as_directeds(rest)
+    ]
+  end
+
+  defp strip_surrounding_pulls_from_as_directeds([trip | rest]) do
+    [
+      trip
+      | strip_surrounding_pulls_from_as_directeds(rest)
+    ]
+  end
+
+  defp lookup_schedule_trips(
+         [],
+         _schedule_trips_by_id
+       ) do
+    []
+  end
+
+  defp lookup_schedule_trips(
+         [%Schedule.AsDirected{} = as_directed_trip | rest],
+         schedule_trips_by_id
+       ) do
+    [as_directed_trip | lookup_schedule_trips(rest, schedule_trips_by_id)]
+  end
+
+  defp lookup_schedule_trips([%Hastus.Trip{} = trip | rest], schedule_trips_by_id) do
+    [
+      Map.get(schedule_trips_by_id, trip.trip_id, trip)
+      | lookup_schedule_trips(rest, schedule_trips_by_id)
+    ]
+  end
+
   @spec as_directed_activities_to_pieces([__MODULE__.t() | Piece.t()]) :: [
           __MODULE__.t() | Piece.t()
         ]
@@ -230,6 +273,7 @@ defmodule Schedule.Hastus.Activity do
       start_place: activity.start_place,
       trips: [
         %AsDirected{
+          id: nil,
           kind:
             case activity.activity_type do
               "wad" -> :wad
@@ -327,11 +371,6 @@ defmodule Schedule.Hastus.Activity do
       trip.start_time <= activity.end_time
   end
 
-  @spec operator_is_as_directed?(__MODULE__.t()) :: boolean()
-  defp operator_is_as_directed?(%__MODULE__{activity_type: "Operator"} = activity) do
-    activity.partial_block_id =~ ~r/^[r|w]ad/i
-  end
-
   @spec start_mid_route?(
           __MODULE__.t(),
           [Hastus.Trip.t()],

diff --git a/test/schedule/block_test.exs b/test/schedule/block_test.exs
@@ -4,7 +4,7 @@ defmodule Schedule.BlockTest do
   import Skate.Factory
 
   alias Schedule.Block
-  alias Schedule.{Block, Trip}
+  alias Schedule.{Block, Trip, AsDirected}
   alias Schedule.Gtfs.StopTime
 
   @pullout %Trip{
@@ -80,6 +80,31 @@ defmodule Schedule.BlockTest do
            pieces: [@piece]
          )
 
+  @as_directed %AsDirected{
+    id: "ad1",
+    kind: :wad,
+    start_time: 7,
+    end_time: 8,
+    start_place: "garage",
+    end_place: "route"
+  }
+
+  @piece_with_as_directed build(:piece,
+                            block_id: @pullout.block_id,
+                            start_time: 1,
+                            end_time: 19,
+                            trips: [@pullout, @trip1, @as_directed, @deadhead, @trip2, @pullback]
+                          )
+
+  @block_with_as_directed build(
+                            :block,
+                            id: "b",
+                            schedule_id: "schedule",
+                            start_time: 1,
+                            end_time: 19,
+                            pieces: [@piece_with_as_directed]
+                          )
+
   describe "blocks_from_trips/ and get/3 " do
     test "can create blocks and then get them" do
       by_id = Block.blocks_from_pieces([@piece])
@@ -133,6 +158,12 @@ defmodule Schedule.BlockTest do
     test "returns :err if the given trip isn't found" do
       assert Block.next_revenue_trip(@block, "t3") == :err
     end
+
+    test "works if there are as-directed trips in the block" do
+      assert Block.next_revenue_trip(@block_with_as_directed, @trip1.id) == {:trip, @as_directed}
+      assert Block.next_revenue_trip(@block_with_as_directed, @as_directed.id) == {:trip, @trip2}
+      assert Block.next_revenue_trip(@block_with_as_directed, @trip2.id) == :last
+    end
   end
 
   describe "active?" do