From 9c25754eed87e70b053231693102378150b8b80d Mon Sep 17 00:00:00 2001 From: Sasha <64744993+r1tsuu@users.noreply.github.com> Date: Tue, 5 Nov 2024 19:52:05 +0200 Subject: [PATCH] fix(db-postgres): sort by localized fields (#9016) ### What? Port of https://github.com/payloadcms/payload/pull/8839 to main Fixes https://github.com/payloadcms/payload/issues/5152 --- packages/db-postgres/src/count.ts | 8 +- packages/db-postgres/src/find/findMany.ts | 8 +- .../src/queries/getTableColumnFromPath.ts | 108 +++++++----------- test/localization/int.spec.ts | 100 +++++++++++++++- 4 files changed, 147 insertions(+), 77 deletions(-) diff --git a/packages/db-postgres/src/count.ts b/packages/db-postgres/src/count.ts index 8c1096b96b4..bbc5b913e88 100644 --- a/packages/db-postgres/src/count.ts +++ b/packages/db-postgres/src/count.ts @@ -1,7 +1,7 @@ import type { Count } from 'payload/database' import type { SanitizedCollectionConfig } from 'payload/types' -import { sql, count as sqlCount } from 'drizzle-orm' +import { count as sqlCount } from 'drizzle-orm' import toSnakeCase from 'to-snake-case' import type { ChainedMethods } from './find/chainMethods' @@ -51,11 +51,7 @@ export const count: Count = async function count( methods: selectCountMethods, query: db .select({ - count: - selectCountMethods.length > 0 - ? sql`count - (DISTINCT ${this.tables[tableName].id})` - : sqlCount(), + count: sqlCount(), }) .from(table) .where(where), diff --git a/packages/db-postgres/src/find/findMany.ts b/packages/db-postgres/src/find/findMany.ts index 3013ac47c2f..d5f28c4e660 100644 --- a/packages/db-postgres/src/find/findMany.ts +++ b/packages/db-postgres/src/find/findMany.ts @@ -1,7 +1,7 @@ import type { FindArgs } from 'payload/database' import type { Field, PayloadRequest, TypeWithID } from 'payload/types' -import { inArray, sql, count as sqlCount } from 'drizzle-orm' +import { inArray, count as sqlCount } from 'drizzle-orm' import type { PostgresAdapter } from '../types' import type { ChainedMethods } from './chainMethods' @@ -143,11 +143,7 @@ export const findMany = async function find({ methods: selectCountMethods, query: db .select({ - count: - selectCountMethods.length > 0 - ? sql`count - (DISTINCT ${adapter.tables[tableName].id})` - : sqlCount(), + count: sqlCount(), }) .from(table) .where(where), diff --git a/packages/db-postgres/src/queries/getTableColumnFromPath.ts b/packages/db-postgres/src/queries/getTableColumnFromPath.ts index 421a88c1ef9..448439b11f1 100644 --- a/packages/db-postgres/src/queries/getTableColumnFromPath.ts +++ b/packages/db-postgres/src/queries/getTableColumnFromPath.ts @@ -185,17 +185,12 @@ export const getTableColumnFromPath = ({ if (locale && field.localized && adapter.payload.config.localization) { newTableName = `${tableName}${adapter.localesSuffix}` - joins[tableName] = eq( - adapter.tables[tableName].id, - adapter.tables[newTableName]._parentID, - ) + let condition = eq(adapter.tables[tableName].id, adapter.tables[newTableName]._parentID) if (locale !== 'all') { - constraints.push({ - columnName: '_locale', - table: adapter.tables[newTableName], - value: locale, - }) + condition = and(condition, eq(adapter.tables[newTableName]._locale, locale)) } + + joins[tableName] = condition } return getTableColumnFromPath({ adapter, @@ -224,17 +219,15 @@ export const getTableColumnFromPath = ({ ) if (locale && field.localized && adapter.payload.config.localization) { - joins[newTableName] = and( + const conditions = [ eq(adapter.tables[tableName].id, adapter.tables[newTableName].parent), eq(adapter.tables[newTableName]._locale, locale), - ) + ] if (locale !== 'all') { - constraints.push({ - columnName: '_locale', - table: adapter.tables[newTableName], - value: locale, - }) + conditions.push(eq(adapter.tables[newTableName]._locale, locale)) } + + joins[newTableName] = and(...conditions) } else { joins[newTableName] = eq( adapter.tables[tableName].id, @@ -268,17 +261,12 @@ export const getTableColumnFromPath = ({ ] if (locale && field.localized && adapter.payload.config.localization) { - joins[newTableName] = and( - ...joinConstraints, - eq(adapter.tables[newTableName]._locale, locale), - ) + const conditions = [...joinConstraints] if (locale !== 'all') { - constraints.push({ - columnName: 'locale', - table: adapter.tables[newTableName], - value: locale, - }) + conditions.push(eq(adapter.tables[newTableName]._locale, locale)) } + + joins[newTableName] = and(...conditions) } else { joins[newTableName] = and(...joinConstraints) } @@ -302,17 +290,12 @@ export const getTableColumnFromPath = ({ constraintPath = `${constraintPath}${field.name}.%.` if (locale && field.localized && adapter.payload.config.localization) { - joins[newTableName] = and( - eq(arrayParentTable.id, adapter.tables[newTableName]._parentID), - eq(adapter.tables[newTableName]._locale, locale), - ) + const conditions = [eq(arrayParentTable.id, adapter.tables[newTableName]._parentID)] if (locale !== 'all') { - constraints.push({ - columnName: '_locale', - table: adapter.tables[newTableName], - value: locale, - }) + conditions.push(eq(adapter.tables[newTableName]._locale, locale)) } + + joins[newTableName] = and(...conditions) } else { joins[newTableName] = eq(arrayParentTable.id, adapter.tables[newTableName]._parentID) } @@ -399,17 +382,18 @@ export const getTableColumnFromPath = ({ constraints = constraints.concat(blockConstraints) selectFields = { ...selectFields, ...blockSelectFields } if (field.localized && adapter.payload.config.localization) { - joins[newTableName] = and( - eq(adapter.tables[tableName].id, adapter.tables[newTableName]._parentID), - eq(adapter.tables[newTableName]._locale, locale), - ) - if (locale) { - constraints.push({ - columnName: '_locale', - table: adapter.tables[newTableName], - value: locale, - }) + const conditions = [ + eq( + (aliasTable || adapter.tables[tableName]).id, + adapter.tables[newTableName]._parentID, + ), + ] + + if (locale !== 'all') { + conditions.push(eq(adapter.tables[newTableName]._locale, locale)) } + + joins[newTableName] = and(...conditions) } else { joins[newTableName] = eq( adapter.tables[tableName].id, @@ -444,21 +428,20 @@ export const getTableColumnFromPath = ({ // Join in the relationships table if (locale && field.localized && adapter.payload.config.localization) { + const conditions = [ + eq((aliasTable || adapter.tables[rootTableName]).id, aliasRelationshipTable.parent), + eq(aliasRelationshipTable.locale, locale), + like(aliasRelationshipTable.path, `${constraintPath}${field.name}`), + ] + + if (locale !== 'all') { + conditions.push(eq(aliasRelationshipTable.locale, locale)) + } + joinAliases.push({ - condition: and( - eq((aliasTable || adapter.tables[rootTableName]).id, aliasRelationshipTable.parent), - eq(aliasRelationshipTable.locale, locale), - like(aliasRelationshipTable.path, `${constraintPath}${field.name}`), - ), + condition: and(...conditions), table: aliasRelationshipTable, }) - if (locale !== 'all') { - constraints.push({ - columnName: 'locale', - table: aliasRelationshipTable, - value: locale, - }) - } } else { // Join in the relationships table joinAliases.push({ @@ -554,17 +537,14 @@ export const getTableColumnFromPath = ({ const parentTable = aliasTable || adapter.tables[tableName] - joins[newTableName] = eq(parentTable.id, adapter.tables[newTableName]._parentID) - - aliasTable = undefined + let condition = eq(parentTable.id, adapter.tables[newTableName]._parentID) if (locale !== 'all') { - constraints.push({ - columnName: '_locale', - table: adapter.tables[newTableName], - value: locale, - }) + condition = and(condition, eq(adapter.tables[newTableName]._locale, locale)) } + joins[newTableName] = condition + + aliasTable = undefined } const targetTable = aliasTable || adapter.tables[newTableName] diff --git a/test/localization/int.spec.ts b/test/localization/int.spec.ts index bc762bbd965..8ec1f1120e9 100644 --- a/test/localization/int.spec.ts +++ b/test/localization/int.spec.ts @@ -2,7 +2,7 @@ import { GraphQLClient } from 'graphql-request' import type { Config } from '../../packages/payload/src/config/types' import type { Where } from '../../packages/payload/src/types' -import type { LocalizedPost, WithLocalizedRelationship } from './payload-types' +import type { LocalizedPost, LocalizedSort, WithLocalizedRelationship } from './payload-types' import payload from '../../packages/payload/src' import { devUser } from '../credentials' @@ -359,6 +359,7 @@ describe('Localization', () => { describe('Localized Sort Count', () => { const expectedTotalDocs = 5 + const posts: LocalizedSort[] = [] beforeAll(async () => { for (let i = 1; i <= expectedTotalDocs; i++) { const post = await payload.create({ @@ -370,6 +371,8 @@ describe('Localization', () => { locale: englishLocale, }) + posts.push(post) + await payload.update({ id: post.id, collection: localizedSortSlug, @@ -409,6 +412,101 @@ describe('Localization', () => { expect(sortByTitleQuery.totalDocs).toEqual(expectedTotalDocs) expect(sortByDateQuery.totalDocs).toEqual(expectedTotalDocs) }) + + it('should return correct order when sorted by localized fields', async () => { + const { docs: docsAsc } = await payload.find({ collection: localizedSortSlug, sort: 'title' }) + docsAsc.forEach((doc, i) => { + expect(posts[i].id).toBe(doc.id) + }) + const { docs: docsDesc } = await payload.find({ + collection: localizedSortSlug, + sort: '-title', + }) + docsDesc.forEach((doc, i) => { + expect(posts.at(posts.length - i - 1).id).toBe(doc.id) + }) + // Test with words + const randomWords = [ + 'sunset', + 'whisper', + 'lighthouse', + 'harmony', + 'crystal', + 'thunder', + 'meadow', + 'voyage', + 'echo', + 'quicksand', + ] + const randomWordsSpanish = [ + 'atardecer', + 'susurro', + 'faro', + 'armonía', + 'cristal', + 'trueno', + 'pradera', + 'viaje', + 'eco', + 'arenas movedizas', + ] + expect(randomWords).toHaveLength(randomWordsSpanish.length) + const randomWordsPosts: (number | string)[] = [] + for (let i = 0; i < randomWords.length; i++) { + const en = randomWords[i] + const post = await payload.create({ collection: 'localized-sort', data: { title: en } }) + const es = randomWordsSpanish[i] + await payload.update({ + collection: 'localized-sort', + data: { title: es }, + id: post.id, + locale: 'es', + }) + randomWordsPosts.push(post.id) + } + const ascSortedWordsEn = randomWords.toSorted((a, b) => a.localeCompare(b)) + const descSortedWordsEn = randomWords.toSorted((a, b) => b.localeCompare(a)) + const q = { id: { in: randomWordsPosts } } + const { docs: randomWordsEnAsc } = await payload.find({ + collection: localizedSortSlug, + sort: 'title', + where: q, + }) + randomWordsEnAsc.forEach((doc, i) => { + expect(ascSortedWordsEn[i]).toBe(doc.title) + }) + const { docs: randomWordsEnDesc } = await payload.find({ + collection: localizedSortSlug, + sort: '-title', + where: q, + }) + randomWordsEnDesc.forEach((doc, i) => { + expect(descSortedWordsEn[i]).toBe(doc.title) + }) + // Test sorting for Spanish locale + const ascSortedWordsEs = randomWordsSpanish.toSorted((a, b) => a.localeCompare(b)) + const descSortedWordsEs = randomWordsSpanish.toSorted((a, b) => b.localeCompare(a)) + // Fetch sorted words in Spanish (ascending) + const { docs: randomWordsEsAsc } = await payload.find({ + collection: localizedSortSlug, + sort: 'title', + where: q, + locale: 'es', + }) + randomWordsEsAsc.forEach((doc, i) => { + expect(ascSortedWordsEs[i]).toBe(doc.title) + }) + // Fetch sorted words in Spanish (descending) + const { docs: randomWordsEsDesc } = await payload.find({ + collection: localizedSortSlug, + sort: '-title', + where: q, + locale: 'es', + }) + randomWordsEsDesc.forEach((doc, i) => { + expect(descSortedWordsEs[i]).toBe(doc.title) + }) + }) }) describe('Localized Relationship', () => {