diff --git a/packages/core/src/collection/index.ts b/packages/core/src/collection/index.ts index 6a900999..d88e53db 100644 --- a/packages/core/src/collection/index.ts +++ b/packages/core/src/collection/index.ts @@ -979,26 +979,29 @@ export class Collection { for (const groupKey in this.groups) { const group = this.getGroup(groupKey, { notExisting: true }); if (!group?.has(oldItemKey)) continue; - group.replace(oldItemKey, newItemKey, { background: config.background }); + group?.replace(oldItemKey, newItemKey, { background: config.background }); } // Update ItemKey in Selectors for (const selectorKey in this.selectors) { const selector = this.getSelector(selectorKey, { notExisting: true }); - if (!selector) continue; - - // Reselect Item in existing Selector which has selected the newItemKey - if (selector.hasSelected(newItemKey)) { - selector.select(newItemKey, { + if (selector == null) continue; + + // Reselect Item in Selector that has selected the newItemKey + // Necessary because the reference placeholder Item got removed + // and replaced with the new Item (Item of which the primaryKey was renamed) + // -> needs to find new Item with the same itemKey + if (selector.hasSelected(newItemKey, false)) { + selector.reselect({ force: true, // Because ItemKeys are the same background: config.background, }); } - // Select newItemKey in existing Selector which has selected the oldItemKey - if (selector.hasSelected(oldItemKey)) + // Select newItemKey in Selector that has selected the oldItemKey + if (selector.hasSelected(oldItemKey, false)) selector.select(newItemKey, { - background: config?.background, + background: config.background, }); } @@ -1034,12 +1037,12 @@ export class Collection { itemKeys: ItemKey | Array ): { fromGroups: (groups: Array | ItemKey) => Collection; - everywhere: () => Collection; + everywhere: (config?: RemoveItemsConfigInterface) => Collection; } { return { fromGroups: (groups: Array | ItemKey) => this.removeFromGroups(itemKeys, groups), - everywhere: () => this.removeItems(itemKeys), + everywhere: (config) => this.removeItems(itemKeys, config || {}), }; } @@ -1088,13 +1091,22 @@ export class Collection { * @public * Removes Item completely from Collection * @param itemKeys - ItemKey/s of Item/s + * @param config - Config */ - public removeItems(itemKeys: ItemKey | Array): this { + public removeItems( + itemKeys: ItemKey | Array, + config: RemoveItemsConfigInterface = {} + ): this { + config = defineConfig(config, { + notExisting: false, + removeSelector: false, + }); const _itemKeys = normalizeArray(itemKeys); _itemKeys.forEach((itemKey) => { - const item = this.getItem(itemKey, { notExisting: true }); + const item = this.getItem(itemKey, { notExisting: config.notExisting }); if (item == null) return; + const wasPlaceholder = item.isPlaceholder; // Remove Item from Groups for (const groupKey in this.groups) { @@ -1108,14 +1120,21 @@ export class Collection { // Remove Item from Collection delete this.data[itemKey]; - // Reselect Item in Selectors (to create new dummyItem that holds reference) + // Reselect or remove Selectors representing the removed Item for (const selectorKey in this.selectors) { const selector = this.getSelector(selectorKey, { notExisting: true }); - if (selector?.hasSelected(itemKey)) - selector?.select(itemKey, { force: true }); + if (selector?.hasSelected(itemKey, false)) { + if (config.removeSelector) { + // Remove Selector + this.removeSelector(selector?._key ?? 'unknown'); + } else { + // Reselect Item in Selector (to create new dummyItem to hold a reference to this removed Item) + selector?.reselect({ force: true }); + } + } } - this.size--; + if (!wasPlaceholder) this.size--; }); return this; @@ -1302,6 +1321,18 @@ export interface CollectionPersistentConfigInterface { defaultStorageKey?: StorageKey; } +/* + * @param notExisting - If not existing Items like placeholder Items can be removed. + * Keep in mind that sometimes it won't remove the Item entirely + * because another Instance (like a Selector) needs to keep reference to it. + * https://github.com/agile-ts/agile/pull/152 + * @param - If Selectors that have selected an Item to be removed, should be removed too + */ +export interface RemoveItemsConfigInterface { + notExisting?: boolean; + removeSelector?: boolean; +} + /** * @param patch - If Data gets patched into existing Item * @param background - If assigning Data happens in background diff --git a/packages/core/src/collection/selector.ts b/packages/core/src/collection/selector.ts index b389c1de..deaf17b7 100644 --- a/packages/core/src/collection/selector.ts +++ b/packages/core/src/collection/selector.ts @@ -143,8 +143,8 @@ export class Selector extends State< /** * @public * Reselects current Item - * Might help if the Selector failed to properly select an Item. - * You can check with 'hasSelected()' if an Item got properly selected. + * Might help if the Selector failed to select an Item correctly. + * You can check with 'hasSelected()' if an Item got correctly selected. * @param config - Config */ public reselect(config: StateRuntimeJobConfigInterface = {}): this { @@ -196,13 +196,17 @@ export class Selector extends State< /** * Checks if Selector has correctly selected the Item at the passed itemKey * @param itemKey - ItemKey + * @param correctlySelected - If it should consider only correctly selected Items */ - public hasSelected(itemKey: ItemKey): boolean { - return ( - this._itemKey === itemKey && - this.item != null && - this.item.selectedBy.has(this._key as any) - ); + public hasSelected(itemKey: ItemKey, correctlySelected = true): boolean { + if (correctlySelected) { + return ( + this._itemKey === itemKey && + this.item != null && + this.item.selectedBy.has(this._key as any) + ); + } + return this._itemKey === itemKey; } //========================================================================================================= diff --git a/packages/core/src/runtime/subscription/sub.controller.ts b/packages/core/src/runtime/subscription/sub.controller.ts index e7f4f9fb..fbd9b37c 100644 --- a/packages/core/src/runtime/subscription/sub.controller.ts +++ b/packages/core/src/runtime/subscription/sub.controller.ts @@ -283,7 +283,7 @@ export class SubController { Agile.logger.if .tag(['runtime', 'subscription']) - .info('15:01:03', callbackSubscriptionContainer); + .info(LogCodeManager.getLog('15:01:03'), callbackSubscriptionContainer); return callbackSubscriptionContainer; } diff --git a/packages/core/tests/unit/collection/collection.test.ts b/packages/core/tests/unit/collection/collection.test.ts index e9233dbc..482e74e2 100644 --- a/packages/core/tests/unit/collection/collection.test.ts +++ b/packages/core/tests/unit/collection/collection.test.ts @@ -1903,6 +1903,9 @@ describe('Collection Tests', () => { dummySelector1.select = jest.fn(); dummySelector2.select = jest.fn(); dummySelector3.select = jest.fn(); + dummySelector1.reselect = jest.fn(); + dummySelector2.reselect = jest.fn(); + dummySelector3.reselect = jest.fn(); }); it('should update ItemKey in Collection, Selectors and Groups (default config)', () => { @@ -1935,7 +1938,7 @@ describe('Collection Tests', () => { background: false, }); expect(dummySelector2.select).not.toHaveBeenCalled(); - expect(dummySelector3.select).toHaveBeenCalledWith('newDummyItem', { + expect(dummySelector3.reselect).toHaveBeenCalledWith({ force: true, background: false, }); @@ -1975,7 +1978,7 @@ describe('Collection Tests', () => { expect(dummySelector1.select).toHaveBeenCalledWith('newDummyItem', { background: true, }); - expect(dummySelector3.select).toHaveBeenCalledWith('newDummyItem', { + expect(dummySelector3.reselect).toHaveBeenCalledWith({ force: true, background: true, }); @@ -2012,7 +2015,7 @@ describe('Collection Tests', () => { expect(dummySelector1.select).toHaveBeenCalledWith('newDummyItem', { background: false, }); - expect(dummySelector3.select).toHaveBeenCalledWith('newDummyItem', { + expect(dummySelector3.reselect).toHaveBeenCalledWith({ force: true, background: false, }); @@ -2109,11 +2112,26 @@ describe('Collection Tests', () => { expect(collection.removeItems).not.toHaveBeenCalled(); }); - it('should remove Items from everywhere', () => { + it('should remove Items from everywhere (default config)', () => { collection.remove(['test1', 'test2']).everywhere(); expect(collection.removeFromGroups).not.toHaveBeenCalled(); - expect(collection.removeItems).toHaveBeenCalledWith(['test1', 'test2']); + expect(collection.removeItems).toHaveBeenCalledWith( + ['test1', 'test2'], + {} + ); + }); + + it('should remove Items from everywhere (specific config)', () => { + collection + .remove(['test1', 'test2']) + .everywhere({ removeSelector: true, notExisting: true }); + + expect(collection.removeFromGroups).not.toHaveBeenCalled(); + expect(collection.removeItems).toHaveBeenCalledWith( + ['test1', 'test2'], + { removeSelector: true, notExisting: true } + ); }); }); @@ -2194,15 +2212,22 @@ describe('Collection Tests', () => { let dummyGroup2: Group; let dummyItem1: Item; let dummyItem2: Item; + let placeholderItem: Item; beforeEach(() => { dummyItem1 = new Item(collection, { id: 'dummyItem1', name: 'Jeff' }); dummyItem1.persistent = new StatePersistent(dummyItem1); dummyItem2 = new Item(collection, { id: 'dummyItem2', name: 'Hans' }); dummyItem2.persistent = new StatePersistent(dummyItem2); + placeholderItem = new Item( + collection, + { id: 'placeholderItem', name: 'placeholder' }, + { isPlaceholder: true } + ); collection.data = { dummyItem1: dummyItem1, dummyItem2: dummyItem2, + placeholderItem: placeholderItem, }; collection.size = 2; @@ -2234,16 +2259,19 @@ describe('Collection Tests', () => { dummyGroup1.remove = jest.fn(); dummyGroup2.remove = jest.fn(); - dummySelector1.select = jest.fn(); - dummySelector2.select = jest.fn(); + dummySelector1.reselect = jest.fn(); + dummySelector2.reselect = jest.fn(); + + collection.removeSelector = jest.fn(); }); - it('should remove Item from Collection, Groups and Selectors', () => { + it('should remove Item from Collection, Groups and reselect Selectors (default config)', () => { collection.removeItems('dummyItem1'); expect(collection.data).not.toHaveProperty('dummyItem1'); expect(collection.data).toHaveProperty('dummyItem2'); expect(collection.size).toBe(1); + expect(collection.removeSelector).not.toHaveBeenCalled(); expect(dummyItem1.persistent?.removePersistedValue).toHaveBeenCalled(); expect( @@ -2253,18 +2281,19 @@ describe('Collection Tests', () => { expect(dummyGroup1.remove).toHaveBeenCalledWith('dummyItem1'); expect(dummyGroup2.remove).not.toHaveBeenCalled(); - expect(dummySelector1.select).toHaveBeenCalledWith('dummyItem1', { + expect(dummySelector1.reselect).toHaveBeenCalledWith({ force: true, }); - expect(dummySelector2.select).not.toHaveBeenCalled(); + expect(dummySelector2.reselect).not.toHaveBeenCalled(); }); - it('should remove Items from Collection, Groups and Selectors', () => { + it('should remove Items from Collection, Groups and reselect Selectors (default config)', () => { collection.removeItems(['dummyItem1', 'dummyItem2', 'notExistingItem']); expect(collection.data).not.toHaveProperty('dummyItem1'); expect(collection.data).not.toHaveProperty('dummyItem2'); expect(collection.size).toBe(0); + expect(collection.removeSelector).not.toHaveBeenCalled(); expect(dummyItem1.persistent?.removePersistedValue).toHaveBeenCalled(); expect(dummyItem2.persistent?.removePersistedValue).toHaveBeenCalled(); @@ -2274,13 +2303,54 @@ describe('Collection Tests', () => { expect(dummyGroup2.remove).not.toHaveBeenCalledWith('dummyItem1'); expect(dummyGroup2.remove).toHaveBeenCalledWith('dummyItem2'); - expect(dummySelector1.select).toHaveBeenCalledWith('dummyItem1', { + expect(dummySelector1.reselect).toHaveBeenCalledWith({ force: true, }); - expect(dummySelector2.select).toHaveBeenCalledWith('dummyItem2', { + expect(dummySelector2.reselect).toHaveBeenCalledWith({ force: true, }); }); + + it('should remove Item from Collection, Groups and remove Selectors (removeSelector = true)', () => { + collection.removeItems('dummyItem1', { removeSelector: true }); + + expect(collection.data).not.toHaveProperty('dummyItem1'); + expect(collection.data).toHaveProperty('dummyItem2'); + expect(collection.size).toBe(1); + expect(collection.removeSelector).toHaveBeenCalledTimes(1); + expect(collection.removeSelector).toHaveBeenCalledWith( + dummySelector1._key + ); + + expect(dummyItem1.persistent?.removePersistedValue).toHaveBeenCalled(); + expect( + dummyItem2.persistent?.removePersistedValue + ).not.toHaveBeenCalled(); + + expect(dummyGroup1.remove).toHaveBeenCalledWith('dummyItem1'); + expect(dummyGroup2.remove).not.toHaveBeenCalled(); + + expect(dummySelector1.reselect).not.toHaveBeenCalled(); + expect(dummySelector2.reselect).not.toHaveBeenCalled(); + }); + + it("shouldn't remove placeholder Items from Collection (default config)", () => { + collection.removeItems(['dummyItem1', 'placeholderItem']); + + expect(collection.data).toHaveProperty('placeholderItem'); + expect(collection.data).not.toHaveProperty('dummyItem1'); + expect(collection.size).toBe(1); + }); + + it('should remove placeholder Items from Collection (config.notExisting = true)', () => { + collection.removeItems(['dummyItem1', 'placeholderItem'], { + notExisting: true, + }); + + expect(collection.data).not.toHaveProperty('placeholderItem'); + expect(collection.data).not.toHaveProperty('dummyItem1'); + expect(collection.size).toBe(1); + }); }); describe('setData function tests', () => { diff --git a/packages/core/tests/unit/collection/selector.test.ts b/packages/core/tests/unit/collection/selector.test.ts index b26b30cc..03f088d0 100644 --- a/packages/core/tests/unit/collection/selector.test.ts +++ b/packages/core/tests/unit/collection/selector.test.ts @@ -651,21 +651,37 @@ describe('Selector Tests', () => { expect(selector.hasSelected('dummyItemKey')).toBeTruthy(); }); - it("should return false if Selector hasn't selected itemKey correctly (itemKey == undefined)", () => { + it("should return false if Selector hasn't selected itemKey correctly (itemKey = undefined)", () => { expect(selector.hasSelected('notSelectedItemKey')).toBeFalsy(); }); - it("should return false if Selector hasn't selected itemKey correctly (item == undefined)", () => { + it("should return false if Selector hasn't selected itemKey correctly (itemKey = undefined, correctlySelected = false)", () => { + expect(selector.hasSelected('notSelectedItemKey', false)).toBeFalsy(); + }); + + it("should return false if Selector hasn't selected itemKey correctly (item = undefined)", () => { selector.item = undefined; expect(selector.hasSelected('dummyItemKey')).toBeFalsy(); }); + it("should return true if Selector hasn't selected itemKey correctly (item = undefined, correctlySelected = false)", () => { + selector.item = undefined; + + expect(selector.hasSelected('dummyItemKey', false)).toBeTruthy(); + }); + it("should return false if Selector has selected itemKey correctly and Item isn't isSelected", () => { if (selector.item) selector.item.selectedBy = new Set(); expect(selector.hasSelected('dummyItemKey')).toBeFalsy(); }); + + it("should return true if Selector has selected itemKey correctly and Item isn't isSelected (correctlySelected = false)", () => { + if (selector.item) selector.item.selectedBy = new Set(); + + expect(selector.hasSelected('dummyItemKey', false)).toBeTruthy(); + }); }); describe('rebuildSelector function tests', () => {