feat: sort by multiple columns

[andershermansen]

This change adds support for sort with multiple columns in local API and REST API. I looked at the comment from DanRibbens in old PR #3236 for v1 and implemented this as a comma separated string only.

Related discussion #2089

I added a new integration test suite for sorting as I did not find any other good place to test this. It includes current functionality for sort with both regular collections, drafts and versions to make sure I did not break anything. And of course testing the new functionality implemented.

I also included documentation updates.

When testing localization the new test suite actually uncovers an existing error in drizzle localized sorting. The integration test for localization works with mongodb, but fails with postgres and sqlite (drizzle). This is already reported by another user in issue #5152
I left the test is in this PR, but commented out. I have not yet used enough time to look into how the drizzle package deals with localization to be able to fix this. Anyways, it's out of scope for this PR and should be a separate fix.

For findVersions I'm a bit surprised that you need to prepend the sort string with "version." for it to work. Is this expected?

[r1tsuu]

Thank you @andershermansen, when I found about Payload I was surprised that it's not a thing and opened a PR

@DanRibbens will review this, but I want to throw my point about type-safety. We plan to make where type-safe, also the select fields API that's already implemented here #8550 is completely type-safe, meaning you get a full autocomplete for it.

And sort should be type-safe at some point too. We'll generate types for it with all the fields that are possible to sort by. And I feel like with comma-separated strings it won't be possible to have a nice autocomplete. With Template Literal Types we can enforce type-safety on those, but they don't provide autocompletion which isn't good for DX

Array (sort: ['isFavorite', '-createdAt']) could work here nicer for this in the Local API, with REST API it'll just map to the same comma-separated string, thoughts?

For findVersions I'm a bit surprised that you need to prepend the sort string with "version." for it to work. Is this expected?

Yep, because you need to sort by a nested property there. As the actual data in versions is stored inside of the version group. We do the same for where when querying drafts, we append version. for each key, like where: { title :{ equals: "1" } }} -> where: {'version.title':{ equals: "1"} } }

[andershermansen]

Array (sort: ['isFavorite', '-createdAt']) could work here nicer for this in the Local API, with REST API it'll just map to the same comma-separated string, thoughts?

I think that looks nice, and it should not be a big problem to adjust this implementation.

[andershermansen]

@r1tsuu Here is PR which uses array instead of comma separated list for local API. I left REST API alone as using an array will complicate the interface a lot.
andershermansen#1

The PR is in my fork against this branch/PR, and if merged will update this branch/PR with the array style.

Let me know what you think.

[r1tsuu]

@r1tsuu Here is PR which uses array instead of comma separated list for local API. I left REST API alone as using an array will complicate the interface a lot. andershermansen#1

The PR is in my fork against this branch/PR, and if merged will update this branch/PR with the array style.

Let me know what you think.

Oh, I didn't mean to make these changes just now, I wanted to see Dan's view on this first.

With REST API, if you use qs or qs-esm (which we recommend) and make an array it will automatically convert to either comma separated string or ?sort[0]=createdAt&sort[1]=updatedAt which will be parsed on the backend to an array I believe, so there's a chance that it already works in your implementation!

[andershermansen]

Oh, I didn't mean to make these changes just now, I wanted to see Dan's view on this first.

Now we have some code to discuss 😄
I'll wait for Dan's input now 😇

With REST API, if you use qs or qs-esm (which we recommend) and make an array it will automatically convert to either comma separated string or ?sort[0]=createdAt&sort[1]=updatedAt which will be parsed on the backend to an array I believe, so there's a chance that it already works in your implementation!

I know, but I did not see the benefit of doing it like this is the REST API. I did it at first, but reverted it to make the input simpler. I understand the benefit of using qs for complex where queries, but here for sort it's very simple input and is much simpler with just a comma separated string IMO.

[DanRibbens]

I like the type safety points that @r1tsuu is making. I approve the idea for it to have a type of string or array of strings that we can derive from the collection slugs, that sounds really good to me.

I would make it so that the database adapter methods and the payload local API have the same type for the sort arg.

[andershermansen]

I like the type safety points that @r1tsuu is making. I approve the idea for it to have a type of string or array of strings that we can derive from the collection slugs, that sounds really good to me.

@DanRibbens Cool. Merged my changes for that into the branch of this PR.

I would make it so that the database adapter methods and the payload local API have the same type for the sort arg.

Yes, this is what I have done. I have defined export type Sort = Array<string> | string in payload and used that everywhere. Except in REST API, where I used string and converted that to array based on comman in sort query param.

[DanRibbens]

Just one question for you. Overall this looks great!

[r1tsuu]

@andershermansen I ran your commented test with my merged changes from here #8839, it passes!

[andershermansen]

@r1tsuu Nice! 💪

Not in front of a computer right now, but there was an integration test inside localization which was only running on mongodb, and disabled for others. You should check if that is OK and can be enabled for all again as well.

[r1tsuu]

@r1tsuu Nice! 💪

Not in front of a computer right now, but there was an integration test inside localization which was only running on mongodb, and disabled for others. You should check if that is OK and can be enabled for all again as well.

I tried, this test doesn't really check localized sorting, it sorts by non-localized description which is actually null in every document.
So it actually sorts by null, _id. Which potentially can have different results in postgres / mongodb. I think there was a different purpose for it with how where.like should work. It was added with this #7294
I added my own test similar to yours there.

[github-actions]

🚀 This is included in version v3.0.0-beta.119