From ee117bb61690853f673260a11d2d1f4824f079b0 Mon Sep 17 00:00:00 2001 From: Dan Ribbens Date: Fri, 8 Nov 2024 15:34:19 -0500 Subject: [PATCH 1/2] fix!: handle custom id logic in mongodb adapter (#9069) ### What? Moved the logic for copying the data.id to data._id to the mongoose adapter. ### Why? If you have any hooks that need to set the `id`, the value does not get sent to mongodb as you would expect since it was copied before the beforeValidate hooks. ### How? Now data._id is assigned only in the mongodb adapter's `create` function. BREAKING CHANGES: When using custom ID fields, if you have any collection hooks for beforeValidate, beforeChange then `data._id` will no longer be assigned as this happens now in the database adapter. Use `data.id` instead. --- docs/fields/overview.mdx | 3 +- packages/db-mongodb/src/create.ts | 4 + .../src/collections/operations/create.ts | 11 - test/database/config.ts | 25 ++ test/database/int.spec.ts | 11 +- test/database/payload-types.ts | 255 ++++++++++++++++++ 6 files changed, 296 insertions(+), 13 deletions(-) diff --git a/docs/fields/overview.mdx b/docs/fields/overview.mdx index fcd346505c..eed310084b 100644 --- a/docs/fields/overview.mdx +++ b/docs/fields/overview.mdx @@ -356,7 +356,7 @@ For full details on Admin Options, see the [Field Admin Options](../admin/fields ## Custom ID Fields -All [Collections](../configuration/collections) automatically generate their own ID field. If needed, you can override this behavior by providing an explicit ID field to your config. This will force users to provide a their own ID value when creating a record. +All [Collections](../configuration/collections) automatically generate their own ID field. If needed, you can override this behavior by providing an explicit ID field to your config. This field should either be required or have a hook to generate the ID dynamically. To define a custom ID field, add a new field with the `name` property set to `id`: @@ -368,6 +368,7 @@ export const MyCollection: CollectionConfig = { fields: [ { name: 'id', // highlight-line + required: true, type: 'number', }, ], diff --git a/packages/db-mongodb/src/create.ts b/packages/db-mongodb/src/create.ts index d1ac9a6829..504cffd854 100644 --- a/packages/db-mongodb/src/create.ts +++ b/packages/db-mongodb/src/create.ts @@ -20,6 +20,10 @@ export const create: Create = async function create( fields: this.payload.collections[collection].config.fields, }) + if (this.payload.collections[collection].customIDType) { + sanitizedData._id = sanitizedData.id + } + try { ;[doc] = await Model.create([sanitizedData], options) } catch (error) { diff --git a/packages/payload/src/collections/operations/create.ts b/packages/payload/src/collections/operations/create.ts index 574d3e2c6d..a5580200f0 100644 --- a/packages/payload/src/collections/operations/create.ts +++ b/packages/payload/src/collections/operations/create.ts @@ -123,17 +123,6 @@ export const createOperation = async < await executeAccess({ data, req }, collectionConfig.access.create) } - // ///////////////////////////////////// - // Custom id - // ///////////////////////////////////// - - if (payload.collections[collectionConfig.slug].customIDType) { - data = { - _id: data.id, - ...data, - } - } - // ///////////////////////////////////// // Generate data for all files and sizes // ///////////////////////////////////// diff --git a/test/database/config.ts b/test/database/config.ts index 2d23bd29a0..641dd7b352 100644 --- a/test/database/config.ts +++ b/test/database/config.ts @@ -4,6 +4,8 @@ const filename = fileURLToPath(import.meta.url) const dirname = path.dirname(filename) import type { TextField } from 'payload' +import { v4 as uuid } from 'uuid' + import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js' import { devUser } from '../credentials.js' @@ -356,6 +358,29 @@ export default buildConfigWithDefaults({ }, ], }, + { + slug: 'custom-ids', + fields: [ + { + name: 'id', + type: 'text', + admin: { + readOnly: true, + }, + hooks: { + beforeChange: [ + ({ value, operation }) => { + if (operation === 'create') { + return uuid() + } + return value + }, + ], + }, + }, + ], + versions: { drafts: true }, + }, ], globals: [ { diff --git a/test/database/int.spec.ts b/test/database/int.spec.ts index 46a40ae762..532eecf574 100644 --- a/test/database/int.spec.ts +++ b/test/database/int.spec.ts @@ -75,10 +75,19 @@ describe('database', () => { expect(updated.id).toStrictEqual(created.doc.id) }) + + it('should create with generated ID text from hook', async () => { + const doc = await payload.create({ + collection: 'custom-ids', + data: {}, + }) + + expect(doc.id).toBeDefined() + }) }) describe('timestamps', () => { - it('should have createdAt and updatedAt timetstamps to the millisecond', async () => { + it('should have createdAt and updatedAt timestamps to the millisecond', async () => { const result = await payload.create({ collection: 'posts', data: { diff --git a/test/database/payload-types.ts b/test/database/payload-types.ts index 354c1c8356..21259d61cc 100644 --- a/test/database/payload-types.ts +++ b/test/database/payload-types.ts @@ -19,21 +19,44 @@ export interface Config { 'custom-schema': CustomSchema; places: Place; 'fields-persistance': FieldsPersistance; + 'custom-ids': CustomId; users: User; 'payload-locked-documents': PayloadLockedDocument; 'payload-preferences': PayloadPreference; 'payload-migrations': PayloadMigration; }; + collectionsSelect: { + posts: PostsSelect | PostsSelect; + 'default-values': DefaultValuesSelect | DefaultValuesSelect; + 'relation-a': RelationASelect | RelationASelect; + 'relation-b': RelationBSelect | RelationBSelect; + 'pg-migrations': PgMigrationsSelect | PgMigrationsSelect; + 'custom-schema': CustomSchemaSelect | CustomSchemaSelect; + places: PlacesSelect | PlacesSelect; + 'fields-persistance': FieldsPersistanceSelect | FieldsPersistanceSelect; + 'custom-ids': CustomIdsSelect | CustomIdsSelect; + users: UsersSelect | UsersSelect; + 'payload-locked-documents': PayloadLockedDocumentsSelect | PayloadLockedDocumentsSelect; + 'payload-preferences': PayloadPreferencesSelect | PayloadPreferencesSelect; + 'payload-migrations': PayloadMigrationsSelect | PayloadMigrationsSelect; + }; db: { defaultIDType: string; }; globals: { global: Global; }; + globalsSelect: { + global: GlobalSelect | GlobalSelect; + }; locale: 'en' | 'es'; user: User & { collection: 'users'; }; + jobs?: { + tasks: unknown; + workflows?: unknown; + }; } export interface UserAuthOperations { forgotPassword: { @@ -232,6 +255,15 @@ export interface FieldsPersistance { updatedAt: string; createdAt: string; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "custom-ids". + */ +export interface CustomId { + id: string; + updatedAt: string; + createdAt: string; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "users". @@ -288,6 +320,10 @@ export interface PayloadLockedDocument { relationTo: 'fields-persistance'; value: string | FieldsPersistance; } | null) + | ({ + relationTo: 'custom-ids'; + value: string | CustomId; + } | null) | ({ relationTo: 'users'; value: string | User; @@ -334,6 +370,215 @@ export interface PayloadMigration { updatedAt: string; createdAt: string; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "posts_select". + */ +export interface PostsSelect { + title?: T; + hasTransaction?: T; + throwAfterChange?: T; + updatedAt?: T; + createdAt?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "default-values_select". + */ +export interface DefaultValuesSelect { + title?: T; + defaultValue?: T; + array?: + | T + | { + defaultValue?: T; + id?: T; + }; + group?: + | T + | { + defaultValue?: T; + }; + select?: T; + updatedAt?: T; + createdAt?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "relation-a_select". + */ +export interface RelationASelect { + title?: T; + richText?: T; + updatedAt?: T; + createdAt?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "relation-b_select". + */ +export interface RelationBSelect { + title?: T; + relationship?: T; + richText?: T; + updatedAt?: T; + createdAt?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "pg-migrations_select". + */ +export interface PgMigrationsSelect { + relation1?: T; + myArray?: + | T + | { + relation2?: T; + mySubArray?: + | T + | { + relation3?: T; + id?: T; + }; + id?: T; + }; + myGroup?: + | T + | { + relation4?: T; + }; + myBlocks?: + | T + | { + myBlock?: + | T + | { + relation5?: T; + relation6?: T; + id?: T; + blockName?: T; + }; + }; + updatedAt?: T; + createdAt?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "custom-schema_select". + */ +export interface CustomSchemaSelect { + text?: T; + localizedText?: T; + relationship?: T; + select?: T; + radio?: T; + array?: + | T + | { + text?: T; + localizedText?: T; + id?: T; + }; + blocks?: + | T + | { + block?: + | T + | { + text?: T; + localizedText?: T; + id?: T; + blockName?: T; + }; + }; + updatedAt?: T; + createdAt?: T; + _status?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "places_select". + */ +export interface PlacesSelect { + country?: T; + city?: T; + updatedAt?: T; + createdAt?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "fields-persistance_select". + */ +export interface FieldsPersistanceSelect { + text?: T; + textHooked?: T; + array?: + | T + | { + id?: T; + }; + textWithinRow?: T; + textWithinCollapsible?: T; + textWithinTabs?: T; + updatedAt?: T; + createdAt?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "custom-ids_select". + */ +export interface CustomIdsSelect { + id?: T; + updatedAt?: T; + createdAt?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "users_select". + */ +export interface UsersSelect { + updatedAt?: T; + createdAt?: T; + email?: T; + resetPasswordToken?: T; + resetPasswordExpiration?: T; + salt?: T; + hash?: T; + loginAttempts?: T; + lockUntil?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "payload-locked-documents_select". + */ +export interface PayloadLockedDocumentsSelect { + document?: T; + globalSlug?: T; + user?: T; + updatedAt?: T; + createdAt?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "payload-preferences_select". + */ +export interface PayloadPreferencesSelect { + user?: T; + key?: T; + value?: T; + updatedAt?: T; + createdAt?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "payload-migrations_select". + */ +export interface PayloadMigrationsSelect { + name?: T; + batch?: T; + updatedAt?: T; + createdAt?: T; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "global". @@ -344,6 +589,16 @@ export interface Global { updatedAt?: string | null; createdAt?: string | null; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "global_select". + */ +export interface GlobalSelect { + text?: T; + updatedAt?: T; + createdAt?: T; + globalType?: T; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "auth". From e3172f1e39ca192c97ba94ed3cbfd6f0248f345e Mon Sep 17 00:00:00 2001 From: Dan Ribbens Date: Fri, 8 Nov 2024 15:35:46 -0500 Subject: [PATCH 2/2] fix: migrateRefresh migrates without previously ran migrations (#9073) fix: migrateRefresh migrates without previously ran migrations chore: adds tests for database migrate:fresh and migrate:refresh --------- Co-authored-by: Sasha <64744993+r1tsuu@users.noreply.github.com> --- .../src/database/migrations/migrateRefresh.ts | 84 +++++++++---------- test/database/int.spec.ts | 36 ++++++-- 2 files changed, 72 insertions(+), 48 deletions(-) diff --git a/packages/payload/src/database/migrations/migrateRefresh.ts b/packages/payload/src/database/migrations/migrateRefresh.ts index 82d90191db..0b53eca724 100644 --- a/packages/payload/src/database/migrations/migrateRefresh.ts +++ b/packages/payload/src/database/migrations/migrateRefresh.ts @@ -14,59 +14,57 @@ export async function migrateRefresh(this: BaseDatabaseAdapter) { const { payload } = this const migrationFiles = await readMigrationFiles({ payload }) - const { existingMigrations, latestBatch } = await getMigrations({ + const { existingMigrations } = await getMigrations({ payload, }) - if (!existingMigrations?.length) { - payload.logger.info({ msg: 'No migrations to rollback.' }) - return - } - - payload.logger.info({ - msg: `Rolling back batch ${latestBatch} consisting of ${existingMigrations.length} migration(s).`, - }) - const req = { payload } as PayloadRequest - // Reverse order of migrations to rollback - existingMigrations.reverse() + if (existingMigrations?.length) { + payload.logger.info({ + msg: `Rolling back all ${existingMigrations.length} migration(s).`, + }) + // Reverse order of migrations to rollback + existingMigrations.reverse() - for (const migration of existingMigrations) { - try { - const migrationFile = migrationFiles.find((m) => m.name === migration.name) - if (!migrationFile) { - throw new Error(`Migration ${migration.name} not found locally.`) - } + for (const migration of existingMigrations) { + try { + const migrationFile = migrationFiles.find((m) => m.name === migration.name) + if (!migrationFile) { + throw new Error(`Migration ${migration.name} not found locally.`) + } - payload.logger.info({ msg: `Migrating down: ${migration.name}` }) - const start = Date.now() - await initTransaction(req) - await migrationFile.down({ payload, req }) - payload.logger.info({ - msg: `Migrated down: ${migration.name} (${Date.now() - start}ms)`, - }) - await payload.delete({ - collection: 'payload-migrations', - req, - where: { - name: { - equals: migration.name, + payload.logger.info({ msg: `Migrating down: ${migration.name}` }) + const start = Date.now() + await initTransaction(req) + await migrationFile.down({ payload, req }) + payload.logger.info({ + msg: `Migrated down: ${migration.name} (${Date.now() - start}ms)`, + }) + await payload.delete({ + collection: 'payload-migrations', + req, + where: { + name: { + equals: migration.name, + }, }, - }, - }) - } catch (err: unknown) { - await killTransaction(req) - let msg = `Error running migration ${migration.name}. Rolling back.` - if (err instanceof Error) { - msg += ` ${err.message}` + }) + } catch (err: unknown) { + await killTransaction(req) + let msg = `Error running migration ${migration.name}. Rolling back.` + if (err instanceof Error) { + msg += ` ${err.message}` + } + payload.logger.error({ + err, + msg, + }) + process.exit(1) } - payload.logger.error({ - err, - msg, - }) - process.exit(1) } + } else { + payload.logger.info({ msg: 'No migrations to rollback.' }) } // Run all migrate up diff --git a/test/database/int.spec.ts b/test/database/int.spec.ts index 532eecf574..ed39466349 100644 --- a/test/database/int.spec.ts +++ b/test/database/int.spec.ts @@ -13,6 +13,7 @@ import { fileURLToPath } from 'url' import { devUser } from '../credentials.js' import { initPayloadInt } from '../helpers/initPayloadInt.js' +import { isMongoose } from '../helpers/isMongoose.js' import removeFiles from '../helpers/removeFiles.js' const filename = fileURLToPath(import.meta.url) @@ -134,8 +135,14 @@ describe('database', () => { }) describe('migrations', () => { + let ranFreshTest = false + beforeEach(async () => { - if (process.env.PAYLOAD_DROP_DATABASE === 'true' && 'drizzle' in payload.db) { + if ( + process.env.PAYLOAD_DROP_DATABASE === 'true' && + 'drizzle' in payload.db && + !ranFreshTest + ) { const db = payload.db as unknown as PostgresAdapter await db.dropDatabase({ adapter: db }) } @@ -195,28 +202,47 @@ describe('database', () => { const migration = docs[0] expect(migration.name).toContain('_test') expect(migration.batch).toStrictEqual(1) + ranFreshTest = true }) - // known issue: https://github.com/payloadcms/payload/issues/4597 - it.skip('should run migrate:down', async () => { + it('should run migrate:down', async () => { + // known drizzle issue: https://github.com/payloadcms/payload/issues/4597 + if (!isMongoose(payload)) { + return + } let error try { await payload.db.migrateDown() } catch (e) { error = e } + + const migrations = await payload.find({ + collection: 'payload-migrations', + }) + expect(error).toBeUndefined() + expect(migrations.docs).toHaveLength(0) }) - // known issue: https://github.com/payloadcms/payload/issues/4597 - it.skip('should run migrate:refresh', async () => { + it('should run migrate:refresh', async () => { + // known drizzle issue: https://github.com/payloadcms/payload/issues/4597 + if (!isMongoose(payload)) { + return + } let error try { await payload.db.migrateRefresh() } catch (e) { error = e } + + const migrations = await payload.find({ + collection: 'payload-migrations', + }) + expect(error).toBeUndefined() + expect(migrations.docs).toHaveLength(1) }) })