Skip to content

fix(content-drive): open the folder context menu from the sidebar tree #36595 - #37029

Open
ihoffmann-dot wants to merge 2 commits into
mainfrom
issue-36595-sidebar-folder-context-menu-fe
Open

fix(content-drive): open the folder context menu from the sidebar tree #36595#37029
ihoffmann-dot wants to merge 2 commits into
mainfrom
issue-36595-sidebar-folder-context-menu-fe

Conversation

@ihoffmann-dot

Copy link
Copy Markdown
Member

Parent Issue

#36595

Summary

Frontend half of #36595, on top of the backend half (#36889). Right-clicking a folder in the Content Drive sidebar tree did nothing, while right-clicking a folder row in the table opened a context menu (Edit folder / Edit permissions).

The shared dot-folder-list-context-menu is already rendered in the shell and driven by the store, so the sidebar does not need its own menu — it needs to publish the right-clicked folder to the store in the shape the menu and the "Edit folder" dialog consume. That is what this PR wires, using the permissions and folder detail fields #36889 added to GET /api/v1/folder/search.

What changed

A right-click on a tree node opens the shared menu. dot-tree-folder gains a rightClick output; the sidebar turns it into patchContextMenu({ triggeredEvent, contentlet }), exactly as the table's rows do. The synthetic "All folders" root and "Load more" sentinels are not folders, so they keep the native browser menu rather than swallowing the event for no reason.

Permissions are requested where they are cheap, and resolved on demand where they are not. The tree is filled by two different calls, and only one of them may ask for permissions:

Call When per_page Permissions
getFolderNodesByPath expanding a node, load-more 40 includePermissions=true
getFolderHierarchyByPath first render / deep-link restore 10000 ❌ exceeds the backend cap (200) — would 400

Requesting them only on expand would have left right-click dead on exactly the folders visible when the portlet opens — the issue's original symptom. So nodes hydrated by the hierarchy load resolve their permissions on first right-click (one name-narrowed lookup against the folder's own level) and the result is cached back onto the node; a second right-click is instant. Nodes from an expand never reach that path.

null and [] are kept distinct end to end. The endpoint returns permissions: null when they were not requested and [] when they were and the user holds none. Collapsing both to [] at the adapter would have made "not fetched yet" indistinguishable from "no rights" — and the on-demand resolution above depends on telling them apart. The adapter maps null → undefined (the optional-field idiom) and leaves [] intact; only the object handed to the menu is guaranteed to carry an array.

Gating no longer throws on a folder without permissions. contentlet.permissions.includes(...) had no optional chaining, so any folder reaching the menu without the field — an older backend, a search that did not opt in, a rollback of #36889 — threw a TypeError instead of degrading to "no actions". Now ?..

Bug fixed here: the Edit-folder dialog built the wrong path

#getAssetPath anchored on store.path() — the folder currently open in the drive — rather than on the folder being edited:

assetPath = //hostname + store.path() + '/' + folderName + '/'

From the table this is right by coincidence: the table lists the children of the open folder, so the two always agree. From the sidebar they do not — you can right-click any folder at any depth. Editing /campaigns/2026/marketing-assets/ while /documents/ is open would have PUT to //host/documents/marketing-assets/: a 404, or worse a silent overwrite of a same-named folder under the open path.

Create still anchors on the open folder (that is where a new folder lands); edit now derives the parent from the edited folder's own path. Both sources already carry a full path, so one fix serves the table and the sidebar. Two tests cover it — a folder in another branch, and a root-level folder.

This was not in the original FE scope, but the feature cannot save correctly without it, so it is fixed here rather than deferred.

Typing: DotContentDriveActionableFolder

The menu and dialog need 11 folder fields; DotContentDriveFolder (the table's row) has 21. The 10 extras — modDate, owner, iDate, description, mimeType, hostId, hasTitleImage, __icon__, extension, parent — are not returned by /folder/search, and only owner and modDate are read anywhere (the table's own template).

Rather than fabricate them for sidebar folders or loosen the table's type, both views converge on a narrow DotContentDriveActionableFolder that DotContentDriveFolder extends. isFolder() narrows to it, so a DotContentDriveItem still narrows to the full DotContentDriveFolder at the table's call sites. No table behavior changes.

Testing

portlets-content-drive 1063/1063, content-drive-ui 186/186, data-access 752/752 — all green, plus lint clean on all three.

New coverage: right-click emits/suppresses per node kind; the sidebar publishes the exact payload shape; already-resolved permissions are not refetched; [] is treated as a final answer while undefined triggers a lookup; the resolved value is cached onto the node; an unresolved lookup opens an empty menu instead of throwing and does not poison the cache; the adapter's null/[]/populated cases; getParentPath; getFolderPermissionsByPath (query shape, short-name fallback, match-by-id, not-found); the service's includePermissions param; and the two dialog path cases above.

Three pre-existing tests were updated rather than loosened. Their folder fixtures used path: '', which is not a real folder path and is what let the dialog bug pass unnoticed; giving them realistic own-paths keeps the same expectations while making them verify the fix.

Unrelated pre-existing failures confirmed by stashing and re-running: 37 utils-testing lint errors, 1 dotcms-models lint warning, 4 mocks.ts categoryId type errors. Untouched.

Notes for the reviewer

  • The tree writes node.data into a data-json-node DOM attribute for drag-and-drop, so the seven new node fields ride along on every node. Small per node, but it is a real (pre-existing) pattern worth knowing about if the payload grows further.
  • Batch folder permission resolution into a single round-trip per page #36940 (batch folder permission resolution) would remove the reason the per_page cap exists. If that cap is ever raised or dropped, the hierarchy load could request permissions directly and the on-demand lookup here becomes dead code — worth revisiting together.

Explicitly out of scope

🤖 Generated with Claude Code

This PR fixes: #36595

@claude

claude Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ihoffmann-dot's task in 3m 56s —— View job


PR Review — folder context menu from the sidebar tree

Reviewed all 22 changed files against origin/main (the PR's two commits on top of 75dd2f1b). Traced the right-click → permission-resolution → shared-menu path, the null vs [] distinction end to end, the type narrowing, and the dialog path fix.

New Issues

  • 🟡 Medium: core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts:379getFolderPermissionsByPath can leave permissions permanently unresolved for a folder whose name is a single character and whose level has more than FOLDER_PERMISSIONS_LOOKUP_PAGE_SIZE (200) siblings. In that case the name filter is dropped (short-name fallback), the query returns only the first 200 siblings ordered by name ASC, and folders.find(f => f.id === folderId) returns undefinedloadFolderPermissions emits undefined → the node is never cached and its context menu opens empty on every right-click (a silent, per-right-click refetch). The short-name fallback is tested, but the ">200 siblings, target not on page 1" combination isn't. Rare and already acknowledged in spirit by the PR, so non-blocking — but worth a follow-up (or a comment noting the limitation) since Batch folder permission resolution into a single round-trip per page #36940's batch resolution would remove it. Fix this →

Notes (non-blocking, not defects)

  • withSidebar.ts:157 uses console.error + return of(undefined) rather than DotHttpErrorManagerService. This is intentional graceful degradation (a failed context-menu permission lookup should not pop an error dialog) and is consistent with the adjacent existing loadFolders (withSidebar.ts:74,76), so not flagged as a new issue.
  • The on-demand permissions cache mutates node.data in place (sidebar.component.ts onNodeRightClick). The [attr.data-json-node] binding uses the pure json pipe, so the stale drag-and-drop attribute won't refresh — harmless, since permissions aren't part of the drag payload.

Verified correct

  • null vs [] preserved end to end: adapter maps null → undefined, keeps []; onNodeRightClick treats truthy [] as a final answer (opens immediately) and only undefined triggers a lookup.
  • ✅ Dialog path fix (#getParentPath): create anchors on store.path(), edit derives the parent from the edited folder's own path. Checked /a/b/c/ → /a/b, root /x/ → '', and the join in #getAssetPath matches the previous create output.
  • getParentPath boundaries: /a/b/ → /a/, /b/ → /.
  • ✅ Type narrowing: DotContentDriveFolder extends DotContentDriveActionableFolder; the dialog's save uses assetPath (not inode/identifier/table-only fields), so the narrower sidebar folder saves correctly.
  • includePermissions only sent on the 40-per-page expand call, never on the 10 000-per-page hierarchy load (would 400); on-demand lookup capped at 200 to match the backend.
  • ✅ Optional chaining in dot-folder-list-context-menu.component.ts degrades to "no actions" instead of throwing.
  • rightClick suppressed for ALL_FOLDER / LOAD_MORE sentinels (native menu preserved); event.preventDefault() only on real folders.

No blocking issues. The single Medium is a rare, acknowledged edge and does not block merge.

· branch issue-36595-sidebar-folder-context-menu-fe

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

Labels

Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[BUG] Content Drive: Sidebar folders don't open a right-click context menu like the table

1 participant