Skip to content

feat(content-drive): preview a workflow action's items before executing - #36884

Open
rjvelazco wants to merge 31 commits into
mainfrom
issue-36819-content-drive-action-center-per-item-drill-down-with-execution-results
Open

feat(content-drive): preview a workflow action's items before executing#36884
rjvelazco wants to merge 31 commits into
mainfrom
issue-36819-content-drive-action-center-per-item-drill-down-with-execution-results

Conversation

@rjvelazco

@rjvelazco rjvelazco commented Aug 4, 2026

Copy link
Copy Markdown
Member

Partially addresses #36819.

Adds the drill-down half of that issue: picking a workflow action now opens a preview of the contentlets it is about to run on, with a checkbox per row, so the payload can be reviewed and trimmed before firing. The execution-results half (per-item spinner / success / error with retry) is not in this PR — see Additional Info.

Proposed Changes

  • Preview step before firing a workflow action. Selecting an action and hitting Continue opens a 4-column table (checkbox / Title / Status / Type) of the contentlets it applies to. Execute then fires only the rows still checked. Quick actions are unchanged — they still fire on click with the existing confirm.
  • Firing now honours the preview. Workflow actions previously sent the entire selection and relied on the backend's skippedCount. They now send only the checked inodes, which also makes them consistent with quick actions (already sending just their eligible subset).
  • Per-action eligibility resolved by content type. The bulk endpoint reports counts but never says which contentlets an action matches, so the preview used to list every selected item — a Blog action showing a VtlInclude row the server was always going to skip. Since workflow schemes are assigned per content type, the lookup now runs once per content type in the selection and each action records the types that can run it. Cost is bounded by distinct content types (typically 1–2), not by contentlet count, and the requests run in parallel.
  • One dialog header. The shared dialog's header lives in the shell, so the preview rendering its own title produced a visibly duplicated header. The body now publishes a drill-down header through the store (dialogDrillDown), and the shell's single header retitles to the action name with a count that tracks the checked rows. Only the back control stays in the body, where it can disable itself while an action is in flight.
  • Long titles truncate. A long title widened the table past the dialog and pushed the Type column behind a horizontal scrollbar. Fixed table layout plus min-w-0 on the title span; full text kept as the hover title.

Reuses existing building blocks rather than adding new ones — DotContentletStatusBadgeComponent and DotContentThumbnailComponent from @dotcms/ui, PrimeNG table checkboxes, and content-drive's existing 20-rows-per-page convention.

Checklist

  • Tests
  • Translations
  • Security Implications Contemplated (add notes if applicable)

Additional Info

Out of scope here — the other half of #36819. No per-item execution results. Execute still fires, toasts, refreshes the grid and closes the dialog. Adding per-row status/retry needs a decision on how per-item outcomes are reported (the synchronous bulkFire returns aggregate counts; the SSE _bulkfire endpoint streams progress but needs a shim, since native EventSource cannot POST a body). Worth splitting into its own issue.

Known limitation: scheme-level, not step-level. Content-type grouping makes eligibility exact at scheme level, but two contentlets of the same type can sit on different workflow steps and only one step may expose the action. The endpoint still won't say which. The action's count remains the authority, so the preview keeps a partial-match warning that fires only on that genuine step-level gap. Making rows individually accurate needs a backend change (returning matching contentlet ids per action).

Rows are keyed on inode, not identifier as the main grid does. Language variants of one contentlet share an identifier but have distinct inodes, and inodes are what gets fired — keying on identifier would collapse two variants into one selection entry. Pinned by a test.

Reviewers: new i18n keys need a dotCMS restart. Language.properties is read by the backend at startup, so the five new content-drive.action-center.* keys render as raw keys against an already-running instance until it restarts (or the language cache is flushed). Not a frontend bug.

One commit is a revert. bd8560ffbc reverts 0304def518, a Vue-template reformatting commit that had reformatted 29 unrelated libs/sdk/vue files against the repo's Prettier config (prettier --check flagged all 29; clean after the revert). The two cancel out, so they contribute nothing to this PR's diff. Root cause worth noting separately: lint-staged.config.mjs globs cover ts,js,mjs,cjs,tsx,jsx and json,html,css,scss,md,yaml,yml.vue is in neither, so the pre-commit hook never formats Vue files while an unscoped nx format:write does.

Branch history is noisy. It carries the pre-squash commits of #36821 plus a merge, all redundant with main's squashed d595cd8e3c. The diff is unaffected (15 files, all listed above); only the commit list is long.

Verification

pnpm nx test portlets-content-drive   # 939/939, 30 suites
pnpm nx test data-access              # 751 passed, 11 skipped
pnpm nx lint portlets-content-drive   # clean
pnpm nx format:check                  # clean
pnpm nx build dotcms-ui               # AOT build passes (templates type-check)

Also exercised manually against a local backend on Angular 22 / TypeScript 6.0: mixed-type selection, unchecking rows, >20-item pagination, back navigation, and that quick actions still fire on a single click.

Videos

video.mov
video-2.mov

rjvelazco and others added 25 commits July 29, 2026 16:50
Adds the client side of POST /api/v1/workflow/contentlet/actions/bulk,
which returns the workflow actions available for a set of contentlets
grouped by scheme and step, each with the number of selected contentlets
it applies to.

The models capture two properties of the response that matter to callers:

- An action's `count` is already summed across every step of its scheme,
  so flattening steps into one list per scheme does not double-count.
- `conditionPresent` means the count is an upper bound. The backend does
  not evaluate the action's Velocity condition while aggregating, since
  there is no per-contentlet permissionable at that point.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Draft of the Action Center: a dialog for acting on a multi-item selection,
opened from the toolbar once more than one contentlet is selected. Built
with PrimeNG (accordion, radiobutton, badge, message, skeleton) and
Tailwind for layout only.

Quick Actions fires system actions over the whole eligible selection in
one request. Counts are derived client-side from row state, and an action
that applies to nothing is omitted rather than shown as "(0)".

Workflow Actions renders one collapsible panel per scheme from the bulk
actions endpoint, with real per-action eligibility counts. Steps are
flattened into a single list per scheme, since the step grouping is a
backend detail the dialog does not need to surface.

Scope limits, all deliberate:

- One action per execute. No endpoint fires several different actions in
  one call, and firing one moves contentlets to a new step, which
  invalidates the other counts. The legacy JSP dialog works the same way.
- Actions needing extra input (push publish, move path, assign/comment)
  are disabled with a tooltip rather than reimplementing the params
  dialog. Conditional counts render as "<= N".
- Lock/Unlock and Add to Bundle are omitted: neither has a bulk REST
  endpoint. Legacy drives unlock through a Struts command that loops
  server-side, and add-to-bundle through a legacy AJAX servlet.
- Fires synchronously. Legacy uses the SSE endpoint for live progress,
  which is the better path for large batches but needs an SSE shim.
- Folders are excluded from every payload, matching the endpoints.

System Workflow is intentionally left visible: hiding it would remove
bulk Copy entirely (it has no system-action mapping) and would empty the
section on Community, where it is the only scheme that can exist.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses design feedback on the dialog shell.

The Action Center now owns its own p-dialog instead of rendering inside
the shell's shared one. That shared dialog gives a title and a scrolling
content area; this dialog needs a custom header, a custom footer, and a
body that is the only scrollable region. Two constraints forced the split:

- PrimeNG queries `#header` / `#footer` with `descendants: false`, so the
  templates must be direct children of `p-dialog` and cannot sit inside
  the shell's `@switch` over dialog types.
- Providing a `#footer` template on the shared dialog would render an
  empty footer for the folder and content-type dialogs.

The shell routes ACTION_CENTER out of the shared dialog's content switch
and mounts the component as a sibling instead, keeping the store as the
single source of open/close state.

Layout changes:

- Header carries the title and the selected-contentlet count.
- Footer carries the "one at a time" hint and Done, pinned.
- Content area is `p-0 overflow-hidden` via `pt`; an inner div owns
  `max-h-[60vh] overflow-y-auto`, so only the body scrolls.
- Quick action rows are plain buttons styled with Tailwind to match the
  design: a Material Symbols icon chip, left-aligned label, count and
  chevron trailing, red chip and label for destructive actions. They were
  p-buttons before, which centered their content and could not express
  the chip.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restyles the dialog against the prototype in
remix_-content-drive/components/ActionCenterDialog.tsx.

The prototype's raw palette is mapped onto the theme tokens that
tailwindcss-primeui exposes — `surface-*`, `primary-*`, `border-surface`
— rather than copying its `slate-*` and hardcoded `#1D1B4B` / `#1D4ED8`
values, so the dialog follows the active PrimeNG theme.

- Fixed-height flex column (`80vh`, `42rem` wide) with the content area
  flexing and scrolling. Replaces the `max-h-[60vh]` guess on an inner
  div; this is how the prototype does it and it keeps header and footer
  pinned without a second scroll container.
- Card treatment: sections sit on `surface-50` inside a rounded-xl
  `surface-100` border, and rows lift to `surface-0` on hover. This was
  inverted before (white card, grey hover).
- Row metrics from the prototype: `gap-4`, `py-3.5`, 20px icon glyphs,
  `size-9` chips with `shadow-sm`, 10px bold section labels.
- Scheme panels are now single-expand, matching the prototype: opening one
  collapses the rest, and the expanded scheme's name takes the primary
  colour. Collapsing a panel clears its pending action so Execute cannot
  stay armed for a hidden panel.

Two deliberate deviations from the prototype, both carried over from the
endpoint analysis:

- Steps use radio semantics, not checkboxes. The prototype multi-selects
  steps, but no endpoint fires several actions in one call and firing one
  moves contentlets to a new step, invalidating the other counts.
- No per-step icon chips. The prototype hand-picks a glyph per step; the
  API returns dotCMS icon names that do not map to Material Symbols, so
  there is nothing to render faithfully yet.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A collapsed scheme panel in the Action Center still took up the height of
its expanded content, leaving a large blank gap under the header.

PrimeNG 21 collapses accordion content by animating the motion wrapper's
`grid-template-rows` to `0fr`, but that wrapper is configured with
`hideStrategy: 'visibility'` and `unmountOnLeave: false`, so it stays
mounted and keeps its layout box. Without `overflow: hidden` the content
simply overflows the zero-height grid row and the panel keeps its full
height. PrimeNG's own stylesheet only ever sets `grid-template-rows: 1fr`
on `.p-accordioncontent .p-motion`, so nothing clips it by default.

Fixed with the same `pt` override already used by
dot-page-scanner-a11y-report:

    { motion: { root: { style: { overflow: 'hidden' } } } }

Also adopts that component's `dt` content-padding reset so the rows own
their padding, which restores the design's full-bleed dividers, and makes
`[multiple]="false"` explicit rather than relying on the default.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The scheme panels rendered as borderless grey cards. A `border` utility on
the panel had no effect: the theme ships `accordion.panel.border.width` as
`0 0 1px 0` — inter-panel dividers, which suit one stacked accordion — and
its runtime-injected CSS wins over a utility class.

Overriding the design token instead gives each panel a real border, which
is what the design shows: the schemes are separate cards separated by a
gap, not a stack.

    panel: { borderWidth: '1px', borderColor: '{surface.200}' }

The content section also gets a top border so an expanded header is
separated from its rows, and its background moves to the token rather than
a utility on an inner div, keeping one source of truth per surface.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gives the scheme panels the same radius and background treatment as the
Quick Actions list, and fixes the reason they did not match.

`p-accordion-panel` exposes only a `value` input — it has no `styleClass`
— so the `rounded-xl bg-surface-50 overflow-hidden` attribute on it was
inert. It has been doing nothing since the panels were introduced; the
grey came from the theme's header background, not from that class. Panel
styling now goes through the `panel` pass-through section, where
`overflow: hidden` is also what clips the header and content corners to
the panel radius.

This also corrects the reasoning in 2a4ebf1: the panel border did not
lose a specificity fight with the theme, the class was never applied at
all. Overriding `accordion.panel.border.*` remains the right fix, since
the theme's `0 0 1px 0` default is meant for a stacked accordion.

Header backgrounds now come from tokens and mirror the Quick Actions rows:
`surface-50` at rest, white on hover, and white while expanded so the open
scheme reads as the active card.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hrough

Applies the scheme panel's radius via `p-accordion-panel`'s `pt.root`
instead of the parent accordion's `pt.panel`, keeping the override on the
component it targets. Also drops a stray unused `PanelClasses` import.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tion Center

The Action Center button was replacing the flat workflow action buttons
once more than one contentlet was selected, so the quick per-item actions
disappeared exactly when a multi-selection was made.

They are now offered together: the workflow action buttons stay visible for
any selection and keep their place first in the toolbar row, with the
Action Center button appended after them. The dialog adds what the flat
buttons cannot express — per-action eligibility counts and the workflow
actions grouped by scheme — rather than standing in for them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tlet

Drops the Action Center button's threshold from more than one contentlet to
one or more, so it is available alongside the flat action buttons for a
single selection.

This widens the original scope, which specced the button as appearing only
for a multi-selection. It holds up: the dialog's value is the per-action
eligibility counts and the workflow actions grouped by scheme, and neither
depends on the selection having more than one item. The bulk endpoints take
an inode list, so a single inode needs no special handling.

Folders still do not count — a folder-only selection offers no Action
Center, since every bulk endpoint ignores folders.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ck actions visible

Two pieces of design feedback.

**Renamed to "Workflow Center"** and dropped the toolbar button's icon. The
button and the dialog header share one message key, so the label change
covers both and they stay consistent.

**Quick actions are no longer filtered out when they apply to nothing.**
Every action is always listed; a count of 0 now means "does not apply to
this selection" and the row renders non-selectable, with a tooltip saying
so. Previously such rows were dropped, which made the list shift as the
selection changed — rows appearing and disappearing under the pointer, and
no indication that an action existed but was unavailable. An empty result
still means there are no contentlets at all, where nothing could apply.

Note this makes the count honest rather than decorative: a disabled row is
the one place the UI admits an action is unavailable for the whole
selection, which the previous behaviour hid entirely.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…x the order

Adds Add to Bundle to the quick actions, always non-selectable with a hint
explaining why, and pins the display order to Publish, Unpublish, Archive,
Delete, Add to Bundle. Rows hold that position whether or not they are
selectable, so the list never reshuffles as the selection changes.

Add to Bundle is not blocked on an endpoint: `POST /api/v1/bundles/assets`
takes a list of asset **identifiers** (note: identifiers, unlike the
workflow bulk endpoints, which take inodes). What it needs is a target
bundle, so a picker step — `DotAddToBundleComponent` accepts a single
identifier and still posts to the legacy AJAX servlet — plus an
enterprise-license gate. That is a shared-component change and is tracked
separately, hence the disabled row rather than a half-wired one.

A `pendingHint` on the action models "no working implementation yet",
distinct from a zero count meaning "does not apply to this selection". The
hint takes precedence, so the row explains itself rather than blaming the
selection, and the execute handler guards on it as well as on the disabled
attribute.

This drops **Unarchive**, which is not in the design's quick-action set. It
is a valid SystemAction and was working, so re-adding it is a one-line
change if that set is revisited.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Puts Unarchive back, positioned after Delete so the two archived-only
actions sit together and Add to Bundle stays last.

Without it, archiving was a one-way trip: Archive shows for items that are
not archived and Delete for items that are, but nothing in the dialog could
un-archive. Unarchive is a valid SystemAction the multi-contentlet fire
endpoint accepts, so it needs no extra plumbing.

Order is now Publish, Unpublish, Archive, Delete, Unarchive, Add to Bundle,
asserted by the fixed-order test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… owns

Removes Publish, Unpublish, Archive, Unarchive and Delete from the
toolbar's flat action buttons. They are offered by the Workflow Center's
Quick Actions, which shows how many of the selected items each one applies
to — something a flat button cannot express. Keeping both meant the same
action appeared twice, reached by two different code paths.

The toolbar keeps what the dialog does not cover: Edit Content, Edit Page,
Rename, Save as Draft and Download.

Two consequences handled rather than shipped:

- **Quick action labels no longer borrow from the toolbar list.** They were
  looked up in `DEFAULT_WORKFLOW_ACTIONS` by id, so trimming that array
  would have rendered raw ids ("PUBLISH"). Each quick action now carries its
  own `nameKey`, reusing the same `Default-Action-*` message keys.
- **Bulk Delete keeps its confirmation.** The toolbar's Delete prompted
  before firing; moving it would have left the only delete path unguarded.
  The Action Center now confirms any quick action carrying a
  `confirmMessage`, which is Delete alone, matching the previous behaviour.

Test changes: the toolbar spec used Publish and Archive as stand-ins for
"a workflow action" in its execute, toast and error tests — those are
repointed to Save as Draft, which fires through the same path. The two tests
whose subject was specifically a removed action are gone, and Delete's
confirm-then-fire flow is now covered in the Action Center spec, including
the dismissal case. The toolbar's confirm branch is left in place as the
mechanism a future confirmable action would use.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…w-fire path

Save as Draft is no longer needed in the toolbar. With it gone, every
remaining toolbar action is navigation, rename or download — none of them
fires a workflow action — so the machinery behind that path goes too:
`DotWorkflowActionsFireService`, the confirmation service and dialog, the
`confirmationMessage` field on `ContentDriveWorkflowAction`, and the
`executeWorkflowAction` / `performWorkflowAction` / `beforeExecuteWorkflowAction`
chain.

The component is now purely presentational dispatch: edit, rename, download.
Lifecycle actions live in the Workflow Center, which owns their confirmation
and error handling.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses the PR review.

**Quick actions fired over the whole selection while the row showed only
the eligible count.** `onExecuteQuickAction` built its payload from
`toContentletInodes($selectedItems())`, but the badge came from
`filter(eligibleWhen).length` — two independent derivations. A Publish
reading "(1)" would publish two items, and Delete would be attempted on
contentlets that are not archived.

The eligible inodes are now derived alongside the count in a single filter
pass and carried on the action as `eligibleInodes`, which is exactly what
gets fired. One source, so the number shown and the items touched cannot
drift apart. The success toast reports the same set.

Two regression tests lock this down: one asserts Publish sends only the
non-live inode, another parses the count out of the rendered row and
asserts the fired payload has that many inodes. The previous spec asserted
the buggy payload, which is why this passed review-less.

Also from the review:

- Errors now go through `DotHttpErrorManagerService.handle()` instead of a
  bespoke toast plus `console.error`. Beyond the documented anti-pattern,
  it matters here: per-item permission failures are an expected outcome of
  these endpoints, and the manager distinguishes 401/403 rather than
  flattening everything into one message.
- `getQuickActions` is fed `$contentlets()` rather than re-deriving folder
  exclusion from the raw selection.

New specs for the three untested seams: `getBulkActions` (inode and query
variants, plus the null-entity fallback), the toolbar's Workflow Center
gating and dialog wiring, and the shell's ACTION_CENTER routing including
that the shared dialog stays hidden while it is active.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-workflow-actions-with-one-action-per-execution
Addresses review feedback (zJaaal) on both the dialog template and the
shell: reuse the existing content-drive dialog instead of a second
`p-dialog`, so there is no duplicate dialog logic.

The Action Center is a case in the shared dialog's content switch again.
`$sharedDialogVisible` / `$actionCenterVisible` / `$isActionCenter` are gone
along with the sibling mount, and `onVisibleChange` no longer needs its
guard — there is one dialog and one open/close path.

The pinned-chrome layout survives without PrimeNG's `#header` / `#footer`
slots, which was the reason I reached for a second dialog in the first
place. Those slots are `ContentChild` queries with `descendants: false`, so
they cannot be provided from inside the shell's `@switch`, and adding a
footer slot would render an empty footer for the folder and content-type
dialogs. Making the *content box* a flex column achieves the same thing:

- The shell gives this type `flex flex-col overflow-hidden p-0!` plus a
  fixed height (the column needs something to flex against, or the body
  never scrolls), and drops the header's bottom rule so the title and the
  "N items selected" line read as one block.
- This component supplies the three rows: a pinned summary, a
  `flex-1 overflow-y-auto` body, and a pinned footer.

Other dialog types are untouched — they get no style overrides and no
footer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… into the header

Three related symptoms, one cause.

The content box was scrolling as a whole — footer included — because the
theme sets `overflow-y: auto` on `.p-dialog-content` and its
runtime-injected CSS outranks a Tailwind `overflow-hidden` utility. The flex
column never took effect, so the footer read as part of the content and the
scrollbar spanned it.

Fixed by driving the content box through `[contentStyle]` instead. Inline
styles beat the theme's class rules without `!` overrides:

    display: flex; flex-direction: column; flex: 1;
    min-height: 0; overflow: hidden; padding: 0

`min-height: 0` is the load-bearing part — a flex item will not shrink below
its content height without it, so the body could never scroll and the footer
was pushed out of view. The body is now the only scroll container and the
footer is pinned as its sibling.

The "N items selected" line moves into a real custom header via PrimeNG's
`#header` slot, which is where it belongs — previously it sat at the top of
the content and scrolled away, which is why it was missing from the report.
The slot is a `ContentChild` with `descendants: false`, so it lives as a
single direct child of `p-dialog` with the dialog type switched inside it;
providing it suppresses PrimeNG's own title span, hence the default branch
renders the title itself. The header's bottom rule is dropped for this type
so title and sub-line read as one block.

The footer stays a flex sibling rather than using the `#footer` slot, so the
Done button can remain disabled while an action is in flight — that state
lives in the dialog component, not the shell.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dupe its header

Three fixes to the workflow-action preview:

- The dialog showed two headers: the shell's "Workflow Center" title and count,
  plus the preview's own action title and count. The preview now publishes a
  drill-down header through the store (`dialogDrillDown`), so the shell's single
  header retitles to the action name with a count that tracks the checked rows.
  Only the back control stays in the body, where it can disable itself while an
  action is in flight.

- The preview listed every selected contentlet, including content types the
  action's scheme is not assigned to — a Blog action showing a VtlInclude row
  the server was always going to skip. The bulk endpoint reports counts without
  saying which contentlets match, but workflow schemes are assigned per content
  type, so the lookup now runs once per content type in the selection and each
  action records the types that can run it. Execute sends only those inodes.
  This is accurate at scheme level, not step level (contentlets of one type can
  sit on different steps), so the partial-match warning stays and now compares
  against the filtered rows.

- A long title widened the table past the dialog and pushed the Type column
  behind a horizontal scrollbar. Fixed table layout plus `min-w-0` on the title
  span so it truncates, with the full text kept as the hover title.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ts for improved readability"

This reverts commit 0304def.
@claude

claude Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @rjvelazco's task in 4m 30s —— View job


Code Review — workflow action preview

Reviewed the full diff (frontend only): action-center.ts utils, the action-center component + template, the new preview component, the shell header plumbing, withDialog store feature, the models, and the new i18n keys. Traced the merge/eligibility logic and the drill-down header flow end to end.

New Issues

No bugs found. The changes are internally consistent and the edge cases I checked hold up:

  • Payload/count coupling is sound. onExecuteWorkflowAction now fires $includedItems().map(i => i.inode) (dot-content-drive-action-center.component.ts:411), and the preview's checked set is seeded from $previewItems() — the same content-type-narrowed list the header count is derived from. No drift between what's shown and what's fired.
  • $actionCenterCount handles the all-unchecked case correctly (dot-content-drive-shell.component.ts:245): dialogDrillDown()?.itemCount ?? …0 is not nullish, so an empty preview renders "0 items selected" rather than falling back to the full selection count.
  • Drill-down lifecycle is clean. setDialog/closeDialog both clear dialogDrillDown (withDialog.ts:24-29), and onBackToActions clears it too. The action-center component is destroyed on dialog hide, so $view resets to 'actions' on reopen.
  • mergeActionCenterSchemes count summing is correct — each contentlet belongs to exactly one content-type group, so summing scheme/action counts across groups reproduces a single combined lookup.
  • Removed key content-drive.action-center.execute-workflow has no remaining references in core-web (confirmed the only other hits are the unrelated Unable-to-execute-workflows backend key). The five new keys are present in Language.properties.

Notes (non-blocking, already documented)

  • 🟡 The partial-match warning (dot-content-drive-action-center.component.html:290) compares $previewCount() > action.count. When approximateCount is true, action.count is an upper bound (), so the warning both (a) may not fire when skips actually occur and (b) uses that upper-bound count as {0} in the message, which can overstate. This is the exact scheme-vs-step-level limitation the PR body calls out and the ≤ count UI already signals it elsewhere — flagging only so it's on record, not as a regression.

Test coverage

The suite is thorough. The scheme-switch deselection (onOpenSchemeChange), the drill-down header state, and the inode-vs-identifier keying are all pinned by tests added in fa5b21b9. This addresses the gaps raised in the earlier unit-test review on this PR.

Verdict: No blocking issues. Frontend-only change; backend @Schema/REST rules and Maven/version conventions don't apply here. New i18n keys require a dotCMS restart to render (correctly noted in the PR body).

issue-36819-content-drive-action-center-per-item-drill-down-with-execution-results

@github-actions github-actions Bot added the Area : Frontend PR changes Angular/TypeScript frontend code label Aug 4, 2026
…p the redundant Execute label

- The dialog body scrolled, not the table, so the column headers scrolled out of
  view and the paginator ended up at the bottom of a scrolled page where it was
  easy to miss. The table is now `scrollable` with `scrollHeight="flex"` and goes
  edge to edge (no body padding), so it fills the dialog, keeps its headers
  pinned, and leaves the paginator outside the scroll region. The host is a
  shrinkable flex column so `flex` has a bounded height to resolve against.

- The Execute button repeated the action name, which the dialog header already
  shows since the drill-down header landed. It now reads just "Execute" with the
  count badge, and no longer grows with the action name. Reuses the existing
  `Execute` key, so `content-drive.action-center.preview.execute` is removed
  rather than left dangling.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rjvelazco rjvelazco changed the title feat(content-drive): preview a workflow action's items before executing #36819 feat(content-drive): preview a workflow action's items before executing Aug 4, 2026
@rjvelazco

Copy link
Copy Markdown
Member Author

@claude review this PR and check what unit test are valid, what can be removed and propuse new unit tests

@claude

claude Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @rjvelazco's task in 6m 43s —— View job


Unit test review — dot-content-drive Action Center preview

  • Gather context (PR diff, CLAUDE.md, changed files)
  • Read dot-content-drive-action-preview component + spec (new)
  • Read dot-content-drive-action-center component + spec
  • Read utils/action-center.ts + spec
  • Read shell component + spec, withDialog.ts, models.ts
  • Identify valid vs. redundant/removable tests
  • Propose new test cases for gaps

I read all 15 changed files end to end (component + template + spec pairs) and compared coverage against implementation branches. Overall: the test suite is thorough and well-targeted — most tests assert observable behavior (DOM/output) rather than internals, and several tests exist specifically to pin regressions the PR description calls out (inode-vs-identifier keying, count/payload drift, per-content-type eligibility). I found no tests worth removing as redundant. I did find real coverage gaps.

Valid, worth keeping (no action needed)

  • dot-content-drive-action-preview.component.spec.ts — clean, single-purpose, and the "language variants stay distinct rows" test (dot-content-drive-action-preview.component.spec.ts:163-186) is a good regression pin for the inode-keying decision called out in the PR body.
  • action-center.spec.ts (utils) — mergeActionCenterSchemes/groupByContentType/eligibleContentlets are pure functions with thorough branch coverage (union of content types, sum of counts, approximate-count propagation, empty-groups fallback). No changes needed.
  • dot-content-drive-action-center.component.spec.ts — the "fire exactly as many inodes as the row advertises" (:356-371) and "keep the fired payload in step with the count shown on the button" (:531-542) tests are good: they assert the displayed count and the fired payload came from the same computation, guarding against exactly the count/payload drift bug the PR fixes for quick actions and preview.

I don't see any test in this diff that duplicates another or tests implementation detail that should be removed — nothing stood out as safe to delete without losing signal.

Gaps — new tests worth adding

  1. onOpenSchemeChange deselection branch is completely untested (dot-content-drive-action-center.component.ts:438-452). Collapsing a scheme panel, or switching to a different one, clears $selectedActionId if the previously-armed action isn't in the newly-opened scheme. No spec calls onOpenSchemeChange or emits (valueChange) on p-accordion. Worth two tests: (a) selecting an action in scheme A, then opening scheme B → $selectedActionId() becomes null and Continue/Execute disable; (b) re-opening the same scheme (or a [scheme.id] array value, since PrimeNG can emit an array) keeps the action armed.

  2. The shell's drill-down header override ($actionCenterHeader / $actionCenterCount) is wired into the mock but never exercised (dot-content-drive-shell.component.spec.ts:95,150,182). dialogDrillDownSignal is declared and passed to the mocked store, but no test ever calls .set() on it. So the actual behavior this PR adds to the shell — "when the body publishes a drill-down, the one dialog header retitles to the action name and count" — has zero direct coverage at the shell level (it's only verified indirectly via the action-center component's store.setDialogDrillDown call args, which doesn't confirm the shell renders it). Suggested tests:

    • Set dialogDrillDownSignal.set({ header: 'Send for Review', itemCount: 2 }) → assert [data-testId="dialog-header"] renders 'Send for Review' and the sub-header renders the count, not the plain selection count.
    • Clear it back to undefined → header falls back to $activeDialog()?.header / $actionCenterSelectionCount().
  3. withDialog.spec.ts has no test for the two new methods it just gained (withDialog.ts:31-38). The spec still only covers setDialog/closeDialog from before this PR; setDialogDrillDown and clearDialogDrillDown — plus the "a fresh setDialog call clears any prior drill-down" and "closeDialog clears drill-down too" behaviors — aren't asserted anywhere. Given this is the actual state-holding logic behind gap Test Branch and Commit #2, this is the cheapest, most direct place to close it:

    it('should set the drill-down header', () => {
        store.setDialogDrillDown({ header: 'Send for Review', itemCount: 2 });
        expect(store.dialogDrillDown()).toEqual({ header: 'Send for Review', itemCount: 2 });
    });
    
    it('should clear the drill-down header', () => {
        store.setDialogDrillDown({ header: 'Send for Review', itemCount: 2 });
        store.clearDialogDrillDown();
        expect(store.dialogDrillDown()).toBeUndefined();
    });
    
    it('should clear a stale drill-down when a new dialog opens', () => {
        store.setDialogDrillDown({ header: 'Send for Review', itemCount: 2 });
        store.setDialog(mockDialog);
        expect(store.dialogDrillDown()).toBeUndefined();
    });
    
    it('should clear the drill-down on close', () => {
        store.setDialogDrillDown({ header: 'Send for Review', itemCount: 2 });
        store.closeDialog();
        expect(store.dialogDrillDown()).toBeUndefined();
    });
  4. Minor: PrimeNG's p-accordion valueChange can emit a single value or an array (the component signature already accounts for string[] | number[]), but no test feeds onOpenSchemeChange an array — only the plain string path is exercised implicitly through the DOM in existing tests. Worth one direct unit test calling spectator.component['onOpenSchemeChange'](['editorial']) to confirm the array-unwrapping branch (value[0]?.toString()) actually works, since accordion's real emission shape isn't simulated by clicking in JSDOM.

  5. onContinueToPreview's early-return when previewItems is empty (dot-content-drive-action-center.component.ts:365-367) has no direct test. It's a real branch: eligibleContentlets can return [] if an action's contentTypes resolved but none match the current selection (e.g., selection changed between load and click). Suggested: mock $selectedAction/selection so $previewItems() is [], call onContinueToPreview(), assert $view() stays 'actions' and setDialogDrillDown is not called.

Suggested fixes

Add missing tests →

Summary

No tests need to be removed — the suite is well-scoped and pins the right regressions. The gaps all cluster around the drill-down header plumbing (store → shell) and the scheme-switch deselection logic: both are real behavior added in this PR that's currently only indirectly exercised (via mock call assertions on the action-center side) rather than directly verified at the layer that owns the behavior (the store feature and the shell). None of these are release-blocking — the manual verification in the PR description covers the same paths — but they're the cheapest places to close the loop before the SSE/execution-results follow-up work builds on top of this dialog state.

…tch deselection

Closes the coverage gaps from the PR's unit-test review. All of them sat on
behavior this PR added that was only exercised indirectly:

- `withDialog`: `setDialogDrillDown` / `clearDialogDrillDown` had no tests at
  all, including the two behaviors that keep a stale header from leaking —
  `setDialog` and `closeDialog` both clearing it.
- Shell: `dialogDrillDown` was wired into the store mock but never set, so
  "publishing a drill-down retitles the one dialog header" was never verified at
  the layer that renders it. Now asserts the retitle and the fallback on clear.
- `onOpenSchemeChange`: the deselection branch was entirely untested — no spec
  called it. Covers switching scheme, collapsing every panel, reopening the same
  scheme, and the array shape PrimeNG's accordion can emit but JSDOM clicking
  never produces.
- `onContinueToPreview`: the early return when an action resolves to no eligible
  contentlets (selection changed after the lookup) had no test.

Each new test was checked against a deliberately mutated implementation to
confirm it actually fails when the behavior is removed. That caught a weak one:
asserting the array branch with a single-element array passes either way, since
`['x'].toString() === 'x'` — it now uses two entries and asserts `$openSchemeId`.

Tests only; no production code changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rjvelazco
rjvelazco marked this pull request as ready for review August 4, 2026 20:53
@rjvelazco
rjvelazco enabled auto-merge August 4, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant