Skip to content

fix(drizzle): skip column if undefined in findMany #12902

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

Merged
merged 5 commits into from
Jun 26, 2025

Conversation

akhrarovsaid
Copy link
Contributor

What?

This PR fixes an issue where sorting on a traditional virtual field with virtual: true while using a drizzle-based db adapter would cause a runtime error.

Why?

To skip attempting to sort virtual fields which are not linked to a relationship/upload and prevent a runtime error from surfacing.

How?

Skipping the deletion of the property from the selectFields object if the column is false-y.

Fixes #12886

Before:

20250621-1932-31.0302457.mp4

After:

virtualfields-sort-Posts---Payload.webm
Collection config used above
export const PostsCollection: CollectionConfig = {
  slug: postsSlug,
  admin: {
    useAsTitle: 'title',
    defaultColumns: ['title', 'exampleField'],
  },
  fields: [
    {
      name: 'title',
      type: 'text',
    },
    {
      name: 'exampleField',
      type: 'text',
      virtual: true,
      admin: {
        readOnly: true,
      },
      hooks: {
        afterRead: [({ data }) => data?.title],
      },
    },
    {
      type: 'relationship',
      name: 'category',
      relationTo: 'categories',
    },
    {
      name: 'categoryTitle',
      type: 'text',
      virtual: 'category.title',
    },
  ],
}

Copy link
Member

@r1tsuu r1tsuu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, do you think we should have a test in database/int.spec.ts that ensures in case we query by a virtual field payload.find doesn't throw an error?

@akhrarovsaid
Copy link
Contributor Author

akhrarovsaid commented Jun 24, 2025

Hey @r1tsuu

Yeah, this is a good idea. More than happy to add a test here, and originally I tried to, but there were some strange errors in the suite for me that I couldn't quite put my finger on concerning blocks. I'll try again here, thank you.

EDIT: Wow, you're really on top of your game. This should fix it #12919

DanRibbens
DanRibbens previously approved these changes Jun 24, 2025
r1tsuu added 2 commits June 26, 2025 19:35

Verified

This commit was signed with the committer’s verified signature.
r1tsuu Sasha
…rtualfield-error

Verified

This commit was signed with the committer’s verified signature.
r1tsuu Sasha
@r1tsuu r1tsuu enabled auto-merge (squash) June 26, 2025 16:38
@r1tsuu r1tsuu merged commit 605c993 into payloadcms:main Jun 26, 2025
296 of 308 checks passed
Copy link
Contributor

🚀 This is included in version v3.44.0

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.

Sorting by virtual field ist not prohibited and results in error
3 participants