fix(next): version view did not properly handle field-level permissions (#13336)

Field-level permissions were not handled correctly at all. If you had a field set with access control, this would mean that nested fields would incorrectly be omitted from the version view. --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1210932060696919
2025-08-02 12:01:48 -07:00
parent 1b31c74d32
commit 1d70d4d36c
4 changed files with 83 additions and 29 deletions
--- a/packages/next/src/views/Version/RenderFieldsToDiff/buildVersionFields.tsx
+++ b/packages/next/src/views/Version/RenderFieldsToDiff/buildVersionFields.tsx
@@ -17,7 +17,13 @@ import {
  type SanitizedFieldPermissions,
  type VersionField,
 } from 'payload'
-import { fieldIsID, fieldShouldBeLocalized, getUniqueListBy, tabHasName } from 'payload/shared'
+import {
+  fieldIsID,
+  fieldShouldBeLocalized,
+  getFieldPermissions,
+  getUniqueListBy,
+  tabHasName,
+} from 'payload/shared'

 import { diffComponents } from './fields/index.js'
 import { getFieldPathsModified } from './utilities/getFieldPathsModified.js'
@@ -223,21 +229,16 @@ const buildVersionField = ({
  BuildVersionFieldsArgs,
  'fields' | 'parentIndexPath' | 'versionFromSiblingData' | 'versionToSiblingData'
 >): BaseVersionField | null => {
-  const fieldName: null | string = 'name' in field ? field.name : null
+  const { permissions, read: hasReadPermission } = getFieldPermissions({
+    field,
+    operation: 'read',
+    parentName: parentPath?.includes('.')
+      ? parentPath.split('.')[parentPath.split('.').length - 1]
+      : parentPath,
+    permissions: fieldPermissions,
+  })

-  const hasPermission =
-    fieldPermissions === true ||
-    !fieldName ||
-    fieldPermissions?.[fieldName] === true ||
-    fieldPermissions?.[fieldName]?.read
-
-  const subFieldPermissions =
-    fieldPermissions === true ||
-    !fieldName ||
-    fieldPermissions?.[fieldName] === true ||
-    fieldPermissions?.[fieldName]?.fields
-
-  if (!hasPermission) {
+  if (!hasReadPermission) {
    return null
  }

@@ -292,13 +293,29 @@ const buildVersionField = ({
        parentPath,
        parentSchemaPath,
      })
+
+      let tabPermissions: typeof fieldPermissions = undefined
+
+      if (typeof permissions === 'boolean') {
+        tabPermissions = permissions
+      } else if (permissions && typeof permissions === 'object') {
+        if ('name' in tab) {
+          tabPermissions =
+            typeof permissions.fields?.[tab.name] === 'object'
+              ? permissions.fields?.[tab.name].fields
+              : permissions.fields?.[tab.name]
+        } else {
+          tabPermissions = permissions.fields
+        }
+      }
+
      const tabVersion = {
        name: 'name' in tab ? tab.name : null,
        fields: buildVersionFields({
          clientSchemaMap,
          customDiffComponents,
          entitySlug,
-          fieldPermissions,
+          fieldPermissions: tabPermissions,
          fields: tab.fields,
          i18n,
          modifiedOnly,
@@ -324,6 +341,13 @@ const buildVersionField = ({
    }
  } // At this point, we are dealing with a `row`, `collapsible`, etc
  else if ('fields' in field) {
+    let subfieldPermissions: typeof fieldPermissions = undefined
+
+    if (typeof permissions === 'boolean') {
+      subfieldPermissions = permissions
+    } else if (permissions && typeof permissions === 'object') {
+      subfieldPermissions = permissions.fields
+    }
    if (field.type === 'array' && (valueTo || valueFrom)) {
      const maxLength = Math.max(
        Array.isArray(valueTo) ? valueTo.length : 0,
@@ -339,7 +363,7 @@ const buildVersionField = ({
          clientSchemaMap,
          customDiffComponents,
          entitySlug,
-          fieldPermissions,
+          fieldPermissions: subfieldPermissions,
          fields: field.fields,
          i18n,
          modifiedOnly,
@@ -363,7 +387,7 @@ const buildVersionField = ({
        clientSchemaMap,
        customDiffComponents,
        entitySlug,
-        fieldPermissions,
+        fieldPermissions: subfieldPermissions,
        fields: field.fields,
        i18n,
        modifiedOnly,
@@ -421,11 +445,24 @@ const buildVersionField = ({
        }
      }

+      let blockPermissions: typeof fieldPermissions = undefined
+
+      if (permissions === true) {
+        blockPermissions = true
+      } else {
+        const permissionsBlockSpecific = permissions?.blocks?.[blockSlugToMatch]
+        if (permissionsBlockSpecific === true) {
+          blockPermissions = true
+        } else {
+          blockPermissions = permissionsBlockSpecific?.fields
+        }
+      }
+
      baseVersionField.rows[i] = buildVersionFields({
        clientSchemaMap,
        customDiffComponents,
        entitySlug,
-        fieldPermissions,
+        fieldPermissions: blockPermissions,
        fields,
        i18n,
        modifiedOnly,
@@ -459,7 +496,7 @@ const buildVersionField = ({
     */
    diffMethod: 'diffWordsWithSpace',
    field: clientField,
-    fieldPermissions: subFieldPermissions,
+    fieldPermissions: typeof permissions === 'object' ? permissions.fields : permissions,
    parentIsLocalized,

    nestingLevel: nestingLevel ? nestingLevel : undefined,
--- a/test/versions/e2e.spec.ts
+++ b/test/versions/e2e.spec.ts
@@ -1776,6 +1776,18 @@ describe('Versions', () => {
        String(uploadDocs?.docs?.[1]?.filename),
      )
    })
+
+    test('does not render diff for fields with read access control false', async () => {
+      await navigateToDiffVersionView()
+
+      const hiddenField1 = page.locator(
+        '[data-field-path="blocks.2.textInUnnamedTab2InBlockAccessFalse"]',
+      )
+      await expect(hiddenField1).toBeHidden()
+
+      const hiddenField2 = page.locator('[data-field-path="textCannotRead"]')
+      await expect(hiddenField2).toBeHidden()
+    })
  })

  describe('Scheduled publish', () => {
--- a/test/versions/payload-types.ts
+++ b/test/versions/payload-types.ts
@@ -366,6 +366,7 @@ export interface Diff {
              textInNamedTab1InBlock?: string | null;
            };
            textInUnnamedTab2InBlock?: string | null;
+            textInUnnamedTab2InBlockAccessFalse?: string | null;
            id?: string | null;
            blockName?: string | null;
            blockType: 'TabsBlock';
@@ -468,6 +469,7 @@ export interface Diff {
  };
  textInUnnamedTab2?: string | null;
  text?: string | null;
+  textCannotRead?: string | null;
  textArea?: string | null;
  upload?: (string | null) | Media;
  uploadHasMany?: (string | Media)[] | null;
@@ -958,6 +960,7 @@ export interface DiffSelect<T extends boolean = true> {
                    textInNamedTab1InBlock?: T;
                  };
              textInUnnamedTab2InBlock?: T;
+              textInUnnamedTab2InBlockAccessFalse?: T;
              id?: T;
              blockName?: T;
            };
@@ -992,6 +995,7 @@ export interface DiffSelect<T extends boolean = true> {
      };
  textInUnnamedTab2?: T;
  text?: T;
+  textCannotRead?: T;
  textArea?: T;
  upload?: T;
  uploadHasMany?: T;
--- a/test/versions/seed.ts
+++ b/test/versions/seed.ts
@@ -235,6 +235,7 @@ export async function seed(_payload: Payload, parallel: boolean = false) {
            textInNamedTab1InBlock: 'textInNamedTab1InBlock',
          },
          textInUnnamedTab2InBlock: 'textInUnnamedTab2InBlock',
+          textInUnnamedTab2InBlockAccessFalse: 'textInUnnamedTab2InBlockAccessFalse',
        },
      ],
      checkbox: true,
@@ -274,6 +275,7 @@ export async function seed(_payload: Payload, parallel: boolean = false) {
      textInCollapsible: 'textInCollapsible',
      textInRow: 'textInRow',
      textInUnnamedTab2: 'textInUnnamedTab2',
+      textCannotRead: 'textCannotRead',
      relationshipPolymorphic: {
        relationTo: 'text',
        value: doc1ID,
@@ -308,8 +310,8 @@ export async function seed(_payload: Payload, parallel: boolean = false) {
  await _payload.db.updateOne({
    collection: diffCollectionSlug,
    id: diffDoc.id,
+    returning: false,
    data: {
-      ...diffDoc,
      point: pointGeoJSON,
      createdAt: new Date(new Date(diffDoc.createdAt).getTime() - 2 * 60 * 10000).toISOString(),
      updatedAt: new Date(new Date(diffDoc.updatedAt).getTime() - 2 * 60 * 10000).toISOString(),
@@ -326,18 +328,15 @@ export async function seed(_payload: Payload, parallel: boolean = false) {
  let i = 0
  for (const version of versions.docs) {
    i += 1
+    const date = new Date(new Date(version.createdAt).getTime() - 2 * 60 * 10000 * i).toISOString()
    await _payload.db.updateVersion({
      id: version.id,
      collection: diffCollectionSlug,
+      returning: false,
      versionData: {
-        ...version.version,
-        createdAt: new Date(
-          new Date(version.createdAt).getTime() - 2 * 60 * 10000 * i,
-        ).toISOString(),
-        updatedAt: new Date(
-          new Date(version.updatedAt).getTime() - 2 * 60 * 10000 * i,
-        ).toISOString(),
-      },
+        createdAt: date,
+        updatedAt: date,
+      } as any,
    })
  }

@@ -373,6 +372,7 @@ export async function seed(_payload: Payload, parallel: boolean = false) {
            textInNamedTab1InBlock: 'textInNamedTab1InBlock2',
          },
          textInUnnamedTab2InBlock: 'textInUnnamedTab2InBlock2',
+          textInUnnamedTab2InBlockAccessFalse: 'textInUnnamedTab2InBlockAccessFalse2',
        },
      ],
      checkbox: false,
@@ -435,6 +435,7 @@ export async function seed(_payload: Payload, parallel: boolean = false) {
      textArea: 'textArea2',
      textInCollapsible: 'textInCollapsible2',
      textInRow: 'textInRow2',
+      textCannotRead: 'textCannotRead2',
      textInUnnamedTab2: 'textInUnnamedTab22',
      upload: uploadedImage2,
      uploadHasMany: [uploadedImage, uploadedImage2],