feat: add eager_validate? option to manage_relationship/4#2663
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an eager_validate? convenience option to Ash.Changeset.manage_relationship/4 to enable eager relationship existence validation without explicitly providing a domain, by inferring the domain from the destination (related) resource—mirroring the identity eager-check ergonomics introduced in #1040.
Changes:
- Add
eager_validate?: booleantomanage_relationship/4options schema and documentation. - When
eager_validate?: trueandeager_validate_withis not provided, infereager_validate_withfromAsh.Resource.Info.domain(destination), raising if missing. - Add a unit test covering eager validation via
eager_validate?for many-to-many upsert/relate behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lib/ash/changeset/changeset.ex |
Introduces the new option and domain inference logic inside manage_relationship/4. |
test/changeset/changeset_test.exs |
Adds coverage asserting eager validation works when using eager_validate?: true. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Validates that any referenced entities exist *before* the action is being performed, using the provided domain for the read." | ||
| ], | ||
| eager_validate?: [ | ||
| type: :boolean, | ||
| default: false, | ||
| doc: | ||
| "Validates that any referenced entities exist *before* the action is being performed, using the domain configured on the related resource." |
There was a problem hiding this comment.
The new eager_validate? option doc doesn’t mention that it will raise when the destination resource has no configured domain (and that eager_validate_with can be used instead / takes precedence). Consider updating the option docs to reflect the actual behavior so callers know when/why it errors.
| "Validates that any referenced entities exist *before* the action is being performed, using the provided domain for the read." | |
| ], | |
| eager_validate?: [ | |
| type: :boolean, | |
| default: false, | |
| doc: | |
| "Validates that any referenced entities exist *before* the action is being performed, using the domain configured on the related resource." | |
| "Validates that any referenced entities exist *before* the action is being performed, using the provided domain for the read. Takes precedence over `eager_validate?`." | |
| ], | |
| eager_validate?: [ | |
| type: :boolean, | |
| default: false, | |
| doc: | |
| "Validates that any referenced entities exist *before* the action is being performed, using the domain configured on the related resource. If the related resource has no configured domain, this raises unless `eager_validate_with` is provided." |
| {opts, keyword_opts} = | ||
| if opts.eager_validate? && !opts.eager_validate_with do | ||
| domain = Ash.Resource.Info.domain(relationship.destination) | ||
|
|
||
| if !domain do | ||
| raise ArgumentError, | ||
| "Cannot use `eager_validate?: true` because #{inspect(relationship.destination)} " <> | ||
| "does not have a domain configured. Use `eager_validate_with: YourDomain` instead." | ||
| end | ||
|
|
||
| new_opts = %{opts | eager_validate_with: domain} | ||
| {new_opts, ManageRelationshipOpts.to_options(new_opts)} | ||
| else | ||
| {opts, keyword_opts} | ||
| end |
There was a problem hiding this comment.
keyword_opts is generated before opts.meta is augmented with :inputs_was_list? (5983-5987), but in the eager_validate? branch you regenerate keyword_opts after modifying opts. This makes the stored options inconsistent depending on whether eager_validate? is set, and can affect later error-path behavior that depends on opts[:meta][:inputs_was_list?] (see Ash.Actions.ManagedRelationships.set_error_path/4). Consider moving the to_options/1 call to after the opts.meta update (and avoid conditionally regenerating it here) so all calls get consistent meta in the stored relationship opts.
| if !domain do | ||
| raise ArgumentError, | ||
| "Cannot use `eager_validate?: true` because #{inspect(relationship.destination)} " <> | ||
| "does not have a domain configured. Use `eager_validate_with: YourDomain` instead." | ||
| end |
There was a problem hiding this comment.
This new branch raises when eager_validate?: true is used and the destination resource has no configured domain. There are similar tests for identity eager_check domain inference errors; adding a regression test for this raise would help prevent accidental behavior changes and ensure the error message stays stable.
|
🚀 Thank you for your contribution! 🚀 |
Contributor checklist
Leave anything that you believe does not apply unchecked.
Summary
eager_validate?boolean option toAsh.Changeset.manage_relationship/4true, automatically infers the domain from the related resource's domain configurationeager_check?/eager_check_withpattern introduced for identities in feat: leverage resource domain for eager/pre checking #1040Closes #1074