Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
f1fe1ea
Tree pickers: Implement noAccess property UI handling for user start …
claude Jan 10, 2026
dfa36ee
Apply suggestions from code review
iOvergaard Jan 12, 2026
ec1886c
Merge branch 'main' into claude/implement-noaccess-property-YrKuI
iOvergaard Jan 12, 2026
60a52a3
Fix noAccess implementation and add E2E tests
iOvergaard Jan 12, 2026
a8f7837
Remove aria-disabled manipulation that interferes with tree expansion
iOvergaard Jan 12, 2026
4e165ea
Fix path comparison bug in UserStartNodeEntitiesService (similar to #…
iOvergaard Jan 12, 2026
fd5c6c3
Merge main to get type guards and latest improvements
iOvergaard Jan 13, 2026
f74852f
Fix remaining merge conflict markers in media-tree-item.element.ts
iOvergaard Jan 13, 2026
5ea54cb
Remove E2E agent markdown file (moved to personal space)
iOvergaard Jan 13, 2026
d4b9e4e
test: adds mock data for noAccess
iOvergaard Jan 13, 2026
e1a4ce0
feat: moves noAccess subscriber to base class
iOvergaard Jan 13, 2026
d9cddf6
test: adds mock data for media
iOvergaard Jan 13, 2026
6dd80a8
feat: moves no-access styling to the base class
iOvergaard Jan 13, 2026
26e2551
fix: media tree items should inherit styling from the base class
iOvergaard Jan 13, 2026
37af0b8
feat: observes noAccess from children and reports back to the base class
iOvergaard Jan 13, 2026
899cc8f
test: spec file should use undefined instead of null
iOvergaard Jan 13, 2026
8e49829
docs: add comprehensive comments explaining noAccess opt-in pattern
iOvergaard Jan 13, 2026
346464e
test: adds timeout for URL to settle
iOvergaard Jan 13, 2026
0945bf4
fix: allow clicks on accessible children of noAccess tree items
iOvergaard Jan 13, 2026
d369578
compare with the closest element to see if we are clicking on the ele…
iOvergaard Jan 13, 2026
1ebed7e
fix: adds forbidden route in case of no variants
iOvergaard Jan 14, 2026
c1a4a35
test: corrects label locator
iOvergaard Jan 14, 2026
a123fa8
test: adds test to check if you can click or deeplink to restricted m…
iOvergaard Jan 14, 2026
e65cf4b
test: removes .only
iOvergaard Jan 14, 2026
eaac4b0
test: removes duplicated tests
iOvergaard Jan 14, 2026
e803143
test: adds test for document no-access
iOvergaard Jan 14, 2026
85d2607
test: add unit tests for user start node path comparison logic
iOvergaard Jan 14, 2026
0ce2866
Merge remote-tracking branch 'origin/main' into claude/implement-noac…
iOvergaard Jan 14, 2026
fc48a72
test: removes .only
iOvergaard Jan 14, 2026
369bc5a
fix: do not overwrite forbidden route
iOvergaard Jan 14, 2026
746dfc6
docs: fixes line number in comment
iOvergaard Jan 14, 2026
9d648a9
test: fixes comment
iOvergaard Jan 14, 2026
91c2340
feat: uses isSelectableContext to disable and scrub 'href' from base …
iOvergaard Jan 15, 2026
768da91
Merge branch 'main' into claude/implement-noaccess-property-YrKuI
iOvergaard Jan 15, 2026
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 @@ -142,7 +142,8 @@ public IEnumerable<UserAccessEntity> ChildUserAccessEntities(IEnumerable<IEntity
}

// is ancestor of a start node?
if (userStartNodePaths.Any(path => path.StartsWith(child.Path)))
// Note: Add trailing comma to prevent false matches (e.g., path "-1,100" should not match "-1,1001")
if (userStartNodePaths.Any(path => path.StartsWith($"{child.Path},")))
{
return new UserAccessEntity(child, false);
}
Expand Down Expand Up @@ -220,5 +221,7 @@ public IEnumerable<UserAccessEntity> UserAccessEntities(IEnumerable<IEntitySlim>
=> entities.Select(entity => new UserAccessEntity(entity, IsDescendantOrSelf(entity, userStartNodePaths))).ToArray();

private static bool IsDescendantOrSelf(IEntitySlim child, string[] userStartNodePaths)
=> userStartNodePaths.Any(path => child.Path.StartsWith(path));
// Note: Add trailing commas to both paths to prevent false matches (e.g., path "-1,100" should not match "-1,1001")
// This matches the pattern used in lines 92 and 192 of this file
=> userStartNodePaths.Any(path => $"{child.Path},".StartsWith($"{path},"));
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,34 @@ const permissionsTestDocument = {

export const data: Array<UmbMockDocumentModel> = [
permissionsTestDocument,
{
...permissionsTestDocument,
ancestors: [{ id: 'permissions-document-id' }],
hasChildren: false,
id: 'permissions-0-document-id',
parent: { id: 'permissions-document-id' },
variants: permissionsTestDocument.variants.map((variant) => ({
...variant,
name: 'Permissions 0 - No Access',
id: 'permissions-0',
})),
flags: [],
noAccess: true,
},
{
...permissionsTestDocument,
ancestors: [{ id: 'permissions-0-document-id' }],
hasChildren: false,
id: 'permissions-0-1-document-id',
parent: { id: 'permissions-0-document-id' },
variants: permissionsTestDocument.variants.map((variant) => ({
...variant,
name: 'Permissions 0.1 - Has access',
id: 'permissions-0-1',
})),
flags: [],
noAccess: false,
},
{
...permissionsTestDocument,
ancestors: [{ id: 'permissions-document-id' }],
Expand Down
37 changes: 37 additions & 0 deletions src/Umbraco.Web.UI.Client/src/mocks/data/media/media.data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,43 @@ export const data: Array<UmbMockMediaModel> = [
],
flags: [],
},
{
hasChildren: false,
id: 'f2f81a40-c989-4b6b-84e2-057cecd3grd4',
createDate: '2023-02-06T15:32:05.350038',
parent: null,
noAccess: true,
isTrashed: false,
mediaType: {
id: 'media-type-1-id',
icon: 'icon-picture',
},
values: [
{
editorAlias: 'Umbraco.UploadField',
alias: 'mediaPicker',
value: {
src: '/umbraco/backoffice/assets/installer-illustration.svg',
},
},
{
editorAlias: 'Umbraco.TextBox',
alias: 'mediaType1Property1',
value: 'The daily life at Umbraco HQ',
},
],
variants: [
{
publishDate: '2023-02-06T15:31:51.354764',
culture: null,
segment: null,
name: 'Permissions - No Access',
createDate: '2023-02-06T15:31:46.876902',
updateDate: '2023-02-06T15:31:51.354764',
},
],
flags: [],
},
{
hasChildren: false,
id: '69431027-8867-45bf-a93b-72bbdabfb177',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,29 @@ export abstract class UmbTreeItemElementBase<
}
protected _item?: TreeItemModelType;

/**
* @internal
* Indicates whether the user has no access to this tree item.
* This property is reflected as an attribute for styling purposes.
*
* **Usage Pattern (opt-in):**
* Child classes that support access restrictions should observe their context's `noAccess` observable
* and update this property. The base class provides the property, styling, and interaction prevention,
* but does not subscribe to the observable to avoid forcing all tree item types to implement it.
*
* **Example (in child class api setter):**
* ```typescript
* this.observe(this.#api.noAccess, (noAccess) => (this._noAccess = noAccess));
* ```
*
* **Why not in the base interface?**
* Adding `noAccess` to `UmbTreeItemContext` would be a breaking change, forcing all tree item
* implementations (users, members, data types, etc.) to provide this property even when access
* restrictions don't apply to them.
*/
@property({ type: Boolean, reflect: true, attribute: 'no-access' })
protected _noAccess = false;

/**
* @param item - The item from which to extract flags.
* @description This method is called whenever the `item` property is set. It extracts the flags from the item and assigns them to the `_flags` state property.
Expand Down Expand Up @@ -205,15 +228,15 @@ export abstract class UmbTreeItemElementBase<
@selected=${this._handleSelectedItem}
@deselected=${this._handleDeselectedItem}
?active=${this._isActive}
?disabled=${this._isSelectableContext && !this._isSelectable}
?disabled=${(this._isSelectableContext && !this._isSelectable) || this._noAccess}
?selectable=${this._isSelectable}
?selected=${this._isSelected}
.loading=${this._isLoading}
.hasChildren=${this._forceShowExpand || this._hasChildren}
.showChildren=${this._isOpen}
.caretLabel=${this.localize.term(caretLabelKey) + ' ' + this._label}
label=${ifDefined(this._label)}
href=${ifDefined(this._isSelectableContext ? undefined : this._href)}
href=${ifDefined(this._isSelectableContext || this._noAccess ? undefined : this._href)}
.renderExpandSymbol=${this._renderExpandSymbol}>
${this.#renderLoadPrevButton()} ${this.renderIconContainer()} ${this.renderLabel()} ${this.#renderActions()}
${this.#renderChildItems()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,29 @@ export class UmbDocumentTreeItemElement extends UmbTreeItemElementBase<
#api: UmbDocumentTreeItemContext | undefined;

@property({ type: Object, attribute: false })
public override get api(): UmbDocumentTreeItemContext | undefined {
return this.#api;
}
public override set api(value: UmbDocumentTreeItemContext | undefined) {
this.#api = value;

if (this.#api) {
this.observe(this.#api.name, (name) => (this._name = name || ''));
this.observe(this.#api.isDraft, (isDraft) => (this._isDraft = isDraft || false));
this.observe(this.#api.noAccess, (noAccess) => (this._noAccess = noAccess || false));
this.observe(this.#api.hasCollection, (has) => {
const oldValue = this._forceShowExpand;
this._forceShowExpand = has;
this.requestUpdate('_forceShowExpand', oldValue);
});
this.observe(this.#api.icon, (icon) => (this.#icon = icon || ''));
this.observe(this.#api.flags, (flags) => (this._flags = flags || ''));
// Observe noAccess from context and update base class property (_noAccess).
// This enables access restriction behavior (click prevention) and styling from the base class.
this.observe(this.#api.noAccess, (noAccess) => (this._noAccess = noAccess));
}

super.api = value;
}
public override get api(): UmbDocumentTreeItemContext | undefined {
return this.#api;
}

@state()
private _name = '';
Expand All @@ -43,27 +45,8 @@ export class UmbDocumentTreeItemElement extends UmbTreeItemElementBase<
@property({ type: Boolean, reflect: true, attribute: 'draft' })
protected _isDraft = false;

/**
* @internal
* Indicates whether the user has no access to this document, this is controlled internally but present as an attribute as it affects styling.
*/
@property({ type: Boolean, reflect: true, attribute: 'no-access' })
protected _noAccess = false;

#icon: string | null | undefined;

constructor() {
super();
this.addEventListener('click', this.#handleClick);
}

#handleClick = (event: MouseEvent) => {
if (this._noAccess) {
event.preventDefault();
event.stopPropagation();
}
};

// eslint-disable-next-line @typescript-eslint/no-unused-vars
protected override _extractFlags(item: UmbDocumentTreeItemModel | undefined) {
// Empty on purpose and NOT calling super to prevent doing what the base does. [NL]
Expand All @@ -85,7 +68,9 @@ export class UmbDocumentTreeItemElement extends UmbTreeItemElementBase<
};

override renderLabel() {
return html`<span id="label" slot="label" class=${classMap({ draft: this._isDraft })}>${this._name}</span> `;
return html`<span id="label" slot="label" class=${classMap({ draft: this._isDraft, noAccess: this._noAccess })}>
${this._name}
</span> `;
}

static override styles = [
Expand All @@ -97,16 +82,6 @@ export class UmbDocumentTreeItemElement extends UmbTreeItemElementBase<
:host([draft]) umb-icon {
opacity: 0.6;
}
:host([no-access]) {
cursor: not-allowed;
}
:host([no-access]) #label {
opacity: 0.6;
font-style: italic;
}
:host([no-access]) umb-icon {
opacity: 0.6;
}
`,
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,86 +72,96 @@
#generateRoutes() {
if (!this.#variants || this.#variants.length === 0 || !this.#appCulture) {
this._routes = [];
this.#ensureForbiddenRoute(this._routes);
return;
}

// Generate split view routes for all available routes
const routes: Array<UmbRoute> = [];

// Split view routes:
this.#variants.forEach((variantA) => {
this.#variants!.forEach((variantB) => {
routes.push({
// TODO: When implementing Segments, be aware if using the unique still is URL Safe, cause its most likely not... [NL]
path: variantA.unique + '_&_' + variantB.unique,
preserveQuery: true,
component: this._splitViewElement,
setup: (_component, info) => {
// Set split view/active info..
this.#workspaceContext?.splitView.setVariantParts(info.match.fragments.consumed);
},
});
});
});

// Single view:
this.#variants.forEach((variant) => {
routes.push({
// TODO: When implementing Segments, be aware if using the unique still is URL Safe, cause its most likely not... [NL]
path: variant.unique,
preserveQuery: true,
component: this._splitViewElement,
setup: (_component, info) => {
// cause we might come from a split-view, we need to reset index 1.
this.#workspaceContext?.splitView.removeActiveVariant(1);
this.#workspaceContext?.splitView.handleVariantFolderPart(0, info.match.fragments.consumed);
},
});
});

if (routes.length !== 0) {
// Using first single view as the default route for now (hence the math below):
routes.push({
path: '',
preserveQuery: true,
pathMatch: 'full',
resolve: async () => {
if (!this.#workspaceContext) {
throw new Error('Workspace context is not available when resolving the default route.');
}

// get current get variables from url, and check if openCollection is set:
const urlSearchParams = new URLSearchParams(window.location.search);
const openCollection = urlSearchParams.has('openCollection');

// Is there a path matching the current culture?
let path = routes.find((route) => route.path === this.#appCulture)?.path;

if (!path) {
// if not is there then a path matching the first variant unique.
path = routes.find((route) => route.path === this.#variants?.[0]?.unique)?.path;
}

if (!path) {
// If not is there then a path matching the first variant unique that is not a culture.
// TODO: Notice: here is a specific index used for fallback, this could be made more solid [NL]
path = routes[routes.length - 3].path;
}

history.replaceState({}, '', `${this.#workspaceRoute}/${path}${openCollection ? `/view/collection` : ''}`);
},
});
}

this.#ensureForbiddenRoute(routes);

this._routes = routes;
}

Check notice on line 150 in src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/document-workspace-editor.element.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ Getting better: Complex Method

UmbDocumentWorkspaceEditorElement.generateRoutes decreases in cyclomatic complexity from 17 to 16, 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.

/**
* Ensure that there is a route to handle forbidden access.
* This route will display a forbidden message when the user does not have permission to access certain resources.
* Also handles not found routes.
*/
#ensureForbiddenRoute(routes: Array<UmbRoute> = []) {
routes.push({
path: '**',
component: async () => {
const router = await import('@umbraco-cms/backoffice/router');
return this.#isForbidden ? router.UmbRouteForbiddenElement : router.UmbRouteNotFoundElement;
},
});

this._routes = routes;
}

private _gotWorkspaceRoute = (e: UmbRouterSlotInitEvent) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,47 +1,18 @@
import type { UmbMediaTreeItemModel } from '../types.js';
import type { UmbMediaTreeItemContext } from './media-tree-item.context.js';
import { css, html, customElement, nothing, property } from '@umbraco-cms/backoffice/external/lit';
import { UmbTextStyles } from '@umbraco-cms/backoffice/style';
import { css, html, customElement, nothing, classMap } from '@umbraco-cms/backoffice/external/lit';
import { UmbTreeItemElementBase } from '@umbraco-cms/backoffice/tree';

const elementName = 'umb-media-tree-item';
@customElement(elementName)
export class UmbMediaTreeItemElement extends UmbTreeItemElementBase<UmbMediaTreeItemModel, UmbMediaTreeItemContext> {
#api: UmbMediaTreeItemContext | undefined;

@property({ type: Object, attribute: false })
public override get api(): UmbMediaTreeItemContext | undefined {
return this.#api;
}
public override set api(value: UmbMediaTreeItemContext | undefined) {
this.#api = value;

if (this.#api) {
this.observe(this.#api.noAccess, (noAccess) => (this._noAccess = noAccess || false));
}

// Observe noAccess from context and update base class property (_noAccess).
// This enables access restriction behavior (click prevention) and styling from the base class.
this.observe(value?.noAccess, (noAccess) => (this._noAccess = noAccess ?? false));
super.api = value;
}

/**
* @internal
* Indicates whether the user has no access to this media item, this is controlled internally but present as an attribute as it affects styling.
*/
@property({ type: Boolean, reflect: true, attribute: 'no-access' })
protected _noAccess = false;

constructor() {
super();
this.addEventListener('click', this.#handleClick);
}

#handleClick = (event: MouseEvent) => {
if (this._noAccess) {
event.preventDefault();
event.stopPropagation();
}
};

override renderIconContainer() {
const icon = this.item?.mediaType.icon;

Expand All @@ -58,7 +29,9 @@ export class UmbMediaTreeItemElement extends UmbTreeItemElementBase<UmbMediaTree
}

override renderLabel() {
return html`<span id="label" slot="label">${this._item?.variants[0].name}</span> `;
return html`<span id="label" slot="label" class=${classMap({ noAccess: this._noAccess })}>
${this._item?.variants[0].name}
</span> `;
}

#renderStateIcon() {
Expand All @@ -74,7 +47,7 @@ export class UmbMediaTreeItemElement extends UmbTreeItemElementBase<UmbMediaTree
}

static override styles = [
UmbTextStyles,
...UmbTreeItemElementBase.styles,
css`
#icon-container {
position: relative;
Expand Down Expand Up @@ -128,18 +101,6 @@ export class UmbMediaTreeItemElement extends UmbTreeItemElementBase<UmbMediaTree
[disabled] #state-icon {
background-color: var(--uui-color-disabled);
}

/** No Access */
:host([no-access]) {
cursor: not-allowed;
}
:host([no-access]) #label {
opacity: 0.6;
font-style: italic;
}
:host([no-access]) umb-icon {
opacity: 0.6;
}
`,
];
}
Expand Down
Loading
Loading