Skip to content

Commit 512a8fa

Browse files
authored
fix: server component readonly state issues with document locking (#13878)
### What? Fixed two bugs with readonly state for server-rendered components (like richtext fields and custom server fields): 1. Server components remained readonly after a user took over a locked document 2. Server components were not readonly when viewing in "read only" mode until page refresh ### Why? Both issues stemmed from server-rendered components using their initial readonly state that was baked in during server-side rendering, rather than respecting dynamic readonly state changes: 1. **Takeover bug**: When a user took over a locked document, client-side readonly state was updated but server components continued using their initial readonly state because the server-side state wasn't refreshed properly. 2. **Read-only view bug**: When entering "read only" mode, server components weren't immediately updated to reflect the new readonly state without a page refresh. The root cause was that server-side `buildFormState` was called with `readOnly: isLocked` during initial render, and individual field components used this initial state rather than respecting dynamic document-level readonly changes. ### How? 1. **Fixed race condition in `handleTakeOver`**: Made the function async and await the `updateDocumentEditor` call before calling `clearRouteCache()` to ensure the database is updated before page reload 2. **Improved editor comparison in `getIsLocked`**: Used `extractID()` helper to properly compare editor IDs when the editor might be a reference object 3. **Ensured cache clearing for all takeover scenarios**: Call `clearRouteCache()` for both DocumentLocked modal and DocumentControls takeovers to refresh server-side state 4. **Added Form key to force re-render**: Added `key={isLocked}` to the Form component so it re-renders when the lock state changes, ensuring all child components get fresh readonly state for both takeover and read-only view scenarios --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211373627247885
1 parent 7f35213 commit 512a8fa

File tree

10 files changed

+387
-38
lines changed

10 files changed

+387
-38
lines changed

packages/next/src/views/Document/getIsLocked.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
} from 'payload'
88

99
import { sanitizeID } from '@payloadcms/ui/shared'
10+
import { extractID } from 'payload/shared'
1011

1112
type Args = {
1213
collectionConfig?: SanitizedCollectionConfig
@@ -93,12 +94,12 @@ export const getIsLocked = async ({
9394
})
9495

9596
if (docs.length > 0) {
96-
const newEditor = docs[0].user?.value
97+
const currentEditor = docs[0].user?.value
9798
const lastUpdateTime = new Date(docs[0].updatedAt).getTime()
9899

99-
if (newEditor?.id !== req.user.id) {
100+
if (extractID(currentEditor) !== req.user.id) {
100101
return {
101-
currentEditor: newEditor,
102+
currentEditor,
102103
isLocked: true,
103104
lastUpdateTime,
104105
}

packages/ui/src/elements/DocumentLocked/index.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { ClientUser } from 'payload'
33

44
import React, { useEffect } from 'react'
55

6+
import { useRouteCache } from '../../providers/RouteCache/index.js'
67
import { useRouteTransition } from '../../providers/RouteTransition/index.js'
78
import { useTranslation } from '../../providers/Translation/index.js'
89
import { isClientUserObject } from '../../utilities/isClientUserObject.js'
@@ -38,6 +39,7 @@ export const DocumentLocked: React.FC<{
3839
}> = ({ handleGoBack, isActive, onReadOnly, onTakeOver, updatedAt, user }) => {
3940
const { closeModal, openModal } = useModal()
4041
const { t } = useTranslation()
42+
const { clearRouteCache } = useRouteCache()
4143
const { startRouteTransition } = useRouteTransition()
4244

4345
useEffect(() => {
@@ -86,6 +88,7 @@ export const DocumentLocked: React.FC<{
8688
onClick={() => {
8789
onReadOnly()
8890
closeModal(modalSlug)
91+
clearRouteCache()
8992
}}
9093
size="large"
9194
>
@@ -95,7 +98,7 @@ export const DocumentLocked: React.FC<{
9598
buttonStyle="primary"
9699
id={`${modalSlug}-take-over`}
97100
onClick={() => {
98-
void onTakeOver()
101+
onTakeOver()
99102
closeModal(modalSlug)
100103
}}
101104
size="large"

packages/ui/src/elements/DocumentTakeOver/index.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use client'
22
import React, { useEffect } from 'react'
33

4+
import { useRouteCache } from '../../providers/RouteCache/index.js'
45
import { useRouteTransition } from '../../providers/RouteTransition/index.js'
56
import { useTranslation } from '../../providers/Translation/index.js'
67
import { Button } from '../Button/index.js'
@@ -19,6 +20,7 @@ export const DocumentTakeOver: React.FC<{
1920
const { closeModal, openModal } = useModal()
2021
const { t } = useTranslation()
2122
const { startRouteTransition } = useRouteTransition()
23+
const { clearRouteCache } = useRouteCache()
2224

2325
useEffect(() => {
2426
if (isActive) {
@@ -52,6 +54,7 @@ export const DocumentTakeOver: React.FC<{
5254
onClick={() => {
5355
onReadOnly()
5456
closeModal(modalSlug)
57+
clearRouteCache()
5558
}}
5659
size="large"
5760
>

packages/ui/src/utilities/handleTakeOver.tsx

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,47 @@
11
import type { ClientUser } from 'payload'
22

3-
export const handleTakeOver = (
4-
id: number | string,
5-
collectionSlug: string,
6-
globalSlug: string,
7-
user: ClientUser | number | string,
8-
isWithinDoc: boolean,
9-
updateDocumentEditor: (
10-
docID: number | string,
11-
slug: string,
12-
user: ClientUser | number | string,
13-
) => Promise<void>,
14-
setCurrentEditor: (value: React.SetStateAction<ClientUser | number | string>) => void,
3+
export interface HandleTakeOverParams {
4+
clearRouteCache?: () => void
5+
collectionSlug?: string
156
documentLockStateRef: React.RefObject<{
167
hasShownLockedModal: boolean
178
isLocked: boolean
189
user: ClientUser | number | string
19-
}>,
20-
isLockingEnabled: boolean,
21-
setIsReadOnlyForIncomingUser?: (value: React.SetStateAction<boolean>) => void,
22-
): void => {
10+
}>
11+
globalSlug?: string
12+
id: number | string
13+
isLockingEnabled: boolean
14+
isWithinDoc: boolean
15+
setCurrentEditor: (value: React.SetStateAction<ClientUser | number | string>) => void
16+
setIsReadOnlyForIncomingUser?: (value: React.SetStateAction<boolean>) => void
17+
updateDocumentEditor: (
18+
docID: number | string,
19+
slug: string,
20+
user: ClientUser | number | string,
21+
) => Promise<void>
22+
user: ClientUser | number | string
23+
}
24+
25+
export const handleTakeOver = async ({
26+
id,
27+
clearRouteCache,
28+
collectionSlug,
29+
documentLockStateRef,
30+
globalSlug,
31+
isLockingEnabled,
32+
isWithinDoc,
33+
setCurrentEditor,
34+
setIsReadOnlyForIncomingUser,
35+
updateDocumentEditor,
36+
user,
37+
}: HandleTakeOverParams): Promise<void> => {
2338
if (!isLockingEnabled) {
2439
return
2540
}
2641

2742
try {
2843
// Call updateDocumentEditor to update the document's owner to the current user
29-
void updateDocumentEditor(id, collectionSlug ?? globalSlug, user)
44+
await updateDocumentEditor(id, collectionSlug ?? globalSlug, user)
3045

3146
if (!isWithinDoc) {
3247
documentLockStateRef.current.hasShownLockedModal = true
@@ -44,6 +59,11 @@ export const handleTakeOver = (
4459
if (isWithinDoc && setIsReadOnlyForIncomingUser) {
4560
setIsReadOnlyForIncomingUser(false)
4661
}
62+
63+
// Need to clear the route cache to refresh the page and update readOnly state for server rendered components
64+
if (clearRouteCache) {
65+
clearRouteCache()
66+
}
4767
} catch (error) {
4868
// eslint-disable-next-line no-console
4969
console.error('Error during document takeover:', error)

packages/ui/src/views/Edit/index.tsx

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import { useDocumentInfo } from '../../providers/DocumentInfo/index.js'
2828
import { useEditDepth } from '../../providers/EditDepth/index.js'
2929
import { useLivePreviewContext } from '../../providers/LivePreview/context.js'
3030
import { OperationProvider } from '../../providers/Operation/index.js'
31+
import { useRouteCache } from '../../providers/RouteCache/index.js'
3132
import { useRouteTransition } from '../../providers/RouteTransition/index.js'
3233
import { useServerFunctions } from '../../providers/ServerFunctions/index.js'
3334
import { UploadControlsProvider } from '../../providers/UploadControls/index.js'
@@ -87,6 +88,7 @@ export function DefaultEditView({
8788
initialState,
8889
isEditing,
8990
isInitializing,
91+
isLocked,
9092
isTrashed,
9193
lastUpdateTime,
9294
redirectAfterCreate,
@@ -134,6 +136,7 @@ export function DefaultEditView({
134136
const { resetUploadEdits } = useUploadEdits()
135137
const { getFormState } = useServerFunctions()
136138
const { startRouteTransition } = useRouteTransition()
139+
const { clearRouteCache } = useRouteCache()
137140
const {
138141
isLivePreviewEnabled,
139142
isLivePreviewing,
@@ -511,6 +514,7 @@ export function DefaultEditView({
511514
initialState={!isInitializing && initialState}
512515
isDocumentForm={true}
513516
isInitializing={isInitializing}
517+
key={`${isLocked}`}
514518
method={id ? 'PATCH' : 'POST'}
515519
onChange={[onChange]}
516520
onSuccess={onSave}
@@ -527,17 +531,18 @@ export function DefaultEditView({
527531
setShowTakeOverModal(false)
528532
}}
529533
onTakeOver={() =>
530-
handleTakeOver(
534+
handleTakeOver({
531535
id,
536+
clearRouteCache,
532537
collectionSlug,
538+
documentLockStateRef: documentLockState,
533539
globalSlug,
534-
user,
535-
false,
536-
updateDocumentEditor,
537-
setCurrentEditor,
538-
documentLockState,
539540
isLockingEnabled,
540-
)
541+
isWithinDoc: false,
542+
setCurrentEditor,
543+
updateDocumentEditor,
544+
user,
545+
})
541546
}
542547
updatedAt={lastUpdateTime}
543548
user={currentEditor}
@@ -597,18 +602,19 @@ export function DefaultEditView({
597602
onRestore={onRestore}
598603
onSave={onSave}
599604
onTakeOver={() =>
600-
handleTakeOver(
605+
handleTakeOver({
601606
id,
607+
clearRouteCache,
602608
collectionSlug,
609+
documentLockStateRef: documentLockState,
603610
globalSlug,
604-
user,
605-
true,
606-
updateDocumentEditor,
607-
setCurrentEditor,
608-
documentLockState,
609611
isLockingEnabled,
612+
isWithinDoc: true,
613+
setCurrentEditor,
610614
setIsReadOnlyForIncomingUser,
611-
)
615+
updateDocumentEditor,
616+
user,
617+
})
612618
}
613619
permissions={docPermissions}
614620
readOnlyForIncomingUser={isReadOnlyForIncomingUser}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import type { TextFieldServerComponent } from 'payload'
2+
import type React from 'react'
3+
4+
import { TextField } from '@payloadcms/ui'
5+
6+
export const CustomTextFieldServer: TextFieldServerComponent = ({
7+
clientField,
8+
path,
9+
permissions,
10+
readOnly,
11+
schemaPath,
12+
}) => {
13+
return (
14+
<TextField
15+
field={clientField}
16+
path={path}
17+
permissions={permissions}
18+
readOnly={readOnly}
19+
schemaPath={schemaPath}
20+
/>
21+
)
22+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import type { CollectionConfig } from 'payload'
2+
3+
export const serverComponentsSlug = 'server-components'
4+
5+
export const ServerComponentsCollection: CollectionConfig = {
6+
slug: serverComponentsSlug,
7+
admin: {
8+
useAsTitle: 'customTextServer',
9+
},
10+
fields: [
11+
{
12+
name: 'customTextServer',
13+
type: 'text',
14+
admin: {
15+
components: {
16+
Field: '/collections/Posts/fields/CustomTextFieldServer.tsx#CustomTextFieldServer',
17+
},
18+
},
19+
},
20+
{
21+
name: 'richText',
22+
type: 'richText',
23+
},
24+
],
25+
}

test/locked-documents/config.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import path from 'path'
44
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
55
import { PagesCollection } from './collections/Pages/index.js'
66
import { PostsCollection } from './collections/Posts/index.js'
7+
import { ServerComponentsCollection } from './collections/ServerComponents/index.js'
78
import { TestsCollection } from './collections/Tests/index.js'
89
import { Users } from './collections/Users/index.js'
910
import { AdminGlobal } from './globals/Admin/index.js'
@@ -19,7 +20,13 @@ export default buildConfigWithDefaults({
1920
baseDir: path.resolve(dirname),
2021
},
2122
},
22-
collections: [PagesCollection, PostsCollection, TestsCollection, Users],
23+
collections: [
24+
PagesCollection,
25+
PostsCollection,
26+
ServerComponentsCollection,
27+
TestsCollection,
28+
Users,
29+
],
2330
globals: [AdminGlobal, MenuGlobal],
2431
onInit: async (payload) => {
2532
if (process.env.SEED_IN_CONFIG_ONINIT !== 'false') {

0 commit comments

Comments
 (0)