Skip to content

bypass required attribute check for atomic_set & remove required attribute errors#2547

Closed
rtorresware wants to merge 1 commit intoash-project:mainfrom
rtorresware:main
Closed

bypass required attribute check for atomic_set & remove required attribute errors#2547
rtorresware wants to merge 1 commit intoash-project:mainfrom
rtorresware:main

Conversation

@rtorresware
Copy link
Contributor

My clanker's justification of this change vs a bigger refactor:

I opted to clear the specific Ash.Error.Changes.Required for the attribute when atomic_set records a create atomic/value, because for_create/3 validates required attrs before atomic_set runs. The test pattern (for_create → atomic_set) is supported today, so without cleanup the changeset stays invalid even though the atomic expression will populate the value at INSERT. This keeps behavior consistent across data layers without re-running validations or invalidating the whole changeset.
Ideal fix: reorder validation so required checks see create_atomics (or update require_values/4 for :create to treat create_atomics as “set”), eliminating the need for post‑hoc error removal.

Opencode session: https://opncd.ai/share/pWJqbSRE

Regression tests live in ash_postgres PR, as this bug can only be replicated in sql backed data layers: ash-project/ash_postgres#684

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

@zachdaniel
Copy link
Contributor

We have a better way that we do this for regular atomics, we just need to adopt a similar strategy for create_atomics 😄 We set the expression itself to if is_nil(expr), do: expr, else: error(...)

  @doc false
  def handle_allow_nil_atomics(changeset, actor) do
    changeset.atomics
    |> Enum.reduce(changeset, fn {key, value}, changeset ->
      attribute = Ash.Resource.Info.attribute(changeset.resource, key)

      if attribute.primary_key? do
        changeset
      else
        allow_nil? =
          attribute.allow_nil? and attribute.name not in changeset.action.require_attributes

        if allow_nil? || not Ash.Expr.can_return_nil?(value) do
          value
        else
          if Ash.DataLayer.data_layer_can?(changeset.resource, :expr_error) do
            expr(
              if is_nil(type(^value, ^attribute.type, ^attribute.constraints)) do
                error(
                  ^Ash.Error.Changes.Required,
                  %{
                    field: ^attribute.name,
                    type: ^:attribute,
                    resource: ^changeset.resource
                  }
                )
              else
                ^value
              end
            )
          else
            {:not_atomic,
             "Failed to validate expression #{inspect(value)}: data layer `#{Ash.DataLayer.data_layer(changeset.resource)}` does not support the expr_error"}
          end
        end
        |> case do
          {:not_atomic, error} ->
            Ash.Changeset.add_error(changeset, error)

          value ->
            %{changeset | atomics: Keyword.put(changeset.atomics, key, value)}
        end
      end
    end)
    |> Ash.Changeset.hydrate_atomic_refs(actor, eager?: true)
    |> then(fn
      {:not_atomic, value} ->
        {:not_atomic, value}

      %Ash.Changeset{} = changeset ->
        if changeset.action.type == :update do
          attributes =
            changeset.attributes
            |> Map.keys()
            |> Enum.reject(&Ash.Resource.Info.attribute(changeset.resource, &1).allow_nil?)

          require_values(changeset, :update, false, attributes)
        else
          changeset
        end
    end)
  end

The correct answer here would be to adapt this.

@zachdaniel zachdaniel closed this Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants