Skip to content

fix: orderable collection not working when collection has group or tab field #12129

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

kristian240
Copy link
Contributor

@kristian240 kristian240 commented Apr 16, 2025

What?

There is an issue when Payload uses postgresAdapter. If collection is orderable and there is a field that is type of group or tab.

I cannot replicate the issue in e2e tests and mongoDB but if I use Postgres the issue is there; can you shed some light why is that? Also, how can I run e2e with Postgres db?

Why?

After the group and tab fields are transformed, there is a piece of code that deletes _order prop on the root of the document, instead, we should delete _order in that field.

How?

Now, we delete _order in ref[refKey] instead on the root document - ref.

@GermanJablo GermanJablo self-requested a review April 21, 2025 13:55
@GermanJablo GermanJablo self-assigned this Apr 21, 2025
@maximseshuk
Copy link
Contributor

This PR should fix this issue #12143

@GermanJablo
Copy link
Contributor

Thanks for the contribution.

This PR does indeed fix the bug described with the group or tab field, which I've been able to reproduce, but it's not related to issue #12143, which is due to an incorrect migration, as I explained here.

For now, e2e tests only run on MongoDB, which is why the bug wasn't properly detected. If you do an integration test, it will run with Postgres in CI. A nice thing to have, although not strictly necessary in this case, as I reproduced the issue and the solution manually.

The only impediment to merging this is that there's a small TypeScript error blocking the build. Can you take a look at it?

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