improvement: handle required but not accepted values better

This commit is contained in:
Zach Daniel 2022-08-17 12:58:43 -04:00
parent c21c5e6ae7
commit 917131c21f
4 changed files with 131 additions and 32 deletions

View file

@ -293,36 +293,59 @@ defmodule Ash.Actions.Create do
if action.manual? do
{:ok, nil}
else
if upsert? do
resource
|> Ash.DataLayer.upsert(changeset, upsert_keys)
|> add_tenant(changeset)
|> manage_relationships(api, changeset,
actor: actor,
authorize?: authorize?,
upsert?: upsert?
)
else
resource
|> Ash.DataLayer.create(changeset)
|> add_tenant(changeset)
|> manage_relationships(api, changeset,
actor: actor,
authorize?: authorize?,
upsert?: upsert?
)
end
|> case do
{:ok, result, notifications} ->
{:ok, result,
Map.update!(
notifications,
:notifications,
&(&1 ++ manage_instructions.notifications)
)}
final_check =
changeset.resource
|> Ash.Resource.Info.attributes()
|> Enum.reject(&(&1.allow_nil? || &1.generated?))
{:error, error} ->
{:error, error}
changeset =
changeset
|> Ash.Changeset.require_values(
:create,
true,
final_check
)
{changeset, _} =
Ash.Actions.ManagedRelationships.validate_required_belongs_to(
{changeset, []},
false
)
if changeset.valid? do
if upsert? do
resource
|> Ash.DataLayer.upsert(changeset, upsert_keys)
|> add_tenant(changeset)
|> manage_relationships(api, changeset,
actor: actor,
authorize?: authorize?,
upsert?: upsert?
)
else
resource
|> Ash.DataLayer.create(changeset)
|> add_tenant(changeset)
|> manage_relationships(api, changeset,
actor: actor,
authorize?: authorize?,
upsert?: upsert?
)
end
|> case do
{:ok, result, notifications} ->
{:ok, result,
Map.update!(
notifications,
:notifications,
&(&1 ++ manage_instructions.notifications)
)}
{:error, error} ->
{:error, error}
end
else
{:error, changeset.errors}
end
end
else

View file

@ -325,14 +325,24 @@ defmodule Ash.Actions.ManagedRelationships do
)
end
defp validate_required_belongs_to({:error, error}), do: {:error, error}
def validate_required_belongs_to(changeset_instructions_or_error, preflight? \\ true)
def validate_required_belongs_to({:error, error}, _), do: {:error, error}
defp validate_required_belongs_to({changeset, instructions}) do
def validate_required_belongs_to({changeset, instructions}, preflight?) do
changeset.resource
|> Ash.Resource.Info.relationships()
|> Enum.filter(&(&1.type == :belongs_to && &1.required?))
|> Enum.reject(fn relationship ->
changeset.context[:private][:error][relationship.name]
errored? = changeset.context[:private][:error][relationship.name]
not_changing? =
if preflight? do
false
else
!Map.has_key?(changeset.relationships, relationship.name)
end
errored? || not_changing?
end)
|> Enum.reduce({changeset, instructions}, fn required_relationship,
{changeset, instructions} ->

View file

@ -1215,6 +1215,19 @@ defmodule Ash.Changeset do
# Attributes that are private and/or are the source field of a belongs_to relationship
# are typically not set by input, so they aren't required until the actual action
# is run.
defp attributes_to_require(resource, %{type: :create, accept: accept}, true) do
resource
|> Ash.Resource.Info.attributes()
|> Enum.reject(&(&1.allow_nil? || &1.generated?))
|> Enum.filter(&(&1.name in accept))
end
defp attributes_to_require(resource, %{type: :create, accept: accept} = action, false) do
resource
|> do_attributes_to_require(action)
|> Enum.filter(&(&1.name in accept))
end
defp attributes_to_require(resource, _action, true = _private_and_belongs_to?) do
resource
|> Ash.Resource.Info.attributes()
@ -1222,6 +1235,10 @@ defmodule Ash.Changeset do
end
defp attributes_to_require(resource, action, false = _private_and_belongs_to?) do
do_attributes_to_require(resource, action)
end
defp do_attributes_to_require(resource, action) do
belongs_to =
resource
|> Ash.Resource.Info.relationships()

View file

@ -99,6 +99,21 @@ defmodule Ash.Test.Actions.CreateTest do
end
end
defmodule ManualCreateAuthorWithRequiredId do
@moduledoc false
use Ash.Resource.Change
def change(changeset, _, _) do
Ash.Changeset.after_action(changeset, fn _, nil ->
{:ok,
Ash.Test.Actions.CreateTest.AuthorWithRequiredId
|> Ash.Changeset.for_create(:create, %{name: "manual"})
|> Ash.Changeset.force_change_attribute(:id, Ash.UUID.generate())
|> Ash.Test.Actions.CreateTest.Api.create!()}
end)
end
end
defmodule Author do
@moduledoc false
use Ash.Resource, data_layer: Ash.DataLayer.Ets
@ -138,6 +153,31 @@ defmodule Ash.Test.Actions.CreateTest do
end
end
defmodule AuthorWithRequiredId do
@moduledoc false
use Ash.Resource, data_layer: Ash.DataLayer.Ets
ets do
private?(true)
end
actions do
defaults [:create, :read, :update, :destroy]
create :manual_create do
accept []
manual? true
change ManualCreateAuthorWithRequiredId
end
end
attributes do
uuid_primary_key :id, generated?: false, default: nil
attribute(:name, :string, allow_nil?: false)
attribute(:bio, :string)
end
end
defmodule PostDefaults do
@moduledoc false
def garbage2, do: "garbage2"
@ -245,6 +285,7 @@ defmodule Ash.Test.Actions.CreateTest do
entries do
entry(Author)
entry(AuthorWithRequiredId)
entry(Post)
entry(Profile)
entry(ProfileWithBelongsTo)
@ -406,6 +447,14 @@ defmodule Ash.Test.Actions.CreateTest do
assert [%{name: "manual"}] = Api.read!(Author)
end
test "the manual action does not require values that aren't accepted" do
AuthorWithRequiredId
|> Ash.Changeset.for_create(:manual_create)
|> Api.create!()
assert [%{name: "manual"}] = Api.read!(AuthorWithRequiredId)
end
end
describe "require_attributes" do