fix(ui): prevents unwanted data overrides when bulk editing (#9842)

### What?

It became possible for fields to reset to a defined `defaultValue` when
bulk editing from the `edit-many` drawer.

### Why?

The form-state of all fields were being considered during a bulk edit -
this also meant using their initial states - this meant any fields with
default values or nested fields (`arrays`) would be overwritten with
their initial states

I.e. empty values or default values.

### How?

Now - we only send through the form data of the fields specifically
being edited in the edit-many drawer and ignore all other fields.

Leaving all other fields stay their current values.

Fixes #9590

---------

Co-authored-by: Dan Ribbens <dan.ribbens@gmail.com>
This commit is contained in:
Patrik
2024-12-10 11:39:15 -05:00
committed by GitHub
parent fee17448e7
commit 563694d930
7 changed files with 236 additions and 31 deletions

View File

@@ -1,5 +1,5 @@
'use client' 'use client'
import type { ClientCollectionConfig, FormState } from 'payload' import type { ClientCollectionConfig, FieldWithPathClient, FormState } from 'payload'
import { useModal } from '@faceless-ui/modal' import { useModal } from '@faceless-ui/modal'
import { getTranslation } from '@payloadcms/translations' import { getTranslation } from '@payloadcms/translations'
@@ -36,10 +36,25 @@ export type EditManyProps = {
readonly collection: ClientCollectionConfig readonly collection: ClientCollectionConfig
} }
const sanitizeUnselectedFields = (formState: FormState, selected: FieldWithPathClient[]) => {
const filteredData = selected.reduce((acc, field) => {
const foundState = formState?.[field.path]
if (foundState) {
acc[field.path] = formState?.[field.path]?.value
}
return acc
}, {} as FormData)
return filteredData
}
const Submit: React.FC<{ const Submit: React.FC<{
readonly action: string readonly action: string
readonly disabled: boolean readonly disabled: boolean
}> = ({ action, disabled }) => { readonly selected?: FieldWithPathClient[]
}> = ({ action, disabled, selected }) => {
const { submit } = useForm() const { submit } = useForm()
const { t } = useTranslation() const { t } = useTranslation()
@@ -47,9 +62,10 @@ const Submit: React.FC<{
void submit({ void submit({
action, action,
method: 'PATCH', method: 'PATCH',
overrides: (formState) => sanitizeUnselectedFields(formState, selected),
skipValidation: true, skipValidation: true,
}) })
}, [action, submit]) }, [action, submit, selected])
return ( return (
<FormSubmit className={`${baseClass}__save`} disabled={disabled} onClick={save}> <FormSubmit className={`${baseClass}__save`} disabled={disabled} onClick={save}>
@@ -58,7 +74,11 @@ const Submit: React.FC<{
) )
} }
const PublishButton: React.FC<{ action: string; disabled: boolean }> = ({ action, disabled }) => { const PublishButton: React.FC<{
action: string
disabled: boolean
selected?: FieldWithPathClient[]
}> = ({ action, disabled, selected }) => {
const { submit } = useForm() const { submit } = useForm()
const { t } = useTranslation() const { t } = useTranslation()
@@ -66,12 +86,13 @@ const PublishButton: React.FC<{ action: string; disabled: boolean }> = ({ action
void submit({ void submit({
action, action,
method: 'PATCH', method: 'PATCH',
overrides: { overrides: (formState) => ({
...sanitizeUnselectedFields(formState, selected),
_status: 'published', _status: 'published',
}, }),
skipValidation: true, skipValidation: true,
}) })
}, [action, submit]) }, [action, submit, selected])
return ( return (
<FormSubmit className={`${baseClass}__publish`} disabled={disabled} onClick={save}> <FormSubmit className={`${baseClass}__publish`} disabled={disabled} onClick={save}>
@@ -80,7 +101,11 @@ const PublishButton: React.FC<{ action: string; disabled: boolean }> = ({ action
) )
} }
const SaveDraftButton: React.FC<{ action: string; disabled: boolean }> = ({ action, disabled }) => { const SaveDraftButton: React.FC<{
action: string
disabled: boolean
selected?: FieldWithPathClient[]
}> = ({ action, disabled, selected }) => {
const { submit } = useForm() const { submit } = useForm()
const { t } = useTranslation() const { t } = useTranslation()
@@ -88,12 +113,13 @@ const SaveDraftButton: React.FC<{ action: string; disabled: boolean }> = ({ acti
void submit({ void submit({
action, action,
method: 'PATCH', method: 'PATCH',
overrides: { overrides: (formState) => ({
...sanitizeUnselectedFields(formState, selected),
_status: 'draft', _status: 'draft',
}, }),
skipValidation: true, skipValidation: true,
}) })
}, [action, submit]) }, [action, submit, selected])
return ( return (
<FormSubmit <FormSubmit
@@ -125,7 +151,7 @@ export const EditMany: React.FC<EditManyProps> = (props) => {
const { count, getQueryParams, selectAll } = useSelection() const { count, getQueryParams, selectAll } = useSelection()
const { i18n, t } = useTranslation() const { i18n, t } = useTranslation()
const [selected, setSelected] = useState([]) const [selected, setSelected] = useState<FieldWithPathClient[]>([])
const searchParams = useSearchParams() const searchParams = useSearchParams()
const router = useRouter() const router = useRouter()
const [initialState, setInitialState] = useState<FormState>() const [initialState, setInitialState] = useState<FormState>()
@@ -184,7 +210,7 @@ export const EditMany: React.FC<EditManyProps> = (props) => {
return state return state
}, },
[slug, getFormState, collectionPermissions], [getFormState, slug, collectionPermissions],
) )
useEffect(() => { useEffect(() => {
@@ -289,16 +315,19 @@ export const EditMany: React.FC<EditManyProps> = (props) => {
<SaveDraftButton <SaveDraftButton
action={`${serverURL}${apiRoute}/${slug}${queryString}&draft=true`} action={`${serverURL}${apiRoute}/${slug}${queryString}&draft=true`}
disabled={selected.length === 0} disabled={selected.length === 0}
selected={selected}
/> />
<PublishButton <PublishButton
action={`${serverURL}${apiRoute}/${slug}${queryString}&draft=true`} action={`${serverURL}${apiRoute}/${slug}${queryString}&draft=true`}
disabled={selected.length === 0} disabled={selected.length === 0}
selected={selected}
/> />
</React.Fragment> </React.Fragment>
) : ( ) : (
<Submit <Submit
action={`${serverURL}${apiRoute}/${slug}${queryString}`} action={`${serverURL}${apiRoute}/${slug}${queryString}`}
disabled={selected.length === 0} disabled={selected.length === 0}
selected={selected}
/> />
)} )}
</div> </div>

View File

@@ -1,5 +1,5 @@
'use client' 'use client'
import type { ClientField, FieldWithPath, FormState } from 'payload' import type { ClientField, FieldWithPathClient, FormState } from 'payload'
import { fieldAffectsData, fieldHasSubFields, fieldIsHiddenOrDisabled } from 'payload/shared' import { fieldAffectsData, fieldHasSubFields, fieldIsHiddenOrDisabled } from 'payload/shared'
import React, { Fragment, useState } from 'react' import React, { Fragment, useState } from 'react'
@@ -16,7 +16,7 @@ const baseClass = 'field-select'
export type FieldSelectProps = { export type FieldSelectProps = {
readonly fields: ClientField[] readonly fields: ClientField[]
readonly setSelected: (fields: FieldWithPath[]) => void readonly setSelected: (fields: FieldWithPathClient[]) => void
} }
export const combineLabel = ({ export const combineLabel = ({
@@ -56,7 +56,7 @@ const reduceFields = ({
formState?: FormState formState?: FormState
labelPrefix?: React.ReactNode labelPrefix?: React.ReactNode
path?: string path?: string
}): { Label: React.ReactNode; value: FieldWithPath }[] => { }): { Label: React.ReactNode; value: FieldWithPathClient }[] => {
if (!fields) { if (!fields) {
return [] return []
} }

View File

@@ -14,6 +14,7 @@ import React, { useCallback, useEffect, useReducer, useRef, useState } from 'rea
import { toast } from 'sonner' import { toast } from 'sonner'
import type { import type {
CreateFormData,
Context as FormContextType, Context as FormContextType,
FormProps, FormProps,
GetDataByPath, GetDataByPath,
@@ -174,7 +175,7 @@ export const Form: React.FC<FormProps> = (props) => {
const { const {
action: actionArg = action, action: actionArg = action,
method: methodToUse = method, method: methodToUse = method,
overrides = {}, overrides: overridesFromArgs = {},
skipValidation, skipValidation,
} = options } = options
@@ -263,6 +264,14 @@ export const Form: React.FC<FormProps> = (props) => {
return return
} }
let overrides = {}
if (typeof overridesFromArgs === 'function') {
overrides = overridesFromArgs(contextRef.current.fields)
} else if (typeof overridesFromArgs === 'object') {
overrides = overridesFromArgs
}
// If submit handler comes through via props, run that // If submit handler comes through via props, run that
if (onSubmit) { if (onSubmit) {
const serializableFields = deepCopyObjectSimpleWithoutReactComponents( const serializableFields = deepCopyObjectSimpleWithoutReactComponents(
@@ -270,10 +279,8 @@ export const Form: React.FC<FormProps> = (props) => {
) )
const data = reduceFieldsToValues(serializableFields, true) const data = reduceFieldsToValues(serializableFields, true)
if (overrides) { for (const [key, value] of Object.entries(overrides)) {
for (const [key, value] of Object.entries(overrides)) { data[key] = value
data[key] = value
}
} }
onSubmit(serializableFields, data) onSubmit(serializableFields, data)
@@ -288,7 +295,9 @@ export const Form: React.FC<FormProps> = (props) => {
return return
} }
const formData = contextRef.current.createFormData(overrides) const formData = contextRef.current.createFormData(overrides, {
mergeOverrideData: Boolean(typeof overridesFromArgs !== 'function'),
})
try { try {
let res let res
@@ -443,9 +452,8 @@ export const Form: React.FC<FormProps> = (props) => {
[], [],
) )
// eslint-disable-next-line @typescript-eslint/no-explicit-any const createFormData = useCallback<CreateFormData>((overrides, { mergeOverrideData = true }) => {
const createFormData = useCallback((overrides: any = {}) => { let data = reduceFieldsToValues(contextRef.current.fields, true)
const data = reduceFieldsToValues(contextRef.current.fields, true)
const file = data?.file const file = data?.file
@@ -453,13 +461,17 @@ export const Form: React.FC<FormProps> = (props) => {
delete data.file delete data.file
} }
const dataWithOverrides = { if (mergeOverrideData) {
...data, data = {
...overrides, ...data,
...overrides,
}
} else {
data = overrides
} }
const dataToSerialize = { const dataToSerialize = {
_payload: JSON.stringify(dataWithOverrides), _payload: JSON.stringify(data),
file, file,
} }

View File

@@ -60,7 +60,7 @@ export type FormProps = {
export type SubmitOptions = { export type SubmitOptions = {
action?: string action?: string
method?: string method?: string
overrides?: Record<string, unknown> overrides?: ((formState) => FormData) | Record<string, unknown>
skipValidation?: boolean skipValidation?: boolean
} }
@@ -70,7 +70,14 @@ export type Submit = (
e?: React.FormEvent<HTMLFormElement>, e?: React.FormEvent<HTMLFormElement>,
) => Promise<void> ) => Promise<void>
export type ValidateForm = () => Promise<boolean> export type ValidateForm = () => Promise<boolean>
export type CreateFormData = (overrides?: any) => FormData export type CreateFormData = (
overrides?: Record<string, unknown>,
/**
* If mergeOverrideData true, the data will be merged with the existing data in the form state.
* @default true
*/
options?: { mergeOverrideData?: boolean },
) => FormData
export type GetFields = () => FormState export type GetFields = () => FormState
export type GetField = (path: string) => FormField export type GetField = (path: string) => FormField
export type GetData = () => Data export type GetData = () => Data

View File

@@ -103,16 +103,64 @@ export const Posts: CollectionConfig = {
}, },
], ],
}, },
{
name: 'arrayOfFields',
type: 'array',
admin: {
initCollapsed: true,
},
fields: [
{
name: 'optional',
type: 'text',
},
{
name: 'innerArrayOfFields',
type: 'array',
fields: [
{
name: 'innerOptional',
type: 'text',
},
],
},
],
},
{ {
name: 'group', name: 'group',
type: 'group', type: 'group',
fields: [ fields: [
{
name: 'defaultValueField',
type: 'text',
defaultValue: 'testing',
},
{ {
name: 'title', name: 'title',
type: 'text', type: 'text',
}, },
], ],
}, },
{
name: 'someBlock',
type: 'blocks',
blocks: [
{
slug: 'textBlock',
fields: [
{
name: 'textFieldForBlock',
type: 'text',
},
],
},
],
},
{
name: 'defaultValueField',
type: 'text',
defaultValue: 'testing',
},
{ {
name: 'relationship', name: 'relationship',
type: 'relationship', type: 'relationship',

View File

@@ -516,6 +516,68 @@ describe('admin3', () => {
await expect(page.locator('.row-3 .cell-title')).toContainText(updatedPostTitle) await expect(page.locator('.row-3 .cell-title')).toContainText(updatedPostTitle)
}) })
test('should not override un-edited values in bulk edit if it has a defaultValue', async () => {
await deleteAllPosts()
const post1Title = 'Post'
const postData = {
title: 'Post',
arrayOfFields: [
{
optional: 'some optional array field',
innerArrayOfFields: [
{
innerOptional: 'some inner optional array field',
},
],
},
],
group: {
defaultValueField: 'not the group default value',
title: 'some title',
},
someBlock: [
{
textFieldForBlock: 'some text for block text',
blockType: 'textBlock',
},
],
defaultValueField: 'not the default value',
}
const updatedPostTitle = `${post1Title} (Updated)`
await Promise.all([createPost(postData)])
await page.goto(postsUrl.list)
await page.locator('input#select-all').check()
await page.locator('.edit-many__toggle').click()
await page.locator('.field-select .rs__control').click()
const titleOption = page.locator('.field-select .rs__option', {
hasText: exactText('Title'),
})
await expect(titleOption).toBeVisible()
await titleOption.click()
const titleInput = page.locator('#field-title')
await expect(titleInput).toBeVisible()
await titleInput.fill(updatedPostTitle)
await page.locator('.form-submit button[type="submit"].edit-many__publish').click()
await expect(page.locator('.payload-toast-container .toast-success')).toContainText(
'Updated 1 Post successfully.',
)
const updatedPost = await payload.find({
collection: 'posts',
limit: 1,
})
expect(updatedPost.docs[0].title).toBe(updatedPostTitle)
expect(updatedPost.docs[0].arrayOfFields.length).toBe(1)
expect(updatedPost.docs[0].arrayOfFields[0].optional).toBe('some optional array field')
expect(updatedPost.docs[0].arrayOfFields[0].innerArrayOfFields.length).toBe(1)
expect(updatedPost.docs[0].someBlock[0].textFieldForBlock).toBe('some text for block text')
expect(updatedPost.docs[0].defaultValueField).toBe('not the default value')
})
test('should bulk update with filters and across pages', async () => { test('should bulk update with filters and across pages', async () => {
// First, delete all posts created by the seed // First, delete all posts created by the seed
await deleteAllPosts() await deleteAllPosts()

View File

@@ -138,9 +138,31 @@ export interface Post {
[k: string]: unknown; [k: string]: unknown;
}[] }[]
| null; | null;
arrayOfFields?:
| {
optional?: string | null;
innerArrayOfFields?:
| {
innerOptional?: string | null;
id?: string | null;
}[]
| null;
id?: string | null;
}[]
| null;
group?: { group?: {
defaultValueField?: string | null;
title?: string | null; title?: string | null;
}; };
someBlock?:
| {
textFieldForBlock?: string | null;
id?: string | null;
blockName?: string | null;
blockType: 'textBlock';
}[]
| null;
defaultValueField?: string | null;
relationship?: (string | null) | Post; relationship?: (string | null) | Post;
customCell?: string | null; customCell?: string | null;
sidebarField?: string | null; sidebarField?: string | null;
@@ -484,11 +506,36 @@ export interface PostsSelect<T extends boolean = true> {
description?: T; description?: T;
number?: T; number?: T;
richText?: T; richText?: T;
arrayOfFields?:
| T
| {
optional?: T;
innerArrayOfFields?:
| T
| {
innerOptional?: T;
id?: T;
};
id?: T;
};
group?: group?:
| T | T
| { | {
defaultValueField?: T;
title?: T; title?: T;
}; };
someBlock?:
| T
| {
textBlock?:
| T
| {
textFieldForBlock?: T;
id?: T;
blockName?: T;
};
};
defaultValueField?: T;
relationship?: T; relationship?: T;
customCell?: T; customCell?: T;
sidebarField?: T; sidebarField?: T;