From bbd3d03eb491420577865d01201121ea19d32e16 Mon Sep 17 00:00:00 2001 From: Kepi Date: Wed, 28 Jan 2026 21:31:02 +0100 Subject: [PATCH 1/2] Fix dropping readonly fields from Change When experimenting with AshBackpex, I found out that :readonly fields aren't correctly dropped from a Change. As I have :update action which accepts only some of fields, I need this working. Only alternative would be to omit read only fields from edit form which doesn't suit my use case. I also added simple tests for this, but as there is currently no LiveView test infrastructure, I used dirty workaround and copied private drop_readonly_changes function directly to tests for now so I can see if it works. --- lib/backpex/live_components/form_component.ex | 4 +- test/field_test.exs | 80 +++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 test/field_test.exs diff --git a/lib/backpex/live_components/form_component.ex b/lib/backpex/live_components/form_component.ex index 82a2a666a..b80ff717f 100644 --- a/lib/backpex/live_components/form_component.ex +++ b/lib/backpex/live_components/form_component.ex @@ -408,8 +408,8 @@ defmodule Backpex.FormComponent do defp drop_readonly_changes(change, fields, assigns) do read_only = fields - |> Enum.filter(&Backpex.Field.readonly?(&1, assigns)) - |> Enum.map(&Atom.to_string(&1.name)) + |> Enum.filter(fn {_name, options} -> Backpex.Field.readonly?(options, assigns) end) + |> Enum.map(fn {name, _options} -> Atom.to_string(name) end) Map.drop(change, read_only) end diff --git a/test/field_test.exs b/test/field_test.exs new file mode 100644 index 000000000..0a1dd5dfd --- /dev/null +++ b/test/field_test.exs @@ -0,0 +1,80 @@ +defmodule Backpex.FieldTest do + use ExUnit.Case, async: true + + alias Backpex.Field + + # Simulates a LiveResource fields/0 callback structure + defmodule UpstreamPrices do + def fields do + [ + name: %{module: Backpex.Fields.Text, label: "Name"}, + upstream_price: %{module: Backpex.Fields.Number, label: "Upstream Price", readonly: true}, + override_price: %{module: Backpex.Fields.Number, label: "Our Price"} + ] + end + end + + describe "readonly fields filtering" do + # This test mirrors the logic in Backpex.LiveComponents.FormComponent.drop_readonly_changes/3 + # which is a private function. We test the same logic here to ensure readonly fields + # are properly filtered from form params before changeset creation. + + test "readonly fields are correctly filtered from LiveResource fields" do + fields = UpstreamPrices.fields() + change = %{"name" => "Backpack", "upstream_price" => "100", "override_price" => "200"} + + filtered = drop_readonly_changes(change, fields, %{}) + + assert filtered == %{"name" => "Backpack", "override_price" => "200"} + refute Map.has_key?(filtered, "upstream_price") + end + + test "readonly function field is filtered when condition is true" do + fields = [ + name: %{module: Backpex.Fields.Text, label: "Name"}, + secret: %{module: Backpex.Fields.Text, label: "Secret", readonly: fn assigns -> assigns[:role] == :viewer end} + ] + + change = %{"name" => "Test", "secret" => "hidden"} + + # As viewer - secret should be filtered + filtered = drop_readonly_changes(change, fields, %{role: :viewer}) + assert filtered == %{"name" => "Test"} + + # As admin - secret should remain + filtered = drop_readonly_changes(change, fields, %{role: :admin}) + assert filtered == %{"name" => "Test", "secret" => "hidden"} + end + end + + describe "readonly?/2" do + test "returns true for boolean true" do + assert Field.readonly?(%{readonly: true}, %{}) == true + end + + test "returns false for boolean false" do + assert Field.readonly?(%{readonly: false}, %{}) == false + end + + test "returns false when readonly key is missing" do + assert Field.readonly?(%{}, %{}) == false + end + + test "evaluates function against assigns" do + readonly_fn = fn assigns -> assigns[:role] == :viewer end + + assert Field.readonly?(%{readonly: readonly_fn}, %{role: :viewer}) == true + assert Field.readonly?(%{readonly: readonly_fn}, %{role: :admin}) == false + end + end + + # Mirrors Backpex.LiveComponents.FormComponent.drop_readonly_changes/3 + defp drop_readonly_changes(change, fields, assigns) do + read_only = + fields + |> Enum.filter(fn {_name, options} -> Field.readonly?(options, assigns) end) + |> Enum.map(fn {name, _options} -> Atom.to_string(name) end) + + Map.drop(change, read_only) + end +end From 24f8c82bf3de0d4acffa8781d71cdeb5e9675c8b Mon Sep 17 00:00:00 2001 From: Florian Arens <60519307+Flo0807@users.noreply.github.com> Date: Fri, 27 Mar 2026 12:12:28 +0100 Subject: [PATCH 2/2] Move `drop_readonly_changes/3` to `Backpex.Field` --- lib/backpex/field.ex | 15 +++++++++++++ lib/backpex/live_components/form_component.ex | 7 +----- test/field_test.exs | 22 ++++--------------- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/lib/backpex/field.ex b/lib/backpex/field.ex index 7dc82554d..3e9e9df67 100644 --- a/lib/backpex/field.ex +++ b/lib/backpex/field.ex @@ -357,6 +357,21 @@ defmodule Backpex.Field do def readonly?(%{readonly: readonly}, assigns) when is_function(readonly, 1), do: readonly.(assigns) def readonly?(_field_options, _assigns), do: false + @doc """ + Drops readonly field changes from the given change map. + + Takes a map of string-keyed form params, a keyword list of field definitions, + and assigns. Returns the change map with readonly field keys removed. + """ + def drop_readonly_changes(change, fields, assigns) do + read_only = + fields + |> Enum.filter(fn {_name, options} -> readonly?(options, assigns) end) + |> Enum.map(fn {name, _options} -> Atom.to_string(name) end) + + Map.drop(change, read_only) + end + def translate_error_fun(%{translate_error: translate_error}, _assigns) when is_function(translate_error, 1), do: translate_error diff --git a/lib/backpex/live_components/form_component.ex b/lib/backpex/live_components/form_component.ex index b80ff717f..a8979ea28 100644 --- a/lib/backpex/live_components/form_component.ex +++ b/lib/backpex/live_components/form_component.ex @@ -406,12 +406,7 @@ defmodule Backpex.FormComponent do end defp drop_readonly_changes(change, fields, assigns) do - read_only = - fields - |> Enum.filter(fn {_name, options} -> Backpex.Field.readonly?(options, assigns) end) - |> Enum.map(fn {name, _options} -> Atom.to_string(name) end) - - Map.drop(change, read_only) + Backpex.Field.drop_readonly_changes(change, fields, assigns) end defp drop_unused_changes(change) do diff --git a/test/field_test.exs b/test/field_test.exs index 0a1dd5dfd..4bc91e473 100644 --- a/test/field_test.exs +++ b/test/field_test.exs @@ -14,16 +14,12 @@ defmodule Backpex.FieldTest do end end - describe "readonly fields filtering" do - # This test mirrors the logic in Backpex.LiveComponents.FormComponent.drop_readonly_changes/3 - # which is a private function. We test the same logic here to ensure readonly fields - # are properly filtered from form params before changeset creation. - + describe "drop_readonly_changes/3" do test "readonly fields are correctly filtered from LiveResource fields" do fields = UpstreamPrices.fields() change = %{"name" => "Backpack", "upstream_price" => "100", "override_price" => "200"} - filtered = drop_readonly_changes(change, fields, %{}) + filtered = Field.drop_readonly_changes(change, fields, %{}) assert filtered == %{"name" => "Backpack", "override_price" => "200"} refute Map.has_key?(filtered, "upstream_price") @@ -38,11 +34,11 @@ defmodule Backpex.FieldTest do change = %{"name" => "Test", "secret" => "hidden"} # As viewer - secret should be filtered - filtered = drop_readonly_changes(change, fields, %{role: :viewer}) + filtered = Field.drop_readonly_changes(change, fields, %{role: :viewer}) assert filtered == %{"name" => "Test"} # As admin - secret should remain - filtered = drop_readonly_changes(change, fields, %{role: :admin}) + filtered = Field.drop_readonly_changes(change, fields, %{role: :admin}) assert filtered == %{"name" => "Test", "secret" => "hidden"} end end @@ -67,14 +63,4 @@ defmodule Backpex.FieldTest do assert Field.readonly?(%{readonly: readonly_fn}, %{role: :admin}) == false end end - - # Mirrors Backpex.LiveComponents.FormComponent.drop_readonly_changes/3 - defp drop_readonly_changes(change, fields, assigns) do - read_only = - fields - |> Enum.filter(fn {_name, options} -> Field.readonly?(options, assigns) end) - |> Enum.map(fn {name, _options} -> Atom.to_string(name) end) - - Map.drop(change, read_only) - end end