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.
2024-11-26 14:33:10 -05:00
parent b61622019e
commit f19053e049
8 changed files with 114 additions and 17 deletions
--- 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}
--- 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}
--- 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}
--- 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
 }
--- 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}
              />
            )
--- 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
--- 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
 }

--- 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()
+    })
+  })
 })