Skip to content

task(content-drive): add permissions tab when editing folders #35160#35327

Merged
zJaaal merged 9 commits intomainfrom
35160-task-content-drive-add-permissions-tab-when-editing-folders
Apr 16, 2026
Merged

task(content-drive): add permissions tab when editing folders #35160#35327
zJaaal merged 9 commits intomainfrom
35160-task-content-drive-add-permissions-tab-when-editing-folders

Conversation

@zJaaal
Copy link
Copy Markdown
Member

@zJaaal zJaaal commented Apr 15, 2026

Summary

  • Added Edit Permissions option to the folder context menu in Content Drive, shown only when the user has EDIT_PERMISSIONS permission on the folder
  • Opens DotPermissionsIframeDialogComponent loading /html/portlet/ext/folders/permissions.jsp in an iframe
  • Added permissions.jsp to serve the existing permissions portlet in popup mode for folders
  • Changed permissions field in DotContentDriveFolder from number[] to string[] and added PERMISSIONS_TYPE const enum
  • Replaced page links with current page report (Page {currentPage}) in folder list view pagination

Test plan

  • Right-click a folder where you have EDIT_PERMISSIONS — verify Edit Permissions appears in the context menu
  • Right-click a folder where you lack EDIT_PERMISSIONS — verify the option is absent
  • Click Edit Permissions — verify the dialog opens with the permissions portlet loaded
  • Verify dialog closes with ESC key and X button
  • Run unit tests: yarn nx test dot-content-drive

🤖 Generated with Claude Code

This PR fixes: #35160

This PR fixes: #35160

@zJaaal zJaaal linked an issue Apr 15, 2026 that may be closed by this pull request
5 tasks
@github-actions github-actions bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code labels Apr 15, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 15, 2026

Claude finished @zJaaal's task in 3m 9s —— View job


PR Review

  • Read changed files
  • Review model changes
  • Review context menu component
  • Review spec file
  • Review JSP and HTML changes
  • Post findings

Three issues worth a second look:

1. Memoization skip for no-permission folders (dot-folder-list-context-menu.component.ts:168–169)

When folderMenuItems is empty the function returns early — before $memoizedMenuItems.set(). Every subsequent right-click on the same no-permission folder re-evaluates the two .includes() checks. The work is trivial (two array scans), but the behaviour is inconsistent: a folder that later gains permissions will still get cached on the next click, while a folder that remains empty never does. If you want strict consistency, memoize the empty array too and only skip contextMenu()?.show(). If the current behaviour is intentional (lets permission grants surface without a full reload), add a comment explaining it — the silent asymmetry will puzzle the next reader.

2. Unrelated changes in examples/nextjs (DestinationListing.js, RecommendedCard.js)

Two console.log removals are bundled into this PR. They're harmless, but they don't belong here and complicate the diff. Should be a separate commit/PR or squashed with a note.

3. JSP: no server-side EDIT_PERMISSIONS guard before rendering the portlet (permissions.jsp:8–18)

The new JSP loads the permissions portlet for any authenticated user who can find() the folder — the only gate is the Angular menu check, which is client-side. In isolation this looks like a missing authorization layer, but it is the established pattern: contentlet/permissions.jsp and categories/permissions.jsp follow the exact same shape. The edit_permissions_tab_js_inc.jsp performs a server-side doesUserHavePermissionsToEdit check before rendering editable controls, so editing is properly protected. Worth documenting with a comment in the JSP so future reviewers don't flag it as a bug. Optional: explicitly short-circuit and redirect/return early if !signedIn or if the folder's permission check fails.


Everything else looks clean: PermissionType union narrowing is correct, URLSearchParams construction is injection-safe, popup=true is correctly handled by top_inc.jsp (line 214), the DotPermissionsIframeDialogComponent same-origin URL guard will reject any attacker-crafted absolute URL, and the dialog is correctly configured with closable: true / closeOnEscape: true per CLAUDE.md rules.

Comment thread core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts Outdated
zJaaal and others added 2 commits April 15, 2026 13:55
- Derive PermissionType union from PERMISSIONS_TYPE const and narrow
  DotContentDriveFolder.permissions from string[] to PermissionType[]
- Replace .find() with .includes() for permission checks in context menu
- Add test for folder with empty permissions yielding zero menu items

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@zJaaal zJaaal marked this pull request as ready for review April 15, 2026 17:01
…ng-folders' of https://github.com/dotCMS/core into 35160-task-content-drive-add-permissions-tab-when-editing-folders
@zJaaal zJaaal added this pull request to the merge queue Apr 16, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 16, 2026
@zJaaal zJaaal added this pull request to the merge queue Apr 16, 2026
Merged via the queue into main with commit de8264f Apr 16, 2026
50 checks passed
@zJaaal zJaaal deleted the 35160-task-content-drive-add-permissions-tab-when-editing-folders branch April 16, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[TASK] Content Drive: add Permissions tab when editing folders

4 participants