From 1d4b2aeedbc2a86560a9ac4d633179bc4faca65d Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sun, 3 May 2026 14:11:38 -0500 Subject: [PATCH] fix(rover-planning): edit-mode remove_station deletes only the targeted row Clicking the trash icon on a stationary station in :edit mode before any other phx-change had fired emptied the form's stations params, which combined with cast_assoc(on_replace: :delete) wiped EVERY existing station instead of just the one the user clicked. Fall back to rebuilding the params from the persisted mission's stations when the form's current params don't already carry them, then delete only the targeted index. --- .../live/rover_planning_live/form.ex | 34 ++++++++++++++++-- .../live/rover_planning_live_test.exs | 35 +++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/lib/microwaveprop_web/live/rover_planning_live/form.ex b/lib/microwaveprop_web/live/rover_planning_live/form.ex index 8c5496e4..f43f813a 100644 --- a/lib/microwaveprop_web/live/rover_planning_live/form.ex +++ b/lib/microwaveprop_web/live/rover_planning_live/form.ex @@ -91,7 +91,7 @@ defmodule MicrowavepropWeb.RoverPlanningLive.Form do def handle_event("add_station", _params, socket) do params = current_params(socket) - stations = Map.get(params, "stations", %{}) + stations = stations_params(socket, params) next_index = next_station_index(stations) # Don't include "input" in the new station's params — `used_input?/1` @@ -113,8 +113,7 @@ defmodule MicrowavepropWeb.RoverPlanningLive.Form do def handle_event("remove_station", %{"index" => index}, socket) do params = current_params(socket) - stations = Map.get(params, "stations", %{}) - stations = Map.delete(stations, index) + stations = socket |> stations_params(params) |> Map.delete(index) cs = socket.assigns.mission @@ -124,6 +123,35 @@ defmodule MicrowavepropWeb.RoverPlanningLive.Form do {:noreply, assign(socket, form: to_form(cs))} end + # When the form changeset already carries a "stations" params map + # (every keystroke fires phx-change="validate" which builds one), + # trust it. Otherwise — e.g. the user clicked add/remove BEFORE + # touching any other input on an :edit-mode form — fall back to + # rebuilding the params from the persisted mission's stations so + # `cast_assoc(on_replace: :delete)` doesn't wipe every existing row. + defp stations_params(socket, %{"stations" => stations}) when is_map(stations) and stations != %{}, do: stations + + defp stations_params(socket, _params), do: stations_from_mission(socket.assigns.mission) + + defp stations_from_mission(%Mission{stations: list}) when is_list(list) do + list + |> Enum.with_index() + |> Map.new(fn {s, i} -> + {Integer.to_string(i), + %{ + "id" => s.id, + "input" => s.input || s.callsign || s.grid, + "callsign" => s.callsign, + "grid" => s.grid, + "lat" => s.lat, + "lon" => s.lon, + "position" => Integer.to_string(s.position || i) + }} + end) + end + + defp stations_from_mission(_), do: %{} + def handle_event("save", %{"mission" => params}, socket) do user = socket.assigns.current_user diff --git a/test/microwaveprop_web/live/rover_planning_live_test.exs b/test/microwaveprop_web/live/rover_planning_live_test.exs index 5526bf3b..199086b4 100644 --- a/test/microwaveprop_web/live/rover_planning_live_test.exs +++ b/test/microwaveprop_web/live/rover_planning_live_test.exs @@ -565,6 +565,41 @@ defmodule MicrowavepropWeb.RoverPlanningLiveTest do live(conn, ~p"/rover-planning/new") end + test "edit-mode remove_station deletes ONLY the targeted station", %{conn: conn} do + user = AccountsFixtures.user_fixture() + _loc = AccountsFixtures.user_fixture() + + mission = + create_mission(user, "Multi-station edit", + stations: %{ + "0" => %{"input" => "EM12kp", "position" => 0}, + "1" => %{"input" => "EM13ks", "position" => 1}, + "2" => %{"input" => "EM14lr", "position" => 2} + } + ) + + conn = log_in_user(conn, user) + {:ok, lv, _html} = live(conn, ~p"/rover-planning/#{mission.id}/edit") + + # Click the trash icon on the second station (index 1) without + # any prior validate firing — this was the bug: an empty + # current_params + cast_assoc(on_replace: :delete) wiped every + # existing station instead of just the targeted index. + _html = + lv + |> element("button[phx-click='remove_station'][phx-value-index='1']") + |> render_click() + + # Save and assert only stations 0 and 2 remain. + {:error, {:live_redirect, _}} = + lv + |> form("#mission-form") + |> render_submit() + + reloaded = Repo.preload(Repo.get!(Microwaveprop.RoverPlanning.Mission, mission.id), :stations) + assert length(reloaded.stations) == 2 + end + test "clicking 'Add station' does not show 'can't be blank' on the new row", %{conn: conn} do user = AccountsFixtures.user_fixture() conn = log_in_user(conn, user)