diff --git a/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.spec.ts b/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.spec.ts index c93d72379418..43dfb45418d7 100644 --- a/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.spec.ts +++ b/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.spec.ts @@ -2,7 +2,11 @@ import { createHttpFactory, HttpMethod, SpectatorHttp } from '@openng/spectator/ import { HttpHeaders } from '@angular/common/http'; -import { DotActionBulkRequestOptions, DotActionBulkResult } from '@dotcms/dotcms-models'; +import { + DotActionBulkRequestOptions, + DotActionBulkResult, + DotFireDefaultActionResult +} from '@dotcms/dotcms-models'; import { dotcmsContentletMock } from '@dotcms/utils-testing'; import { DotWorkflowActionsFireService } from './dot-workflow-actions-fire.service'; @@ -346,6 +350,31 @@ describe('DotWorkflowActionsFireService', () => { }); }); + it('should fire a default system action over multiple inodes and return its summary', (done) => { + // The endpoint streams one entry per contentlet plus a summary. The summary is the only + // honest source of success/fail counts: individual items can fail while the request is 200. + const mockResult: DotFireDefaultActionResult = { + results: [], + summary: { affected: 2, successCount: 1, failCount: 1, time: 12 } + }; + + spectator.service + .fireDefaultAction({ action: 'UNLOCK', inodes: ['1', '2'] }) + .subscribe((res) => { + expect(res).toEqual(mockResult); + done(); + }); + + const req = spectator.expectOne( + '/api/v1/workflow/actions/default/fire/UNLOCK?indexPolicy=WAIT_FOR', + HttpMethod.POST + ); + + expect(req.request.body).toEqual({ contentlet: [{ inode: '1' }, { inode: '2' }] }); + + req.flush({ entity: mockResult }); + }); + afterEach(() => { spectator.controller.verify(); }); diff --git a/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.ts b/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.ts index deae5161f80e..f97839c27edf 100644 --- a/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.ts +++ b/core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.ts @@ -8,7 +8,8 @@ import { map, take } from 'rxjs/operators'; import { DotActionBulkRequestOptions, DotCMSContentlet, - DotActionBulkResult + DotActionBulkResult, + DotFireDefaultActionResult } from '@dotcms/dotcms-models'; export interface DotActionRequestOptions { @@ -81,13 +82,20 @@ export class DotWorkflowActionsFireService { } /** - * Fire a default workflow action over one or multiple contentlets + * Fire a default workflow action over one or multiple contentlets. + * + * Resolves to the endpoint's `{ results, summary }` entity — **not** a contentlet list, which is + * what this used to claim. Individual contentlets can fail while the request itself succeeds + * with a 200, so callers reporting an outcome must read `summary.successCount` / + * `summary.failCount` rather than assume every inode they sent was acted on. * * @param {DotFireDefaultActionOptions} options - * @return {*} {Observable} + * @return {*} {Observable} * @memberof DotWorkflowActionsFireService */ - fireDefaultAction(options: DotFireDefaultActionOptions): Observable { + fireDefaultAction( + options: DotFireDefaultActionOptions + ): Observable { const { action, inodes } = options; const url = `${this.BASE_URL}/actions/default/fire/${action}`; const urlParams = new HttpParams().set('indexPolicy', 'WAIT_FOR'); @@ -96,7 +104,7 @@ export class DotWorkflowActionsFireService { }; return this.httpClient - .post<{ entity: DotCMSContentlet[] }>(url, body, { + .post<{ entity: DotFireDefaultActionResult }>(url, body, { headers: this.defaultHeaders, params: urlParams }) diff --git a/core-web/libs/dotcms-models/src/lib/dot-action-bulk-result.model.ts b/core-web/libs/dotcms-models/src/lib/dot-action-bulk-result.model.ts index 2881d4cc6d05..27cf21606dda 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-action-bulk-result.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-action-bulk-result.model.ts @@ -11,6 +11,28 @@ export interface DotActionBulkResult { action?: string; } +/** + * Response of `POST /api/v1/workflow/actions/default/fire/{systemAction}`, the multi-contentlet + * system-action endpoint. + * + * Streamed rather than assembled, which is why the shape differs from {@link DotActionBulkResult}: + * one entry per contentlet keyed by its id, then a summary. A per-item failure does **not** fail the + * request — the status stays 200 and the item is counted in {@link summary.failCount} — so the + * summary is the only honest source of what actually happened. + */ +export interface DotFireDefaultActionResult { + /** One entry per contentlet, keyed by identifier. A failed item holds an error payload. */ + results: Record[]; + summary: { + /** Number of contentlets the request was asked to act on. */ + affected: number; + successCount: number; + failCount: number; + /** Server-side duration in ms. */ + time: number; + }; +} + // optional attrs because api is not consistent export interface DotBulkFailItem { errorMessage: string; diff --git a/core-web/libs/dotcms-models/src/lib/dot-contentlet.model.ts b/core-web/libs/dotcms-models/src/lib/dot-contentlet.model.ts index 69b8a993aefc..2f3b3ce1041f 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-contentlet.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-contentlet.model.ts @@ -11,6 +11,15 @@ export interface DotCMSContentlet { binaryContentAsset?: string; binaryVersion?: string; contentType: string; + /** + * Whether the current user may edit the contentlet *right now* — stamped by the browser/drive + * search, not by the contentlet itself. + * + * True only when the caller has WRITE permission **and** holds the lock, so on a `locked` row a + * `false` means the lock belongs to somebody else. That makes it the cheapest available answer + * to "can I unlock this?", short of a per-item `_canlock` call. + */ + contentEditable?: boolean; file?: string; folder: string; hasLiveVersion?: boolean; diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.html b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.html index fab326ba6b1c..4458955cbb14 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.html +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.html @@ -31,7 +31,11 @@ class="flex min-h-0 flex-1 flex-col gap-8 overflow-y-auto px-6 py-4" data-testid="action-center"> @if ($ignoredFolderCount() > 0) { - + + {{ 'content-drive.action-center.folders-ignored' | dm: [$ignoredFolderCount().toString()] @@ -51,7 +55,8 @@

+ (click)="onSelectQuickAction(quickAction)"> + + @if (quickAction.warningCount > 0) { + + } + ({{ quickAction.count }}) @@ -281,7 +303,9 @@

- @if ($selectedAction(); as action) { + + @if (!$pendingQuickAction() && $selectedAction(); as action) { + @if ($actionExecution()) { +
+ + +
+ } diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/dot-content-drive-toolbar.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/dot-content-drive-toolbar.component.spec.ts index 5d8144a7bcb7..2d8e8fdc3fa7 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/dot-content-drive-toolbar.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/dot-content-drive-toolbar.component.spec.ts @@ -30,6 +30,7 @@ import { DotContentDriveToolbarComponent } from './dot-content-drive-toolbar.com import { DIALOG_TYPE } from '../../shared/constants'; import { MOCK_BASE_TYPES, MOCK_CONTENT_TYPES, MOCK_ITEMS } from '../../shared/mocks'; +import { DotContentDriveActionExecution } from '../../shared/models'; import { DotContentDriveNavigationService } from '../../shared/services'; import { DotContentDriveStore } from '../../store/dot-content-drive.store'; @@ -57,6 +58,7 @@ describe('DotContentDriveToolbarComponent', () => { const selectedNodeSignal = signal<{ data?: { defaultBaseType?: string | null } } | undefined>( undefined ); + const actionExecutionSignal = signal(undefined); const createComponent = createComponentFactory({ component: DotContentDriveToolbarComponent, @@ -80,7 +82,8 @@ describe('DotContentDriveToolbarComponent', () => { userSearchableActive: signal([]), setUserSearchableFields: jest.fn(), addUserSearchableField: jest.fn(), - clearUserSearchableFilters: jest.fn() + clearUserSearchableFilters: jest.fn(), + actionExecution: actionExecutionSignal }), mockProvider(DotContentTypeService, { getContentTypes: jest.fn().mockReturnValue(of(MOCK_CONTENT_TYPES)), @@ -140,6 +143,7 @@ describe('DotContentDriveToolbarComponent', () => { filtersSignal.set({}); selectedItemsSignal.set([]); selectedNodeSignal.set(undefined); + actionExecutionSignal.set(undefined); }); it('should render toolbar container', () => { @@ -468,6 +472,38 @@ describe('DotContentDriveToolbarComponent', () => { }); }); + describe('running-action indicator', () => { + it('should stay hidden when nothing is running', () => { + spectator.detectChanges(); + + expect(spectator.query(byTestId('action-execution-indicator'))).toBeNull(); + }); + + it('should report the action and the number of items once a run starts', () => { + // The toolbar is the only place still reporting the run after the Action Center dialog is + // closed, which is the whole reason the indicator lives out here. + actionExecutionSignal.set({ actionName: 'Publish', total: 3 }); + spectator.detectChanges(); + + const indicator = spectator.query(byTestId('action-execution-indicator')); + + expect(indicator).toBeTruthy(); + expect(spectator.component.$actionExecutionLabel()).toBe( + 'content-drive.action-center.applying' + ); + }); + + it('should disappear again once the run settles', () => { + actionExecutionSignal.set({ actionName: 'Publish', total: 3 }); + spectator.detectChanges(); + + actionExecutionSignal.set(undefined); + spectator.detectChanges(); + + expect(spectator.query(byTestId('action-execution-indicator'))).toBeNull(); + }); + }); + describe('field-filter chips', () => { it('should render a chip only for active variables resolved against loaded fields', () => { store.userSearchableFields.set([ diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/dot-content-drive-toolbar.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/dot-content-drive-toolbar.component.ts index c05aa8dd19a8..897c5e425124 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/dot-content-drive-toolbar.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-toolbar/dot-content-drive-toolbar.component.ts @@ -246,6 +246,29 @@ export class DotContentDriveToolbarComponent { readonly $showWorkflowActions = computed(() => !!this.#store.selectedItems().length); readonly $hasFilters = computed(() => Object.keys(this.#store.filters()).length > 0); + /** + * The action currently being applied, surfaced here because the run outlives the Action Center + * dialog. Once the user closes that dialog the toolbar is the only place still reporting the run, + * so without this the work would continue with no indication until the completion toast fired. + */ + readonly $actionExecution = this.#store.actionExecution; + + /** + * Resolved indicator label. Built here rather than in the template because `DotMessagePipe` takes + * `string[]` arguments and the item count is a number. + */ + readonly $actionExecutionLabel = computed(() => { + const execution = this.$actionExecution(); + + return execution + ? this.#dotMessageService.get( + 'content-drive.action-center.applying', + execution.actionName, + String(execution.total) + ) + : ''; + }); + /** * Active field-filter chips, in the order the user added them (the store keeps `userSearchableActive` * in add order). Each variable is resolved to its field metadata, so chips render only once the 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.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-folder-list-context-menu/dot-folder-list-context-menu.component.spec.ts index 2c9d134268ae..2ce3673bef10 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-folder-list-context-menu/dot-folder-list-context-menu.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-folder-list-context-menu/dot-folder-list-context-menu.component.spec.ts @@ -15,6 +15,7 @@ import { DotContentletService, DotCurrentUserService, DotFolderService, + DotHttpErrorManagerService, DotMessageService, DotRenderMode, DotSiteService, @@ -94,6 +95,8 @@ describe('DotFolderListViewContextMenuComponent', () => { mockProvider(DotWorkflowActionsFireService, { fireTo: jest.fn().mockReturnValue(of({})) }), + // Required by the store's `withActionExecution`, which routes fire failures through it. + mockProvider(DotHttpErrorManagerService), mockProvider(DotWorkflowEventHandlerService, { open: jest.fn() }), diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.spec.ts index 0854fa79a25f..677c34ed2dae 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.spec.ts @@ -79,6 +79,7 @@ import { } from '../shared/mocks'; import { DotContentDriveDialog, + DotContentDriveActionExecutionResult, DotContentDriveDialogDrillDown, DotContentDriveSortOrder, DotContentDriveStatus @@ -104,6 +105,10 @@ describe('DotContentDriveShellComponent', () => { let dialogSignal: WritableSignal; // Header override published by a dialog body that has drilled into a sub-screen. let dialogDrillDownSignal: WritableSignal; + // Result of a finished workflow action, which the shell turns into a toast. + let actionExecutionResultSignal: WritableSignal< + DotContentDriveActionExecutionResult | undefined + >; // Reactive so the shell's $extraColumns computed recomputes when the fields change. let showInListFieldsSignal: WritableSignal; @@ -176,6 +181,9 @@ describe('DotContentDriveShellComponent', () => { statusSignal = signal(DotContentDriveStatus.LOADING); dialogSignal = signal(undefined); dialogDrillDownSignal = signal(undefined); + actionExecutionResultSignal = signal( + undefined + ); showInListFieldsSignal = signal([]); editPanelRequestSignal.set(null); @@ -211,6 +219,10 @@ describe('DotContentDriveShellComponent', () => { contextMenu: jest.fn().mockReturnValue(null), dialog: dialogSignal, dialogDrillDown: dialogDrillDownSignal, + // Read by the toolbar, which the shell renders for real. + actionExecution: signal(undefined), + actionExecutionResult: actionExecutionResultSignal, + clearActionExecutionResult: jest.fn(), setDialog: jest.fn(), setDialogDrillDown: jest.fn(), clearDialogDrillDown: jest.fn(), @@ -309,6 +321,84 @@ describe('DotContentDriveShellComponent', () => { jest.clearAllMocks(); }); + describe('workflow action result', () => { + // The run outlives the Action Center dialog, so the shell is what reports the outcome — it + // owns and is never destroyed while the portlet is open. + const settle = (result: DotContentDriveActionExecutionResult) => { + actionExecutionResultSignal.set(result); + spectator.detectChanges(); + }; + + it('should report a plain success when nothing failed or skipped', () => { + settle({ + actionName: 'Publish', + successCount: 3, + skippedCount: 0, + failCount: 0 + }); + + expect(messageService.add).toHaveBeenCalledWith( + expect.objectContaining({ + severity: 'success', + detail: 'content-drive.action-center.toast.executed-detail' + }) + ); + }); + + it('should downgrade to a warning when items failed', () => { + // Partial failure is a normal outcome (a lock held by somebody else, a per-contentlet + // permission) and must not read as an unqualified success. + settle({ + actionName: 'Publish', + successCount: 1, + skippedCount: 0, + failCount: 1 + }); + + expect(messageService.add).toHaveBeenCalledWith( + expect.objectContaining({ + severity: 'warn', + detail: 'content-drive.action-center.toast.executed-with-fails' + }) + ); + }); + + it('should surface skipped items when nothing failed', () => { + settle({ + actionName: 'Send for Review', + successCount: 1, + skippedCount: 1, + failCount: 0 + }); + + expect(messageService.add).toHaveBeenCalledWith( + expect.objectContaining({ + severity: 'success', + detail: 'content-drive.action-center.toast.executed-with-skips' + }) + ); + }); + + it('should refresh the grid, close the dialog and consume the result', () => { + settle({ + actionName: 'Publish', + successCount: 1, + skippedCount: 0, + failCount: 0 + }); + + expect(store.loadItems).toHaveBeenCalled(); + expect(store.closeDialog).toHaveBeenCalled(); + expect(store.clearActionExecutionResult).toHaveBeenCalled(); + }); + + it('should stay silent while no result is published', () => { + spectator.detectChanges(); + + expect(messageService.add).not.toHaveBeenCalled(); + }); + }); + describe('Query Params Update Effect', () => { it('should update query params when store changes', () => { // Arrange store values for this run 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 06132361fbef..b819be69d8b0 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 @@ -449,6 +449,73 @@ export class DotContentDriveShellComponent { : limit * (currentPage - 1) + items.length; }); + /** + * Reports a finished workflow action as a toast, refreshes the grid, and closes the dialog if it + * is still open. + * + * Lives in the shell rather than in the Action Center because the run outlives that dialog: the + * user may close it mid-flight and the result still has to be reported. The shell owns + * `` and is never destroyed while the portlet is open, so it is the only place that can + * present a result whose originating dialog may already be gone. It also keeps the store data-only. + * + * The reload lands here for the same reason, plus a mechanical one: `loadItems` belongs to the + * base store's `withMethods`, which `withActionExecution` cannot reach from inside the + * composition. `loadItems` clears the selection and sets `LOADING` itself, so this one call is the + * whole post-run refresh. + * + * `failCount` downgrades the toast to a warning. Partial failure is a normal outcome for these + * endpoints (a lock held by somebody else, a per-contentlet permission), and reporting it as an + * unqualified success would be the one thing the user cannot recover from — the grid has already + * reloaded and the selection is gone. + */ + readonly actionExecutionResultEffect = effect(() => { + const result = this.#store.actionExecutionResult(); + + if (!result) { + return; + } + + const { actionName, successCount, skippedCount, failCount } = result; + + const detail = + failCount > 0 + ? this.#dotMessageService.get( + 'content-drive.action-center.toast.executed-with-fails', + actionName, + String(successCount), + String(failCount) + ) + : skippedCount > 0 + ? this.#dotMessageService.get( + 'content-drive.action-center.toast.executed-with-skips', + actionName, + String(successCount), + String(skippedCount) + ) + : this.#dotMessageService.get( + 'content-drive.action-center.toast.executed-detail', + actionName, + String(successCount) + ); + + this.#messageService.add({ + severity: failCount > 0 ? 'warn' : 'success', + summary: this.#dotMessageService.get('content-drive.action-center.toast.executed'), + detail, + life: failCount > 0 ? WARNING_MESSAGE_LIFE : SUCCESS_MESSAGE_LIFE + }); + + untracked(() => { + // Contentlets have moved step, so the grid is stale; `loadItems` also drops the selection + // the run consumed. + this.#store.loadItems(); + // A no-op when the user already closed the dialog, which is the common path now that + // firing hands off to the toolbar. + this.#store.closeDialog(); + this.#store.clearActionExecutionResult(); + }); + }); + readonly updateQueryParamsEffect = effect(() => { const isTreeExpanded = this.#store.isTreeExpanded(); const path = this.#store.path(); 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 7756b1688fc1..52ec0b8916ac 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 @@ -111,6 +111,33 @@ export interface DotContentDriveDialog { | DotContentDriveUploadSelectorPayload; } +/** + * A workflow action currently being applied to the selection. + * + * Held in the store rather than in the Action Center dialog so it survives the dialog being closed: + * the run continues, the toolbar keeps reporting it, and reopening the dialog sees a run already in + * progress instead of offering to fire it again. + */ +export interface DotContentDriveActionExecution { + /** Already-resolved action label, not an i18n key — it goes straight into the indicator. */ + actionName: string; + /** Number of contentlets the run was fired over. */ + total: number; +} + +/** + * Outcome of a finished run, published for the shell to present as a toast. + * + * Counts come from the response, never from the number of items submitted: both endpoints answer 200 + * with per-item failures inside, so an item locked by another user would otherwise read as a success. + */ +export interface DotContentDriveActionExecutionResult { + actionName: string; + successCount: number; + skippedCount: number; + failCount: number; +} + /** * Payload for the content-type selector dialog: the palette list type that * encodes which base type(s) to show (e.g. ALL_CONTENT_TYPES or a single base type). diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.spec.ts index cf9f535204d9..fda885cf7663 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.spec.ts @@ -1,15 +1,17 @@ import { describe, expect } from '@jest/globals'; import { createServiceFactory, SpectatorService, mockProvider } from '@openng/spectator/jest'; -import { of, throwError } from 'rxjs'; +import { NEVER, of, throwError } from 'rxjs'; import { Location } from '@angular/common'; -import { provideHttpClient } from '@angular/common/http'; +import { HttpErrorResponse, provideHttpClient } from '@angular/common/http'; import { ActivatedRoute } from '@angular/router'; import { DotContentDriveService, DotFolderService, - DotPropertiesService + DotHttpErrorManagerService, + DotPropertiesService, + DotWorkflowActionsFireService } from '@dotcms/data-access'; import { DotContentDriveItem, DotContentDriveSearchResponse, DotSite } from '@dotcms/dotcms-models'; import { GlobalStore } from '@dotcms/store'; @@ -46,6 +48,9 @@ describe('DotContentDriveStore', () => { mockProvider(DotFolderService, { getFolders: jest.fn().mockReturnValue(of([])) }), + // Required by `withActionExecution`, which fires workflow actions from the store. + mockProvider(DotWorkflowActionsFireService), + mockProvider(DotHttpErrorManagerService), // The store subscribes to Location (popstate re-hydration); capture the handler here. mockProvider(Location, { subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }) @@ -702,6 +707,9 @@ describe('DotContentDriveStore - onInit', () => { mockProvider(DotFolderService, { getFolders: jest.fn().mockReturnValue(of([])) }), + // Required by `withActionExecution`, which fires workflow actions from the store. + mockProvider(DotWorkflowActionsFireService), + mockProvider(DotHttpErrorManagerService), // The store subscribes to Location (popstate re-hydration); capture the handler here. mockProvider(Location, { subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }) @@ -751,6 +759,13 @@ describe('DotContentDriveStore - Browser Back/Forward (popstate) re-hydration', mockProvider(Location, { subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }) }), + // Required by `withActionExecution`, which fires workflow actions from the store. + mockProvider(DotWorkflowActionsFireService), + mockProvider(DotHttpErrorManagerService), + // withFlags fetches feature flags on init; stub so no real HTTP fires. + mockProvider(DotPropertiesService, { + getFeatureFlags: jest.fn().mockReturnValue(of({})) + }), provideHttpClient() ] }); @@ -836,6 +851,9 @@ describe('DotContentDriveStore - Content Loading Effect', () => { mockProvider(DotFolderService, { getFolders: jest.fn().mockReturnValue(of([])) }), + // Required by `withActionExecution`, which fires workflow actions from the store. + mockProvider(DotWorkflowActionsFireService), + mockProvider(DotHttpErrorManagerService), // The store subscribes to Location (popstate re-hydration); capture the handler here. mockProvider(Location, { subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }) @@ -1092,3 +1110,202 @@ describe('DotContentDriveStore - Content Loading Effect', () => { }); }); }); + +describe('DotContentDriveStore - withActionExecution', () => { + let spectator: SpectatorService>; + let store: InstanceType; + let fireService: jest.Mocked; + let httpErrorManager: jest.Mocked; + + const createService = createServiceFactory({ + service: DotContentDriveStore, + providers: [ + mockProvider(ActivatedRoute, { snapshot: { queryParams: {} } }), + mockProvider(GlobalStore, { + siteDetails: jest.fn().mockReturnValue(MOCK_SITES[0]) + }), + mockProvider(DotContentDriveService, { + search: jest.fn().mockReturnValue(of(MOCK_SEARCH_RESPONSE)) + }), + mockProvider(DotFolderService, { + getFolders: jest.fn().mockReturnValue(of([])) + }), + mockProvider(DotWorkflowActionsFireService, { + fireDefaultAction: jest.fn(), + bulkFire: jest.fn() + }), + mockProvider(DotHttpErrorManagerService, { handle: jest.fn() }), + // The store subscribes to Location (popstate re-hydration); stub so it is inert here. + mockProvider(Location, { + subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }) + }), + // withFlags fetches feature flags on init; stub so no real HTTP fires. + mockProvider(DotPropertiesService, { + getFeatureFlags: jest.fn().mockReturnValue(of({})) + }), + provideHttpClient() + ] + }); + + beforeEach(() => { + // The provider mocks live in the factory closure, so call counts would otherwise accumulate + // across tests in this block. + jest.clearAllMocks(); + + spectator = createService(); + store = spectator.service; + fireService = spectator.inject( + DotWorkflowActionsFireService + ) as jest.Mocked; + httpErrorManager = spectator.inject( + DotHttpErrorManagerService + ) as jest.Mocked; + + fireService.fireDefaultAction.mockReturnValue( + of({ results: [], summary: { affected: 2, successCount: 2, failCount: 0, time: 1 } }) + ); + fireService.bulkFire.mockReturnValue(of({ successCount: 2, skippedCount: 0, fails: [] })); + }); + + describe('executeQuickAction', () => { + it('should publish the running action so the toolbar can report it', () => { + // Never settles, so the in-flight state is observable. + fireService.fireDefaultAction.mockReturnValue(NEVER); + + store.executeQuickAction('PUBLISH', 'Publish', ['inode-1', 'inode-2']); + + expect(store.actionExecution()).toEqual({ actionName: 'Publish', total: 2 }); + }); + + it('should fire the default action with the given inodes', () => { + store.executeQuickAction('PUBLISH', 'Publish', ['inode-1']); + + expect(fireService.fireDefaultAction).toHaveBeenCalledWith({ + action: 'PUBLISH', + inodes: ['inode-1'] + }); + }); + + it('should report the counts the endpoint returned, not the number of inodes sent', () => { + // Per-item failures are an expected outcome (a lock held by another user, a permission + // the row state cannot see), so the result has to reflect what the server actually did. + fireService.fireDefaultAction.mockReturnValue( + of({ + results: [], + summary: { affected: 2, successCount: 1, failCount: 1, time: 1 } + }) + ); + + store.executeQuickAction('PUBLISH', 'Publish', ['inode-1', 'inode-2']); + + expect(store.actionExecutionResult()).toEqual({ + actionName: 'Publish', + successCount: 1, + skippedCount: 0, + failCount: 1 + }); + }); + + it('should clear the running action once settled', () => { + store.executeQuickAction('PUBLISH', 'Publish', ['inode-1']); + + expect(store.actionExecution()).toBeUndefined(); + expect(store.actionExecutionResult()).toBeDefined(); + }); + + it('should not fire when there are no inodes', () => { + store.executeQuickAction('PUBLISH', 'Publish', []); + + expect(fireService.fireDefaultAction).not.toHaveBeenCalled(); + expect(store.actionExecution()).toBeUndefined(); + }); + + it('should refuse to start a second run while one is in flight', () => { + // Guards the double-fire the old component-owned flag allowed: closing and reopening the + // dialog used to reset it, letting the same rows be fired twice. + fireService.fireDefaultAction.mockReturnValue(NEVER); + + store.executeQuickAction('PUBLISH', 'Publish', ['inode-1']); + store.executeQuickAction('PUBLISH', 'Publish', ['inode-1']); + + expect(fireService.fireDefaultAction).toHaveBeenCalledTimes(1); + }); + + it('should hand errors to the error manager and clear the running action', () => { + const error = new HttpErrorResponse({ status: 403 }); + fireService.fireDefaultAction.mockReturnValue(throwError(() => error)); + + store.executeQuickAction('PUBLISH', 'Publish', ['inode-1']); + + expect(httpErrorManager.handle).toHaveBeenCalledWith(error); + expect(store.actionExecution()).toBeUndefined(); + expect(store.actionExecutionResult()).toBeUndefined(); + }); + }); + + describe('executeWorkflowAction', () => { + it('should fire the bulk request with the given contentlet ids', () => { + store.executeWorkflowAction('action-review', 'Send for Review', ['inode-1', 'inode-2']); + + expect(fireService.bulkFire).toHaveBeenCalledWith( + expect.objectContaining({ + workflowActionId: 'action-review', + contentletIds: ['inode-1', 'inode-2'] + }) + ); + }); + + it('should carry skipped items through to the result', () => { + // A mixed-type selection partially skips by design: contentlets whose scheme does not own + // the action are skipped server-side. + fireService.bulkFire.mockReturnValue( + of({ successCount: 1, skippedCount: 1, fails: [] }) + ); + + store.executeWorkflowAction('action-review', 'Send for Review', ['inode-1', 'inode-2']); + + expect(store.actionExecutionResult()).toEqual({ + actionName: 'Send for Review', + successCount: 1, + skippedCount: 1, + failCount: 0 + }); + }); + + it('should count per-item failures from the fails list', () => { + fireService.bulkFire.mockReturnValue( + of({ + successCount: 1, + skippedCount: 0, + fails: [{ inode: 'inode-2', errorMessage: 'locked' }] + }) + ); + + store.executeWorkflowAction('action-review', 'Send for Review', ['inode-1', 'inode-2']); + + expect(store.actionExecutionResult()?.failCount).toBe(1); + }); + + it('should hand errors to the error manager and clear the running action', () => { + fireService.bulkFire.mockReturnValue( + throwError(() => new HttpErrorResponse({ status: 500 })) + ); + + store.executeWorkflowAction('action-review', 'Send for Review', ['inode-1']); + + expect(httpErrorManager.handle).toHaveBeenCalled(); + expect(store.actionExecution()).toBeUndefined(); + }); + }); + + describe('clearActionExecutionResult', () => { + it('should drop the result once it has been presented', () => { + store.executeQuickAction('PUBLISH', 'Publish', ['inode-1']); + expect(store.actionExecutionResult()).toBeDefined(); + + store.clearActionExecutionResult(); + + expect(store.actionExecutionResult()).toBeUndefined(); + }); + }); +}); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.ts index ac16194cddac..f719480e3c8c 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.ts @@ -23,6 +23,7 @@ import { } from '@dotcms/dotcms-models'; import { GlobalStore, withFlags } from '@dotcms/store'; +import { withActionExecution } from './features/action-execution/withActionExecution'; import { withContextMenu } from './features/context-menu/withContextMenu'; import { withDialog } from './features/dialog/withDialog'; import { withDragging } from './features/dragging/withDragging'; @@ -503,5 +504,6 @@ export const DotContentDriveStore = signalStore( withContextMenu(), withDialog(), withSidebar(), - withDragging() + withDragging(), + withActionExecution() ); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/action-execution/withActionExecution.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/action-execution/withActionExecution.ts new file mode 100644 index 000000000000..377fa62cb3d8 --- /dev/null +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/action-execution/withActionExecution.ts @@ -0,0 +1,176 @@ +import { patchState, signalStoreFeature, type, withMethods, withState } from '@ngrx/signals'; +import { EMPTY } from 'rxjs'; + +import { inject } from '@angular/core'; + +import { catchError, take } from 'rxjs/operators'; + +import { DotHttpErrorManagerService, DotWorkflowActionsFireService } from '@dotcms/data-access'; +import { DotActionBulkRequestOptions } from '@dotcms/dotcms-models'; + +import { + DotContentDriveActionExecution, + DotContentDriveActionExecutionResult, + DotContentDriveState +} from '../../../shared/models'; + +interface WithActionExecutionState { + /** The action currently being applied, or `undefined` when nothing is running. */ + actionExecution?: DotContentDriveActionExecution; + /** + * Outcome of the last finished execution, awaiting presentation. The shell consumes this and + * calls {@link clearActionExecutionResult}; the store never shows the toast itself. + */ + actionExecutionResult?: DotContentDriveActionExecutionResult; +} + +/** + * Owns the firing of workflow actions over the current selection. + * + * **Why this lives in the store and not in the Action Center dialog.** The dialog is rendered inside + * the shell's `@switch`, so closing it destroys the component. An execution owned by that component + * would either be cancelled on close (if correctly tied to its lifecycle) or leak (if not) — and it + * previously leaked, which made "closing the dialog does not abort the action" true only by accident. + * Holding the subscription here makes surviving the close a deliberate property: the store outlives + * every dialog, so `takeUntilDestroyed` in the dialog stays correct and nothing silently aborts. + * + * It also gives a reopened dialog a truthful state. Because {@link actionExecution} is store state + * rather than a component signal, reopening mid-flight still reports the run as in progress, which is + * what stops the same action being fired twice over the same rows. + */ +export function withActionExecution() { + return signalStoreFeature( + { + state: type() + }, + withState({ + actionExecution: undefined, + actionExecutionResult: undefined + }), + withMethods( + ( + store, + workflowActionsFireService = inject(DotWorkflowActionsFireService), + httpErrorManagerService = inject(DotHttpErrorManagerService) + ) => { + /** + * Settles a finished run by publishing its result for the shell to present. + * + * Refreshing the grid is deliberately *not* done here. `loadItems` belongs to the base + * store's own `withMethods`, and a feature cannot reach it: the accumulated methods + * type at this point in the composition widens to `MethodsDictionary`, so declaring it + * via `methods: type<...>()` does not compile. It would also be redundant — `loadItems` + * already sets `LOADING` and clears the selection itself. The shell reloads when it + * consumes the result, which is where the rest of the post-run UI work already lives. + */ + const onSettled = (result: DotContentDriveActionExecutionResult): void => { + patchState(store, { + actionExecution: undefined, + actionExecutionResult: result + }); + }; + + return { + /** + * Fires a quick action (publish, unpublish, archive, …) over the given inodes. + * + * Counts come from the response rather than `inodes.length`: the endpoint answers + * 200 with per-item failures inside, so a lock held by another user or a + * permission the row state could not see would otherwise read as a success. + */ + executeQuickAction: ( + actionId: string, + actionName: string, + inodes: string[] + ): void => { + if (!inodes.length || store.actionExecution()) { + return; + } + + patchState(store, { + actionExecution: { actionName, total: inodes.length }, + actionExecutionResult: undefined + }); + + workflowActionsFireService + .fireDefaultAction({ action: actionId, inodes }) + .pipe( + take(1), + catchError((error) => { + patchState(store, { actionExecution: undefined }); + httpErrorManagerService.handle(error); + + return EMPTY; + }) + ) + .subscribe((result) => + onSettled({ + actionName, + successCount: result?.summary?.successCount ?? inodes.length, + skippedCount: 0, + failCount: result?.summary?.failCount ?? 0 + }) + ); + }, + + /** + * Fires the selected workflow action over the given contentlet inodes. + * + * Contentlets whose scheme does not own the action are skipped server-side and + * reported in `skippedCount`, so a mixed-type selection partially skips by + * design — the result carries that through to the toast. + */ + executeWorkflowAction: ( + workflowActionId: string, + actionName: string, + contentletIds: string[] + ): void => { + if (!contentletIds.length || store.actionExecution()) { + return; + } + + patchState(store, { + actionExecution: { actionName, total: contentletIds.length }, + actionExecutionResult: undefined + }); + + const request: DotActionBulkRequestOptions = { + workflowActionId, + contentletIds, + additionalParams: { + assignComment: { assign: '', comment: '' }, + pushPublish: {}, + additionalParamsMap: { _path_to_move: '' } + } + }; + + workflowActionsFireService + .bulkFire(request) + .pipe( + take(1), + catchError((error) => { + patchState(store, { actionExecution: undefined }); + httpErrorManagerService.handle(error); + + return EMPTY; + }) + ) + .subscribe((result) => + onSettled({ + actionName, + successCount: result?.successCount ?? 0, + skippedCount: result?.skippedCount ?? 0, + failCount: result?.fails?.length ?? 0 + }) + ); + }, + + /** Called by the shell once the result has been presented. */ + clearActionExecutionResult: (): void => { + patchState(store, { actionExecutionResult: undefined }); + } + }; + } + ) + ); +} diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.spec.ts index 7484d4b3a5af..0c44547e780a 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.spec.ts @@ -144,6 +144,88 @@ describe('action-center utils', () => { expect(byId.get(WORKFLOW_ACTION_ID.UNARCHIVE)).toBe(1); }); + it('should count Lock for items that are not locked', () => { + const items = [ + contentlet({ inode: 'a', locked: false }), + contentlet({ inode: 'b', locked: true }), + contentlet({ inode: 'c', locked: false }) + ]; + + const lock = getQuickActions(items).find( + (action) => action.id === WORKFLOW_ACTION_ID.LOCK + ); + + expect(lock?.count).toBe(2); + expect(lock?.eligibleInodes).toEqual(['a', 'c']); + }); + + it('should not count archived items as lockable or unlockable', () => { + // Archived content is a dead end until it is unarchived: locking it has no purpose and + // `deleteContentlets` honours `canLock`, so a stray lock quietly makes the item + // undeletable by anyone but the lock holder. Unlock is covered by `locked` alone + // (archive refuses locked content), but excluded explicitly so the pair reads the same. + const items = [contentlet({ inode: 'a', archived: true, locked: false })]; + + const byId = new Map(getQuickActions(items).map((action) => [action.id, action.count])); + + expect(byId.get(WORKFLOW_ACTION_ID.LOCK)).toBe(0); + expect(byId.get(WORKFLOW_ACTION_ID.UNLOCK)).toBe(0); + }); + + it('should count Unlock only for locked items', () => { + const items = [ + contentlet({ inode: 'a', locked: true }), + contentlet({ inode: 'b', locked: false }), + contentlet({ inode: 'c', locked: true }) + ]; + + const unlock = getQuickActions(items).find( + (action) => action.id === WORKFLOW_ACTION_ID.UNLOCK + ); + + expect(unlock?.count).toBe(2); + expect(unlock?.eligibleInodes).toEqual(['a', 'c']); + }); + + it('should warn on Unlock about locks held by other users', () => { + // `contentEditable` is the server's answer to "is this locked by *me*?" — false on a + // locked row means someone else holds it, and only an administrator can release it. + const items = [ + contentlet({ inode: 'mine', locked: true, contentEditable: true }), + contentlet({ inode: 'theirs', locked: true, contentEditable: false }), + contentlet({ inode: 'also-theirs', locked: true, contentEditable: false }) + ]; + + const unlock = getQuickActions(items).find( + (action) => action.id === WORKFLOW_ACTION_ID.UNLOCK + ); + + // Every locked item is still fired — the server is the authority on who may unlock. + expect(unlock?.count).toBe(3); + expect(unlock?.warningCount).toBe(2); + expect(unlock?.warningHint).toBeTruthy(); + }); + + it('should not warn on Unlock when every lock is the current user’s own', () => { + const items = [contentlet({ inode: 'mine', locked: true, contentEditable: true })]; + + const unlock = getQuickActions(items).find( + (action) => action.id === WORKFLOW_ACTION_ID.UNLOCK + ); + + expect(unlock?.warningCount).toBe(0); + }); + + it('should not warn on actions other than Unlock', () => { + const items = [contentlet({ inode: 'a', locked: true, contentEditable: false })]; + + for (const action of getQuickActions(items)) { + if (action.id !== WORKFLOW_ACTION_ID.UNLOCK) { + expect(action.warningCount).toBe(0); + } + } + }); + it('should still list actions that apply to nothing, with a zero count', () => { // Nothing archived, so Delete applies to no item — but stays in the list so the dialog // can render it as non-selectable rather than dropping the row. @@ -156,6 +238,8 @@ describe('action-center utils', () => { it('should keep a fixed display order regardless of the selection', () => { const expected = [ + WORKFLOW_ACTION_ID.LOCK, + WORKFLOW_ACTION_ID.UNLOCK, WORKFLOW_ACTION_ID.PUBLISH, WORKFLOW_ACTION_ID.UNPUBLISH, WORKFLOW_ACTION_ID.ARCHIVE, @@ -172,6 +256,9 @@ describe('action-center utils', () => { (a) => a.id ) ).toEqual(expected); + expect( + getQuickActions([contentlet({ inode: 'c', locked: true })]).map((a) => a.id) + ).toEqual(expected); }); it('should mark Add to Bundle as pending so it can never be fired', () => { diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.ts index b3f9b7d18d4d..30702bb75261 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.ts @@ -46,14 +46,24 @@ export interface DotActionCenterQuickAction { * When set the row is always non-selectable and shows this as its hint. */ pendingHint?: string; + /** + * How many of the {@link eligibleInodes} are expected to fail, with {@link warningHint} + * explaining why. `0` for actions with nothing to warn about. + * + * These items are still fired — the count is a heads-up, not a filter. See the Unlock entry in + * {@link QUICK_ACTIONS} for why the client cannot decide this on its own. + */ + warningCount: number; + /** i18n key describing what {@link warningCount} counts. Absent when it is `0`. */ + warningHint?: string; } /** * Quick actions offered in the Action Center. * - * **The order of this array is the display order and is fixed** — Publish, Unpublish, Archive, - * Delete, Unarchive, Add to Bundle. Rows keep their position whether or not they are selectable, so - * the list never reshuffles as the selection changes. + * **The order of this array is the display order and is fixed** — Lock, Unlock, Publish, Unpublish, + * Archive, Delete, Unarchive, Add to Bundle. Rows keep their position whether or not they are + * selectable, so the list never reshuffles as the selection changes. * * Scope notes for v1: * - Every entry except Add to Bundle is a `SystemAction` the multi-contentlet endpoint accepts @@ -62,11 +72,12 @@ export interface DotActionCenterQuickAction { * list of asset identifiers, so the endpoint is not the blocker: it needs a target bundle, which * means a picker step (`DotAddToBundleComponent` takes a single identifier today) and an * enterprise-license gate. Tracked separately. - * - **Lock / Unlock are absent**: no bulk REST endpoint exists. The legacy JSP drives unlock through - * a Struts command (`full_unlock_list`) that loops server-side. * * `eligibleWhen` derives the count from row state the grid already has. It is a state heuristic, * not a permission check — an item can be counted and still fail at execution. + * + * `warnWhen` marks eligible items that are *likely* to fail, so the row can say so up front without + * dropping them from the payload. */ const QUICK_ACTIONS: { id: DotActionCenterQuickActionId; @@ -79,7 +90,38 @@ const QUICK_ACTIONS: { /** Confirmation message key. Set for actions destructive enough to warrant a prompt. */ confirmMessage?: string; pendingHint?: string; + /** Counted among the eligible items to produce `warningCount`. */ + warnWhen?: (item: DotCMSContentlet) => boolean; + /** Explains what `warnWhen` matched. Required whenever `warnWhen` is set. */ + warningHint?: string; }[] = [ + { + // Lock and Unlock lead the list: they are the least destructive actions here and the ones a + // user reaches for mid-edit, so they sit furthest from Archive and Delete. + id: WORKFLOW_ACTION_ID.LOCK, + nameKey: 'content-drive.context-menu.lock', + icon: 'lock', + danger: false, + // Archived content is a dead end until unarchived, and `deleteContentlets` honours + // `canLock`, so locking an archived item would quietly make it undeletable by anyone but + // the lock holder. + eligibleWhen: (item) => !item.locked && !item.archived + }, + { + id: WORKFLOW_ACTION_ID.UNLOCK, + nameKey: 'content-drive.context-menu.unlock', + icon: 'lock_open', + danger: false, + // `locked` alone would do — archive refuses locked content — but the archived exclusion is + // spelled out so the pair reads the same way. + eligibleWhen: (item) => !!item.locked && !item.archived, + // A lock belonging to someone else can only be released by a CMS Administrator, and the + // grid has no idea whether the current user holds that role. So these items are counted, + // fired, and reported on rather than filtered out: `contentEditable` is false on a locked + // row the current user does not hold. + warnWhen: (item) => !item.contentEditable, + warningHint: 'content-drive.action-center.unlock.locked-by-others' + }, { id: WORKFLOW_ACTION_ID.PUBLISH, nameKey: 'Default-Action-Publish', @@ -175,9 +217,13 @@ export const getQuickActions = (items: DotContentDriveItem[]): DotActionCenterQu return QUICK_ACTIONS.map((quickAction) => { // One filter pass feeds both the count and the inodes that get fired, so the row can never // advertise a different number of items than the action actually touches. - const eligibleInodes = contentlets - .filter(quickAction.eligibleWhen) - .map((item) => item.inode); + const eligible = contentlets.filter(quickAction.eligibleWhen); + const eligibleInodes = eligible.map((item) => item.inode); + // Counted over `eligible` rather than the whole selection: warning about items the action + // was never going to touch would make the number meaningless. + const warningCount = quickAction.warnWhen + ? eligible.filter(quickAction.warnWhen).length + : 0; return { id: quickAction.id, @@ -187,7 +233,9 @@ export const getQuickActions = (items: DotContentDriveItem[]): DotActionCenterQu eligibleInodes, count: eligibleInodes.length, confirmMessage: quickAction.confirmMessage, - pendingHint: quickAction.pendingHint + pendingHint: quickAction.pendingHint, + warningCount, + warningHint: warningCount > 0 ? quickAction.warningHint : undefined }; }); }; diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/workflow-actions.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/workflow-actions.ts index 6b1480de765f..386ccec36b7b 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/workflow-actions.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/workflow-actions.ts @@ -15,7 +15,9 @@ export const WORKFLOW_ACTION_ID = { COPY: 'COPY', MOVE: 'MOVE', RENAME: 'RENAME', - DOWNLOAD: 'DOWNLOAD' + DOWNLOAD: 'DOWNLOAD', + LOCK: 'LOCK', + UNLOCK: 'UNLOCK' } as const; export type WORKFLOW_ACTION_ID = (typeof WORKFLOW_ACTION_ID)[keyof typeof WORKFLOW_ACTION_ID]; diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/workflow/SystemActionApiFireCommandFactory.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/workflow/SystemActionApiFireCommandFactory.java index befd924ca33a..e9ba038a7060 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/workflow/SystemActionApiFireCommandFactory.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/workflow/SystemActionApiFireCommandFactory.java @@ -26,9 +26,11 @@ import static com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction.DELETE; import static com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction.DESTROY; import static com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction.EDIT; +import static com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction.LOCK; import static com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction.NEW; import static com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction.PUBLISH; import static com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction.UNARCHIVE; +import static com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction.UNLOCK; import static com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction.UNPUBLISH; /** @@ -72,6 +74,10 @@ public static SystemActionApiFireCommandFactory getInstance() { this.commandMap.put(UNARCHIVE, new UnArchiveSystemActionApiFireCommandImpl()); this.commandMap.put(DELETE, new DeleteSystemActionApiFireCommandImpl()); this.commandMap.put(DESTROY, new DestroySystemActionApiFireCommandImpl()); + // Deliberately absent from `systemActionHasActionletHandlerMap`: locking is not a workflow + // transition, so there is no actionlet to look for and these commands always win. + this.commandMap.put(LOCK, new LockSystemActionApiFireCommandImpl()); + this.commandMap.put(UNLOCK, new UnlockSystemActionApiFireCommandImpl()); } private boolean hasPublishValid (final Tuple2 params) { @@ -624,4 +630,72 @@ public Contentlet fire(final Contentlet contentlet, final boolean needSave, fina return contentlet; } } + + ////////////////////////////// + + /** + * Implements a {@link SystemActionApiFireCommand} that locks the contentlet, covering the + * {@link com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction#LOCK} system + * action. + * + *

Unlike the commands above this is not a fallback for a missing actionlet — it is the only + * implementation. A lock is per-user state on the version info, not a step transition, so there + * is no workflow action to defer to and nothing to save: {@code needSave} is ignored, and body + * fields sent alongside a LOCK are not persisted.

+ * + *

{@link com.dotmarketing.portlets.contentlet.business.ContentletAPI#lock} enforces + * permission itself: it delegates to {@code canLock}, which requires EDIT on the contentlet or + * its content type and refuses a lock already held by somebody else. Locking content the caller + * already holds is a no-op, so re-firing over a stale selection is harmless.

+ */ + private class LockSystemActionApiFireCommandImpl implements SystemActionApiFireCommand { + + @WrapInTransaction + @Override + public Contentlet fire(final Contentlet contentlet, final boolean needSave, + final ContentletDependencies dependencies) + throws DotDataException, DotSecurityException { + + final User user = dependencies.getModUser(); + + Logger.info(this, () -> "The contentlet: " + contentlet.getIdentifier() + + ", will be locked for the user: " + user.getUserId()); + + contentletAPI.lock(contentlet, user, dependencies.isRespectAnonymousPermissions()); + + return contentlet; + } + } + + ////////////////////////////// + + /** + * Implements a {@link SystemActionApiFireCommand} that unlocks the contentlet, covering the + * {@link com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction#UNLOCK} system + * action. + * + *

{@link com.dotmarketing.portlets.contentlet.business.ContentletAPI#unlock} also goes + * through {@code canLock}, so releasing a lock held by another user throws unless the caller + * holds the CMS Administrator role. Over a collection that surfaces as a per-item failure in + * the response summary rather than a rejected batch, which is what lets a mixed selection + * partially succeed.

+ */ + private class UnlockSystemActionApiFireCommandImpl implements SystemActionApiFireCommand { + + @WrapInTransaction + @Override + public Contentlet fire(final Contentlet contentlet, final boolean needSave, + final ContentletDependencies dependencies) + throws DotDataException, DotSecurityException { + + final User user = dependencies.getModUser(); + + Logger.info(this, () -> "The contentlet: " + contentlet.getIdentifier() + + ", will be unlocked by the user: " + user.getUserId()); + + contentletAPI.unlock(contentlet, user, dependencies.isRespectAnonymousPermissions()); + + return contentlet; + } + } } diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/workflow/WorkflowResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/workflow/WorkflowResource.java index a0e2525d4af7..acdb31406501 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/workflow/WorkflowResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/workflow/WorkflowResource.java @@ -3420,7 +3420,8 @@ public final Response fireActionDefaultSinglePart(@Context final HttpServletRequ allowableValues = { "NEW", "EDIT", "PUBLISH", "UNPUBLISH", "ARCHIVE", "UNARCHIVE", - "DELETE", "DESTROY" + "DELETE", "DESTROY", + "LOCK", "UNLOCK" } ), description = "Default system action." @@ -3652,7 +3653,8 @@ public final Response fireMultipleActionDefault(@Context final HttpServletReques allowableValues = { "NEW", "EDIT", "PUBLISH", "UNPUBLISH", "ARCHIVE", "UNARCHIVE", - "DELETE", "DESTROY" + "DELETE", "DESTROY", + "LOCK", "UNLOCK" } ), description = "Default system action." @@ -4292,7 +4294,7 @@ private Response mergeContentlet(SystemAction systemAction, FireActionForm fireA /** * Check preconditions. * If contentlet can not be found, 404 - * if contentlet is not can not be a default action: UNPUBLISH, UNARCHIVE, DELETE, DESTROY + * if contentlet is not can not be a default action: UNPUBLISH, UNARCHIVE, DELETE, DESTROY, LOCK, UNLOCK * @param contentlet * @param systemAction * @throws NotFoundInDbException @@ -4307,12 +4309,17 @@ private void checkContentletState(final Contentlet contentlet, final SystemActio if (contentlet.isNew()) { + // LOCK/UNLOCK act on the version info of an existing contentlet. On a new one there is + // nothing to lock, and `lock` would otherwise fail deeper with a blank-inode state + // exception rather than a clear bad request. if ( systemAction == SystemAction.UNPUBLISH || systemAction == SystemAction.UNARCHIVE || systemAction == SystemAction.DELETE || - systemAction == SystemAction.DESTROY) { + systemAction == SystemAction.DESTROY || + systemAction == SystemAction.LOCK || + systemAction == SystemAction.UNLOCK) { - throw new IllegalArgumentException("A new Contentlet can not fire any of these actions: [EDIT, UNPUBLISH, UNARCHIVE, DELETE, DESTROY]"); + throw new IllegalArgumentException("A new Contentlet can not fire any of these actions: [EDIT, UNPUBLISH, UNARCHIVE, DELETE, DESTROY, LOCK, UNLOCK]"); } } } @@ -4545,7 +4552,8 @@ public final Response fireActionDefaultMultipartNewPath( allowableValues = { "NEW", "EDIT", "PUBLISH", "UNPUBLISH", "ARCHIVE", "UNARCHIVE", - "DELETE", "DESTROY" + "DELETE", "DESTROY", + "LOCK", "UNLOCK" } ), description = "Default system action." diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/business/WorkflowAPI.java b/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/business/WorkflowAPI.java index 0c556918fc1b..12a44b98d50c 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/business/WorkflowAPI.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/business/WorkflowAPI.java @@ -1235,7 +1235,17 @@ enum SystemAction { ARCHIVE, UNARCHIVE, DELETE, - DESTROY; + DESTROY, + /** + * Locking is per user rather than a workflow transition, so unlike the actions above + * these two have no actionlet and no shipped workflow action to map to. They are served by + * the API-call fallback in + * {@link com.dotcms.rest.api.v1.workflow.SystemActionApiFireCommandFactory}, which calls + * {@link com.dotmarketing.portlets.contentlet.business.ContentletAPI#lock} / + * {@code unlock} directly. Mapping either one to a workflow action has no effect. + */ + LOCK, + UNLOCK; /** * Prefer this over valueOf(String..) since mySQL sends lowercased vals diff --git a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties index 3b830907a426..e2604f4057d5 100644 --- a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties +++ b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties @@ -7177,9 +7177,12 @@ content-drive.action-center.one-at-a-time=Actions are executed one at a time. content-drive.action-center.done=Done content-drive.action-center.requires-input=This action needs extra information before it can run and can't be executed in bulk yet. content-drive.action-center.approximate-count=This action has a condition that is evaluated per item, so it may apply to fewer items than shown. +content-drive.action-center.applying=Applying {0} to {1} item(s)… content-drive.action-center.toast.executed=Action executed content-drive.action-center.toast.executed-detail={0} ran on {1} item(s). content-drive.action-center.toast.executed-with-skips={0} ran on {1} item(s). {2} skipped because their workflow doesn't include this action. +content-drive.action-center.toast.executed-with-fails={0} ran on {1} item(s). {2} failed — you may not have permission, or the content is locked by another user. +content-drive.action-center.unlock.locked-by-others={0} of these are locked by another user. Only an administrator can unlock those, and they will be reported as failed. content-drive.action-center.toast.error=Action failed content-drive.action-center.toast.error-detail=Something went wrong. No changes were applied. diff --git a/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml b/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml index 099e017443ea..7de88f0b0b1d 100644 --- a/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml +++ b/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml @@ -18671,6 +18671,8 @@ paths: - UNARCHIVE - DELETE - DESTROY + - LOCK + - UNLOCK requestBody: content: application/json: @@ -18899,6 +18901,8 @@ paths: - UNARCHIVE - DELETE - DESTROY + - LOCK + - UNLOCK requestBody: content: application/json: @@ -19049,6 +19053,8 @@ paths: - UNARCHIVE - DELETE - DESTROY + - LOCK + - UNLOCK requestBody: content: multipart/form-data: @@ -35964,6 +35970,8 @@ components: - UNARCHIVE - DELETE - DESTROY + - LOCK + - UNLOCK workflowAction: $ref: "#/components/schemas/WorkflowAction" TabDividerField: @@ -37011,6 +37019,8 @@ components: - UNARCHIVE - DELETE - DESTROY + - LOCK + - UNLOCK _2: type: string UniqueBySessionResume: @@ -38938,6 +38948,8 @@ components: - UNARCHIVE - DELETE - DESTROY + - LOCK + - UNLOCK required: - actionId - systemAction diff --git a/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/workflow/WorkflowResourceLockUnlockIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/workflow/WorkflowResourceLockUnlockIntegrationTest.java new file mode 100644 index 000000000000..3519d2aff50e --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/workflow/WorkflowResourceLockUnlockIntegrationTest.java @@ -0,0 +1,428 @@ +package com.dotcms.rest.api.v1.workflow; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import com.dotcms.datagen.ContentTypeDataGen; +import com.dotcms.datagen.ContentletDataGen; +import com.dotcms.datagen.TestUserUtils; +import com.dotcms.datagen.UserDataGen; +import com.dotcms.mock.request.MockAttributeRequest; +import com.dotcms.mock.request.MockHeaderRequest; +import com.dotcms.mock.request.MockHttpRequestIntegrationTest; +import com.dotcms.mock.request.MockSessionRequest; +import com.dotcms.contenttype.model.type.ContentType; +import com.dotcms.rest.EmptyHttpResponse; +import com.dotcms.util.IntegrationTestInitService; +import com.dotcms.workflow.form.FireActionForm; +import com.dotcms.workflow.form.FireMultipleActionForm; +import com.dotmarketing.beans.Permission; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.PermissionAPI; +import com.dotmarketing.business.Role; +import com.dotmarketing.portlets.contentlet.business.ContentletAPI; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.portlets.languagesmanager.model.Language; +import com.dotmarketing.portlets.workflows.business.WorkflowAPI.SystemAction; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.liferay.portal.model.User; +import java.io.ByteArrayOutputStream; +import java.util.ArrayList; +import java.util.Base64; +import java.util.List; +import java.util.Map; +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.Response.Status; +import javax.ws.rs.core.StreamingOutput; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * Covers firing {@link SystemAction#LOCK} and {@link SystemAction#UNLOCK} through the default + * system-action endpoints, both for a single contentlet and for a collection. + * + *

Locking is per user, which makes these two system actions behave unlike the rest: + * {@code canLock} refuses to release a lock held by somebody else unless the caller holds the CMS + * Administrator role. The bulk endpoint therefore reports those items as failures instead of + * refusing the whole batch, and that contract is what most of these tests pin down.

+ * + * @see SystemActionApiFireCommandFactory + */ +public class WorkflowResourceLockUnlockIntegrationTest { + + private static WorkflowResource workflowResource; + private static ContentletAPI contentletAPI; + private static PermissionAPI permissionAPI; + private static User systemUser; + private static Language defaultLanguage; + private static ContentType contentType; + + private static final String ADMIN_EMAIL = "admin@dotcms.com"; + private static final String ADMIN_PASSWORD = "admin"; + + @BeforeClass + public static void prepare() throws Exception { + IntegrationTestInitService.getInstance().init(); + + // The real WebResource on purpose: these tests hinge on *which* user is calling, so the + // caller has to be resolved from the request's Basic auth rather than stubbed. + workflowResource = new WorkflowResource(); + contentletAPI = APILocator.getContentletAPI(); + permissionAPI = APILocator.getPermissionAPI(); + systemUser = APILocator.systemUser(); + defaultLanguage = APILocator.getLanguageAPI().getDefaultLanguage(); + contentType = new ContentTypeDataGen().nextPersisted(); + } + + /** + * Method to test: {@link WorkflowResource#fireActionDefaultSinglePart} + *

+ * Given scenario: An unlocked contentlet and an administrator firing {@code LOCK} over it. + *

+ * Expected result: 200, and the contentlet is locked by the calling user. + */ + @Test + public void test_fireLock_singleContentlet_locksItForTheCaller() throws Exception { + final Contentlet contentlet = newContentlet(); + + final Response response = fireDefaultAction(adminRequest(), SystemAction.LOCK, contentlet); + + assertEquals(Status.OK.getStatusCode(), response.getStatus()); + assertEquals(adminUser().getUserId(), lockedBy(contentlet)); + } + + /** + * Method to test: {@link WorkflowResource#fireActionDefaultSinglePart} + *

+ * Given scenario: A contentlet the caller has locked, then firing {@code UNLOCK} over it. + *

+ * Expected result: 200, and the lock is released. + */ + @Test + public void test_fireUnlock_singleContentlet_releasesTheLock() throws Exception { + final Contentlet contentlet = newContentlet(); + contentletAPI.lock(contentlet, adminUser(), false); + + final Response response = fireDefaultAction(adminRequest(), SystemAction.UNLOCK, contentlet); + + assertEquals(Status.OK.getStatusCode(), response.getStatus()); + assertNull(lockedBy(contentlet)); + } + + /** + * Method to test: {@link WorkflowResource#fireActionDefaultSinglePart} + *

+ * Given scenario: A brand new (never persisted) contentlet firing {@code LOCK}. + *

+ * Expected result: A non-OK response — there is nothing to lock yet, and letting this through + * would lock a contentlet the caller never created. + */ + @Test + public void test_fireLock_newContentlet_isRejected() throws Exception { + final FireActionForm form = new FireActionForm( + new FireActionForm.Builder().contentlet(Map.of("stInode", contentType.inode()))); + + final Response response = workflowResource.fireActionDefaultSinglePart(adminRequest(), + new EmptyHttpResponse(), null, null, "WAIT_FOR", + String.valueOf(defaultLanguage.getId()), "DEFAULT", SystemAction.LOCK, form); + + assertNotEquals(Status.OK.getStatusCode(), response.getStatus()); + } + + /** + * Method to test: {@link WorkflowResource#fireMultipleActionDefault} + *

+ * Given scenario: Three unlocked contentlets, one {@code LOCK} call over all of them. + *

+ * Expected result: Every contentlet is locked and the streamed summary reports three + * successes and no failures — one request, not three. + */ + @Test + public void test_fireLock_multipleContentlets_locksAllOfThem() throws Exception { + final List contentlets = List.of(newContentlet(), newContentlet(), + newContentlet()); + + final JsonNode summary = fireMultiple(adminRequest(), SystemAction.LOCK, contentlets); + + assertEquals(3, summary.get("successCount").asInt()); + assertEquals(0, summary.get("failCount").asInt()); + for (final Contentlet contentlet : contentlets) { + assertEquals(adminUser().getUserId(), lockedBy(contentlet)); + } + } + + /** + * Method to test: {@link WorkflowResource#fireMultipleActionDefault} + *

+ * Given scenario: Two locked contentlets, one {@code UNLOCK} call over both. + *

+ * Expected result: Both locks are released and the summary reports two successes. + */ + @Test + public void test_fireUnlock_multipleContentlets_releasesAllOfThem() throws Exception { + final List contentlets = List.of(newContentlet(), newContentlet()); + for (final Contentlet contentlet : contentlets) { + contentletAPI.lock(contentlet, adminUser(), false); + } + + final JsonNode summary = fireMultiple(adminRequest(), SystemAction.UNLOCK, contentlets); + + assertEquals(2, summary.get("successCount").asInt()); + assertEquals(0, summary.get("failCount").asInt()); + for (final Contentlet contentlet : contentlets) { + assertNull(lockedBy(contentlet)); + } + } + + /** + * Method to test: {@link WorkflowResource#fireMultipleActionDefault} + *

+ * Given scenario: Two contentlets locked by an administrator. A limited user who does + * hold EDIT permission on them — so the permission gate passes — fires {@code UNLOCK}. + *

+ * Expected result: The batch is not refused; both items come back as failures and the locks + * survive. This is the documented rule: a lock held by another user is reported per item, not + * silently skipped and not fatal to the whole request. + */ + @Test + public void test_fireUnlock_lockHeldByAnotherUser_reportsPerItemFailure() throws Exception { + final String password = "TestPass" + System.currentTimeMillis() + "!"; + final User limitedUser = newLimitedUser(password); + final List contentlets = List.of(newContentlet(), newContentlet()); + + for (final Contentlet contentlet : contentlets) { + grantEdit(contentlet, limitedUser); + contentletAPI.lock(contentlet, adminUser(), false); + } + + final JsonNode entity = fireMultipleEntity( + requestForUser(limitedUser.getEmailAddress(), password), SystemAction.UNLOCK, + contentlets); + final JsonNode summary = entity.get("summary"); + + assertEquals(0, summary.get("successCount").asInt()); + assertEquals(2, summary.get("failCount").asInt()); + for (final Contentlet contentlet : contentlets) { + assertEquals("The lock must survive a denied unlock", adminUser().getUserId(), + lockedBy(contentlet)); + } + // Guards against the failure coming from the wrong place. Without the content-type half of + // `grantEdit` these items are rejected by `populateContentlet` on permissions and never + // reach `canLock`, so the counts above would be satisfied without the lock rule ever being + // exercised — green, and proving nothing. + assertTrue("Expected a lock failure, got: " + failMessages(entity), + failMessages(entity).toLowerCase().contains("lock")); + } + + /** + * Method to test: {@link WorkflowResource#fireMultipleActionDefault} + *

+ * Given scenario: A mixed batch — one contentlet locked by the calling user, one locked by + * somebody else — fired as {@code UNLOCK} by that limited user. + *

+ * Expected result: The caller's own lock is released while the other one is reported as a + * failure. A partial result, not an all-or-nothing one. + */ + @Test + public void test_fireUnlock_mixedOwnership_unlocksOwnLockAndFailsTheOther() throws Exception { + final String password = "TestPass" + System.currentTimeMillis() + "!"; + final User limitedUser = newLimitedUser(password); + final Contentlet ownLock = newContentlet(); + final Contentlet foreignLock = newContentlet(); + + grantEdit(ownLock, limitedUser); + grantEdit(foreignLock, limitedUser); + contentletAPI.lock(ownLock, limitedUser, false); + contentletAPI.lock(foreignLock, adminUser(), false); + + final JsonNode summary = fireMultiple( + requestForUser(limitedUser.getEmailAddress(), password), SystemAction.UNLOCK, + List.of(ownLock, foreignLock)); + + assertEquals(1, summary.get("successCount").asInt()); + assertEquals(1, summary.get("failCount").asInt()); + assertNull(lockedBy(ownLock)); + assertEquals(adminUser().getUserId(), lockedBy(foreignLock)); + } + + /** + * Method to test: {@link WorkflowResource#fireMultipleActionDefault} + *

+ * Given scenario: A limited user granted nothing at all on the target fires {@code LOCK}. + *

+ * Expected result: The item is reported as a failure and stays unlocked — permission is + * enforced server-side, regardless of what the UI offered. + *

+ * No permissions are stripped to set this up: a freshly created user holds only the backend and + * frontend roles, so it starts with no access to the content type. An earlier attempt overrode + * the contentlet's permissions in favour of the system role, which fails outright because that + * role is locked for editing. + */ + @Test + public void test_fireLock_withoutAnyPermission_reportsFailure() throws Exception { + final String password = "TestPass" + System.currentTimeMillis() + "!"; + final User limitedUser = newLimitedUser(password); + final Contentlet contentlet = newContentlet(); + + final JsonNode summary = fireMultiple( + requestForUser(limitedUser.getEmailAddress(), password), SystemAction.LOCK, + List.of(contentlet)); + + assertEquals(0, summary.get("successCount").asInt()); + assertEquals(1, summary.get("failCount").asInt()); + assertNull(lockedBy(contentlet)); + } + + /** + * Method to test: {@link WorkflowResource#fireActionDefaultSinglePart} + *

+ * Given scenario: A locked contentlet, unlocked and then locked again by the same caller. + *

+ * Expected result: Both calls succeed — LOCK on content the caller already holds is a no-op + * rather than an error, so re-firing over a stale selection cannot fail the batch. + */ + @Test + public void test_fireLock_alreadyLockedByCaller_succeeds() throws Exception { + final Contentlet contentlet = newContentlet(); + contentletAPI.lock(contentlet, adminUser(), false); + + final Response response = fireDefaultAction(adminRequest(), SystemAction.LOCK, contentlet); + + assertEquals(Status.OK.getStatusCode(), response.getStatus()); + assertEquals(adminUser().getUserId(), lockedBy(contentlet)); + } + + // ------------------------------------------------------------------------ + // helpers + // ------------------------------------------------------------------------ + + private static Contentlet newContentlet() { + return new ContentletDataGen(contentType.id()).languageId(defaultLanguage.getId()) + .nextPersisted(); + } + + private static User adminUser() throws Exception { + return APILocator.getUserAPI().loadByUserByEmail(ADMIN_EMAIL, systemUser, false); + } + + private static User newLimitedUser(final String password) throws Exception { + final Role role = TestUserUtils.getBackendRole(); + + return new UserDataGen().password(password) + .roles(role, TestUserUtils.getFrontendRole()).nextPersisted(); + } + + /** + * Grants the user READ+EDIT on the contentlet and on its content type. + * + * The content type half is not optional. {@code fireTransactionalAction} loads the target + * through {@code populateContentlet}, which rejects a caller without content-type permission + * before any lock logic runs — so a contentlet-only grant never reaches {@code canLock}, and a + * test written to prove the lock-ownership rule would pass on the wrong exception. + */ + private static void grantEdit(final Contentlet contentlet, final User user) throws Exception { + final int readEdit = PermissionAPI.PERMISSION_READ | PermissionAPI.PERMISSION_EDIT; + final String roleId = APILocator.getRoleAPI().getUserRole(user).getId(); + + final List typePermissions = new ArrayList<>(); + typePermissions.add(new Permission(contentType.getPermissionId(), roleId, readEdit, true)); + permissionAPI.save(typePermissions, contentType, systemUser, false); + + final List permissions = new ArrayList<>(); + permissions.add(new Permission(contentlet.getPermissionId(), roleId, readEdit, true)); + permissionAPI.save(permissions, contentlet, systemUser, false); + } + + /** + * The user id currently holding the lock, or {@code null} when the contentlet is unlocked. + */ + private static String lockedBy(final Contentlet contentlet) throws Exception { + return APILocator.getVersionableAPI() + .getContentletVersionInfo(contentlet.getIdentifier(), contentlet.getLanguageId()) + .map(versionInfo -> versionInfo.getLockedBy()) + .orElse(null); + } + + private static Response fireDefaultAction(final HttpServletRequest request, + final SystemAction systemAction, final Contentlet contentlet) { + + return workflowResource.fireActionDefaultSinglePart(request, new EmptyHttpResponse(), + contentlet.getInode(), null, "WAIT_FOR", + String.valueOf(contentlet.getLanguageId()), "DEFAULT", systemAction, null); + } + + /** + * Fires the multi-contentlet variant and returns the streamed {@code summary} node, which is + * where the per-item success/failure counts live. + */ + private static JsonNode fireMultiple(final HttpServletRequest request, + final SystemAction systemAction, final List contentlets) throws Exception { + + return fireMultipleEntity(request, systemAction, contentlets).get("summary"); + } + + /** + * As {@link #fireMultiple}, but returns the whole entity so a test can inspect the per-item + * {@code results} — needed to tell *why* an item failed, not merely that it did. + */ + private static JsonNode fireMultipleEntity(final HttpServletRequest request, + final SystemAction systemAction, final List contentlets) throws Exception { + + final List> contentletForms = new ArrayList<>(); + for (final Contentlet contentlet : contentlets) { + contentletForms.add(Map.of("inode", contentlet.getInode())); + } + + final FireMultipleActionForm form = new FireMultipleActionForm.Builder() + .contentlets(contentletForms).build(); + + final Response response = workflowResource.fireMultipleActionDefault(request, + new EmptyHttpResponse(), systemAction, form); + + assertEquals(Status.OK.getStatusCode(), response.getStatus()); + + final ByteArrayOutputStream output = new ByteArrayOutputStream(); + StreamingOutput.class.cast(response.getEntity()).write(output); + + final JsonNode entity = new ObjectMapper().readTree(output.toByteArray()).get("entity"); + assertTrue("The response must carry a summary", entity.has("summary")); + + return entity; + } + + /** Every {@code errorMessage} in the streamed results, flattened for assertion. */ + private static String failMessages(final JsonNode entity) { + final StringBuilder messages = new StringBuilder(); + + for (final JsonNode result : entity.get("results")) { + result.fields().forEachRemaining(field -> { + final JsonNode errorMessage = field.getValue().get("errorMessage"); + if (null != errorMessage) { + messages.append(errorMessage.asText()).append(' '); + } + }); + } + + return messages.toString(); + } + + private static HttpServletRequest adminRequest() { + return requestForUser(ADMIN_EMAIL, ADMIN_PASSWORD); + } + + private static HttpServletRequest requestForUser(final String email, final String password) { + final MockHeaderRequest request = new MockHeaderRequest(new MockSessionRequest( + new MockAttributeRequest( + new MockHttpRequestIntegrationTest("localhost", "/").request()).request()) + .request()); + + request.setHeader("Authorization", "Basic " + Base64.getEncoder() + .encodeToString((email + ":" + password).getBytes())); + + return request; + } +}