Skip to content

Commit

Permalink
fix(drizzle): hasMany joins - localized, limit and schema paths (#8633)
Browse files Browse the repository at this point in the history
Fixes #8630

- Fixes `hasMany: true` and `localized: true` on the foreign field
- Adds `limit` to the subquery instead of hardcoded `11`.
- Adds the schema path `field.on` to the subquery, without this having 2
or more relationship fields to the same collection breaks joins
- Properly checks if the field is `hasMany`
  • Loading branch information
r1tsuu authored Oct 10, 2024
1 parent 1dcae37 commit f6acfdb
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 13 deletions.
28 changes: 19 additions & 9 deletions packages/drizzle/src/find/traverseFields.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { DBQueryConfig } from 'drizzle-orm'
import type { LibSQLDatabase } from 'drizzle-orm/libsql'
import type { Field, JoinQuery } from 'payload'

import { and, type DBQueryConfig, eq, sql } from 'drizzle-orm'
import { and, eq, sql } from 'drizzle-orm'
import { fieldAffectsData, fieldIsVirtual, tabHasName } from 'payload/shared'
import toSnakeCase from 'to-snake-case'

Expand Down Expand Up @@ -245,12 +246,15 @@ export const traverseFields = ({

const fields = adapter.payload.collections[field.collection].config.fields
const joinCollectionTableName = adapter.tableNameMap.get(toSnakeCase(field.collection))
const joinTableName = `${adapter.tableNameMap.get(toSnakeCase(field.collection))}${
let joinTableName = `${adapter.tableNameMap.get(toSnakeCase(field.collection))}${
field.localized && adapter.payload.config.localization ? adapter.localesSuffix : ''
}`

if (!adapter.tables[joinTableName][field.on]) {
if (field.hasMany) {
const db = adapter.drizzle as LibSQLDatabase
if (field.localized) {
joinTableName = adapter.tableNameMap.get(toSnakeCase(field.collection))
}
const joinTable = `${joinTableName}${adapter.relationshipsSuffix}`

const joins: BuildQueryJoinAliases = [
Expand All @@ -262,6 +266,7 @@ export const traverseFields = ({
sql.raw(`"${joinTable}"."${topLevelTableName}_id"`),
adapter.tables[currentTableName].id,
),
eq(adapter.tables[joinTable].path, field.on),
),
table: adapter.tables[joinTable],
},
Expand Down Expand Up @@ -291,30 +296,35 @@ export const traverseFields = ({
query: db
.select({
id: adapter.tables[joinTableName].id,
...(field.localized && {
locale: adapter.tables[joinTable].locale,
}),
})
.from(adapter.tables[joinTableName])
.where(subQueryWhere)
.orderBy(orderBy.order(orderBy.column))
.limit(11),
.limit(limit),
})

const columnName = `${path.replaceAll('.', '_')}${field.name}`

const extras = field.localized ? _locales.extras : currentArgs.extras
const jsonObjectSelect = field.localized
? sql.raw(`'_parentID', "id", '_locale', "locale"`)
: sql.raw(`'id', "id"`)

if (adapter.name === 'sqlite') {
extras[columnName] = sql`
currentArgs.extras[columnName] = sql`
COALESCE((
SELECT json_group_array("id")
SELECT json_group_array(json_object(${jsonObjectSelect}))
FROM (
${subQuery}
) AS ${sql.raw(`${columnName}_sub`)}
), '[]')
`.as(columnName)
} else {
extras[columnName] = sql`
currentArgs.extras[columnName] = sql`
COALESCE((
SELECT json_agg("id")
SELECT json_agg(json_build_object(${jsonObjectSelect}))
FROM (
${subQuery}
) AS ${sql.raw(`${columnName}_sub`)}
Expand Down
4 changes: 2 additions & 2 deletions packages/drizzle/src/transform/read/traverseFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,8 @@ export const traverseFields = <T extends Record<string, unknown>>({
} else {
const hasNextPage = limit !== 0 && fieldData.length > limit
fieldResult = {
docs: (hasNextPage ? fieldData.slice(0, limit) : fieldData).map((objOrID) => ({
id: typeof objOrID === 'object' ? objOrID.id : objOrID,
docs: (hasNextPage ? fieldData.slice(0, limit) : fieldData).map(({ id }) => ({
id,
})),
hasNextPage,
}
Expand Down
2 changes: 2 additions & 0 deletions packages/payload/src/fields/config/sanitizeJoinField.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ export const sanitizeJoinField = ({

// override the join field localized property to use whatever the relationship field has
field.localized = joinRelationship.localized
// override the join field hasMany property to use whatever the relationship field has
field.hasMany = joinRelationship.hasMany

if (!joins[field.collection]) {
joins[field.collection] = [join]
Expand Down
4 changes: 4 additions & 0 deletions packages/payload/src/fields/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1452,6 +1452,10 @@ export type JoinField = {
*/
collection: CollectionSlug
defaultValue?: never
/**
* This does not need to be set and will be overridden by the relationship field's hasMany property.
*/
hasMany?: boolean
hidden?: false
index?: never
/**
Expand Down
6 changes: 6 additions & 0 deletions test/joins/collections/Categories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ export const Categories: CollectionConfig = {
collection: postsSlug,
on: 'categories',
},
{
name: 'hasManyPostsLocalized',
type: 'join',
collection: postsSlug,
on: 'categoriesLocalized',
},
{
name: 'group',
type: 'group',
Expand Down
7 changes: 7 additions & 0 deletions test/joins/collections/Posts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ export const Posts: CollectionConfig = {
relationTo: categoriesSlug,
hasMany: true,
},
{
name: 'categoriesLocalized',
type: 'relationship',
relationTo: categoriesSlug,
hasMany: true,
localized: true,
},
{
name: 'group',
type: 'group',
Expand Down
89 changes: 87 additions & 2 deletions test/joins/int.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { getFileByPath } from 'payload'
import { fileURLToPath } from 'url'

import type { NextRESTClient } from '../helpers/NextRESTClient.js'
import type { Category, Post } from './payload-types.js'
import type { Category, Config, Post } from './payload-types.js'

import { devUser } from '../credentials.js'
import { idToString } from '../helpers/idToString.js'
Expand Down Expand Up @@ -80,6 +80,7 @@ describe('Joins Field', () => {
category: category.id,
upload: uploadedImage,
categories,
categoriesLocalized: categories,
group: {
category: category.id,
camelCaseCategory: category.id,
Expand Down Expand Up @@ -212,6 +213,89 @@ describe('Joins Field', () => {
expect(otherCategoryWithPosts.hasManyPosts.docs[0].title).toBe('test 14')
})

it('should populate joins using find with hasMany localized relationships', async () => {
const post_1 = await createPost(
{
title: `test es localized 1`,
categoriesLocalized: [category.id],
group: {
category: category.id,
camelCaseCategory: category.id,
},
},
'es',
)

const post_2 = await createPost(
{
title: `test es localized 2`,
categoriesLocalized: [otherCategory.id],
group: {
category: category.id,
camelCaseCategory: category.id,
},
},
'es',
)

const resultEn = await payload.find({
collection: 'categories',
where: {
id: { equals: category.id },
},
})
const otherResultEn = await payload.find({
collection: 'categories',
where: {
id: { equals: otherCategory.id },
},
})

const [categoryWithPostsEn] = resultEn.docs
const [otherCategoryWithPostsEn] = otherResultEn.docs

expect(categoryWithPostsEn.hasManyPostsLocalized.docs).toHaveLength(10)
expect(categoryWithPostsEn.hasManyPostsLocalized.docs[0]).toHaveProperty('title')
expect(categoryWithPostsEn.hasManyPostsLocalized.docs[0].title).toBe('test 14')
expect(otherCategoryWithPostsEn.hasManyPostsLocalized.docs).toHaveLength(8)
expect(otherCategoryWithPostsEn.hasManyPostsLocalized.docs[0]).toHaveProperty('title')
expect(otherCategoryWithPostsEn.hasManyPostsLocalized.docs[0].title).toBe('test 14')

const resultEs = await payload.find({
collection: 'categories',
locale: 'es',
where: {
id: { equals: category.id },
},
})
const otherResultEs = await payload.find({
collection: 'categories',
locale: 'es',
where: {
id: { equals: otherCategory.id },
},
})

const [categoryWithPostsEs] = resultEs.docs
const [otherCategoryWithPostsEs] = otherResultEs.docs

expect(categoryWithPostsEs.hasManyPostsLocalized.docs).toHaveLength(1)
expect(categoryWithPostsEs.hasManyPostsLocalized.docs[0].title).toBe('test es localized 1')

expect(otherCategoryWithPostsEs.hasManyPostsLocalized.docs).toHaveLength(1)
expect(otherCategoryWithPostsEs.hasManyPostsLocalized.docs[0].title).toBe('test es localized 2')

// clean up
await payload.delete({
collection: 'posts',
where: {
id: {
in: [post_1.id, post_2.id],
},
},
})
})

it('should not error when deleting documents with joins', async () => {
const category = await payload.create({
collection: 'categories',
Expand Down Expand Up @@ -499,9 +583,10 @@ describe('Joins Field', () => {
})
})

async function createPost(overrides?: Partial<Post>) {
async function createPost(overrides?: Partial<Post>, locale?: Config['locale']) {
return payload.create({
collection: 'posts',
locale,
data: {
title: 'test',
...overrides,
Expand Down
5 changes: 5 additions & 0 deletions test/joins/payload-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export interface Post {
upload?: (string | null) | Upload;
category?: (string | null) | Category;
categories?: (string | Category)[] | null;
categoriesLocalized?: (string | Category)[] | null;
group?: {
category?: (string | null) | Category;
camelCaseCategory?: (string | null) | Category;
Expand Down Expand Up @@ -102,6 +103,10 @@ export interface Category {
docs?: (string | Post)[] | null;
hasNextPage?: boolean | null;
} | null;
hasManyPostsLocalized?: {
docs?: (string | Post)[] | null;
hasNextPage?: boolean | null;
} | null;
group?: {
relatedPosts?: {
docs?: (string | Post)[] | null;
Expand Down

0 comments on commit f6acfdb

Please sign in to comment.