fix(ui): refreshes column state during hmr and respects admin.disableListColumn despite preferences (#9846)

Partial fix for #9774. When `admin.disableListColumn` is set
retroactively, it continues to appear in column state, but shouldn't.
This was because the table column context was not refreshing after HMR
runs, and would instead hold onto these stale columns until the page
itself refreshes. Similarly, this was also a problem when the user had
saved any of these columns to their list preferences, where those prefs
would take precedence despite these properties being set on the
underlying fields. The fix is to filter these columns from all requests
that send them, and ensure local component state properly refreshes
itself.
This commit is contained in:
Jacob Fletcher
2024-12-10 15:11:44 -05:00
committed by GitHub
parent 563694d930
commit f7172b5b2c
19 changed files with 194 additions and 120 deletions

View File

@@ -171,13 +171,13 @@ export const renderListView = async (
const clientCollectionConfig = clientConfig.collections.find((c) => c.slug === collectionSlug)
const { columnState, Table } = renderTable({
collectionConfig: clientCollectionConfig,
clientCollectionConfig,
collectionConfig,
columnPreferences: listPreferences?.columns,
customCellProps,
docs: data.docs,
drawerSlug,
enableRowSelections,
fields,
i18n: req.i18n,
payload,
useAsTitle,

View File

@@ -57,6 +57,8 @@ export {
} from '../utilities/deepMerge.js'
export { fieldSchemaToJSON } from '../utilities/fieldSchemaToJSON.js'
export { flattenAllFields } from '../utilities/flattenAllFields.js'
export { default as flattenTopLevelFields } from '../utilities/flattenTopLevelFields.js'
export { getDataByPath } from '../utilities/getDataByPath.js'
export { getSelectMode } from '../utilities/getSelectMode.js'

View File

@@ -33,18 +33,14 @@ function flattenFields<TField extends ClientField | Field>(
fields: TField[],
keepPresentationalFields?: boolean,
): FlattenedField<TField>[] {
return fields.reduce<FlattenedField<TField>[]>((fieldsToUse, field) => {
return fields.reduce<FlattenedField<TField>[]>((acc, field) => {
if (fieldAffectsData(field) || (keepPresentationalFields && fieldIsPresentationalOnly(field))) {
return [...fieldsToUse, field as FlattenedField<TField>]
}
if (fieldHasSubFields(field)) {
return [...fieldsToUse, ...flattenFields(field.fields as TField[], keepPresentationalFields)]
}
if (field.type === 'tabs' && 'tabs' in field) {
acc.push(field as FlattenedField<TField>)
} else if (fieldHasSubFields(field)) {
acc.push(...flattenFields(field.fields as TField[], keepPresentationalFields))
} else if (field.type === 'tabs' && 'tabs' in field) {
return [
...fieldsToUse,
...acc,
...field.tabs.reduce<FlattenedField<TField>[]>((tabFields, tab: TabType<TField>) => {
if (tabHasName(tab)) {
return [...tabFields, { ...tab, type: 'tab' } as unknown as FlattenedField<TField>]
@@ -58,7 +54,7 @@ function flattenFields<TField extends ClientField | Field>(
]
}
return fieldsToUse
return acc
}, [])
}

View File

@@ -1,16 +1,15 @@
'use client'
import type { ClientField } from 'payload'
import { fieldAffectsData } from 'payload/shared'
import { flattenFieldMap } from '../../utilities/flattenFieldMap.js'
import { fieldAffectsData, flattenTopLevelFields } from 'payload/shared'
export const getTextFieldsToBeSearched = (
listSearchableFields: string[],
fields: ClientField[],
): ClientField[] => {
if (listSearchableFields) {
const flattenedFields = flattenFieldMap(fields)
const flattenedFields = flattenTopLevelFields(fields) as ClientField[]
return flattenedFields.filter(
(field) => fieldAffectsData(field) && listSearchableFields.includes(field.name),
)

View File

@@ -1,6 +1,7 @@
import type { I18nClient } from '@payloadcms/translations'
import type {
ClientCollectionConfig,
ClientField,
DefaultCellComponentProps,
DefaultServerCellComponentProps,
Field,
@@ -16,6 +17,7 @@ import {
fieldIsHiddenOrDisabled,
fieldIsID,
fieldIsPresentationalOnly,
flattenTopLevelFields,
} from 'payload/shared'
import React from 'react'
@@ -31,19 +33,19 @@ import {
SortColumn,
// eslint-disable-next-line payload/no-imports-from-exports-dir
} from '../../exports/client/index.js'
import { flattenFieldMap } from '../../utilities/flattenFieldMap.js'
import { RenderServerComponent } from '../RenderServerComponent/index.js'
import { filterFields } from './filterFields.js'
type Args = {
beforeRows?: Column[]
collectionConfig: ClientCollectionConfig
clientCollectionConfig: ClientCollectionConfig
collectionConfig: SanitizedCollectionConfig
columnPreferences: ColumnPreferences
columns?: ColumnPreferences
customCellProps: DefaultCellComponentProps['customCellProps']
docs: PaginatedDocs['docs']
enableRowSelections: boolean
enableRowTypes?: boolean
fields: Field[]
i18n: I18nClient
payload: Payload
sortColumnProps?: Partial<SortColumnProps>
@@ -53,24 +55,29 @@ type Args = {
export const buildColumnState = (args: Args): Column[] => {
const {
beforeRows,
clientCollectionConfig,
collectionConfig,
columnPreferences,
columns,
customCellProps,
docs,
enableRowSelections,
fields,
i18n,
payload,
sortColumnProps,
useAsTitle,
} = args
const clientFields = collectionConfig.fields
// clientFields contains the fake `id` column
let sortedFieldMap = flattenFieldMap(clientFields)
let _sortedFieldMap = flattenFieldMap(fields) // TODO: think of a way to avoid this additional flatten
let sortedFieldMap = flattenTopLevelFields(
filterFields(clientCollectionConfig.fields),
true,
) as ClientField[]
let _sortedFieldMap = flattenTopLevelFields(
filterFields(collectionConfig.fields),
true,
) as Field[] // TODO: think of a way to avoid this additional flatten
// place the `ID` field first, if it exists
// do the same for the `useAsTitle` field with precedence over the `ID` field
@@ -180,7 +187,7 @@ export const buildColumnState = (args: Args): Column[] => {
const baseCellClientProps: DefaultCellComponentProps = {
cellData: undefined,
collectionConfig: deepCopyObjectSimple(collectionConfig),
collectionConfig: deepCopyObjectSimple(clientCollectionConfig),
customCellProps,
field,
rowData: undefined,

View File

@@ -1,17 +1,22 @@
import type { ClientField, Field } from 'payload'
// 1. Skips fields that are hidden, disabled, or presentational-only (i.e. `ui` fields)
// 2. Maps through top-level `tabs` fields and filters out the same
import { fieldIsHiddenOrDisabled, fieldIsID } from 'payload/shared'
/**
* Filters fields that are hidden, disabled, or have `disableListColumn` set to `true`
* Does so recursively for `tabs` fields.
*/
export const filterFields = <T extends ClientField | Field>(incomingFields: T[]): T[] => {
const shouldSkipField = (field: T): boolean =>
(field.type !== 'ui' && field.admin?.disabled === true) ||
(field.type !== 'ui' && fieldIsHiddenOrDisabled(field) && !fieldIsID(field)) ||
field?.admin?.disableListColumn === true
const fields: T[] = incomingFields?.reduce((formatted, field) => {
const fields: T[] = incomingFields?.reduce((acc, field) => {
if (shouldSkipField(field)) {
return formatted
return acc
}
// extract top-level `tabs` fields and filter out the same
const formattedField: T =
field.type === 'tabs' && 'tabs' in field
? {
@@ -23,7 +28,9 @@ export const filterFields = <T extends ClientField | Field>(incomingFields: T[])
}
: field
return [...formatted, formattedField]
acc.push(formattedField)
return acc
}, [])
return fields

View File

@@ -1,4 +1,4 @@
import type { ClientField, Field } from 'payload'
import type { ClientField, CollectionConfig, Field } from 'payload'
import { fieldAffectsData } from 'payload/shared'
@@ -33,10 +33,15 @@ const getRemainingColumns = <T extends ClientField[] | Field[]>(
return [...remaining, field.name]
}, [])
/**
* Returns the initial columns to display in the table based on the following criteria:
* 1. If `defaultColumns` is set in the collection config, use those columns
* 2. Otherwise take `useAtTitle, if set, and the next 3 fields that are not hidden or disabled
*/
export const getInitialColumns = <T extends ClientField[] | Field[]>(
fields: T,
useAsTitle: string,
defaultColumns: string[],
useAsTitle: CollectionConfig['admin']['useAsTitle'],
defaultColumns: CollectionConfig['admin']['defaultColumns'],
): ColumnPreferences => {
let initialColumns = []

View File

@@ -1,7 +1,7 @@
'use client'
import type { ClientCollectionConfig, SanitizedCollectionConfig } from 'payload'
import React, { createContext, useCallback, useContext } from 'react'
import React, { createContext, useCallback, useContext, useEffect } from 'react'
import type { ColumnPreferences } from '../../providers/ListQuery/index.js'
import type { SortColumnProps } from '../SortColumn/index.js'
@@ -204,6 +204,7 @@ export const TableColumnsProvider: React.FC<Props> = ({
return indexOfFirst > indexOfSecond ? 1 : -1
})
const { state: columnState, Table } = await getTableState({
collectionSlug,
columns: activeColumns,
@@ -275,7 +276,11 @@ export const TableColumnsProvider: React.FC<Props> = ({
sortColumnProps,
])
React.useEffect(() => {
useEffect(() => {
setTableColumns(columnState)
}, [columnState])
useEffect(() => {
return () => {
abortAndIgnore(tableStateControllerRef.current)
}

View File

@@ -1,7 +1,7 @@
'use client'
import type { ClientCollectionConfig, ClientField } from 'payload'
import { flattenFieldMap } from '../utilities/flattenFieldMap.js'
import { flattenTopLevelFields } from 'payload/shared'
export const useUseTitleField = (collection: ClientCollectionConfig): ClientField => {
const {
@@ -9,6 +9,7 @@ export const useUseTitleField = (collection: ClientCollectionConfig): ClientFiel
fields,
} = collection
const topLevelFields = flattenFieldMap(fields)
const topLevelFields = flattenTopLevelFields(fields) as ClientField[]
return topLevelFields?.find((field) => 'name' in field && field.name === useAsTitle)
}

View File

@@ -198,8 +198,6 @@ export const buildTableState = async (
}
}
const fields = collectionConfig.fields
let docs = docsFromArgs
let data: PaginatedDocs
@@ -219,12 +217,12 @@ export const buildTableState = async (
}
const { columnState, Table } = renderTable({
collectionConfig: clientCollectionConfig,
clientCollectionConfig,
collectionConfig,
columnPreferences: undefined, // TODO, might not be needed
columns,
docs,
enableRowSelections,
fields,
i18n: req.i18n,
payload,
renderRowTypes,
@@ -232,7 +230,7 @@ export const buildTableState = async (
useAsTitle: collectionConfig.admin.useAsTitle,
})
const renderedFilters = renderFilters(fields, req.payload.importMap)
const renderedFilters = renderFilters(collectionConfig.fields, req.payload.importMap)
return {
data,

View File

@@ -1,38 +0,0 @@
import type { ClientField, Field } from 'payload'
import { fieldIsPresentationalOnly } from 'payload/shared'
/**
* Flattens a collection's fields into a single array of fields, as long
* as the fields do not affect data.
*
* @param fields
* @param keepPresentationalFields if true, will skip flattening fields that are presentational only
*/
export const flattenFieldMap = <T extends ClientField | Field>(
fields: T[],
keepPresentationalFields?: boolean,
): T[] => {
return fields?.reduce((acc, field) => {
if ('name' in field || (keepPresentationalFields && fieldIsPresentationalOnly(field))) {
acc.push(field)
return acc
} else if ('fields' in field) {
acc.push(...flattenFieldMap(field.fields as T[], keepPresentationalFields))
} else if (field.type === 'tabs' && 'tabs' in field && Array.isArray(field.tabs)) {
return [
...acc,
...field.tabs.reduce((tabAcc, tab) => {
return [
...tabAcc,
...('name' in tab
? [{ ...tab }]
: flattenFieldMap(tab.fields as T[], keepPresentationalFields)),
]
}, []),
]
}
return acc
}, [])
}

View File

@@ -1,14 +1,14 @@
import type {
ClientCollectionConfig,
CollectionConfig,
Field,
ImportMap,
PaginatedDocs,
Payload,
} from 'payload'
import { getTranslation, type I18nClient } from '@payloadcms/translations'
import { fieldIsHiddenOrDisabled, fieldIsID } from 'payload/shared'
import {
type ClientCollectionConfig,
type CollectionConfig,
type Field,
type ImportMap,
type PaginatedDocs,
type Payload,
type SanitizedCollectionConfig,
} from 'payload'
import { fieldIsHiddenOrDisabled, flattenTopLevelFields } from 'payload/shared'
// eslint-disable-next-line payload/no-imports-from-exports-dir
import type { Column } from '../exports/client/index.js'
@@ -18,7 +18,6 @@ import { RenderServerComponent } from '../elements/RenderServerComponent/index.j
import { buildColumnState } from '../elements/TableColumns/buildColumnState.js'
import { filterFields } from '../elements/TableColumns/filterFields.js'
import { getInitialColumns } from '../elements/TableColumns/getInitialColumns.js'
// eslint-disable-next-line payload/no-imports-from-exports-dir
import { Pill, Table } from '../exports/client/index.js'
@@ -48,27 +47,27 @@ export const renderFilters = (
)
export const renderTable = ({
clientCollectionConfig,
collectionConfig,
columnPreferences,
columns: columnsFromArgs,
customCellProps,
docs,
enableRowSelections,
fields,
i18n,
payload,
renderRowTypes,
tableAppearance,
useAsTitle,
}: {
collectionConfig: ClientCollectionConfig
clientCollectionConfig: ClientCollectionConfig
collectionConfig: SanitizedCollectionConfig
columnPreferences: ColumnPreferences
columns?: ColumnPreferences
customCellProps?: Record<string, any>
docs: PaginatedDocs['docs']
drawerSlug?: string
enableRowSelections: boolean
fields: Field[]
i18n: I18nClient
payload: Payload
renderRowTypes?: boolean
@@ -78,9 +77,18 @@ export const renderTable = ({
columnState: Column[]
Table: React.ReactNode
} => {
const columns =
columnsFromArgs ||
getInitialColumns(filterFields(fields), useAsTitle, collectionConfig?.admin?.defaultColumns)
// Ensure that columns passed as args comply with the field config, i.e. `hidden`, `disableListColumn`, etc.
const columns = columnsFromArgs
? columnsFromArgs?.filter((column) =>
flattenTopLevelFields(clientCollectionConfig.fields, true)?.some(
(field) => 'name' in field && field.name === column.accessor,
),
)
: getInitialColumns(
filterFields(clientCollectionConfig.fields),
useAsTitle,
clientCollectionConfig?.admin?.defaultColumns,
)
const columnState = buildColumnState({
beforeRows: renderRowTypes
@@ -91,16 +99,16 @@ export const renderTable = ({
field: null,
Heading: i18n.t('version:type'),
renderedCells: docs.map((_, i) => (
<Pill key={i}>{getTranslation(collectionConfig.labels.singular, i18n)}</Pill>
<Pill key={i}>{getTranslation(clientCollectionConfig.labels.singular, i18n)}</Pill>
)),
},
]
: undefined,
clientCollectionConfig,
collectionConfig,
columnPreferences,
columns,
enableRowSelections,
fields,
i18n,
// sortColumnProps,
customCellProps,

View File

@@ -27,7 +27,7 @@ const description = 'Description'
let payload: PayloadTestSDK<Config>
import { goToFirstCell, navigateToDoc } from 'helpers/e2e/navigateToDoc.js'
import { toggleColumn } from 'helpers/e2e/toggleColumn.js'
import { openListColumns, toggleColumn } from 'helpers/e2e/toggleColumn.js'
import path from 'path'
import { wait } from 'payload/shared'
import { fileURLToPath } from 'url'
@@ -206,21 +206,16 @@ describe('admin2', () => {
test('should toggle columns', async () => {
const columnCountLocator = 'table > thead > tr > th'
await createPost()
await page.locator('.list-controls__toggle-columns').click()
await openListColumns(page, {})
const numberOfColumns = await page.locator(columnCountLocator).count()
await expect(page.locator('.column-selector')).toBeVisible()
await expect(page.locator('table > thead > tr > th:nth-child(2)')).toHaveText('ID')
const idButton = page.locator(`.column-selector .column-selector__column`, {
hasText: exactText('ID'),
})
await idButton.click()
await toggleColumn(page, { columnLabel: 'ID', targetState: 'off' })
await page.locator('#heading-id').waitFor({ state: 'detached' })
await page.locator('.cell-id').first().waitFor({ state: 'detached' })
await expect(page.locator(columnCountLocator)).toHaveCount(numberOfColumns - 1)
await expect(page.locator('table > thead > tr > th:nth-child(2)')).toHaveText('Number')
await idButton.click()
await toggleColumn(page, { columnLabel: 'ID', targetState: 'on' })
await expect(page.locator('.cell-id').first()).toBeVisible()
await expect(page.locator(columnCountLocator)).toHaveCount(numberOfColumns)
await expect(page.locator('table > thead > tr > th:nth-child(2)')).toHaveText('ID')
@@ -744,8 +739,7 @@ describe('admin2', () => {
test('should sort with existing filters', async () => {
await page.goto(postsUrl.list)
const column = await toggleColumn(page, { columnLabel: 'ID' })
await expect(column).not.toHaveClass('column-selector__column--active')
await toggleColumn(page, { columnLabel: 'ID', targetState: 'off' })
await page.locator('#heading-id').waitFor({ state: 'detached' })
await page.locator('#heading-title button.sort-column__asc').click()
await page.waitForURL(/sort=title/)

View File

@@ -1,7 +1,9 @@
import type { Page } from '@playwright/test'
import type { GeneratedTypes } from 'helpers/sdk/types.js'
import { expect, test } from '@playwright/test'
import { openListColumns, toggleColumn } from 'helpers/e2e/toggleColumn.js'
import { upsertPrefs } from 'helpers/e2e/upsertPrefs.js'
import path from 'path'
import { wait } from 'payload/shared'
import { fileURLToPath } from 'url'
@@ -203,6 +205,32 @@ describe('Text', () => {
).toBeVisible()
})
test('should respect admin.disableListColumn despite preferences', async () => {
await upsertPrefs<Config, GeneratedTypes<any>>({
payload,
user: client.user,
value: {
columns: [
{
accessor: 'disableListColumnText',
active: true,
},
],
},
})
await page.goto(url.list)
await openListColumns(page, {})
await expect(
page.locator(`.column-selector .column-selector__column`, {
hasText: exactText('Disable List Column Text'),
}),
).toBeHidden()
await expect(page.locator('#heading-disableListColumnText')).toBeHidden()
await expect(page.locator('table .row-1 .cell-disableListColumnText')).toBeHidden()
})
test('should hide field in filter when admin.disableListFilter is true', async () => {
await page.goto(url.list)
await page.locator('.list-controls__toggle-where').click()

View File

@@ -156,14 +156,12 @@ const TextFields: CollectionConfig = {
type: 'text',
admin: {
disableListColumn: true,
disableListFilter: false,
},
},
{
name: 'disableListFilterText',
type: 'text',
admin: {
disableListColumn: false,
disableListFilter: true,
},
},

View File

@@ -1,7 +1,6 @@
import type { Page } from '@playwright/test'
import { expect } from '@playwright/test'
import { wait } from 'payload/shared'
import { exactText } from '../../helpers.js'

View File

@@ -0,0 +1,59 @@
import type { PayloadTestSDK } from 'helpers/sdk/index.js'
import type { GeneratedTypes } from 'helpers/sdk/types.js'
import type { TypedUser } from 'payload'
export const upsertPrefs = async <
TConfig extends GeneratedTypes<any>,
TGeneratedTypes extends GeneratedTypes<any>,
>({
payload,
user,
value,
}: {
payload: PayloadTestSDK<TConfig>
user: TypedUser
value: Record<string, any>
}): Promise<TGeneratedTypes['collections']['payload-preferences']> => {
let prefs = await payload
.find({
collection: 'payload-preferences',
depth: 0,
limit: 1,
where: {
and: [
{ key: { equals: 'text-fields-list' } },
{ 'user.value': { equals: user.id } },
{ 'user.relationTo': { equals: user.collection } },
],
},
})
?.then((res) => res.docs?.[0])
if (!prefs) {
prefs = await payload.create({
collection: 'payload-preferences',
depth: 0,
data: {
key: 'text-fields-list',
user: {
relationTo: user.collection,
value: user.id,
},
value,
},
})
} else {
prefs = await payload.update({
collection: 'payload-preferences',
id: prefs.id,
data: {
value: {
...(prefs?.value ?? {}),
...value,
},
},
})
}
return prefs
}

View File

@@ -1,7 +1,5 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import type { Where } from 'payload'
import type { Config } from 'payload'
import type { PaginatedDocs } from 'payload'
import type { Config, PaginatedDocs, TypedUser, Where } from 'payload'
import * as qs from 'qs-esm'
@@ -120,6 +118,8 @@ export class RESTClient {
serverURL: string
public user: TypedUser
constructor(config: Config, args: Args) {
this.config = config
this.serverURL = args.serverURL
@@ -254,7 +254,9 @@ export class RESTClient {
const response = await fetch(`${this.serverURL}/api/${slug}${whereQuery}`, options)
const { status } = response
const result = await response.json()
if (result.errors) throw new Error(result.errors[0].message)
if (result.errors) {
throw new Error(result.errors[0].message)
}
return { result, status }
}
@@ -310,7 +312,9 @@ export class RESTClient {
method: 'POST',
})
let { token } = await response.json()
const { user } = await response.json()
let token = user.token
// If the token is not in the response body, then we can extract it from the cookies
if (!token) {
@@ -319,6 +323,7 @@ export class RESTClient {
token = tokenMatchResult?.groups?.token
}
this.user = user
this.token = token
return token

View File

@@ -7,6 +7,7 @@ export const handler: PayloadHandler = async (req) => {
await addDataAndFileToRequest(req)
const { data, payload, user } = req
const operation = data?.operation ? String(data.operation) : undefined
if (data?.operation && typeof payload[operation] === 'function') {