From e19c0f92cbc6e3400349044e4f821eceabd952be Mon Sep 17 00:00:00 2001 From: Josh <37798644+joshuaellis@users.noreply.github.com> Date: Tue, 21 Mar 2023 10:08:54 +0000 Subject: [PATCH 1/2] fix: do not populate a single component if it doesn't exist in initialdata --- .../content-types/relation-locale/schema.json | 21 +++++++++++++++++++ .../EditViewDataManagerProvider/reducer.js | 4 +++- .../tests/reducer.test.js | 1 - .../utils/findAllAndReplace.js | 13 +++++++++--- .../utils/tests/findAllAndReplace.test.js | 17 +-------------- 5 files changed, 35 insertions(+), 21 deletions(-) diff --git a/examples/getstarted/src/api/relation-locale/content-types/relation-locale/schema.json b/examples/getstarted/src/api/relation-locale/content-types/relation-locale/schema.json index d82bcb0fbf..021b8a7c13 100644 --- a/examples/getstarted/src/api/relation-locale/content-types/relation-locale/schema.json +++ b/examples/getstarted/src/api/relation-locale/content-types/relation-locale/schema.json @@ -51,6 +51,27 @@ "basic.relation", "basic.simple" ] + }, + "single_relation": { + "type": "component", + "repeatable": false, + "pluginOptions": { + "i18n": { + "localized": true + } + }, + "component": "basic.relation" + }, + "require_single_relation": { + "type": "component", + "repeatable": false, + "pluginOptions": { + "i18n": { + "localized": true + } + }, + "component": "basic.relation", + "required": true } } } diff --git a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/reducer.js b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/reducer.js index 8029a2d853..7afdc42522 100644 --- a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/reducer.js +++ b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/reducer.js @@ -220,7 +220,9 @@ const reducer = (state, action) => const findAllRelationsAndReplaceWithEmptyArray = findAllAndReplace( components, - (value) => value.type === 'relation', + (value) => { + return value.type === 'relation'; + }, (_, { path }) => { if (state.modifiedData?.id === data.id && get(state.modifiedData, path)) { return get(state.modifiedData, path); diff --git a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/tests/reducer.test.js b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/tests/reducer.test.js index c2956e32b0..73298c7e76 100644 --- a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/tests/reducer.test.js +++ b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/tests/reducer.test.js @@ -571,7 +571,6 @@ describe('CONTENT MANAGER | COMPONENTS | EditViewDataManagerProvider | reducer', level_one_repeatable: [ { __temp_key__: 0, - level_two_single_component: {}, }, ], }, diff --git a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/findAllAndReplace.js b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/findAllAndReplace.js index 390b85a567..f5bf9fbdb9 100644 --- a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/findAllAndReplace.js +++ b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/findAllAndReplace.js @@ -26,7 +26,11 @@ const findAllAndReplaceSetup = (components, predicate = () => false, replacement /** * @type {(data: TData, attributes: Attributes, options?: { ignoreFalseyValues?: boolean}) => TData} */ - const findAllAndReplace = (data, attributes, { ignoreFalseyValues = false, path = [] } = {}) => { + const findAllAndReplace = ( + data, + attributes, + { ignoreFalseyValues = false, path = [], parent = attributes } = {} + ) => { return Object.entries(attributes).reduce( (acc, [key, value]) => { if ( @@ -36,7 +40,7 @@ const findAllAndReplaceSetup = (components, predicate = () => false, replacement return acc; } - if (predicate(value, { path: [...path, key], parent: acc })) { + if (predicate(value, { path: [...path, key], parent })) { acc[key] = typeof replacement === 'function' ? replacement(acc[key], { path: [...path, key], parent: acc }) @@ -46,16 +50,18 @@ const findAllAndReplaceSetup = (components, predicate = () => false, replacement if (value.type === 'component') { const componentAttributes = components[value.component].attributes; - if (!value.repeatable) { + if (!value.repeatable && typeof acc[key] === 'object') { acc[key] = findAllAndReplace(acc[key], componentAttributes, { ignoreFalseyValues, path: [...path, key], + parent: attributes[key], }); } else if (value.repeatable && Array.isArray(acc[key])) { acc[key] = acc[key].map((datum, index) => { const data = findAllAndReplace(datum, componentAttributes, { ignoreFalseyValues, path: [...path, key, index], + parent: attributes[key], }); return data; @@ -67,6 +73,7 @@ const findAllAndReplaceSetup = (components, predicate = () => false, replacement const data = findAllAndReplace(datum, componentAttributes, { ignoreFalseyValues, path: [...path, key, index], + parent: attributes[key], }); return data; diff --git a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/tests/findAllAndReplace.test.js b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/tests/findAllAndReplace.test.js index b035f4604d..0552e6cea6 100644 --- a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/tests/findAllAndReplace.test.js +++ b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/tests/findAllAndReplace.test.js @@ -69,14 +69,11 @@ describe('findAllAndReplace', () => { expect(data).toMatchInlineSnapshot(` { "categories": "replaced", - "comp_relation": { - "categories": "replaced", - }, } `); }); - it('should replace relations in single components', () => { + it('should not replace relations in single components', () => { const data = findAllAndReplace( components, (value) => value.type === 'relation', @@ -86,9 +83,6 @@ describe('findAllAndReplace', () => { expect(data).toMatchInlineSnapshot(` { "categories": "replaced", - "comp_relation": { - "categories": "replaced", - }, } `); }); @@ -103,9 +97,6 @@ describe('findAllAndReplace', () => { expect(data).toMatchInlineSnapshot(` { "categories": "replaced", - "comp_relation": { - "categories": "replaced", - }, } `); }); @@ -120,9 +111,6 @@ describe('findAllAndReplace', () => { expect(data).toMatchInlineSnapshot(` { "categories": "replaced", - "comp_relation": { - "categories": "replaced", - }, } `); }); @@ -137,9 +125,6 @@ describe('findAllAndReplace', () => { expect(data).toMatchInlineSnapshot(` { "categories": "replaced", - "comp_relation": { - "categories": "replaced", - }, } `); }); From 687a421933ff0b30ffe62e66d51d60b35131d2bd Mon Sep 17 00:00:00 2001 From: Josh <37798644+joshuaellis@users.noreply.github.com> Date: Tue, 21 Mar 2023 12:56:34 +0000 Subject: [PATCH 2/2] fix: nullish values should only be replaced when they're top level attributes --- .../utils/findAllAndReplace.js | 2 +- .../utils/tests/findAllAndReplace.test.js | 100 ++++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/findAllAndReplace.js b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/findAllAndReplace.js index f5bf9fbdb9..ca7c486f5a 100644 --- a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/findAllAndReplace.js +++ b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/findAllAndReplace.js @@ -50,7 +50,7 @@ const findAllAndReplaceSetup = (components, predicate = () => false, replacement if (value.type === 'component') { const componentAttributes = components[value.component].attributes; - if (!value.repeatable && typeof acc[key] === 'object') { + if (!value.repeatable && acc[key] && typeof acc[key] === 'object') { acc[key] = findAllAndReplace(acc[key], componentAttributes, { ignoreFalseyValues, path: [...path, key], diff --git a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/tests/findAllAndReplace.test.js b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/tests/findAllAndReplace.test.js index 0552e6cea6..a85dc7e4d3 100644 --- a/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/tests/findAllAndReplace.test.js +++ b/packages/core/admin/admin/src/content-manager/components/EditViewDataManagerProvider/utils/tests/findAllAndReplace.test.js @@ -130,6 +130,106 @@ describe('findAllAndReplace', () => { }); }); + describe('null values', () => { + const nullishData = { + categories: null, + repeatable_repeatable_relations: null, + repeatable_relations: null, + dynamic_relations: null, + comp_relation: null, + }; + + it('should replace the first level of relations', () => { + const data = findAllAndReplace( + components, + (value) => value.type === 'relation', + 'replaced' + )(nullishData, schema); + + expect(data).toMatchInlineSnapshot(` + { + "categories": "replaced", + "comp_relation": null, + "dynamic_relations": null, + "repeatable_relations": null, + "repeatable_repeatable_relations": null, + } + `); + }); + + it('should not replace relations in single components', () => { + const data = findAllAndReplace( + components, + (value) => value.type === 'relation', + 'replaced' + )(nullishData, schema); + + expect(data).toMatchInlineSnapshot(` + { + "categories": "replaced", + "comp_relation": null, + "dynamic_relations": null, + "repeatable_relations": null, + "repeatable_repeatable_relations": null, + } + `); + }); + + it('should not replace relation instances in a repeatable component', () => { + const data = findAllAndReplace( + components, + (value) => value.type === 'relation', + 'replaced' + )(nullishData, schema); + + expect(data).toMatchInlineSnapshot(` + { + "categories": "replaced", + "comp_relation": null, + "dynamic_relations": null, + "repeatable_relations": null, + "repeatable_repeatable_relations": null, + } + `); + }); + + it('should not replace all relation instances in nested repeatable components', () => { + const data = findAllAndReplace( + components, + (value) => value.type === 'relation', + 'replaced' + )(nullishData, schema); + + expect(data).toMatchInlineSnapshot(` + { + "categories": "replaced", + "comp_relation": null, + "dynamic_relations": null, + "repeatable_relations": null, + "repeatable_repeatable_relations": null, + } + `); + }); + + it('should not replace relation instances in dynamic zones correctly', () => { + const data = findAllAndReplace( + components, + (value) => value.type === 'relation', + 'replaced' + )(nullishData, schema); + + expect(data).toMatchInlineSnapshot(` + { + "categories": "replaced", + "comp_relation": null, + "dynamic_relations": null, + "repeatable_relations": null, + "repeatable_repeatable_relations": null, + } + `); + }); + }); + describe('replacement as a function', () => { it('should pass the data object (not the schema) and use the returned value', () => { const data = findAllAndReplace(