-
-
Notifications
You must be signed in to change notification settings - Fork 25
respect read action sort if relationship does not specify one #194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
lib/join.ex
Outdated
| query | ||
| end | ||
| end) | ||
| |> Ash.Query.unset([:distinct, :select, :limit, :offset]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need all three of these unset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, my bad. I think i copied them together with an IO.inspect and only removed the inspect. Fixed now
| |> Ash.Query.sort(relationship.sort) | ||
| else | ||
| Ash.Query.unset(query, :sort) | ||
| query |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sort? being false means that we aren't in a situation where sorting should happen at all (like exists/2. I think you want to always unset the sort, and then set the sort to relationship.sort || query.sort, and if sort? is false then you just unset it so we don't sort for no reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I miss-interpreted what sort? stood for, fixed
|
🚀 Thank you for your contribution! 🚀 |
Test for this in ash-project/ash_postgres#654
Contributor checklist
Leave anything that you believe does not apply unchecked.