refactor(TripDetails): use UpcomingDepartures vehicle if same trip

lib/dotcom/schedule_finder/trip_details.ex

@@ -50,7 +50,8 @@ defmodule Dotcom.ScheduleFinder.TripDetails do
       :crowding,
       :status,
       :stop_id,
-      :stop_name
+      :stop_name,
+      :stop_sequence
     ]
 
     @type trip_vehicle_status_t() ::
@@ -61,7 +62,8 @@ defmodule Dotcom.ScheduleFinder.TripDetails do
             crowding: Vehicles.Vehicle.crowding(),
             status: trip_vehicle_status_t(),
             stop_id: Stops.Stop.id_t(),
-            stop_name: String.t()
+            stop_name: String.t(),
+            stop_sequence: non_neg_integer()
           }
   end
 
@@ -70,20 +72,13 @@ defmodule Dotcom.ScheduleFinder.TripDetails do
   alias Vehicles.Vehicle
 
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
-  @vehicles_repo Application.compile_env!(:dotcom, :repo_modules)[:vehicles]
 
   @spec trip_details(%{
           predicted_schedules: [PredictedSchedule.t()],
-          trip_id: Schedules.Trip.id_t()
+          trip_vehicle: Vehicles.Vehicle.t() | nil
         }) :: __MODULE__.t()
-  def trip_details(%{predicted_schedules: predicted_schedules, trip_id: trip_id}) do
-    vehicle =
-      trip_id
-      |> @vehicles_repo.trip()
-
-    vehicle_info =
-      vehicle
-      |> vehicle_info()
+  def trip_details(%{predicted_schedules: predicted_schedules, trip_vehicle: vehicle}) do
+    vehicle_info = vehicle_info(vehicle)
 
     stops =
       predicted_schedules
@@ -101,7 +96,7 @@ defmodule Dotcom.ScheduleFinder.TripDetails do
         }
       end)
       |> Enum.sort_by(& &1.stop_sequence)
-      |> drop_prediction_for_current_station(vehicle_info)
+      |> drop_predictions_before_current_station(vehicle_info)
 
     %__MODULE__{
       stops: stops,
@@ -134,23 +129,53 @@ defmodule Dotcom.ScheduleFinder.TripDetails do
 
   defp vehicle_info(%Vehicle{stop_id: nil}), do: %VehicleInfo{status: :location_unavailable}
 
-  defp vehicle_info(%Vehicle{crowding: crowding, status: status, stop_id: stop_id}) do
+  defp vehicle_info(%Vehicle{
+         crowding: crowding,
+         status: status,
+         stop_id: stop_id,
+         stop_sequence: stop_sequence
+       }) do
     stop = @stops_repo.get(stop_id)
 
     %VehicleInfo{
       crowding: crowding,
       status: status,
       stop_id: stop.parent_id || stop.id,
-      stop_name: stop.name
+      stop_name: stop.name,
+      stop_sequence: stop_sequence
     }
   end
 
-  defp drop_prediction_for_current_station(stops, %{status: :stopped, stop_id: stop_id}) do
-    case stops do
-      [%{stop_id: ^stop_id} | remaining_stops] -> remaining_stops
-      _ -> stops
+  # Sometimes predictions might not keep up with vehicles
+  # If we have a vehicle status, use it to omit past stops
+  defp drop_predictions_before_current_station(
+         trip_stops,
+         %VehicleInfo{} = vehicle_info
+       ) do
+    current = current_stop_index(trip_stops, vehicle_info)
+
+    if is_nil(current) do
+      # finishing another trip -- show all the stops
+      trip_stops
+    else
+      upcoming_stops = Enum.take(trip_stops, current - length(trip_stops))
+
+      # Omit current trip_stop if the vehicle is stopped
+      if vehicle_info.status == :stopped do
+        Enum.drop(upcoming_stops, 1)
+      else
+        upcoming_stops
+      end
     end
   end
 
-  defp drop_prediction_for_current_station(stops, _), do: stops
+  defp drop_predictions_before_current_station(trip_stops, _), do: trip_stops
+
+  defp current_stop_index(trip_stops, vehicle_info) do
+    Enum.find_index(trip_stops, &vehicle_at_stop?(&1, vehicle_info))
+  end
+
+  defp vehicle_at_stop?(stop, vehicle) do
+    stop.stop_id == vehicle.stop_id && stop.stop_sequence == vehicle.stop_sequence
+  end
 end

lib/dotcom/schedule_finder/upcoming_departures.ex

@@ -247,15 +247,18 @@ defmodule Dotcom.ScheduleFinder.UpcomingDepartures do
         stop_id: stop_id
       }) do
     trip = predicted_schedule |> PredictedSchedule.trip()
+    stop = predicted_schedule |> PredictedSchedule.stop()
     stop_sequence = PredictedSchedule.stop_sequence(predicted_schedule)
-    vehicle_at_stop_status = PredictedSchedule.vehicle_at_stop_status(predicted_schedule)
+    vehicle = PredictedSchedule.vehicle(predicted_schedule)
+    vehicle_at_stop_status = vehicle_at_stop_status(vehicle, stop, stop_sequence)
 
     trip_details =
       trip_details(%{
         predicted_schedules_by_trip_id: predicted_schedules_by_trip_id,
         trip_id: trip.id,
         stop_id: stop_id,
-        stop_sequence: stop_sequence
+        stop_sequence: stop_sequence,
+        vehicle: vehicle
       })
 
     %UpcomingDeparture{
@@ -282,16 +285,25 @@ defmodule Dotcom.ScheduleFinder.UpcomingDepartures do
     }
   end
 
+  # Retrieves status if a vehicle is associated with the given stop/sequence
+  defp vehicle_at_stop_status(nil, _, _), do: nil
+
+  defp vehicle_at_stop_status(vehicle, stop, stop_sequence) do
+    at_stop? = vehicle.stop_id in [stop.id | stop.child_ids]
+    if at_stop? && vehicle.stop_sequence == stop_sequence, do: vehicle.status
+  end
+
   defp trip_details(%{
          predicted_schedules_by_trip_id: predicted_schedules_by_trip_id,
          trip_id: trip_id,
          stop_id: stop_id,
-         stop_sequence: stop_sequence
+         stop_sequence: stop_sequence,
+         vehicle: vehicle
        }) do
     %TripDetails{stops: stops, vehicle_info: vehicle_info} =
       TripDetails.trip_details(%{
         predicted_schedules: predicted_schedules_by_trip_id |> Map.get(trip_id, []),
-        trip_id: trip_id
+        trip_vehicle: if(vehicle && vehicle.trip_id == trip_id, do: vehicle)
       })
 
     {stops_before, stop, stops_after} =

lib/predicted_schedule.ex

@@ -290,23 +290,6 @@ defmodule PredictedSchedule do
 
   def vehicle(_), do: nil
 
-  @doc """
-  Retrieves status from predicted schedule vehicle if a vehicle is at or approaching the predicted schedule trip/stop/stop_sequence
-  """
-  @spec vehicle_at_stop_status(PredictedSchedule.t()) :: Vehicles.Vehicle.status() | nil
-  def vehicle_at_stop_status(ps) do
-    stop = stop(ps)
-    stop_sequence = stop_sequence(ps)
-    vehicle = vehicle(ps)
-
-    if not is_nil(vehicle) and vehicle.stop_sequence == stop_sequence and
-         vehicle.stop_id in [stop.id | stop.child_ids] do
-      vehicle.status
-    end
-  end
-
-  def vehicle_status(_predicted_schedule), do: nil
-
   @doc """
   Determines if the given predicted schedule occurs after the given time
   """

test/dotcom/schedule_finder/trip_details_test.exs

@@ -16,8 +16,6 @@ defmodule Dotcom.ScheduleFinder.TripDetailsTest do
       Factories.Stops.Stop.build(:stop, id: id, parent_id: nil)
     end)
 
-    stub(Vehicles.Repo.Mock, :trip, fn _ -> Factories.Vehicles.Vehicle.build(:vehicle) end)
-
     :ok
   end
 
@@ -58,7 +56,7 @@ defmodule Dotcom.ScheduleFinder.TripDetailsTest do
       trip_details =
         TripDetails.trip_details(%{
           predicted_schedules: predicted_schedules,
-          trip_id: trip.id
+          trip_vehicle: nil
         })
 
       assert trip_details.stops |> Enum.map(& &1.stop_id) == stop_ids
@@ -92,7 +90,7 @@ defmodule Dotcom.ScheduleFinder.TripDetailsTest do
       trip_details =
         TripDetails.trip_details(%{
           predicted_schedules: predicted_schedules,
-          trip_id: trip.id
+          trip_vehicle: nil
         })
 
       assert [trip_stop] = trip_details.stops
@@ -133,7 +131,7 @@ defmodule Dotcom.ScheduleFinder.TripDetailsTest do
       trip_details =
         TripDetails.trip_details(%{
           predicted_schedules: predicted_schedules,
-          trip_id: trip.id
+          trip_vehicle: nil
         })
 
       assert [trip_stop] = trip_details.stops
@@ -174,7 +172,7 @@ defmodule Dotcom.ScheduleFinder.TripDetailsTest do
       trip_details =
         TripDetails.trip_details(%{
           predicted_schedules: predicted_schedules,
-          trip_id: trip.id
+          trip_vehicle: nil
         })
 
       assert [trip_stop] = trip_details.stops
@@ -241,7 +239,7 @@ defmodule Dotcom.ScheduleFinder.TripDetailsTest do
       trip_details =
         TripDetails.trip_details(%{
           predicted_schedules: predicted_schedules,
-          trip_id: trip.id
+          trip_vehicle: nil
         })
 
       assert [trip_stop_1, trip_stop_2, trip_stop_3] = trip_details.stops
@@ -269,15 +267,13 @@ defmodule Dotcom.ScheduleFinder.TripDetailsTest do
           }
         end)
 
-      trip_id = FactoryHelpers.build(:id)
       crowding = Faker.Util.pick([:not_crowded, :crowded, :some_crowding])
       vehicle = Factories.Vehicles.Vehicle.build(:vehicle, stop_id: stop_id, crowding: crowding)
-      stub(Vehicles.Repo.Mock, :trip, fn ^trip_id -> vehicle end)
 
       trip_details =
         TripDetails.trip_details(%{
           predicted_schedules: predicted_schedules,
-          trip_id: trip_id
+          trip_vehicle: vehicle
         })
 
       vehicle_info = trip_details.vehicle_info
@@ -297,15 +293,12 @@ defmodule Dotcom.ScheduleFinder.TripDetailsTest do
           }
         end)
 
-      trip_id = FactoryHelpers.build(:id)
-
       vehicle = Factories.Vehicles.Vehicle.build(:vehicle, stop_id: nil)
-      stub(Vehicles.Repo.Mock, :trip, fn ^trip_id -> vehicle end)
 
       trip_details =
         TripDetails.trip_details(%{
           predicted_schedules: predicted_schedules,
-          trip_id: trip_id
+          trip_vehicle: vehicle
         })
 
       vehicle_info = trip_details.vehicle_info
@@ -332,15 +325,12 @@ defmodule Dotcom.ScheduleFinder.TripDetailsTest do
           }
         end)
 
-      trip_id = FactoryHelpers.build(:id)
-
       vehicle = Factories.Vehicles.Vehicle.build(:vehicle, stop_id: child_stop_id)
-      stub(Vehicles.Repo.Mock, :trip, fn ^trip_id -> vehicle end)
 
       trip_details =
         TripDetails.trip_details(%{
           predicted_schedules: predicted_schedules,
-          trip_id: trip_id
+          trip_vehicle: vehicle
         })
 
       vehicle_info = trip_details.vehicle_info
@@ -368,26 +358,84 @@ defmodule Dotcom.ScheduleFinder.TripDetailsTest do
 
     predicted_schedules =
       stops
-      |> Enum.map(
-        &%PredictedSchedule{
-          prediction: Factories.Predictions.Prediction.build(:prediction, trip: trip, stop: &1),
-          schedule: Factories.Schedules.Schedule.build(:schedule, trip: trip, stop: &1)
+      |> Enum.with_index()
+      |> Enum.map(fn {stop, index} ->
+        %PredictedSchedule{
+          prediction:
+            Factories.Predictions.Prediction.build(:prediction,
+              trip: trip,
+              stop: stop,
+              stop_sequence: index
+            ),
+          schedule:
+            Factories.Schedules.Schedule.build(:schedule,
+              trip: trip,
+              stop: stop,
+              stop_sequence: index
+            )
         }
-      )
-
-    trip_id = trip.id
+      end)
 
-    stub(Vehicles.Repo.Mock, :trip, fn ^trip_id ->
-      Factories.Vehicles.Vehicle.build(:vehicle, status: :stopped, stop_id: current_stop_id)
-    end)
+    vehicle =
+      Factories.Vehicles.Vehicle.build(:vehicle,
+        status: :stopped,
+        stop_id: current_stop_id,
+        stop_sequence: 0
+      )
 
     trip_details =
       TripDetails.trip_details(%{
         predicted_schedules: predicted_schedules,
-        trip_id: trip_id
+        trip_vehicle: vehicle
       })
 
     assert trip_details.stops |> Enum.map(& &1.stop_id) == future_stop_ids
     assert trip_details.stops |> Enum.map(& &1.stop_name) == future_stop_names
   end
+
+  test "drops earlier stops before the active vehicle" do
+    trip = Factories.Schedules.Trip.build(:trip)
+
+    stops = Faker.Util.sample_uniq(20, fn -> Factories.Stops.Stop.build(:stop) end)
+
+    predicted_schedules =
+      stops
+      |> Enum.with_index()
+      |> Enum.map(fn {stop, index} ->
+        %PredictedSchedule{
+          prediction:
+            Factories.Predictions.Prediction.build(:prediction,
+              trip: trip,
+              stop: stop,
+              stop_sequence: index
+            ),
+          schedule:
+            Factories.Schedules.Schedule.build(:schedule,
+              trip: trip,
+              stop: stop,
+              stop_sequence: index
+            )
+        }
+      end)
+
+    current_index = Faker.random_between(5, 15)
+    {_earlier_stops, [current_stop | future_stops]} = stops |> Enum.split(current_index)
+
+    vehicle =
+      Factories.Vehicles.Vehicle.build(:vehicle,
+        status: :stopped,
+        stop_id: current_stop.id,
+        stop_sequence: current_index,
+        trip_id: trip.id
+      )
+
+    trip_details =
+      TripDetails.trip_details(%{
+        predicted_schedules: predicted_schedules,
+        trip_vehicle: vehicle
+      })
+
+    assert trip_details.stops |> Enum.map(& &1.stop_id) == Enum.map(future_stops, & &1.id)
+    assert trip_details.stops |> Enum.map(& &1.stop_name) == Enum.map(future_stops, & &1.name)
+  end
 end