From 74abe2f50c9784288ba5009f2e855c59bcd864ea Mon Sep 17 00:00:00 2001 From: ihoffmann-dot Date: Tue, 11 Aug 2026 19:49:41 -0300 Subject: [PATCH 1/6] fix(content-drive): open the folder context menu from the sidebar tree (#36595) --- .../lib/dot-folder/dot-folder.service.spec.ts | 23 ++ .../src/lib/dot-folder/dot-folder.service.ts | 6 + .../src/lib/dot-content-drive.model.ts | 61 +++- .../dotcms-models/src/lib/dot-folder.model.ts | 48 +++- ...tent-drive-dialog-folder.component.spec.ts | 73 ++++- ...t-content-drive-dialog-folder.component.ts | 53 +++- .../dot-content-drive-sidebar.component.html | 1 + ...ot-content-drive-sidebar.component.spec.ts | 151 +++++++++- .../dot-content-drive-sidebar.component.ts | 72 ++++- .../dot-folder-list-context-menu.component.ts | 7 +- .../dot-content-drive-shell.component.ts | 4 +- .../portlet/src/lib/shared/constants.ts | 11 + .../portlet/src/lib/shared/models.ts | 11 +- .../lib/store/features/sidebar/withSidebar.ts | 42 ++- .../portlet/src/lib/utils/functions.spec.ts | 265 ++++++++++++++++-- .../portlet/src/lib/utils/functions.ts | 102 ++++++- .../src/lib/utils/tree-folder.utils.ts | 12 +- .../dot-tree-folder.component.html | 3 +- .../dot-tree-folder.component.spec.ts | 76 ++++- .../dot-tree-folder.component.ts | 26 +- .../ui/src/lib/shared/models.ts | 35 ++- .../utils-testing/src/lib/dot-folder.mock.ts | 13 +- 22 files changed, 1015 insertions(+), 80 deletions(-) diff --git a/core-web/libs/data-access/src/lib/dot-folder/dot-folder.service.spec.ts b/core-web/libs/data-access/src/lib/dot-folder/dot-folder.service.spec.ts index 5e5505f4ad1e..e2d158f00c1e 100644 --- a/core-web/libs/data-access/src/lib/dot-folder/dot-folder.service.spec.ts +++ b/core-web/libs/data-access/src/lib/dot-folder/dot-folder.service.spec.ts @@ -210,6 +210,29 @@ describe('DotFolderService', () => { const req = spectator.expectOne(url, HttpMethod.GET); req.flush({ entity: [], pagination: { currentPage: 1, perPage: 40, totalEntries: 0 } }); }); + + it('should send includePermissions when the caller opts in', () => { + spectator.service + .searchFolders({ siteId: 'site-1', includePermissions: true }) + .subscribe(); + + const url = + '/api/v1/folder/search?siteId=site-1&includePermissions=true&page=1&per_page=40'; + const req = spectator.expectOne(url, HttpMethod.GET); + req.flush({ entity: [], pagination: mockPagination }); + }); + + it('should omit includePermissions entirely when not requested', () => { + // The backend defaults it to false, so sending `includePermissions=false` on every tree + // request would be pure noise. + spectator.service + .searchFolders({ siteId: 'site-1', includePermissions: false }) + .subscribe(); + + const url = '/api/v1/folder/search?siteId=site-1&page=1&per_page=40'; + const req = spectator.expectOne(url, HttpMethod.GET); + req.flush({ entity: [], pagination: mockPagination }); + }); }); describe('createFolder', () => { diff --git a/core-web/libs/data-access/src/lib/dot-folder/dot-folder.service.ts b/core-web/libs/data-access/src/lib/dot-folder/dot-folder.service.ts index 2869ff854b0d..7de9146148be 100644 --- a/core-web/libs/data-access/src/lib/dot-folder/dot-folder.service.ts +++ b/core-web/libs/data-access/src/lib/dot-folder/dot-folder.service.ts @@ -96,6 +96,12 @@ export class DotFolderService { httpParams = httpParams.set('direction', params.direction); } + // Only sent when opting in: the backend defaults it to false and caps `per_page` when it is + // true, so an always-on `includePermissions=false` would be noise on every tree request. + if (params.includePermissions) { + httpParams = httpParams.set('includePermissions', 'true'); + } + httpParams = httpParams.set('page', String(params.page ?? DEFAULT_FOLDER_SEARCH_PAGE)); httpParams = httpParams.set( 'per_page', diff --git a/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts b/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts index 5c633c1f4d00..93677993ea0d 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts @@ -17,33 +17,58 @@ export interface DotContentDriveLazyLoadEvent { forceUpdate?: () => void; } -export interface DotContentDriveFolder { - __icon__: 'folderIcon'; +/** + * The folder fields required to drive the shared folder actions — the context menu's permission + * gating and the "Edit folder" dialog's payload. + * + * Deliberately narrower than {@link DotContentDriveFolder}: the table sources folders from + * `POST /api/v1/drive/search` (a full folder row), while the sidebar tree sources them from + * `GET /api/v1/folder/search`, which returns only the fields listed here. Rather than fabricate + * the table-only fields (`modDate`, `owner`, `iDate`, …) for sidebar folders, both views converge + * on this contract, and only consumers that genuinely need the full row ask for + * `DotContentDriveFolder`. + */ +export interface DotContentDriveActionableFolder { + type: 'folder'; + identifier: string; + /** The folder's own name (last path segment). */ + name: string; + /** The folder's own full path, e.g. `/application/blog/`. */ + path: string; + title: string; + sortOrder: number; + showOnMenu: boolean; + /** Comma-separated file-name masks allowed in this folder, e.g. `*.jpg,*.png`. */ + filesMasks: string; defaultFileType: string; /** * Folder upload preference: `DOTASSET`/`FILEASSET` forces every upload to that base type, * `null`/`undefined` means "ask each time" (no preference). Backed by #35577. */ defaultBaseType?: string | null; + /** + * Permission types the requesting user holds on this folder. + * + * Required, and always an array by the time a folder reaches an action: whoever builds this + * object resolves the folder's permissions first (or substitutes `[]` when they cannot be + * resolved), so gating never runs against `null`/`undefined`. The "not yet resolved" state + * lives upstream, on `DotFolder.permissions` / the tree node's data. + */ + permissions: PermissionType[]; +} + +export interface DotContentDriveFolder extends DotContentDriveActionableFolder { + __icon__: 'folderIcon'; description: string; extension: 'folder'; - filesMasks: string; hasTitleImage: boolean; hostId: string; iDate: number; - identifier: string; inode: string; mimeType: string; modDate: number; - name: string; owner: string | null; parent: string; - path: string; - permissions: PermissionType[]; - showOnMenu: boolean; - sortOrder: number; - title: string; - type: 'folder'; } export const PERMISSIONS_TYPE = { @@ -60,6 +85,15 @@ export type PermissionType = (typeof PERMISSIONS_TYPE)[keyof typeof PERMISSIONS_ // but for now we will just use the DotCMSContentlet until we have folders on the request response export type DotContentDriveItem = DotCMSContentlet | DotContentDriveFolder; +/** + * An item the shared folder actions (context menu, Edit-folder dialog) can act on. + * + * Wider than {@link DotContentDriveItem} on the folder side: the table passes a full + * {@link DotContentDriveFolder} and the sidebar tree passes a {@link DotContentDriveActionableFolder}, + * so one gating implementation serves both. Every `DotContentDriveItem` is assignable to this. + */ +export type DotContentDriveActionableItem = DotCMSContentlet | DotContentDriveActionableFolder; + /** * Pagination event emitted by the folder list view, * extending the lazy-load event shape with a resolved 1-indexed page number. @@ -70,11 +104,12 @@ export type DotContentDrivePaginateEvent = DotContentDriveLazyLoadEvent & { page * Interface representing data needed for context menu interactions * @interface ContextMenuData * @property {Event} event - The DOM event that triggered the context menu - * @property {DotContentDriveItem} contentlet - The content item associated with the context menu + * @property {DotContentDriveActionableItem} contentlet - The item associated with the context menu. + * Accepts folders from either the table (full row) or the sidebar tree (search view). */ export interface ContextMenuData { event: Event; - contentlet: DotContentDriveItem; + contentlet: DotContentDriveActionableItem; } /** diff --git a/core-web/libs/dotcms-models/src/lib/dot-folder.model.ts b/core-web/libs/dotcms-models/src/lib/dot-folder.model.ts index faa95dab8628..8034c6415348 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-folder.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-folder.model.ts @@ -1,3 +1,5 @@ +import { PermissionType } from './dot-content-drive.model'; + /** * Represents a folder in the DotCMS system * @@ -23,6 +25,25 @@ export interface DotFolder { * `null`/`undefined` means "ask each time" (no preference). Backed by #35577. */ defaultBaseType?: string | null; + /** + * The folder's own name (last path segment). Populated by the folder-search adapter; other + * producers leave it unset and callers fall back to deriving it from `path`. + */ + name?: string; + /** + * Fields below back the shared folder actions (context menu gating + "Edit folder" dialog) and + * are populated only by {@link FolderSearchView} sources. + * + * `permissions` stays `undefined` when the search did not request them — distinct from `[]` + * ("resolved: the user holds none"), so a consumer can resolve them on demand instead of + * silently rendering an empty menu. + */ + title?: string; + sortOrder?: number; + filesMasks?: string; + defaultFileType?: string; + showOnMenu?: boolean; + permissions?: PermissionType[]; } /** @@ -74,9 +95,24 @@ export interface FolderSearchView { hasChildren: boolean; /** * Folder upload preference (`DOTASSET`/`FILEASSET`, or `null`/absent for "ask each time"). - * Pending backend support on `/api/v1/folder/search` — see #36649. */ defaultBaseType?: string | null; + title: string; + sortOrder: number; + /** Comma-separated file-name masks allowed in this folder, e.g. `*.jpg,*.png`. */ + filesMasks: string; + /** Velocity variable name of the Content Type used by default for new files in this folder. */ + defaultFileType: string; + showOnMenu: boolean; + /** + * Permission types the requesting user holds on this folder, drawn from the same set the table + * exposes (`READ`, `EDIT`, `PUBLISH`, `EDIT_PERMISSIONS`, `CAN_ADD_CHILDREN`). + * + * `null` when the request did not pass `includePermissions=true` — deliberately distinct from + * `[]` ("requested, but the user holds none"), so callers can tell "not fetched" from "no + * grants". Consumers must normalize before gating; see `folderSearchViewToDotFolder`. + */ + permissions: PermissionType[] | null; } /** @@ -101,4 +137,14 @@ export interface FolderSearchParams { direction?: 'ASC' | 'DESC'; page?: number; per_page?: number; + /** + * Request per-folder `permissions` on each result. Off by default: resolving them costs extra + * batch permission queries per page, so only callers that are about to gate a folder action + * (e.g. opening the sidebar context menu) should opt in. + * + * The backend caps `per_page` when this is `true` + * (`content.drive.folder.search.permissions.max.per.page`, default 200) and rejects larger + * pages with a 400 — so never combine it with a bulk load such as the deep-link tree hydration. + */ + includePermissions?: boolean; } diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts index 2829a087c421..4603154274d7 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts @@ -6,7 +6,7 @@ import { MessageService } from 'primeng/api'; import { AutoComplete, AutoCompleteCompleteEvent } from 'primeng/autocomplete'; import { DotContentTypeService, DotFolderService, DotMessageService } from '@dotcms/data-access'; -import { DotContentDriveFolder } from '@dotcms/dotcms-models'; +import { DotContentDriveActionableFolder, DotContentDriveFolder } from '@dotcms/dotcms-models'; import { createFakeSite, MockDotMessageService } from '@dotcms/utils-testing'; import { DotContentDriveDialogFolderComponent } from './dot-content-drive-dialog-folder.component'; @@ -50,7 +50,7 @@ const editableFolder = (overrides: Partial = {}): DotCont modDate: 1, owner: null, parent: '', - path: '', + path: '/documents/app/', permissions: [], type: 'folder', ...overrides @@ -304,7 +304,7 @@ describe('DotContentDriveDialogFolderComponent', () => { modDate: 1234567890, owner: null, parent: '', - path: '', + path: '/documents/existing-folder/', permissions: [], type: 'folder' }; @@ -1001,7 +1001,7 @@ describe('DotContentDriveDialogFolderComponent', () => { modDate: 1234567890, owner: null, parent: '', - path: '', + path: '/documents/original-folder/', permissions: [], type: 'folder' }; @@ -1036,6 +1036,65 @@ describe('DotContentDriveDialogFolderComponent', () => { expect(lastCall?.assetPath).toBe('//demo.dotcms.com/documents/test-folder/'); }); + it('should anchor the edit assetPath on the folder itself, not the open folder', () => { + // The sidebar tree can open this dialog for any folder in the site, not just a child of + // the folder currently open in the drive (`store.path()` is '/documents'). Anchoring on + // the open path would build '//demo.dotcms.com/documents/marketing-assets/' — a + // different folder, which would 404 on save or silently overwrite a same-named sibling. + const folderInAnotherBranch: DotContentDriveActionableFolder = { + name: 'marketing-assets', + title: 'Marketing Assets', + sortOrder: 1, + filesMasks: '', + defaultFileType: 'FileAsset', + showOnMenu: false, + identifier: 'other-branch-id', + path: '/campaigns/2026/marketing-assets/', + permissions: [], + type: 'folder' + }; + + spectator.setInput('folder', folderInAnotherBranch); + spectator.detectChanges(); + + const saveButton = spectator.query( + '[data-testid="content-drive-dialog-folder-create"]' + ); + spectator.click(saveButton); + + expect(folderService.saveFolder).toHaveBeenCalled(); + const lastCall = folderService.saveFolder.mock.calls.at(-1)?.[0]; + expect(lastCall?.assetPath).toBe( + '//demo.dotcms.com/campaigns/2026/marketing-assets/' + ); + }); + + it('should anchor the edit assetPath at the site root for a root-level folder', () => { + const rootLevelFolder: DotContentDriveActionableFolder = { + name: 'archive', + title: 'Archive', + sortOrder: 1, + filesMasks: '', + defaultFileType: 'FileAsset', + showOnMenu: false, + identifier: 'root-level-id', + path: '/archive/', + permissions: [], + type: 'folder' + }; + + spectator.setInput('folder', rootLevelFolder); + spectator.detectChanges(); + + const saveButton = spectator.query( + '[data-testid="content-drive-dialog-folder-create"]' + ); + spectator.click(saveButton); + + const lastCall = folderService.saveFolder.mock.calls.at(-1)?.[0]; + expect(lastCall?.assetPath).toBe('//demo.dotcms.com/archive/'); + }); + it('should include name in data when originalName exists and name has changed', () => { // Simulate editing an existing folder const mockFolder: DotContentDriveFolder = { @@ -1057,7 +1116,7 @@ describe('DotContentDriveDialogFolderComponent', () => { modDate: 1234567890, owner: null, parent: '', - path: '', + path: '/documents/original-folder/', permissions: [], type: 'folder' }; @@ -1122,7 +1181,7 @@ describe('DotContentDriveDialogFolderComponent', () => { modDate: 1234567890, owner: null, parent: '', - path: '', + path: '/documents/original-folder/', permissions: [], type: 'folder' }; @@ -1177,7 +1236,7 @@ describe('DotContentDriveDialogFolderComponent', () => { modDate: 1234567890, owner: null, parent: '', - path: '', + path: '/documents/existing-folder/', permissions: [], type: 'folder' }; diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.ts index b4e76592e207..8d7915a9c5cc 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.ts @@ -31,7 +31,7 @@ import { TabsModule } from 'primeng/tabs'; import { ToggleSwitchModule } from 'primeng/toggleswitch'; import { DotContentTypeService, DotFolderService, DotMessageService } from '@dotcms/data-access'; -import { DotContentDriveFolder, DotFolderEntity } from '@dotcms/dotcms-models'; +import { DotContentDriveActionableFolder, DotFolderEntity } from '@dotcms/dotcms-models'; import { DotFieldRequiredDirective, DotMessagePipe } from '@dotcms/ui'; import { @@ -81,7 +81,12 @@ export class DotContentDriveDialogFolderComponent { #hostName = this.#store.currentSite().hostname; - $folder = input(undefined, { alias: 'folder' }); + /** + * The folder being edited, or `undefined` in create mode. Typed as the narrow + * {@link DotContentDriveActionableFolder} so both sources work: a full row from the table and a + * search view from the sidebar tree. + */ + $folder = input(undefined, { alias: 'folder' }); readonly $fileAssetTypes = toSignal( this.#dotContentTypeService.getContentTypes({ type: 'FILEASSET' }) @@ -424,26 +429,52 @@ export class DotContentDriveDialogFolderComponent { this.#store.closeDialog(); } + /** + * Path of the folder that will contain the folder being created or edited, without a trailing + * slash (empty at the site root). + * + * **Create** anchors on the folder currently open in the drive (`store.path()`), so the preview + * reflects where the new folder will land — even before a name is typed. + * + * **Edit** must anchor on the edited folder's *own* parent instead. The table only ever opens + * this dialog for a row of the open folder, so the two coincide there; the sidebar tree can open + * it for any folder at any depth, and anchoring on the open path would then build a path to a + * different folder entirely — saving would 404, or silently overwrite a same-named folder under + * the open one. + * + * @returns {string} The parent path, e.g. `/application/blog` or `''` at the site root + */ + #getParentPath(): string { + const folder = this.$folder(); + + if (!folder) { + return this.#store.path()?.replace(/\/$/, '') ?? ''; + } + + const withoutTrailingSlash = folder.path.replace(/\/$/, ''); + const lastSeparator = withoutTrailingSlash.lastIndexOf('/'); + + return lastSeparator <= 0 ? '' : withoutTrailingSlash.slice(0, lastSeparator); + } + /** * Generates the asset path for a given name - * Combines hostname, current path, and URL to create the complete folder path + * Combines hostname, parent path, and name to create the complete folder path * Ensures proper path formatting by removing trailing slashes * + * Reads signals in a pure computed only; it never writes a form control, so it can't + * re-introduce the form-control-write feedback loop that the old title→name `urlEffect` caused. + * * @param {string} name - The name of the folder * @returns {string} The asset path */ #getAssetPath(name: string) { const slugName = this.#getSlugTitle(name); - - // Always anchor on the currently opened folder (store.path()) so the preview reflects where - // the folder will actually be created — even before a name is typed. This reads a signal in - // a pure computed only; it never writes a form control, so it can't re-introduce the - // form-control-write feedback loop that the old title→name `urlEffect` caused. - const path = this.#store.path(); + const parentPath = this.#getParentPath(); let finalPath = this.#hostName; - if (path) { - finalPath += `${path.replace(/\/$/, '')}`; + if (parentPath) { + finalPath += parentPath; } if (!slugName) { diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.html b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.html index 44b4f033a14b..0e6d0b9f6720 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.html +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.html @@ -28,5 +28,6 @@

(onNodeExpand)="onNodeExpand($event)" (onNodeCollapse)="onNodeCollapse($event)" (loadMore)="onLoadMore($event)" + (rightClick)="onNodeRightClick($event)" (uploadFiles)="uploadFiles.emit($event)" (moveItems)="moveItems.emit($event)" /> diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts index ed9ba21aa855..b41de115429c 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts @@ -8,13 +8,14 @@ import { TreeNodeCollapseEvent, TreeNodeExpandEvent, TreeNodeSelectEvent } from import { delay } from 'rxjs/operators'; import { DotFolderService, DotMessageService } from '@dotcms/data-access'; -import { DotFolder } from '@dotcms/dotcms-models'; +import { DotFolder, PermissionType, PERMISSIONS_TYPE } from '@dotcms/dotcms-models'; import { DotContentDriveUploadFiles, DotTreeFolderComponent, DotFolderTreeNodeItem, DotContentDriveMoveItems, - ALL_FOLDER + ALL_FOLDER, + LOAD_MORE_NODE_TYPE } from '@dotcms/portlets/content-drive/ui'; import { GlobalStore } from '@dotcms/store'; @@ -124,6 +125,8 @@ describe('DotContentDriveSidebarComponent', () => { sidebarLoading: jest.fn().mockReturnValue(false), loadFolders: jest.fn(), loadChildFolders: jest.fn(), + loadFolderPermissions: jest.fn().mockReturnValue(of(undefined)), + patchContextMenu: jest.fn(), updateFolders: jest.fn(), setSelectedNode: jest.fn() }), @@ -1151,4 +1154,148 @@ describe('DotContentDriveSidebarComponent', () => { expect(contentDriveStore.loadChildFolders).not.toHaveBeenCalled(); }); }); + + describe('right-click context menu', () => { + beforeEach(() => { + // The store mock is built once by the component factory, so its jest.fn call counts + // accumulate across tests in this file. Clear them (implementations are preserved) and + // restore the default lookup so each case starts from a known state. + jest.clearAllMocks(); + contentDriveStore.loadFolderPermissions.mockReturnValue(of(undefined)); + }); + + const buildFolderNode = ( + permissions?: PermissionType[] + ): DotFolderTreeNodeItem => ({ + key: 'docs-id', + label: '/documents/reports/', + data: { + id: 'docs-id', + inode: 'docs-inode', + hostname: 'demo.dotcms.com', + path: '/documents/reports/', + type: 'folder', + name: 'reports', + title: 'Reports', + sortOrder: 2, + filesMasks: '*.pdf', + defaultFileType: 'FileAsset', + defaultBaseType: 'DOTASSET', + showOnMenu: true, + permissions + } + }); + + const rightClick = (node: DotFolderTreeNodeItem) => { + const event = new MouseEvent('contextmenu'); + spectator.triggerEventHandler(DotTreeFolderComponent, 'rightClick', { event, node }); + + return event; + }; + + it('should publish the folder to the store in the shape the menu and dialog consume', () => { + const event = rightClick(buildFolderNode([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT])); + + expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith({ + triggeredEvent: event, + contentlet: { + type: 'folder', + identifier: 'docs-id', + name: 'reports', + path: '/documents/reports/', + title: 'Reports', + sortOrder: 2, + showOnMenu: true, + filesMasks: '*.pdf', + defaultFileType: 'FileAsset', + defaultBaseType: 'DOTASSET', + permissions: [PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT] + } + }); + }); + + it('should not refetch permissions for a node that already carries them', () => { + rightClick(buildFolderNode([PERMISSIONS_TYPE.READ])); + + expect(contentDriveStore.loadFolderPermissions).not.toHaveBeenCalled(); + }); + + it('should treat an empty permissions array as resolved and not refetch', () => { + // `[]` is a final answer ("no grants"), unlike undefined ("never fetched"). + rightClick(buildFolderNode([])); + + expect(contentDriveStore.loadFolderPermissions).not.toHaveBeenCalled(); + expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith( + expect.objectContaining({ + contentlet: expect.objectContaining({ permissions: [] }) + }) + ); + }); + + it('should resolve permissions on demand for a node hydrated without them', () => { + contentDriveStore.loadFolderPermissions.mockReturnValue( + of([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT_PERMISSIONS]) + ); + + rightClick(buildFolderNode(undefined)); + + expect(contentDriveStore.loadFolderPermissions).toHaveBeenCalledWith( + '/documents/reports/', + 'docs-id', + 'reports' + ); + expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith( + expect.objectContaining({ + contentlet: expect.objectContaining({ + permissions: [PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT_PERMISSIONS] + }) + }) + ); + }); + + it('should cache resolved permissions onto the node so the next right-click is instant', () => { + contentDriveStore.loadFolderPermissions.mockReturnValue(of([PERMISSIONS_TYPE.EDIT])); + const node = buildFolderNode(undefined); + + rightClick(node); + rightClick(node); + + expect(contentDriveStore.loadFolderPermissions).toHaveBeenCalledTimes(1); + }); + + it('should open an empty menu rather than throwing when the lookup cannot resolve', () => { + contentDriveStore.loadFolderPermissions.mockReturnValue(of(undefined)); + const node = buildFolderNode(undefined); + + rightClick(node); + + expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith( + expect.objectContaining({ + contentlet: expect.objectContaining({ permissions: [] }) + }) + ); + // Unresolved stays unresolved, so a later right-click retries instead of caching a lie. + expect(node.data.permissions).toBeUndefined(); + }); + + it('should ignore a "Load more" node', () => { + spectator.triggerEventHandler(DotTreeFolderComponent, 'rightClick', { + event: new MouseEvent('contextmenu'), + node: { + key: 'load-more:/documents/', + label: '', + data: { + type: LOAD_MORE_NODE_TYPE, + id: 'load-more:/documents/', + path: '/documents/', + hostname: 'demo.dotcms.com', + nextPage: 2, + remaining: 5 + } + } as DotFolderTreeNodeItem + }); + + expect(contentDriveStore.patchContextMenu).not.toHaveBeenCalled(); + }); + }); }); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts index c6657f9db47a..c1ab11050468 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts @@ -3,12 +3,14 @@ import { signalMethod } from '@ngrx/signals'; import { ChangeDetectionStrategy, Component, + DestroyRef, effect, inject, output, untracked, viewChild } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import type { TreeNodeCollapseEvent, @@ -16,11 +18,13 @@ import type { TreeNodeSelectEvent } from 'primeng/types/tree'; -import { TreeNodeLoadMoreData } from '@dotcms/dotcms-models'; +import { DotContentDriveActionableFolder, TreeNodeLoadMoreData } from '@dotcms/dotcms-models'; import { ALL_FOLDER, DotContentDriveMoveItems, + DotContentDriveTreeRightClick, DotContentDriveUploadFiles, + DotFolderTreeNodeContentData, DotFolderTreeNodeItem, DotTreeFolderComponent, LOAD_MORE_NODE_TYPE @@ -48,6 +52,7 @@ import { appendLoadMoreNodes } from '../../utils/functions'; }) export class DotContentDriveSidebarComponent { readonly #store = inject(DotContentDriveStore); + readonly #destroyRef = inject(DestroyRef); readonly $loading = this.#store.sidebarLoading; readonly $folders = this.#store.folders; @@ -232,6 +237,71 @@ export class DotContentDriveSidebarComponent { return undefined; } + /** + * Opens the shared folder context menu for a right-clicked tree node, giving the sidebar the + * same folder actions the table offers. + * + * Nodes loaded by expanding a folder already carry their permissions. Nodes hydrated by the + * deep-link hierarchy load do not (that call's page size exceeds the backend cap for + * `includePermissions`), so those resolve them on demand on first right-click and the result is + * cached back onto the node — a second right-click on the same folder opens immediately. + * + * @param {DotContentDriveTreeRightClick} rightClick - The originating event and the clicked node + */ + protected onNodeRightClick({ event, node }: DotContentDriveTreeRightClick): void { + const data = node.data; + + if (!data || data.type === LOAD_MORE_NODE_TYPE) { + return; + } + + if (data.permissions) { + this.#openContextMenu(event, data); + + return; + } + + this.#store + .loadFolderPermissions(data.path, data.id, data.name ?? '') + .pipe(takeUntilDestroyed(this.#destroyRef)) + .subscribe((permissions) => { + // Cache onto the node so the next right-click skips the lookup. `[]` is a valid + // cached answer ("no grants"); only an unresolved lookup stays undefined and retries. + if (permissions) { + data.permissions = permissions; + } + + this.#openContextMenu(event, { ...data, permissions: permissions ?? [] }); + }); + } + + /** + * Publishes the clicked folder to the store in the shape the shared context menu and the + * "Edit folder" dialog consume. + * + * @param {MouseEvent} event - The originating right-click, used to anchor the menu + * @param {DotFolderTreeNodeContentData} data - The clicked node's folder data + */ + #openContextMenu(event: MouseEvent, data: DotFolderTreeNodeContentData): void { + this.#store.patchContextMenu({ + triggeredEvent: event, + contentlet: { + type: 'folder', + identifier: data.id, + // The tree labels nodes by full path; `name` comes from the folder-search view. + name: data.name ?? '', + path: data.path, + title: data.title ?? '', + sortOrder: data.sortOrder ?? 0, + showOnMenu: data.showOnMenu ?? false, + filesMasks: data.filesMasks ?? '', + defaultFileType: data.defaultFileType ?? '', + defaultBaseType: data.defaultBaseType, + permissions: data.permissions ?? [] + } satisfies DotContentDriveActionableFolder + }); + } + /** * Handles node collapse events * Prevents collapse of the special 'ALL_FOLDER' node diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-folder-list-context-menu/dot-folder-list-context-menu.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-folder-list-context-menu/dot-folder-list-context-menu.component.ts index 81faa8053c2e..7245d696d893 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-folder-list-context-menu/dot-folder-list-context-menu.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-folder-list-context-menu/dot-folder-list-context-menu.component.ts @@ -143,7 +143,10 @@ export class DotFolderListViewContextMenuComponent { if (isFolder(contentlet)) { const folderMenuItems = []; - if (contentlet.permissions.includes(PERMISSIONS_TYPE.EDIT)) { + // Optional chaining is deliberate: a folder can reach here without `permissions` if it + // came from a source that did not resolve them (an older backend, or a search that did + // not opt into `includePermissions`). Gating must degrade to "no actions", never throw. + if (contentlet.permissions?.includes(PERMISSIONS_TYPE.EDIT)) { folderMenuItems.push({ label: this.#dotMessageService.get('content-drive.context-menu.edit-folder'), command: () => { @@ -158,7 +161,7 @@ export class DotFolderListViewContextMenuComponent { }); } - if (contentlet.permissions.includes(PERMISSIONS_TYPE.EDIT_PERMISSIONS)) { + if (contentlet.permissions?.includes(PERMISSIONS_TYPE.EDIT_PERMISSIONS)) { folderMenuItems.push({ label: this.#dotMessageService.get('Edit-Permissions'), command: () => this.#openPermissionsDialog(contentlet.identifier) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts index d5bba629ea09..b0b073643976 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts @@ -40,7 +40,7 @@ import { DotCMSContentTypeField, DotCMSDataTypes, DotCMSFieldTypes, - DotContentDriveFolder, + DotContentDriveActionableFolder, DotContentDriveItem, DotContentDrivePaginateEvent } from '@dotcms/dotcms-models'; @@ -216,7 +216,7 @@ export class DotContentDriveShellComponent { const dialog = this.$activeDialog(); return dialog?.type === DIALOG_TYPE.FOLDER - ? (dialog.payload as DotContentDriveFolder) + ? (dialog.payload as DotContentDriveActionableFolder) : undefined; }); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/constants.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/constants.ts index 116a1d6a2313..57a123d0a9dc 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/constants.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/constants.ts @@ -35,6 +35,17 @@ export const FOLDER_TREE_PAGE_SIZE = DOT_FOLDER_TREE_PAGE_SIZE; */ export const FOLDER_TREE_HIERARCHY_PAGE_SIZE = 10000; +/** + * Page size for the on-demand lookup that resolves a single folder's permissions when its tree node + * was hydrated without them (see `getFolderPermissionsByPath`). Matches the backend cap for + * `includePermissions=true` (`content.drive.folder.search.permissions.max.per.page`, default 200) — + * requesting more would be rejected with a 400. + */ +export const FOLDER_PERMISSIONS_LOOKUP_PAGE_SIZE = 200; + +/** Minimum length the folder-search `name` filter accepts; shorter values are rejected with a 400. */ +export const FOLDER_NAME_FILTER_MIN_LENGTH = 2; + export const DEFAULT_SORT = { field: 'modDate', order: DotContentDriveSortOrder.DESC diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/models.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/models.ts index 365d8ad729d6..4848f4ba2941 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/models.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/models.ts @@ -1,6 +1,7 @@ import { DotCMSContentTypeField, - DotContentDriveFolder, + DotContentDriveActionableFolder, + DotContentDriveActionableItem, DotContentDriveItem, DotFolder, DotSite @@ -83,7 +84,11 @@ export interface DotContentDriveInit { */ export interface DotContentDriveContextMenu { triggeredEvent: Event | null; - contentlet: DotContentDriveItem | null; + /** + * The item the menu acts on. Folders arrive from two sources — a full row from the table and a + * search view from the sidebar tree — so this is the narrower actionable shape both satisfy. + */ + contentlet: DotContentDriveActionableItem | null; showAddToBundle: boolean; } @@ -106,7 +111,7 @@ export interface DotContentDriveDialog { type: keyof typeof DIALOG_TYPE; header: string; payload?: - | DotContentDriveFolder + | DotContentDriveActionableFolder | DotContentDriveContentTypeSelectorPayload | DotContentDriveUploadSelectorPayload; } diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts index a7e0187b6765..a43aa666380b 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts @@ -13,6 +13,7 @@ import { inject } from '@angular/core'; import { catchError, take } from 'rxjs/operators'; import { DotFolderService } from '@dotcms/data-access'; +import { PermissionType } from '@dotcms/dotcms-models'; import { ALL_FOLDER, DotFolderTreeNodeItem } from '@dotcms/portlets/content-drive/ui'; import { SYSTEM_HOST } from '../../../shared/constants'; @@ -21,7 +22,8 @@ import { applyLoadMoreToHierarchy, FolderTreeHierarchyLevel, getFolderHierarchyByPath, - getFolderNodesByPath + getFolderNodesByPath, + getFolderPermissionsByPath } from '../../../utils/functions'; import { buildTreeFolderNodes } from '../../../utils/tree-folder.utils'; @@ -121,6 +123,44 @@ export function withSidebar() { page ); }, + /** + * Resolves the permission types the current user holds on a single folder. + * + * Only needed for nodes hydrated by {@link loadFolders}: that call resolves the whole + * deep-link hierarchy in one large page, which exceeds the backend's cap for + * `includePermissions`, so those nodes arrive without permissions. Nodes loaded by + * expanding a folder already carry them and never reach this method. + * + * Emits `undefined` when the lookup fails or the folder is not found, so the caller can + * tell that apart from "the user holds no permissions" (`[]`). + */ + loadFolderPermissions: ( + folderPath: string, + folderId: string, + folderName: string + ): Observable => { + const currentSite = store.currentSite(); + + if (!currentSite) { + return of(undefined); + } + + return getFolderPermissionsByPath( + folderPath, + folderId, + folderName, + currentSite, + dotFolderService + ).pipe( + take(1), + catchError((response) => { + console.error('Error loading folder permissions:', response); + + return of(undefined); + }) + ); + }, + /** * Sets the selected node */ diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts index c1123f04c56a..6f18a565cd12 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts @@ -8,7 +8,8 @@ import { DotContentDriveItem, DotPagination, FolderSearchView, - isTreeNodeContentData + isTreeNodeContentData, + PERMISSIONS_TYPE } from '@dotcms/dotcms-models'; import { createFakeCheckboxField, @@ -30,6 +31,8 @@ import { folderSearchViewToDotFolder, getFolderHierarchyByPath, getFolderNodesByPath, + getFolderPermissionsByPath, + getParentPath, getUserSearchableActive, isBinaryCheckboxField, isDateFieldFilterType, @@ -433,18 +436,16 @@ describe('Utility Functions', () => { }); it('should adapt search results into DotFolder full paths with the site hostname', (done) => { - mockDotFolderService.searchFolders.mockReturnValueOnce( - searchResult([ - createFakeFolderSearchView({ - id: 'm', - inode: 'im', - name: 'main', - path: '/', - addChildrenAllowed: true, - hasChildren: true - }) - ]) - ); + const view = createFakeFolderSearchView({ + id: 'm', + inode: 'im', + name: 'main', + path: '/', + addChildrenAllowed: true, + hasChildren: true + }); + + mockDotFolderService.searchFolders.mockReturnValueOnce(searchResult([view])); getFolderHierarchyByPath('/main', SITE, mockDotFolderService).subscribe({ next: (levels) => { @@ -454,7 +455,17 @@ describe('Utility Functions', () => { hostName: HOSTNAME, path: '/main/', addChildrenAllowed: true, - hasChildren: true + hasChildren: true, + name: 'main', + title: view.title, + sortOrder: view.sortOrder, + filesMasks: view.filesMasks, + defaultFileType: view.defaultFileType, + showOnMenu: view.showOnMenu, + defaultBaseType: view.defaultBaseType, + // The hierarchy load cannot opt into permissions, so the endpoint's `null` + // is carried through as "unresolved" rather than "no grants". + permissions: undefined }); done(); }, @@ -462,6 +473,18 @@ describe('Utility Functions', () => { }); }); + it('should not request permissions (its page size exceeds the backend cap)', (done) => { + getFolderHierarchyByPath('/main', SITE, mockDotFolderService).subscribe({ + next: () => { + expect(mockDotFolderService.searchFolders).not.toHaveBeenCalledWith( + expect.objectContaining({ includePermissions: true }) + ); + done(); + }, + error: done + }); + }); + it('should query only the site root for the root path', (done) => { getFolderHierarchyByPath('/', SITE, mockDotFolderService).subscribe({ next: (levels) => { @@ -643,16 +666,19 @@ describe('Utility Functions', () => { }); it('should transform child folders into tree nodes', (done) => { + const firstChild = createFakeFolderSearchView({ + id: 'child-1', + inode: 'inode-1', + name: 'child1', + path: '/main/sub-folder/', + addChildrenAllowed: true, + hasChildren: true, + permissions: [PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT] + }); + mockDotFolderService.searchFolders.mockReturnValue( searchResult([ - createFakeFolderSearchView({ - id: 'child-1', - inode: 'inode-1', - name: 'child1', - path: '/main/sub-folder/', - addChildrenAllowed: true, - hasChildren: true - }), + firstChild, createFakeFolderSearchView({ id: 'child-2', inode: 'inode-2', @@ -675,7 +701,16 @@ describe('Utility Functions', () => { inode: 'inode-1', hostname: HOSTNAME, path: '/main/sub-folder/child1/', - type: 'folder' + type: 'folder', + // Carried so a right-click can gate the menu and fill the edit dialog. + name: 'child1', + title: firstChild.title, + sortOrder: firstChild.sortOrder, + filesMasks: firstChild.filesMasks, + defaultFileType: firstChild.defaultFileType, + showOnMenu: firstChild.showOnMenu, + defaultBaseType: firstChild.defaultBaseType, + permissions: [PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT] }, // hasChildren: true → expandable (chevron shown) leaf: false @@ -690,6 +725,18 @@ describe('Utility Functions', () => { }); }); + it('should request permissions so an expanded node can gate its context menu', (done) => { + getFolderNodesByPath('/main/sub-folder/', SITE, mockDotFolderService).subscribe({ + next: () => { + expect(mockDotFolderService.searchFolders).toHaveBeenCalledWith( + expect.objectContaining({ includePermissions: true }) + ); + done(); + }, + error: done + }); + }); + it('should normalize a parent path that is missing its trailing slash', (done) => { mockDotFolderService.searchFolders.mockReturnValue( searchResult([createFakeFolderSearchView({ id: 'x', name: 'sub', path: '/main' })]) @@ -1219,4 +1266,176 @@ describe('folderSearchViewToDotFolder', () => { expect(folder.defaultBaseType).toBeUndefined(); }); + + it('should carry the fields the Edit-folder dialog reads', () => { + const view = createFakeFolderSearchView({ + name: 'docs', + path: '/', + title: 'Documents', + sortOrder: 3, + filesMasks: '*.pdf,*.docx', + defaultFileType: 'FileAsset', + showOnMenu: true + }); + + const folder = folderSearchViewToDotFolder(view, 'demo.dotcms.com'); + + expect(folder).toEqual( + expect.objectContaining({ + name: 'docs', + title: 'Documents', + sortOrder: 3, + filesMasks: '*.pdf,*.docx', + defaultFileType: 'FileAsset', + showOnMenu: true + }) + ); + }); + + it('should carry granted permissions through unchanged', () => { + const view = createFakeFolderSearchView({ + name: 'docs', + path: '/', + permissions: [PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT] + }); + + const folder = folderSearchViewToDotFolder(view, 'demo.dotcms.com'); + + expect(folder.permissions).toEqual([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT]); + }); + + it('should keep an empty permissions array as a resolved "no grants" answer', () => { + const view = createFakeFolderSearchView({ name: 'docs', path: '/', permissions: [] }); + + const folder = folderSearchViewToDotFolder(view, 'demo.dotcms.com'); + + expect(folder.permissions).toEqual([]); + }); + + it('should turn a null permissions response into undefined ("not resolved")', () => { + // The distinction matters: `[]` is final, `undefined` makes the sidebar resolve them on + // demand before opening the context menu. + const view = createFakeFolderSearchView({ name: 'docs', path: '/', permissions: null }); + + const folder = folderSearchViewToDotFolder(view, 'demo.dotcms.com'); + + expect(folder.permissions).toBeUndefined(); + }); +}); + +describe('getParentPath', () => { + it.each([ + ['/a/b/', '/a/'], + ['/a/b', '/a/'], + ['/b/', '/'], + ['/b', '/'], + ['/', '/'], + ['', '/'] + ])('should resolve the parent of %s as %s', (path, expected) => { + expect(getParentPath(path)).toBe(expected); + }); +}); + +describe('getFolderPermissionsByPath', () => { + let mockDotFolderService: { searchFolders: jest.Mock }; + + const searchResult = (folders: FolderSearchView[]) => + of({ folders, pagination: { totalEntries: folders.length } as DotPagination }); + + beforeEach(() => { + mockDotFolderService = { searchFolders: jest.fn().mockReturnValue(searchResult([])) }; + }); + + it('should query the parent level with permissions, narrowed by folder name', (done) => { + getFolderPermissionsByPath( + '/main/docs/', + 'docs-id', + 'docs', + createFakeSite({ identifier: 'site-1', hostname: 'demo.dotcms.com' }), + mockDotFolderService as unknown as DotFolderService + ).subscribe({ + next: () => { + expect(mockDotFolderService.searchFolders).toHaveBeenCalledWith( + expect.objectContaining({ + siteId: 'site-1', + path: '/main/', + recursive: false, + name: 'docs', + includePermissions: true + }) + ); + done(); + }, + error: done + }); + }); + + it('should omit the name filter when the folder name is too short for the endpoint', (done) => { + getFolderPermissionsByPath( + '/main/a/', + 'a-id', + 'a', + createFakeSite({ identifier: 'site-1' }), + mockDotFolderService as unknown as DotFolderService + ).subscribe({ + next: () => { + expect(mockDotFolderService.searchFolders).toHaveBeenCalledWith( + expect.objectContaining({ name: undefined }) + ); + done(); + }, + error: done + }); + }); + + it('should return the permissions of the matching folder', (done) => { + mockDotFolderService.searchFolders.mockReturnValue( + searchResult([ + createFakeFolderSearchView({ + id: 'other', + name: 'docs-archive', + permissions: [PERMISSIONS_TYPE.READ] + }), + createFakeFolderSearchView({ + id: 'docs-id', + name: 'docs', + permissions: [PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT] + }) + ]) + ); + + getFolderPermissionsByPath( + '/main/docs/', + 'docs-id', + 'docs', + createFakeSite({ identifier: 'site-1' }), + mockDotFolderService as unknown as DotFolderService + ).subscribe({ + next: (permissions) => { + expect(permissions).toEqual([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT]); + done(); + }, + error: done + }); + }); + + it('should resolve undefined when the folder is not in the page', (done) => { + mockDotFolderService.searchFolders.mockReturnValue( + searchResult([createFakeFolderSearchView({ id: 'someone-else' })]) + ); + + getFolderPermissionsByPath( + '/main/docs/', + 'docs-id', + 'docs', + createFakeSite({ identifier: 'site-1' }), + mockDotFolderService as unknown as DotFolderService + ).subscribe({ + next: (permissions) => { + expect(permissions).toBeUndefined(); + done(); + }, + error: done + }); + }); }); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts index eacf47db9f8e..9853295989a9 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts @@ -8,13 +8,14 @@ import { createLoadMoreTreeNode, DotCMSContentTypeField, DotContentDriveDateRange, - DotContentDriveFolder, - DotContentDriveItem, + DotContentDriveActionableFolder, + DotContentDriveActionableItem, DotContentDriveUserSearchableValue, DotFolder, DotSite, FolderSearchView, - LOAD_MORE_NODE_TYPE + LOAD_MORE_NODE_TYPE, + PermissionType } from '@dotcms/dotcms-models'; import { getSingleSelectableFieldOptions } from '@dotcms/edit-content'; import { DotFolderTreeNodeItem } from '@dotcms/portlets/content-drive/ui'; @@ -26,6 +27,8 @@ import { FIELD_FILTER_DATE_TYPES, FIELD_FILTER_KEY_VALUE_TYPE, FIELD_FILTER_MULTI_VALUE_TYPES, + FOLDER_NAME_FILTER_MIN_LENGTH, + FOLDER_PERMISSIONS_LOOKUP_PAGE_SIZE, FOLDER_TREE_HIERARCHY_PAGE_SIZE, FOLDER_TREE_PAGE_SIZE, USER_SEARCHABLE_PREFIX, @@ -266,7 +269,18 @@ export function folderSearchViewToDotFolder(view: FolderSearchView, hostName: st path: `${parentPath}${view.name}/`, addChildrenAllowed: view.addChildrenAllowed, hasChildren: view.hasChildren, - defaultBaseType: view.defaultBaseType + defaultBaseType: view.defaultBaseType, + name: view.name, + title: view.title, + sortOrder: view.sortOrder, + filesMasks: view.filesMasks, + defaultFileType: view.defaultFileType, + showOnMenu: view.showOnMenu, + // `null` (not requested) and `[]` (requested, no grants) mean different things, and the + // difference drives behavior: a node whose permissions were never fetched must resolve them + // on demand before its context menu can gate correctly, while an empty array is a final + // answer. Collapse `null` to `undefined` (the optional-field idiom) and keep `[]` intact. + permissions: view.permissions ?? undefined }; } @@ -313,6 +327,11 @@ export function getFolderHierarchyByPath( orderby: 'name', direction: 'ASC', page: 1, + // Deliberately NOT requesting `includePermissions` here: the backend caps the page + // size when permissions are requested (default 200) and rejects anything larger with + // a 400, and this call intentionally uses a much larger page to resolve deep-link + // ancestors in one shot. Nodes hydrated here therefore arrive without permissions; + // `getFolderPermissionsByPath` resolves them on demand when one is right-clicked. per_page: FOLDER_TREE_HIERARCHY_PAGE_SIZE }) .pipe( @@ -355,7 +374,11 @@ export function getFolderNodesByPath( orderby: 'name', direction: 'ASC', page, - per_page: FOLDER_TREE_PAGE_SIZE + per_page: FOLDER_TREE_PAGE_SIZE, + // Safe to request here: this page size (40) is well under the backend cap, so nodes + // loaded by expanding a folder carry their permissions and their context menu opens + // without a second round-trip. + includePermissions: true }) .pipe( map(({ folders, pagination }) => ({ @@ -367,6 +390,65 @@ export function getFolderNodesByPath( ); } +/** + * The parent path of a folder path: `/a/b/` → `/a/`, `/b/` → `/`. + * `GET /api/v1/folder/search` scopes a non-recursive search by the *parent* path, while tree nodes + * carry their own full path. + * + * @param {string} folderPath - The folder's own path, with or without a trailing slash + * @returns {string} the parent path, always trailing-slashed + */ +export function getParentPath(folderPath: string): string { + const withoutTrailing = folderPath.endsWith('/') ? folderPath.slice(0, -1) : folderPath; + const lastSeparator = withoutTrailing.lastIndexOf('/'); + + return lastSeparator <= 0 ? '/' : withoutTrailing.slice(0, lastSeparator + 1); +} + +/** + * Resolves the permission types the current user holds on a single folder. + * + * Needed because the deep-link hierarchy load ({@link getFolderHierarchyByPath}) cannot request + * permissions — its page size exceeds the backend cap — so the folders visible on first render + * arrive without them. Without this, right-clicking those nodes would produce an empty menu that + * is indistinguishable from "you have no rights on this folder". + * + * Queries the folder's own level, narrowed by name so the response stays small, and matches the + * folder by id. Resolves to `undefined` when the folder is not found in the page, letting the + * caller tell "no grants" (`[]`) from "could not resolve". + * + * @param {string} folderPath - The folder's own full path, e.g. `/a/b/` + * @param {string} folderId - Identifier of the folder to match in the response + * @param {string} folderName - The folder's own name, used to narrow the query + * @param {DotSite} site - Site scoping the search + * @param {DotFolderService} dotFolderService - The folder service + * @returns {Observable} the folder's permissions, if resolved + */ +export function getFolderPermissionsByPath( + folderPath: string, + folderId: string, + folderName: string, + site: DotSite, + dotFolderService: DotFolderService +): Observable { + return dotFolderService + .searchFolders({ + siteId: site.identifier, + path: getParentPath(folderPath), + recursive: false, + // The endpoint rejects a filter shorter than 2 characters, so single-character folder + // names fall back to an unfiltered page of the level. + name: folderName.length >= FOLDER_NAME_FILTER_MIN_LENGTH ? folderName : undefined, + page: 1, + per_page: FOLDER_PERMISSIONS_LOOKUP_PAGE_SIZE, + includePermissions: true + }) + .pipe( + map(({ folders }) => folders.find((folder) => folder.id === folderId)), + map((folder) => folder?.permissions ?? undefined) + ); +} + /** * Builds the synthetic "Load more" node appended to the end of a paginated folder level. It is not * a real folder: it is not selectable and carries the paging cursor (`nextPage`) and how many @@ -485,10 +567,16 @@ function findFolderNodeByPath( /** * Checks if an item is a folder. * - * @param {DotContentDriveItem} item - The item to check + * Narrows to the actionable folder shape rather than the full table row, so it serves folders from + * both views. Called with a `DotContentDriveItem` (the table's list) it still narrows to + * `DotContentDriveFolder`, since that is the only folder member of that union. + * + * @param {DotContentDriveActionableItem} item - The item to check * @returns {boolean} True if the item is a folder, false otherwise */ -export function isFolder(item: DotContentDriveItem): item is DotContentDriveFolder { +export function isFolder( + item: DotContentDriveActionableItem +): item is DotContentDriveActionableFolder { return item != null && 'type' in item && item.type === 'folder'; } diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/tree-folder.utils.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/tree-folder.utils.ts index 26b585868f92..5683e89ef7b2 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/tree-folder.utils.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/tree-folder.utils.ts @@ -41,7 +41,17 @@ export const createTreeNode = ( hostname: folder.hostName, path: folder.path, type: 'folder', - defaultBaseType: folder.defaultBaseType + defaultBaseType: folder.defaultBaseType, + // Carried so a right-click can gate the shared context menu and pre-populate the + // "Edit folder" dialog without refetching. `permissions` stays undefined for nodes + // built by the deep-link hierarchy load, which cannot request them. + name: folder.name, + title: folder.title, + sortOrder: folder.sortOrder, + filesMasks: folder.filesMasks, + defaultFileType: folder.defaultFileType, + showOnMenu: folder.showOnMenu, + permissions: folder.permissions }, // Hide the expand toggle for folders the search endpoint reports as having no visible // children. When `hasChildren` is undefined (legacy source) the folder stays expandable. diff --git a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.html b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.html index 298e4aa41bb4..c928a6af3e17 100644 --- a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.html +++ b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.html @@ -22,7 +22,8 @@ [attr.data-json-node]="node.data | json" [attr.data-id]="node?.data?.id" class="font-normal" - [class.active]="$activeDropNode()?.id === node?.data.id"> + [class.active]="$activeDropNode()?.id === node?.data.id" + (contextmenu)="onContextMenu($event, node)"> {{ node.key === ALL_FOLDER_KEY ? (node.label | dm) : (node.label | dotFolderName) }} diff --git a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.spec.ts b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.spec.ts index a930209f3ccc..b35008b5dd70 100644 --- a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.spec.ts @@ -10,7 +10,8 @@ import { MockDotMessageService } from '@dotcms/utils-testing'; import { DotTreeFolderComponent } from './dot-tree-folder.component'; -import { SYSTEM_HOST_ID } from '../shared/constants'; +import { ALL_FOLDER, LOAD_MORE_NODE_TYPE, SYSTEM_HOST_ID } from '../shared/constants'; +import { DotFolderTreeNodeItem } from '../shared/models'; // Mock DragEvent since it's not available in Jest environment class DragEventMock extends Event { @@ -821,4 +822,77 @@ describe('DotTreeFolderComponent', () => { }); }); }); + + describe('right-click', () => { + const folderNode: DotFolderTreeNodeItem = { + key: 'folder-1', + label: '/application/content/', + data: { + id: 'folder-1', + hostname: 'demo.dotcms.com', + path: '/application/content/', + type: 'folder' + } + }; + + const rightClickOn = (node: DotFolderTreeNodeItem) => { + const event = new MouseEvent('contextmenu', { cancelable: true }); + jest.spyOn(event, 'preventDefault'); + // The handler is protected — reached the way the template reaches it. + ( + component as unknown as { + onContextMenu: (e: MouseEvent, n: DotFolderTreeNodeItem) => void; + } + ).onContextMenu(event, node); + + return event; + }; + + it('should emit the event and node for a folder node', () => { + const emitted = jest.fn(); + component.rightClick.subscribe(emitted); + + const event = rightClickOn(folderNode); + + expect(emitted).toHaveBeenCalledWith({ event, node: folderNode }); + }); + + it('should suppress the native browser menu for a folder node', () => { + const event = rightClickOn(folderNode); + + expect(event.preventDefault).toHaveBeenCalled(); + }); + + it('should ignore the "All folders" root, which is not a real folder', () => { + const emitted = jest.fn(); + component.rightClick.subscribe(emitted); + + const event = rightClickOn({ ...folderNode, key: ALL_FOLDER.key }); + + expect(emitted).not.toHaveBeenCalled(); + // No menu to show, so the native one is left alone rather than swallowed. + expect(event.preventDefault).not.toHaveBeenCalled(); + }); + + it('should ignore "Load more" sentinels', () => { + const emitted = jest.fn(); + component.rightClick.subscribe(emitted); + + const event = rightClickOn({ + key: 'load-more:/application/', + label: '', + data: { + type: LOAD_MORE_NODE_TYPE, + id: 'load-more:/application/', + path: '/application/', + hostname: 'demo.dotcms.com', + nextPage: 2, + remaining: 10 + } + } as DotFolderTreeNodeItem); + + expect(emitted).not.toHaveBeenCalled(); + expect(event.preventDefault).not.toHaveBeenCalled(); + }); + }); }); diff --git a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.ts b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.ts index 9119dcb4659c..6bfa89e82be3 100644 --- a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.ts +++ b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.ts @@ -15,12 +15,13 @@ import { TreeNodeExpandEvent, TreeNodeCollapseEvent } from 'primeng/types/tree'; import { DotFolderTreeComponent, DotFolderNamePipe, DotMessagePipe } from '@dotcms/ui'; -import { ALL_FOLDER } from '../shared/constants'; +import { ALL_FOLDER, LOAD_MORE_NODE_TYPE } from '../shared/constants'; import { DotFolderTreeNodeData, DotFolderTreeNodeItem, DotContentDriveUploadFiles, - DotContentDriveMoveItems + DotContentDriveMoveItems, + DotContentDriveTreeRightClick } from '../shared/models'; /** @@ -48,6 +49,8 @@ export class DotTreeFolderComponent { loadMore = output(); uploadFiles = output(); moveItems = output(); + /** Right-click on a folder node — drives the shared folder context menu, as the table's rows do. */ + rightClick = output(); readonly elementRef = inject(ElementRef); @@ -64,6 +67,25 @@ export class DotTreeFolderComponent { this.loadMore.emit(node as DotFolderTreeNodeItem); } + /** + * @description Emits a right-click on a real folder node so the consumer can open the shared + * context menu. The browser menu is suppressed only for nodes that can actually produce one: + * the synthetic "All folders" root and "Load more" sentinels are not folders, so they keep the + * native menu rather than swallowing the event for no reason. + * @param event - The contextmenu MouseEvent + * @param node - The tree node under the cursor + */ + protected onContextMenu(event: MouseEvent, node: DotFolderTreeNodeItem): void { + const data = node?.data; + + if (!data || data.type === LOAD_MORE_NODE_TYPE || node.key === this.ALL_FOLDER_KEY) { + return; + } + + event.preventDefault(); + this.rightClick.emit({ event, node }); + } + /** * @description Set the dropzone as active when the drag enters the dropzone * @param event - DragEvent diff --git a/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts b/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts index 40917f7b28d6..691f3b2eb3c7 100644 --- a/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts +++ b/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts @@ -1,6 +1,10 @@ import type { TreeNode } from 'primeng/api'; -import type { TreeNodeContentData, TreeNodeLoadMoreData } from '@dotcms/dotcms-models'; +import type { + PermissionType, + TreeNodeContentData, + TreeNodeLoadMoreData +} from '@dotcms/dotcms-models'; /** * @export @@ -84,6 +88,24 @@ export type DotFolderTreeNodeContentData = TreeNodeContentData & { */ defaultBaseType?: string | null; fromTable?: boolean; + /** + * Fields below back the shared folder context menu and the "Edit folder" dialog, so a + * right-click on a tree node can gate and pre-populate without a refetch. Populated from + * `GET /api/v1/folder/search`; optional because other node sources (e.g. the synthetic + * "All folders" root) do not carry them. + */ + name?: string; + title?: string; + sortOrder?: number; + filesMasks?: string; + defaultFileType?: string; + showOnMenu?: boolean; + /** + * Permission types the user holds on this folder. `undefined` means "not resolved yet" — the + * deep-link hierarchy load cannot request permissions, so those nodes resolve them on demand + * on first right-click. An empty array is a final answer: the user holds none. + */ + permissions?: PermissionType[]; }; /** @@ -99,3 +121,14 @@ export type DotFolderTreeNodeData = DotFolderTreeNodeContentData | TreeNodeLoadM * @description Tree node item */ export type DotFolderTreeNodeItem = TreeNode; + +/** + * @export + * @interface DotContentDriveTreeRightClick + * @description Right-click on a folder node in the sidebar tree. Carries the original event (the + * shared context menu anchors itself to it) and the node that was clicked. + */ +export interface DotContentDriveTreeRightClick { + event: MouseEvent; + node: DotFolderTreeNodeItem; +} diff --git a/core-web/libs/utils-testing/src/lib/dot-folder.mock.ts b/core-web/libs/utils-testing/src/lib/dot-folder.mock.ts index 7d0faf2f33d4..698fbb3f2557 100644 --- a/core-web/libs/utils-testing/src/lib/dot-folder.mock.ts +++ b/core-web/libs/utils-testing/src/lib/dot-folder.mock.ts @@ -15,17 +15,28 @@ export function createFakeFolder(overrides: Partial = {}): DotFolder /** * Create a fake `FolderSearchView` as returned by `GET /api/v1/folder/search`. * Note: `path` is the folder's parent path, `name` is the folder's own name. + * + * `permissions` defaults to `null`, matching a response that did not pass + * `includePermissions=true` — the common case. Override it to exercise gating. */ export function createFakeFolderSearchView( overrides: Partial = {} ): FolderSearchView { + const name = faker.string.alphanumeric(8); + return { id: faker.string.uuid(), inode: faker.string.uuid(), - name: faker.string.alphanumeric(8), + name, path: '/', addChildrenAllowed: faker.datatype.boolean(), hasChildren: faker.datatype.boolean(), + title: name, + sortOrder: 0, + filesMasks: '', + defaultFileType: faker.string.uuid(), + showOnMenu: faker.datatype.boolean(), + permissions: null, ...overrides }; } From b7e1a56fbf3a75861b07a24267e2fe9edf48e939 Mon Sep 17 00:00:00 2001 From: ihoffmann-dot Date: Tue, 11 Aug 2026 20:55:22 -0300 Subject: [PATCH 2/6] style(content-drive): apply prettier formatting to the new specs --- .../dot-content-drive-dialog-folder.component.spec.ts | 4 +--- .../dot-content-drive-sidebar.component.spec.ts | 8 ++++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts index 4603154274d7..ee8f9bb6201b 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts @@ -1064,9 +1064,7 @@ describe('DotContentDriveDialogFolderComponent', () => { expect(folderService.saveFolder).toHaveBeenCalled(); const lastCall = folderService.saveFolder.mock.calls.at(-1)?.[0]; - expect(lastCall?.assetPath).toBe( - '//demo.dotcms.com/campaigns/2026/marketing-assets/' - ); + expect(lastCall?.assetPath).toBe('//demo.dotcms.com/campaigns/2026/marketing-assets/'); }); it('should anchor the edit assetPath at the site root for a root-level folder', () => { diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts index b41de115429c..5ae536d92c8a 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts @@ -1164,9 +1164,7 @@ describe('DotContentDriveSidebarComponent', () => { contentDriveStore.loadFolderPermissions.mockReturnValue(of(undefined)); }); - const buildFolderNode = ( - permissions?: PermissionType[] - ): DotFolderTreeNodeItem => ({ + const buildFolderNode = (permissions?: PermissionType[]): DotFolderTreeNodeItem => ({ key: 'docs-id', label: '/documents/reports/', data: { @@ -1194,7 +1192,9 @@ describe('DotContentDriveSidebarComponent', () => { }; it('should publish the folder to the store in the shape the menu and dialog consume', () => { - const event = rightClick(buildFolderNode([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT])); + const event = rightClick( + buildFolderNode([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT]) + ); expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith({ triggeredEvent: event, From bfd9f3683868738b7af0043d9d6c7dccac7313d4 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Thu, 13 Aug 2026 13:13:43 -0300 Subject: [PATCH 3/6] fix(content-drive): resolve folder permissions on cold load and open the menu from the whole row (#36595) Right-clicking a folder in the sidebar tree opened the shared context menu only for folders loaded by expanding a node, and only on the folder name itself. The folders visible when the portlet opens stayed dead, which was the issue's original symptom. Cold load could not ask for permissions: the deep-link hierarchy fetches one large page per ancestor level, and the backend caps `per_page` at `content.drive.folder.search.permissions.max.per.page` (200) whenever `includePermissions=true`, rejecting anything larger with a 400. The previous approach papered over that with an on-demand lookup fired on the first right-click. Instead, size the hierarchy page to the cap and request permissions there, so every node carries them however it reached the tree. A deep-linked ancestor sorting past that page is fetched individually and pinned to the top of its level, rather than widening the page or paging until it turns up. `mergeFolderNodePage` folds the pinned node back into sort order if the user pages far enough to meet it again, keeping the on-screen node so its loaded children and expansion survive. This removes the on-demand lookup entirely, and with it a race between concurrent right-clicks, a silent paging miss indistinguishable from "no grants", and a component writing straight into store state. An unresolvable ancestor is left unpinned and the selection falls back to the root. That is also what a folder the user cannot READ looks like: `FolderAPIImpl.searchFolders` filters by permission rather than rejecting the request, so the tree still renders what is readable and the folder's existence is never revealed. Also: - Bind the context menu on the row, not the label span, so the toggler, the indent gutter and the space past a short name all respond, as the table's rows do. Resolved from the DOM like the tree's drag-and-drop, keeping the component presentational. - Thread each level's `nextPage` through `FolderTreeHierarchyLevel`, replacing a hardcoded 2. The hierarchy pages at 200 and load-more at 40, so a fixed resume point re-requested folders already on screen. Derived from folders fetched, never the rendered count, so a pinned node cannot shift it. - Anchor the Edit-folder dialog on the edited folder's own parent instead of the open folder. From the table the two coincide; from the sidebar any folder at any depth can be edited, so the old path built a request against a different folder entirely. - Guard permission gating with `?.` so a folder arriving without the field degrades to "no actions" instead of throwing. Verified against a local instance: the 200 cap and its 400 are real, the load-more resume lands exactly on the first unseen folder, and the name-narrowed lookup returns the pinned ancestor in one request. Co-Authored-By: Claude Opus 5 --- ...tent-drive-dialog-folder.component.spec.ts | 30 +- ...ot-content-drive-sidebar.component.spec.ts | 118 ++------ .../dot-content-drive-sidebar.component.ts | 49 +-- .../portlet/src/lib/shared/constants.ts | 27 +- .../lib/store/features/sidebar/withSidebar.ts | 41 +-- .../portlet/src/lib/utils/functions.spec.ts | 278 +++++++++++++++--- .../portlet/src/lib/utils/functions.ts | 265 +++++++++++------ .../dot-tree-folder.component.html | 4 +- .../dot-tree-folder.component.spec.ts | 133 ++++++--- .../dot-tree-folder.component.ts | 48 ++- .../ui/src/lib/shared/models.ts | 10 +- 11 files changed, 653 insertions(+), 350 deletions(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts index ee8f9bb6201b..e2669017c0bb 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-dialog-folder/dot-content-drive-dialog-folder.component.spec.ts @@ -50,7 +50,7 @@ const editableFolder = (overrides: Partial = {}): DotCont modDate: 1, owner: null, parent: '', - path: '/documents/app/', + path: '', permissions: [], type: 'folder', ...overrides @@ -569,6 +569,34 @@ describe('DotContentDriveDialogFolderComponent', () => { }); describe('upload behavior (defaultBaseType)', () => { + const editableFolder = ( + overrides: Partial = {} + ): DotContentDriveFolder => + ({ + name: 'app', + title: 'App', + sortOrder: 1, + filesMasks: '', + defaultFileType: 'FileAsset', + showOnMenu: false, + __icon__: 'folderIcon', + description: '', + extension: 'folder', + hasTitleImage: false, + hostId: '1', + iDate: 1, + identifier: '1', + inode: '1', + mimeType: '', + modDate: 1, + owner: null, + parent: '', + path: '/documents/app/', + permissions: [], + type: 'folder', + ...overrides + }) as DotContentDriveFolder; + it('should render the three upload-behavior options', () => { expect(spectator.query('[data-testid="upload-behavior-option-null"]')).toBeTruthy(); expect(spectator.query('[data-testid="upload-behavior-option-DOTASSET"]')).toBeTruthy(); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts index 5ae536d92c8a..b928dbcc227b 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts @@ -12,10 +12,10 @@ import { DotFolder, PermissionType, PERMISSIONS_TYPE } from '@dotcms/dotcms-mode import { DotContentDriveUploadFiles, DotTreeFolderComponent, + DotFolderTreeNodeContentData, DotFolderTreeNodeItem, DotContentDriveMoveItems, - ALL_FOLDER, - LOAD_MORE_NODE_TYPE + ALL_FOLDER } from '@dotcms/portlets/content-drive/ui'; import { GlobalStore } from '@dotcms/store'; @@ -125,7 +125,6 @@ describe('DotContentDriveSidebarComponent', () => { sidebarLoading: jest.fn().mockReturnValue(false), loadFolders: jest.fn(), loadChildFolders: jest.fn(), - loadFolderPermissions: jest.fn().mockReturnValue(of(undefined)), patchContextMenu: jest.fn(), updateFolders: jest.fn(), setSelectedNode: jest.fn() @@ -1161,39 +1160,34 @@ describe('DotContentDriveSidebarComponent', () => { // accumulate across tests in this file. Clear them (implementations are preserved) and // restore the default lookup so each case starts from a known state. jest.clearAllMocks(); - contentDriveStore.loadFolderPermissions.mockReturnValue(of(undefined)); }); - const buildFolderNode = (permissions?: PermissionType[]): DotFolderTreeNodeItem => ({ - key: 'docs-id', - label: '/documents/reports/', - data: { - id: 'docs-id', - inode: 'docs-inode', - hostname: 'demo.dotcms.com', - path: '/documents/reports/', - type: 'folder', - name: 'reports', - title: 'Reports', - sortOrder: 2, - filesMasks: '*.pdf', - defaultFileType: 'FileAsset', - defaultBaseType: 'DOTASSET', - showOnMenu: true, - permissions - } + const buildFolderData = (permissions?: PermissionType[]): DotFolderTreeNodeContentData => ({ + id: 'docs-id', + inode: 'docs-inode', + hostname: 'demo.dotcms.com', + path: '/documents/reports/', + type: 'folder', + name: 'reports', + title: 'Reports', + sortOrder: 2, + filesMasks: '*.pdf', + defaultFileType: 'FileAsset', + defaultBaseType: 'DOTASSET', + showOnMenu: true, + permissions }); - const rightClick = (node: DotFolderTreeNodeItem) => { + const rightClick = (data: DotFolderTreeNodeContentData) => { const event = new MouseEvent('contextmenu'); - spectator.triggerEventHandler(DotTreeFolderComponent, 'rightClick', { event, node }); + spectator.triggerEventHandler(DotTreeFolderComponent, 'rightClick', { event, data }); return event; }; it('should publish the folder to the store in the shape the menu and dialog consume', () => { const event = rightClick( - buildFolderNode([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT]) + buildFolderData([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT]) ); expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith({ @@ -1214,17 +1208,9 @@ describe('DotContentDriveSidebarComponent', () => { }); }); - it('should not refetch permissions for a node that already carries them', () => { - rightClick(buildFolderNode([PERMISSIONS_TYPE.READ])); - - expect(contentDriveStore.loadFolderPermissions).not.toHaveBeenCalled(); - }); - - it('should treat an empty permissions array as resolved and not refetch', () => { - // `[]` is a final answer ("no grants"), unlike undefined ("never fetched"). - rightClick(buildFolderNode([])); + it('should treat an empty permissions array as a resolved "no grants"', () => { + rightClick(buildFolderData([])); - expect(contentDriveStore.loadFolderPermissions).not.toHaveBeenCalled(); expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith( expect.objectContaining({ contentlet: expect.objectContaining({ permissions: [] }) @@ -1232,70 +1218,24 @@ describe('DotContentDriveSidebarComponent', () => { ); }); - it('should resolve permissions on demand for a node hydrated without them', () => { - contentDriveStore.loadFolderPermissions.mockReturnValue( - of([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT_PERMISSIONS]) - ); - - rightClick(buildFolderNode(undefined)); + it('should open an empty menu rather than throwing for a folder without permissions', () => { + // Every source now resolves permissions, but an older backend or a rolled-back search + // can still deliver a folder without them; gating must degrade, not blow up. + rightClick(buildFolderData(undefined)); - expect(contentDriveStore.loadFolderPermissions).toHaveBeenCalledWith( - '/documents/reports/', - 'docs-id', - 'reports' - ); expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith( expect.objectContaining({ - contentlet: expect.objectContaining({ - permissions: [PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT_PERMISSIONS] - }) + contentlet: expect.objectContaining({ permissions: [] }) }) ); }); - it('should cache resolved permissions onto the node so the next right-click is instant', () => { - contentDriveStore.loadFolderPermissions.mockReturnValue(of([PERMISSIONS_TYPE.EDIT])); - const node = buildFolderNode(undefined); - - rightClick(node); - rightClick(node); - - expect(contentDriveStore.loadFolderPermissions).toHaveBeenCalledTimes(1); - }); - - it('should open an empty menu rather than throwing when the lookup cannot resolve', () => { - contentDriveStore.loadFolderPermissions.mockReturnValue(of(undefined)); - const node = buildFolderNode(undefined); - - rightClick(node); + it('should anchor the menu on the originating event', () => { + const event = rightClick(buildFolderData([PERMISSIONS_TYPE.EDIT])); expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith( - expect.objectContaining({ - contentlet: expect.objectContaining({ permissions: [] }) - }) + expect.objectContaining({ triggeredEvent: event }) ); - // Unresolved stays unresolved, so a later right-click retries instead of caching a lie. - expect(node.data.permissions).toBeUndefined(); - }); - - it('should ignore a "Load more" node', () => { - spectator.triggerEventHandler(DotTreeFolderComponent, 'rightClick', { - event: new MouseEvent('contextmenu'), - node: { - key: 'load-more:/documents/', - label: '', - data: { - type: LOAD_MORE_NODE_TYPE, - id: 'load-more:/documents/', - path: '/documents/', - hostname: 'demo.dotcms.com', - nextPage: 2, - remaining: 5 - } - } as DotFolderTreeNodeItem - }); - - expect(contentDriveStore.patchContextMenu).not.toHaveBeenCalled(); }); }); }); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts index c1ab11050468..51aa2b414011 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts @@ -3,14 +3,12 @@ import { signalMethod } from '@ngrx/signals'; import { ChangeDetectionStrategy, Component, - DestroyRef, effect, inject, output, untracked, viewChild } from '@angular/core'; -import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import type { TreeNodeCollapseEvent, @@ -31,7 +29,7 @@ import { } from '@dotcms/portlets/content-drive/ui'; import { DotContentDriveStore } from '../../store/dot-content-drive.store'; -import { appendLoadMoreNodes } from '../../utils/functions'; +import { appendLoadMoreNodes, mergeFolderNodePage } from '../../utils/functions'; /** * @description DotContentDriveSidebarComponent is the component that renders the sidebar for the content drive * @@ -52,7 +50,6 @@ import { appendLoadMoreNodes } from '../../utils/functions'; }) export class DotContentDriveSidebarComponent { readonly #store = inject(DotContentDriveStore); - readonly #destroyRef = inject(DestroyRef); readonly $loading = this.#store.sidebarLoading; readonly $folders = this.#store.folders; @@ -174,7 +171,9 @@ export class DotContentDriveSidebarComponent { folder.key !== ALL_FOLDER.key && folder.data?.type !== LOAD_MORE_NODE_TYPE ); - const combined = [...loaded, ...folders]; + // Merge rather than concatenate: the hierarchy load can pin a deep-linked folder to + // the top of a level out of sort order, and paging far enough returns it again. + const combined = mergeFolderNodePage(loaded, folders); this.#store.updateFolders([ allFolder, @@ -199,7 +198,9 @@ export class DotContentDriveSidebarComponent { const loaded = (parent.children ?? []).filter( (child) => child.data?.type !== LOAD_MORE_NODE_TYPE ); - const combined = [...loaded, ...folders]; + // Merge rather than concatenate: the hierarchy load can pin a deep-linked folder to + // the top of a level out of sort order, and paging far enough returns it again. + const combined = mergeFolderNodePage(loaded, folders); parent.children = appendLoadMoreNodes( combined, @@ -241,38 +242,14 @@ export class DotContentDriveSidebarComponent { * Opens the shared folder context menu for a right-clicked tree node, giving the sidebar the * same folder actions the table offers. * - * Nodes loaded by expanding a folder already carry their permissions. Nodes hydrated by the - * deep-link hierarchy load do not (that call's page size exceeds the backend cap for - * `includePermissions`), so those resolve them on demand on first right-click and the result is - * cached back onto the node — a second right-click on the same folder opens immediately. + * Every folder node carries its permissions, whichever way it reached the tree: expand, + * load-more and the deep-link hierarchy load all request them. So this stays synchronous, and a + * right-click opens the menu immediately. * - * @param {DotContentDriveTreeRightClick} rightClick - The originating event and the clicked node + * @param {DotContentDriveTreeRightClick} rightClick - The originating event and clicked folder */ - protected onNodeRightClick({ event, node }: DotContentDriveTreeRightClick): void { - const data = node.data; - - if (!data || data.type === LOAD_MORE_NODE_TYPE) { - return; - } - - if (data.permissions) { - this.#openContextMenu(event, data); - - return; - } - - this.#store - .loadFolderPermissions(data.path, data.id, data.name ?? '') - .pipe(takeUntilDestroyed(this.#destroyRef)) - .subscribe((permissions) => { - // Cache onto the node so the next right-click skips the lookup. `[]` is a valid - // cached answer ("no grants"); only an unresolved lookup stays undefined and retries. - if (permissions) { - data.permissions = permissions; - } - - this.#openContextMenu(event, { ...data, permissions: permissions ?? [] }); - }); + protected onNodeRightClick({ event, data }: DotContentDriveTreeRightClick): void { + this.#openContextMenu(event, data); } /** diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/constants.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/constants.ts index 57a123d0a9dc..f3e701d40d7a 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/constants.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/constants.ts @@ -28,20 +28,21 @@ export const DEFAULT_PAGINATION: DotContentDrivePagination = { export const FOLDER_TREE_PAGE_SIZE = DOT_FOLDER_TREE_PAGE_SIZE; /** - * Page size for the deep-link / initial hierarchy fetch only. - * One request per ancestor level (parallel); large enough that path segments past - * the interactive page of 40 still appear so {@link buildTreeFolderNodes} can select them. - * Expand and load-more keep using {@link FOLDER_TREE_PAGE_SIZE}. - */ -export const FOLDER_TREE_HIERARCHY_PAGE_SIZE = 10000; - -/** - * Page size for the on-demand lookup that resolves a single folder's permissions when its tree node - * was hydrated without them (see `getFolderPermissionsByPath`). Matches the backend cap for - * `includePermissions=true` (`content.drive.folder.search.permissions.max.per.page`, default 200) — - * requesting more would be rejected with a 400. + * Page size for the deep-link / initial hierarchy fetch only. One request per ancestor level + * (parallel). Expand and load-more keep using {@link FOLDER_TREE_PAGE_SIZE}. + * + * Pinned to the backend's cap for `includePermissions=true` + * (`content.drive.folder.search.permissions.max.per.page`, default 200): anything larger is + * rejected with a 400, and the hierarchy load must carry permissions so every node the tree + * renders on first paint can gate its context menu without a second round-trip. + * + * An ancestor sorting past this page is fetched individually instead of by widening the page, + * see `getFolderHierarchyByPath`. + * + * Deliberately a whole multiple of {@link FOLDER_TREE_PAGE_SIZE}: load-more resumes in + * 40-sized pages, so the hierarchy's page count has to convert to a clean page boundary. */ -export const FOLDER_PERMISSIONS_LOOKUP_PAGE_SIZE = 200; +export const FOLDER_TREE_HIERARCHY_PAGE_SIZE = 200; /** Minimum length the folder-search `name` filter accepts; shorter values are rejected with a 400. */ export const FOLDER_NAME_FILTER_MIN_LENGTH = 2; diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts index a43aa666380b..d56a8fe9d15b 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts @@ -13,7 +13,6 @@ import { inject } from '@angular/core'; import { catchError, take } from 'rxjs/operators'; import { DotFolderService } from '@dotcms/data-access'; -import { PermissionType } from '@dotcms/dotcms-models'; import { ALL_FOLDER, DotFolderTreeNodeItem } from '@dotcms/portlets/content-drive/ui'; import { SYSTEM_HOST } from '../../../shared/constants'; @@ -22,8 +21,7 @@ import { applyLoadMoreToHierarchy, FolderTreeHierarchyLevel, getFolderHierarchyByPath, - getFolderNodesByPath, - getFolderPermissionsByPath + getFolderNodesByPath } from '../../../utils/functions'; import { buildTreeFolderNodes } from '../../../utils/tree-folder.utils'; @@ -123,43 +121,6 @@ export function withSidebar() { page ); }, - /** - * Resolves the permission types the current user holds on a single folder. - * - * Only needed for nodes hydrated by {@link loadFolders}: that call resolves the whole - * deep-link hierarchy in one large page, which exceeds the backend's cap for - * `includePermissions`, so those nodes arrive without permissions. Nodes loaded by - * expanding a folder already carry them and never reach this method. - * - * Emits `undefined` when the lookup fails or the folder is not found, so the caller can - * tell that apart from "the user holds no permissions" (`[]`). - */ - loadFolderPermissions: ( - folderPath: string, - folderId: string, - folderName: string - ): Observable => { - const currentSite = store.currentSite(); - - if (!currentSite) { - return of(undefined); - } - - return getFolderPermissionsByPath( - folderPath, - folderId, - folderName, - currentSite, - dotFolderService - ).pipe( - take(1), - catchError((response) => { - console.error('Error loading folder permissions:', response); - - return of(undefined); - }) - ); - }, /** * Sets the selected node diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts index 6f18a565cd12..af0779f4bec8 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts @@ -31,8 +31,7 @@ import { folderSearchViewToDotFolder, getFolderHierarchyByPath, getFolderNodesByPath, - getFolderPermissionsByPath, - getParentPath, + getPathLeafName, getUserSearchableActive, isBinaryCheckboxField, isDateFieldFilterType, @@ -40,7 +39,9 @@ import { isMultiValueFieldFilterType, parseUserSearchableValue, parseWorkflowFilter, + mergeFolderNodePage, parseWorkflowToken, + resolveHierarchyAncestor, serializeUserSearchableValue, toLocalIsoString, workflowEntryToToken @@ -408,6 +409,19 @@ describe('Utility Functions', () => { it('should search the root and every parent path with the hierarchy page size', (done) => { const folderPath = '/main/sub-folder/inner-folder'; + // Every level returns the ancestor the next one descends into, so the hierarchy + // resolves in one request per level with no follow-up lookups. + const childOf: Record = { + '/': 'main', + '/main/': 'sub-folder', + '/main/sub-folder/': 'inner-folder' + }; + mockDotFolderService.searchFolders.mockImplementation(({ path }) => + searchResult( + childOf[path] ? [createFakeFolderSearchView({ path, name: childOf[path] })] : [] + ) + ); + getFolderHierarchyByPath(folderPath, SITE, mockDotFolderService).subscribe({ next: () => { expect(mockDotFolderService.searchFolders).toHaveBeenCalledTimes(4); @@ -473,10 +487,10 @@ describe('Utility Functions', () => { }); }); - it('should not request permissions (its page size exceeds the backend cap)', (done) => { + it('should request permissions so first-paint nodes can gate their context menu', (done) => { getFolderHierarchyByPath('/main', SITE, mockDotFolderService).subscribe({ next: () => { - expect(mockDotFolderService.searchFolders).not.toHaveBeenCalledWith( + expect(mockDotFolderService.searchFolders).toHaveBeenCalledWith( expect.objectContaining({ includePermissions: true }) ); done(); @@ -616,6 +630,126 @@ describe('Utility Functions', () => { } }); }); + + describe('deep-link ancestor pinning', () => { + const page = (names: string[], parentPath: string, total: number) => + of({ + folders: names.map((name) => + createFakeFolderSearchView({ id: `id-${name}`, name, path: parentPath }) + ), + pagination: { totalEntries: total } as DotPagination + }); + + it('should pin an ancestor that sorts past the first page to the top of its level', (done) => { + mockDotFolderService.searchFolders.mockImplementation(({ path, name }) => + path === '/' + ? name + ? page(['zzz'], '/', 1) + : page(['a-one', 'a-two'], '/', 253) + : page([], path, 0) + ); + + getFolderHierarchyByPath('/zzz/', SITE, mockDotFolderService).subscribe({ + next: (levels) => { + // Top of the level, not appended after its siblings. + expect(levels[0].folders.map(({ path }) => path)).toEqual([ + '/zzz/', + '/a-one/', + '/a-two/' + ]); + done(); + }, + error: done + }); + }); + + it('should pin a nested ancestor into its own level, leaving the root level alone', (done) => { + mockDotFolderService.searchFolders.mockImplementation(({ path, name }) => { + if (path === '/') { + return page(['parent'], '/', 1); + } + + if (path === '/parent/') { + return name + ? page(['zzz'], '/parent/', 1) + : page(['a-one'], '/parent/', 253); + } + + return page([], path, 0); + }); + + getFolderHierarchyByPath('/parent/zzz/', SITE, mockDotFolderService).subscribe({ + next: (levels) => { + expect(levels[0].folders.map(({ path }) => path)).toEqual(['/parent/']); + expect(levels[1].folders.map(({ path }) => path)).toEqual([ + '/parent/zzz/', + '/parent/a-one/' + ]); + done(); + }, + error: done + }); + }); + + it('should not look the ancestor up when it is already on the first page', (done) => { + mockDotFolderService.searchFolders.mockImplementation(({ path }) => + path === '/' ? page(['zzz'], '/', 1) : page([], path, 0) + ); + + getFolderHierarchyByPath('/zzz/', SITE, mockDotFolderService).subscribe({ + next: () => { + // One request per level ('/' and '/zzz/'), no follow-up lookup. + expect(mockDotFolderService.searchFolders).toHaveBeenCalledTimes(2); + done(); + }, + error: done + }); + }); + + it('should leave the level untouched when the ancestor cannot be resolved', (done) => { + // What a folder the user cannot READ looks like: filtered out of every response, + // never a 403. It must not be pinned, and the readable siblings must still render. + mockDotFolderService.searchFolders.mockImplementation(({ path, name }) => + path === '/' && !name ? page(['a-one'], '/', 253) : page([], path, 0) + ); + + getFolderHierarchyByPath('/secret/', SITE, mockDotFolderService).subscribe({ + next: (levels) => { + expect(levels[0].folders.map(({ path }) => path)).toEqual(['/a-one/']); + done(); + }, + error: done + }); + }); + + it('should derive nextPage from folders fetched, not from the pinned node', (done) => { + const fullPage = Array.from( + { length: FOLDER_TREE_HIERARCHY_PAGE_SIZE }, + (_, i) => `folder-${String(i).padStart(3, '0')}` + ); + + mockDotFolderService.searchFolders.mockImplementation(({ path, name }) => + path === '/' + ? name + ? page(['zzz'], '/', 1) + : page(fullPage, '/', FOLDER_TREE_HIERARCHY_PAGE_SIZE + 53) + : page([], path, 0) + ); + + getFolderHierarchyByPath('/zzz/', SITE, mockDotFolderService).subscribe({ + next: (levels) => { + // 200 fetched / 40 per load-more page + 1. The pinned node brings the + // rendered count to 201, which must not shift the resume point. + expect(levels[0].folders).toHaveLength(FOLDER_TREE_HIERARCHY_PAGE_SIZE + 1); + expect(levels[0].nextPage).toBe( + FOLDER_TREE_HIERARCHY_PAGE_SIZE / FOLDER_TREE_PAGE_SIZE + 1 + ); + done(); + }, + error: done + }); + }); + }); }); describe('getFolderNodesByPath', () => { @@ -841,7 +975,7 @@ describe('Utility Functions', () => { }); describe('applyLoadMoreToHierarchy', () => { - it('should append a load-more sentinel with nextPage 2 when more entries remain', () => { + it('should append a load-more sentinel resuming at the level own nextPage', () => { const rootFolder = createTreeNode({ id: 'root-1', inode: 'inode-1', @@ -864,7 +998,10 @@ describe('Utility Functions', () => { addChildrenAllowed: true } ], - totalEntries: 50 + totalEntries: 50, + // The hierarchy pages at 200 while load-more pages at 40, so a level that + // consumed one hierarchy page resumes at 40-sized page 6, not page 2. + nextPage: 6 } ], 'test.com' @@ -875,7 +1012,7 @@ describe('Utility Functions', () => { expect(loadMore.data).toEqual( expect.objectContaining({ type: 'load-more', - nextPage: 2, + nextPage: 6, remaining: 49 }) ); @@ -1323,20 +1460,19 @@ describe('folderSearchViewToDotFolder', () => { }); }); -describe('getParentPath', () => { +describe('getPathLeafName', () => { it.each([ - ['/a/b/', '/a/'], - ['/a/b', '/a/'], - ['/b/', '/'], - ['/b', '/'], - ['/', '/'], - ['', '/'] - ])('should resolve the parent of %s as %s', (path, expected) => { - expect(getParentPath(path)).toBe(expected); + ['/a/b/', 'b'], + ['/a/b', 'b'], + ['/b/', 'b'], + ['/', ''], + ['', ''] + ])('should resolve the own name of %s as %s', (path, expected) => { + expect(getPathLeafName(path)).toBe(expected); }); }); -describe('getFolderPermissionsByPath', () => { +describe('resolveHierarchyAncestor', () => { let mockDotFolderService: { searchFolders: jest.Mock }; const searchResult = (folders: FolderSearchView[]) => @@ -1346,11 +1482,10 @@ describe('getFolderPermissionsByPath', () => { mockDotFolderService = { searchFolders: jest.fn().mockReturnValue(searchResult([])) }; }); - it('should query the parent level with permissions, narrowed by folder name', (done) => { - getFolderPermissionsByPath( + it('should query the level with permissions, narrowed by the folder own name', (done) => { + resolveHierarchyAncestor( + '/main/', '/main/docs/', - 'docs-id', - 'docs', createFakeSite({ identifier: 'site-1', hostname: 'demo.dotcms.com' }), mockDotFolderService as unknown as DotFolderService ).subscribe({ @@ -1361,6 +1496,7 @@ describe('getFolderPermissionsByPath', () => { path: '/main/', recursive: false, name: 'docs', + per_page: FOLDER_TREE_HIERARCHY_PAGE_SIZE, includePermissions: true }) ); @@ -1371,10 +1507,9 @@ describe('getFolderPermissionsByPath', () => { }); it('should omit the name filter when the folder name is too short for the endpoint', (done) => { - getFolderPermissionsByPath( + resolveHierarchyAncestor( + '/main/', '/main/a/', - 'a-id', - 'a', createFakeSite({ identifier: 'site-1' }), mockDotFolderService as unknown as DotFolderService ).subscribe({ @@ -1388,54 +1523,119 @@ describe('getFolderPermissionsByPath', () => { }); }); - it('should return the permissions of the matching folder', (done) => { + it('should return the folder matching the exact path, not a partial name match', (done) => { mockDotFolderService.searchFolders.mockReturnValue( searchResult([ createFakeFolderSearchView({ id: 'other', + path: '/main/', name: 'docs-archive', permissions: [PERMISSIONS_TYPE.READ] }), createFakeFolderSearchView({ id: 'docs-id', + path: '/main/', name: 'docs', permissions: [PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT] }) ]) ); - getFolderPermissionsByPath( + resolveHierarchyAncestor( + '/main/', '/main/docs/', - 'docs-id', - 'docs', - createFakeSite({ identifier: 'site-1' }), + createFakeSite({ identifier: 'site-1', hostname: 'demo.dotcms.com' }), mockDotFolderService as unknown as DotFolderService ).subscribe({ - next: (permissions) => { - expect(permissions).toEqual([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT]); + next: (folder) => { + expect(folder?.id).toBe('docs-id'); + expect(folder?.permissions).toEqual([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT]); done(); }, error: done }); }); - it('should resolve undefined when the folder is not in the page', (done) => { + it('should issue a single request and not page the level', (done) => { mockDotFolderService.searchFolders.mockReturnValue( - searchResult([createFakeFolderSearchView({ id: 'someone-else' })]) + of({ + folders: [createFakeFolderSearchView({ path: '/main/', name: 'someone-else' })], + pagination: { totalEntries: 5000 } as DotPagination + }) ); - getFolderPermissionsByPath( + resolveHierarchyAncestor( + '/main/', '/main/docs/', - 'docs-id', - 'docs', - createFakeSite({ identifier: 'site-1' }), + createFakeSite({ identifier: 'site-1', hostname: 'demo.dotcms.com' }), mockDotFolderService as unknown as DotFolderService ).subscribe({ - next: (permissions) => { - expect(permissions).toBeUndefined(); + next: (folder) => { + expect(folder).toBeUndefined(); + expect(mockDotFolderService.searchFolders).toHaveBeenCalledTimes(1); done(); }, error: done }); }); }); + +describe('mergeFolderNodePage', () => { + const node = (id: string, path: string) => + createTreeNode({ + id, + inode: `inode-${id}`, + hostName: 'test.com', + path, + addChildrenAllowed: true + }); + + const ids = (nodes: ReturnType[]) => nodes.map((item) => item.data?.id); + + it('should append the page when nothing overlaps', () => { + const merged = mergeFolderNodePage([node('a', '/a/')], [node('b', '/b/')]); + + expect(ids(merged)).toEqual(['a', 'b']); + }); + + it('should render a folder once when the page repeats one already on screen', () => { + // The hierarchy pinned `z` to the top; paging far enough returns it in sort order. + const merged = mergeFolderNodePage( + [node('z', '/z/'), node('a', '/a/')], + [node('b', '/b/'), node('z', '/z/')] + ); + + expect(ids(merged)).toEqual(['a', 'b', 'z']); + }); + + it('should move the repeated folder from its pinned slot to where it belongs', () => { + const merged = mergeFolderNodePage( + [node('z', '/z/'), node('a', '/a/')], + [node('z', '/z/'), node('b', '/b/')] + ); + + expect(ids(merged)).toEqual(['a', 'z', 'b']); + }); + + it('should keep the on-screen node, so its loaded children and expansion survive', () => { + const pinned = node('z', '/z/'); + pinned.expanded = true; + pinned.children = [node('inner', '/z/inner/')]; + + const merged = mergeFolderNodePage([pinned, node('a', '/a/')], [node('z', '/z/')]); + const retained = merged[merged.length - 1]; + + // Identity matters: the incoming copy is a bare node with no children or expansion. + expect(retained).toBe(pinned); + expect(retained.expanded).toBe(true); + expect(retained.children).toHaveLength(1); + }); + + it('should leave a load-more sentinel in place rather than treating it as a folder', () => { + const loadMore = buildLoadMoreNode('/', 'test.com', 2, 5); + + const merged = mergeFolderNodePage([node('a', '/a/'), loadMore], [node('b', '/b/')]); + + expect(ids(merged)).toEqual(['a', loadMore.data?.id, 'b']); + }); +}); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts index 9853295989a9..029245d76e1e 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts @@ -1,7 +1,7 @@ import { format } from 'date-fns'; -import { forkJoin, Observable } from 'rxjs'; +import { forkJoin, Observable, of } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { map, switchMap } from 'rxjs/operators'; import { DotFolderService } from '@dotcms/data-access'; import { @@ -14,8 +14,7 @@ import { DotFolder, DotSite, FolderSearchView, - LOAD_MORE_NODE_TYPE, - PermissionType + LOAD_MORE_NODE_TYPE } from '@dotcms/dotcms-models'; import { getSingleSelectableFieldOptions } from '@dotcms/edit-content'; import { DotFolderTreeNodeItem } from '@dotcms/portlets/content-drive/ui'; @@ -28,7 +27,6 @@ import { FIELD_FILTER_KEY_VALUE_TYPE, FIELD_FILTER_MULTI_VALUE_TYPES, FOLDER_NAME_FILTER_MIN_LENGTH, - FOLDER_PERMISSIONS_LOOKUP_PAGE_SIZE, FOLDER_TREE_HIERARCHY_PAGE_SIZE, FOLDER_TREE_PAGE_SIZE, USER_SEARCHABLE_PREFIX, @@ -292,18 +290,108 @@ export type FolderTreeHierarchyLevel = { path: string; folders: DotFolder[]; totalEntries: number; + /** + * The 1-based page "Load more" should request next for this level, expressed in + * {@link FOLDER_TREE_PAGE_SIZE} units because that is what load-more pages by. + * + * Derived from the folders actually fetched, never from the rendered node count: a level can + * carry one extra folder that {@link resolveHierarchyAncestor} appended out of sort order, and + * counting that as paged-through would make load-more skip a page of real folders. + */ + nextPage: number; }; +/** + * The last segment of a folder path: `/a/b/` → `b`, `/a/` → `a`. + * + * @param {string} folderPath - A folder's own path, with or without a trailing slash + * @returns {string} the folder's own name, or `''` for the site root + */ +export function getPathLeafName(folderPath: string): string { + const segments = folderPath.split('/').filter(Boolean); + + return segments[segments.length - 1] ?? ''; +} + +/** + * Fetches one specific folder of a level directly, for the case where a deep-linked ancestor sorts + * past the level's first hierarchy page and would otherwise be missing from the tree. + * + * The tree has to show the folder the drive is open on. Widening the hierarchy page to guarantee + * that is not an option: `includePermissions=true` caps `per_page` + * (`content.drive.folder.search.permissions.max.per.page`), and the nodes on first paint need + * permissions to gate their context menu. Paging the level until the folder turns up is not one + * either: it trades one request for an unbounded chain to find something we already know the exact + * path of. So the level is queried once more, narrowed by the folder's own name, and matched on + * exact path. + * + * `POST /api/v1/folder/byPath` would be the natural "fetch this one folder" call, but it is + * deprecated for removal, returns a path's *subfolders* rather than the folder itself, and carries + * no permissions. + * + * Resolves to `undefined` in two cases, both of which leave the folder unpinned. The `name` filter + * needs {@link FOLDER_NAME_FILTER_MIN_LENGTH} characters, so a one-character folder name drops the + * filter and falls back to the level's first page; and `name` is a case-insensitive *partial* match, + * so a level holding more same-substring siblings than fit one page can still exclude the target. + * Both need a level wide enough to have overflowed in the first place. An exact-match filter, or a + * folder-search that accepts an identifier, would close them. + * + * @param {string} levelPath - Parent path being listed, e.g. `/a/` + * @param {string} ancestorPath - Full path of the folder to resolve, e.g. `/a/b/` + * @param {DotSite} site - The site to scope the search + * @param {DotFolderService} dotFolderService - The folder service + * @returns {Observable} the folder, or `undefined` if it could not be reached + */ +export function resolveHierarchyAncestor( + levelPath: string, + ancestorPath: string, + site: DotSite, + dotFolderService: DotFolderService +): Observable { + const name = getPathLeafName(ancestorPath); + + return dotFolderService + .searchFolders({ + siteId: site.identifier, + path: levelPath, + recursive: false, + name: name.length >= FOLDER_NAME_FILTER_MIN_LENGTH ? name : undefined, + orderby: 'name', + direction: 'ASC', + page: 1, + per_page: FOLDER_TREE_HIERARCHY_PAGE_SIZE, + includePermissions: true + }) + .pipe( + map(({ folders }) => + folders + .map((view) => folderSearchViewToDotFolder(view, site.hostname)) + .find((folder) => folder.path === ancestorPath) + ) + ); +} + /** * Fetches the folders for every level of a target path using parallel search calls, so the sidebar * tree can be rendered expanded down to that path (deep-link restore). * * One `GET /api/v1/folder/search` (non-recursive) call is made per level, starting at the site root - * (`'/'`) and descending through each parent path. Uses {@link FOLDER_TREE_HIERARCHY_PAGE_SIZE} - * (large, page 1 only) so ancestors past the interactive page of 40 still resolve without a - * sequential page-until-found waterfall. Interactive expand/load-more use - * {@link getFolderNodesByPath} with {@link FOLDER_TREE_PAGE_SIZE}. Callers should append load-more - * via {@link applyLoadMoreToHierarchy} when `totalEntries` exceeds the returned page. + * (`'/'`) and descending through each parent path, all in parallel. Every level requests + * `includePermissions`, so each node the tree renders on first paint can gate its context menu + * without a second round-trip. That pins the page to {@link FOLDER_TREE_HIERARCHY_PAGE_SIZE}, the + * backend's cap when permissions are requested. + * + * Because the page is capped, a level wide enough can sort the next ancestor past it. The drive + * still has to show the folder it is open on, so that one folder is fetched individually (see + * {@link resolveHierarchyAncestor}) rather than the page being widened, and is *pinned to the top* + * of its level. Pinning rather than appending is deliberate: dropped in at the end it would read as + * the next folder in sort order, which it is not, and it would sit next to the level's "Load more" + * where it is easy to miss. At the top it reads as "the folder you are in". If the user later pages + * far enough to reach its real position, {@link mergeFolderNodePage} moves it there. + * + * Interactive expand/load-more use {@link getFolderNodesByPath} with {@link FOLDER_TREE_PAGE_SIZE}. + * Callers should append load-more via {@link applyLoadMoreToHierarchy} when `totalEntries` exceeds + * the returned page. * * @param {string} folderPath - The folder path (without hostname) to expand to, e.g. `/a/b/` * @param {DotSite} site - The site to scope the search (its `identifier` and `hostname` are used) @@ -318,7 +406,12 @@ export function getFolderHierarchyByPath( // The root level (`'/'`) is always fetched first; deeper levels come from the target path. const paths = ['/', ...generateAllParentPaths(folderPath)]; - const folderRequests = paths.map((path) => + // Level `i` is the one that must contain `expectedPaths[i]` for the tree to keep descending. + // The deepest level has no entry here: it holds the target folder's own children, so there is + // nothing further to reach and its first page is all the tree needs. + const expectedPaths = generateAllParentPaths(folderPath); + + const folderRequests = paths.map((path, levelIndex) => dotFolderService .searchFolders({ siteId: site.identifier, @@ -327,12 +420,8 @@ export function getFolderHierarchyByPath( orderby: 'name', direction: 'ASC', page: 1, - // Deliberately NOT requesting `includePermissions` here: the backend caps the page - // size when permissions are requested (default 200) and rejects anything larger with - // a 400, and this call intentionally uses a much larger page to resolve deep-link - // ancestors in one shot. Nodes hydrated here therefore arrive without permissions; - // `getFolderPermissionsByPath` resolves them on demand when one is right-clicked. - per_page: FOLDER_TREE_HIERARCHY_PAGE_SIZE + per_page: FOLDER_TREE_HIERARCHY_PAGE_SIZE, + includePermissions: true }) .pipe( map(({ folders, pagination }) => ({ @@ -340,8 +429,30 @@ export function getFolderHierarchyByPath( folders: folders.map((view) => folderSearchViewToDotFolder(view, site.hostname) ), - totalEntries: pagination?.totalEntries ?? folders.length - })) + totalEntries: pagination?.totalEntries ?? folders.length, + // Whole pages consumed, converted to load-more's page size. Safe because the + // hierarchy page is a multiple of it; a partial page means the level is fully + // loaded and no "Load more" is appended, so the value goes unused. + nextPage: Math.floor(folders.length / FOLDER_TREE_PAGE_SIZE) + 1 + })), + switchMap((level) => { + const expectedPath = expectedPaths[levelIndex]; + + if (!expectedPath || level.folders.some(({ path }) => path === expectedPath)) { + return of(level); + } + + return resolveHierarchyAncestor( + path, + expectedPath, + site, + dotFolderService + ).pipe( + map((ancestor) => + ancestor ? { ...level, folders: [ancestor, ...level.folders] } : level + ) + ); + }) ) ); @@ -390,65 +501,6 @@ export function getFolderNodesByPath( ); } -/** - * The parent path of a folder path: `/a/b/` → `/a/`, `/b/` → `/`. - * `GET /api/v1/folder/search` scopes a non-recursive search by the *parent* path, while tree nodes - * carry their own full path. - * - * @param {string} folderPath - The folder's own path, with or without a trailing slash - * @returns {string} the parent path, always trailing-slashed - */ -export function getParentPath(folderPath: string): string { - const withoutTrailing = folderPath.endsWith('/') ? folderPath.slice(0, -1) : folderPath; - const lastSeparator = withoutTrailing.lastIndexOf('/'); - - return lastSeparator <= 0 ? '/' : withoutTrailing.slice(0, lastSeparator + 1); -} - -/** - * Resolves the permission types the current user holds on a single folder. - * - * Needed because the deep-link hierarchy load ({@link getFolderHierarchyByPath}) cannot request - * permissions — its page size exceeds the backend cap — so the folders visible on first render - * arrive without them. Without this, right-clicking those nodes would produce an empty menu that - * is indistinguishable from "you have no rights on this folder". - * - * Queries the folder's own level, narrowed by name so the response stays small, and matches the - * folder by id. Resolves to `undefined` when the folder is not found in the page, letting the - * caller tell "no grants" (`[]`) from "could not resolve". - * - * @param {string} folderPath - The folder's own full path, e.g. `/a/b/` - * @param {string} folderId - Identifier of the folder to match in the response - * @param {string} folderName - The folder's own name, used to narrow the query - * @param {DotSite} site - Site scoping the search - * @param {DotFolderService} dotFolderService - The folder service - * @returns {Observable} the folder's permissions, if resolved - */ -export function getFolderPermissionsByPath( - folderPath: string, - folderId: string, - folderName: string, - site: DotSite, - dotFolderService: DotFolderService -): Observable { - return dotFolderService - .searchFolders({ - siteId: site.identifier, - path: getParentPath(folderPath), - recursive: false, - // The endpoint rejects a filter shorter than 2 characters, so single-character folder - // names fall back to an unfiltered page of the level. - name: folderName.length >= FOLDER_NAME_FILTER_MIN_LENGTH ? folderName : undefined, - page: 1, - per_page: FOLDER_PERMISSIONS_LOOKUP_PAGE_SIZE, - includePermissions: true - }) - .pipe( - map(({ folders }) => folders.find((folder) => folder.id === folderId)), - map((folder) => folder?.permissions ?? undefined) - ); -} - /** * Builds the synthetic "Load more" node appended to the end of a paginated folder level. It is not * a real folder: it is not selectable and carries the paging cursor (`nextPage`) and how many @@ -477,6 +529,52 @@ export function buildLoadMoreNode( }) as DotFolderTreeNodeItem; } +/** + * Merges a freshly loaded page of folder nodes into the ones a level already shows. + * + * Plain concatenation is not enough because the hierarchy load can pin a folder to the top of a + * level out of sort order (see {@link getFolderHierarchyByPath}). Page far enough and that same + * folder arrives again in its real position, which would render it twice. + * + * The already-rendered node wins on identity but takes the incoming node's position: it may be + * expanded, hold loaded children and carry the current selection, none of which the fresh copy has. + * So the pinned folder stops being pinned and settles where it belongs, with its subtree intact. + * + * @param {DotFolderTreeNodeItem[]} loaded - Nodes already rendered for the level (no "Load more") + * @param {DotFolderTreeNodeItem[]} page - The newly fetched page, in sort order + * @returns {DotFolderTreeNodeItem[]} the merged level, free of duplicates + */ +export function mergeFolderNodePage( + loaded: DotFolderTreeNodeItem[], + page: DotFolderTreeNodeItem[] +): DotFolderTreeNodeItem[] { + const nodeId = (node: DotFolderTreeNodeItem): string | undefined => + node.data?.type !== LOAD_MORE_NODE_TYPE ? node.data?.id : undefined; + + const existingById = new Map( + loaded.flatMap((node) => { + const id = nodeId(node); + + return id ? [[id, node] as const] : []; + }) + ); + + const incomingIds = new Set(page.flatMap((node) => nodeId(node) ?? [])); + + return [ + ...loaded.filter((node) => { + const id = nodeId(node); + + return !id || !incomingIds.has(id); + }), + ...page.map((node) => { + const id = nodeId(node); + + return (id && existingById.get(id)) || node; + }) + ]; +} + /** * Appends a "Load more" sentinel when more folders remain beyond the loaded page. */ @@ -501,8 +599,9 @@ export function appendLoadMoreNodes( * Applies load-more sentinels to each level of a freshly built hierarchy. * Root-level sentinels sit as siblings of root folders; nested ones go under the parent node. * - * Hierarchy always fetches page 1 (with {@link FOLDER_TREE_HIERARCHY_PAGE_SIZE}), so the next - * interactive page is always `2` when `totalEntries` exceeds the returned folders. + * Each level carries its own `nextPage`, because the hierarchy pages at + * {@link FOLDER_TREE_HIERARCHY_PAGE_SIZE} while load-more pages at {@link FOLDER_TREE_PAGE_SIZE}. + * Resuming at a fixed page would re-request folders already on screen. */ export function applyLoadMoreToHierarchy( rootNodes: DotFolderTreeNodeItem[], @@ -513,14 +612,12 @@ export function applyLoadMoreToHierarchy( return rootNodes; } - const nextPageAfterHierarchy = 2; - const roots = appendLoadMoreNodes( rootNodes, levels[0].totalEntries, levels[0].path, hostname, - nextPageAfterHierarchy + levels[0].nextPage ); for (let i = 1; i < levels.length; i++) { @@ -536,7 +633,7 @@ export function applyLoadMoreToHierarchy( level.totalEntries, level.path, hostname, - nextPageAfterHierarchy + level.nextPage ); } diff --git a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.html b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.html index c928a6af3e17..7b22e521bb5d 100644 --- a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.html +++ b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.html @@ -21,9 +21,9 @@ data-testid="tree-node-label" [attr.data-json-node]="node.data | json" [attr.data-id]="node?.data?.id" + [attr.data-node-key]="node.key" class="font-normal" - [class.active]="$activeDropNode()?.id === node?.data.id" - (contextmenu)="onContextMenu($event, node)"> + [class.active]="$activeDropNode()?.id === node?.data.id"> {{ node.key === ALL_FOLDER_KEY ? (node.label | dm) : (node.label | dotFolderName) }} diff --git a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.spec.ts b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.spec.ts index b35008b5dd70..4a9ef8d65c86 100644 --- a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.spec.ts @@ -1,16 +1,17 @@ -import { createComponentFactory, Spectator } from '@openng/spectator/jest'; +import { byTestId, createComponentFactory, Spectator } from '@openng/spectator/jest'; import type { TreeNode } from 'primeng/api'; import { SkeletonModule } from 'primeng/skeleton'; import { Tree, TreeModule, TreeNodeExpandEvent, TreeNodeCollapseEvent } from 'primeng/tree'; import { DotMessageService } from '@dotcms/data-access'; +import { createLoadMoreTreeNode } from '@dotcms/dotcms-models'; import { DotFolderTreeComponent, DotFolderNamePipe } from '@dotcms/ui'; import { MockDotMessageService } from '@dotcms/utils-testing'; import { DotTreeFolderComponent } from './dot-tree-folder.component'; -import { ALL_FOLDER, LOAD_MORE_NODE_TYPE, SYSTEM_HOST_ID } from '../shared/constants'; +import { ALL_FOLDER, SYSTEM_HOST_ID } from '../shared/constants'; import { DotFolderTreeNodeItem } from '../shared/models'; // Mock DragEvent since it's not available in Jest environment @@ -824,6 +825,8 @@ describe('DotTreeFolderComponent', () => { }); describe('right-click', () => { + const ALL_FOLDER_ID = 'site-1'; + const folderNode: DotFolderTreeNodeItem = { key: 'folder-1', label: '/application/content/', @@ -835,61 +838,123 @@ describe('DotTreeFolderComponent', () => { } }; - const rightClickOn = (node: DotFolderTreeNodeItem) => { - const event = new MouseEvent('contextmenu', { cancelable: true }); + const childNode: DotFolderTreeNodeItem = { + key: 'folder-2', + label: '/application/content/images/', + data: { + id: 'folder-2', + hostname: 'demo.dotcms.com', + path: '/application/content/images/', + type: 'folder' + } + }; + + // Built by the real factory: PrimeNG matches its template on `node.type`, so a hand-rolled + // literal carrying only `data.type` would never render as a sentinel. + const loadMoreNode = createLoadMoreTreeNode({ + levelKey: '/application/content/', + nextPage: 2, + remaining: 10, + path: '/application/content/', + hostname: 'demo.dotcms.com' + }) as DotFolderTreeNodeItem; + + const allFolderNode: DotFolderTreeNodeItem = { + key: ALL_FOLDER.key, + label: 'All folders', + data: { + id: ALL_FOLDER_ID, + hostname: 'demo.dotcms.com', + path: '', + type: 'folder' + } + }; + + /** The node's own row: toggler, icon and label, but not its children. */ + const rowFor = (id: string): HTMLElement => + spectator + .query(`[data-testid="tree-node-label"][data-id="${id}"]`) + .closest('.p-tree-node-content'); + + const rightClickOn = (target: Element) => { + const event = new MouseEvent('contextmenu', { cancelable: true, bubbles: true }); jest.spyOn(event, 'preventDefault'); - // The handler is protected — reached the way the template reaches it. - ( - component as unknown as { - onContextMenu: (e: MouseEvent, n: DotFolderTreeNodeItem) => void; - } - ).onContextMenu(event, node); + target.dispatchEvent(event); return event; }; - it('should emit the event and node for a folder node', () => { - const emitted = jest.fn(); + let emitted: jest.Mock; + + beforeEach(() => { + emitted = jest.fn(); component.rightClick.subscribe(emitted); - const event = rightClickOn(folderNode); + spectator.fixture.componentRef.setInput('folders', [ + allFolderNode, + { + ...folderNode, + expanded: true, + children: [childNode, loadMoreNode] + } + ]); + spectator.detectChanges(); + }); + + it('should emit the event and the clicked folder', () => { + const event = rightClickOn(rowFor('folder-1')); - expect(emitted).toHaveBeenCalledWith({ event, node: folderNode }); + expect(emitted).toHaveBeenCalledWith({ + event, + data: expect.objectContaining({ id: 'folder-1' }) + }); }); it('should suppress the native browser menu for a folder node', () => { - const event = rightClickOn(folderNode); + const event = rightClickOn(rowFor('folder-1')); expect(event.preventDefault).toHaveBeenCalled(); }); - it('should ignore the "All folders" root, which is not a real folder', () => { - const emitted = jest.fn(); - component.rightClick.subscribe(emitted); + it('should respond anywhere on the row, not only on the label text', () => { + const label = spectator.query('[data-testid="tree-node-label"][data-id="folder-1"]'); + + rightClickOn(label); + + expect(emitted).toHaveBeenCalledWith({ + event: expect.anything(), + data: expect.objectContaining({ id: 'folder-1' }) + }); + }); + + it('should emit the nested folder, not its parent, when a child row is clicked', () => { + rightClickOn(rowFor('folder-2')); - const event = rightClickOn({ ...folderNode, key: ALL_FOLDER.key }); + expect(emitted).toHaveBeenCalledWith({ + event: expect.anything(), + data: expect.objectContaining({ id: 'folder-2' }) + }); + }); + + it('should ignore the "All folders" root, which is not a real folder', () => { + const event = rightClickOn(rowFor(ALL_FOLDER_ID)); expect(emitted).not.toHaveBeenCalled(); // No menu to show, so the native one is left alone rather than swallowed. expect(event.preventDefault).not.toHaveBeenCalled(); }); - it('should ignore "Load more" sentinels', () => { - const emitted = jest.fn(); - component.rightClick.subscribe(emitted); + it('should ignore "Load more" sentinels rather than opening the parent menu', () => { + const loadMore = spectator.query(byTestId('tree-load-more')); - const event = rightClickOn({ - key: 'load-more:/application/', - label: '', - data: { - type: LOAD_MORE_NODE_TYPE, - id: 'load-more:/application/', - path: '/application/', - hostname: 'demo.dotcms.com', - nextPage: 2, - remaining: 10 - } - } as DotFolderTreeNodeItem); + const event = rightClickOn(loadMore); + + expect(emitted).not.toHaveBeenCalled(); + expect(event.preventDefault).not.toHaveBeenCalled(); + }); + + it('should ignore a right-click outside any node row', () => { + const event = rightClickOn(spectator.element); expect(emitted).not.toHaveBeenCalled(); expect(event.preventDefault).not.toHaveBeenCalled(); diff --git a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.ts b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.ts index 6bfa89e82be3..d8b1ff713c70 100644 --- a/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.ts +++ b/core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.ts @@ -13,9 +13,10 @@ import { import { TreeNode } from 'primeng/api'; import { TreeNodeExpandEvent, TreeNodeCollapseEvent } from 'primeng/types/tree'; +import { isTreeNodeContentData } from '@dotcms/dotcms-models'; import { DotFolderTreeComponent, DotFolderNamePipe, DotMessagePipe } from '@dotcms/ui'; -import { ALL_FOLDER, LOAD_MORE_NODE_TYPE } from '../shared/constants'; +import { ALL_FOLDER } from '../shared/constants'; import { DotFolderTreeNodeData, DotFolderTreeNodeItem, @@ -69,21 +70,50 @@ export class DotTreeFolderComponent { /** * @description Emits a right-click on a real folder node so the consumer can open the shared - * context menu. The browser menu is suppressed only for nodes that can actually produce one: - * the synthetic "All folders" root and "Load more" sentinels are not folders, so they keep the - * native menu rather than swallowing the event for no reason. + * context menu. + * + * Bound on the host and resolved from the row rather than on the label element, so the whole + * node responds the way the table's rows do: the toggler, the indent gutter and the empty space + * past a short folder name all open the menu, instead of only the few characters of text. + * Resolution mirrors `onDragEnter`/`onDragOver`, which already treat the row as the target. + * + * The browser menu is suppressed only for nodes that can actually produce one: the synthetic + * "All folders" root and "Load more" sentinels are not folders, so they keep the native menu + * rather than swallowing the event for no reason. + * * @param event - The contextmenu MouseEvent - * @param node - The tree node under the cursor */ - protected onContextMenu(event: MouseEvent, node: DotFolderTreeNodeItem): void { - const data = node?.data; + @HostListener('contextmenu', ['$event']) + onContextMenu(event: MouseEvent): void { + // A node's content wrapper holds its toggler, icon and label but *not* its children list, + // so the nearest one is always the clicked node's own row. Resolving against the `treeitem` + // element instead would climb to the parent whenever a row has no label of its own, such as + // a "Load more" sentinel, and open that parent's menu. + const label = (event.target as HTMLElement | null) + ?.closest('.p-tree-node-content') + ?.querySelector('[data-testid="tree-node-label"]'); + + const nodeData = label?.getAttribute('data-json-node'); - if (!data || data.type === LOAD_MORE_NODE_TYPE || node.key === this.ALL_FOLDER_KEY) { + if (!nodeData) { + return; + } + + const data = JSON.parse(nodeData) as DotFolderTreeNodeData; + + // Only nodes that can actually produce a menu get the native one suppressed. "Load more" + // sentinels are not folders (and render a different template, so they should not reach + // here at all) and neither is the synthetic "All folders" root. Both keep the browser menu + // rather than having it swallowed for nothing. + if ( + !isTreeNodeContentData(data) || + label?.getAttribute('data-node-key') === this.ALL_FOLDER_KEY + ) { return; } event.preventDefault(); - this.rightClick.emit({ event, node }); + this.rightClick.emit({ event, data }); } /** diff --git a/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts b/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts index 691f3b2eb3c7..a307a5d1139f 100644 --- a/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts +++ b/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts @@ -125,10 +125,14 @@ export type DotFolderTreeNodeItem = TreeNode; /** * @export * @interface DotContentDriveTreeRightClick - * @description Right-click on a folder node in the sidebar tree. Carries the original event (the - * shared context menu anchors itself to it) and the node that was clicked. + * @description Right-click on a folder row in the sidebar tree. Carries the original event (the + * shared context menu anchors itself to it) and the folder the row renders. + * + * Folder data rather than the `TreeNode`: the tree reads the clicked row straight from the DOM, as + * its drag-and-drop already does, instead of searching its own input for a matching node. That + * keeps the component presentational, and the data is all a consumer needs to act on the folder. */ export interface DotContentDriveTreeRightClick { event: MouseEvent; - node: DotFolderTreeNodeItem; + data: DotFolderTreeNodeContentData; } From 54a810a803fed5b1880375a6c4e75979ff43cc48 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Thu, 13 Aug 2026 13:34:57 -0300 Subject: [PATCH 4/6] fix(content-drive): reveal the folder the drive opened on after a cold load (#36595) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hierarchy load already expands the tree down to the deep-linked folder, but a level can hold hundreds of folders, so on a cold load the folder was drawn far below the fold with the viewport still at the top. The reveal already existed for table navigation (`handleSelectedNodeFromTable` expands the path and scrolls), but it is gated on `data.fromTable`, which only the table's double-click sets. Selections built by `buildTreeFolderNodes` carry no flag, so nothing scrolled. Both paths now share `#revealNode`. Only the trigger differs, and it has to: the table's is "selection changed", which a cold load cannot use, because at the moment the store publishes that selection the tree is not on screen yet — `dot-tree-folder` still shows its loading placeholder, so `querySelector` finds no row and the scroll silently no-ops. The cold-load reveal therefore hangs off the load finishing instead. `#revealNode` defers through `afterNextRender`, which both callers need. The table's reveal runs straight after `recursiveExpandOneNode`, and that only marks ancestors expanded — a branch still fetching its children has no row to scroll to either. This also fixes `loadFolders` never setting `sidebarLoading` back to `true`. Only the initial state ever set it, so every later cold load (a site change) left the previous site's tree on screen while its replacement was fetched, with no indication anything was happening. Setting it is also what gives the reveal a loaded edge to key off. Co-Authored-By: Claude Opus 5 --- ...ot-content-drive-sidebar.component.spec.ts | 82 ++++++++++++++++++- .../dot-content-drive-sidebar.component.ts | 60 +++++++++++++- .../features/sidebar/withSidebar.spec.ts | 14 +++- .../lib/store/features/sidebar/withSidebar.ts | 6 ++ 4 files changed, 156 insertions(+), 6 deletions(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts index b928dbcc227b..3f3f6bc2303d 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts @@ -15,7 +15,8 @@ import { DotFolderTreeNodeContentData, DotFolderTreeNodeItem, DotContentDriveMoveItems, - ALL_FOLDER + ALL_FOLDER, + LOAD_MORE_NODE_TYPE } from '@dotcms/portlets/content-drive/ui'; import { GlobalStore } from '@dotcms/store'; @@ -840,6 +841,81 @@ describe('DotContentDriveSidebarComponent', () => { }); }); + describe('revealing the folder the drive opened on', () => { + const targetNode: DotFolderTreeNodeItem = { + key: 'deep-folder', + label: '/documents/reports/', + data: { + id: 'deep-folder', + hostname: 'demo.dotcms.com', + path: '/documents/reports/', + type: 'folder' + }, + leaf: false + }; + + let scrollIntoView: jest.Mock; + + beforeEach(() => { + scrollIntoView = jest.fn(); + const treeFolder = spectator.query(DotTreeFolderComponent); + jest.spyOn(treeFolder.elementRef.nativeElement, 'querySelector').mockReturnValue({ + scrollIntoView + } as unknown as Element); + }); + + it('should bring the selected folder into view once the cold load finishes', () => { + contentDriveStore.selectedNode.mockReturnValue(targetNode); + + spectator.component.revealSelectedNodeOnLoad(false); + spectator.detectChanges(); + + expect(scrollIntoView).toHaveBeenCalledWith({ + // Instant: this is where the tree should have opened, not somewhere to animate to. + behavior: 'instant', + block: 'center' + }); + }); + + it('should not scroll while the tree is still loading', () => { + contentDriveStore.selectedNode.mockReturnValue(targetNode); + + spectator.component.revealSelectedNodeOnLoad(true); + spectator.detectChanges(); + + expect(scrollIntoView).not.toHaveBeenCalled(); + }); + + it('should wait for the tree to render rather than scrolling as the store publishes', () => { + contentDriveStore.selectedNode.mockReturnValue(targetNode); + + spectator.component.revealSelectedNodeOnLoad(false); + + // The loading placeholder is still mounted at this point, so there is no row yet. + expect(scrollIntoView).not.toHaveBeenCalled(); + }); + + it('should not scroll when the selection is a load-more sentinel', () => { + contentDriveStore.selectedNode.mockReturnValue({ + key: 'load-more:/documents/', + label: '', + data: { + type: LOAD_MORE_NODE_TYPE, + id: 'load-more:/documents/', + path: '/documents/', + hostname: 'demo.dotcms.com', + nextPage: 2, + remaining: 5 + } + } as DotFolderTreeNodeItem); + + spectator.component.revealSelectedNodeOnLoad(false); + spectator.detectChanges(); + + expect(scrollIntoView).not.toHaveBeenCalled(); + }); + }); + describe('handleSelectedNodeFromTable', () => { it('should handle selectedNode with fromTable flag', () => { const mockScrollIntoView = jest.fn(); @@ -886,7 +962,9 @@ describe('DotContentDriveSidebarComponent', () => { // The method calls it with just segments, which defaults to this.$folders() expect(recursiveExpandSpy).toHaveBeenCalledWith(['documents']); - // Verify scrollIntoView was called if element was found + // The reveal waits for the tree to render the row before scrolling to it. + spectator.detectChanges(); + expect(mockScrollIntoView).toHaveBeenCalledWith({ behavior: 'smooth', block: 'center' diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts index 51aa2b414011..5470db8b726a 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.ts @@ -1,10 +1,12 @@ import { signalMethod } from '@ngrx/signals'; import { + afterNextRender, ChangeDetectionStrategy, Component, effect, inject, + Injector, output, untracked, viewChild @@ -50,6 +52,7 @@ import { appendLoadMoreNodes, mergeFolderNodePage } from '../../utils/functions' }) export class DotContentDriveSidebarComponent { readonly #store = inject(DotContentDriveStore); + readonly #injector = inject(Injector); readonly $loading = this.#store.sidebarLoading; readonly $folders = this.#store.folders; @@ -90,14 +93,65 @@ export class DotContentDriveSidebarComponent { this.recursiveExpandOneNode(segments); - this.treeFolder() - ?.elementRef.nativeElement.querySelector(`[data-id="${data.id}"]`) - ?.scrollIntoView({ behavior: 'smooth', block: 'center' }); + this.#revealNode(selectedNode, 'smooth'); + }); + + /** + * Brings the folder the drive is open on into view once a cold load has rendered. + * + * The hierarchy load already expands the tree down to that folder, but a level can be hundreds + * of folders deep, so on a deep link it was drawn far below the fold with the viewport still at + * the top. Selecting a node in the tree must not scroll — it is under the cursor already — so + * this hangs off the load finishing rather than off the selection changing. + * + * Keyed off the load finishing rather than off the selection changing, because at the moment + * the store publishes a cold-loaded selection the tree is not on screen yet — the loading + * placeholder still is. Both reveals share {@link #revealSelectedNode}. + * + * @param {boolean} loading - The sidebar's loading state + */ + readonly revealSelectedNodeOnLoad = signalMethod((loading) => { + if (loading) { + return; + } + + // Instant, not smooth: this is where the tree should have opened, not a place to animate to. + this.#revealNode(this.$selectedNode(), 'instant'); }); constructor() { // Call signalMethod with the signal - it will automatically subscribe to changes this.handleSelectedNodeFromTable(this.$selectedNode); + this.revealSelectedNodeOnLoad(this.$loading); + } + + /** + * Scrolls a node's row into the middle of the tree's viewport, once the tree has actually + * rendered it. + * + * The wait matters for both callers. A cold load publishes its selection while the loading + * placeholder is still mounted, and the table's reveal runs straight after + * `recursiveExpandOneNode`, which only marks ancestors expanded — a branch whose children are + * still being fetched has no row to scroll to yet either. + * + * @param {DotFolderTreeNodeItem | undefined} node - The node to bring into view + * @param {ScrollBehavior} behavior - How to travel there + */ + #revealNode(node: DotFolderTreeNodeItem | undefined, behavior: ScrollBehavior): void { + const data = node?.data; + + if (!data || data.type === LOAD_MORE_NODE_TYPE) { + return; + } + + afterNextRender( + () => { + this.treeFolder() + ?.elementRef.nativeElement.querySelector(`[data-id="${data.id}"]`) + ?.scrollIntoView({ behavior, block: 'center' }); + }, + { injector: this.#injector } + ); } /** * Handles node selection events diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.spec.ts index 662cef9d338b..c4ab2be8edf9 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.spec.ts @@ -1,7 +1,7 @@ import { describe, it, expect } from '@jest/globals'; import { signalStore, withState } from '@ngrx/signals'; import { createServiceFactory, SpectatorService, mockProvider } from '@openng/spectator/jest'; -import { of } from 'rxjs'; +import { NEVER, of } from 'rxjs'; import { DotFolderService } from '@dotcms/data-access'; import { DotPagination, FolderSearchView } from '@dotcms/dotcms-models'; @@ -140,6 +140,18 @@ describe('withSidebar', () => { }, 0); }); + it('should flag loading while a reload is in flight', () => { + // Only the initial state used to set this, so a site change left the previous + // site's tree on screen with no indication anything was happening — and gave + // consumers no loaded edge to reveal the opened folder on. That it clears again is + // covered by the cases above. + folderService.searchFolders.mockReturnValue(NEVER); + + store.loadFolders(); + + expect(store.sidebarLoading()).toBe(true); + }); + it('should handle empty folder response', (done) => { folderService.searchFolders.mockReturnValue(searchResult([])); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts index d56a8fe9d15b..7d2e23b9cf8c 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/sidebar/withSidebar.ts @@ -63,6 +63,12 @@ export function withSidebar() { const urlFolderPath = store.path() || ''; + // Only the initial state used to set this, so every later cold load (a site + // change) left the previous site's tree on screen while its replacement was + // fetched, with no indication anything was happening. It also gives consumers the + // loaded edge they need to reveal the folder the drive opened on. + patchState(store, { sidebarLoading: true }); + getFolderHierarchyByPath(urlFolderPath, currentSite, dotFolderService) .pipe( take(1), From fd001fcb67f016438de2ad2ec13f352ad25642e3 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Thu, 13 Aug 2026 15:42:56 -0300 Subject: [PATCH 5/6] fix(content-drive): keep the tree standing when the pin request fails (#36595) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review on #37029. `resolveHierarchyAncestor` is a best-effort extra request on top of the page its level already has, but it ran inside the per-level pipe of a `forkJoin` with no `catchError`. A transient 500 or dropped connection on that one request rejected the entire hierarchy load, which `loadFolders` turns into `of([])` — collapsing every readable folder to just the root node. That made a failure to pin *more* destructive than being unable to see the folder at all: an inaccessible folder is filtered server-side and returns `200` with the row missing, which degrades to "unpinned" as intended. Only the new request could take the whole tree down with it, so this was a failure mode the change introduced. The `catchError` goes inside `resolveHierarchyAncestor` rather than at the call site, so its documented contract — resolves to `undefined` when the folder cannot be reached — is true for every caller. A request that fails is exactly "could not be reached". Also drops two comments describing the on-demand permission lookup that this branch removed. Both claimed the hierarchy load "cannot request permissions, so those nodes resolve them on demand on first right-click", which would send a maintainer looking for lazy-resolution code that no longer exists. Co-Authored-By: Claude Opus 5 --- .../portlet/src/lib/utils/functions.spec.ts | 21 ++++++++++++++++++ .../portlet/src/lib/utils/functions.ts | 22 ++++++++++++------- .../src/lib/utils/tree-folder.utils.ts | 4 ++-- .../ui/src/lib/shared/models.ts | 8 ++++--- 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts index af0779f4bec8..e4df61bbd53d 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts @@ -722,6 +722,27 @@ describe('Utility Functions', () => { }); }); + it('should leave the tree standing when the pin request itself fails', (done) => { + // The pin is a best-effort extra request inside a forkJoin. Letting a transient + // failure through would reject the whole hierarchy load, which loadFolders turns + // into an empty tree — costing every readable folder to save one pin. + mockDotFolderService.searchFolders.mockImplementation(({ path, name }) => { + if (path === '/' && name) { + return throwError(() => new Error('Service error')); + } + + return path === '/' ? page(['a-one'], '/', 253) : page([], path, 0); + }); + + getFolderHierarchyByPath('/zzz/', SITE, mockDotFolderService).subscribe({ + next: (levels) => { + expect(levels[0].folders.map(({ path }) => path)).toEqual(['/a-one/']); + done(); + }, + error: () => done(new Error('Should not have rejected the hierarchy load')) + }); + }); + it('should derive nextPage from folders fetched, not from the pinned node', (done) => { const fullPage = Array.from( { length: FOLDER_TREE_HIERARCHY_PAGE_SIZE }, diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts index 029245d76e1e..e681b1a63ab4 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts @@ -1,7 +1,7 @@ import { format } from 'date-fns'; import { forkJoin, Observable, of } from 'rxjs'; -import { map, switchMap } from 'rxjs/operators'; +import { catchError, map, switchMap } from 'rxjs/operators'; import { DotFolderService } from '@dotcms/data-access'; import { @@ -329,12 +329,17 @@ export function getPathLeafName(folderPath: string): string { * deprecated for removal, returns a path's *subfolders* rather than the folder itself, and carries * no permissions. * - * Resolves to `undefined` in two cases, both of which leave the folder unpinned. The `name` filter - * needs {@link FOLDER_NAME_FILTER_MIN_LENGTH} characters, so a one-character folder name drops the - * filter and falls back to the level's first page; and `name` is a case-insensitive *partial* match, - * so a level holding more same-substring siblings than fit one page can still exclude the target. - * Both need a level wide enough to have overflowed in the first place. An exact-match filter, or a - * folder-search that accepts an identifier, would close them. + * Resolves to `undefined` rather than failing, in three cases, all of which leave the folder + * unpinned. The `name` filter needs {@link FOLDER_NAME_FILTER_MIN_LENGTH} characters, so a + * one-character folder name drops the filter and falls back to the level's first page; `name` is a + * case-insensitive *partial* match, so a level holding more same-substring siblings than fit one + * page can still exclude the target (both need a level wide enough to have overflowed in the first + * place; an exact-match or identifier filter would close them); and the request itself can fail. + * + * Swallowing that failure is the point. This is a best-effort extra request on top of the page the + * level already has, and its caller runs inside a `forkJoin`: letting a transient 500 through would + * reject the whole hierarchy load, which `loadFolders` turns into an empty tree. A folder that + * cannot be pinned must cost that folder's pin, not every readable folder on screen. * * @param {string} levelPath - Parent path being listed, e.g. `/a/` * @param {string} ancestorPath - Full path of the folder to resolve, e.g. `/a/b/` @@ -367,7 +372,8 @@ export function resolveHierarchyAncestor( folders .map((view) => folderSearchViewToDotFolder(view, site.hostname)) .find((folder) => folder.path === ancestorPath) - ) + ), + catchError(() => of(undefined)) ); } diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/tree-folder.utils.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/tree-folder.utils.ts index 5683e89ef7b2..84b217919756 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/tree-folder.utils.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/tree-folder.utils.ts @@ -43,8 +43,8 @@ export const createTreeNode = ( type: 'folder', defaultBaseType: folder.defaultBaseType, // Carried so a right-click can gate the shared context menu and pre-populate the - // "Edit folder" dialog without refetching. `permissions` stays undefined for nodes - // built by the deep-link hierarchy load, which cannot request them. + // "Edit folder" dialog without refetching. Every folder-search call behind this one + // asks for `permissions`, so a node arrives ready to gate however it reached the tree. name: folder.name, title: folder.title, sortOrder: folder.sortOrder, diff --git a/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts b/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts index a307a5d1139f..bb5b57e5edfc 100644 --- a/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts +++ b/core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts @@ -101,9 +101,11 @@ export type DotFolderTreeNodeContentData = TreeNodeContentData & { defaultFileType?: string; showOnMenu?: boolean; /** - * Permission types the user holds on this folder. `undefined` means "not resolved yet" — the - * deep-link hierarchy load cannot request permissions, so those nodes resolve them on demand - * on first right-click. An empty array is a final answer: the user holds none. + * Permission types the user holds on this folder. Every source that builds a folder node + * requests them — expand, load-more and the deep-link hierarchy load alike — so in practice + * this is populated and an empty array means the user holds none. Optional only because a + * folder can still arrive without it from a source that did not opt in, in which case gating + * degrades to "no actions" rather than throwing. */ permissions?: PermissionType[]; }; From e840269a2021d01b4cfe6af6709db459dc600e59 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Thu, 13 Aug 2026 15:54:27 -0300 Subject: [PATCH 6/6] test(content-drive): cover the sidebar actually using mergeFolderNodePage (#36595) `mergeFolderNodePage` was unit tested, but nothing asserted that `onLoadMore` calls it. Removing it from both call sites and falling back to a plain concat left all 1242 tests green, so the duplicate-render it exists to prevent could have been reintroduced silently. Covers both branches, which are separate code paths: a nested level merging `parent.children`, and the root level merging the top-level array around the synthetic "All folders" node. Both assert the folder renders once, and the nested case also asserts the on-screen node object is the one kept, so an open branch does not collapse when paging reaches it. Co-Authored-By: Claude Opus 5 --- ...ot-content-drive-sidebar.component.spec.ts | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts index 3f3f6bc2303d..51c8153e2c62 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts @@ -573,6 +573,138 @@ describe('DotContentDriveSidebarComponent', () => { expect(contentDriveStore.updateFolders).toHaveBeenCalled(); }); + it('should render a pinned folder once when paging reaches its real position', () => { + // The hierarchy pins a deep-linked folder to the top of its level, out of sort + // order. Page far enough and the same folder comes back in its proper place. + const pinned: DotFolderTreeNodeItem = { + key: 'z', + label: '/big/z/', + data: { id: 'z', hostname: 'demo.dotcms.com', path: '/big/z/', type: 'folder' }, + leaf: false, + expanded: true, + children: [ + { + key: 'inner', + label: '/big/z/inner/', + data: { + id: 'inner', + hostname: 'demo.dotcms.com', + path: '/big/z/inner/', + type: 'folder' + }, + leaf: true + } + ] + }; + const loadMoreNode: DotFolderTreeNodeItem = { + key: 'load-more:/big/', + label: 'content-drive.tree.load-more', + data: { + type: 'load-more', + path: '/big/', + hostname: 'demo.dotcms.com', + id: 'load-more:/big/', + nextPage: 2, + remaining: 1 + }, + leaf: true, + selectable: false + }; + const parent: DotFolderTreeNodeItem = { + key: 'big-folder', + label: '/big/', + data: { + id: 'big-folder', + hostname: 'demo.dotcms.com', + path: '/big/', + type: 'folder' + }, + leaf: false, + expanded: true, + children: [pinned, loadMoreNode] + }; + contentDriveStore.folders.mockReturnValue([parent]); + + contentDriveStore.loadChildFolders.mockReturnValue( + of({ + folders: [ + { + key: 'z', + label: '/big/z/', + data: { + id: 'z', + hostname: 'demo.dotcms.com', + path: '/big/z/', + type: 'folder' + }, + leaf: false + } + ], + totalEntries: 1 + }) + ); + + spectator.triggerEventHandler(DotTreeFolderComponent, 'loadMore', loadMoreNode); + + expect(parent.children?.map((child) => child.key)).toEqual(['z']); + // The on-screen node is kept, not the bare copy from the page, so the branch the + // user already has open does not collapse under them. + expect(parent.children?.[0]).toBe(pinned); + expect(parent.children?.[0].children).toHaveLength(1); + }); + + it('should render a pinned root folder once when paging reaches its real position', () => { + const pinned: DotFolderTreeNodeItem = { + key: 'z', + label: '/z/', + data: { id: 'z', hostname: 'demo.dotcms.com', path: '/z/', type: 'folder' }, + leaf: false + }; + const loadMoreNode: DotFolderTreeNodeItem = { + key: 'load-more:/', + label: 'content-drive.tree.load-more', + data: { + type: 'load-more', + path: '/', + hostname: 'demo.dotcms.com', + id: 'load-more:/', + nextPage: 2, + remaining: 1 + }, + leaf: true, + selectable: false + }; + contentDriveStore.folders.mockReturnValue([realAllFolder, pinned, loadMoreNode]); + + contentDriveStore.loadChildFolders.mockReturnValue( + of({ + folders: [ + { + key: 'z', + label: '/z/', + data: { + id: 'z', + hostname: 'demo.dotcms.com', + path: '/z/', + type: 'folder' + }, + leaf: false + } + ], + totalEntries: 1 + }) + ); + + spectator.triggerEventHandler(DotTreeFolderComponent, 'loadMore', loadMoreNode); + + // Root folders are siblings of "All folders", so this branch merges the top-level + // array rather than a parent's children. + expect(contentDriveStore.updateFolders).toHaveBeenCalledWith([ + realAllFolder, + pinned + ]); + }); + it('should keep a refreshed "Load more" node when more pages still remain', () => { const loadMoreNode: DotFolderTreeNodeItem = { key: 'load-more:/big/',