Skip to content

Global Elements: Adds "Element or Element Folder" User Permission Condition - #23051

Merged
AndyButland merged 5 commits into
mainfrom
v18/feature/element-joint-item-folder-conditon
Jul 13, 2026
Merged

Global Elements: Adds "Element or Element Folder" User Permission Condition#23051
AndyButland merged 5 commits into
mainfrom
v18/feature/element-joint-item-folder-conditon

Conversation

@leekelleher

@leekelleher leekelleher commented Jun 3, 2026

Copy link
Copy Markdown
Member

Description

There is an issue where the contents of the Element Library Recycle Bin is visible to users without "Read" permission. (This is the same issue that PR #23041 resolves for the Document Recycle Bin).

The complexity with the Element Library is that we have separate permissions for Elements and Element Folders, where a user could have various combinations of permission, e.g. "Read" on Element Folders but not Elements. So we needed a condition mechanism to handle the OR logic.

This new condition also resolves an issue with the Element Library's "Create" entity-action, e.g. where a user could have "Create" on Element Folders but not Elements.

Summary (AI/Claude generated) 🤖

  • Adds UmbElementOrElementFolderUserPermissionCondition — a new condition that permits when a user has the configured element OR element folder permissions (AND-within-each-side, OR-across-sides). This is needed because the manifest conditions array is hard-AND, so an OR across permission types requires a dedicated combinator.
  • Gates the Element Library recycle bin (menu item + workspace collection view) on Umb.Element.Read OR Umb.ElementContainer.Read, replacing the previous element-read-only check that wrongly hid the bin from folder-read-only users.
  • Gates the Create entity action on Umb.Element.Create OR Umb.ElementContainer.Create.
  • Gates the Create options modal's element type listing on Umb.Element.Create via createExtensionApiByAlias, so folder-only users see only the folder create option, not element types.

Test plan

  • User with element read only → recycle bin visible, workspace collection visible
  • User with folder read only → recycle bin visible, workspace collection visible
  • User with neither → recycle bin hidden
  • User with folder create only → Create action visible, modal shows only folder option (no element types)
  • User with element create → Create action visible, modal shows element types
  • Unit tests: npm test -- --files "src/packages/elements/user-permissions/conditions/element-or-element-folder-user-permission.condition.test.ts" — all 4 OR-logic cases pass

Adds a combined permission condition that permits when a user has read
access to elements OR element folders, using the OR semantics needed
for the element recycle bin (which contains both entity types).

The condition composes UmbElementUserPermissionCondition and
UmbElementFolderUserPermissionCondition as child controllers and exposes
configurable element/folder verb blocks (allOf/oneOf each side).
…mission

The element recycle bin contains both trashed elements and trashed element
folders, so users with only folder read access had legitimate contents but
the bin was hidden from them. Replaces the element-read-only condition with
UmbElementOrElementFolderUserPermissionCondition on both the menu item and
the root workspace collection view.
Adds UmbElementUserPermissionCondition via createExtensionApiByAlias to
the create-options modal; allowed element types are only fetched when the
user has element-create permission. Users with folder-create permission
only will see the folder option without element types.

Also applies UmbElementOrElementFolderUserPermissionCondition to the
Create entity action so it appears for users with either permission.
@claude

claude Bot commented Jun 3, 2026

Copy link
Copy Markdown

Claude finished @leekelleher's task in 7m 41s —— View job


PR Review

Target: origin/main · Based on commit: e87638a5 · Skipped: 1 file out of 11 total (user-group.data.ts — mock fixture)

Adds UmbElementOrElementFolderUserPermissionCondition, a composable condition that short-circuit-ORs an element permission check against an element-folder permission check, then gates the element recycle bin menu/workspace-view and the Create entity action on the combined condition.

  • Modified public API: UmbExtensionConditionConfigMap extended with UmbElementOrElementFolderUserPermissionConditionConfig
  • Other changes: Recycle bin now visible to folder-read-only users (previously hidden); Create action now visible to folder-create-only users; element type listing in the Create modal is separately gated on element-create permission only.

Important

  • element-or-element-folder-user-permission.condition.test.ts:133-138: The "Neither" case asserts condition.permitted === false after a 200ms setTimeout, but UmbConditionBase initialises permitted to false by default. If sub-conditions haven't evaluated within 200ms (slow CI), the assertion passes via the default — not because the logic was verified. More critically, if onChange fires after done() is called (evaluation > 200ms, and erroneously returns true), Mocha silently drops the error. Consider waiting for a deterministic signal (both sub-conditions having evaluated), or at minimum add a comment explaining why 200ms is safe for the mock environment. Fix this →

Suggestions

  • types.ts:19-22: UmbPermissionVerbsConfig is not exported. External extension developers referencing UmbElementOrElementFolderUserPermissionConditionConfig would need to redeclare this shape to build similar combinators. Consider exporting it. Fix this →

  • element-create-options-modal.element.ts:44-48: #retrieveAllowedElementTypes() fires every time permitted transitions to true. In a normal modal session this is once, but if the user context is refreshed (auth refresh), a duplicate request would fire. A simple if (permitted && this._allowedElementTypes.length === 0) guard is sufficient to make it idempotent. Fix this →


Approved with Suggestions for improvement

The OR-combinator pattern is clean and idiomatic — sub-conditions are composed as child controllers, context observation propagates correctly, and the manifest registrations are consistent. Logic is correct for all four scenarios described in the test plan. No breaking changes.

Labels applied: area/frontend, category/ux, category/ui

@leekelleher leekelleher changed the title Elements: Add UmbElementOrElementFolderUserPermissionCondition and gate recycle bin / create modal on combined permissions Global Elements: Adds "Element or Element Folder" User Permission Condition Jun 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new extension-registry condition to express OR logic across Element vs Element Folder (container) permissions, then uses it to correctly gate the Element Library recycle bin visibility and the “Create” entry points (including limiting element type listing in the create-options modal).

Changes:

  • Introduces UmbElementOrElementFolderUserPermissionCondition (+ config types, alias constant, manifest registration) to support (element permissions) OR (folder permissions).
  • Updates Element recycle bin menu item + root workspace view to permit Umb.Element.Read OR Umb.ElementContainer.Read.
  • Updates Element “Create” entity action gating to permit Umb.Element.Create OR Umb.ElementContainer.Create, and gates the create-options modal element type listing on Umb.Element.Create only.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Umbraco.Web.UI.Client/src/packages/elements/user-permissions/conditions/types.ts Adds the new condition config type (element vs folder verb sets) and extends the global config map.
src/Umbraco.Web.UI.Client/src/packages/elements/user-permissions/conditions/constants.ts Defines the new condition alias constant.
src/Umbraco.Web.UI.Client/src/packages/elements/user-permissions/conditions/manifests.ts Registers the new condition as a condition manifest.
src/Umbraco.Web.UI.Client/src/packages/elements/user-permissions/conditions/index.ts Exposes the new condition from the package index.
src/Umbraco.Web.UI.Client/src/packages/elements/user-permissions/conditions/element-or-element-folder-user-permission.condition.ts Implements the OR-combinator condition (Element vs Element Folder permissions).
src/Umbraco.Web.UI.Client/src/packages/elements/user-permissions/conditions/element-or-element-folder-user-permission.condition.test.ts Adds unit tests covering the OR logic permutations.
src/Umbraco.Web.UI.Client/src/packages/elements/recycle-bin/root/workspace/manifests.ts Gates the recycle bin root collection view on combined read permissions.
src/Umbraco.Web.UI.Client/src/packages/elements/recycle-bin/menu/manifests.ts Gates recycle bin menu item visibility on combined read permissions.
src/Umbraco.Web.UI.Client/src/packages/elements/entity-actions/create/manifests.ts Gates the Create action on combined create permissions (element OR folder).
src/Umbraco.Web.UI.Client/src/packages/elements/entity-actions/create/element-create-options-modal.element.ts Loads allowed element types only if the user has element-create permission (folder-only users won’t see element types).
src/Umbraco.Web.UI.Client/mocks/data/sets/user-permissions/user-group.data.ts Extends mock permission data to support the new unit test scenarios.

claude[bot]

This comment was marked as resolved.

claude[bot]

This comment was marked as resolved.

claude[bot]

This comment was marked as resolved.

@claude claude Bot added category/ux User experience category/ui User interface labels Jun 3, 2026
…ssion condition

- Only instantiate sub-conditions when the matching config side is provided,
  preventing always-permit when element/folder config is omitted
- Add idempotency guard in create modal onChange to avoid duplicate fetches
- Export UmbPermissionVerbsConfig for external reuse
- Document why 200ms timeout is safe in the neither-permitted test
@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2026

Copy link
Copy Markdown

@AndyButland AndyButland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good to me @leekelleher, and I've run through the tests cases to make sure they all check out.

@AndyButland
AndyButland merged commit 655b75c into main Jul 13, 2026
37 of 39 checks passed
@AndyButland
AndyButland deleted the v18/feature/element-joint-item-folder-conditon branch July 13, 2026 06:30
AndyButland added a commit that referenced this pull request Jul 13, 2026
UmbElementUserPermissionCondition evaluated its permission state on every emit of the current-user context, including the `undefined` emits that occur before the user loads and during teardown. Because `#elementPermissions`/`#fallbackPermissions` defaulted to `[]`, those `undefined` emits recomputed against empty arrays and flipped `permitted` back to `false`.

This surfaced through the new combined condition (UmbElementOrElementFolderUserPermissionCondition, #23051): in the test's success path `permitted` correctly went false -> true, but the teardown triggered by hostDisconnected() re-emitted the current-user context as `undefined`, flipping the child element condition (and therefore the combined condition) back to false and re-firing onChange. The assertion then ran against a `false` value and threw uncaught, failing all four tests in the file.

Mirror the sibling UmbElementFolderUserPermissionCondition (#22274), which  already solved this: keep `#elementPermissions`/`#fallbackPermissions` undefined until the current user has loaded and return early from #checkPermissions while they are undefined, so we never permit/deny on incomplete data and never re-emit a spurious `false` on teardown.
@AndyButland

Copy link
Copy Markdown
Contributor

Just to add @leekelleher - I saw we had the FE test check failing on main after I merged this, which I've fixed via f176100.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants