Skip to content

Conversation

@zackattackz
Copy link
Contributor

@zackattackz zackattackz commented Feb 9, 2026

Reworks the ValidateRelationshipAttributes verifier so that it also warns of missing source attributes for manual relationships instead of ignoring them.

For manual, only do this specific type of verification, Don't do the rest of the verifications since they aren't applicable

Notes

Reason I made this is because I spent longer than I'd like debugging an error when defining a manual relationship on a resource without an :id primary key.

Specifically if you do this and then try to load the relationship you will get an error like this (See the NoSuchAttribute :id error):

     ** (Ash.Error.Unknown) 
     Bread Crumbs:
       > Exception raised in: Ash.Test.Actions.LoadTest.PostSecret.read

     Unknown Error

     * ** (WithClauseError) no with clause matching:

         #Ash.Query<
           resource: Ash.Test.Actions.LoadTest.PostSecret,
           action: :read,
           filter: #Ash.Filter<secret == "4">,
           load: [
             posts_with_secret: #Ash.Query<resource: Ash.Test.Actions.LoadTest.Post>
           ],
           errors: [
             %Ash.Error.Query.NoSuchAttribute{
               resource: Ash.Test.Actions.LoadTest.PostSecret,
               attribute: :id,
               splode: Ash.Error,
               bread_crumbs: [],
               vars: [],
               path: [],
               stacktrace: #Splode.Stacktrace<>,
               class: :invalid
             }
           ],
           select: [:secret]
         >

       (ash 3.15.0) lib/ash/actions/read/read.ex:88: Ash.Actions.Read.run/3
       (ash 3.15.0) lib/ash.ex:2535: Ash.load/3
       (ash 3.15.0) lib/ash.ex:2489: Ash.load/3
       (ash 3.15.0) lib/ash.ex:2391: Ash.load!/3
       test/actions/load_test.exs:1457: Ash.Test.Actions.LoadTest."test loads it allows loading manual relationships with non :id pkey, regardless of source_attribute"/1
       (ex_unit 1.19.0) lib/ex_unit/runner.ex:528: ExUnit.Runner.exec_test/2
       (stdlib 7.1) timer.erl:599: :timer.tc/2
       (ex_unit 1.19.0) lib/ex_unit/runner.ex:450: anonymous fn/6 in ExUnit.Runner.spawn_test_monitor/4
     code: |> Ash.load!([:posts_with_secret])
     stacktrace:
       (ash 3.15.0) lib/ash/actions/read/read.ex:459: anonymous fn/3 in Ash.Actions.Read.do_run/3
       (ash 3.15.0) lib/ash/actions/read/read.ex:459: Ash.Actions.Read.do_run/3
       (ash 3.15.0) lib/ash/actions/read/read.ex:89: anonymous fn/3 in Ash.Actions.Read.run/3
       (ash 3.15.0) lib/ash/actions/read/read.ex:88: Ash.Actions.Read.run/3
       (ash 3.15.0) lib/ash.ex:2535: Ash.load/3
       (ash 3.15.0) lib/ash.ex:2489: Ash.load/3
       (ash 3.15.0) lib/ash.ex:2391: Ash.load!/3
       test/actions/load_test.exs:1457: (test)

This is because source_attribute (:id by default) is selected for manual actions here

It might seem obvious, but it was confusing to me, and I hope that emitting this warning can help save people some time debugging this if they make this mistake.

If you want to reproduce the error yourself you can check out this branch and run the test it allows loading manual relationships with non :id pkey, regardless of source_attribute via mix test test/actions/load_test.exs:1443

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

Reworks the ValidateRelationshipAttributes verifier so that it also warns of missing source attributes for manual relationships instead of ignoring them.

For manual, only do this specific type of verification, Don't do the rest of the verifications since they aren't applicable
@zackattackz
Copy link
Contributor Author

zackattackz commented Feb 9, 2026

Also I added "Either define it or set source_attribute to another attribute." to the error msg because I thought it was more clear instructions. But if you don't want this (or want similar wording on other error messages?) just lmk

@zachdaniel
Copy link
Contributor

🤔 I don't believe that manual relationships need a source attribute. What wasn't working that caused you to add one?

@zackattackz
Copy link
Contributor Author

zackattackz commented Feb 9, 2026

It gets selected here

%{manual: {module, opts}, source_attribute: source_attribute} ->
fields =
module.select(opts)
[source_attribute | fields]
%{source_attribute: source_attribute} ->
[source_attribute]
end

@zackattackz
Copy link
Contributor Author

I'm not sure if the source attribute is actually is necessary beyond being selected - if its not then removing it from being selected in the code here could work?

Doing no_attributes? true and loading the manual relationship also seems to work.

@zachdaniel
Copy link
Contributor

Yeah, I think what needs to happen is that at compile time we should set no_attributes? to true for manual relationships automatically. There is likely a transformer that this could be added to.

@zackattackz
Copy link
Contributor Author

Makes sense yeah. I'll open a new PR with that soon

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