From d9ef803d203c03a161dedff7076381ed944cf8d6 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 6 Jul 2022 10:52:52 -0700 Subject: [PATCH] fix: ensures old data from arrays is not persisted --- demo/collections/Arrays.ts | 24 +++++++++++++++++++ demo/payload.config.ts | 2 ++ src/collections/tests/collections.spec.ts | 24 ++++++++++--------- .../hooks/beforeChange/getExistingRowDoc.ts | 14 +++++++++++ src/fields/hooks/beforeChange/promise.ts | 9 +++---- 5 files changed, 58 insertions(+), 15 deletions(-) create mode 100644 demo/collections/Arrays.ts create mode 100644 src/fields/hooks/beforeChange/getExistingRowDoc.ts diff --git a/demo/collections/Arrays.ts b/demo/collections/Arrays.ts new file mode 100644 index 0000000000..4917e81d8f --- /dev/null +++ b/demo/collections/Arrays.ts @@ -0,0 +1,24 @@ +import { CollectionConfig } from '../../src/collections/config/types'; + +const Arrays: CollectionConfig = { + slug: 'arrays', + fields: [ + { + name: 'array', + type: 'array', + fields: [ + { + type: 'text', + name: 'required', + required: true, + }, + { + type: 'text', + name: 'optional', + }, + ], + }, + ], +}; + +export default Arrays; diff --git a/demo/payload.config.ts b/demo/payload.config.ts index 9d94fa434d..5f678cec40 100644 --- a/demo/payload.config.ts +++ b/demo/payload.config.ts @@ -5,6 +5,7 @@ import Admin from './collections/Admin'; import AllFields from './collections/AllFields'; import AutoLabel from './collections/AutoLabel'; import Autosave from './collections/Autosave'; +import Arrays from './collections/Arrays'; import Code from './collections/Code'; import Conditions from './collections/Conditions'; // import CustomComponents from './collections/CustomComponents'; @@ -93,6 +94,7 @@ export default buildConfig({ collections: [ Admin, AllFields, + Arrays, AutoLabel, Autosave, Blocks, diff --git a/src/collections/tests/collections.spec.ts b/src/collections/tests/collections.spec.ts index 2254a137e2..0dadd5cec3 100644 --- a/src/collections/tests/collections.spec.ts +++ b/src/collections/tests/collections.spec.ts @@ -9,7 +9,7 @@ const { serverURL: url } = getConfig(); let token = null; let headers = null; -let localizedPostID; +let localizedPost; let relationshipBID; const localizedPostTitle = 'title'; const englishPostDesc = 'english description'; @@ -91,7 +91,7 @@ describe('Collections - REST', () => { expect(data.doc.createdAt).toStrictEqual(expect.stringMatching(timestampRegex)); expect(data.doc.updatedAt).toStrictEqual(expect.stringMatching(timestampRegex)); - localizedPostID = data.doc.id; + localizedPost = data.doc; }); it('should add id to array items', async () => { @@ -170,7 +170,7 @@ describe('Collections - REST', () => { }); it('should allow a Spanish locale to be added to an existing post', async () => { - const response = await fetch(`${url}/api/localized-posts/${localizedPostID}?locale=es`, { + const response = await fetch(`${url}/api/localized-posts/${localizedPost.id}?locale=es`, { body: JSON.stringify({ title: 'title in spanish', description: spanishPostDesc, @@ -183,11 +183,13 @@ describe('Collections - REST', () => { }, nonLocalizedArray: [ { + id: localizedPost.nonLocalizedArray[0].id, localizedEmbeddedText: 'spanish', }, ], richTextBlocks: [ { + id: localizedPost.richTextBlocks[0].id, blockType: 'richTextBlock', blockName: 'Test Block Name', content: [ @@ -216,7 +218,7 @@ describe('Collections - REST', () => { describe('Read', () => { describe('Localized', () => { it('should allow a localized post to be retrieved in unspecified locale, defaulting to English', async () => { - const response = await fetch(`${url}/api/localized-posts/${localizedPostID}`); + const response = await fetch(`${url}/api/localized-posts/${localizedPost.id}`); const data = await response.json(); expect(response.status).toBe(200); @@ -228,7 +230,7 @@ describe('Collections - REST', () => { }); it('should allow a localized post to be retrieved in specified English locale', async () => { - const response = await fetch(`${url}/api/localized-posts/${localizedPostID}?locale=en`); + const response = await fetch(`${url}/api/localized-posts/${localizedPost.id}?locale=en`); const data = await response.json(); expect(response.status).toBe(200); @@ -240,7 +242,7 @@ describe('Collections - REST', () => { }); it('should allow a localized post to be retrieved in Spanish', async () => { - const response = await fetch(`${url}/api/localized-posts/${localizedPostID}?locale=es`); + const response = await fetch(`${url}/api/localized-posts/${localizedPost.id}?locale=es`); const data = await response.json(); expect(response.status).toBe(200); @@ -252,7 +254,7 @@ describe('Collections - REST', () => { }); it('should allow a localized post to be retrieved in all locales', async () => { - const response = await fetch(`${url}/api/localized-posts/${localizedPostID}?locale=all`); + const response = await fetch(`${url}/api/localized-posts/${localizedPost.id}?locale=all`); const data = await response.json(); expect(response.status).toBe(200); @@ -417,7 +419,7 @@ describe('Collections - REST', () => { postLocalizedMultiple: [ { relationTo: 'localized-posts', - value: localizedPostID, + value: localizedPost.id, }, ], }), @@ -442,7 +444,7 @@ describe('Collections - REST', () => { it('should allow querying by a localized nested relationship property', async () => { const res = await fetch(`${url}/api/relationship-a`, { body: JSON.stringify({ - LocalizedPost: [localizedPostID], + LocalizedPost: [localizedPost.id], }), headers, method: 'post', @@ -496,7 +498,7 @@ describe('Collections - REST', () => { title: 'awefjlaiwejfalweiijfaew', nonLocalizedRelationToMany: { relationTo: 'localized-posts', - value: localizedPostID, + value: localizedPost.id, }, }), headers, @@ -507,7 +509,7 @@ describe('Collections - REST', () => { relationshipBID = relationshipB.doc.id; const queryRes = await fetch( - `${url}/api/relationship-b?where[nonLocalizedRelationToMany.value][equals]=${localizedPostID}`, + `${url}/api/relationship-b?where[nonLocalizedRelationToMany.value][equals]=${localizedPost.id}`, ); const data = await queryRes.json(); expect(data.docs).toHaveLength(1); diff --git a/src/fields/hooks/beforeChange/getExistingRowDoc.ts b/src/fields/hooks/beforeChange/getExistingRowDoc.ts new file mode 100644 index 0000000000..84e237d304 --- /dev/null +++ b/src/fields/hooks/beforeChange/getExistingRowDoc.ts @@ -0,0 +1,14 @@ +/** + * If there is an incoming row id, + * and it matches the existing sibling doc id, + * this is an existing row, so it should be merged. + * Otherwise, return an empty object. + */ + +export const getExistingRowDoc = (incomingRow: Record, existingRow?: Record): Record => { + if (incomingRow.id && incomingRow.id === existingRow?.id) { + return existingRow; + } + + return {}; +}; diff --git a/src/fields/hooks/beforeChange/promise.ts b/src/fields/hooks/beforeChange/promise.ts index 9b3fb95aca..8459bc67cf 100644 --- a/src/fields/hooks/beforeChange/promise.ts +++ b/src/fields/hooks/beforeChange/promise.ts @@ -5,6 +5,7 @@ import { Operation } from '../../../types'; import { PayloadRequest } from '../../../express/types'; import getValueWithDefault from '../../getDefaultValue'; import { traverseFields } from './traverseFields'; +import { getExistingRowDoc } from './getExistingRowDoc'; type Args = { data: Record @@ -217,8 +218,8 @@ export const promise = async ({ promises, req, siblingData: row, - siblingDoc: siblingDoc[field.name]?.[i] || {}, - siblingDocWithLocales: siblingDocWithLocales[field.name]?.[i] || {}, + siblingDoc: getExistingRowDoc(row, siblingDoc[field.name]?.[i]), + siblingDocWithLocales: getExistingRowDoc(row, siblingDocWithLocales[field.name]?.[i]), skipValidation: skipValidationFromHere, }); }); @@ -247,8 +248,8 @@ export const promise = async ({ promises, req, siblingData: row, - siblingDoc: siblingDoc[field.name]?.[i] || {}, - siblingDocWithLocales: siblingDocWithLocales[field.name]?.[i] || {}, + siblingDoc: getExistingRowDoc(row, siblingDoc[field.name]?.[i]), + siblingDocWithLocales: getExistingRowDoc(row, siblingDocWithLocales[field.name]?.[i]), skipValidation: skipValidationFromHere, }); }