Skip to content

feat(content-drive): add workflow filter (scheme → step) to toolbar #35880#35906

Open
zJaaal wants to merge 28 commits into
mainfrom
issue-35880-content-drive-workflow-filter
Open

feat(content-drive): add workflow filter (scheme → step) to toolbar #35880#35906
zJaaal wants to merge 28 commits into
mainfrom
issue-35880-content-drive-workflow-filter

Conversation

@zJaaal
Copy link
Copy Markdown
Member

@zJaaal zJaaal commented Jun 2, 2026

Proposed Changes

Frontend and backend for the Content Drive Workflow filter (scheme → optional step), implemented together in this PR. Closes #35880 (frontend) and #35470 (backend — consolidated here).

Frontend (#35880)

  • Workflow filter in the Content Drive toolbar — a two-column popover (schemes → steps) mirroring the content-type filter.
  • Multi-select schemes, single step per scheme: each selected scheme can pin one step; pinning a step cascades to select its scheme, deselecting a scheme drops its pinned step.
  • Selection persists as a single structured filter key workflow: string[] (entries are schemeId or schemeId:stepId), so it round-trips through the URL via the existing encode/decode machinery with no step lookups.
  • Scheme source follows the content-type selection (all schemes / schemes for the selected content types); steps load on demand and are cached per scheme.
  • Empty-schemes state shows a dedicated, i18n'd message (singular/plural by selected content-type count); empty/loading states are centered.
  • New DotWorkflowService methods: getSchemesByContentTypes() and getSteps().

Backend (#35470) — DB-first

  • POST /api/v1/drive/search now honors an optional workflow: [{ scheme, step? }] field.
  • Filter lives in the shared SQL builder (BrowserAPIImpl.selectQueryappendWorkflowQuery), keeping the drive's DB-first hybrid intact — no forced Elasticsearch path.
  • Scheme-only entry → matches by content-type assignment (workflow_scheme_x_structure), so imported / push-published / never-actioned content (no workflow_task) still appears under its scheme.
  • Step-pinned entry → matches the contentlet's current task (workflow_task.status). Entries combine with OR. Folders are suppressed when a workflow filter is active.
  • New immutable WorkflowFilterForm on DriveRequestForm; BrowserQuery.withWorkflowSchemeIds / withWorkflowStepIds. All ids are bound as ? parameters (no string concatenation).
  • FE $request wires the stored workflow tokens into the drive-search request.

Chore

  • chore(vscode): add MCP server config (primeng, nx, angular-cli).

Checklist

  • Tests
    • FE: workflow-filter component spec + workflow encode/decode round-trip in functions.spec.ts + store empty-result pagination test
    • BE: ContentDriveWorkflowFilterTest integration test — scheme filter (incl. task-less content), step filter (current task), OR semantics, no-filter regression, folder suppression
  • Translations — content-drive.workflow-filter.* keys added to Language.properties
  • Security Implications Contemplated — read-only filter; all workflow ids bound as SQL parameters (no concatenation); MCP config contains no secrets

Additional Info

The backend ticket #35470 has been consolidated into this PR (and its design folded into #35880); it is closed in favor of #35906. The request contract there matches the structured workflow field used here.

🤖 Generated with Claude Code

zJaaal and others added 3 commits June 1, 2026 17:55
…35880

Add a two-column popover filter mirroring the content-type filter: workflow
schemes on the left, steps of the focused scheme on the right. Single-select
on both columns, stored as a one-element array so a future multi-select
migration is trivial.

- DotWorkflowService: getSchemesByContentTypes() and getSteps()
- Scheme source follows the content-type selection (all schemes / schemes for
  selected content types); steps loaded on demand and cached per scheme
- Selection persists via the store filters (workflowScheme/workflowStep),
  round-trips in the URL, drives the chip and Clear-all
- Reconciles the selected scheme on content-type change (kept if still present,
  else cleared)

First draft. Backend drive-search support is tracked in #35470 — until it lands
the filter is UI-complete but does not yet narrow listing results. Tests
deferred pending manual verification.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…er scheme #35880

Reshape the workflow filter selection model and lock it down with tests:

- Schemes are multi-select; each selected scheme can pin one step (single per
  scheme). Pinning a step cascades to select its scheme; deselecting a scheme
  drops its pinned step.
- Persist the selection as one structured filter key `workflow: string[]`, where
  each entry is `schemeId` or `schemeId:stepId`. Co-locating the step with its
  scheme keeps the pairing explicit, so reconcile and URL round-trip need no
  step lookups. Registered with the existing multiSelector decoder.
- Empty-schemes state now shows a dedicated, i18n'd message (singular/plural by
  selected content-type count) instead of reusing the step column's copy.
- Center the empty/loading states; render them as flex siblings of the listbox.
- Tests: component spec (loading, chip, selection, per-scheme step, empty copy,
  listbox config) and a workflow encode/decode round-trip in functions.spec.

Backend search support remains tracked in #35470.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Configure VS Code MCP servers for the workspace so the editor's AI tooling can
reach the PrimeNG, Nx, and Angular CLI MCP servers via npx.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Jun 2, 2026

Claude finished @zJaaal's task in 6m 53s —— View job


PR Review

  • Gathering context and reading changed files
  • Reviewing backend changes (Java)
  • Reviewing frontend changes (TypeScript/Angular)
  • Final assessment

Issues Found

Backend

  1. BrowserQuery.Builder(BrowserQuery) copy constructor does not copy workflowSchemeIds/workflowStepIds (BrowserQuery.java:285-318). If any caller ever uses BrowserQuery.from(existing) to clone and modify a query that already has a workflow filter, the workflow fields silently fall back to empty LinkedHashSets. There is no call site that does this today (no BrowserQuery.from() usage exists in the codebase), but it is a silent trap for future code and inconsistent with every other field in the same copy constructor. Fix this →

  2. workflow_task.status holds a step UUID, not a step name. BrowserAPIImpl.appendWorkflowQuery joins workflow_task.status in (?) and the step IDs come from WorkflowFilterForm::step. This relies on workflow_task.status being a step ID, not a step name — this appears to be correct in dotCMS, but it's worth confirming against the schema, as status is an ambiguous column name that could mislead future maintainers. No fix needed if confirmed, but a brief inline comment would help.

  3. Scheme-only filter uses workflow_scheme_x_structure.structure_id = struc.inode — this matches the content type, not the actual contentlet. A contentlet of a type governed by scheme A but reassigned away (if content-type reassignment is possible) could appear or not appear erroneously. This is an inherent limitation of the "scheme-only = assignment-based" design that the PR documents, but it is not tested as a negative case.

  4. parseWorkflowFilter passes content-type IDs (variable names) to getSchemesByContentTypes — the frontend getSchemesByContentTypes passes contentTypeIds (the raw filter value, which may be variable names like "Blog" rather than UUIDs). The WorkflowResource.getWorkflowSchemesByContentTypeList endpoint resolves either inodes or variable names, so this is fine, but it's worth noting explicitly.

Frontend

  1. getSchemesByContentTypes passes content type IDs (possibly variable names) from the filter — the #loadSchemes call reads rawContentTypes directly from getFilterValue('contentType') which is the encoded filter array (could be variable names or IDs). The backend handles both, but if content type filters are stored as variable names (as the store spec implies — contentType: ['Blog']), then the passed value matches by variable name via the WorkflowResource endpoint's normalization. This is fine but not explicitly tested.

  2. $selection is a linkedSignal that recomputes from the store filter. When #reconcileSelection calls this.$selection.set(kept) and then this.#syncStore(), the #syncStore writes back to the store, which re-triggers the linkedSignal computation — but linkedSignal only recomputes when its source changes, so this is fine. However, the pattern is subtle enough that a future developer could easily break it by changing the signal type. Not a bug, but worth noting.

  3. Step loading error in #focusScheme uses of([]) as the fallback, which then calls #cacheSteps(schemeId, []). An empty step list gets cached, so if the user navigates away and back to the same scheme, the component treats "empty" as loaded and never retries — the error is permanent for that session. The review discussion flagged the scheme-loading path (comment by oidacra on 2026-06-03) and it was fixed, but the step-loading error path in #focusScheme has the same problem: of([]) gets cached at line 385. Should probably not cache on error (return EMPTY and skip #cacheSteps in the error case). Fix this →

  4. URL encode/decode collision risk: encodeFilters joins workflow tokens with ,, and each token is schemeId[:stepId]. UUIDs do not contain commas or semicolons, so this is safe in practice. But if a non-UUID scheme/step ID were ever used, a comma in the ID would silently corrupt the encoding. No current risk, but worth a comment in the multiSelector / workflow decode path.

  5. decodeFilters processes workflow:schemeA:stepX,schemeB by splitting schemeA:stepX,schemeB at commas first (via multiSelector), yielding ['schemeA:stepX', 'schemeB']. This is correct. The encodeFilters path joins the array with commas. The round-trip test verifies this works. However, note that the URL segment workflow:schemeA:stepX,schemeB contains bare colons and commas — if the URL is ever percent-encoded by a router, the whole string survives only if the router doesn't encode those characters. The existing content-type filter has the same exposure, so this is consistent behavior.

Chore

  1. .vscode/mcp.json (root) is added in this PR. MCP server configs can contain sensitive information. This file contains no credentials, only localhost/port references — this is fine, but the file should likely be in .gitignore (workspace-local tool config), or at minimum the PR author should confirm the team intends to commit this globally for all contributors.

Summary

The core logic is sound: SQL is parameterized, sanitization via SQLUtil.sanitizeParameter is applied, the scheme-only vs. step-pinned split is clear, and the copy constructor omission (item 1) is the only correctness issue worth fixing before merge. Item 7 (step error caching) is a UX defect but not a data integrity issue. The rest are minor notes.

@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code labels Jun 2, 2026
@zJaaal zJaaal linked an issue Jun 2, 2026 that may be closed by this pull request
11 tasks
@zJaaal zJaaal marked this pull request as draft June 2, 2026 18:42
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Avoid a duplicate getSteps request in #reconcileSelection by excluding the
  newly-focused scheme (#focusScheme already loads it) from #ensureStepsLoaded.
- Add a distinct empty-state message for the no-content-type-selected case
  (`no-schemes`); gate the singular/plural "no workflows" copy behind count > 0.
- Guard #loadSchemes against stale responses with a request-id token so a late
  reply from a superseded content-type change can't overwrite a newer one.
- Defensively short-circuit getSchemesByContentTypes on an empty id list.
- Tests: no-content-type empty copy, step deselect (ngModelChange null), and the
  stale getSteps late-response guard; re-establish mock defaults per test to
  prevent of([]) return-value leakage across cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…arch #35470

Implements the backend (#35470) for the Content Drive workflow filter (#35880),
filtering in the SQL layer — no forced Elasticsearch path.

- BrowserQuery: workflowSchemeIds (scheme-only) + workflowStepIds (pinned steps)
  with withWorkflowSchemeIds/withWorkflowStepIds builders.
- BrowserAPIImpl.selectQuery: appendWorkflowQuery emits an OR'd EXISTS block —
  scheme-only entries match by content-type assignment
  (workflow_scheme_x_structure) so imported / push-published content with no
  workflow_task still appears; step-pinned entries match the current task
  (workflow_task.status). All ids bound as parameters. Shared selectQuery means
  both drive/search and /v1/browser are covered.
- DriveRequestForm: structured `workflow: [{ scheme, step? }]` field via a nested
  immutable WorkflowFilterForm; ContentDriveHelper splits entries into the two
  sets and suppresses folders when a workflow filter is active.
- FE: DotContentDriveSearchRequest.workflow field; store $request derives it from
  the structured `workflow` filter tokens (parseWorkflowFilter) and drops folders
  when present.

openapi.yaml unchanged — the drive search endpoint is @hidden. Integration tests
to follow once verified against a running instance.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
zJaaal and others added 3 commits June 3, 2026 13:33
…lts #35880

When a search lands on cursors matching an already-seen page, loadItems reused
the existing pages array verbatim — so an empty result that coincided with
DEFAULT_PAGE's 0,0 cursors kept its optimistic hasMoreContent: true, and the
paginator's $totalItems reported limit*2, enabling "next" on zero items.

Refresh the matched page's hasMore flags from the actual response (new array ref
so $totalItems recomputes). Fixes any empty filter, not just workflow.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover the DB-first workflow filter semantics in ContentDriveHelper#driveSearch:
- scheme-only entry matches by content-type assignment (workflow_scheme_x_structure),
  including task-less / never-actioned content
- step-pinned entry matches the current workflow_task only
- OR semantics across entries
- no-filter regression guard
- folder suppression when a workflow filter is active

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@oidacra
Copy link
Copy Markdown
Member

oidacra commented Jun 5, 2026

This PR removes $query / buildContentDriveQuery / escapeLuceneSpecialChars / BASE_QUERY. Per git history (2d7eb3e6, #35503) that $query computed was intentionally kept with an inline note tying it to #33338 ("Enable select all checkbox"), which is still open. The removal looks like reasonable cleanup if select-all is now going server-side via the new $request pipeline — could you confirm that's the plan (and ideally note it on #33338, or close it), so whoever picks that issue up later doesn't have to reconstruct the Lucene-query infra from history?

Comment thread core-web/libs/data-access/src/lib/dot-workflow/dot-workflow.service.ts Outdated
@zJaaal
Copy link
Copy Markdown
Member Author

zJaaal commented Jun 5, 2026

This PR removes $query / buildContentDriveQuery / escapeLuceneSpecialChars / BASE_QUERY. Per git history (2d7eb3e6, #35503) that $query computed was intentionally kept with an inline note tying it to #33338 ("Enable select all checkbox"), which is still open. The removal looks like reasonable cleanup if select-all is now going server-side via the new $request pipeline — could you confirm that's the plan (and ideally note it on #33338, or close it), so whoever picks that issue up later doesn't have to reconstruct the Lucene-query infra from history?

The removal was intentional

…rializers #35880

Cover parseWorkflowToken / workflowEntryToToken / parseWorkflowFilter directly
(previously only exercised via the URL encode/decode round-trip): empty token,
leading separator, step id containing colons (first-colon split), scheme-only
vs scheme+step serialize, and a parse<->serialize round-trip.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…35880

Adds the AND-semantics case the PR review flagged: a content-type filter and a
workflow scheme filter active together return the intersection (type A at
scheme A), and type C is excluded by the content-type clause despite having its
own scheme. Promotes typeA/typeC to fields so the new case can reference them.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…uite3a #35880

Address PR review (fabrizzio): integration tests only run in CI when listed in
a suite. Add ContentDriveWorkflowFilterTest alongside the sibling
ContentDriveHelperContentletAPIComparisonTest in MainSuite3a.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zJaaal zJaaal requested a review from fabrizzio-dotCMS June 5, 2026 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[TASK] Content Drive: Add Workflow filter (Scheme → Step) to the toolbar

4 participants