fix(ui): sort resets columns (#10402)

Fixes #10018. When toggling columns, then sorting them, the table is
reset to the collection's default columns instead of the user's
preferred columns. This is because when sorting columns, a stale
client-side cache of the user's preferences is used to update their sort
preference. This is because when column state is constructed
server-side, it completely bypasses the client-side cache. To fix this,
sort preferences are now also set on the server right alongside column
preferences, which performs an upsert-like operation to ensure that no
existing preferences are lost.

packages/next/src/views/List/index.tsx

@@ -8,7 +8,7 @@ import type { AdminViewProps, ListQuery, Where } from 'payload'
 
 import { DefaultListView, HydrateAuthProvider, ListQueryProvider } from '@payloadcms/ui'
 import { RenderServerComponent } from '@payloadcms/ui/elements/RenderServerComponent'
-import { renderFilters, renderTable } from '@payloadcms/ui/rsc'
+import { renderFilters, renderTable, upsertPreferences } from '@payloadcms/ui/rsc'
 import { formatAdminURL, mergeListSearchAndWhere } from '@payloadcms/ui/shared'
 import { notFound } from 'next/navigation.js'
 import { isNumber } from 'payload/shared'
@@ -48,12 +48,7 @@ export const renderListView = async (
 
   const {
     collectionConfig,
-    collectionConfig: {
-      slug: collectionSlug,
-      admin: { useAsTitle },
-      defaultSort,
-      fields,
-    },
+    collectionConfig: { slug: collectionSlug },
     locale: fullLocale,
     permissions,
     req,
@@ -74,39 +69,13 @@ export const renderListView = async (
 
   const query = queryFromArgs || queryFromReq
 
-  let listPreferences: ListPreferences
   const preferenceKey = `${collectionSlug}-list`
 
-  try {
-    listPreferences = (await payload
-      .find({
-        collection: 'payload-preferences',
-        depth: 0,
-        limit: 1,
-        req,
-        user,
-        where: {
-          and: [
-            {
-              key: {
-                equals: preferenceKey,
-              },
-            },
-            {
-              'user.relationTo': {
-                equals: user.collection,
-              },
-            },
-            {
-              'user.value': {
-                equals: user?.id,
-              },
-            },
-          ],
-        },
-      })
-      ?.then((res) => res?.docs?.[0]?.value)) as ListPreferences
-  } catch (_err) {} // eslint-disable-line no-empty
+  const listPreferences = await upsertPreferences<ListPreferences>({
+    key: `${collectionSlug}-list`,
+    req,
+    value: { limit: isNumber(query?.limit) ? Number(query.limit) : undefined, sort: query?.sort },
+  })
 
   const {
     routes: { admin: adminRoute },
@@ -119,24 +88,18 @@ export const renderListView = async (
 
     const page = isNumber(query?.page) ? Number(query.page) : 0
 
-    let whereQuery = mergeListSearchAndWhere({
+    const limit = listPreferences?.limit || collectionConfig.admin.pagination.defaultLimit
+
+    const sort =
+      listPreferences?.sort ||
+      (typeof collectionConfig.defaultSort === 'string' ? collectionConfig.defaultSort : undefined)
+
+    let where = mergeListSearchAndWhere({
       collectionConfig,
       search: typeof query?.search === 'string' ? query.search : undefined,
       where: (query?.where as Where) || undefined,
     })
 
-    const limit = isNumber(query?.limit)
-      ? Number(query.limit)
-      : listPreferences?.limit || collectionConfig.admin.pagination.defaultLimit
-
-    const sort =
-      query?.sort && typeof query.sort === 'string'
-        ? query.sort
-        : listPreferences?.sort ||
-          (typeof collectionConfig.defaultSort === 'string'
-            ? collectionConfig.defaultSort
-            : undefined)
-
     if (typeof collectionConfig.admin?.baseListFilter === 'function') {
       const baseListFilter = await collectionConfig.admin.baseListFilter({
         limit,
@@ -146,8 +109,8 @@ export const renderListView = async (
       })
 
       if (baseListFilter) {
-        whereQuery = {
-          and: [whereQuery, baseListFilter].filter(Boolean),
+        where = {
+          and: [where, baseListFilter].filter(Boolean),
         }
       }
     }
@@ -165,7 +128,7 @@ export const renderListView = async (
       req,
       sort,
       user,
-      where: whereQuery || {},
+      where: where || {},
     })
 
     const clientCollectionConfig = clientConfig.collections.find((c) => c.slug === collectionSlug)
@@ -180,10 +143,10 @@ export const renderListView = async (
       enableRowSelections,
       i18n: req.i18n,
       payload,
-      useAsTitle,
+      useAsTitle: collectionConfig.admin.useAsTitle,
     })
 
-    const renderedFilters = renderFilters(fields, req.payload.importMap)
+    const renderedFilters = renderFilters(collectionConfig.fields, req.payload.importMap)
 
     const staticDescription =
       typeof collectionConfig.admin.description === 'function'

packages/next/src/views/List/renderListViewSlots.tsx

@@ -14,6 +14,7 @@ type Args = {
   payload: Payload
   serverProps: ListComponentServerProps
 }
+
 export const renderListViewSlots = ({
   clientProps,
   collectionConfig,

packages/ui/src/elements/TableColumns/buildColumnState.tsx

@@ -28,8 +28,6 @@ import type { Column } from '../Table/index.js'
 import {
   RenderCustomComponent,
   RenderDefaultCell,
-  SelectAll,
-  SelectRow,
   SortColumn,
   // eslint-disable-next-line payload/no-imports-from-exports-dir
 } from '../../exports/client/index.js'

packages/ui/src/exports/rsc/index.ts

@@ -1,2 +1,3 @@
 export { copyDataFromLocaleHandler } from '../../utilities/copyDataFromLocale.js'
 export { renderFilters, renderTable } from '../../utilities/renderTable.js'
+export { upsertPreferences } from '../../utilities/upsertPreferences.js'

packages/ui/src/providers/ListQuery/index.tsx

@@ -81,6 +81,7 @@ export const ListQueryProvider: React.FC<ListQueryProps> = ({
   }, [searchParams, modifySearchParams])
 
   const refineListData = useCallback(
+    // eslint-disable-next-line @typescript-eslint/require-await
     async (query: ListQuery) => {
       let page = 'page' in query ? query.page : currentQuery?.page
 
@@ -88,23 +89,6 @@ export const ListQueryProvider: React.FC<ListQueryProps> = ({
         page = '1'
       }
 
-      const updatedPreferences: Record<string, unknown> = {}
-      let updatePreferences = false
-
-      if ('limit' in query) {
-        updatedPreferences.limit = Number(query.limit)
-        updatePreferences = true
-      }
-
-      if ('sort' in query) {
-        updatedPreferences.sort = query.sort
-        updatePreferences = true
-      }
-
-      if (updatePreferences && preferenceKey) {
-        await setPreference(preferenceKey, updatedPreferences, true)
-      }
-
       const newQuery: ListQuery = {
         limit: 'limit' in query ? query.limit : (currentQuery?.limit ?? String(defaultLimit)),
         page,
@@ -131,13 +115,11 @@ export const ListQueryProvider: React.FC<ListQueryProps> = ({
       currentQuery?.search,
       currentQuery?.sort,
       currentQuery?.where,
-      preferenceKey,
       defaultLimit,
       defaultSort,
       modifySearchParams,
       onQueryChange,
       onQueryChangeFromProps,
-      setPreference,
       router,
     ],
   )

packages/ui/src/providers/Preferences/index.tsx

@@ -116,6 +116,7 @@ export const PreferencesProvider: React.FC<{ children?: React.ReactNode }> = ({
         if (newValue === currentPreference) {
           return
         }
+
         pendingUpdate.current[key] = newValue
       }
 

packages/ui/src/utilities/buildTableState.ts

@@ -7,14 +7,15 @@ import type {
   SanitizedCollectionConfig,
 } from 'payload'
 
-import { dequal } from 'dequal/lite'
 import { formatErrors } from 'payload'
+import { isNumber } from 'payload/shared'
 
 import type { Column } from '../elements/Table/index.js'
 import type { ListPreferences } from '../elements/TableColumns/index.js'
 
 import { getClientConfig } from './getClientConfig.js'
 import { renderFilters, renderTable } from './renderTable.js'
+import { upsertPreferences } from './upsertPreferences.js'
 
 type BuildTableStateSuccessResult = {
   clientConfig?: ClientConfig
@@ -135,68 +136,15 @@ export const buildTableState = async (
     )
   }
 
-  // get prefs, then set update them using the columns that we just received
-  const preferencesKey = `${collectionSlug}-list`
-
-  const preferencesResult = await payload
-    .find({
-      collection: 'payload-preferences',
-      depth: 0,
-      limit: 1,
-      pagination: false,
-      where: {
-        and: [
-          {
-            key: {
-              equals: preferencesKey,
-            },
-          },
-          {
-            'user.relationTo': {
-              equals: user.collection,
-            },
-          },
-          {
-            'user.value': {
-              equals: user.id,
-            },
-          },
-        ],
-      },
-    })
-    .then((res) => res.docs[0] ?? { id: null, value: {} })
-
-  let newPrefs = preferencesResult.value
-
-  if (!preferencesResult.id || !dequal(columns, preferencesResult?.columns)) {
-    const preferencesArgs = {
-      collection: 'payload-preferences',
-      data: {
-        key: preferencesKey,
-        user: {
-          collection: user.collection,
-          value: user.id,
-        },
-        value: {
-          ...(preferencesResult?.value || {}),
-          columns,
-        },
-      },
-      depth: 0,
-      req,
-    }
-
-    if (preferencesResult.id) {
-      newPrefs = await payload
-        .update({
-          ...preferencesArgs,
-          id: preferencesResult.id,
-        })
-        ?.then((res) => res.value as ListPreferences)
-    } else {
-      newPrefs = await payload.create(preferencesArgs)?.then((res) => res.value as ListPreferences)
-    }
-  }
+  const listPreferences = await upsertPreferences<ListPreferences>({
+    key: `${collectionSlug}-list`,
+    req,
+    value: {
+      columns,
+      limit: isNumber(query?.limit) ? Number(query.limit) : undefined,
+      sort: query?.sort,
+    },
+  })
 
   let docs = docsFromArgs
   let data: PaginatedDocs
@@ -236,7 +184,7 @@ export const buildTableState = async (
 
   return {
     data,
-    preferences: newPrefs,
+    preferences: listPreferences,
     renderedFilters,
     state: columnState,
     Table,

packages/ui/src/utilities/upsertPreferences.ts

@@ -0,0 +1,95 @@
+import type { PayloadRequest } from 'payload'
+
+import { dequal } from 'dequal/lite'
+
+import { removeUndefined } from './removeUndefined.js'
+
+/**
+ * Will update the given preferences by key, creating a new record if it doesn't already exist, or merging existing preferences with the new value.
+ * This is not possible to do with the existing `db.upsert` operation because it stores on the `value` key and does not perform a deep merge beyond the first level.
+ * I.e. if you have a preferences record with a `value` key, `db.upsert` will overwrite the existing value. In the future if this supported we should use that instead.
+ * @param req - The PayloadRequest object
+ * @param key - The key of the preferences to update
+ * @param value - The new value to merge with the existing preferences
+ */
+export const upsertPreferences = async <T extends Record<string, any>>({
+  key,
+  req,
+  value: incomingValue,
+}: {
+  key: string
+  req: PayloadRequest
+  value: Record<string, any>
+}): Promise<T> => {
+  const preferencesResult = await req.payload
+    .find({
+      collection: 'payload-preferences',
+      depth: 0,
+      limit: 1,
+      pagination: false,
+      where: {
+        and: [
+          {
+            key: {
+              equals: key,
+            },
+          },
+          {
+            'user.relationTo': {
+              equals: req.user.collection,
+            },
+          },
+          {
+            'user.value': {
+              equals: req.user.id,
+            },
+          },
+        ],
+      },
+    })
+    .then((res) => res.docs[0] ?? { id: null, value: {} })
+
+  let newPrefs = preferencesResult.value
+
+  if (!preferencesResult.id) {
+    await req.payload.create({
+      collection: 'payload-preferences',
+      data: {
+        key,
+        user: {
+          collection: req.user.collection,
+          value: req.user.id,
+        },
+        value: incomingValue,
+      },
+      depth: 0,
+      req,
+    })
+  } else {
+    const mergedPrefs = {
+      ...(preferencesResult?.value || {}), // Shallow merge existing prefs to acquire any missing keys from incoming value
+      ...removeUndefined(incomingValue || {}),
+    }
+
+    if (!dequal(mergedPrefs, preferencesResult.value)) {
+      newPrefs = await req.payload
+        .update({
+          id: preferencesResult.id,
+          collection: 'payload-preferences',
+          data: {
+            key,
+            user: {
+              collection: req.user.collection,
+              value: req.user.id,
+            },
+            value: mergedPrefs,
+          },
+          depth: 0,
+          req,
+        })
+        ?.then((res) => res.value)
+    }
+  }
+
+  return newPrefs
+}

test/admin/e2e/list-view/e2e.spec.ts

@@ -1001,6 +1001,57 @@ describe('List View', () => {
       await expect(page.locator('#heading-id')).toBeHidden()
       await expect(page.locator('.cell-id')).toHaveCount(0)
     })
+
+    test('should sort without resetting column preferences', async () => {
+      await payload.delete({
+        collection: 'payload-preferences',
+        where: {
+          key: {
+            equals: `${postsCollectionSlug}.list`,
+          },
+        },
+      })
+
+      await page.goto(postsUrl.list)
+
+      // sort by title
+      await page.locator('#heading-title button.sort-column__asc').click()
+      await page.waitForURL(/sort=title/)
+
+      // enable a column that is _not_ part of this collection's default columns
+      await toggleColumn(page, { columnLabel: 'Status', targetState: 'on' })
+      await page.locator('#heading-_status').waitFor({ state: 'visible' })
+
+      const columnAfterSort = page.locator(
+        `.list-controls__columns .column-selector .column-selector__column`,
+        {
+          hasText: exactText('Status'),
+        },
+      )
+
+      await expect(columnAfterSort).toHaveClass(/column-selector__column--active/)
+      await expect(page.locator('#heading-_status')).toBeVisible()
+      await expect(page.locator('.cell-_status').first()).toBeVisible()
+
+      // sort by title again in descending order
+      await page.locator('#heading-title button.sort-column__desc').click()
+      await page.waitForURL(/sort=-title/)
+
+      // allow time for components to re-render
+      await wait(100)
+
+      // ensure the column is still visible
+      const columnAfterSecondSort = page.locator(
+        `.list-controls__columns .column-selector .column-selector__column`,
+        {
+          hasText: exactText('Status'),
+        },
+      )
+
+      await expect(columnAfterSecondSort).toHaveClass(/column-selector__column--active/)
+      await expect(page.locator('#heading-_status')).toBeVisible()
+      await expect(page.locator('.cell-_status').first()).toBeVisible()
+    })
   })
 
   describe('i18n', () => {

test/admin/payload-types.ts

@@ -135,6 +135,8 @@ export interface Upload {
   };
 }
 /**
+ * This is a custom collection description.
+ *
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "posts".
  */
@@ -175,10 +177,14 @@ export interface Post {
   defaultValueField?: string | null;
   relationship?: (string | null) | Post;
   customCell?: string | null;
+  upload?: (string | null) | Upload;
   hiddenField?: string | null;
   adminHiddenField?: string | null;
   disableListColumnText?: string | null;
   disableListFilterText?: string | null;
+  /**
+   * This is a very long description that takes many characters to complete and hopefully will wrap instead of push the sidebar open, lorem ipsum dolor sit amet consectetur adipisicing elit. Quisquam, voluptatum voluptates. Quisquam, voluptatum voluptates.
+   */
   sidebarField?: string | null;
   updatedAt: string;
   createdAt: string;
@@ -260,7 +266,13 @@ export interface CustomField {
   id: string;
   customTextServerField?: string | null;
   customTextClientField?: string | null;
+  /**
+   * Static field description.
+   */
   descriptionAsString?: string | null;
+  /**
+   * Function description
+   */
   descriptionAsFunction?: string | null;
   descriptionAsComponent?: string | null;
   customSelectField?: string | null;
@@ -548,6 +560,7 @@ export interface PostsSelect<T extends boolean = true> {
   defaultValueField?: T;
   relationship?: T;
   customCell?: T;
+  upload?: T;
   hiddenField?: T;
   adminHiddenField?: T;
   disableListColumnText?: T;

tsconfig.base.json

@@ -28,7 +28,7 @@
       }
     ],
     "paths": {
-      "@payload-config": ["./test/_community/config.ts"],
+      "@payload-config": ["./test/admin/config.ts"],
       "@payloadcms/live-preview": ["./packages/live-preview/src"],
       "@payloadcms/live-preview-react": ["./packages/live-preview-react/src/index.ts"],
       "@payloadcms/live-preview-vue": ["./packages/live-preview-vue/src/index.ts"],