fix(next): properly threads field permissions through versions diff (#…

…9543) The version diff view at `/admin/collections/:collection/:id/versions/:version` was not properly displaying diffs for iterable fields, such as blocks. There were two main things wrong here: 1. Fields not properly inheriting parent permissions based on the new sanitized permissions pattern in #7335 1. The diff components were expecting `permissions` but receiving `fieldPermissions`. This was not picked up by TS because of our use of dynamic keys when choosing which component to render for that particular field. We should change this in the future to use a switch case that explicitly renders each diff component. This way props are strictly typed.
payloadcms · Nov 26, 2024 · f19053e · f19053e
1 parent b616220
commit f19053e
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 17 deletions.
diff --git a/packages/next/src/views/Version/RenderFieldsToDiff/fields/Iterable/index.tsx b/packages/next/src/views/Version/RenderFieldsToDiff/fields/Iterable/index.tsx
@@ -17,10 +17,10 @@ const Iterable: React.FC<DiffComponentProps> = ({
   comparison,
   diffComponents,
   field,
+  fieldPermissions,
   i18n,
   locale,
   locales,
-  permissions,
   version,
 }) => {
   const versionRowCount = Array.isArray(version) ? version.length : 0
@@ -86,7 +86,7 @@ const Iterable: React.FC<DiffComponentProps> = ({
                 <RenderFieldsToDiff
                   comparison={comparisonRow}
                   diffComponents={diffComponents}
-                  fieldPermissions={permissions}
+                  fieldPermissions={fieldPermissions}
                   fields={fields}
                   i18n={i18n}
                   locales={locales}

diff --git a/packages/next/src/views/Version/RenderFieldsToDiff/fields/Nested/index.tsx b/packages/next/src/views/Version/RenderFieldsToDiff/fields/Nested/index.tsx
@@ -15,11 +15,11 @@ const Nested: React.FC<DiffComponentProps> = ({
   diffComponents,
   disableGutter = false,
   field,
+  fieldPermissions,
   fields,
   i18n,
   locale,
   locales,
-  permissions,
   version,
 }) => {
   return (
@@ -38,7 +38,7 @@ const Nested: React.FC<DiffComponentProps> = ({
         <RenderFieldsToDiff
           comparison={comparison}
           diffComponents={diffComponents}
-          fieldPermissions={permissions}
+          fieldPermissions={fieldPermissions}
           fields={fields}
           i18n={i18n}
           locales={locales}

diff --git a/packages/next/src/views/Version/RenderFieldsToDiff/fields/Tabs/index.tsx b/packages/next/src/views/Version/RenderFieldsToDiff/fields/Tabs/index.tsx
@@ -14,10 +14,10 @@ const Tabs: React.FC<DiffComponentProps<TabsFieldClient>> = ({
   comparison,
   diffComponents,
   field,
+  fieldPermissions,
   i18n,
   locale,
   locales,
-  permissions,
   version,
 }) => {
   return (
@@ -30,12 +30,12 @@ const Tabs: React.FC<DiffComponentProps<TabsFieldClient>> = ({
                 comparison={comparison?.[tab.name]}
                 diffComponents={diffComponents}
                 field={field}
+                fieldPermissions={fieldPermissions}
                 fields={tab.fields}
                 i18n={i18n}
                 key={i}
                 locale={locale}
                 locales={locales}
-                permissions={permissions}
                 version={version?.[tab.name]}
               />
             )
@@ -45,7 +45,7 @@ const Tabs: React.FC<DiffComponentProps<TabsFieldClient>> = ({
             <RenderFieldsToDiff
               comparison={comparison}
               diffComponents={diffComponents}
-              fieldPermissions={permissions}
+              fieldPermissions={fieldPermissions}
               fields={tab.fields}
               i18n={i18n}
               key={i}

diff --git a/packages/next/src/views/Version/RenderFieldsToDiff/fields/types.ts b/packages/next/src/views/Version/RenderFieldsToDiff/fields/types.ts
@@ -11,15 +11,15 @@ export type DiffComponentProps<TField extends ClientField = ClientField> = {
   readonly diffMethod?: DiffMethod
   readonly disableGutter?: boolean
   readonly field: TField
+  readonly fieldPermissions?:
+    | {
+        [key: string]: SanitizedFieldPermissions
+      }
+    | true
   readonly fields: ClientField[]
   readonly i18n: I18nClient
   readonly isRichText?: boolean
   readonly locale?: string
   readonly locales?: string[]
-  readonly permissions?:
-    | {
-        [key: string]: SanitizedFieldPermissions
-      }
-    | true
   readonly version: any
 }
diff --git a/packages/next/src/views/Version/RenderFieldsToDiff/index.tsx b/packages/next/src/views/Version/RenderFieldsToDiff/index.tsx
@@ -57,7 +57,9 @@ const RenderFieldsToDiff: React.FC<Props> = ({
               fieldPermissions?.[fieldName]?.read
 
             const subFieldPermissions =
-              fieldPermissions?.[fieldName] === true || fieldPermissions?.[fieldName]?.fields
+              fieldPermissions === true ||
+              fieldPermissions?.[fieldName] === true ||
+              fieldPermissions?.[fieldName]?.fields
 
             if (!hasPermission) {
               return null
@@ -116,6 +118,7 @@ const RenderFieldsToDiff: React.FC<Props> = ({
                 comparison={comparison}
                 diffComponents={diffComponents}
                 field={field}
+                fieldPermissions={fieldPermissions}
                 fields={[]}
                 i18n={i18n}
                 key={i}
@@ -133,11 +136,11 @@ const RenderFieldsToDiff: React.FC<Props> = ({
                 diffComponents={diffComponents}
                 disableGutter
                 field={field}
+                fieldPermissions={fieldPermissions}
                 fields={field.fields}
                 i18n={i18n}
                 key={i}
                 locales={locales}
-                permissions={fieldPermissions}
                 version={version}
               />
             )

diff --git a/test/helpers/sdk/index.ts b/test/helpers/sdk/index.ts
@@ -21,7 +21,9 @@ export class PayloadTestSDK<TGeneratedTypes extends GeneratedTypes<TGeneratedTyp
       'Content-Type': 'application/json',
     }
 
-    if (jwt) headers.Authorization = `JWT ${jwt}`
+    if (jwt) {
+      headers.Authorization = `JWT ${jwt}`
+    }
 
     const json: T = await fetch(`${this.serverURL}/api/local-api`, {
       method: 'post',
@@ -32,7 +34,9 @@ export class PayloadTestSDK<TGeneratedTypes extends GeneratedTypes<TGeneratedTyp
       }),
     }).then((res) => res.json())
 
-    if (reduceJSON) return reduceJSON<T>(json)
+    if (reduceJSON) {
+      return reduceJSON<T>(json)
+    }
 
     return json
   }
@@ -70,6 +74,17 @@ export class PayloadTestSDK<TGeneratedTypes extends GeneratedTypes<TGeneratedTyp
     })
   }
 
+  findVersions = async <T extends keyof TGeneratedTypes['collections']>({
+    jwt,
+    ...args
+  }: FindArgs<TGeneratedTypes, T>) => {
+    return this.fetch<PaginatedDocs<TGeneratedTypes['collections'][T]>>({
+      operation: 'findVersions',
+      args,
+      jwt,
+    })
+  }
+
   login = async <T extends keyof TGeneratedTypes['collections']>({
     jwt,
     ...args

diff --git a/test/helpers/sdk/types.ts b/test/helpers/sdk/types.ts
@@ -25,7 +25,15 @@ export type GeneratedTypes<T extends BaseTypes> = {
 export type FetchOptions = {
   args?: Record<string, unknown>
   jwt?: string
-  operation: 'create' | 'delete' | 'find' | 'login' | 'sendEmail' | 'update' | 'updateGlobal'
+  operation:
+    | 'create'
+    | 'delete'
+    | 'find'
+    | 'findVersions'
+    | 'login'
+    | 'sendEmail'
+    | 'update'
+    | 'updateGlobal'
   reduceJSON?: <R>(json: any) => R
 }
 

diff --git a/test/versions/e2e.spec.ts b/test/versions/e2e.spec.ts
@@ -734,4 +734,75 @@ describe('versions', () => {
       await expect(publishSpecificLocale).toContainText('English')
     })
   })
+
+  describe('Versions diff view', () => {
+    let postID: string
+    let versionID: string
+
+    beforeAll(() => {
+      url = new AdminUrlUtil(serverURL, draftCollectionSlug)
+    })
+
+    beforeEach(async () => {
+      const newPost = await payload.create({
+        collection: draftCollectionSlug,
+        data: {
+          title: 'new post',
+          description: 'new description',
+        },
+      })
+
+      postID = newPost.id
+
+      await payload.update({
+        collection: draftCollectionSlug,
+        id: postID,
+        draft: true,
+        data: {
+          title: 'draft post',
+          description: 'draft description',
+          blocksField: [
+            {
+              blockName: 'block1',
+              blockType: 'block',
+              text: 'block text',
+            },
+          ],
+        },
+      })
+
+      const versions = await payload.findVersions({
+        collection: draftCollectionSlug,
+        where: {
+          parent: { equals: postID },
+        },
+      })
+
+      versionID = versions.docs[0].id
+    })
+
+    test('should render diff', async () => {
+      const versionURL = `${serverURL}/admin/collections/${draftCollectionSlug}/${postID}/versions/${versionID}`
+      await page.goto(versionURL)
+      await page.waitForURL(versionURL)
+      await expect(page.locator('.render-field-diffs').first()).toBeVisible()
+    })
+
+    test('should render diff for nested fields', async () => {
+      const versionURL = `${serverURL}/admin/collections/${draftCollectionSlug}/${postID}/versions/${versionID}`
+      await page.goto(versionURL)
+      await page.waitForURL(versionURL)
+      await expect(page.locator('.render-field-diffs').first()).toBeVisible()
+
+      const blocksDiffLabel = page.locator('.field-diff-label', {
+        hasText: exactText('Blocks Field'),
+      })
+
+      await expect(blocksDiffLabel).toBeVisible()
+      const blocksDiff = blocksDiffLabel.locator('+ .iterable-diff__wrap > .render-field-diffs')
+      await expect(blocksDiff).toBeVisible()
+      const blockTypeDiffLabel = blocksDiff.locator('.render-field-diffs__field').first()
+      await expect(blockTypeDiffLabel).toBeVisible()
+    })
+  })
 })