diff --git a/lib/arrow/shuttles/definition_upload.ex b/lib/arrow/shuttles/definition_upload.ex index b74d7740..5f178d01 100644 --- a/lib/arrow/shuttles/definition_upload.ex +++ b/lib/arrow/shuttles/definition_upload.ex @@ -81,7 +81,11 @@ defmodule Arrow.Shuttles.DefinitionUpload do end) |> Enum.reverse() - if Enum.empty?(errors), do: {:ok, stop_ids}, else: {:errors, errors} + if Enum.empty?(errors) do + {:ok, stop_ids |> Enum.map(&Integer.to_string(&1))} + else + {:errors, errors} + end else {:errors, ["Unable to parse Stop ID column"]} end diff --git a/lib/arrow_web/live/shuttle_live/shuttle_live.ex b/lib/arrow_web/live/shuttle_live/shuttle_live.ex index fc6ab66c..4c74a9fd 100644 --- a/lib/arrow_web/live/shuttle_live/shuttle_live.ex +++ b/lib/arrow_web/live/shuttle_live/shuttle_live.ex @@ -317,10 +317,12 @@ defmodule ArrowWeb.ShuttleViewLive do end def handle_event("validate", %{"shuttle" => shuttle_params}, socket) do - change = Shuttles.change_shuttle(socket.assigns.shuttle, shuttle_params) - form = to_form(change, action: :validate) + form = + socket.assigns.shuttle + |> Shuttles.change_shuttle(shuttle_params) + |> to_form(action: :validate) - {:noreply, socket |> assign(form: form) |> update_map(change)} + {:noreply, socket |> assign(form: form) |> update_map()} end def handle_event("edit", %{"shuttle" => shuttle_params}, socket) do @@ -403,7 +405,7 @@ defmodule ArrowWeb.ShuttleViewLive do changeset = Ecto.Changeset.put_assoc(changeset, :routes, new_routes) - socket = socket |> assign(:form, to_form(changeset)) |> update_map(changeset) + socket = socket |> assign(form: to_form(changeset)) |> update_map() {:noreply, socket} end @@ -440,7 +442,7 @@ defmodule ArrowWeb.ShuttleViewLive do |> update(:errors, fn errors -> put_in(errors, [:route_stops, Access.key(direction_id_string)], nil) end) - |> assign(:form, to_form(changeset))} + |> assign(form: to_form(changeset))} {:error, error} -> {:noreply, @@ -451,32 +453,6 @@ defmodule ArrowWeb.ShuttleViewLive do end end - defp update_route_changeset_with_uploaded_stops(route_changeset, stop_ids, direction_id) do - if Ecto.Changeset.get_field(route_changeset, :direction_id) == direction_id do - new_route_stops = - stop_ids - |> Enum.with_index() - |> Enum.map(fn {stop_id, i} -> - Arrow.Shuttles.RouteStop.changeset( - %Arrow.Shuttles.RouteStop{}, - %{ - direction_id: direction_id, - stop_sequence: i, - display_stop_id: Integer.to_string(stop_id) - } - ) - end) - - Ecto.Changeset.put_assoc( - route_changeset, - :route_stops, - new_route_stops - ) - else - route_changeset - end - end - @spec get_stop_travel_times(list({:ok, any()})) :: {:ok, list(number())} | {:error, any()} defp get_stop_travel_times(stop_coordinates) do @@ -572,7 +548,9 @@ defmodule ArrowWeb.ShuttleViewLive do end end - defp update_map(socket, changeset) do + defp update_map(socket) do + changeset = socket.assigns.form.source + layers = changeset |> Ecto.Changeset.get_assoc(:routes, :struct) @@ -600,23 +578,98 @@ defmodule ArrowWeb.ShuttleViewLive do end end + defp get_new_route_stops_changeset_with_uploaded_stops(stop_ids, direction_id) do + new_route_stops = + stop_ids + |> Enum.with_index(1) + |> Enum.map(fn {stop_id, i} -> + Arrow.Shuttles.RouteStop.changeset( + %Arrow.Shuttles.RouteStop{}, + %{ + direction_id: direction_id, + stop_sequence: i, + display_stop_id: stop_id + } + ) + end) + + if Enum.all?(new_route_stops, & &1.valid?) do + {:ok, new_route_stops} + else + {:error, + new_route_stops + |> Enum.flat_map(fn %Ecto.Changeset{errors: errors} -> errors end) + |> Enum.map(&elem(&1, 1))} + end + end + defp populate_stop_ids(socket, stop_ids) do changeset = socket.assigns.form.source existing_routes = Ecto.Changeset.get_assoc(changeset, :routes) - new_routes = - Enum.map(existing_routes, fn route_changeset -> + new_route_stops = + existing_routes + |> Enum.map(fn route_changeset -> direction_id = Ecto.Changeset.get_field(route_changeset, :direction_id) - update_route_changeset_with_uploaded_stops( - route_changeset, - elem(stop_ids, direction_id |> Atom.to_string() |> String.to_integer()), - direction_id - ) + stop_ids + |> elem(direction_id |> Atom.to_string() |> String.to_integer()) + |> get_new_route_stops_changeset_with_uploaded_stops(direction_id) end) + |> Enum.split_with(fn + {:ok, _} -> true + _ -> false + end) + |> case do + {valid_route_stops, []} -> + {:ok, valid_route_stops |> Enum.flat_map(&elem(&1, 1))} - changeset = Ecto.Changeset.put_assoc(changeset, :routes, new_routes) + {_, errors} -> + {:error, errors |> Enum.flat_map(&elem(&1, 1))} + end - socket |> assign(:form, to_form(changeset)) |> update_map(changeset) + case new_route_stops do + {:ok, route_stops} -> + new_routes = + Enum.map(existing_routes, fn route_changeset -> + direction_id = Ecto.Changeset.get_field(route_changeset, :direction_id) + + direction_new_route_stops = + route_stops + |> Enum.filter(&(Ecto.Changeset.get_field(&1, :direction_id) == direction_id)) + + Ecto.Changeset.put_assoc( + route_changeset, + :route_stops, + direction_new_route_stops + ) + end) + + changeset = Ecto.Changeset.put_assoc(changeset, :routes, new_routes) + + case Ecto.Changeset.apply_action(changeset, :update) do + {:ok, shuttle} -> + # We replaced any existing associated stops + # so we create a new changeset here to track additional changes + # Related Ecto error: + # https://github.com/elixir-ecto/ecto/blob/18288287f18ce205b03b3b3dc8cb80f0f1b06dbe/lib/ecto/changeset/relation.ex#L448-L453 + new_changeset = Shuttles.change_shuttle(shuttle) + socket |> assign(form: to_form(new_changeset)) |> update_map() + + {:error, _invalid_changeset} -> + # The changeset from the upload data wasn't valid, so we don't retain it + socket |> assign(form: to_form(changeset)) |> update_map() + end + + {:error, errors} -> + socket + |> put_flash( + :errors, + { + "Failed to upload definition: ", + errors |> Enum.map(&translate_error/1) + } + ) + end end end diff --git a/test/arrow_web/live/shuttle_live/shuttle_live_test.exs b/test/arrow_web/live/shuttle_live/shuttle_live_test.exs index 38150f26..2fb29441 100644 --- a/test/arrow_web/live/shuttle_live/shuttle_live_test.exs +++ b/test/arrow_web/live/shuttle_live/shuttle_live_test.exs @@ -465,13 +465,67 @@ defmodule ArrowWeb.ShuttleLiveTest do direction_0_stop_sequence = ~w(9328 5327 5271) direction_1_stop_sequence = ~w(5271 5072 9328) + + (direction_0_stop_sequence ++ direction_1_stop_sequence) + |> Enum.uniq() + |> Enum.map(fn stop_id -> + insert(:gtfs_stop, %{id: stop_id}) + end) + + html = render_upload(definition, "valid.xlsx") + + direction_0_stop_rows = Floki.find(html, "#stops-dir-0 > .row") + direction_1_stop_rows = Floki.find(html, "#stops-dir-1 > .row") + + for {stop_id, index} <- Enum.with_index(direction_0_stop_sequence, 1) do + [stop] = + Floki.attribute( + direction_0_stop_rows, + "[data-stop_sequence=#{index}] > div > div.form-group > div > div > div > input[type=text]", + "value" + ) + + assert stop =~ stop_id + end + + for {stop_id, index} <- Enum.with_index(direction_1_stop_sequence, 1) do + [stop] = + Floki.attribute( + direction_1_stop_rows, + "[data-stop_sequence=#{index}] > div > div.form-group > div > div > div > input[type=text]", + "value" + ) + + assert stop =~ stop_id + end + end + + @tag :authenticated_admin + test "displays error if uploaded stop IDs contain invalid stop IDs", %{conn: conn} do + {:ok, new_live, _html} = live(conn, ~p"/shuttles/new") + + definition = + file_input(new_live, "#shuttle-form", :definition, [ + %{ + name: "valid.xlsx", + content: File.read!("test/support/fixtures/xlsx/valid.xlsx") + } + ]) + + direction_0_stop_sequence = ~w(9328 5327 5271) + direction_1_stop_sequence = ~w(5271 5072 9328) + html = render_upload(definition, "valid.xlsx") + assert html =~ "Failed to upload definition:" + assert html =~ "not a valid stop ID '9328" + assert html =~ "not a valid stop ID '5072" + direction_0_stop_rows = Floki.find(html, "#stops-dir-0 > .row") direction_1_stop_rows = Floki.find(html, "#stops-dir-1 > .row") - for {stop_id, index} <- Enum.with_index(direction_0_stop_sequence) do - assert [^stop_id] = + for {_stop_id, index} <- Enum.with_index(direction_0_stop_sequence, 1) do + assert [] = Floki.attribute( direction_0_stop_rows, "[data-stop_sequence=#{index}] > div > div.form-group > div > div > div > input[type=text]", @@ -479,8 +533,8 @@ defmodule ArrowWeb.ShuttleLiveTest do ) end - for {stop_id, index} <- Enum.with_index(direction_1_stop_sequence) do - assert [^stop_id] = + for {_stop_id, index} <- Enum.with_index(direction_1_stop_sequence, 1) do + assert [] = Floki.attribute( direction_1_stop_rows, "[data-stop_sequence=#{index}] > div > div.form-group > div > div > div > input[type=text]",