Skip to content

feat(content-drive): gate quick actions on the workflow default action mapping - #37075

Draft
rjvelazco wants to merge 4 commits into
mainfrom
spike/content-drive-quick-actions-workflow-gated-v2
Draft

feat(content-drive): gate quick actions on the workflow default action mapping#37075
rjvelazco wants to merge 4 commits into
mainfrom
spike/content-drive-quick-actions-workflow-gated-v2

Conversation

@rjvelazco

Copy link
Copy Markdown
Member

Draft / spike — not for merge as-is. Route 2 of two options for how Content Drive's Action Center should relate to workflow rules. Route 1 is #37073. Opened as a draft so both can be tested side by side. No linked issue yet.

Route 1 (#37073) deletes the state actions. This one keeps them and gates them.

The problem

A quick action fires POST /v1/workflow/actions/default/fire/{systemAction}. The backend resolves that through the scheme's Default Actions mapping — and when nothing resolves, SystemActionApiFireCommandFactory performs the raw API operation instead: the content changes state, the workflow never moves, no actionlet runs, no history is written.

Until now the dialog offered those rows anyway, filtered only on content state. The file said so itself:

{ id: PUBLISH, eligibleWhen: (item) => !item.live && !item.archived }
// "UX filter only… Server still allows it."

So a content type with a strict Draft → QA → Publish scheme and no PUBLISH mapping still showed Publish, and the backend still published it. That is the bypass this PR closes.

What changed

Quick actions are now narrowed per contentlet to the content types whose workflow actually maps the system action.

The gate is the backend's own rule, in two parts

Resolution mirrors WorkflowAPIImpl#findActionMappedBySystemActionContentlet:

Gate Question Source
1 — mapped Does a mapping resolve? GET /contenttypes/{var}/system/actions + GET /schemes/{id}/system/actions. Content-type mappings win outright; otherwise scheme mappings, with the System Workflow breaking ties between schemes.
2 — reachable Can the selection run it now? The getBulkActions response the dialog already fetches. The mapped action id must appear with a non-zero count.

Gate 2 is the free one. The backend also requires the action to be available from the contentlet's current step and permitted for the user (isActionAvailable), which no client can compute. The bulk lookup is the server's own answer to "what can these rows run right now", so it stands in.

Why both halves of gate 1 are needed

systemActionMappings is already on the content type payload — and it is the wrong data on its own. ContentTypesPaginator builds it from:

select * from workflow_action_mappings where scheme_or_content_type in (%s)  -- content type variables

Content-type-level only. Task05175AssignDefaultActionsToTheSystemWorkflow ships the System Workflow mapped at scheme level and runs with forceRun() == true, so on a stock install almost no content type carries an override. Gating on that field alone would report "nothing mapped" for practically everything — precisely backwards.

Narrowing is per contentlet, not all-or-nothing

On a mixed selection where PUBLISH is mapped for Blog but not for Banner, Publish fires on the Blogs and marks the Banners excluded. Refusing the whole selection would be the safer-looking choice and the less useful one: the Blogs can be published perfectly legitimately, and a row that just says "no" gives the user nothing to act on.

Lock, Unlock and Add to Bundle are exempt — and sit apart

They have no mapping to gate on. Lock/Unlock are per-user state on the version info with no actionlet (WorkflowAPI.SystemAction documents that mapping them has no effect); Add to Bundle is not a SystemAction and posts to the legacy bundle servlet.

They now render under their own Always available heading. Left in one list they would make the section heading's claim false for three of its rows on day one.

Fails closed

Gated rows stay disabled while the lookup is pending and if it errors. Treating "not known yet" as permissive would flash a live Publish for the moment before the answer lands — the exact bypass being closed. A mapping failure does not break the dialog: workflow actions are unaffected and still usable.

The row says which problem it has

Same greyed row, entirely different fix:

  • "Doesn't apply to any of the selected items" → change the selection
  • "None of the selected content types map this action to a workflow action" → change the scheme, or ask someone who can
  • Partly mapped → the row stays live, with a lock icon and a count of what was excluded

Info icon

Added on the Quick Actions heading with the agreed copy — "Quick actions only run when mapped to a workflow action — this prevents bypassing your workflow rules." The link is a placeholder (marked TODO in the source) pointing at the current workflow docs until the Default Actions page exists, so it is never dead but does not yet land on the right anchor.

Cost, and the backend follow-up

One request per content type plus one per distinct scheme. Fine for the typical one-or-two-type selection, and it means this MVP is not blocked on backend work — both endpoints already exist.

It is still client-side duplication of what the backend resolves in a single query. A GET returning the effective mapping per content type — precedence and availability already applied — would collapse it to one call and remove the risk of the two implementations drifting. loadSystemActionMappings is the shape to replace when that lands; the comment there says so.

Testing

  • pnpm nx test portlets-content-drive1267 passed, 32 suites
  • pnpm nx test data-access757 passed
  • pnpm nx run-many -t lint -p portlets-content-drive data-access — clean
  • pnpm nx format:check --base=main — clean

New coverage: precedence (scheme-only, content-type override, override-whose-action-is-unavailable does not fall back, System Workflow tiebreak), gate 2 rejection, per-content-type narrowing on a mixed selection, the exemptions, pending and error states, the group split, and the info link.

nx affected also flags block-editor / dotcms-block-editor as failing (37 tests, plus lint errors). Verified identical on clean main — pre-existing, pulled in only because data-access changed.

Not done here

Manual QA against a real instance. The gate's entire value is that it behaves correctly against real mapping data, so this is the testing that matters most. Worth covering:

  • a content type on a custom scheme with no PUBLISH mapping → Publish disabled, hint names the mapping
  • the same content type after mapping PUBLISH → Publish live
  • a mixed selection where only one type is mapped → partial count plus the exclusion marker
  • content sitting on a step the mapped action is not reachable from → still disabled (gate 2)
  • Lock / Unlock / Add to Bundle unaffected throughout

Comparison with #37073

#37073 (route 1) This PR (route 2)
Publish/Unpublish/Archive/Unarchive/Delete removed from Quick Actions kept, gated on the mapping
Reaching them Workflow Actions section only either, when mapped
Closes the bypass yes, by removal yes, by gating
New requests per open none 1 per content type + 1 per scheme
Diff size −298 / +285 +903 / −82
Backend follow-up none optional: resolved-mapping endpoint

🤖 Generated with Claude Code

A quick action fires `POST /v1/workflow/actions/default/fire/{systemAction}`,
which the backend resolves through the scheme's Default Actions mapping. When
nothing resolves, `SystemActionApiFireCommandFactory` performs the raw API
operation instead: the content changes state, the workflow never moves, no
actionlet runs. Until now the dialog offered those rows anyway, filtered only
on content state — its own comment admitted the filter was cosmetic ("UX
filter only... Server still allows it"). A content type with a strict
Draft/QA/Publish scheme and no PUBLISH mapping still showed Publish, and the
backend still published.

Quick actions are now narrowed per contentlet to the content types whose
workflow actually maps the system action.

Resolution mirrors `findActionMappedBySystemActionContentlet` and is a
two-gate rule:

- Gate 1, mapped: content-type mappings win outright, else the scheme
  mappings, with the System Workflow breaking ties between schemes. Both
  halves matter — most installs carry no content-type-level mapping at all,
  so the `systemActionMappings` already on the content type payload would
  report nothing mapped for practically everything.
- Gate 2, reachable: the mapped action must appear in the bulk-actions lookup
  the dialog already runs, with a non-zero count. That is the server's own
  answer to what these rows can run now, standing in for `isActionAvailable`,
  which no client can compute.

Narrowing is per contentlet rather than all-or-nothing: on a mixed selection
where PUBLISH is mapped for Blog but not Banner, Publish fires on the Blogs
and reports the Banners as excluded. Refusing the whole selection would be
the safer-looking choice and the less useful one.

Lock, Unlock and Add to Bundle are exempt and sit under their own heading.
The first two are per-user version-info state with no actionlet and can never
be mapped; the third is not a SystemAction. Left in one list they would make
the section heading's claim false for three of its rows.

Fails closed throughout: rows stay disabled while the lookup is pending and
if it errors, and the row hint distinguishes "doesn't apply to your
selection" from "your workflow doesn't map this" — same greyed row, entirely
different fix.

Costs one request per content type plus one per scheme. A backend endpoint
returning the *effective* mapping per content type, precedence and
availability already applied, would collapse that to one call and remove the
risk of the two implementations drifting.

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

Copy link
Copy Markdown
Contributor

❌ Issue Linking Required

This PR could not be linked to an issue. All PRs must be linked to an issue for tracking purposes.

How to fix this:

Option 1: Add keyword to PR body (Recommended - auto-removes this comment)
Edit this PR description and add one of these lines:

  • Fixes #123 or Closes #123 — same-repo issue
  • Fixes org/repo#123 or Closes org/repo#123 — cross-repo or private issue
  • Closes https://github.com/org/repo/issues/123 — full GitHub URL
  • Other supported keywords: fix, fixed, close, closed, resolve, resolved

Option 2: Link via GitHub UI (Note: won't clear the failed check)

  1. Go to the PR → Development section (right sidebar)

  2. Click "Link issue" and select an existing issue

  3. Push a new commit or re-run the workflow to clear the failed check
    Option 3: Use branch naming
    Create a new branch with one of these patterns:

  • 123-feature-description (number at start)

  • issue-123-feature-description (issue-number at start)

  • feature-issue-123 (issue-number anywhere)

Why is this required?

Issue linking ensures proper tracking, documentation, and helps maintain project history. It connects your code changes to the problem they solve.
---This comment was automatically generated by the issue linking workflow

@github-actions github-actions Bot added the Area : Frontend PR changes Angular/TypeScript frontend code label Aug 14, 2026
@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Code Review — Content Drive quick actions workflow gate

Reviewed the diff against origin/main. The two-gate resolution (resolvedSystemActions), the per-contentlet narrowing, the fail-closed handling, and the fire path (fired inodes come from eligibleInodes, so unmapped items are correctly excluded) all look sound. WORKFLOW_ACTION_ID values (PUBLISH, ARCHIVE, …) match the backend SystemAction names, so keying the mapping set by system-action name is correct. The empty-forkJoin gotcha is avoided: results is guaranteed non-empty (early return in loadWorkflowActions) and the schemes branch is guarded with of([]).

A few things worth a look before merge — none blocking.

New Issues

  • 🟡 Medium: dot-content-drive-action-center.component.ts:1017 (loadWorkflowActions) / :1090 (loadSystemActionMappings finalize) — The finalize(() => this.$loadingSchemes.set(false)) was moved off the bulk-actions call and onto the mapping fan-out. $loadingSchemes also gates the Workflow Actions section (.html:105), which is functionally independent of the mapping. Result: on the happy path the Workflow Actions panel now stays in its skeleton state until the extra 1-per-content-type + 1-per-scheme requests finish, even though its data ($schemes) was ready at bulk-lookup completion. The PR notes workflow actions still render on mapping failure (true — finalize runs, $schemesError stays false), but doesn't account for this added latency on success. Consider driving the quick-action gate off $mappedSystemActions === undefined and letting $loadingSchemes flip as soon as the schemes resolve, so the two sections stay decoupled. Fix this →

  • 🟡 Medium: dot-content-drive-action-center.component.ts:1058 — Scheme mappings are pooled across the whole selection (schemes.flat()) and then fed to resolvedSystemActions for every content type. A scheme that belongs only to content type Y therefore contributes its mappings when resolving content type X. Today gate 2 (availableWorkflowActionIds, per-content-type) rescues this — X's view won't contain Y's scheme's action ids, so the stray mapping is filtered out — and workflow-action ids are UUIDs so they can't collide. So it's correct now, but the isolation is accidental: the precedence/System-Workflow tiebreak in resolvedSystemActions is computed over a cross-contaminated pool rather than per content type. Assumption: action ids are globally unique across schemes and gate 2 always runs. What to verify: a mixed selection where type X (custom scheme, no PUBLISH mapping) and type Y (scheme that maps PUBLISH) are selected together — confirm Publish stays unmapped for X's items.

  • 🟡 Medium: utils/action-center.ts:190 (isSystemWorkflowMapping) — The System-Workflow tiebreak relies on mapping.workflowAction.schemeId being populated in the GET /schemes/{id}/system/actions and /contenttypes/{var}/system/actions responses. If either endpoint omits schemeId on the nested workflowAction, sortSystemWorkflowFirst silently no-ops and the tiebreak reverts to insertion order. Assumption: both endpoints serialize workflowAction.schemeId. What to verify: inspect a real payload and confirm schemeId is present; this only matters when two schemes on one content type map the same system action, so it's an edge case, but the failure is silent.

No correctness bugs found in the fire path, the fail-closed gating, or the i18n keys (all four new keys — not-mapped, partly-mapped, mapping-lookup-failed, gate-explainer — are added to Language.properties and referenced correctly). The {0} placeholder in partly-mapped is fed by quickAction.unmappedCount, which is passed to every hint via the template's | dm: [...] and ignored by the other keys.

View job run · spike/content-drive-quick-actions-workflow-gated-v2

…gatable rows

Two fixes from testing.

**One list, ungatable rows first.** The gated and exempt rows were split into
two groups under separate headings. They are back in a single list, with Lock,
Unlock and Add to Bundle leading it — the three rows whose availability no
mapping can change, so the top of the list stays put as the selection and
content types move underneath it. The info icon on the section heading carries
the explanation instead of a second heading.

**Gated rows no longer filter on row state.** Publish was hidden for live
content, Delete shown only for archived, and so on. Those filters assumed a
system action's only effect is the state change in its label, and a mapping
makes that false: `PUBLISH` mapped to "Send for Review" moves the workflow
step, assigns a reviewer and runs that action's actionlets, all of which is
worth doing to a live item. What a gated row means is now decided by the
mapping alone, so the mapping alone decides whether it appears — a gated row is
offered wherever it is mapped, over every mapped item in the selection.

The exempt rows keep their filters, because there the label really is the whole
effect: Unlock on unlocked content has nothing to do, and no mapping can change
that.

`eligibleWhen` is optional now; omitting it means "every contentlet", which is
what all five gated rows do.

Two notes for manual QA, where the server is stricter than the row is:

- `ESContentletAPIImpl` refuses to delete unarchived content that has more than
  one language version. Those rows now reach the fire and come back as per-item
  failures in the result toast rather than being filtered out up front.
- Archive keeps its danger styling whatever it maps to. When the mapped action
  carries no archive actionlet, `SystemActionApiFireCommandFactory` forces the
  archive anyway, so the destructive intent holds even where the mapping does
  not.

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

Copy link
Copy Markdown
Member Author

Pushed 48b2b82 — two fixes from testing.

1. One list, ungatable rows lead

The two-group split is gone. Lock, Unlock and Add to Bundle now head a single list, followed by the gated rows. They are the three whose availability no mapping can change, so the top of the list stays put while everything below it moves with the selection and the content types. The info icon on the section heading carries the explanation instead of a second heading.

2. Gated rows no longer filter on row state

Publish was hidden for live content, Delete shown only for archived, and so on. Those filters assumed a system action's only effect is the state change named in its label — and a mapping makes that false. PUBLISH mapped to Send for Review moves the workflow step, assigns a reviewer and runs that action's actionlets, all of which is worth doing to a live item.

A gated row is now offered wherever it is mapped, over every mapped item in the selection. eligibleWhen became optional; omitting it means "every contentlet", which is what all five gated rows do.

The exempt rows keep their filters — there the label really is the whole effect, and no mapping can change that Unlock on unlocked content has nothing to do.

One correction to the reasoning, worth recording

The premise was that Publish might not publish. It always does: when the mapped action carries no publish actionlet, PublishSystemActionApiFireCommandImpl forces contentletAPI.publish() as well, so the content ends up live either way.

The filter was still wrong, for a different reason — publishing is no longer the only effect, so "already live" stopped meaning "nothing to do". Same conclusion, different mechanism. Archive keeps its danger styling on the same logic: the forced fallback means the destructive intent holds even where the mapping does not carry it.

Two places the server is stricter than the row now is

  • ESContentletAPIImpl refuses to delete unarchived content with more than one language version. Those rows now reach the fire and come back as per-item failures in the result toast rather than being filtered out up front. Worth watching in QA.
  • Gate 2 aggregates per content type, not per contentlet, so a contentlet on a step the mapped action is unreachable from can still be included if a sibling of the same type is on a step where it is reachable. Same imprecision the Workflow Actions section already has, which is what its partial-match warning is for.

Tests: 1269 passed, 32 suites. Lint and format:check clean.

The excluded-items lock icon is gone. It put a second tooltip on the same row
as the row's own, and the two overlapped each other on hover — visible in
testing. The row tooltip already names the count via
`content-drive.action-center.partly-mapped`, so `unmappedCount` still does its
job; only the extra marker goes. Its message key
(`content-drive.action-center.unmapped-items`) goes with it, having never
shipped.

The docs info link beside the Quick Actions heading picked up the default
anchor underline, which read as a typo under an uppercase label. Now
`no-underline`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`no-underline` alone lost to a higher-specificity global anchor style, so the
underline was still rendering. Uses the Tailwind 4 suffix form
(`no-underline!`), matching `text-[20px]!` elsewhere in this template.

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

Copy link
Copy Markdown
Member Author
video.mov

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