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

Author: AlessioGr

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 hasPermission =
-    fieldPermissions === true ||
-    !fieldName ||
-    fieldPermissions?.[fieldName] === true ||
-    fieldPermissions?.[fieldName]?.read
-
-  const subFieldPermissions =
-    fieldPermissions === true ||
-    !fieldName ||
-    fieldPermissions?.[fieldName] === true ||
-    fieldPermissions?.[fieldName]?.fields
+  const { permissions, read: hasReadPermission } = getFieldPermissions({
+    field,
+    operation: 'read',
+    parentName: parentPath?.includes('.')
+      ? parentPath.split('.')[parentPath.split('.').length - 1]
+      : parentPath,
+    permissions: fieldPermissions,
+  })
 
-  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,

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', () => {

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;

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],