From bbcdea5450e7c38d856541e7c56d545cb0b9b260 Mon Sep 17 00:00:00 2001 From: Said Akhrarov <36972061+akhrarovsaid@users.noreply.github.com> Date: Sat, 6 Sep 2025 00:52:15 -0400 Subject: [PATCH] fix(next): display deleted relations and uploads in version diff views (#12955) This PR fixes an issue where `relationship` and `upload` fields that had their document-counterpart deleted would cause a runtime error. This happens because the version diff components expected a populated document instead of an id. Two e2e tests were also added to the existing suite to test that the diff view is reachable even after deletions. ### Why? To prevent a runtime error when viewing diffs for documents that have relationships to deleted documents. ### How? Adjusting the diff view to accommodate deleted document relations. Fixes #12915 Before: [version-diff-collections-before.mp4](https://private-user-images.githubusercontent.com/20878653/458373129-745a5313-b85a-4ef0-a0d4-a13b52994f6f.mp4?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NTA5Njc3NDEsIm5iZiI6MTc1MDk2NzQ0MSwicGF0aCI6Ii8yMDg3ODY1My80NTgzNzMxMjktNzQ1YTUzMTMtYjg1YS00ZWYwLWEwZDQtYTEzYjUyOTk0ZjZmLm1wND9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTA2MjYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwNjI2VDE5NTA0MVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTUwOTUzMmJmYWI4YmM4ODZjYjhiNWVkOWE0NDgwODRiYjUxNjVkZmNiOWE2NWM3ODVhMTJiY2ViMGQ4MDE4NjYmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.Of4Os9WCeOo_aN2kHNMShEgOc4-mYAwc41_E3YsX1tw) After: [versions-collections-after-Posts---Payload.webm](https://github.com/user-attachments/assets/ce4031da-b703-4944-857d-9ff627c58fdd) Notes: - I renamed `PopulatedRelationshipValue` to `RelationshipValue` as we don't actually know if the value is populated. Such as in the case of when the doc was deleted. --- .../Relationship/generateLabelFromValue.ts | 14 ++++-- .../fields/Relationship/index.tsx | 24 +++++----- .../fields/Upload/index.tsx | 46 ++++++++++++------- test/versions/e2e.spec.ts | 35 ++++++++++++++ 4 files changed, 87 insertions(+), 32 deletions(-) diff --git a/packages/next/src/views/Version/RenderFieldsToDiff/fields/Relationship/generateLabelFromValue.ts b/packages/next/src/views/Version/RenderFieldsToDiff/fields/Relationship/generateLabelFromValue.ts index ec47b7db94..f094a56d43 100644 --- a/packages/next/src/views/Version/RenderFieldsToDiff/fields/Relationship/generateLabelFromValue.ts +++ b/packages/next/src/views/Version/RenderFieldsToDiff/fields/Relationship/generateLabelFromValue.ts @@ -7,7 +7,7 @@ import { flattenTopLevelFields, } from 'payload/shared' -import type { PopulatedRelationshipValue } from './index.js' +import type { RelationshipValue } from './index.js' export const generateLabelFromValue = ({ field, @@ -20,9 +20,9 @@ export const generateLabelFromValue = ({ locale: string parentIsLocalized: boolean req: PayloadRequest - value: PopulatedRelationshipValue + value: RelationshipValue }): string => { - let relatedDoc: TypeWithID + let relatedDoc: number | string | TypeWithID let relationTo: string = field.relationTo as string let valueToReturn: string = '' @@ -30,7 +30,7 @@ export const generateLabelFromValue = ({ relatedDoc = value.value relationTo = value.relationTo } else { - // Non-polymorphic relationship + // Non-polymorphic relationship or deleted document relatedDoc = value } @@ -54,7 +54,11 @@ export const generateLabelFromValue = ({ if (typeof relatedDoc?.[useAsTitle] !== 'undefined') { valueToReturn = relatedDoc[useAsTitle] } else { - valueToReturn = String(relatedDoc.id) + valueToReturn = String( + typeof relatedDoc === 'object' + ? relatedDoc.id + : `${req.i18n.t('general:untitled')} - ID: ${relatedDoc}`, + ) } if ( diff --git a/packages/next/src/views/Version/RenderFieldsToDiff/fields/Relationship/index.tsx b/packages/next/src/views/Version/RenderFieldsToDiff/fields/Relationship/index.tsx index bf99d7a531..5bfed16324 100644 --- a/packages/next/src/views/Version/RenderFieldsToDiff/fields/Relationship/index.tsx +++ b/packages/next/src/views/Version/RenderFieldsToDiff/fields/Relationship/index.tsx @@ -16,7 +16,9 @@ import { generateLabelFromValue } from './generateLabelFromValue.js' const baseClass = 'relationship-diff' -export type PopulatedRelationshipValue = { relationTo: string; value: TypeWithID } | TypeWithID +export type RelationshipValue = + | { relationTo: string; value: number | string | TypeWithID } + | (number | string | TypeWithID) export const Relationship: RelationshipFieldDiffServerComponent = ({ comparisonValue: valueFrom, @@ -41,8 +43,8 @@ export const Relationship: RelationshipFieldDiffServerComponent = ({ parentIsLocalized={parentIsLocalized} polymorphic={polymorphic} req={req} - valueFrom={valueFrom as PopulatedRelationshipValue[] | undefined} - valueTo={valueTo as PopulatedRelationshipValue[] | undefined} + valueFrom={valueFrom as RelationshipValue[] | undefined} + valueTo={valueTo as RelationshipValue[] | undefined} /> ) } @@ -56,8 +58,8 @@ export const Relationship: RelationshipFieldDiffServerComponent = ({ parentIsLocalized={parentIsLocalized} polymorphic={polymorphic} req={req} - valueFrom={valueFrom as PopulatedRelationshipValue} - valueTo={valueTo as PopulatedRelationshipValue} + valueFrom={valueFrom as RelationshipValue} + valueTo={valueTo as RelationshipValue} /> ) } @@ -70,8 +72,8 @@ export const SingleRelationshipDiff: React.FC<{ parentIsLocalized: boolean polymorphic: boolean req: PayloadRequest - valueFrom: PopulatedRelationshipValue - valueTo: PopulatedRelationshipValue + valueFrom: RelationshipValue + valueTo: RelationshipValue }> = async (args) => { const { field, @@ -151,8 +153,8 @@ const ManyRelationshipDiff: React.FC<{ parentIsLocalized: boolean polymorphic: boolean req: PayloadRequest - valueFrom: PopulatedRelationshipValue[] | undefined - valueTo: PopulatedRelationshipValue[] | undefined + valueFrom: RelationshipValue[] | undefined + valueTo: RelationshipValue[] | undefined }> = async ({ field, i18n, @@ -169,7 +171,7 @@ const ManyRelationshipDiff: React.FC<{ const fromArr = Array.isArray(valueFrom) ? valueFrom : [] const toArr = Array.isArray(valueTo) ? valueTo : [] - const makeNodes = (list: PopulatedRelationshipValue[]) => + const makeNodes = (list: RelationshipValue[]) => list.map((val, idx) => ( { const localeToUse = locale ?? diff --git a/packages/next/src/views/Version/RenderFieldsToDiff/fields/Upload/index.tsx b/packages/next/src/views/Version/RenderFieldsToDiff/fields/Upload/index.tsx index 40f555893a..75f382ada9 100644 --- a/packages/next/src/views/Version/RenderFieldsToDiff/fields/Upload/index.tsx +++ b/packages/next/src/views/Version/RenderFieldsToDiff/fields/Upload/index.tsx @@ -15,6 +15,8 @@ import React from 'react' const baseClass = 'upload-diff' +type UploadDoc = (FileData & TypeWithID) | string + export const Upload: UploadFieldDiffServerComponent = (args) => { const { comparisonValue: valueFrom, @@ -34,8 +36,8 @@ export const Upload: UploadFieldDiffServerComponent = (args) => { locale={locale} nestingLevel={nestingLevel} req={req} - valueFrom={valueFrom as any} - valueTo={valueTo as any} + valueFrom={valueFrom as UploadDoc[]} + valueTo={valueTo as UploadDoc[]} /> ) } @@ -47,8 +49,8 @@ export const Upload: UploadFieldDiffServerComponent = (args) => { locale={locale} nestingLevel={nestingLevel} req={req} - valueFrom={valueFrom as any} - valueTo={valueTo as any} + valueFrom={valueFrom as UploadDoc} + valueTo={valueTo as UploadDoc} /> ) } @@ -59,8 +61,8 @@ export const HasManyUploadDiff: React.FC<{ locale: string nestingLevel?: number req: PayloadRequest - valueFrom: Array - valueTo: Array + valueFrom: Array + valueTo: Array }> = async (args) => { const { field, i18n, locale, nestingLevel, req, valueFrom, valueTo } = args const ReactDOMServer = (await import('react-dom/server')).default @@ -74,7 +76,7 @@ export const HasManyUploadDiff: React.FC<{ ? valueFrom.map((uploadDoc) => ( ( = async (args) => { const { field, i18n, locale, nestingLevel, req, valueFrom, valueTo } = args @@ -204,12 +206,24 @@ const UploadDocumentDiff = (args: { relationTo: string req: PayloadRequest showCollectionSlug?: boolean - uploadDoc: FileData & TypeWithID + uploadDoc: UploadDoc }) => { const { i18n, relationTo, req, showCollectionSlug, uploadDoc } = args - const thumbnailSRC: string = - ('thumbnailURL' in uploadDoc && (uploadDoc?.thumbnailURL as string)) || uploadDoc?.url || '' + let thumbnailSRC: string = '' + if (uploadDoc && typeof uploadDoc === 'object' && 'thumbnailURL' in uploadDoc) { + thumbnailSRC = + (typeof uploadDoc.thumbnailURL === 'string' && uploadDoc.thumbnailURL) || + (typeof uploadDoc.url === 'string' && uploadDoc.url) || + '' + } + + let filename: string + if (uploadDoc && typeof uploadDoc === 'object') { + filename = uploadDoc.filename + } else { + filename = `${i18n.t('general:untitled')} - ID: ${uploadDoc as number | string}` + } let pillLabel: null | string = null @@ -224,12 +238,12 @@ const UploadDocumentDiff = (args: {
- {thumbnailSRC?.length ? {uploadDoc?.filename} : } + {thumbnailSRC?.length ? {filename} : }
{pillLabel && (
@@ -237,7 +251,7 @@ const UploadDocumentDiff = (args: {
)}
- {uploadDoc?.filename} + {filename}
diff --git a/test/versions/e2e.spec.ts b/test/versions/e2e.spec.ts index 7761cfb584..d6151e35f3 100644 --- a/test/versions/e2e.spec.ts +++ b/test/versions/e2e.spec.ts @@ -1982,6 +1982,41 @@ describe('Versions', () => { const hiddenField2 = page.locator('[data-field-path="textCannotRead"]') await expect(hiddenField2).toBeHidden() }) + + test('correctly renders diff for relationship fields with deleted relation', async () => { + await payload.delete({ + collection: 'draft-posts', + }) + + await navigateToDiffVersionView() + + const diffsToCheck = [ + 'relationship', + 'relationshipHasMany', + 'relationshipHasManyPolymorphic', + 'relationshipHasManyPolymorphic2', + ] + const checkPromises = diffsToCheck.map(async (dataFieldPath) => { + const relation = page.locator(`[data-field-path="${dataFieldPath}"]`) + return expect(relation).toBeVisible() + }) + await Promise.all(checkPromises) + }) + + test('correctly renders diff for upload fields with deleted upload', async () => { + await payload.delete({ + collection: 'media', + }) + + await navigateToDiffVersionView() + + const diffsToCheck = ['upload', 'uploadHasMany'] + const checkPromises = diffsToCheck.map(async (dataFieldPath) => { + const relation = page.locator(`[data-field-path="${dataFieldPath}"]`) + return expect(relation).toBeVisible() + }) + await Promise.all(checkPromises) + }) }) describe('Scheduled publish', () => {