fix: don't count hidden fields in the version view

Author: franciscolourenco

Diff for: packages/next/src/views/Version/RenderFieldsToDiff/index.tsx

@@ -8,6 +8,7 @@ import type { diffComponents as _diffComponents } from './fields/index.js'
 import type { FieldDiffProps, Props } from './types.js'
 
 import { diffMethods } from './fields/diffMethods.js'
+import { isFieldHiddenInVersionView } from './utilities/isFieldHiddenInVersionView.js'
 import './index.scss'
 
 const baseClass = 'render-field-diffs'
@@ -28,22 +29,7 @@ export const RenderFieldsToDiff: React.FC<Props> = ({
   return (
     <div className={baseClass}>
       {fields?.map((field, i) => {
-        if (fieldIsID(field)) {
-          return null
-        }
-
-        // Don't render hidden fields
-        if (field.hidden) {
-          return null
-        }
-
-        // Don't render fields with admin.disabled
-        if (field.admin.disabled) {
-          return null
-        }
-
-        // Don't render fields with admin.hiddenInVersionView
-        if (field.admin.hiddenInVersionView) {
+        if (isFieldHiddenInVersionView(field)) {
           return null
         }
 

Diff for: packages/next/src/views/Version/RenderFieldsToDiff/utilities/countChangedFields.spec.ts

@@ -1,6 +1,19 @@
 import { countChangedFields, countChangedFieldsInRows } from './countChangedFields.js'
+import { isFieldHiddenInVersionView } from './isFieldHiddenInVersionView.js'
+
 import type { ClientField } from 'payload'
 
+// Mock isFieldHiddenInVersionView with it's original implementation so we can
+// replace it later in the hidden fields tests.
+// (Couldn't use jest.spyOn directly due to "TypeError: Cannot redefine property")
+jest.mock('./isFieldHiddenInVersionView', () => {
+  const actual = jest.requireActual('./isFieldHiddenInVersionView')
+  return {
+    ...actual,
+    isFieldHiddenInVersionView: jest.fn(actual.isFieldHiddenInVersionView),
+  }
+})
+
 describe('countChangedFields', () => {
   // locales can be undefined when not configured in payload.config.js
   const locales = undefined
@@ -40,16 +53,20 @@ describe('countChangedFields', () => {
     expect(result).toBe(2)
   })
 
-  it('should not count the id field because it is not displayed in the version view', () => {
+  it('should not count changed fields when isFieldHiddenInVersionView() returns true', async () => {
+    // Make isFieldHiddenInVersionView return true for one of the fields
+    // jest.mocked() is used to correct the type of isFieldHiddenInVersionView
+    jest.mocked(isFieldHiddenInVersionView).mockImplementationOnce(() => true)
+
     const fields: ClientField[] = [
-      { name: 'id', type: 'text' },
       { name: 'a', type: 'text' },
+      { name: 'b', type: 'text' },
     ]
-    const comparison = { id: 'original', a: 'original' }
-    const version = { id: 'changed', a: 'original' }
+    const comparison = { a: 'original', b: 'original' }
+    const version = { a: 'changed', b: 'changed' }
 
     const result = countChangedFields({ comparison, fields, version, locales })
-    expect(result).toBe(0)
+    expect(result).toBe(1)
   })
 
   it('should count changed fields inside collapsible fields', () => {

Diff for: packages/next/src/views/Version/RenderFieldsToDiff/utilities/countChangedFields.ts

@@ -2,6 +2,7 @@ import type { ArrayFieldClient, BlocksFieldClient, ClientField } from 'payload'
 
 import { fieldHasChanges } from './fieldHasChanges.js'
 import { getFieldsForRowComparison } from './getFieldsForRowComparison.js'
+import { isFieldHiddenInVersionView } from './isFieldHiddenInVersionView.js'
 
 type Args = {
   comparison: unknown
@@ -18,10 +19,11 @@ export function countChangedFields({ comparison, fields, locales, version }: Arg
   let count = 0
 
   fields.forEach((field) => {
-    // Don't count the id field since it is not displayed in the UI
-    if ('name' in field && field.name === 'id') {
+    // Don't count fields that are hidden in the version view
+    if (isFieldHiddenInVersionView(field)) {
       return
     }
+
     const fieldType = field.type
     switch (fieldType) {
       // Iterable fields are arrays and blocks fields. We iterate over each row and

Diff for: packages/next/src/views/Version/RenderFieldsToDiff/utilities/isFieldHiddenInVersionView.ts

@@ -0,0 +1,29 @@
+import type { ClientField } from 'payload'
+
+import { fieldIsID } from 'payload/shared'
+
+export function isFieldHiddenInVersionView(field: ClientField) {
+  // Don't render id fields
+  if (fieldIsID(field)) {
+    return true
+  }
+
+  // Don't render hidden fields, because they are supposed to be hidden everywhere
+  if (field.hidden) {
+    return true
+  }
+
+  // Don't render fields with admin.disabled, because they are supposed to be
+  // hidden everywhere in the admin UI.
+  if (field.admin?.disabled) {
+    return true
+  }
+
+  // Don't render fields with admin.hiddenInVersionView, because they are
+  // supposed to be hidden specifically in the version view.
+  if (field.admin?.hiddenInVersionView) {
+    return true
+  }
+
+  return false
+}