From d293d5931044a8cc5175b6aabb3a1b3b7b8b6f6e Mon Sep 17 00:00:00 2001 From: mohamedsalem401 Date: Mon, 20 Jan 2025 02:15:41 +0200 Subject: [PATCH] Make collectionId a required field --- .../ComponentDataCollection.ts | 63 ++++++++++++------- .../ComponentDataCollectionVariable.ts | 2 +- .../data_collection/DataCollectionVariable.ts | 4 +- .../model/data_collection/constants.ts | 1 - .../model/data_collection/types.ts | 6 +- .../ComponentDataCollection.ts | 42 +++++++++++++ .../ComponentDataCollectionVariable.ts | 8 +++ .../ComponentDataCollectionVariable.ts.snap | 6 ++ 8 files changed, 104 insertions(+), 28 deletions(-) diff --git a/packages/core/src/data_sources/model/data_collection/ComponentDataCollection.ts b/packages/core/src/data_sources/model/data_collection/ComponentDataCollection.ts index daba285ef..0dff5b698 100644 --- a/packages/core/src/data_sources/model/data_collection/ComponentDataCollection.ts +++ b/packages/core/src/data_sources/model/data_collection/ComponentDataCollection.ts @@ -1,7 +1,7 @@ import DataVariable, { DataVariableType } from '../DataVariable'; import { isArray } from 'underscore'; import Component from '../../../dom_components/model/Component'; -import { ComponentOptions } from '../../../dom_components/model/types'; +import { ComponentDefinition, ComponentOptions } from '../../../dom_components/model/types'; import { toLowerCase } from '../../../utils/mixins'; import DataSource from '../DataSource'; import { ObjectAny } from '../../../common'; @@ -9,16 +9,12 @@ import EditorModel from '../../../editor/model/Editor'; import { keyCollectionsStateMap } from '../../../dom_components/model/Component'; import { ComponentDataCollectionDefinition, + DataCollectionConfig, DataCollectionDefinition, DataCollectionState, DataCollectionStateMap, } from './types'; -import { - keyCollectionDefinition, - keyInnerCollectionState, - CollectionComponentType, - keyIsCollectionItem, -} from './constants'; +import { keyCollectionDefinition, CollectionComponentType, keyIsCollectionItem } from './constants'; import DynamicVariableListenerManager from '../DataVariableListenerManager'; export default class ComponentDataCollection extends Component { @@ -114,18 +110,8 @@ function getCollectionItems( opt: ComponentOptions, ) { const { componentDef, collectionConfig } = collectionDef; - if (!collectionConfig) { - em.logError('The "collectionConfig" property is required in the collection definition.'); - return []; - } - - if (!componentDef) { - em.logError('The "componentDef" property is required in the collection definition.'); - return []; - } - - if (!collectionConfig?.dataSource) { - em.logError('The "collectionConfig.dataSource" property is required in the collection definition.'); + const result = validateCollectionConfig(collectionConfig, componentDef, em); + if (!result) { return []; } @@ -154,10 +140,16 @@ function getCollectionItems( remainingItems: totalItems - (index + 1), }; + if (parentCollectionStateMap[collectionId]) { + em.logError( + `The collection ID "${collectionId}" already exists in the parent collection state. Overriding it is not allowed.`, + ); + return []; + } + const collectionsStateMap: DataCollectionStateMap = { ...parentCollectionStateMap, - ...(collectionId && { [collectionId]: collectionState }), - [keyInnerCollectionState]: collectionState, + [collectionId]: collectionState, }; if (index === startIndex) { @@ -226,6 +218,35 @@ function setCollectionStateMapAndPropagate( }; } +function logErrorIfMissing(property: any, propertyPath: string, em: EditorModel) { + if (!property) { + em.logError(`The "${propertyPath}" property is required in the collection definition.`); + return false; + } + return true; +} + +function validateCollectionConfig( + collectionConfig: DataCollectionConfig, + componentDef: ComponentDefinition, + em: EditorModel, +) { + const validations = [ + { property: collectionConfig, propertyPath: 'collectionConfig' }, + { property: componentDef, propertyPath: 'componentDef' }, + { property: collectionConfig?.collectionId, propertyPath: 'collectionConfig.collectionId' }, + { property: collectionConfig?.dataSource, propertyPath: 'collectionConfig.dataSource' }, + ]; + + for (const { property, propertyPath } of validations) { + if (!logErrorIfMissing(property, propertyPath, em)) { + return []; + } + } + + return true; +} + function setCollectionStateMap(collectionsStateMap: DataCollectionStateMap) { return (cmp: Component) => { cmp.set(keyIsCollectionItem, true); diff --git a/packages/core/src/data_sources/model/data_collection/ComponentDataCollectionVariable.ts b/packages/core/src/data_sources/model/data_collection/ComponentDataCollectionVariable.ts index 14e2e6a43..78fba3d5a 100644 --- a/packages/core/src/data_sources/model/data_collection/ComponentDataCollectionVariable.ts +++ b/packages/core/src/data_sources/model/data_collection/ComponentDataCollectionVariable.ts @@ -2,7 +2,7 @@ import Component, { keyCollectionsStateMap, keySymbolOvrd } from '../../../dom_c import { ComponentOptions, ComponentProperties } from '../../../dom_components/model/types'; import { toLowerCase } from '../../../utils/mixins'; import DataCollectionVariable from './DataCollectionVariable'; -import { CollectionVariableType, keyInnerCollectionState } from './constants'; +import { CollectionVariableType } from './constants'; import { DataCollectionStateMap, DataCollectionVariableDefinition } from './types'; export default class ComponentDataCollectionVariable extends Component { diff --git a/packages/core/src/data_sources/model/data_collection/DataCollectionVariable.ts b/packages/core/src/data_sources/model/data_collection/DataCollectionVariable.ts index 6ee2422f0..4132c61c5 100644 --- a/packages/core/src/data_sources/model/data_collection/DataCollectionVariable.ts +++ b/packages/core/src/data_sources/model/data_collection/DataCollectionVariable.ts @@ -2,7 +2,7 @@ import { DataCollectionVariableDefinition } from './types'; import { Model } from '../../../common'; import EditorModel from '../../../editor/model/Editor'; import DataVariable, { DataVariableType } from '../DataVariable'; -import { CollectionVariableType, keyInnerCollectionState } from './constants'; +import { CollectionVariableType } from './constants'; import { DataCollectionState, DataCollectionStateMap } from './types'; import DynamicVariableListenerManager from '../DataVariableListenerManager'; type ResolvedDataCollectionVariable = DataCollectionVariableDefinition & { @@ -106,7 +106,7 @@ function resolveCollectionVariable( collectionsStateMap: DataCollectionStateMap, em: EditorModel, ) { - const { collectionId = keyInnerCollectionState, variableType, path } = collectionVariableDefinition; + const { collectionId, variableType, path } = collectionVariableDefinition; if (!collectionsStateMap) return; const collectionItem = collectionsStateMap[collectionId]; diff --git a/packages/core/src/data_sources/model/data_collection/constants.ts b/packages/core/src/data_sources/model/data_collection/constants.ts index 590635f5d..e3193802a 100644 --- a/packages/core/src/data_sources/model/data_collection/constants.ts +++ b/packages/core/src/data_sources/model/data_collection/constants.ts @@ -1,5 +1,4 @@ export const CollectionComponentType = 'collection-component'; export const keyCollectionDefinition = 'collectionDef'; -export const keyInnerCollectionState = 'innerCollectionState'; export const keyIsCollectionItem = '__is_collection_item'; export const CollectionVariableType = 'parent-collection-variable'; diff --git a/packages/core/src/data_sources/model/data_collection/types.ts b/packages/core/src/data_sources/model/data_collection/types.ts index 6246aeddd..a6793f59c 100644 --- a/packages/core/src/data_sources/model/data_collection/types.ts +++ b/packages/core/src/data_sources/model/data_collection/types.ts @@ -5,7 +5,7 @@ import { DataVariableDefinition } from '../DataVariable'; export type DataCollectionDataSource = any[] | DataVariableDefinition | DataCollectionVariableDefinition; export interface DataCollectionConfig { - collectionId?: string; + collectionId: string; startIndex?: number; endIndex?: number; dataSource: DataCollectionDataSource; @@ -26,7 +26,7 @@ export interface DataCollectionState { [DataCollectionStateVariableType.startIndex]: number; [DataCollectionStateVariableType.currentItem]: any; [DataCollectionStateVariableType.endIndex]: number; - [DataCollectionStateVariableType.collectionId]?: string; + [DataCollectionStateVariableType.collectionId]: string; [DataCollectionStateVariableType.totalItems]: number; [DataCollectionStateVariableType.remainingItems]: number; } @@ -48,6 +48,6 @@ export interface DataCollectionDefinition { export type DataCollectionVariableDefinition = { type: typeof CollectionVariableType; variableType: DataCollectionStateVariableType; - collectionId?: string; + collectionId: string; path?: string; }; diff --git a/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollection.ts b/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollection.ts index 3506399d4..8a6c05fdd 100644 --- a/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollection.ts +++ b/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollection.ts @@ -47,6 +47,7 @@ describe('Collection component', () => { type: 'default', }, collectionConfig: { + collectionId: 'my_collection', dataSource: { type: DataVariableType, path: 'my_data_source_id', @@ -66,6 +67,7 @@ describe('Collection component', () => { type: 'default', }, collectionConfig: { + collectionId: 'my_collection', dataSource: { type: DataVariableType, path: 'my_data_source_id', @@ -92,6 +94,7 @@ describe('Collection component', () => { ], }, collectionConfig: { + collectionId: 'my_collection', dataSource: { type: DataVariableType, path: 'my_data_source_id', @@ -131,6 +134,7 @@ describe('Collection component', () => { name: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, @@ -138,15 +142,18 @@ describe('Collection component', () => { name: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, custom_property: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, collectionConfig: { + collectionId: 'my_collection', dataSource: { type: DataVariableType, path: 'my_data_source_id', @@ -206,6 +213,7 @@ describe('Collection component', () => { // @ts-ignore type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'age', }); expect(firstChild.get('name')).toBe('12'); @@ -287,6 +295,7 @@ describe('Collection component', () => { name: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, @@ -296,11 +305,13 @@ describe('Collection component', () => { name: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, }, collectionConfig: { + collectionId: 'my_collection', dataSource: { type: DataVariableType, path: 'my_data_source_id', @@ -377,6 +388,7 @@ describe('Collection component', () => { // @ts-ignore type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'age', }, }); @@ -436,6 +448,7 @@ describe('Collection component', () => { value: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, @@ -445,12 +458,14 @@ describe('Collection component', () => { value: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, ], }, collectionConfig: { + collectionId: 'my_collection', dataSource: { type: DataVariableType, path: 'my_data_source_id', @@ -487,17 +502,20 @@ describe('Collection component', () => { name: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, custom_prop: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentIndex, + collectionId: 'my_collection', path: 'user', }, attributes: { name: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, @@ -507,6 +525,7 @@ describe('Collection component', () => { value: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, @@ -516,6 +535,7 @@ describe('Collection component', () => { value: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, @@ -553,6 +573,7 @@ describe('Collection component', () => { name: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentIndex, + collectionId: 'my_collection', path: 'user', }, }; @@ -574,6 +595,7 @@ describe('Collection component', () => { name: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentIndex, + collectionId: 'my_collection', path: 'user', }, }; @@ -602,6 +624,7 @@ describe('Collection component', () => { name: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, }, @@ -611,27 +634,32 @@ describe('Collection component', () => { attribute_trait: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, name: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, }, name: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, custom_prop: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: 'currentIndex', }, property_trait: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, type: 'default', @@ -641,27 +669,32 @@ describe('Collection component', () => { attribute_trait: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, name: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, }, name: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, custom_prop: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: 'currentIndex', }, property_trait: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, type: 'default', @@ -670,16 +703,19 @@ describe('Collection component', () => { name: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, custom_prop: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: 'currentIndex', }, property_trait: { path: 'user', type: CollectionVariableType, + collectionId: 'my_collection', variableType: DataCollectionStateVariableType.currentItem, }, type: 'default', @@ -767,12 +803,14 @@ describe('Collection component', () => { name: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, collectionConfig: { startIndex: 1, endIndex: 2, + collectionId: 'my_collection', dataSource: { type: DataVariableType, path: 'my_data_source_id', @@ -813,11 +851,13 @@ describe('Collection component', () => { name: { type: CollectionVariableType, variableType: variableType, + collectionId: 'my_collection', }, attributes: { custom_attribute: { type: CollectionVariableType, variableType: variableType, + collectionId: 'my_collection', }, }, traits: [ @@ -826,6 +866,7 @@ describe('Collection component', () => { value: { type: CollectionVariableType, variableType: variableType, + collectionId: 'my_collection', }, }, { @@ -834,6 +875,7 @@ describe('Collection component', () => { value: { type: CollectionVariableType, variableType: variableType, + collectionId: 'my_collection', }, }, ], diff --git a/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollectionVariable.ts b/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollectionVariable.ts index 8ee78f8b7..3fbd07b29 100644 --- a/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollectionVariable.ts +++ b/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollectionVariable.ts @@ -48,11 +48,13 @@ describe('Collection variable components', () => { { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, ], }, collectionConfig: { + collectionId: 'my_collection', dataSource: { type: DataVariableType, path: 'my_data_source_id', @@ -77,10 +79,12 @@ describe('Collection variable components', () => { components: { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, }, collectionConfig: { + collectionId: 'my_collection', dataSource: { type: DataVariableType, path: 'my_data_source_id', @@ -104,6 +108,7 @@ describe('Collection variable components', () => { const variableCmpDef = { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }; @@ -141,6 +146,7 @@ describe('Collection variable components', () => { const newChildDefinition = { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentIndex, + collectionId: 'my_collection', path: 'user', }; firstChild.components().at(0).components(newChildDefinition); @@ -159,6 +165,7 @@ describe('Collection variable components', () => { const newChildDefinition = { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentIndex, + collectionId: 'my_collection', path: 'user', }; @@ -183,6 +190,7 @@ describe('Collection variable components', () => { { type: CollectionVariableType, variableType: DataCollectionStateVariableType.currentItem, + collectionId: 'my_collection', path: 'user', }, ], diff --git a/packages/core/test/specs/data_sources/model/data_collection/__snapshots__/ComponentDataCollectionVariable.ts.snap b/packages/core/test/specs/data_sources/model/data_collection/__snapshots__/ComponentDataCollectionVariable.ts.snap index ba96ea6e8..e22d545fc 100644 --- a/packages/core/test/specs/data_sources/model/data_collection/__snapshots__/ComponentDataCollectionVariable.ts.snap +++ b/packages/core/test/specs/data_sources/model/data_collection/__snapshots__/ComponentDataCollectionVariable.ts.snap @@ -18,6 +18,7 @@ exports[`Collection variable components Serialization Saving: Collection with co "type": "default", }, { + "collectionId": "my_collection", "path": "user", "type": "parent-collection-variable", "variableType": "currentItem", @@ -47,6 +48,7 @@ exports[`Collection variable components Serialization Saving: Collection with co { "components": [ { + "collectionId": "my_collection", "path": "user", "type": "parent-collection-variable", "variableType": "currentIndex", @@ -55,6 +57,7 @@ exports[`Collection variable components Serialization Saving: Collection with co "type": "default", }, { + "collectionId": "my_collection", "path": "user", "type": "parent-collection-variable", "variableType": "currentItem", @@ -85,6 +88,7 @@ exports[`Collection variable components Serialization Serializion to JSON: Colle "type": "default", }, { + "collectionId": "my_collection", "path": "user", "type": "parent-collection-variable", "variableType": "currentItem", @@ -114,6 +118,7 @@ exports[`Collection variable components Serialization Serializion to JSON: Colle { "components": [ { + "collectionId": "my_collection", "path": "user", "type": "parent-collection-variable", "variableType": "currentIndex", @@ -122,6 +127,7 @@ exports[`Collection variable components Serialization Serializion to JSON: Colle "type": "default", }, { + "collectionId": "my_collection", "path": "user", "type": "parent-collection-variable", "variableType": "currentItem",