From fc019079e0c889b05931e9aedf261eaac8b5514b Mon Sep 17 00:00:00 2001 From: mohamedsalem401 Date: Thu, 9 Jan 2025 23:49:04 +0200 Subject: [PATCH] Fix Collection props and attributes propagation --- .../model/collection_component/types.ts | 2 +- .../src/dom_components/model/Component.ts | 26 ++++-- .../model/ComponentDynamicValueWatcher.ts | 26 ++++-- .../src/dom_components/model/Components.ts | 18 ++-- .../model/DynamicValueWatcher.ts | 23 +++++ .../src/dom_components/model/SymbolUtils.ts | 83 ++++++++++--------- .../core/src/dom_components/model/types.ts | 2 + .../CollectionComponent.ts | 58 ++++++++++++- 8 files changed, 173 insertions(+), 65 deletions(-) diff --git a/packages/core/src/data_sources/model/collection_component/types.ts b/packages/core/src/data_sources/model/collection_component/types.ts index e670974fb..e7cc14676 100644 --- a/packages/core/src/data_sources/model/collection_component/types.ts +++ b/packages/core/src/data_sources/model/collection_component/types.ts @@ -1,6 +1,6 @@ import { CollectionComponentType, keyCollectionDefinition } from './constants'; -import { ComponentDefinition, ComponentProperties } from '../../../dom_components/model/types'; +import { ComponentDefinition } from '../../../dom_components/model/types'; import { CollectionVariableDefinition } from '../../../../test/specs/dom_components/model/ComponentTypes'; import { DataVariableDefinition } from '../DataVariable'; diff --git a/packages/core/src/dom_components/model/Component.ts b/packages/core/src/dom_components/model/Component.ts index c20e69708..2a10cc718 100644 --- a/packages/core/src/dom_components/model/Component.ts +++ b/packages/core/src/dom_components/model/Component.ts @@ -300,8 +300,7 @@ export default class Component extends StyleableModel { opt.em = em; this.opt = { ...opt, - // @ts-ignore - [keyCollectionsStateMap]: props[keyCollectionsStateMap], + collectionsStateMap: props[keyCollectionsStateMap], isCollectionItem: !!props['isCollectionItem'], }; this.em = em!; @@ -320,6 +319,7 @@ export default class Component extends StyleableModel { this.listenTo(this, 'change:tagName', this.tagUpdated); this.listenTo(this, 'change:attributes', this.attrUpdated); this.listenTo(this, 'change:attributes:id', this._idUpdated); + this.listenTo(this, `change:${keyCollectionsStateMap}`, this._collectionsStateUpdated); this.on('change:toolbar', this.__emitUpdateTlb); this.on('change', this.__onChange); this.on(keyUpdateInside, this.__propToParent); @@ -347,6 +347,16 @@ export default class Component extends StyleableModel { } } + getCollectionStateMap(): CollectionsStateMap { + const collectionStateMapProp = this.get(keyCollectionsStateMap); + if (collectionStateMapProp) { + return collectionStateMapProp; + } + + const parent = this.parent() || this.opt.parent; + return parent?.getCollectionStateMap() || {}; + } + set( keyOrAttributes: A | Partial, valueOrOptions?: ComponentProperties[A] | ComponentSetOptions, @@ -1595,6 +1605,7 @@ export default class Component extends StyleableModel { delete obj.open; // used in Layers delete obj._undoexc; delete obj.delegate; + delete obj[keyCollectionsStateMap]; if (!opts.fromUndo) { const symbol = obj[keySymbol]; @@ -1661,9 +1672,7 @@ export default class Component extends StyleableModel { * @return {this} */ setId(id: string, opts?: SetOptions & { idUpdate?: boolean }) { - const attrs = { ...this.get('attributes') }; - attrs.id = id; - this.set('attributes', attrs, opts); + this.addAttributes({ id }); return this; } @@ -1966,6 +1975,13 @@ export default class Component extends StyleableModel { selector && selector.set({ name: id, label: id }); } + _collectionsStateUpdated(m: any, v: CollectionsStateMap, opts = {}) { + this.componentDVListener.updateCollectionStateMap(v); + this.components().forEach((child) => { + child.set(keyCollectionsStateMap, v); + }); + } + static typeExtends = new Set(); static getDefaults() { diff --git a/packages/core/src/dom_components/model/ComponentDynamicValueWatcher.ts b/packages/core/src/dom_components/model/ComponentDynamicValueWatcher.ts index 3c465dec7..016f521bd 100644 --- a/packages/core/src/dom_components/model/ComponentDynamicValueWatcher.ts +++ b/packages/core/src/dom_components/model/ComponentDynamicValueWatcher.ts @@ -2,7 +2,7 @@ import { ObjectAny } from '../../common'; import { CollectionVariableType } from '../../data_sources/model/collection_component/constants'; import { CollectionsStateMap } from '../../data_sources/model/collection_component/types'; import EditorModel from '../../editor/model/Editor'; -import Component from './Component'; +import Component, { keyCollectionsStateMap } from './Component'; import { DynamicWatchersOptions } from './DynamicValueWatcher'; import { DynamicValueWatcher } from './DynamicValueWatcher'; import { getSymbolsToUpdate } from './SymbolUtils'; @@ -43,10 +43,15 @@ export class ComponentDynamicValueWatcher { this.updateSymbolOverride(); } + updateCollectionStateMap(collectionsStateMap: CollectionsStateMap) { + this.propertyWatcher.updateCollectionStateMap(collectionsStateMap); + this.attributeWatcher.updateCollectionStateMap(collectionsStateMap); + } + addProps(props: ObjectAny, options: DynamicWatchersOptions = {}) { const evaluatedProps = this.propertyWatcher.addDynamicValues(props, options); if (props.attributes) { - const evaluatedAttributes = this.attributeWatcher.setDynamicValues({ ...props.attributes }, options); + const evaluatedAttributes = this.attributeWatcher.setDynamicValues(props.attributes, options); evaluatedProps['attributes'] = evaluatedAttributes; } @@ -63,17 +68,20 @@ export class ComponentDynamicValueWatcher { this.updateSymbolOverride(); } - updateSymbolOverride() { + private updateSymbolOverride() { if (!this.component || !this.component.get('isCollectionItem')) return; - + const keys = this.propertyWatcher.getDynamicValuesOfType(CollectionVariableType); const attributesKeys = this.attributeWatcher.getDynamicValuesOfType(CollectionVariableType); - - const combinedKeys = [...keys]; + + const combinedKeys = [keyCollectionsStateMap, ...keys]; const haveOverridenAttributes = Object.keys(attributesKeys).length; if (haveOverridenAttributes) combinedKeys.push('attributes'); - if (!combinedKeys.length && !this.component.getSymbolOverride()) return; + const toUp = getSymbolsToUpdate(this.component); + toUp.forEach((child) => { + child.setSymbolOverride(combinedKeys, { fromDataSource: true }); + }); this.component.setSymbolOverride(combinedKeys, { fromDataSource: true }); } @@ -85,6 +93,10 @@ export class ComponentDynamicValueWatcher { return this.attributeWatcher.getAllSerializableValues(); } + getPropsDefsOrValues(props: ObjectAny) { + return this.propertyWatcher.getSerializableValues(props); + } + getAttributesDefsOrValues(attributes: ObjectAny) { return this.attributeWatcher.getSerializableValues(attributes); } diff --git a/packages/core/src/dom_components/model/Components.ts b/packages/core/src/dom_components/model/Components.ts index 1c6635b8c..964478ca6 100644 --- a/packages/core/src/dom_components/model/Components.ts +++ b/packages/core/src/dom_components/model/Components.ts @@ -346,13 +346,7 @@ Component> { let model = mdl; if (processor) { - model = { - [keyCollectionsStateMap]: { - ...this.opt.collectionsStateMap, - }, - isCollectionItem: this.opt.isCollectionItem, - ...model, - }; // Avoid 'Cannot delete property ...' + model = { ...model }; // Avoid 'Cannot delete property ...' const modelPr = processor(model); if (modelPr) { //@ts-ignore @@ -394,7 +388,15 @@ Component> { extend(model, res.props); } - return model; + return { + ...(this.opt.isCollectionItem && { + isCollectionItem: this.opt.isCollectionItem, + [keyCollectionsStateMap]: { + ...this.opt.collectionsStateMap, + }, + }), + ...model, + }; } onAdd(model: Component, c?: any, opts: { temporary?: boolean } = {}) { diff --git a/packages/core/src/dom_components/model/DynamicValueWatcher.ts b/packages/core/src/dom_components/model/DynamicValueWatcher.ts index 2a090fe3f..27229b7c9 100644 --- a/packages/core/src/dom_components/model/DynamicValueWatcher.ts +++ b/packages/core/src/dom_components/model/DynamicValueWatcher.ts @@ -5,6 +5,7 @@ import DynamicVariableListenerManager from '../../data_sources/model/DataVariabl import { evaluateDynamicValueDefinition, isDynamicValueDefinition } from '../../data_sources/model/utils'; import EditorModel from '../../editor/model/Editor'; import Component from './Component'; +import { CollectionVariableType } from '../../data_sources/model/collection_component/constants'; export interface DynamicWatchersOptions { skipWatcherUpdates?: boolean; @@ -26,6 +27,28 @@ export class DynamicValueWatcher { this.component = component; } + updateCollectionStateMap(collectionsStateMap: CollectionsStateMap) { + this.options = { + ...this.options, + collectionsStateMap, + }; + + const collectionVariablesKeys = this.getDynamicValuesOfType(CollectionVariableType); + const collectionVariablesObject = collectionVariablesKeys.reduce( + (acc: { [key: string]: DynamicValueDefinition | null }, key) => { + acc[key] = null; + return acc; + }, + {}, + ); + const newVariables = this.getSerializableValues(collectionVariablesObject); + const evaluatedValues = this.addDynamicValues(newVariables); + + Object.keys(evaluatedValues).forEach((key) => { + this.updateFn(this.component, key, evaluatedValues[key]); + }); + } + setDynamicValues(values: ObjectAny | undefined, options: DynamicWatchersOptions = {}) { const shouldSkipWatcherUpdates = options.skipWatcherUpdates || options.fromDataSource; if (!shouldSkipWatcherUpdates) { diff --git a/packages/core/src/dom_components/model/SymbolUtils.ts b/packages/core/src/dom_components/model/SymbolUtils.ts index 6f14c2336..60708c56c 100644 --- a/packages/core/src/dom_components/model/SymbolUtils.ts +++ b/packages/core/src/dom_components/model/SymbolUtils.ts @@ -130,55 +130,58 @@ export const logSymbol = (symb: Component, type: string, toUp: Component[], opts symb.em.log(type, { model: symb, toUp, context: 'symbols', opts }); }; -export const updateSymbolProps = (symbol: Component, opts: SymbolToUpOptions = {}) => { - const changed = { - ...(symbol.changedAttributes() || {}), - ...symbol.componentDVListener.getDynamicPropsDefs(), - }; - const attrs = { - ...(changed.attributes || {}), - ...symbol.componentDVListener.getDynamicAttributesDefs(), - }; - delete changed.status; - delete changed.open; - const toUp = getSymbolsToUpdate(symbol, opts); - if ( - symbol.get('isCollectionItem') && - !(Object.keys(changed).length === 1 && Object.keys(changed)[0] === keySymbolOvrd) - ) { - toUp.forEach((child) => { - child.setSymbolOverride(symbol.getSymbolOverride() || [], { fromDataSource: true }); +export const updateSymbolProps = (symbol: Component, opts: SymbolToUpOptions = {}): void => { + const changed = symbol.componentDVListener.getPropsDefsOrValues({ ...symbol.changedAttributes() } || {}); + const attrs = symbol.componentDVListener.getAttributesDefsOrValues({ ...changed.attributes } || {}); + + cleanChangedProperties(changed, attrs); + + if (!isEmptyObj(changed)) { + const toUpdate = getSymbolsToUpdate(symbol, opts); + + // Filter properties to propagate + filterPropertiesForPropagation(changed, symbol); + + logSymbol(symbol, 'props', toUpdate, { opts, changed }); + + // Update child symbols + toUpdate.forEach((child) => { + const propsToUpdate = { ...changed }; + filterPropertiesForPropagation(propsToUpdate, child); + child.set(propsToUpdate, { fromInstance: symbol, ...opts }); }); } - delete changed[keySymbols]; - delete changed[keySymbol]; - delete changed[keySymbolOvrd]; - delete changed.attributes; +}; + +const cleanChangedProperties = (changed: Record, attrs: Record): void => { + const keysToDelete = ['status', 'open', keySymbols, keySymbol, keySymbolOvrd, 'attributes']; + keysToDelete.forEach((key) => delete changed[key]); delete attrs.id; if (!isEmptyObj(attrs)) { changed.attributes = attrs; } +}; - if (!isEmptyObj(changed)) { - const toUp = getSymbolsToUpdate(symbol, opts); - // Avoid propagating overrides to other symbols - keys(changed).map((prop) => { - const shouldPropagate = isSymbolOverride(symbol, prop) && !(changed[prop].type === CollectionVariableType); - if (shouldPropagate) delete changed[prop]; - }); +const filterPropertiesForPropagation = (props: Record, component: Component): void => { + keys(props).forEach((prop) => { + if (!shouldPropagateProperty(props, prop, component)) { + delete props[prop]; + } + }); +}; - logSymbol(symbol, 'props', toUp, { opts, changed }); - toUp.forEach((child) => { - const propsChanged = { ...changed }; - // Avoid updating those with override - keys(propsChanged).map((prop) => { - const shouldPropagate = isSymbolOverride(child, prop) && !(propsChanged[prop].type === CollectionVariableType); - if (shouldPropagate) delete propsChanged[prop]; - }); - child.set(propsChanged, { fromInstance: symbol, ...opts }); - }); - } +const shouldPropagateProperty = (props: Record, prop: string, component: Component): boolean => { + const isCollectionVariableDefinition = (() => { + if (prop === 'attributes') { + const attributes = props['attributes']; + return Object.values(attributes).some((attr: any) => attr?.type === CollectionVariableType); + } + + return props[prop]?.type === CollectionVariableType; + })(); + + return !isSymbolOverride(component, prop) || isCollectionVariableDefinition; }; export const updateSymbolCls = (symbol: Component, opts: any = {}) => { diff --git a/packages/core/src/dom_components/model/types.ts b/packages/core/src/dom_components/model/types.ts index d10f34f3c..25f5ddc15 100644 --- a/packages/core/src/dom_components/model/types.ts +++ b/packages/core/src/dom_components/model/types.ts @@ -323,4 +323,6 @@ export interface ComponentOptions { temporary?: boolean; avoidChildren?: boolean; collectionsStateMap?: CollectionsStateMap; + isCollectionItem?: boolean; + parent?: Component; } diff --git a/packages/core/test/specs/data_sources/model/collection_component/CollectionComponent.ts b/packages/core/test/specs/data_sources/model/collection_component/CollectionComponent.ts index b726cb4d5..1d5485cb9 100644 --- a/packages/core/test/specs/data_sources/model/collection_component/CollectionComponent.ts +++ b/packages/core/test/specs/data_sources/model/collection_component/CollectionComponent.ts @@ -91,7 +91,9 @@ describe('Collection component', () => { describe('Properties', () => { let cmp: Component; let firstChild!: Component; + let firstGrandchild!: Component; let secondChild!: Component; + let secondGrandchild!: Component; let thirdChild!: Component; beforeEach(() => { @@ -100,6 +102,16 @@ describe('Collection component', () => { collectionDefinition: { block: { type: 'default', + components: [ + { + type: 'default', + content: { + type: CollectionVariableType, + variable_type: CollectionStateVariableType.current_item, + path: 'user', + }, + }, + ], content: { type: CollectionVariableType, variable_type: CollectionStateVariableType.current_item, @@ -121,25 +133,31 @@ describe('Collection component', () => { })[0]; firstChild = cmp.components().at(0); + firstGrandchild = firstChild.components().at(0); secondChild = cmp.components().at(1); + secondGrandchild = secondChild.components().at(0); thirdChild = cmp.components().at(2); }); test('Evaluating to static value', () => { expect(firstChild.get('content')).toBe('user1'); expect(firstChild.get('custom_property')).toBe('user1'); + expect(firstGrandchild.get('content')).toBe('user1'); expect(secondChild.get('content')).toBe('user2'); expect(secondChild.get('custom_property')).toBe('user2'); + expect(secondGrandchild.get('content')).toBe('user2'); }); test('Updating the record', async () => { firstRecord.set('user', 'new_user1_value'); expect(firstChild.get('content')).toBe('new_user1_value'); expect(firstChild.get('custom_property')).toBe('new_user1_value'); + expect(firstGrandchild.get('content')).toBe('new_user1_value'); expect(secondChild.get('content')).toBe('user2'); expect(secondChild.get('custom_property')).toBe('user2'); + expect(secondGrandchild.get('content')).toBe('user2'); }); test('Updating the value to a static value', async () => { @@ -150,6 +168,14 @@ describe('Collection component', () => { firstRecord.set('user', 'wrong_value'); expect(firstChild.get('content')).toBe('new_content_value'); expect(secondChild.get('content')).toBe('new_content_value'); + + firstGrandchild.set('content', 'new_content_value'); + expect(firstGrandchild.get('content')).toBe('new_content_value'); + expect(secondGrandchild.get('content')).toBe('new_content_value'); + + firstRecord.set('user', 'wrong_value'); + expect(firstGrandchild.get('content')).toBe('new_content_value'); + expect(secondGrandchild.get('content')).toBe('new_content_value'); }); test('Updating the value to a diffirent collection variable', async () => { @@ -170,6 +196,21 @@ describe('Collection component', () => { expect(firstChild.get('content')).toBe('new_value_12'); expect(secondChild.get('content')).toBe('new_value_14'); + + firstGrandchild.set('content', { + // @ts-ignore + type: CollectionVariableType, + variable_type: CollectionStateVariableType.current_item, + path: 'age', + }); + expect(firstGrandchild.get('content')).toBe('new_value_12'); + expect(secondGrandchild.get('content')).toBe('new_value_14'); + + firstRecord.set('age', 'most_new_value_12'); + secondRecord.set('age', 'most_new_value_14'); + + expect(firstGrandchild.get('content')).toBe('most_new_value_12'); + expect(secondGrandchild.get('content')).toBe('most_new_value_14'); }); test('Updating the value to a diffirent dynamic variable', async () => { @@ -186,6 +227,19 @@ describe('Collection component', () => { expect(firstChild.get('content')).toBe('new_value'); expect(secondChild.get('content')).toBe('new_value'); expect(thirdChild.get('content')).toBe('new_value'); + + firstGrandchild.set('content', { + // @ts-ignore + type: DataVariableType, + path: 'my_data_source_id.user2.user', + }); + expect(firstGrandchild.get('content')).toBe('new_value'); + expect(secondGrandchild.get('content')).toBe('new_value'); + + secondRecord.set('user', 'most_new_value'); + + expect(firstGrandchild.get('content')).toBe('most_new_value'); + expect(secondGrandchild.get('content')).toBe('most_new_value'); }); }); @@ -401,7 +455,3 @@ describe('Collection component', () => { }); }); }); - -function delay(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)); -}