Skip to content

Add sorting by custom fields #549

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mattmatters
Copy link

Initial implementation of custom field sorting as describe in 511. This allows users to define fields they want to do arbitrary sorting without having to alias a field.

Another bonus is you can technically pass in variables to do some pretty fun dynamic sorting. Right now I'm using them to surface relavent information to a user (PostGIS coordinate compared to user location).

For now we handle those custom variables in our code and pass them in. In the future it would be cool to see that a bit more formalized since we have to basically repeat some boilerplate for verifying some of these custom sort variables.

One additional sticky situation I found while doing this implementation is cursor fields, given other complex fields are not supported I opted to do the same here. However in theory it wouldn't be much work to support, it really just requires that a get_field/1 callback can execute at runtime (a bit weird though if you rely on runtime options like I'm doing above), then we would just need to validate that a filter function is set in addition to the sorter function for the custom field.

Initial implementation of custom field sorting. This allows users to
define fields they want to do arbitrary sorting without having to
alias a field.

Another bonus is you can technically pass in variables to do some
pretty fun dynamic sorting. Right now I'm using them to surface
relavent information to a user (PostGIS coordinate compared to user
location).

For now we handle those custom variables in our code and pass them
in. In the future it would be cool to see that a bit more formalized
since we have to basically repeat some boilerplate for verifying some
of these custom sort variables.

One additional sticky situation I found while doing this
implementation is cursor fields, given other complex fields are not
supported I opted to do the same here. However in theory it wouldn't
be much work to support, it really just requires that a `get_field/1`
callback can execute at runtime (a bit weird though if you rely on
runtime options like I'm doing above), then we would just need to
validate that a filter function is set in addition to the sorter
function for the custom field.
@mattmatters mattmatters requested a review from woylie as a code owner March 23, 2025 20:34
@mattmatters
Copy link
Author

I haven't added any documentation yet, just wanted to make sure this was the direction you were thinking first.

@woylie
Copy link
Owner

woylie commented May 14, 2025

Hi @mattmatters, thanks for opening the PR. Looks good to me! Once you add documentation and fix the doc tests, we can merge this. If you want to add support for cursor pagination on custom fields, feel free to do so, but we can also deal with this later.

@bjoern-kahle
Copy link

@woylie Your work on this PR is awesome — great job!
I could help complete so we can get this merged urgently. I really need it, haha. Would it be okay with you if I push it to your PR?

@woylie
Copy link
Owner

woylie commented May 23, 2025

@woylie Your work on this PR is awesome — great job! I could help complete so we can get this merged urgently. I really need it, haha. Would it be okay with you if I push it to your PR?

I think you meant to ping @mattmatters.

@mattmatters
Copy link
Author

@woylie Your work on this PR is awesome — great job!
I could help complete so we can get this merged urgently. I really need it, haha. Would it be okay with you if I push it to your PR?

Of course! I will not have major time to work on this until next week or so. Absolutely fine if you want to use this as a launch point or if you want to clean up the tests.

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.

3 participants