Skip to content

fix: setting undeclared argument was done with nil key - #2925

Open
Rutgerdj wants to merge 2 commits into
ash-project:mainfrom
Rutgerdj:bugs/missing-argument-set-on-changeset
Open

fix: setting undeclared argument was done with nil key#2925
Rutgerdj wants to merge 2 commits into
ash-project:mainfrom
Rutgerdj:bugs/missing-argument-set-on-changeset

Conversation

@Rutgerdj

Copy link
Copy Markdown
Contributor

Setting an argument on the changeset that is not defined by the action put the value in the arguments map with a nil key.
This was caused by the argument variable being reassigned

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

@@ -6612,29 +6612,33 @@ defmodule Ash.Changeset do

defp do_set_argument(changeset, argument, value, store_casted? \\ false) do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I didn't even realize we stored undefined arguments. We shouldn't be putting these into the arguments at all. Could we make that change instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I just thought this was undocumented bahaviour :D

I've updated the logic to attach an error to the Changeset. Thats a breaking change tho. Before it failed silently, now it fails loudly.

That causes a Reactor test (test/reactor/change_test.exs:44) to fail.
I've had our shared friend take a look at it but couldn't confidently review it's suggestions so leaving this open.

@Rutgerdj
Rutgerdj force-pushed the bugs/missing-argument-set-on-changeset branch from 0c979d1 to 7ebec67 Compare September 11, 2026 13:16
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