Skip to content

Fix #19676 #19886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 13, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export * from './default-memoization.function.js';
export * from './filter-frozen-array.function.js';
export * from './json-string-comparison.function.js';
export * from './merge-observables.function.js';
export * from './observation-as-promise.function.js';
export * from './observe-multiple.function.js';
export * from './partial-update-frozen-array.function.js';
export * from './push-at-to-unique-array.function.js';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import type { Observable } from '@umbraco-cms/backoffice/external/rxjs';

/**
* @function observationAsPromise
* @param {Observable<unknown>} observable - an Array of Observables to use for this combined observation.
* @param {Promise<condition>} condition - an method which should return true or false, if rejected or returning undefined the observation will result in a rejected Promise.
* @description - TODO:...
* @returns {Promise<unknown>} - Returns a Promise which resolves when the condition returns true or rejects when the condition returns undefined or is rejecting it self.
* @example
*
* TODO: ...
*/
export function observationAsPromise<T>(
observable: Observable<T>,
condition: (value: T) => Promise<boolean | undefined>,
): Promise<T> {
// Notice, we do not want to store and reuse the Promise, cause this promise guarantees that the value is not undefined when resolved. and reusing the promise would not ensure that.
return new Promise<T>((resolve, reject) => {
let initialCallback = true;
let wantedToClose = false;
const subscription = observable.subscribe(async (value) => {
const shouldClose = await condition(value).catch(() => {
if (initialCallback) {
wantedToClose = true;
} else {
subscription.unsubscribe();
}
reject(value);
});
if (shouldClose === true) {
if (initialCallback) {
wantedToClose = true;
} else {
subscription.unsubscribe();
}
resolve(value);
}
});
initialCallback = false;
if (wantedToClose) {
subscription.unsubscribe();
}
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,15 @@
appendToFrozenArray,
filterFrozenArray,
createObservablePart,
observationAsPromise,
mergeObservables,
} from '@umbraco-cms/backoffice/observable-api';
import { incrementString } from '@umbraco-cms/backoffice/utils';
import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api';
import { UmbExtensionApiInitializer } from '@umbraco-cms/backoffice/extension-api';
import { umbExtensionsRegistry, type ManifestRepository } from '@umbraco-cms/backoffice/extension-registry';
import { firstValueFrom } from '@umbraco-cms/backoffice/external/rxjs';
import { UmbError } from '@umbraco-cms/backoffice/resources';

type UmbPropertyTypeUnique = UmbPropertyTypeModel['unique'];

Expand Down Expand Up @@ -112,6 +115,13 @@
readonly contentTypeUniques = this.#contentTypes.asObservablePart((x) => x.map((y) => y.unique));
readonly contentTypeAliases = this.#contentTypes.asObservablePart((x) => x.map((y) => y.alias));

readonly contentTypeLoaded = mergeObservables(
[this.contentTypeCompositions, this.contentTypeUniques],
([comps, uniques]) => {
return comps.every((x) => uniques.includes(x.contentType.unique));
},
);

readonly variesByCulture = createObservablePart(this.ownerContentType, (x) => x?.variesByCulture);
readonly variesBySegment = createObservablePart(this.ownerContentType, (x) => x?.variesBySegment);

Expand Down Expand Up @@ -191,9 +201,28 @@
);
}
this.#repoManager!.setUniques([unique]);
const result = await this.observe(this.#repoManager!.entryByUnique(unique)).asPromise();
const observable = this.#repoManager!.entryByUnique(unique);
const result = await this.observe(observable).asPromise();
if (!result) {
this.#initRejection?.(`Content Type structure manager could not load: ${unique}`);
return {
error: new UmbError(`Content Type structure manager could not load: ${unique}`),
asObservable: () => observable,
};
}

// Awaits that everything is loaded:
await observationAsPromise(this.contentTypeLoaded, async (loaded) => {
return loaded === true;
}).catch(() => {
const msg = `Content Type structure manager could not load: ${unique}. Not all Content Types loaded successfully.`;
this.#initRejection?.(msg);
return Promise.reject(new UmbError(msg));
});

console.log('dd', this.#contentTypes.getValue());

this.#initResolver?.(result);

Check warning on line 225 in src/Umbraco.Web.UI.Client/src/packages/content/content-type/structure/content-type-structure-manager.class.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

UmbContentTypeStructureManager.loadType has a cyclomatic complexity of 9, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
await this.#init;
return { data: result, asObservable: () => this.ownerContentType };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ export class UmbRepositoryDetailsManager<DetailType extends { unique: string }>

#statuses = new UmbArrayState<UmbRepositoryRequestStatus>([], (x) => x.unique);
statuses = this.#statuses.asObservable();
allSuccess = this.#statuses.asObservablePart(
// If there are no other statuses than success, then we consider it all successful.
(statuses) => statuses.length > 0 && !statuses.some((status) => status.state.type !== 'success'),
);

/**
* Creates an instance of UmbRepositoryDetailsManager.
Expand Down Expand Up @@ -151,14 +155,14 @@ export class UmbRepositoryDetailsManager<DetailType extends { unique: string }>
*/
addEntry(data: DetailType): void {
const unique = data.unique;
this.#entries.appendOne(data);
this.#uniques.appendOne(unique);
this.#statuses.appendOne({
state: {
type: 'success',
},
unique,
});
this.#entries.appendOne(data);
this.#uniques.appendOne(unique);
// Notice in this case we do not have a observable from the repo, but it should maybe be fine that we just listen for ACTION EVENTS.
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ export class UmbDocumentWorkspaceContext
documentTypeUnique,
blueprintUnique,
);
// TODO: make sure to capture promise rejection here [NL]
// Also check what the router does when setup is rejected...

new UmbWorkspaceIsNewRedirectController(
this,
Expand Down Expand Up @@ -415,7 +417,7 @@ export class UmbDocumentWorkspaceContext
this.readOnlyGuard?.addRule({
unique: identifier,
message,
/* This guard is a bit backwards. The rule is permitted to be read-only.
/* This guard is a bit backwards. The rule is permitted to be read-only.
If the user does not have permission, we set it to true = permitted to be read-only. */
permitted: true,
});
Expand Down
Loading