perf(ui): do not fetch doc permissions on autosave (#13477)
No need to re-fetch doc permissions during autosave. This will save us from making two additional client-side requests on every autosave interval, on top of the two existing requests needed to autosave and refresh form state. This _does_ mean that the UI will not fully reflect permissions again until you fully save, or until you navigating back, but that has always been the behavior anyway (until #13416). Maybe we can find another solution for this in the future, or otherwise consider this to be expected behavior. --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211094073049052
This commit is contained in:
@@ -6,6 +6,8 @@ import { dequal } from 'dequal/lite'
|
||||
import { reduceFieldsToValues, versionDefaults } from 'payload/shared'
|
||||
import React, { useDeferredValue, useEffect, useRef, useState } from 'react'
|
||||
|
||||
import type { OnSaveContext } from '../../views/Edit/index.js'
|
||||
|
||||
import {
|
||||
useAllFormFields,
|
||||
useForm,
|
||||
@@ -45,7 +47,6 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
|
||||
|
||||
const {
|
||||
docConfig,
|
||||
incrementVersionCount,
|
||||
lastUpdateTime,
|
||||
mostRecentVersionIsAutosaved,
|
||||
setMostRecentVersionIsAutosaved,
|
||||
@@ -149,15 +150,14 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
|
||||
submitted && !valid && versionsConfig?.drafts && versionsConfig?.drafts?.validate
|
||||
|
||||
if (!skipSubmission && modifiedRef.current && url) {
|
||||
const result = await submit<{
|
||||
incrementVersionCount: boolean
|
||||
}>({
|
||||
const result = await submit<any, OnSaveContext>({
|
||||
acceptValues: {
|
||||
overrideLocalChanges: false,
|
||||
},
|
||||
action: url,
|
||||
context: {
|
||||
incrementVersionCount: false,
|
||||
getDocPermissions: false,
|
||||
incrementVersionCount: !mostRecentVersionIsAutosaved,
|
||||
},
|
||||
disableFormWhileProcessing: false,
|
||||
disableSuccessStatus: true,
|
||||
@@ -169,7 +169,6 @@ export const Autosave: React.FC<Props> = ({ id, collection, global: globalDoc })
|
||||
})
|
||||
|
||||
if (result && result?.res?.ok && !mostRecentVersionIsAutosaved) {
|
||||
incrementVersionCount()
|
||||
setMostRecentVersionIsAutosaved(true)
|
||||
setUnpublishedVersionCount((prev) => prev + 1)
|
||||
}
|
||||
|
||||
@@ -16,6 +16,20 @@ export type Preferences = {
|
||||
[key: string]: unknown
|
||||
}
|
||||
|
||||
export type FormOnSuccess<T = unknown, C = Record<string, unknown>> = (
|
||||
json: T,
|
||||
options?: {
|
||||
/**
|
||||
* Arbitrary context passed to the onSuccess callback.
|
||||
*/
|
||||
context?: C
|
||||
/**
|
||||
* Form state at the time of the request used to retrieve the JSON response.
|
||||
*/
|
||||
formState?: FormState
|
||||
},
|
||||
) => Promise<FormState | void> | void
|
||||
|
||||
export type FormProps = {
|
||||
beforeSubmit?: ((args: { formState: FormState }) => Promise<FormState>)[]
|
||||
children?: React.ReactNode
|
||||
@@ -54,16 +68,7 @@ export type FormProps = {
|
||||
log?: boolean
|
||||
onChange?: ((args: { formState: FormState; submitted?: boolean }) => Promise<FormState>)[]
|
||||
onSubmit?: (fields: FormState, data: Data) => void
|
||||
onSuccess?: (
|
||||
json: unknown,
|
||||
options?: {
|
||||
/**
|
||||
* Arbitrary context passed to the onSuccess callback.
|
||||
*/
|
||||
context?: Record<string, unknown>
|
||||
formState?: FormState
|
||||
},
|
||||
) => Promise<FormState | void> | void
|
||||
onSuccess?: FormOnSuccess
|
||||
redirect?: string
|
||||
submitted?: boolean
|
||||
uuid?: string
|
||||
@@ -79,14 +84,14 @@ export type FormProps = {
|
||||
}
|
||||
)
|
||||
|
||||
export type SubmitOptions<T = Record<string, unknown>> = {
|
||||
export type SubmitOptions<C = Record<string, unknown>> = {
|
||||
acceptValues?: AcceptValues
|
||||
action?: string
|
||||
/**
|
||||
* @experimental - Note: this property is experimental and may change in the future. Use at your own discretion.
|
||||
* If you want to pass additional data to the onSuccess callback, you can use this context object.
|
||||
*/
|
||||
context?: T
|
||||
context?: C
|
||||
/**
|
||||
* When true, will disable the form while it is processing.
|
||||
* @default true
|
||||
@@ -108,14 +113,14 @@ export type SubmitOptions<T = Record<string, unknown>> = {
|
||||
|
||||
export type DispatchFields = React.Dispatch<any>
|
||||
|
||||
export type Submit = <T extends Record<string, unknown>>(
|
||||
options?: SubmitOptions<T>,
|
||||
export type Submit = <T extends Response, C extends Record<string, unknown>>(
|
||||
options?: SubmitOptions<C>,
|
||||
e?: React.FormEvent<HTMLFormElement>,
|
||||
) => Promise</**
|
||||
* @experimental - Note: the `{ res: ... }` return type is experimental and may change in the future. Use at your own discretion.
|
||||
* Returns the form state and the response from the server.
|
||||
*/
|
||||
{ formState?: FormState; res: Response } | void>
|
||||
{ formState?: FormState; res: T } | void>
|
||||
|
||||
export type ValidateForm = () => Promise<boolean>
|
||||
|
||||
|
||||
@@ -14,6 +14,8 @@ import type {
|
||||
|
||||
import React from 'react'
|
||||
|
||||
import type { GetDocPermissions } from './useGetDocPermissions.js'
|
||||
|
||||
export type DocumentInfoProps = {
|
||||
readonly action?: string
|
||||
readonly AfterDocument?: React.ReactNode
|
||||
@@ -57,7 +59,7 @@ export type DocumentInfoContext = {
|
||||
isLocked: boolean
|
||||
user: ClientUser | number | string
|
||||
} | null>
|
||||
getDocPermissions: (data?: Data) => Promise<void>
|
||||
getDocPermissions: GetDocPermissions
|
||||
getDocPreferences: () => Promise<DocumentPreferences>
|
||||
incrementVersionCount: () => void
|
||||
isInitializing: boolean
|
||||
|
||||
@@ -6,6 +6,8 @@ import React from 'react'
|
||||
import { hasSavePermission as getHasSavePermission } from '../../utilities/hasSavePermission.js'
|
||||
import { isEditing as getIsEditing } from '../../utilities/isEditing.js'
|
||||
|
||||
export type GetDocPermissions = (data?: Data) => Promise<void>
|
||||
|
||||
export const useGetDocPermissions = ({
|
||||
id,
|
||||
api,
|
||||
@@ -30,7 +32,7 @@ export const useGetDocPermissions = ({
|
||||
setDocPermissions: React.Dispatch<React.SetStateAction<SanitizedDocumentPermissions>>
|
||||
setHasPublishPermission: React.Dispatch<React.SetStateAction<boolean>>
|
||||
setHasSavePermission: React.Dispatch<React.SetStateAction<boolean>>
|
||||
}) =>
|
||||
}): GetDocPermissions =>
|
||||
React.useCallback(
|
||||
async (data: Data) => {
|
||||
const params = {
|
||||
@@ -111,5 +113,18 @@ export const useGetDocPermissions = ({
|
||||
)
|
||||
}
|
||||
},
|
||||
[serverURL, api, id, permissions, i18n.language, locale, collectionSlug, globalSlug],
|
||||
[
|
||||
locale,
|
||||
id,
|
||||
collectionSlug,
|
||||
globalSlug,
|
||||
serverURL,
|
||||
api,
|
||||
i18n.language,
|
||||
setDocPermissions,
|
||||
setHasSavePermission,
|
||||
setHasPublishPermission,
|
||||
permissions?.collections,
|
||||
permissions?.globals,
|
||||
],
|
||||
)
|
||||
|
||||
@@ -1,13 +1,14 @@
|
||||
/* eslint-disable react-compiler/react-compiler -- TODO: fix */
|
||||
'use client'
|
||||
|
||||
import type { ClientUser, DocumentViewClientProps } from 'payload'
|
||||
import type { ClientUser, DocumentViewClientProps, FormState } from 'payload'
|
||||
|
||||
import { useRouter, useSearchParams } from 'next/navigation.js'
|
||||
import { formatAdminURL } from 'payload/shared'
|
||||
import React, { Fragment, useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||
|
||||
import type { FormProps } from '../../forms/Form/index.js'
|
||||
import type { FormOnSuccess } from '../../forms/Form/types.js'
|
||||
import type { LockedState } from '../../utilities/buildFormState.js'
|
||||
|
||||
import { DocumentControls } from '../../elements/DocumentControls/index.js'
|
||||
@@ -42,6 +43,11 @@ import './index.scss'
|
||||
|
||||
const baseClass = 'collection-edit'
|
||||
|
||||
export type OnSaveContext = {
|
||||
getDocPermissions?: boolean
|
||||
incrementVersionCount?: boolean
|
||||
}
|
||||
|
||||
// This component receives props only on _pages_
|
||||
// When rendered within a drawer, props are empty
|
||||
// This is solely to support custom edit views which get server-rendered
|
||||
@@ -256,13 +262,12 @@ export function DefaultEditView({
|
||||
user?.id,
|
||||
])
|
||||
|
||||
const onSave = useCallback<FormProps['onSuccess']>(
|
||||
const onSave: FormOnSuccess<any, OnSaveContext> = useCallback(
|
||||
async (json, options) => {
|
||||
const { context, formState } = options || {}
|
||||
|
||||
const controller = handleAbortRef(abortOnSaveRef)
|
||||
|
||||
// @ts-expect-error can ignore
|
||||
const document = json?.doc || json?.result
|
||||
|
||||
const updatedAt = document?.updatedAt || new Date().toISOString()
|
||||
@@ -316,7 +321,9 @@ export function DefaultEditView({
|
||||
resetUploadEdits()
|
||||
}
|
||||
|
||||
await getDocPermissions(json)
|
||||
if (context?.getDocPermissions !== false) {
|
||||
await getDocPermissions(json)
|
||||
}
|
||||
|
||||
if (id || globalSlug) {
|
||||
const docPreferences = await getDocPreferences()
|
||||
|
||||
@@ -18,7 +18,6 @@ export const AutosavePostsCollection: CollectionConfig = {
|
||||
hooks: {
|
||||
beforeChange: [({ data }) => data?.title],
|
||||
},
|
||||
label: 'Computed Title',
|
||||
},
|
||||
],
|
||||
versions: {
|
||||
|
||||
@@ -10,6 +10,7 @@ import { addArrayRowAsync, removeArrayRow } from 'helpers/e2e/fields/array/index
|
||||
import { addBlock } from 'helpers/e2e/fields/blocks/index.js'
|
||||
import { waitForAutoSaveToRunAndComplete } from 'helpers/e2e/waitForAutoSaveToRunAndComplete.js'
|
||||
import * as path from 'path'
|
||||
import { wait } from 'payload/shared'
|
||||
import { fileURLToPath } from 'url'
|
||||
|
||||
import type { Config, Post } from './payload-types.js'
|
||||
@@ -330,6 +331,94 @@ test.describe('Form State', () => {
|
||||
).toHaveValue('This is a computed value.')
|
||||
})
|
||||
|
||||
test('should fetch new doc permissions after save', async () => {
|
||||
const doc = await createPost({ title: 'Initial Title' })
|
||||
await page.goto(postsUrl.edit(doc.id))
|
||||
const titleField = page.locator('#field-title')
|
||||
await expect(titleField).toBeEnabled()
|
||||
|
||||
await assertNetworkRequests(
|
||||
page,
|
||||
`${serverURL}/api/posts/access/${doc.id}`,
|
||||
async () => {
|
||||
await titleField.fill('Updated Title')
|
||||
await wait(500)
|
||||
await page.click('#action-save', { delay: 100 })
|
||||
},
|
||||
{
|
||||
allowedNumberOfRequests: 2,
|
||||
minimumNumberOfRequests: 2,
|
||||
timeout: 3000,
|
||||
},
|
||||
)
|
||||
|
||||
await assertNetworkRequests(
|
||||
page,
|
||||
`${serverURL}/api/posts/access/${doc.id}`,
|
||||
async () => {
|
||||
await titleField.fill('Updated Title 2')
|
||||
await wait(500)
|
||||
await page.click('#action-save', { delay: 100 })
|
||||
},
|
||||
{
|
||||
minimumNumberOfRequests: 2,
|
||||
allowedNumberOfRequests: 2,
|
||||
timeout: 3000,
|
||||
},
|
||||
)
|
||||
})
|
||||
|
||||
test('autosave - should not fetch new doc permissions on every autosave', async () => {
|
||||
const doc = await payload.create({
|
||||
collection: autosavePostsSlug,
|
||||
data: {
|
||||
title: 'Initial Title',
|
||||
},
|
||||
})
|
||||
|
||||
await page.goto(autosavePostsUrl.edit(doc.id))
|
||||
const titleField = page.locator('#field-title')
|
||||
await expect(titleField).toBeEnabled()
|
||||
|
||||
await assertNetworkRequests(
|
||||
page,
|
||||
`${serverURL}/api/${autosavePostsSlug}/access/${doc.id}`,
|
||||
async () => {
|
||||
await titleField.fill('Updated Title')
|
||||
},
|
||||
{
|
||||
allowedNumberOfRequests: 0,
|
||||
timeout: 3000,
|
||||
},
|
||||
)
|
||||
|
||||
await assertNetworkRequests(
|
||||
page,
|
||||
`${serverURL}/api/${autosavePostsSlug}/access/${doc.id}`,
|
||||
async () => {
|
||||
await titleField.fill('Updated Title Again')
|
||||
},
|
||||
{
|
||||
allowedNumberOfRequests: 0,
|
||||
timeout: 3000,
|
||||
},
|
||||
)
|
||||
|
||||
// save manually and ensure the permissions are fetched again
|
||||
await assertNetworkRequests(
|
||||
page,
|
||||
`${serverURL}/api/${autosavePostsSlug}/access/${doc.id}`,
|
||||
async () => {
|
||||
await page.click('#action-save', { delay: 100 })
|
||||
},
|
||||
{
|
||||
allowedNumberOfRequests: 2,
|
||||
minimumNumberOfRequests: 2,
|
||||
timeout: 3000,
|
||||
},
|
||||
)
|
||||
})
|
||||
|
||||
test('autosave - should render computed values after autosave', async () => {
|
||||
await page.goto(autosavePostsUrl.create)
|
||||
const titleField = page.locator('#field-title')
|
||||
|
||||
Reference in New Issue
Block a user