Make collectionId a required field

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 {
+import { keyCollectionDefinition, CollectionComponentType, keyIsCollectionItem } from './constants';
-  keyCollectionDefinition,
-  keyInnerCollectionState,
-  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) {
+  const result = validateCollectionConfig(collectionConfig, componentDef, em);
-    em.logError('The "collectionConfig" property is required in the collection definition.');
+  if (!result) {
-    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.');
     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 }),
+      [collectionId]: collectionState,
-      [keyInnerCollectionState]: 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);

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 {

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];

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';

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;
 };

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',
                   },
                 },
               ],

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',
                             },
                           ],

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",