fix: bulk action global validation ordering and before_action? support#2623
Closed
nallwhy wants to merge 3 commits intoash-project:mainfrom
Closed
fix: bulk action global validation ordering and before_action? support#2623nallwhy wants to merge 3 commits intoash-project:mainfrom
nallwhy wants to merge 3 commits intoash-project:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes bulk create’s change/validation pipeline so it matches the non-bulk for_create behavior, including honoring before_action? validations.
Changes:
- Reorders global changes vs global validations in
bulk_createso global changes run before global validations. - Adds
before_action?support for bulk validations by registering them viaAsh.Changeset.before_action/2. - Adds regression tests covering both ordering and
before_action?behavior for global validations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
lib/ash/actions/create/bulk.ex |
Fixes global change/validation ordering and defers before_action? validations via changeset hooks (with extracted helper). |
test/actions/bulk/bulk_create_test.exs |
Adds coverage to ensure global changes precede global validations and before_action? validations behave like the non-bulk pipeline. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9e839b0 to
c9a44fa
Compare
af9f175 to
a1fb430
Compare
516bdf0 to
ad92994
Compare
Contributor
|
Can we separate these two changes? The reason for that is that |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributor checklist
Leave anything that you believe does not apply unchecked.
Summary
bulk_create,bulk_update, andbulk_destroyto match non-bulk behaviorbefore_action?support for validations in the bulk create and bulk update pipelinesProblem
Two bugs across all bulk action modules (
Ash.Actions.Create.Bulk,Ash.Actions.Update.Bulk,Ash.Actions.Destroy.Bulk):Reversed ordering: Global validations were concatenated before global changes in
pre_template_all_changes.The non-bulk path runs action changes + global changes first, then global validations,
but the bulk path was concatenating in action changes → global validations → global changes order.
before_action?ignored: Thebefore_action?flag on validations was completely ignored in the bulk path, causing them to run inline.The non-bulk path checks this flag and defers via
Ash.Changeset.before_action/2,but the bulk
run_action_changesnever read this field.Changes
lib/ash/actions/create/bulk.expre_template_all_changes: Reorder concat so global changes come before global validationsrun_action_changes: Checkbefore_action?on validations; when true, register viaAsh.Changeset.before_action/2instead of running inlinerun_bulk_validation/6validation.messageerror handling to reduce duplicationlib/ash/actions/update/bulk.expre_template_all_changes: Same ordering fix as bulk_createrun_action_changes: Addbefore_action?support for validationsvalidation.messageerror handling invalidate_batch_non_atomicallylib/ash/actions/destroy/bulk.expre_template_all_changes: Same ordering fix as bulk_createbefore_action?support inherited via delegation toAsh.Actions.Update.Bulk.run_action_changes