fix: add handling for retrieve estimates on new shuttle creation (#1059)

* feat: add display_stop virtual field to route_stop for changeset display

* fix: on create shuttle, fix error messages not displaying and use display_stop before db commit

* fix: allow changing of stops by guarding against nil error

* tests: fix tests by adding id and display_stop_id to RouteStop

lib/arrow/shuttles.ex

@@ -369,11 +369,49 @@ defmodule Arrow.Shuttles do
 
   @spec get_stop_coordinates(RouteStop.t() | Stop.t() | GtfsStop.t()) ::
           {:ok, map()} | {:error, any}
+  def get_stop_coordinates(%RouteStop{display_stop: nil, display_stop_id: nil}) do
+    {:error, "Incomplete stop data, please check your input"}
+  end
+
+  def get_stop_coordinates(%RouteStop{id: nil, display_stop: nil, display_stop_id: id}) do
+    {:error, "Missing lat/lon data for stop #{id}"}
+  end
+
+  def get_stop_coordinates(%RouteStop{id: nil, display_stop: display_stop, display_stop_id: id}) do
+    if id,
+      do: get_stop_coordinates(display_stop),
+      else: {:error, "Missing id for stop"}
+  end
+
+  def get_stop_coordinates(%RouteStop{
+        display_stop: %Stop{} = display_stop,
+        display_stop_id: _display_stop_id
+      }) do
+    get_stop_coordinates(display_stop)
+  end
+
+  def get_stop_coordinates(%RouteStop{
+        display_stop: %GtfsStop{} = display_stop,
+        display_stop_id: _display_stop_id
+      }) do
+    get_stop_coordinates(display_stop)
+  end
+
   def get_stop_coordinates(%RouteStop{gtfs_stop: stop, stop: nil}) do
+    stop =
+      if Ecto.assoc_loaded?(stop),
+        do: stop,
+        else: Arrow.Repo.preload(stop, :gtfs_stop, force: true)
+
     get_stop_coordinates(stop)
   end
 
   def get_stop_coordinates(%RouteStop{gtfs_stop: nil, stop: stop}) do
+    stop =
+      if Ecto.assoc_loaded?(stop),
+        do: stop,
+        else: Arrow.Repo.preload(stop, :stop, force: true)
+
     get_stop_coordinates(stop)
   end
 
@@ -385,10 +423,6 @@ defmodule Arrow.Shuttles do
     {:ok, %{lat: lat, lon: lon}}
   end
 
-  def get_stop_coordinates(%RouteStop{id: nil, display_stop_id: id}) do
-    {:error, "Missing lat/lon data for stop #{id}"}
-  end
-
   def get_stop_coordinates(stop) do
     {:error, "Missing lat/lon data for stop #{inspect(stop)}"}
   end

lib/arrow/shuttles/route_stop.ex

@@ -12,6 +12,7 @@ defmodule Arrow.Shuttles.RouteStop do
           stop_sequence: integer(),
           time_to_next_stop: float(),
           display_stop_id: String.t(),
+          display_stop: Arrow.Shuttles.Stop.t() | Arrow.Gtfs.Stop.t() | nil,
           inserted_at: DateTime.t() | nil,
           updated_at: DateTime.t() | nil,
           shuttle_route: Arrow.Gtfs.Level.t() | Ecto.Association.NotLoaded.t() | nil,
@@ -24,6 +25,7 @@ defmodule Arrow.Shuttles.RouteStop do
     field :stop_sequence, :integer
     field :time_to_next_stop, :decimal
     field :display_stop_id, :string, virtual: true
+    field :display_stop, :map, virtual: true
     belongs_to :shuttle_route, Arrow.Shuttles.Route
     belongs_to :stop, Arrow.Shuttles.Stop
     belongs_to :gtfs_stop, Arrow.Gtfs.Stop, type: :string
@@ -46,17 +48,18 @@ defmodule Arrow.Shuttles.RouteStop do
 
     change =
       if display_stop_id = Ecto.Changeset.get_change(change, :display_stop_id) do
-        {stop_id, gtfs_stop_id, error} =
+        {stop_id, gtfs_stop_id, stop, error} =
           case Shuttles.stop_or_gtfs_stop_for_stop_id(display_stop_id) do
-            %Stop{id: id} -> {id, nil, nil}
-            %GtfsStop{id: id} -> {nil, id, nil}
-            nil -> {nil, nil, "not a valid stop ID"}
+            %Stop{id: id} = stop -> {id, nil, stop, nil}
+            %GtfsStop{id: id} = stop -> {nil, id, stop, nil}
+            nil -> {nil, nil, nil, "not a valid stop ID"}
           end
 
         change =
           change
           |> change(stop_id: stop_id)
           |> change(gtfs_stop_id: gtfs_stop_id)
+          |> change(display_stop: stop)
 
         if is_nil(error) do
           change

lib/arrow_web/live/shuttle_live/shuttle_live.ex

@@ -213,11 +213,11 @@ defmodule ArrowWeb.ShuttleViewLive do
             Retrieve Estimates
           </button>
           <aside
-            :if={@errors[:route_stops][input_value(f_route, :direction_id)]}
+            :if={@errors[:route_stops][to_string(input_value(f_route, :direction_id))]}
             class="mt-2 text-sm alert alert-danger"
             role="alert"
           >
-            <%= @errors[:route_stops][input_value(f_route, :direction_id)] %>
+            <%= @errors[:route_stops][to_string(input_value(f_route, :direction_id))] %>
           </aside>
         </div>
       </div>
@@ -245,9 +245,10 @@ defmodule ArrowWeb.ShuttleViewLive do
 
   defp render_route_stop(%RouteStop{stop_id: stop_id} = route_stop) when not is_nil(stop_id) do
     route_stop =
-      if !Ecto.assoc_loaded?(route_stop.stop) or route_stop.stop.id != stop_id,
-        do: Arrow.Repo.preload(route_stop, :stop, force: true),
-        else: route_stop
+      if !Ecto.assoc_loaded?(route_stop.stop) or
+           (route_stop.stop && route_stop.stop.id != stop_id),
+         do: Arrow.Repo.preload(route_stop, :stop, force: true),
+         else: route_stop
 
     if route_stop.stop do
       %{
@@ -264,9 +265,10 @@ defmodule ArrowWeb.ShuttleViewLive do
   defp render_route_stop(%RouteStop{gtfs_stop_id: gtfs_stop_id} = route_stop)
        when not is_nil(gtfs_stop_id) do
     route_stop =
-      if !Ecto.assoc_loaded?(route_stop.gtfs_stop) or route_stop.gtfs_stop.id != gtfs_stop_id,
-        do: Arrow.Repo.preload(route_stop, :gtfs_stop, force: true),
-        else: route_stop
+      if !Ecto.assoc_loaded?(route_stop.gtfs_stop) or
+           (route_stop.gtfs_stop && route_stop.gtfs_stop.id != gtfs_stop_id),
+         do: Arrow.Repo.preload(route_stop, :gtfs_stop, force: true),
+         else: route_stop
 
     if route_stop.gtfs_stop do
       %{
@@ -482,8 +484,8 @@ defmodule ArrowWeb.ShuttleViewLive do
     {:noreply, socket}
   end
 
-  def handle_event("get_time_to_next_stop", %{"value" => direction_id}, socket) do
-    direction_id = String.to_existing_atom(direction_id)
+  def handle_event("get_time_to_next_stop", %{"value" => direction_id_string}, socket) do
+    direction_id = String.to_existing_atom(direction_id_string)
 
     changeset = socket.assigns.form.source
 
@@ -511,15 +513,17 @@ defmodule ArrowWeb.ShuttleViewLive do
 
         {:noreply,
          socket
+         |> update(:errors, fn errors ->
+           put_in(errors, [:route_stops, Access.key(direction_id_string)], nil)
+         end)
          |> assign(:form, to_form(changeset))}
 
       {:error, error} ->
-        socket =
-          update(socket, :errors, fn errors ->
-            put_in(errors, [:route_stops, Access.key(direction_id, [])], error)
-          end)
-
-        {:noreply, socket}
+        {:noreply,
+         socket
+         |> update(:errors, fn errors ->
+           put_in(errors, [:route_stops, Access.key(direction_id_string)], error)
+         end)}
     end
   end
 
@@ -574,9 +578,13 @@ defmodule ArrowWeb.ShuttleViewLive do
   @spec get_stop_travel_times(list({:ok, any()})) ::
           {:ok, list(number())} | {:error, any()}
   defp get_stop_travel_times(stop_coordinates) do
-    stop_coordinates
-    |> Enum.map(fn {:ok, c} -> c end)
-    |> Shuttles.get_travel_times()
+    stop_coordinates = stop_coordinates |> Enum.map(fn {:ok, c} -> c end)
+
+    if length(stop_coordinates) > 1 do
+      stop_coordinates |> Shuttles.get_travel_times()
+    else
+      {:error, "Incomplete stop data, please provide more than one stop"}
+    end
   end
 
   defp update_route_changeset_with_stop_time_estimates(route_changeset) do
@@ -607,6 +615,7 @@ defmodule ArrowWeb.ShuttleViewLive do
         coordinate_errors =
           stop_coordinates
           |> Enum.filter(&match?({:error, _}, &1))
+          |> Enum.uniq()
           |> Enum.map_join(", ", fn {:error, msg} -> "#{msg}" end)
 
         {:error, coordinate_errors}

test/arrow/shuttles_test.exs

@@ -362,9 +362,14 @@ defmodule Arrow.ShuttlesTest do
       lat = 42.38758
       lon = -71.11934
 
+      arrow_stop = stop_fixture(%{stop_lat: lat, stop_lon: lon})
+
       stop = %Shuttles.RouteStop{
-        stop: stop_fixture(%{stop_lat: lat, stop_lon: lon}),
-        gtfs_stop: nil
+        id: 1,
+        stop: arrow_stop,
+        gtfs_stop: nil,
+        display_stop: nil,
+        display_stop_id: arrow_stop.stop_id
       }
 
       coordinates = %{lat: lat, lon: lon}
@@ -375,7 +380,14 @@ defmodule Arrow.ShuttlesTest do
     test "gets the stop coordinates for an gtfs stop from a RouteStop" do
       gtfs_stop = insert(:gtfs_stop)
       coordinates = %{lat: gtfs_stop.lat, lon: gtfs_stop.lon}
-      stop = %Shuttles.RouteStop{gtfs_stop: gtfs_stop, stop: nil}
+
+      stop = %Shuttles.RouteStop{
+        id: 1,
+        gtfs_stop: gtfs_stop,
+        stop: nil,
+        display_stop: nil,
+        display_stop_id: gtfs_stop.id
+      }
 
       assert {:ok, ^coordinates} = Shuttles.get_stop_coordinates(stop)
     end