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..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 @@ -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'; @@ -304,7 +304,7 @@ describe('DotContentDriveDialogFolderComponent', () => { modDate: 1234567890, owner: null, parent: '', - path: '', + path: '/documents/existing-folder/', permissions: [], type: 'folder' }; @@ -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(); @@ -1001,7 +1029,7 @@ describe('DotContentDriveDialogFolderComponent', () => { modDate: 1234567890, owner: null, parent: '', - path: '', + path: '/documents/original-folder/', permissions: [], type: 'folder' }; @@ -1036,6 +1064,63 @@ 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 +1142,7 @@ describe('DotContentDriveDialogFolderComponent', () => { modDate: 1234567890, owner: null, parent: '', - path: '', + path: '/documents/original-folder/', permissions: [], type: 'folder' }; @@ -1122,7 +1207,7 @@ describe('DotContentDriveDialogFolderComponent', () => { modDate: 1234567890, owner: null, parent: '', - path: '', + path: '/documents/original-folder/', permissions: [], type: 'folder' }; @@ -1177,7 +1262,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..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 @@ -8,13 +8,15 @@ 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, + DotFolderTreeNodeContentData, DotFolderTreeNodeItem, DotContentDriveMoveItems, - ALL_FOLDER + ALL_FOLDER, + LOAD_MORE_NODE_TYPE } from '@dotcms/portlets/content-drive/ui'; import { GlobalStore } from '@dotcms/store'; @@ -124,6 +126,7 @@ describe('DotContentDriveSidebarComponent', () => { sidebarLoading: jest.fn().mockReturnValue(false), loadFolders: jest.fn(), loadChildFolders: jest.fn(), + patchContextMenu: jest.fn(), updateFolders: jest.fn(), setSelectedNode: jest.fn() }), @@ -570,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/', @@ -838,6 +973,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(); @@ -884,7 +1094,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' @@ -1151,4 +1363,89 @@ 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(); + }); + + 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 = (data: DotFolderTreeNodeContentData) => { + const event = new MouseEvent('contextmenu'); + 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( + buildFolderData([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 treat an empty permissions array as a resolved "no grants"', () => { + rightClick(buildFolderData([])); + + expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith( + expect.objectContaining({ + contentlet: expect.objectContaining({ permissions: [] }) + }) + ); + }); + + 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.patchContextMenu).toHaveBeenCalledWith( + expect.objectContaining({ + contentlet: expect.objectContaining({ permissions: [] }) + }) + ); + }); + + it('should anchor the menu on the originating event', () => { + const event = rightClick(buildFolderData([PERMISSIONS_TYPE.EDIT])); + + expect(contentDriveStore.patchContextMenu).toHaveBeenCalledWith( + expect.objectContaining({ triggeredEvent: event }) + ); + }); + }); }); 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..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 @@ -16,18 +18,20 @@ 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 } 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 * @@ -48,6 +52,7 @@ import { appendLoadMoreNodes } from '../../utils/functions'; }) export class DotContentDriveSidebarComponent { readonly #store = inject(DotContentDriveStore); + readonly #injector = inject(Injector); readonly $loading = this.#store.sidebarLoading; readonly $folders = this.#store.folders; @@ -88,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 @@ -169,7 +225,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, @@ -194,7 +252,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, @@ -232,6 +292,47 @@ 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. + * + * 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 clicked folder + */ + protected onNodeRightClick({ event, data }: DotContentDriveTreeRightClick): void { + this.#openContextMenu(event, data); + } + + /** + * 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..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,12 +28,24 @@ 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}. + * 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_TREE_HIERARCHY_PAGE_SIZE = 10000; +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; export const DEFAULT_SORT = { field: 'modDate', 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.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 a7e0187b6765..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), @@ -121,6 +127,7 @@ export function withSidebar() { page ); }, + /** * 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..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 @@ -8,7 +8,8 @@ import { DotContentDriveItem, DotPagination, FolderSearchView, - isTreeNodeContentData + isTreeNodeContentData, + PERMISSIONS_TYPE } from '@dotcms/dotcms-models'; import { createFakeCheckboxField, @@ -30,6 +31,7 @@ import { folderSearchViewToDotFolder, getFolderHierarchyByPath, getFolderNodesByPath, + getPathLeafName, getUserSearchableActive, isBinaryCheckboxField, isDateFieldFilterType, @@ -37,7 +39,9 @@ import { isMultiValueFieldFilterType, parseUserSearchableValue, parseWorkflowFilter, + mergeFolderNodePage, parseWorkflowToken, + resolveHierarchyAncestor, serializeUserSearchableValue, toLocalIsoString, workflowEntryToToken @@ -405,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); @@ -433,18 +450,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 +469,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 +487,18 @@ describe('Utility Functions', () => { }); }); + it('should request permissions so first-paint nodes can gate their context menu', (done) => { + getFolderHierarchyByPath('/main', SITE, mockDotFolderService).subscribe({ + next: () => { + expect(mockDotFolderService.searchFolders).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) => { @@ -593,6 +630,147 @@ 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 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 }, + (_, 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', () => { @@ -643,16 +821,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 +856,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 +880,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' })]) @@ -794,7 +996,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', @@ -817,7 +1019,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' @@ -828,7 +1033,7 @@ describe('Utility Functions', () => { expect(loadMore.data).toEqual( expect.objectContaining({ type: 'load-more', - nextPage: 2, + nextPage: 6, remaining: 49 }) ); @@ -1219,4 +1424,239 @@ 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('getPathLeafName', () => { + it.each([ + ['/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('resolveHierarchyAncestor', () => { + 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 level with permissions, narrowed by the folder own name', (done) => { + resolveHierarchyAncestor( + '/main/', + '/main/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', + per_page: FOLDER_TREE_HIERARCHY_PAGE_SIZE, + includePermissions: true + }) + ); + done(); + }, + error: done + }); + }); + + it('should omit the name filter when the folder name is too short for the endpoint', (done) => { + resolveHierarchyAncestor( + '/main/', + '/main/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 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] + }) + ]) + ); + + resolveHierarchyAncestor( + '/main/', + '/main/docs/', + createFakeSite({ identifier: 'site-1', hostname: 'demo.dotcms.com' }), + mockDotFolderService as unknown as DotFolderService + ).subscribe({ + next: (folder) => { + expect(folder?.id).toBe('docs-id'); + expect(folder?.permissions).toEqual([PERMISSIONS_TYPE.READ, PERMISSIONS_TYPE.EDIT]); + done(); + }, + error: done + }); + }); + + it('should issue a single request and not page the level', (done) => { + mockDotFolderService.searchFolders.mockReturnValue( + of({ + folders: [createFakeFolderSearchView({ path: '/main/', name: 'someone-else' })], + pagination: { totalEntries: 5000 } as DotPagination + }) + ); + + resolveHierarchyAncestor( + '/main/', + '/main/docs/', + createFakeSite({ identifier: 'site-1', hostname: 'demo.dotcms.com' }), + mockDotFolderService as unknown as DotFolderService + ).subscribe({ + 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 eacf47db9f8e..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,15 +1,15 @@ import { format } from 'date-fns'; -import { forkJoin, Observable } from 'rxjs'; +import { forkJoin, Observable, of } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { catchError, map, switchMap } from 'rxjs/operators'; import { DotFolderService } from '@dotcms/data-access'; import { createLoadMoreTreeNode, DotCMSContentTypeField, DotContentDriveDateRange, - DotContentDriveFolder, - DotContentDriveItem, + DotContentDriveActionableFolder, + DotContentDriveActionableItem, DotContentDriveUserSearchableValue, DotFolder, DotSite, @@ -26,6 +26,7 @@ import { FIELD_FILTER_DATE_TYPES, FIELD_FILTER_KEY_VALUE_TYPE, FIELD_FILTER_MULTI_VALUE_TYPES, + FOLDER_NAME_FILTER_MIN_LENGTH, FOLDER_TREE_HIERARCHY_PAGE_SIZE, FOLDER_TREE_PAGE_SIZE, USER_SEARCHABLE_PREFIX, @@ -266,7 +267,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 }; } @@ -278,18 +290,114 @@ 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` 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/` + * @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) + ), + catchError(() => of(undefined)) + ); +} + /** * 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) @@ -304,7 +412,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, @@ -313,7 +426,8 @@ export function getFolderHierarchyByPath( orderby: 'name', direction: 'ASC', page: 1, - per_page: FOLDER_TREE_HIERARCHY_PAGE_SIZE + per_page: FOLDER_TREE_HIERARCHY_PAGE_SIZE, + includePermissions: true }) .pipe( map(({ folders, pagination }) => ({ @@ -321,8 +435,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 + ) + ); + }) ) ); @@ -355,7 +491,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 }) => ({ @@ -395,6 +535,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. */ @@ -419,8 +605,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[], @@ -431,14 +618,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++) { @@ -454,7 +639,7 @@ export function applyLoadMoreToHierarchy( level.totalEntries, level.path, hostname, - nextPageAfterHierarchy + level.nextPage ); } @@ -485,10 +670,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..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 @@ -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. 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, + 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..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,6 +21,7 @@ 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"> {{ 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..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,18 @@ -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 { 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 class DragEventMock extends Event { @@ -821,4 +823,141 @@ describe('DotTreeFolderComponent', () => { }); }); }); + + describe('right-click', () => { + const ALL_FOLDER_ID = 'site-1'; + + const folderNode: DotFolderTreeNodeItem = { + key: 'folder-1', + label: '/application/content/', + data: { + id: 'folder-1', + hostname: 'demo.dotcms.com', + path: '/application/content/', + type: 'folder' + } + }; + + 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'); + target.dispatchEvent(event); + + return event; + }; + + let emitted: jest.Mock; + + beforeEach(() => { + emitted = jest.fn(); + component.rightClick.subscribe(emitted); + + 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, + data: expect.objectContaining({ id: 'folder-1' }) + }); + }); + + it('should suppress the native browser menu for a folder node', () => { + const event = rightClickOn(rowFor('folder-1')); + + expect(event.preventDefault).toHaveBeenCalled(); + }); + + 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')); + + 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 rather than opening the parent menu', () => { + const loadMore = spectator.query(byTestId('tree-load-more')); + + 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 9119dcb4659c..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,6 +13,7 @@ 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 } from '../shared/constants'; @@ -20,7 +21,8 @@ import { DotFolderTreeNodeData, DotFolderTreeNodeItem, DotContentDriveUploadFiles, - DotContentDriveMoveItems + DotContentDriveMoveItems, + DotContentDriveTreeRightClick } from '../shared/models'; /** @@ -48,6 +50,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 +68,54 @@ 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. + * + * 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 + */ + @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 (!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, data }); + } + /** * @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..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 @@ -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,26 @@ 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. 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[]; }; /** @@ -99,3 +123,18 @@ export type DotFolderTreeNodeData = DotFolderTreeNodeContentData | TreeNodeLoadM * @description Tree node item */ export type DotFolderTreeNodeItem = TreeNode; + +/** + * @export + * @interface DotContentDriveTreeRightClick + * @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; + data: DotFolderTreeNodeContentData; +} 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 }; }