chore: use updatedAt instead of editedAt for locked documents (#8324)
- Removes locked documents `editedAt` as it was redundant with the `updatedAt` timestamp - Adjust stale lock tests to configure the duration down to 1 second and await it to not lose any test coverage - DB performance changes: 1. Switch to payload.db.find instead of payload.find for checkDocumentLockStatus to avoid populating the user and other payload find overhead 2. Add maxDepth: 1 to user relationship 3. Add index to global slug
This commit is contained in:
@@ -25,13 +25,13 @@ const formatDate = (date) => {
|
||||
}
|
||||
|
||||
export const DocumentLocked: React.FC<{
|
||||
editedAt?: null | number
|
||||
handleGoBack: () => void
|
||||
isActive: boolean
|
||||
onReadOnly: () => void
|
||||
onTakeOver: () => void
|
||||
updatedAt?: null | number
|
||||
user?: ClientUser
|
||||
}> = ({ editedAt, handleGoBack, isActive, onReadOnly, onTakeOver, user }) => {
|
||||
}> = ({ handleGoBack, isActive, onReadOnly, onTakeOver, updatedAt, user }) => {
|
||||
const { closeModal, openModal } = useModal()
|
||||
const { t } = useTranslation()
|
||||
|
||||
@@ -52,7 +52,7 @@ export const DocumentLocked: React.FC<{
|
||||
<strong>{user?.email ?? user?.id}</strong> {t('general:currentlyEditing')}
|
||||
</p>
|
||||
<p>
|
||||
{t('general:editedSince')} <strong>{formatDate(editedAt)}</strong>
|
||||
{t('general:editedSince')} <strong>{formatDate(updatedAt)}</strong>
|
||||
</p>
|
||||
</div>
|
||||
<div className={`${baseClass}__controls`}>
|
||||
|
||||
@@ -421,7 +421,6 @@ export const DefaultEditView: React.FC = () => {
|
||||
{BeforeDocument}
|
||||
{isLockingEnabled && shouldShowDocumentLockedModal && !isReadOnlyForIncomingUser && (
|
||||
<DocumentLocked
|
||||
editedAt={lastUpdateTime}
|
||||
handleGoBack={handleGoBack}
|
||||
isActive={shouldShowDocumentLockedModal}
|
||||
onReadOnly={() => {
|
||||
@@ -429,6 +428,7 @@ export const DefaultEditView: React.FC = () => {
|
||||
setShowTakeOverModal(false)
|
||||
}}
|
||||
onTakeOver={handleTakeOver}
|
||||
updatedAt={lastUpdateTime}
|
||||
user={currentEditor}
|
||||
/>
|
||||
)}
|
||||
|
||||
@@ -14,17 +14,15 @@ export const getLockedDocumentsCollection = (config: Config): CollectionConfig =
|
||||
maxDepth: 0,
|
||||
relationTo: [...config.collections.map((collectionConfig) => collectionConfig.slug)],
|
||||
},
|
||||
{
|
||||
name: 'editedAt',
|
||||
type: 'date',
|
||||
},
|
||||
{
|
||||
name: 'globalSlug',
|
||||
type: 'text',
|
||||
index: true,
|
||||
},
|
||||
{
|
||||
name: 'user',
|
||||
type: 'relationship',
|
||||
maxDepth: 1,
|
||||
relationTo: config.collections
|
||||
.filter((collectionConfig) => collectionConfig.auth)
|
||||
.map((collectionConfig) => collectionConfig.slug),
|
||||
|
||||
@@ -55,9 +55,8 @@ export const checkDocumentLockStatus = async ({
|
||||
|
||||
const finalLockErrorMessage = lockErrorMessage || defaultLockErrorMessage
|
||||
|
||||
const lockedDocumentResult: PaginatedDocs<JsonObject & TypeWithID> = await payload.find({
|
||||
const lockedDocumentResult: PaginatedDocs<JsonObject & TypeWithID> = await payload.db.find({
|
||||
collection: 'payload-locked-documents',
|
||||
depth: 1,
|
||||
limit: 1,
|
||||
pagination: false,
|
||||
req,
|
||||
@@ -68,8 +67,8 @@ export const checkDocumentLockStatus = async ({
|
||||
// If there's a locked document, check lock conditions
|
||||
const lockedDoc = lockedDocumentResult?.docs[0]
|
||||
if (lockedDoc) {
|
||||
const lastEditedAt = new Date(lockedDoc?.editedAt)
|
||||
const now = new Date()
|
||||
const lastEditedAt = new Date(lockedDoc?.updatedAt).getTime()
|
||||
const now = new Date().getTime()
|
||||
|
||||
const lockDuration =
|
||||
typeof lockDocumentsProp === 'object' ? lockDocumentsProp.duration : lockDurationDefault
|
||||
@@ -79,8 +78,8 @@ export const checkDocumentLockStatus = async ({
|
||||
|
||||
// document is locked by another user and the lock hasn't expired
|
||||
if (
|
||||
lockedDoc.user?.value?.id !== currentUserId &&
|
||||
now.getTime() - lastEditedAt.getTime() <= lockDurationInMilliseconds
|
||||
lockedDoc.user?.value !== currentUserId &&
|
||||
now - lastEditedAt <= lockDurationInMilliseconds
|
||||
) {
|
||||
throw new Locked(finalLockErrorMessage)
|
||||
}
|
||||
|
||||
@@ -193,7 +193,6 @@ const DocumentInfo: React.FC<
|
||||
// Send a patch request to update the _lastEdited info
|
||||
await requests.patch(`${serverURL}${api}/payload-locked-documents/${lockId}`, {
|
||||
body: JSON.stringify({
|
||||
editedAt: new Date(),
|
||||
user: { relationTo: user?.collection, value: user?.id },
|
||||
}),
|
||||
headers: {
|
||||
|
||||
@@ -265,9 +265,7 @@ export const buildFormState = async ({
|
||||
await req.payload.db.updateOne({
|
||||
id: lockedDocument.docs[0].id,
|
||||
collection: 'payload-locked-documents',
|
||||
data: {
|
||||
editedAt: new Date().toISOString(),
|
||||
},
|
||||
data: {},
|
||||
req,
|
||||
})
|
||||
}
|
||||
@@ -284,7 +282,6 @@ export const buildFormState = async ({
|
||||
value: id,
|
||||
}
|
||||
: undefined,
|
||||
editedAt: new Date().toISOString(),
|
||||
globalSlug: globalSlug ? globalSlug : undefined,
|
||||
user: {
|
||||
relationTo: [req.user.collection],
|
||||
|
||||
@@ -98,7 +98,6 @@ describe('locked documents', () => {
|
||||
relationTo: 'posts',
|
||||
value: postDoc.id,
|
||||
},
|
||||
editedAt: new Date().toISOString(),
|
||||
globalSlug: undefined,
|
||||
user: {
|
||||
relationTo: 'users',
|
||||
@@ -318,7 +317,6 @@ describe('locked documents', () => {
|
||||
relationTo: 'posts',
|
||||
value: postDoc.id,
|
||||
},
|
||||
editedAt: new Date().toISOString(),
|
||||
globalSlug: undefined,
|
||||
user: {
|
||||
relationTo: 'users',
|
||||
@@ -422,7 +420,6 @@ describe('locked documents', () => {
|
||||
relationTo: 'posts',
|
||||
value: postDoc.id,
|
||||
},
|
||||
editedAt: new Date().toISOString(),
|
||||
globalSlug: undefined,
|
||||
user: {
|
||||
relationTo: 'users',
|
||||
@@ -512,7 +509,6 @@ describe('locked documents', () => {
|
||||
relationTo: 'posts',
|
||||
value: postDoc.id,
|
||||
},
|
||||
editedAt: new Date().toISOString(),
|
||||
globalSlug: undefined,
|
||||
user: {
|
||||
relationTo: 'users',
|
||||
@@ -784,7 +780,6 @@ describe('locked documents', () => {
|
||||
collection: lockedDocumentCollection,
|
||||
data: {
|
||||
document: undefined,
|
||||
editedAt: new Date().toISOString(),
|
||||
globalSlug: 'menu',
|
||||
user: {
|
||||
relationTo: 'users',
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import type { Payload, SanitizedCollectionConfig } from 'payload'
|
||||
|
||||
import path from 'path'
|
||||
import { Locked, NotFound, type Payload } from 'payload'
|
||||
import { Locked, NotFound } from 'payload'
|
||||
import { wait } from 'payload/shared'
|
||||
import { fileURLToPath } from 'url'
|
||||
|
||||
import type { NextRESTClient } from '../helpers/NextRESTClient.js'
|
||||
@@ -26,10 +29,13 @@ describe('Locked documents', () => {
|
||||
let menu: Menu
|
||||
let user: any
|
||||
let user2: any
|
||||
let postConfig: SanitizedCollectionConfig
|
||||
|
||||
beforeAll(async () => {
|
||||
;({ payload, restClient } = await initPayloadInt(dirname))
|
||||
|
||||
postConfig = payload.config.collections.find(({ slug }) => slug === postsSlug)
|
||||
|
||||
const loginResult = await payload.login({
|
||||
collection: 'users',
|
||||
data: {
|
||||
@@ -77,6 +83,10 @@ describe('Locked documents', () => {
|
||||
}
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
postConfig.lockDocuments = { duration: 300 }
|
||||
})
|
||||
|
||||
it('should update unlocked document - collection', async () => {
|
||||
const updatedPost = await payload.update({
|
||||
collection: postsSlug,
|
||||
@@ -129,16 +139,13 @@ describe('Locked documents', () => {
|
||||
},
|
||||
})
|
||||
|
||||
// Subtract 3.5 minutes (210 seconds) from the current time
|
||||
const pastEditedAt = new Date()
|
||||
pastEditedAt.setMinutes(pastEditedAt.getMinutes() - 3)
|
||||
pastEditedAt.setSeconds(pastEditedAt.getSeconds() - 30)
|
||||
// Set lock duration to 1 second for testing purposes
|
||||
postConfig.lockDocuments = { duration: 1 }
|
||||
|
||||
// Give locking ownership to another user
|
||||
const lockedDocInstance = await payload.create({
|
||||
collection: lockedDocumentCollection,
|
||||
data: {
|
||||
editedAt: pastEditedAt.toISOString(),
|
||||
user: {
|
||||
relationTo: 'users',
|
||||
value: user2.id,
|
||||
@@ -151,6 +158,8 @@ describe('Locked documents', () => {
|
||||
},
|
||||
})
|
||||
|
||||
await wait(1100)
|
||||
|
||||
const updateLockedDoc = await payload.update({
|
||||
collection: postsSlug,
|
||||
data: {
|
||||
@@ -159,6 +168,7 @@ describe('Locked documents', () => {
|
||||
overrideLock: false,
|
||||
id: newPost2.id,
|
||||
})
|
||||
postConfig.lockDocuments = { duration: 300 }
|
||||
|
||||
// Should allow update since editedAt date is past expiration duration.
|
||||
// Therefore the document is considered stale
|
||||
@@ -187,16 +197,13 @@ describe('Locked documents', () => {
|
||||
})
|
||||
|
||||
it('should allow update of stale locked document - global', async () => {
|
||||
// Subtract 5.5 minutes (330 seconds) from the current time
|
||||
const pastEditedAt = new Date()
|
||||
pastEditedAt.setMinutes(pastEditedAt.getMinutes() - 5)
|
||||
pastEditedAt.setSeconds(pastEditedAt.getSeconds() - 30)
|
||||
|
||||
// Set lock duration to 1 second for testing purposes
|
||||
const globalConfig = payload.config.globals.find(({ slug }) => slug === menuSlug)
|
||||
globalConfig.lockDocuments = { duration: 1 }
|
||||
// Give locking ownership to another user
|
||||
const lockedGlobalInstance = await payload.create({
|
||||
collection: lockedDocumentCollection,
|
||||
data: {
|
||||
editedAt: pastEditedAt.toISOString(), // stale date
|
||||
user: {
|
||||
relationTo: 'users',
|
||||
value: user2.id,
|
||||
@@ -206,6 +213,8 @@ describe('Locked documents', () => {
|
||||
},
|
||||
})
|
||||
|
||||
await wait(1100)
|
||||
|
||||
const updateGlobalLockedDoc = await payload.updateGlobal({
|
||||
data: {
|
||||
globalText: 'global text 2',
|
||||
@@ -213,6 +222,7 @@ describe('Locked documents', () => {
|
||||
overrideLock: false,
|
||||
slug: menuSlug,
|
||||
})
|
||||
globalConfig.lockDocuments = { duration: 300 }
|
||||
|
||||
// Should allow update since editedAt date is past expiration duration.
|
||||
// Therefore the document is considered stale
|
||||
@@ -379,16 +389,13 @@ describe('Locked documents', () => {
|
||||
},
|
||||
})
|
||||
|
||||
// Subtract 3.5 minutes (210 seconds) from the current time
|
||||
const pastEditedAt = new Date()
|
||||
pastEditedAt.setMinutes(pastEditedAt.getMinutes() - 3)
|
||||
pastEditedAt.setSeconds(pastEditedAt.getSeconds() - 30)
|
||||
// Set lock duration to 1 second for testing purposes
|
||||
postConfig.lockDocuments = { duration: 1 }
|
||||
|
||||
// Give locking ownership to another user
|
||||
const lockedDocInstance = await payload.create({
|
||||
collection: lockedDocumentCollection,
|
||||
data: {
|
||||
editedAt: pastEditedAt.toISOString(), // stale date
|
||||
user: {
|
||||
relationTo: 'users',
|
||||
value: user2.id,
|
||||
@@ -401,6 +408,8 @@ describe('Locked documents', () => {
|
||||
},
|
||||
})
|
||||
|
||||
await wait(1100)
|
||||
|
||||
await payload.delete({
|
||||
collection: postsSlug,
|
||||
id: newPost4.id,
|
||||
|
||||
@@ -111,7 +111,6 @@ export interface PayloadLockedDocument {
|
||||
relationTo: 'users';
|
||||
value: string | User;
|
||||
};
|
||||
editedAt?: string | null;
|
||||
updatedAt: string;
|
||||
createdAt: string;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user