fix(content-drive): open the folder context menu from the sidebar tree #36595 - #37029
Open
ihoffmann-dot wants to merge 2 commits into
Open
fix(content-drive): open the folder context menu from the sidebar tree #36595#37029ihoffmann-dot wants to merge 2 commits into
ihoffmann-dot wants to merge 2 commits into
Conversation
Open
5 tasks
Contributor
|
Claude finished @ihoffmann-dot's task in 3m 56s —— View job PR Review — folder context menu from the sidebar treeReviewed all 22 changed files against New Issues
Notes (non-blocking, not defects)
Verified correct
No blocking issues. The single Medium is a rare, acknowledged edge and does not block merge. · branch |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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-menuis 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 thepermissionsand folder detail fields #36889 added toGET /api/v1/folder/search.What changed
A right-click on a tree node opens the shared menu.
dot-tree-foldergains arightClickoutput; the sidebar turns it intopatchContextMenu({ 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:
per_pagegetFolderNodesByPathincludePermissions=truegetFolderHierarchyByPathRequesting 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.
nulland[]are kept distinct end to end. The endpoint returnspermissions: nullwhen 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 mapsnull → 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 aTypeErrorinstead of degrading to "no actions". Now?..Bug fixed here: the Edit-folder dialog built the wrong path
#getAssetPathanchored onstore.path()— the folder currently open in the drive — rather than on the folder being edited: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:
DotContentDriveActionableFolderThe 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 onlyownerandmodDateare 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
DotContentDriveActionableFolderthatDotContentDriveFolderextends.isFolder()narrows to it, so aDotContentDriveItemstill narrows to the fullDotContentDriveFolderat the table's call sites. No table behavior changes.Testing
portlets-content-drive1063/1063,content-drive-ui186/186,data-access752/752 — all green, pluslintclean 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 whileundefinedtriggers 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'snull/[]/populated cases;getParentPath;getFolderPermissionsByPath(query shape, short-name fallback, match-by-id, not-found); the service'sincludePermissionsparam; 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-testinglint errors, 1dotcms-modelslint warning, 4mocks.tscategoryIdtype errors. Untouched.Notes for the reviewer
node.datainto adata-json-nodeDOM 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.per_pagecap 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
DotFolderTransformerImpl.contentDriveView's per-folder N+1 permission loop (tracked by Batch folder permission resolution into a single round-trip per page #36940).🤖 Generated with Claude Code
This PR fixes: #36595