Skip to content

Commit

Permalink
fix: detect non-changing but setting attributes to honor `require_att…
Browse files Browse the repository at this point in the history
…ributes` on update
  • Loading branch information
zachdaniel committed Sep 25, 2024
1 parent 42dd2ea commit d3aaf56
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
12 changes: 10 additions & 2 deletions lib/ash/changeset/changeset.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1645,8 +1645,11 @@ defmodule Ash.Changeset do
if attribute.primary_key? do
changeset
else
allow_nil? =
attribute.allow_nil? and attribute.name not in changeset.action.require_attributes

value =
if attribute.allow_nil? || not Ash.Expr.can_return_nil?(value) do
if allow_nil? || not Ash.Expr.can_return_nil?(value) do
value
else
expr(
Expand Down Expand Up @@ -3111,7 +3114,12 @@ defmodule Ash.Changeset do
end
end)
|> Enum.reduce(changeset, fn required_attribute, changeset ->
if changing_attribute?(changeset, required_attribute.name) do
setting? =
Map.has_key?(changeset.attributes, required_attribute.name) ||
Keyword.has_key?(changeset.atomics, required_attribute.name) ||
Map.has_key?(changeset.casted_attributes, required_attribute.name)

if setting? do
if is_nil(get_attribute(changeset, required_attribute.name)) do
if required_attribute.name in changeset.invalid_keys do
changeset
Expand Down
1 change: 0 additions & 1 deletion lib/ash/filter/filter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2998,7 +2998,6 @@ defmodule Ash.Filter do
end
|> case do
false ->
IO.inspect(context)
false

true ->
Expand Down
16 changes: 12 additions & 4 deletions test/actions/update_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ defmodule Ash.Test.Actions.UpdateTest do

update :set_nilable do
accept [:nilable]
require_atomic? false
atomic_upgrade? true
require_attributes [:nilable]
end

Expand Down Expand Up @@ -434,17 +436,23 @@ defmodule Ash.Test.Actions.UpdateTest do
end

describe "require_attributes" do
test "it requires providing the attribute" do
test "it prevents setting the attribute to `nil`" do
profile =
Profile
|> Ash.Changeset.for_create(:create, %{nilable: "foobar"})
|> Ash.Changeset.for_create(:create, %{bio: "foobar"})
|> Ash.create!()

assert_raise Ash.Error.Invalid, ~r/bio is required/, fn ->
assert_raise Ash.Error.Invalid, ~r/nilable is required/, fn ->
profile
|> Ash.Changeset.for_update(:set_nilable, %{})
|> Ash.Changeset.for_update(:set_nilable, %{nilable: nil})
|> Ash.update!()
end

assert_raise Ash.Error.Invalid, ~r/nilable is required/, fn ->
profile
|> Ash.Changeset.for_update(:set_nilable, %{nilable: nil})
|> Ash.update!(atomic_upgrade?: false)
end
end
end

Expand Down

0 comments on commit d3aaf56

Please sign in to comment.