Skip to content

Conversation

@kbrock
Copy link
Member

@kbrock kbrock commented Oct 9, 2025

Pulled out of #23617

We can treat the 2 PRs separately (and I'll remove these commits from ^)
or we can just merge the other PR.

@kbrock
Copy link
Member Author

kbrock commented Oct 9, 2025

We had missed adding eager_load and references to the scope. I was feeling concerned.
Turns out, the way this abstraction works, we only need to forward all and the rest of the operations are performed on the real active record scope.

update:

  • added tests
  • added a few comments

update:

  • updated commit comment to basically include what I typed in this comment.

@kbrock kbrock force-pushed the aaar_scope_dependency branch from cc8438f to 011d57c Compare October 9, 2025 21:09
@kbrock kbrock force-pushed the aaar_scope_dependency branch from 011d57c to 3aac4f3 Compare October 14, 2025 18:48
Turns out for AAAS case, we currently only need to define all
Rbac has already used all, so all methods go directly to the aar_scope (which an an active record scope)
So all of these other methods are moot. Well, for this implementation.

We do want it to be complete in case the code changes and someone decides
to do something like call eager_load before calling all
@kbrock kbrock force-pushed the aaar_scope_dependency branch from 3aac4f3 to b409771 Compare October 14, 2025 18:51
@kbrock
Copy link
Member Author

kbrock commented Oct 14, 2025

update:

  • rebase
  • dropped notes around the working of AAArM - there is a PR (from @Fryguy) that is moving that around
  • changed spec so I could drop a NOTE

@kbrock
Copy link
Member Author

kbrock commented Oct 14, 2025

update:

  • drop includes_to_references. This was dropped in a18d3da but did not remove this method

@Fryguy Fryguy merged commit 54e58a9 into ManageIQ:master Oct 15, 2025
9 checks passed
@kbrock kbrock deleted the aaar_scope_dependency branch October 15, 2025 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants