From fd91f88a7e8f5e8c276667e2e33ada906e584e85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Wed, 22 Oct 2025 11:52:09 +0200 Subject: [PATCH] Item Repository: Sort statuses by order of unique (#20603) * utility * ability to replace * deprecate removeStatus * no need to call this any longer * Sort statuses and ensure not appending statuses, only updating them # Conflicts: # src/Umbraco.Web.UI.Client/src/packages/core/repository/repository-items.manager.ts --- .../observable-api/states/array-state.test.ts | 70 ++++++++++++------- .../libs/observable-api/states/array-state.ts | 33 +++++++++ .../src/libs/observable-api/utils/index.ts | 1 + .../utils/replace-in-unique-array.function.ts | 19 +++++ .../core/picker-input/picker-input.context.ts | 1 - .../repository/repository-items.manager.ts | 34 ++++++--- 6 files changed, 119 insertions(+), 39 deletions(-) create mode 100644 src/Umbraco.Web.UI.Client/src/libs/observable-api/utils/replace-in-unique-array.function.ts diff --git a/src/Umbraco.Web.UI.Client/src/libs/observable-api/states/array-state.test.ts b/src/Umbraco.Web.UI.Client/src/libs/observable-api/states/array-state.test.ts index d9d5d1ad4f..74c2e63c5c 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/observable-api/states/array-state.test.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/observable-api/states/array-state.test.ts @@ -5,7 +5,7 @@ describe('ArrayState', () => { type ObjectType = { key: string; another: string }; type ArrayType = ObjectType[]; - let subject: UmbArrayState; + let state: UmbArrayState; let initialData: ArrayType; beforeEach(() => { @@ -14,12 +14,12 @@ describe('ArrayState', () => { { key: '2', another: 'myValue2' }, { key: '3', another: 'myValue3' }, ]; - subject = new UmbArrayState(initialData, (x) => x.key); + state = new UmbArrayState(initialData, (x) => x.key); }); it('replays latests, no matter the amount of subscriptions.', (done) => { let amountOfCallbacks = 0; - const observer = subject.asObservable(); + const observer = state.asObservable(); observer.subscribe((value) => { amountOfCallbacks++; expect(value).to.be.equal(initialData); @@ -36,8 +36,8 @@ describe('ArrayState', () => { it('remove method, removes the one with the key', (done) => { const expectedData = [initialData[0], initialData[2]]; - subject.remove(['2']); - const observer = subject.asObservable(); + state.remove(['2']); + const observer = state.asObservable(); observer.subscribe((value) => { expect(JSON.stringify(value)).to.be.equal(JSON.stringify(expectedData)); done(); @@ -45,17 +45,17 @@ describe('ArrayState', () => { }); it('getHasOne method, return true when key exists', () => { - expect(subject.getHasOne('2')).to.be.true; + expect(state.getHasOne('2')).to.be.true; }); it('getHasOne method, return false when key does not exists', () => { - expect(subject.getHasOne('1337')).to.be.false; + expect(state.getHasOne('1337')).to.be.false; }); it('filter method, removes anything that is not true of the given predicate method', (done) => { const expectedData = [initialData[0], initialData[2]]; - subject.filter((x) => x.key !== '2'); - const observer = subject.asObservable(); + state.filter((x) => x.key !== '2'); + const observer = state.asObservable(); observer.subscribe((value) => { expect(JSON.stringify(value)).to.be.equal(JSON.stringify(expectedData)); done(); @@ -64,11 +64,11 @@ describe('ArrayState', () => { it('add new item via appendOne method.', (done) => { const newItem = { key: '4', another: 'myValue4' }; - subject.appendOne(newItem); + state.appendOne(newItem); const expectedData = [...initialData, newItem]; - const observer = subject.asObservable(); + const observer = state.asObservable(); observer.subscribe((value) => { expect(value.length).to.be.equal(expectedData.length); expect(value[3].another).to.be.equal(expectedData[3].another); @@ -78,9 +78,25 @@ describe('ArrayState', () => { it('partially update an existing item via updateOne method.', (done) => { const newItem = { another: 'myValue2.2' }; - subject.updateOne('2', newItem); + state.updateOne('2', newItem); - const observer = subject.asObservable(); + const observer = state.asObservable(); + observer.subscribe((value) => { + expect(value.length).to.be.equal(initialData.length); + expect(value[0].another).to.be.equal('myValue1'); + expect(value[1].another).to.be.equal('myValue2.2'); + done(); + }); + }); + + it('replaces only existing items via replace method.', (done) => { + const newItems = [ + { key: '2', another: 'myValue2.2' }, + { key: '4', another: 'myValue4.4' }, + ]; + state.replace(newItems); + + const observer = state.asObservable(); observer.subscribe((value) => { expect(value.length).to.be.equal(initialData.length); expect(value[0].another).to.be.equal('myValue1'); @@ -90,7 +106,7 @@ describe('ArrayState', () => { }); it('getObservablePart for a specific entry of array', (done) => { - const subObserver = subject.asObservablePart((data) => data.find((x) => x.key === '2')); + const subObserver = state.asObservablePart((data) => data.find((x) => x.key === '2')); subObserver.subscribe((entry) => { if (entry) { expect(entry.another).to.be.equal(initialData[1].another); @@ -103,7 +119,7 @@ describe('ArrayState', () => { let amountOfCallbacks = 0; const newItem = { key: '4', another: 'myValue4' }; - const subObserver = subject.asObservablePart((data) => data.find((x) => x.key === newItem.key)); + const subObserver = state.asObservablePart((data) => data.find((x) => x.key === newItem.key)); subObserver.subscribe((entry) => { amountOfCallbacks++; if (amountOfCallbacks === 1) { @@ -118,16 +134,16 @@ describe('ArrayState', () => { } }); - subject.appendOne(newItem); + state.appendOne(newItem); }); it('asObservable returns the replaced item', (done) => { const newItem = { key: '2', another: 'myValue4' }; - subject.appendOne(newItem); + state.appendOne(newItem); const expectedData = [initialData[0], newItem, initialData[2]]; - const observer = subject.asObservable(); + const observer = state.asObservable(); observer.subscribe((value) => { expect(value.length).to.be.equal(expectedData.length); expect(value[1].another).to.be.equal(newItem.another); @@ -137,9 +153,9 @@ describe('ArrayState', () => { it('getObservablePart returns the replaced item', (done) => { const newItem = { key: '2', another: 'myValue4' }; - subject.appendOne(newItem); + state.appendOne(newItem); - const subObserver = subject.asObservablePart((data) => data.find((x) => x.key === newItem.key)); + const subObserver = state.asObservablePart((data) => data.find((x) => x.key === newItem.key)); subObserver.subscribe((entry) => { expect(entry).to.be.equal(newItem); // Second callback should give us the right data: if (entry) { @@ -152,7 +168,7 @@ describe('ArrayState', () => { it('getObservablePart replays existing data to any amount of subscribers.', (done) => { let amountOfCallbacks = 0; - const subObserver = subject.asObservablePart((data) => data.find((x) => x.key === '2')); + const subObserver = state.asObservablePart((data) => data.find((x) => x.key === '2')); subObserver.subscribe((entry) => { if (entry) { amountOfCallbacks++; @@ -173,7 +189,7 @@ describe('ArrayState', () => { it('getObservablePart replays existing data to any amount of subscribers.', (done) => { let amountOfCallbacks = 0; - const subObserver = subject.asObservablePart((data) => data.find((x) => x.key === '2')); + const subObserver = state.asObservablePart((data) => data.find((x) => x.key === '2')); subObserver.subscribe((entry) => { if (entry) { amountOfCallbacks++; @@ -194,7 +210,7 @@ describe('ArrayState', () => { it('append only updates observable if changes item', (done) => { let count = 0; - const observer = subject.asObservable(); + const observer = state.asObservable(); observer.subscribe((value) => { count++; if (count === 1) { @@ -212,12 +228,12 @@ describe('ArrayState', () => { Promise.resolve().then(() => { // Despite how many times this happens it should not trigger any change. - subject.append(initialData); - subject.append(initialData); - subject.append(initialData); + state.append(initialData); + state.append(initialData); + state.append(initialData); Promise.resolve().then(() => { - subject.appendOne({ key: '4', another: 'myValue4' }); + state.appendOne({ key: '4', another: 'myValue4' }); }); }); }); diff --git a/src/Umbraco.Web.UI.Client/src/libs/observable-api/states/array-state.ts b/src/Umbraco.Web.UI.Client/src/libs/observable-api/states/array-state.ts index 26cf9261ab..3dab6a21c0 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/observable-api/states/array-state.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/observable-api/states/array-state.ts @@ -1,6 +1,7 @@ import { partialUpdateFrozenArray } from '../utils/partial-update-frozen-array.function.js'; import { pushAtToUniqueArray } from '../utils/push-at-to-unique-array.function.js'; import { pushToUniqueArray } from '../utils/push-to-unique-array.function.js'; +import { replaceInUniqueArray } from '../utils/replace-in-unique-array.function.js'; import { UmbDeepState } from './deep-state.js'; /** @@ -262,6 +263,38 @@ export class UmbArrayState extends UmbDeepState { return this; } + /** + * @function replace + * @param {Partial} entires - data of entries to be replaced. + * @returns {UmbArrayState} Reference to it self. + * @description - Replaces one or more entries, requires the ArrayState to be constructed with a getUnique method. + * @example Example append some data. + * const data = [ + * { key: 1, value: 'foo'}, + * { key: 2, value: 'bar'} + * ]; + * const myState = new UmbArrayState(data, (x) => x.key); + * const updates = [ + * { key: 1, value: 'foo2'}, + * { key: 3, value: 'bar2'} + * ]; + * myState.replace(updates); + * // Only the existing item gets replaced: + * myState.getValue(); // -> [{ key: 1, value: 'foo2'}, { key: 2, value: 'bar'}] + */ + replace(entries: Array): UmbArrayState { + if (this.getUniqueMethod) { + const next = [...this.getValue()]; + entries.forEach((entry) => { + replaceInUniqueArray(next, entry as T, this.getUniqueMethod!); + }); + this.setValue(next); + } else { + throw new Error("Can't replace entries of an ArrayState without a getUnique method provided when constructed."); + } + return this; + } + /** * @function updateOne * @param {U} unique - Unique value to find entry to update. diff --git a/src/Umbraco.Web.UI.Client/src/libs/observable-api/utils/index.ts b/src/Umbraco.Web.UI.Client/src/libs/observable-api/utils/index.ts index 3b3452ab11..0c90285e6c 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/observable-api/utils/index.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/observable-api/utils/index.ts @@ -12,5 +12,6 @@ export * from './observe-multiple.function.js'; export * from './partial-update-frozen-array.function.js'; export * from './push-at-to-unique-array.function.js'; export * from './push-to-unique-array.function.js'; +export * from './replace-in-unique-array.function.js'; export * from './simple-hash-code.function.js'; export * from './strict-equality-memoization.function.js'; diff --git a/src/Umbraco.Web.UI.Client/src/libs/observable-api/utils/replace-in-unique-array.function.ts b/src/Umbraco.Web.UI.Client/src/libs/observable-api/utils/replace-in-unique-array.function.ts new file mode 100644 index 0000000000..1970c0b2a9 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/libs/observable-api/utils/replace-in-unique-array.function.ts @@ -0,0 +1,19 @@ +/** + * @function replaceInUniqueArray + * @param {T[]} data - An array of objects. + * @param {T} entry - The object to replace with. + * @param {getUniqueMethod: (entry: T) => unknown} [getUniqueMethod] - Method to get the unique value of an entry. + * @description - Replaces an item of an Array. + * @example Example replace an entry of an Array. Where the key is unique and the item will only be replaced if matched with existing. + * const data = [{key: 'myKey', value:'initialValue'}]; + * const entry = {key: 'myKey', value: 'replacedValue'}; + * const newDataSet = replaceInUniqueArray(data, entry, x => x.key === key); + */ +export function replaceInUniqueArray(data: T[], entry: T, getUniqueMethod: (entry: T) => unknown): T[] { + const unique = getUniqueMethod(entry); + const indexToReplace = data.findIndex((x) => getUniqueMethod(x) === unique); + if (indexToReplace !== -1) { + data[indexToReplace] = entry; + } + return data; +} diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/picker-input/picker-input.context.ts b/src/Umbraco.Web.UI.Client/src/packages/core/picker-input/picker-input.context.ts index de943aa88a..c345fd22d4 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/picker-input/picker-input.context.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/picker-input/picker-input.context.ts @@ -133,7 +133,6 @@ export class UmbPickerInputContext< #removeItem(unique: string) { const newSelection = this.getSelection().filter((value) => value !== unique); this.setSelection(newSelection); - this.#itemManager.removeStatus(unique); this.getHostElement().dispatchEvent(new UmbChangeEvent()); } } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/repository/repository-items.manager.ts b/src/Umbraco.Web.UI.Client/src/packages/core/repository/repository-items.manager.ts index cf227b3cf2..c60dc8badf 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/repository/repository-items.manager.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/repository/repository-items.manager.ts @@ -75,17 +75,20 @@ export class UmbRepositoryItemsManager exte (uniques) => { if (uniques.length === 0) { this.#items.setValue([]); + this.#statuses.setValue([]); return; } // TODO: This could be optimized so we only load the appended items, but this requires that the response checks that an item is still present in uniques. [NL] - // Check if we already have the items, and then just sort them: - const items = this.#items.getValue(); + // Check if we already have the statuses, and then just sort them: + const statuses = this.#statuses.getValue(); if ( - uniques.length === items.length && - uniques.every((unique) => items.find((item) => this.#getUnique(item) === unique)) + uniques.length === statuses.length && + uniques.every((unique) => statuses.find((status) => status.unique === unique)) ) { + const items = this.#items.getValue(); this.#items.setValue(this.#sortByUniques(items)); + this.#statuses.setValue(this.#sortByUniques(statuses)); } else { // We need to load new items, so ... this.#requestItems(); @@ -124,9 +127,17 @@ export class UmbRepositoryItemsManager exte return this.#items.asObservablePart((items) => items.find((item) => this.#getUnique(item) === unique)); } + /** + * @deprecated - This is resolved by setUniques, no need to update statuses. + * @param unique {string} - The unique identifier of the item to remove the status of. + */ removeStatus(unique: string) { - const newStatuses = this.#statuses.getValue().filter((status) => status.unique !== unique); - this.#statuses.setValue(newStatuses); + new UmbDeprecation({ + removeInVersion: '18.0.0', + deprecated: 'removeStatus', + solution: 'Statuses are removed automatically when setting uniques', + }).warn(); + this.#statuses.filter((status) => status.unique !== unique); } async getItemByUnique(unique: string) { @@ -144,6 +155,7 @@ export class UmbRepositoryItemsManager exte const requestedUniques = this.getUniques(); this.#statuses.setValue( + // No need to do sorting here as we just got the unique in the right order above. requestedUniques.map((unique) => ({ state: { type: 'loading', @@ -164,7 +176,7 @@ export class UmbRepositoryItemsManager exte } if (error) { - this.#statuses.append( + this.#statuses.replace( requestedUniques.map((unique) => ({ state: { type: 'error', @@ -185,7 +197,7 @@ export class UmbRepositoryItemsManager exte const resolvedUniques = requestedUniques.filter((unique) => !rejectedUniques.includes(unique)); this.#items.remove(rejectedUniques); - this.#statuses.append([ + this.#statuses.replace([ ...rejectedUniques.map( (unique) => ({ @@ -226,12 +238,11 @@ export class UmbRepositoryItemsManager exte const { data, error } = await this.repository.requestItems([unique]); if (error) { - this.#statuses.appendOne({ + this.#statuses.updateOne(unique, { state: { type: 'error', error: '#general_notFound', }, - unique, } as UmbRepositoryItemsStatus); } @@ -244,11 +255,12 @@ export class UmbRepositoryItemsManager exte const newItems = [...items]; newItems[index] = data[0]; this.#items.setValue(this.#sortByUniques(newItems)); + // No need to update statuses here, as the item is the same, just updated. } } } - #sortByUniques(data?: Array): Array { + #sortByUniques>(data?: Array): Array { if (!data) return []; const uniques = this.getUniques(); return [...data].sort((a, b) => {