From 8daac4e6706fb8c29eb971cc90e7ac7546e27138 Mon Sep 17 00:00:00 2001 From: Sasha <64744993+r1tsuu@users.noreply.github.com> Date: Fri, 11 Oct 2024 17:01:21 +0300 Subject: [PATCH] fix: properly store timestamps in versions (#8646) This PR makes a more clear gap between `version_createdAt` / `version_updatedAt` and `createdAt` / `updatedAt` columns / fields in mongodb. - `createdAt` - This should be a new value in a new version. Before this change it was the same all the time. Should remain the same on autosave. - The same for `updatedAt`, but it should be updated on every change (including autosave) - `version_createdAt` - Should remain equal to `createdAt` from the parent collection / table - `version_updatedAt` - On a latest version it makes sense this be the same as `updatedAt` from the parent collection / table, as all the `version_*` fields should be just synced with it --- packages/db-mongodb/src/queryDrafts.ts | 2 - packages/drizzle/src/createGlobalVersion.ts | 4 + packages/drizzle/src/createVersion.ts | 8 +- packages/drizzle/src/queryDrafts.ts | 2 - .../src/collections/operations/duplicate.ts | 5 +- .../src/collections/operations/find.ts | 2 +- .../src/collections/operations/update.ts | 5 +- .../src/collections/operations/updateByID.ts | 5 +- .../payload/src/globals/operations/update.ts | 6 +- .../src/versions/drafts/getQueryDraftsSort.ts | 16 +++- .../versions/getLatestCollectionVersion.ts | 9 +- .../src/versions/getLatestGlobalVersion.ts | 14 +-- packages/payload/src/versions/saveVersion.ts | 8 +- .../ui/src/providers/DocumentInfo/index.tsx | 5 ++ test/versions/int.spec.ts | 85 ++++++++++++++++++- 15 files changed, 128 insertions(+), 48 deletions(-) diff --git a/packages/db-mongodb/src/queryDrafts.ts b/packages/db-mongodb/src/queryDrafts.ts index b7badbd64c..5754905d69 100644 --- a/packages/db-mongodb/src/queryDrafts.ts +++ b/packages/db-mongodb/src/queryDrafts.ts @@ -99,8 +99,6 @@ export const queryDrafts: QueryDrafts = async function queryDrafts( _id: doc.parent, id: doc.parent, ...doc.version, - createdAt: doc.createdAt, - updatedAt: doc.updatedAt, } return sanitizeInternalFields(doc) diff --git a/packages/drizzle/src/createGlobalVersion.ts b/packages/drizzle/src/createGlobalVersion.ts index 3af9547b99..d4c72db4bd 100644 --- a/packages/drizzle/src/createGlobalVersion.ts +++ b/packages/drizzle/src/createGlobalVersion.ts @@ -12,10 +12,12 @@ export async function createGlobalVersion( this: DrizzleAdapter, { autosave, + createdAt, globalSlug, publishedLocale, req = {} as PayloadRequest, snapshot, + updatedAt, versionData, }: CreateGlobalVersionArgs, ) { @@ -28,9 +30,11 @@ export async function createGlobalVersion( adapter: this, data: { autosave, + createdAt, latest: true, publishedLocale, snapshot, + updatedAt, version: versionData, }, db, diff --git a/packages/drizzle/src/createVersion.ts b/packages/drizzle/src/createVersion.ts index a52d01d9b9..0eda45ffcb 100644 --- a/packages/drizzle/src/createVersion.ts +++ b/packages/drizzle/src/createVersion.ts @@ -13,10 +13,12 @@ export async function createVersion( { autosave, collectionSlug, + createdAt, parent, publishedLocale, req = {} as PayloadRequest, snapshot, + updatedAt, versionData, }: CreateVersionArgs, ) { @@ -33,17 +35,15 @@ export async function createVersion( const data: Record = { autosave, + createdAt, latest: true, parent, publishedLocale, snapshot, + updatedAt, version, } - if ('createdAt' in version) { - data.createdAt = version.createdAt - } - const result = await upsertRow>({ adapter: this, data, diff --git a/packages/drizzle/src/queryDrafts.ts b/packages/drizzle/src/queryDrafts.ts index 636d486feb..00386a2a50 100644 --- a/packages/drizzle/src/queryDrafts.ts +++ b/packages/drizzle/src/queryDrafts.ts @@ -38,8 +38,6 @@ export const queryDrafts: QueryDrafts = async function queryDrafts( doc = { id: doc.parent, ...doc.version, - createdAt: doc.createdAt, - updatedAt: doc.updatedAt, } return doc diff --git a/packages/payload/src/collections/operations/duplicate.ts b/packages/payload/src/collections/operations/duplicate.ts index 9b65fecf66..6539b802e9 100644 --- a/packages/payload/src/collections/operations/duplicate.ts +++ b/packages/payload/src/collections/operations/duplicate.ts @@ -267,10 +267,7 @@ export const duplicateOperation = async ( result = await saveVersion({ id: versionDoc.id, collection: collectionConfig, - docWithLocales: { - ...versionDoc, - createdAt: result.createdAt, - }, + docWithLocales: versionDoc, draft: shouldSaveDraft, payload, req, diff --git a/packages/payload/src/collections/operations/find.ts b/packages/payload/src/collections/operations/find.ts index e0e0368998..ccb41f8119 100644 --- a/packages/payload/src/collections/operations/find.ts +++ b/packages/payload/src/collections/operations/find.ts @@ -131,7 +131,7 @@ export const findOperation = async ( page: sanitizedPage, pagination: usePagination, req, - sort: getQueryDraftsSort(sort), + sort: getQueryDraftsSort({ collectionConfig, sort }), where: fullWhere, }) } else { diff --git a/packages/payload/src/collections/operations/update.ts b/packages/payload/src/collections/operations/update.ts index 135e440573..5ee4ae6e48 100644 --- a/packages/payload/src/collections/operations/update.ts +++ b/packages/payload/src/collections/operations/update.ts @@ -329,10 +329,7 @@ export const updateOperation = async ( result = await saveVersion({ id, collection: collectionConfig, - docWithLocales: { - ...result, - createdAt: doc.createdAt, - }, + docWithLocales: result, payload, req, }) diff --git a/packages/payload/src/collections/operations/updateByID.ts b/packages/payload/src/collections/operations/updateByID.ts index 5433258716..f9146b6211 100644 --- a/packages/payload/src/collections/operations/updateByID.ts +++ b/packages/payload/src/collections/operations/updateByID.ts @@ -356,10 +356,7 @@ export const updateByIDOperation = async ( id, autosave, collection: collectionConfig, - docWithLocales: { - ...result, - createdAt: docWithLocales.createdAt, - }, + docWithLocales: result, draft: shouldSaveDraft, payload, publishSpecificLocale, diff --git a/packages/payload/src/globals/operations/update.ts b/packages/payload/src/globals/operations/update.ts index d52924e2de..9d981b8e37 100644 --- a/packages/payload/src/globals/operations/update.ts +++ b/packages/payload/src/globals/operations/update.ts @@ -245,11 +245,7 @@ export const updateOperation = async ( const { globalType } = result result = await saveVersion({ autosave, - docWithLocales: { - ...result, - createdAt: result.createdAt, - updatedAt: result.updatedAt, - }, + docWithLocales: result, draft: shouldSaveDraft, global: globalConfig, payload, diff --git a/packages/payload/src/versions/drafts/getQueryDraftsSort.ts b/packages/payload/src/versions/drafts/getQueryDraftsSort.ts index a6a5448d15..e4ccd3b6f4 100644 --- a/packages/payload/src/versions/drafts/getQueryDraftsSort.ts +++ b/packages/payload/src/versions/drafts/getQueryDraftsSort.ts @@ -1,10 +1,22 @@ +import type { SanitizedCollectionConfig } from '../../collections/config/types.js' + /** * Takes the incoming sort argument and prefixes it with `versions.` and preserves any `-` prefixes for descending order * @param sort */ -export const getQueryDraftsSort = (sort: string): string => { +export const getQueryDraftsSort = ({ + collectionConfig, + sort, +}: { + collectionConfig: SanitizedCollectionConfig + sort: string +}): string => { if (!sort) { - return sort + if (collectionConfig.defaultSort) { + sort = collectionConfig.defaultSort + } else { + sort = '-createdAt' + } } let direction = '' diff --git a/packages/payload/src/versions/getLatestCollectionVersion.ts b/packages/payload/src/versions/getLatestCollectionVersion.ts index 8366d3bbe2..ada7ff212e 100644 --- a/packages/payload/src/versions/getLatestCollectionVersion.ts +++ b/packages/payload/src/versions/getLatestCollectionVersion.ts @@ -48,10 +48,7 @@ export const getLatestCollectionVersion = async ({ return undefined } - return { - ...latestVersion.version, - id, - createdAt: latestVersion.createdAt, - updatedAt: latestVersion.updatedAt, - } + latestVersion.version.id = id + + return latestVersion.version } diff --git a/packages/payload/src/versions/getLatestGlobalVersion.ts b/packages/payload/src/versions/getLatestGlobalVersion.ts index 5d0413bf63..e32689b318 100644 --- a/packages/payload/src/versions/getLatestGlobalVersion.ts +++ b/packages/payload/src/versions/getLatestGlobalVersion.ts @@ -56,12 +56,16 @@ export const getLatestGlobalVersion = async ({ } } + if (!latestVersion.version.createdAt) { + latestVersion.version.createdAt = latestVersion.createdAt + } + + if (!latestVersion.version.updatedAt) { + latestVersion.version.updatedAt = latestVersion.updatedAt + } + return { - global: { - ...latestVersion.version, - createdAt: latestVersion.createdAt, - updatedAt: latestVersion.updatedAt, - }, + global: latestVersion.version, globalExists, } } diff --git a/packages/payload/src/versions/saveVersion.ts b/packages/payload/src/versions/saveVersion.ts index 161c245d0c..0febf4ac02 100644 --- a/packages/payload/src/versions/saveVersion.ts +++ b/packages/payload/src/versions/saveVersion.ts @@ -82,7 +82,7 @@ export const saveVersion = async ({ const data: Record = { createdAt: new Date(latestVersion.createdAt).toISOString(), - updatedAt: draft ? now : new Date(doc.updatedAt).toISOString(), + updatedAt: now, version: { ...versionData, }, @@ -114,12 +114,12 @@ export const saveVersion = async ({ const createVersionArgs = { autosave: Boolean(autosave), collectionSlug: undefined, - createdAt: doc?.createdAt ? new Date(doc.createdAt).toISOString() : now, + createdAt: now, globalSlug: undefined, parent: collection ? id : undefined, publishedLocale: publishSpecificLocale || undefined, req, - updatedAt: draft ? now : new Date(doc.updatedAt).toISOString(), + updatedAt: now, versionData, } @@ -194,8 +194,6 @@ export const saveVersion = async ({ } let createdVersion = result.version - createdVersion.createdAt = result.createdAt - createdVersion.updatedAt = result.updatedAt createdVersion = sanitizeInternalFields(createdVersion) createdVersion.id = result.parent diff --git a/packages/ui/src/providers/DocumentInfo/index.tsx b/packages/ui/src/providers/DocumentInfo/index.tsx index c190386a22..07978ba9a3 100644 --- a/packages/ui/src/providers/DocumentInfo/index.tsx +++ b/packages/ui/src/providers/DocumentInfo/index.tsx @@ -339,6 +339,11 @@ const DocumentInfo: React.FC< ...versionParams.where, and: [ ...versionParams.where.and, + { + 'version._status': { + equals: 'draft', + }, + }, { updatedAt: { greater_than: publishedJSON.updatedAt, diff --git a/test/versions/int.spec.ts b/test/versions/int.spec.ts index 1ce308b953..6dbfed6198 100644 --- a/test/versions/int.spec.ts +++ b/test/versions/int.spec.ts @@ -14,6 +14,7 @@ import AutosavePosts from './collections/Autosave.js' import AutosaveGlobal from './globals/Autosave.js' import { autosaveCollectionSlug, + autoSaveGlobalSlug, draftCollectionSlug, localizedCollectionSlug, localizedGlobalSlug, @@ -306,21 +307,55 @@ describe('Versions', () => { ) }) - it('should have the same createdAt on new version create', async () => { + it('should have different createdAt in a new version while the same version.createdAt', async () => { const doc = await payload.create({ collection: autosaveCollectionSlug, data: { description: 'descr', title: 'title' }, }) - await wait(30) + await wait(10) - const updated = await payload.update({ + const upd = await payload.update({ collection: autosaveCollectionSlug, id: doc.id, data: {}, }) - expect(doc.createdAt).toBe(updated.createdAt) + expect(upd.createdAt).toBe(doc.createdAt) + + const { + docs: [latestVersionData], + } = await payload.findVersions({ + collection: autosaveCollectionSlug, + where: { + and: [ + { + parent: { + equals: doc.id, + }, + latest: { + equals: true, + }, + }, + ], + }, + }) + + // Version itself should have new createdAt + expect(new Date(latestVersionData.createdAt) > new Date(doc.createdAt)).toBe(true) + // But the same createdAt in version data! + expect(latestVersionData.version.createdAt).toBe(doc.createdAt) + + const fromNonVersionsTable = await payload.findByID({ + draft: false, + id: doc.id, + collection: autosaveCollectionSlug, + }) + + // createdAt from non-versions should be the same as version_createdAt in versions + expect(fromNonVersionsTable.createdAt).toBe(latestVersionData.version.createdAt) + // When creating new version - updatedAt should match version.updatedAt + expect(fromNonVersionsTable.updatedAt).toBe(latestVersionData.version.updatedAt) }) }) @@ -1265,6 +1300,48 @@ describe('Versions', () => { expect(updatedGlobal._status).toStrictEqual('draft') expect(globalLocalVersionID).toBeDefined() }) + + it('should have different createdAt in a new version while the same version.createdAt', async () => { + const doc = await payload.updateGlobal({ + slug: autoSaveGlobalSlug, + data: { title: 'asd' }, + }) + + await wait(10) + + const upd = await payload.updateGlobal({ + slug: autoSaveGlobalSlug, + data: { title: 'asd2' }, + }) + + expect(upd.createdAt).toBe(doc.createdAt) + + const { + docs: [latestVersionData], + } = await payload.findGlobalVersions({ + slug: autoSaveGlobalSlug, + where: { + latest: { + equals: true, + }, + }, + }) + + // Version itself should have new createdAt + expect(new Date(latestVersionData.createdAt) > new Date(doc.createdAt)).toBe(true) + // But the same version.createdAt! + expect(latestVersionData.version.createdAt).toBe(doc.createdAt) + + const fromNonVersionsTable = await payload.findGlobal({ + draft: false, + slug: autoSaveGlobalSlug, + }) + + // createdAt from non-versions should be the same as version_createdAt in versions + expect(fromNonVersionsTable.createdAt).toBe(latestVersionData.version.createdAt) + // When creating a new version - updatedAt should match + expect(fromNonVersionsTable.updatedAt).toBe(latestVersionData.version.updatedAt) + }) }) describe('Read', () => {