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.
This commit is contained in:
@@ -7,7 +7,7 @@ import {
|
|||||||
flattenTopLevelFields,
|
flattenTopLevelFields,
|
||||||
} from 'payload/shared'
|
} from 'payload/shared'
|
||||||
|
|
||||||
import type { PopulatedRelationshipValue } from './index.js'
|
import type { RelationshipValue } from './index.js'
|
||||||
|
|
||||||
export const generateLabelFromValue = ({
|
export const generateLabelFromValue = ({
|
||||||
field,
|
field,
|
||||||
@@ -20,9 +20,9 @@ export const generateLabelFromValue = ({
|
|||||||
locale: string
|
locale: string
|
||||||
parentIsLocalized: boolean
|
parentIsLocalized: boolean
|
||||||
req: PayloadRequest
|
req: PayloadRequest
|
||||||
value: PopulatedRelationshipValue
|
value: RelationshipValue
|
||||||
}): string => {
|
}): string => {
|
||||||
let relatedDoc: TypeWithID
|
let relatedDoc: number | string | TypeWithID
|
||||||
let relationTo: string = field.relationTo as string
|
let relationTo: string = field.relationTo as string
|
||||||
let valueToReturn: string = ''
|
let valueToReturn: string = ''
|
||||||
|
|
||||||
@@ -30,7 +30,7 @@ export const generateLabelFromValue = ({
|
|||||||
relatedDoc = value.value
|
relatedDoc = value.value
|
||||||
relationTo = value.relationTo
|
relationTo = value.relationTo
|
||||||
} else {
|
} else {
|
||||||
// Non-polymorphic relationship
|
// Non-polymorphic relationship or deleted document
|
||||||
relatedDoc = value
|
relatedDoc = value
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -54,7 +54,11 @@ export const generateLabelFromValue = ({
|
|||||||
if (typeof relatedDoc?.[useAsTitle] !== 'undefined') {
|
if (typeof relatedDoc?.[useAsTitle] !== 'undefined') {
|
||||||
valueToReturn = relatedDoc[useAsTitle]
|
valueToReturn = relatedDoc[useAsTitle]
|
||||||
} else {
|
} else {
|
||||||
valueToReturn = String(relatedDoc.id)
|
valueToReturn = String(
|
||||||
|
typeof relatedDoc === 'object'
|
||||||
|
? relatedDoc.id
|
||||||
|
: `${req.i18n.t('general:untitled')} - ID: ${relatedDoc}`,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
if (
|
if (
|
||||||
|
|||||||
@@ -16,7 +16,9 @@ import { generateLabelFromValue } from './generateLabelFromValue.js'
|
|||||||
|
|
||||||
const baseClass = 'relationship-diff'
|
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 = ({
|
export const Relationship: RelationshipFieldDiffServerComponent = ({
|
||||||
comparisonValue: valueFrom,
|
comparisonValue: valueFrom,
|
||||||
@@ -41,8 +43,8 @@ export const Relationship: RelationshipFieldDiffServerComponent = ({
|
|||||||
parentIsLocalized={parentIsLocalized}
|
parentIsLocalized={parentIsLocalized}
|
||||||
polymorphic={polymorphic}
|
polymorphic={polymorphic}
|
||||||
req={req}
|
req={req}
|
||||||
valueFrom={valueFrom as PopulatedRelationshipValue[] | undefined}
|
valueFrom={valueFrom as RelationshipValue[] | undefined}
|
||||||
valueTo={valueTo as PopulatedRelationshipValue[] | undefined}
|
valueTo={valueTo as RelationshipValue[] | undefined}
|
||||||
/>
|
/>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -56,8 +58,8 @@ export const Relationship: RelationshipFieldDiffServerComponent = ({
|
|||||||
parentIsLocalized={parentIsLocalized}
|
parentIsLocalized={parentIsLocalized}
|
||||||
polymorphic={polymorphic}
|
polymorphic={polymorphic}
|
||||||
req={req}
|
req={req}
|
||||||
valueFrom={valueFrom as PopulatedRelationshipValue}
|
valueFrom={valueFrom as RelationshipValue}
|
||||||
valueTo={valueTo as PopulatedRelationshipValue}
|
valueTo={valueTo as RelationshipValue}
|
||||||
/>
|
/>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -70,8 +72,8 @@ export const SingleRelationshipDiff: React.FC<{
|
|||||||
parentIsLocalized: boolean
|
parentIsLocalized: boolean
|
||||||
polymorphic: boolean
|
polymorphic: boolean
|
||||||
req: PayloadRequest
|
req: PayloadRequest
|
||||||
valueFrom: PopulatedRelationshipValue
|
valueFrom: RelationshipValue
|
||||||
valueTo: PopulatedRelationshipValue
|
valueTo: RelationshipValue
|
||||||
}> = async (args) => {
|
}> = async (args) => {
|
||||||
const {
|
const {
|
||||||
field,
|
field,
|
||||||
@@ -151,8 +153,8 @@ const ManyRelationshipDiff: React.FC<{
|
|||||||
parentIsLocalized: boolean
|
parentIsLocalized: boolean
|
||||||
polymorphic: boolean
|
polymorphic: boolean
|
||||||
req: PayloadRequest
|
req: PayloadRequest
|
||||||
valueFrom: PopulatedRelationshipValue[] | undefined
|
valueFrom: RelationshipValue[] | undefined
|
||||||
valueTo: PopulatedRelationshipValue[] | undefined
|
valueTo: RelationshipValue[] | undefined
|
||||||
}> = async ({
|
}> = async ({
|
||||||
field,
|
field,
|
||||||
i18n,
|
i18n,
|
||||||
@@ -169,7 +171,7 @@ const ManyRelationshipDiff: React.FC<{
|
|||||||
const fromArr = Array.isArray(valueFrom) ? valueFrom : []
|
const fromArr = Array.isArray(valueFrom) ? valueFrom : []
|
||||||
const toArr = Array.isArray(valueTo) ? valueTo : []
|
const toArr = Array.isArray(valueTo) ? valueTo : []
|
||||||
|
|
||||||
const makeNodes = (list: PopulatedRelationshipValue[]) =>
|
const makeNodes = (list: RelationshipValue[]) =>
|
||||||
list.map((val, idx) => (
|
list.map((val, idx) => (
|
||||||
<RelationshipDocumentDiff
|
<RelationshipDocumentDiff
|
||||||
field={field}
|
field={field}
|
||||||
@@ -234,7 +236,7 @@ const RelationshipDocumentDiff = ({
|
|||||||
relationTo: string
|
relationTo: string
|
||||||
req: PayloadRequest
|
req: PayloadRequest
|
||||||
showPill?: boolean
|
showPill?: boolean
|
||||||
value: PopulatedRelationshipValue
|
value: RelationshipValue
|
||||||
}) => {
|
}) => {
|
||||||
const localeToUse =
|
const localeToUse =
|
||||||
locale ??
|
locale ??
|
||||||
|
|||||||
@@ -15,6 +15,8 @@ import React from 'react'
|
|||||||
|
|
||||||
const baseClass = 'upload-diff'
|
const baseClass = 'upload-diff'
|
||||||
|
|
||||||
|
type UploadDoc = (FileData & TypeWithID) | string
|
||||||
|
|
||||||
export const Upload: UploadFieldDiffServerComponent = (args) => {
|
export const Upload: UploadFieldDiffServerComponent = (args) => {
|
||||||
const {
|
const {
|
||||||
comparisonValue: valueFrom,
|
comparisonValue: valueFrom,
|
||||||
@@ -34,8 +36,8 @@ export const Upload: UploadFieldDiffServerComponent = (args) => {
|
|||||||
locale={locale}
|
locale={locale}
|
||||||
nestingLevel={nestingLevel}
|
nestingLevel={nestingLevel}
|
||||||
req={req}
|
req={req}
|
||||||
valueFrom={valueFrom as any}
|
valueFrom={valueFrom as UploadDoc[]}
|
||||||
valueTo={valueTo as any}
|
valueTo={valueTo as UploadDoc[]}
|
||||||
/>
|
/>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -47,8 +49,8 @@ export const Upload: UploadFieldDiffServerComponent = (args) => {
|
|||||||
locale={locale}
|
locale={locale}
|
||||||
nestingLevel={nestingLevel}
|
nestingLevel={nestingLevel}
|
||||||
req={req}
|
req={req}
|
||||||
valueFrom={valueFrom as any}
|
valueFrom={valueFrom as UploadDoc}
|
||||||
valueTo={valueTo as any}
|
valueTo={valueTo as UploadDoc}
|
||||||
/>
|
/>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -59,8 +61,8 @@ export const HasManyUploadDiff: React.FC<{
|
|||||||
locale: string
|
locale: string
|
||||||
nestingLevel?: number
|
nestingLevel?: number
|
||||||
req: PayloadRequest
|
req: PayloadRequest
|
||||||
valueFrom: Array<FileData & TypeWithID>
|
valueFrom: Array<UploadDoc>
|
||||||
valueTo: Array<FileData & TypeWithID>
|
valueTo: Array<UploadDoc>
|
||||||
}> = async (args) => {
|
}> = async (args) => {
|
||||||
const { field, i18n, locale, nestingLevel, req, valueFrom, valueTo } = args
|
const { field, i18n, locale, nestingLevel, req, valueFrom, valueTo } = args
|
||||||
const ReactDOMServer = (await import('react-dom/server')).default
|
const ReactDOMServer = (await import('react-dom/server')).default
|
||||||
@@ -74,7 +76,7 @@ export const HasManyUploadDiff: React.FC<{
|
|||||||
? valueFrom.map((uploadDoc) => (
|
? valueFrom.map((uploadDoc) => (
|
||||||
<UploadDocumentDiff
|
<UploadDocumentDiff
|
||||||
i18n={i18n}
|
i18n={i18n}
|
||||||
key={uploadDoc.id}
|
key={typeof uploadDoc === 'object' ? uploadDoc.id : uploadDoc}
|
||||||
relationTo={field.relationTo}
|
relationTo={field.relationTo}
|
||||||
req={req}
|
req={req}
|
||||||
showCollectionSlug={showCollectionSlug}
|
showCollectionSlug={showCollectionSlug}
|
||||||
@@ -86,7 +88,7 @@ export const HasManyUploadDiff: React.FC<{
|
|||||||
? valueTo.map((uploadDoc) => (
|
? valueTo.map((uploadDoc) => (
|
||||||
<UploadDocumentDiff
|
<UploadDocumentDiff
|
||||||
i18n={i18n}
|
i18n={i18n}
|
||||||
key={uploadDoc.id}
|
key={typeof uploadDoc === 'object' ? uploadDoc.id : uploadDoc}
|
||||||
relationTo={field.relationTo}
|
relationTo={field.relationTo}
|
||||||
req={req}
|
req={req}
|
||||||
showCollectionSlug={showCollectionSlug}
|
showCollectionSlug={showCollectionSlug}
|
||||||
@@ -138,8 +140,8 @@ export const SingleUploadDiff: React.FC<{
|
|||||||
locale: string
|
locale: string
|
||||||
nestingLevel?: number
|
nestingLevel?: number
|
||||||
req: PayloadRequest
|
req: PayloadRequest
|
||||||
valueFrom: FileData & TypeWithID
|
valueFrom: UploadDoc
|
||||||
valueTo: FileData & TypeWithID
|
valueTo: UploadDoc
|
||||||
}> = async (args) => {
|
}> = async (args) => {
|
||||||
const { field, i18n, locale, nestingLevel, req, valueFrom, valueTo } = args
|
const { field, i18n, locale, nestingLevel, req, valueFrom, valueTo } = args
|
||||||
|
|
||||||
@@ -204,12 +206,24 @@ const UploadDocumentDiff = (args: {
|
|||||||
relationTo: string
|
relationTo: string
|
||||||
req: PayloadRequest
|
req: PayloadRequest
|
||||||
showCollectionSlug?: boolean
|
showCollectionSlug?: boolean
|
||||||
uploadDoc: FileData & TypeWithID
|
uploadDoc: UploadDoc
|
||||||
}) => {
|
}) => {
|
||||||
const { i18n, relationTo, req, showCollectionSlug, uploadDoc } = args
|
const { i18n, relationTo, req, showCollectionSlug, uploadDoc } = args
|
||||||
|
|
||||||
const thumbnailSRC: string =
|
let thumbnailSRC: string = ''
|
||||||
('thumbnailURL' in uploadDoc && (uploadDoc?.thumbnailURL as string)) || uploadDoc?.url || ''
|
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
|
let pillLabel: null | string = null
|
||||||
|
|
||||||
@@ -224,12 +238,12 @@ const UploadDocumentDiff = (args: {
|
|||||||
<div
|
<div
|
||||||
className={`${baseClass}`}
|
className={`${baseClass}`}
|
||||||
data-enable-match="true"
|
data-enable-match="true"
|
||||||
data-id={uploadDoc?.id}
|
data-id={typeof uploadDoc === 'object' ? uploadDoc?.id : uploadDoc}
|
||||||
data-relation-to={relationTo}
|
data-relation-to={relationTo}
|
||||||
>
|
>
|
||||||
<div className={`${baseClass}__card`}>
|
<div className={`${baseClass}__card`}>
|
||||||
<div className={`${baseClass}__thumbnail`}>
|
<div className={`${baseClass}__thumbnail`}>
|
||||||
{thumbnailSRC?.length ? <img alt={uploadDoc?.filename} src={thumbnailSRC} /> : <File />}
|
{thumbnailSRC?.length ? <img alt={filename} src={thumbnailSRC} /> : <File />}
|
||||||
</div>
|
</div>
|
||||||
{pillLabel && (
|
{pillLabel && (
|
||||||
<div className={`${baseClass}__pill`} data-enable-match="false">
|
<div className={`${baseClass}__pill`} data-enable-match="false">
|
||||||
@@ -237,7 +251,7 @@ const UploadDocumentDiff = (args: {
|
|||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
<div className={`${baseClass}__info`} data-enable-match="false">
|
<div className={`${baseClass}__info`} data-enable-match="false">
|
||||||
<strong>{uploadDoc?.filename}</strong>
|
<strong>{filename}</strong>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -1982,6 +1982,41 @@ describe('Versions', () => {
|
|||||||
const hiddenField2 = page.locator('[data-field-path="textCannotRead"]')
|
const hiddenField2 = page.locator('[data-field-path="textCannotRead"]')
|
||||||
await expect(hiddenField2).toBeHidden()
|
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', () => {
|
describe('Scheduled publish', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user