From 5f6e91afd5a213a45889776c144bed4d5225cc22 Mon Sep 17 00:00:00 2001 From: Chris McCord Date: Mon, 5 Feb 2024 14:21:46 -0500 Subject: [PATCH 1/5] Consider form action and method changing as changed --- lib/phoenix_html/form.ex | 13 +++++++++---- lib/phoenix_html/form_data.ex | 14 ++++++++++++++ test/phoenix_html/form_test.exs | 7 +++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/lib/phoenix_html/form.ex b/lib/phoenix_html/form.ex index 794252d..1d4ab67 100644 --- a/lib/phoenix_html/form.ex +++ b/lib/phoenix_html/form.ex @@ -64,6 +64,8 @@ defmodule Phoenix.HTML.Form do id: nil, name: nil, data: nil, + action: nil, + method: nil, hidden: [], params: %{}, errors: [], @@ -74,6 +76,8 @@ defmodule Phoenix.HTML.Form do source: Phoenix.HTML.FormData.t(), name: String.t(), data: %{field => term}, + action: nil | atom() | String.t(), + method: nil | atom() | String.t(), params: %{binary => term}, hidden: Keyword.t(), options: Keyword.t(), @@ -191,9 +195,9 @@ defmodule Phoenix.HTML.Form do @doc """ Receives two forms structs and checks if the given field changed. - The field will have changed if either its associated value or errors - changed. This is mostly used for optimization engines as an extension - of the `Access` behaviour. + The field will have changed if either its associated value, errors, + action, method, or implementation changed. This is mostly used for optimization + engines as an extension of the `Access` behaviour. """ @spec input_changed?(t, t, field()) :: boolean() def input_changed?( @@ -202,7 +206,8 @@ defmodule Phoenix.HTML.Form do field ) when is_atom(field) or is_binary(field) do - impl1 != impl2 or id1 != id2 or name1 != name2 or + impl1 != impl2 or id1 != id2 or name1 != name2 or form1.action != form2.action or + form1.method != form2.method or field_errors(errors1, field) != field_errors(errors2, field) or impl1.input_value(source1, form1, field) != impl2.input_value(source2, form2, field) end diff --git a/lib/phoenix_html/form_data.ex b/lib/phoenix_html/form_data.ex index 02eec75..afa5a6a 100644 --- a/lib/phoenix_html/form_data.ex +++ b/lib/phoenix_html/form_data.ex @@ -56,6 +56,10 @@ defprotocol Phoenix.HTML.FormData do applies if the field value is a list and no parameters were sent through the form. + * `:action` - The form action, such as the HTML `action` attribute + or LiveView action. + + * `:method` - The form method, such as the HTML `method` attribute. """ @spec to_form(t, Phoenix.HTML.Form.t(), Phoenix.HTML.Form.field(), Keyword.t()) :: [Phoenix.HTML.Form.t()] @@ -79,6 +83,8 @@ defimpl Phoenix.HTML.FormData, for: Map do def to_form(conn_or_atom_or_map, opts) do {name, params, opts} = name_params_and_opts(conn_or_atom_or_map, opts) {errors, opts} = Keyword.pop(opts, :errors, []) + {action, opts} = Keyword.pop(opts, :action, nil) + {method, opts} = Keyword.pop(opts, :method, nil) id = Keyword.get(opts, :id) || name unless is_binary(id) or is_nil(id) do @@ -93,6 +99,8 @@ defimpl Phoenix.HTML.FormData, for: Map do params: params, data: %{}, errors: errors, + action: action, + method: method, options: opts } end @@ -118,6 +126,8 @@ defimpl Phoenix.HTML.FormData, for: Map do {name, opts} = Keyword.pop(opts, :as) {id, opts} = Keyword.pop(opts, :id) {hidden, opts} = Keyword.pop(opts, :hidden, []) + {action, opts} = Keyword.pop(opts, :action) + {method, opts} = Keyword.pop(opts, :method) id = to_string(id || form.id <> "_#{field}") name = to_string(name || form.name <> "[#{field}]") @@ -133,6 +143,8 @@ defimpl Phoenix.HTML.FormData, for: Map do id: id, name: name, data: default, + action: action, + method: method, params: params || %{}, hidden: hidden, options: opts @@ -157,6 +169,8 @@ defimpl Phoenix.HTML.FormData, for: Map do source: conn_or_atom_or_map, impl: __MODULE__, index: index, + action: action, + method: method, id: id <> "_" <> index_string, name: name <> "[" <> index_string <> "]", data: data, diff --git a/test/phoenix_html/form_test.exs b/test/phoenix_html/form_test.exs index cb11c45..bcc51ac 100644 --- a/test/phoenix_html/form_test.exs +++ b/test/phoenix_html/form_test.exs @@ -138,6 +138,13 @@ defmodule Phoenix.HTML.FormTest do assert input_changed?(form, form(%{"foo" => "bar"}), "foo") end + test "input_changed? with changed action or method" do + form = form(%{}, action: :validate, method: "post") + refute input_changed?(form, %{form | action: :validate, method: "post"}, :foo) + assert input_changed?(form, %{form | action: :save}, :foo) + assert input_changed?(form, %{form | method: "put"}, :foo) + end + describe "access" do test "without name and atom keys" do form = From f7bd1f6613f855d7bc6661f3d58289bfe7837a0f Mon Sep 17 00:00:00 2001 From: Chris McCord Date: Mon, 5 Feb 2024 14:40:35 -0500 Subject: [PATCH 2/5] No need to track id --- lib/phoenix_html/form.ex | 5 +---- lib/phoenix_html/form_data.ex | 7 ------- test/phoenix_html/form_test.exs | 5 ++--- 3 files changed, 3 insertions(+), 14 deletions(-) diff --git a/lib/phoenix_html/form.ex b/lib/phoenix_html/form.ex index 1d4ab67..93d8cd2 100644 --- a/lib/phoenix_html/form.ex +++ b/lib/phoenix_html/form.ex @@ -65,7 +65,6 @@ defmodule Phoenix.HTML.Form do name: nil, data: nil, action: nil, - method: nil, hidden: [], params: %{}, errors: [], @@ -77,7 +76,6 @@ defmodule Phoenix.HTML.Form do name: String.t(), data: %{field => term}, action: nil | atom() | String.t(), - method: nil | atom() | String.t(), params: %{binary => term}, hidden: Keyword.t(), options: Keyword.t(), @@ -196,7 +194,7 @@ defmodule Phoenix.HTML.Form do Receives two forms structs and checks if the given field changed. The field will have changed if either its associated value, errors, - action, method, or implementation changed. This is mostly used for optimization + action, or implementation changed. This is mostly used for optimization engines as an extension of the `Access` behaviour. """ @spec input_changed?(t, t, field()) :: boolean() @@ -207,7 +205,6 @@ defmodule Phoenix.HTML.Form do ) when is_atom(field) or is_binary(field) do impl1 != impl2 or id1 != id2 or name1 != name2 or form1.action != form2.action or - form1.method != form2.method or field_errors(errors1, field) != field_errors(errors2, field) or impl1.input_value(source1, form1, field) != impl2.input_value(source2, form2, field) end diff --git a/lib/phoenix_html/form_data.ex b/lib/phoenix_html/form_data.ex index afa5a6a..ecd3e3b 100644 --- a/lib/phoenix_html/form_data.ex +++ b/lib/phoenix_html/form_data.ex @@ -58,8 +58,6 @@ defprotocol Phoenix.HTML.FormData do * `:action` - The form action, such as the HTML `action` attribute or LiveView action. - - * `:method` - The form method, such as the HTML `method` attribute. """ @spec to_form(t, Phoenix.HTML.Form.t(), Phoenix.HTML.Form.field(), Keyword.t()) :: [Phoenix.HTML.Form.t()] @@ -84,7 +82,6 @@ defimpl Phoenix.HTML.FormData, for: Map do {name, params, opts} = name_params_and_opts(conn_or_atom_or_map, opts) {errors, opts} = Keyword.pop(opts, :errors, []) {action, opts} = Keyword.pop(opts, :action, nil) - {method, opts} = Keyword.pop(opts, :method, nil) id = Keyword.get(opts, :id) || name unless is_binary(id) or is_nil(id) do @@ -100,7 +97,6 @@ defimpl Phoenix.HTML.FormData, for: Map do data: %{}, errors: errors, action: action, - method: method, options: opts } end @@ -127,7 +123,6 @@ defimpl Phoenix.HTML.FormData, for: Map do {id, opts} = Keyword.pop(opts, :id) {hidden, opts} = Keyword.pop(opts, :hidden, []) {action, opts} = Keyword.pop(opts, :action) - {method, opts} = Keyword.pop(opts, :method) id = to_string(id || form.id <> "_#{field}") name = to_string(name || form.name <> "[#{field}]") @@ -144,7 +139,6 @@ defimpl Phoenix.HTML.FormData, for: Map do name: name, data: default, action: action, - method: method, params: params || %{}, hidden: hidden, options: opts @@ -170,7 +164,6 @@ defimpl Phoenix.HTML.FormData, for: Map do impl: __MODULE__, index: index, action: action, - method: method, id: id <> "_" <> index_string, name: name <> "[" <> index_string <> "]", data: data, diff --git a/test/phoenix_html/form_test.exs b/test/phoenix_html/form_test.exs index bcc51ac..75fb40f 100644 --- a/test/phoenix_html/form_test.exs +++ b/test/phoenix_html/form_test.exs @@ -139,10 +139,9 @@ defmodule Phoenix.HTML.FormTest do end test "input_changed? with changed action or method" do - form = form(%{}, action: :validate, method: "post") - refute input_changed?(form, %{form | action: :validate, method: "post"}, :foo) + form = form(%{}, action: :validate) + refute input_changed?(form, %{form | action: :validate}, :foo) assert input_changed?(form, %{form | action: :save}, :foo) - assert input_changed?(form, %{form | method: "put"}, :foo) end describe "access" do From 304dfc387e274040a11daa577bb740675a57de29 Mon Sep 17 00:00:00 2001 From: Chris McCord Date: Tue, 6 Feb 2024 10:10:25 -0500 Subject: [PATCH 3/5] Bump docs --- lib/phoenix_html/form_data.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/phoenix_html/form_data.ex b/lib/phoenix_html/form_data.ex index ecd3e3b..0efc546 100644 --- a/lib/phoenix_html/form_data.ex +++ b/lib/phoenix_html/form_data.ex @@ -56,8 +56,8 @@ defprotocol Phoenix.HTML.FormData do applies if the field value is a list and no parameters were sent through the form. - * `:action` - The form action, such as the HTML `action` attribute - or LiveView action. + * `:action` - The user defined action being taken by the form, such + as `:validate`, `:save`, etc. """ @spec to_form(t, Phoenix.HTML.Form.t(), Phoenix.HTML.Form.field(), Keyword.t()) :: [Phoenix.HTML.Form.t()] From 3b960bed766392643c94843122d231272f45b11a Mon Sep 17 00:00:00 2001 From: Chris McCord Date: Tue, 6 Feb 2024 10:15:40 -0500 Subject: [PATCH 4/5] =?UTF-8?q?Make=20jos=C3=A9=20happy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/phoenix_html/form.ex | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/phoenix_html/form.ex b/lib/phoenix_html/form.ex index 93d8cd2..80b7888 100644 --- a/lib/phoenix_html/form.ex +++ b/lib/phoenix_html/form.ex @@ -199,12 +199,26 @@ defmodule Phoenix.HTML.Form do """ @spec input_changed?(t, t, field()) :: boolean() def input_changed?( - %Form{impl: impl1, id: id1, name: name1, errors: errors1, source: source1} = form1, - %Form{impl: impl2, id: id2, name: name2, errors: errors2, source: source2} = form2, + %Form{ + impl: impl1, + id: id1, + name: name1, + errors: errors1, + source: source1, + action: action1 + } = form1, + %Form{ + impl: impl2, + id: id2, + name: name2, + errors: errors2, + source: source2, + action: action2 + } = form2, field ) when is_atom(field) or is_binary(field) do - impl1 != impl2 or id1 != id2 or name1 != name2 or form1.action != form2.action or + impl1 != impl2 or id1 != id2 or name1 != name2 or action1 != action2 or field_errors(errors1, field) != field_errors(errors2, field) or impl1.input_value(source1, form1, field) != impl2.input_value(source2, form2, field) end From 323a65ae73fb45b97c9fea6af26057f3256e6835 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 6 Feb 2024 18:18:31 +0100 Subject: [PATCH 5/5] Update lib/phoenix_html/form.ex --- lib/phoenix_html/form.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/phoenix_html/form.ex b/lib/phoenix_html/form.ex index 80b7888..67e8c8b 100644 --- a/lib/phoenix_html/form.ex +++ b/lib/phoenix_html/form.ex @@ -75,7 +75,7 @@ defmodule Phoenix.HTML.Form do source: Phoenix.HTML.FormData.t(), name: String.t(), data: %{field => term}, - action: nil | atom() | String.t(), + action: atom(), params: %{binary => term}, hidden: Keyword.t(), options: Keyword.t(),