🛠️ Fix active form vs form data loading

lib/phoenix_test/active_form.ex

@@ -1,17 +1,15 @@
 defmodule PhoenixTest.ActiveForm do
   @moduledoc false
 
-  alias PhoenixTest.Form
-
   defstruct [:id, :selector, form_data: [], uploads: []]
 
-  def new(form_or_opts \\ [])
-
-  def new(%Form{} = form) do
-    %__MODULE__{id: form.id, selector: form.selector, form_data: form.form_data}
-  end
+  @doc """
+  Data structure for tracking active form fields filled.
 
-  def new(opts) when is_list(opts) do
+  Do not keep track of default form data on the page. That's what
+  `PhoenixTest.Form.form_data` is for.
+  """
+  def new(opts \\ []) when is_list(opts) do
     struct!(%__MODULE__{}, opts)
   end
 

lib/phoenix_test/live.ex

@@ -70,7 +70,7 @@ defmodule PhoenixTest.Live do
 
         form_data =
           if active_form.selector == form.selector do
-            active_form.form_data
+            form.form_data ++ active_form.form_data
           else
             form.form_data
           end
@@ -200,24 +200,6 @@ defmodule PhoenixTest.Live do
     end
   end
 
-  defp fill_in_field_data(session, field) do
-    active_form = session.active_form
-    existing_data = active_form.form_data
-    new_form_data = FormData.to_form_data!(field)
-    form = Field.parent_form!(field)
-
-    form_data =
-      if active_form.selector == form.selector do
-        existing_data ++ new_form_data
-      else
-        new_form_data
-      end
-
-    additional_data = %{"_target" => field.name}
-
-    fill_form(session, form.selector, form_data, additional_data)
-  end
-
   def upload(session, input_selector, label, path, opts) do
     field =
       session
@@ -244,22 +226,26 @@ defmodule PhoenixTest.Live do
     |> maybe_redirect(session)
   end
 
-  defp fill_form(session, selector, form_data, additional_data) do
-    form =
-      session
-      |> render_html()
-      |> Form.find!(selector)
-
-    active_form =
-      form
-      |> ActiveForm.new()
-      |> ActiveForm.add_form_data(form_data)
+  defp fill_in_field_data(session, field) do
+    new_form_data = FormData.to_form_data!(field)
+    form = Field.parent_form!(field)
 
-    session = Map.put(session, :active_form, active_form)
+    session =
+      Map.update!(session, :active_form, fn active_form ->
+        if active_form.selector == form.selector do
+          ActiveForm.add_form_data(session.active_form, new_form_data)
+        else
+          ActiveForm.new(id: form.id, form_data: new_form_data, selector: form.selector)
+        end
+      end)
 
     if Form.phx_change?(form) do
+      active_form = session.active_form
+      data_to_submit = form.form_data ++ active_form.form_data
+      additional_data = %{"_target" => field.name}
+
       session.view
-      |> form(selector, Form.build_data(active_form.form_data))
+      |> form(form.selector, Form.build_data(data_to_submit))
       |> render_change(additional_data)
       |> maybe_redirect(session)
     else

lib/phoenix_test/static.ex

@@ -155,7 +155,7 @@ defmodule PhoenixTest.Static do
       if active_form.selector == form.selector do
         ActiveForm.add_upload(active_form, {field.name, upload})
       else
-        form
+        [id: form.id, selector: form.selector]
         |> ActiveForm.new()
         |> ActiveForm.add_upload({field.name, upload})
       end
@@ -185,10 +185,15 @@ defmodule PhoenixTest.Static do
       session
       |> render_html()
       |> Form.find!(selector)
+      |> then(fn form ->
+        Form.put_button_data(form, form.submit_button)
+      end)
+
+    to_submit = Form.build_data(form.form_data ++ form_data)
 
     session
-    |> update_active_form(form, form_data)
-    |> submit()
+    |> Map.put(:active_form, ActiveForm.new())
+    |> perform_submit(form, to_submit)
   end
 
   def open_browser(session, open_fun \\ &OpenBrowser.open_with_system_cmd/1) do
@@ -214,20 +219,16 @@ defmodule PhoenixTest.Static do
   end
 
   defp fill_in_field_data(session, field) do
-    active_form = session.active_form
-    existing_data = active_form.form_data
     new_form_data = FormData.to_form_data!(field)
-
     form = Field.parent_form!(field)
 
-    form_data =
+    Map.update!(session, :active_form, fn active_form ->
       if active_form.selector == form.selector do
-        existing_data ++ new_form_data
+        ActiveForm.add_form_data(active_form, new_form_data)
       else
-        new_form_data
+        ActiveForm.new(id: form.id, selector: form.selector, form_data: new_form_data)
       end
-
-    update_active_form(session, form, form_data)
+    end)
   end
 
   defp submit_active_form(session, form) do
@@ -251,15 +252,6 @@ defmodule PhoenixTest.Static do
     Enum.map(conn.req_headers, &elem(&1, 0))
   end
 
-  defp update_active_form(session, form, form_data) do
-    active_form =
-      form
-      |> ActiveForm.new()
-      |> ActiveForm.add_form_data(form_data)
-
-    Map.put(session, :active_form, active_form)
-  end
-
   defp no_active_form_error do
     %ArgumentError{
       message: "There's no active form. Fill in a form with `fill_in`, `select`, etc."

test/phoenix_test/active_form_test.exs

@@ -0,0 +1,16 @@
+defmodule PhoenixTest.ActiveFormTest do
+  use ExUnit.Case, async: true
+
+  alias PhoenixTest.ActiveForm
+
+  describe "add_form_data" do
+    test "adds form data passed" do
+      active_form =
+        [id: "user-form", selector: "#user-form"]
+        |> ActiveForm.new()
+        |> ActiveForm.add_form_data([{"user[name]", "Frodo"}])
+
+      assert active_form.form_data == [{"user[name]", "Frodo"}]
+    end
+  end
+end

test/phoenix_test/live_test.exs

@@ -369,40 +369,6 @@ defmodule PhoenixTest.LiveTest do
       |> assert_has("#form-data", text: "user:role: El Jefe")
     end
 
-    test "triggers phx-change validations", %{conn: conn} do
-      conn
-      |> visit("/live/index")
-      |> fill_in("Email", with: nil)
-      |> assert_has("#form-errors", text: "Errors present")
-    end
-
-    test "sends _target with phx-change events", %{conn: conn} do
-      conn
-      |> visit("/live/index")
-      |> fill_in("Email", with: "[email protected]")
-      |> assert_has("#form-data", text: "_target: [email]")
-    end
-
-    test "does not trigger phx-change event if one isn't present", %{conn: conn} do
-      session = visit(conn, "/live/index")
-
-      starting_html = Driver.render_html(session)
-
-      ending_html =
-        session
-        |> within("#no-phx-change-form", &fill_in(&1, "Name", with: "Aragorn"))
-        |> Driver.render_html()
-
-      assert starting_html == ending_html
-    end
-
-    test "follows redirects on phx-change", %{conn: conn} do
-      conn
-      |> visit("/live/index")
-      |> fill_in("Email with redirect", with: "[email protected]")
-      |> assert_has("h1", text: "LiveView page 2")
-    end
-
     test "can be used to submit form", %{conn: conn} do
       conn
       |> visit("/live/index")
@@ -1032,4 +998,53 @@ defmodule PhoenixTest.LiveTest do
       assert PhoenixTest.Driver.current_path(session) == "/live/index?foo=bar"
     end
   end
+
+  describe "shared form helpers behavior" do
+    test "triggers phx-change validations", %{conn: conn} do
+      conn
+      |> visit("/live/index")
+      |> fill_in("Email", with: nil)
+      |> assert_has("#form-errors", text: "Errors present")
+    end
+
+    test "sends _target with phx-change events", %{conn: conn} do
+      conn
+      |> visit("/live/index")
+      |> fill_in("Email", with: "[email protected]")
+      |> assert_has("#form-data", text: "_target: [email]")
+    end
+
+    test "does not trigger phx-change event if one isn't present", %{conn: conn} do
+      session = visit(conn, "/live/index")
+
+      starting_html = Driver.render_html(session)
+
+      ending_html =
+        session
+        |> within("#no-phx-change-form", &fill_in(&1, "Name", with: "Aragorn"))
+        |> Driver.render_html()
+
+      assert starting_html == ending_html
+    end
+
+    test "follows redirects on phx-change", %{conn: conn} do
+      conn
+      |> visit("/live/index")
+      |> fill_in("Email with redirect", with: "[email protected]")
+      |> assert_has("h1", text: "LiveView page 2")
+    end
+
+    test "preserves correct order of active form vs form data", %{conn: conn} do
+      conn
+      |> visit("/live/index")
+      |> within("#changes-hidden-input-form", fn session ->
+        session
+        |> fill_in("Name", with: "Frodo")
+        |> fill_in("Email", with: "[email protected]")
+      end)
+      |> assert_has("#form-data", text: "name: Frodo")
+      |> assert_has("#form-data", text: "email: [email protected]")
+      |> assert_has("#form-data", text: "hidden_race: hobbit")
+    end
+  end
 end

test/support/index_live.ex

@@ -376,6 +376,13 @@ defmodule PhoenixTest.IndexLive do
       </button>
     </form>
 
+    <form id="changes-hidden-input-form" phx-change="set-hidden-race">
+      <input type="hidden" name="hidden_race" value={@hidden_input_race} />
+
+      <label>Email <input type="email" name="email" /></label>
+      <label>Name <input type="name" name="name" /></label>
+    </form>
+
     <div id="not-a-form">
       <fieldset>
         <legend>Select a maintenance drone:</legend>
@@ -452,6 +459,7 @@ defmodule PhoenixTest.IndexLive do
       |> assign(:form_data, %{})
       |> assign(:show_form_errors, false)
       |> assign(:cities, [])
+      |> assign(:hidden_input_race, "human")
       |> allow_upload(:avatar, accept: ~w(.jpg .jpeg))
       |> allow_upload(:main_avatar, accept: ~w(.jpg .jpeg))
       |> allow_upload(:backup_avatar, accept: ~w(.jpg .jpeg))
@@ -488,6 +496,22 @@ defmodule PhoenixTest.IndexLive do
     }
   end
 
+  def handle_event("set-hidden-race", form_data, socket) do
+    race =
+      case form_data["name"] do
+        "Frodo" -> "hobbit"
+        _ -> "human"
+      end
+
+    {
+      :noreply,
+      socket
+      |> assign(:form_saved, true)
+      |> assign(:form_data, form_data)
+      |> assign(:hidden_input_race, race)
+    }
+  end
+
   def handle_event("save-redirect-form", _, socket) do
     {:noreply, push_navigate(socket, to: "/live/page_2")}
   end