From da24191afee277d52a650f927e89f226ae647757 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 10:51:24 -0300 Subject: [PATCH 01/16] docs(content-drive): add spec for Status filter (#37066) Spec-Kit PR 1 for #37066: the Archived / Unpublished / Locked filter on drive search and in the Content Drive toolbar. Carries spec.md alone, per the two-PR spec-driven flow. The plan artifacts land in the next PR of the stack, based on this branch. Co-Authored-By: Claude Opus 5 (1M context) --- .specify/feature.json | 1 + .../37066-content-drive-status-filter/spec.md | 276 ++++++++++++++++++ 2 files changed, 277 insertions(+) create mode 100644 .specify/feature.json create mode 100644 specs/37066-content-drive-status-filter/spec.md diff --git a/.specify/feature.json b/.specify/feature.json new file mode 100644 index 000000000000..4081e8f69aec --- /dev/null +++ b/.specify/feature.json @@ -0,0 +1 @@ +{"feature_directory":"specs/37066-content-drive-status-filter"} diff --git a/specs/37066-content-drive-status-filter/spec.md b/specs/37066-content-drive-status-filter/spec.md new file mode 100644 index 000000000000..23cc81468dc0 --- /dev/null +++ b/specs/37066-content-drive-status-filter/spec.md @@ -0,0 +1,276 @@ +# Feature Specification: Content Drive Status Filter + +**Feature Branch**: `issue-37066-content-drive-status-filter` + +**Created**: 2026-08-24 + +**Status**: Draft + +**Type**: New Feature (Task) + +**GitHub Issue**: [dotCMS/core#37066](https://github.com/dotCMS/core/issues/37066) (absorbed [#37067](https://github.com/dotCMS/core/issues/37067); parent epic [#33999](https://github.com/dotCMS/core/issues/33999)) + +**Input**: User description: "Content Drive needs a Status filter (Archived, Unpublished, Locked): the search clauses on the drive search endpoint and the multiselect in the toolbar. None of the three predicates is expressible on the endpoint today. Selecting several narrows the results (AND), because they are independent states an item can hold at the same time. Nothing selected must mean exactly today's behavior: archived hidden, everything else returned." + +--- + +## Scope Note *(read this first)* + +This is **one vertical slice**: the search capability and the control that drives it ship together. +The ticket originally split them across two issues; #37067 was merged into #37066 because each half +restated the same contract and duplicated contract text is where drift starts. + +The Status filter is deliberately **not** the same shape as the existing Content Drive filters. +Content type, base type and language are single-valued attributes, so selecting several is an OR +("either of these types"). Status is a set of independent flags a single item can hold at once, so +selecting several is an AND ("both unpublished *and* locked"). This difference is the reason the +control is a multiselect rather than a dropdown, and it is the single most important thing for a +reader to carry into planning. + +--- + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - An editor finds content that was archived (Priority: P1) + +An editor is looking for a page a colleague archived last week, to check what it said before +deciding whether to restore it. Content Drive hides archived content by default, so today the only +way to see it is to leave Content Drive for the legacy Content Search portlet. The editor selects +**Archived** in the Status filter and the drive lists archived items — and only archived items. + +**Why this priority**: This is the capability people currently leave Content Drive to get. It is +also the only one of the three that is *partially* present today in a form that does the wrong +thing (a flag that returns archived content **plus** everything else), so shipping it correctly is +what makes the filter trustworthy. + +**Independent Test**: Archive one item in a folder that also holds live and draft items. Select +Archived. The result set contains the archived item and nothing else. + +**Acceptance Scenarios**: + +1. **Given** a folder holding live, draft and archived items, **When** the editor selects Archived, + **Then** only the archived items are listed. +2. **Given** Archived is selected, **When** the editor clears it, **Then** the drive returns to + hiding archived content, exactly as before the filter existed. +3. **Given** Archived is selected, **When** the editor also types a keyword in the search box, + **Then** the results are archived items matching that keyword — the two filters narrow together. + +--- + +### User Story 2 - An editor reviews what is not live yet (Priority: P1) + +Before a release an editor wants to see everything in a section that has never been published or +whose published version has been taken down. They select **Unpublished** and the drive lists content +with no live version. Archived items do not appear: archived content is a separate question, and an +editor auditing drafts is not asking about the recycle bin. + +**Why this priority**: "What is not live?" is the most common pre-release question content teams +ask, and Content Drive cannot express it at all today. The existing live/working switch answers a +different question (show me the live version) and is not the inverse of this one. + +**Independent Test**: Create a never-published item, publish a second, archive a third. Select +Unpublished. Only the never-published item is listed. + +**Acceptance Scenarios**: + +1. **Given** a folder with published, unpublished and archived items, **When** the editor selects + Unpublished, **Then** only the unpublished, non-archived items are listed. +2. **Given** an item that was published and then unpublished, **When** the editor selects + Unpublished, **Then** that item is listed. +3. **Given** Unpublished is selected, **When** the editor also selects Archived, **Then** archived + items are admitted to the results (see Edge Cases for why this pair reads as it does). + +--- + +### User Story 3 - A manager finds content someone has checked out (Priority: P2) + +A content manager notices work is stalled and wants to see everything currently locked. They select +**Locked** and the drive lists items with a lock held, whoever holds it, so the manager can chase +the owner or unlock the item. + +**Why this priority**: Real and frequently asked, but it is a supervisory question rather than part +of the daily editing loop, and there is a workaround today (open items one at a time and look at the +lock indicator). It also has no partial implementation to correct, so it is pure addition. + +**Independent Test**: Lock one item in a folder of otherwise unlocked items. Select Locked. Only the +locked item is listed. + +**Acceptance Scenarios**: + +1. **Given** a folder with one locked item, **When** the manager selects Locked, **Then** only that + item is listed. +2. **Given** the lock is released, **When** the manager reloads the filtered view, **Then** the item + no longer appears. + +--- + +### User Story 4 - Combining statuses to ask a sharper question (Priority: P2) + +A manager wants "drafts currently checked out by someone" — work in progress that is both blocked and +unpublished. They select **Unpublished** and **Locked** together and the drive lists only content +that is both. + +**Why this priority**: This is the reason the control is a multiselect rather than a dropdown. A +single-select could not express it, and the combination is a question content teams actually ask. +It is P2 rather than P1 because it builds on stories 2 and 3 rather than standing alone. + +**Independent Test**: Create four items covering every unpublished/locked combination. Select both +statuses. Only the item that is both unpublished and locked is listed. + +**Acceptance Scenarios**: + +1. **Given** items covering all four unpublished/locked combinations, **When** both statuses are + selected, **Then** exactly the item holding both states is listed. +2. **Given** both statuses are selected, **When** one is cleared, **Then** the results widen to the + remaining status alone. + +--- + +### User Story 5 - A filtered view survives reload and can be shared (Priority: P3) + +An editor sends a colleague a link to "everything unpublished in Marketing". The colleague opens the +link and sees the same filtered view, with the Status selection shown as an active chip. Reloading +the page keeps it. Clearing all filters removes it along with everything else. + +**Why this priority**: Every other Content Drive filter behaves this way, so a Status filter that +did not would read as broken. It is P3 only because the filter is useful before it is shareable. + +**Independent Test**: Select two statuses, copy the address, open it in a new session. The same two +statuses are selected and the same results are listed. + +**Acceptance Scenarios**: + +1. **Given** a Status selection, **When** the page is reloaded, **Then** the selection and results + are unchanged. +2. **Given** a Status selection, **When** "Clear all" is used, **Then** the Status selection is + removed along with the other filters. +3. **Given** a Status selection, **When** the view is shared as a link, **Then** the recipient sees + the same selection. + +--- + +### Edge Cases + +- **Archived + Unpublished returns the same items as Archived alone.** Archiving an item removes its + live version, so every archived item is already unpublished. The pair is redundant, not broken. It + must not be presented as an error, an empty state, or a warning — it is simply a narrower question + whose answer happens to coincide with a wider one. +- **Archived + Locked is reachable but uncommon**, since it needs an item that was locked by its own + holder or archived by an administrator while a lock stood. An empty result there is legitimate and + shows the ordinary empty state, never an error. +- **Folders have no status.** Whenever any status is selected the results are content only. This + matches how the drive already behaves for the other narrowing filters. +- **No status selected** must produce exactly the behavior that exists today — archived content + hidden, everything else returned — with no change to result counts, ordering or pagination. +- **An unrecognized status value** submitted directly to the search endpoint is rejected with a + clear client error naming the accepted values, rather than being silently ignored (which would + return a wider result set than the caller asked for). +- **Status combined with a workflow filter.** Content Drive can already filter by workflow, including + steps that archive content. A status selection combined with such a workflow filter must return a + coherent result, not an empty one caused by two rules contradicting each other about archived + content. +- **Text search plus status.** The drive uses different search strategies depending on whether a + keyword is present and how the environment is configured. Every strategy must apply the status + filter identically, so the same selection never returns different results because of a + configuration the user cannot see. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The drive search capability MUST accept a set of content statuses drawn from + Archived, Unpublished and Locked, defaulting to an empty set. +- **FR-002**: An empty set MUST preserve today's behavior exactly: archived content excluded, all + other content returned. +- **FR-003**: Archived MUST return only archived content, never archived content in addition to + everything else. +- **FR-004**: Unpublished MUST return only content with no live version, and MUST exclude archived + content unless Archived is also selected. +- **FR-005**: Locked MUST return only content on which a lock is held, regardless of who holds it. +- **FR-006**: Multiple selected statuses MUST combine with AND — the result is content holding every + selected state at once. +- **FR-007**: Selecting Archived together with Unpublished MUST return the same set as Archived + alone, and this MUST be documented rather than treated as a defect. +- **FR-008**: The existing inclusive "show archived" behavior relied on by the legacy Site Browser + MUST be left unchanged; the exclusive Archived behavior is added alongside it. +- **FR-009**: A status selection MUST produce identical results whether or not a keyword search is + active, and under every supported search strategy the environment can be configured to use. +- **FR-010**: An unrecognized status value MUST be rejected with a client error, consistent with how + the drive already rejects unknown field-filter keys. +- **FR-011**: A status selection MUST NOT conflict with a workflow filter that also constrains + archived content; the two MUST combine into one coherent result set. +- **FR-012**: Users MUST be able to select any combination of the three statuses from a single + control in the Content Drive toolbar, alongside the existing filters. +- **FR-013**: The control's labels MUST be localizable, following the Content Drive naming + convention already used by the other filters. +- **FR-014**: The active selection MUST be reflected as a chip, consistent with the other toolbar + filters, and MUST be clearable from that chip. +- **FR-015**: Folders MUST be excluded from the results whenever any status is selected. +- **FR-016**: A status selection MUST round-trip through the address bar like every other filter, so + a filtered view is shareable and survives a reload. +- **FR-017**: The existing "Clear all" action MUST clear the status selection. +- **FR-018**: An empty result set arising from a legitimate combination MUST show the standard empty + state, never an error. +- **FR-019**: The Content Drive request MUST stop pinning archived content off unconditionally; that + decision MUST come from the status selection instead. +- **FR-020**: The control and each of its options MUST carry stable test identifiers, and the + control MUST carry an accessible label. + +### Key Entities + +- **Content Status**: An independent state an item can hold, from a closed set of three — *Archived* + (removed from circulation but recoverable), *Unpublished* (no live version), and *Locked* (checked + out by a user). An item may hold several at once, which is what makes the selection additive. +- **Status Selection**: The set of statuses the user has chosen. Empty by default; every member + narrows the result set further. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: An editor can locate archived content from within Content Drive in a single action, + without leaving for another part of the product. +- **SC-002**: Each single status returns exactly the items in that state and no others, verified + against a known fixture covering all three states plus unaffected content. +- **SC-003**: Every pair of statuses, and all three together, return exactly the items holding all + the selected states. +- **SC-004**: With no status selected, result counts and ordering are identical to those produced + before the filter existed, for the same folder and filters. +- **SC-005**: The same status selection returns the same result set with and without a keyword + search, and under every supported search strategy. +- **SC-006**: A shared link to a status-filtered view reproduces the same selection and the same + results for a second user. +- **SC-007**: No legitimate combination — including the redundant and the rarely-populated ones — + presents as an error to the user. + +## Legacy Considerations *(dotCMS-specific — mandatory)* + +- **Existing behavior touched**: The shared content-browsing capability behind both Content Drive + (modern) and the Site Browser (legacy). The legacy Site Browser exposes a "Show Archived" checkbox + whose meaning is *inclusive* — archived content **in addition to** everything else. The new + Archived status is *exclusive* — archived content **only**. These are different questions and both + must remain expressible; the new behavior is added alongside the old one rather than replacing it. + The legacy Content Search portlet already offers all three of these predicates and already combines + them additively, so this feature brings Content Drive to parity rather than inventing semantics. +- **Backward-compatibility expectations**: The legacy Site Browser's "Show Archived" checkbox must + behave exactly as before. Existing callers of the drive search capability that send no status must + see byte-identical results. No content, stored configuration, or admin workflow changes. +- **Known related decisions**: The workflow filter delivered earlier in this epic established how a + new drive-search filter is plumbed end to end, and the later archive-step work established the + precedent for a filter that manipulates the archived condition — including the care needed when two + filters both have an opinion about it. Both are binding shape precedents. The plan phase will + formally consult `dotCMS/platform-adrs`. + +## Assumptions + +- The three statuses are the complete set for this feature. Other states an item can be in (for + example "has a scheduled publish date") are out of scope. +- "Locked" means a lock is held by anyone, not "locked by me". A per-user variant is not requested + and would be a separate filter. +- Users of this filter already have permission to see the content it surfaces; the filter narrows a + result set that permission checks have already constrained, and grants no new visibility. +- The redundancy of Archived + Unpublished is acceptable to expose rather than something to prevent + in the control. Disabling one option based on another would be a second, hidden rule the user has + to learn, and the combination is harmless. +- The Shared Assets / System Host toggle is explicitly out of scope, tracked separately in + [#34760](https://github.com/dotCMS/core/issues/34760). From 82352801efac361ffd80b065e0e12e7b3fd896a4 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 10:57:09 -0300 Subject: [PATCH 02/16] docs(content-drive): add Status filter data model and API contract (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 design artifacts for the Status filter, stacked on the spec PR. Carries only the two artifacts .specify/CUSTOMIZATIONS.md considers durable: data-model.md (the ContentStatus enum, the `status` transport shape, the filter-bag entry) and contracts/ (the `status` field on POST /api/v1/drive/search, including the AND semantics and the 400 on an unknown value). plan.md, research.md and quickstart.md stay local by design — the repo gitignores them as process-only artifacts. Co-Authored-By: Claude Opus 5 (1M context) --- .../contracts/drive-search-status.md | 125 +++++++++++++ .../data-model.md | 169 ++++++++++++++++++ 2 files changed, 294 insertions(+) create mode 100644 specs/37066-content-drive-status-filter/contracts/drive-search-status.md create mode 100644 specs/37066-content-drive-status-filter/data-model.md diff --git a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md new file mode 100644 index 000000000000..5aff4f2da52d --- /dev/null +++ b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md @@ -0,0 +1,125 @@ +# Contract: `status` on `POST /api/v1/drive/search` + +**Feature**: [../spec.md](../spec.md) | **Date**: 2026-08-24 + +One new optional field on an existing request body. No new endpoint, no breaking change: a request +that omits `status` behaves byte-identically to today (FR-002). + +Resource: `dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ContentDriveResource.java` (`@Path("/v1/drive")`, `@Path("/search")`, `@POST`). + +--- + +## Request + +```jsonc +{ + "assetPath": "//demo.dotcms.com/marketing/", + "status": ["UNPUBLISHED", "LOCKED"] // NEW — optional, defaults to [] + // …every existing field unchanged +} +``` + +| Field | Type | Required | Default | Description | +|---|---|---|---|---| +| `status` | `string[]` | No | `[]` | Content states to narrow by. Accepted: `ARCHIVED`, `UNPUBLISHED`, `LOCKED`. Entries combine with **AND**. | + +### Semantics + +| Selection | Returns | +|---|---| +| `[]` (or omitted) | Today's behavior: archived excluded, everything else returned | +| `["ARCHIVED"]` | Only archived content | +| `["UNPUBLISHED"]` | Only content with no live version, archived excluded | +| `["LOCKED"]` | Only content with a lock held, by anyone, archived excluded | +| `["UNPUBLISHED","LOCKED"]` | Content that is both — the driving multiselect use case | +| `["ARCHIVED","UNPUBLISHED"]` | Same set as `["ARCHIVED"]` alone. Redundant, not an error | +| `["ARCHIVED","LOCKED"]` | Archived content that still holds a lock. Reachable; often empty | +| all three | Archived **and** unpublished **and** locked | + +**AND, not OR.** Unlike `contentTypes`, `baseTypes` and `language` — single-valued attributes where +an intersection is always empty — these are independent flags one item can hold at once. + +### Side effects on other fields + +| Field | Effect when `status` is non-empty | +|---|---| +| `showFolders` | Forced to `false`. Folders carry no status (FR-015), consistent with the workflow filter | +| `archived` | Unaffected and unchanged. The legacy inclusive flag keeps its meaning (FR-008) | + +The Content Drive UI stops sending `archived: false` altogether (FR-019); the server default already +supplies it. + +--- + +## Responses + +### 200 — success + +Response shape is unchanged (`ResponseEntityView`). Only the contents of `list`, +and the counts, narrow. + +### 400 — unrecognized status value + +```jsonc +// request +{ "assetPath": "//demo.dotcms.com/", "status": ["ARCHIVED", "DRAFT"] } +``` + +Returns `400` with a message naming the accepted values. Silently ignoring the unknown entry would +return a **wider** result set than the caller asked for, which is worse than failing. + +Consistent with the existing `userSearchable` rejection in `ContentDriveHelper` — the same +`BadRequestException` path, thrown explicitly rather than left to Jackson. + +### Other statuses + +Unchanged: `401` unauthenticated, `403` no portlet access, `500` unexpected. + +--- + +## Query-path guarantees + +The same `status` selection must return the same set regardless of which search strategy the +environment runs (FR-009 / SC-005). Both paths are in scope: + +| `BROWSE_API_HEURISTIC_TYPE` | Path | How `status` applies | +|---|---|---| +| `HYBRID_SINGLE_CHUNKED_QUERY_ES` (**default**) | `BrowserAPIImpl.selectQuery` supplies the candidate set; the index only narrows by text | SQL clauses — so they apply with **and** without a keyword | +| `PURE_ES` | `BrowserAPIImpl.buildPureESQuery` | Index terms, replacing the hardcoded `+deleted:false` | + +Aligned with [ADR-0018](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0018-database-first-content-drive-search-with-index-deferred-text-filtering.md), +which routes version-info flags (archived/deleted) to the **database**. `PURE_ES` is patched not to +promote it, but because it is a supported configuration where the filter would otherwise silently +no-op. + +--- + +## OpenAPI + +`openapi.yaml` is generated by `swagger-maven-plugin` at compile. The field's description goes in the +Java annotations; the regenerated +`dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml` is committed alongside: + +```bash +./mvnw compile -pl :dotcms-core -DskipTests +git diff -- '*openapi.yaml' +``` + +CI verifies the committed file matches what the build produces. + +--- + +## Frontend contract + +`DotContentDriveSearchRequest.status?: string[]` in +`core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts`. + +URL round-trip via the shared filter bag, so a filtered view is shareable and survives reload +(FR-016): + +``` +…?filters=languageId:1;sharedAssets:true;status:UNPUBLISHED,LOCKED +``` + +Encoding needs no new code — `encodeFilters` already comma-joins array values. Decoding is one entry +in `decodeByFilterKey` (`status: multiSelector`). diff --git a/specs/37066-content-drive-status-filter/data-model.md b/specs/37066-content-drive-status-filter/data-model.md new file mode 100644 index 000000000000..0c8327232b89 --- /dev/null +++ b/specs/37066-content-drive-status-filter/data-model.md @@ -0,0 +1,169 @@ +# Phase 1 Data Model: Content Drive Status Filter + +**Feature**: [spec.md](./spec.md) | **Plan**: [plan.md](./plan.md) | **Date**: 2026-08-24 + +No database schema changes. Every column this feature reads already exists on +`contentlet_version_info` and is already indexed into the search index. This document describes the +**in-memory and over-the-wire shapes** the feature introduces. + +--- + +## Entity: `ContentStatus` (new) + +`dotCMS/src/main/java/com/dotcms/browser/ContentStatus.java` + +A closed enum of three independent states a contentlet version can hold. + +| Constant | Meaning | Backing column (`contentlet_version_info`) | Index term | +|---|---|---|---| +| `ARCHIVED` | Removed from circulation, recoverable | `deleted = true` | `+deleted:true` | +| `UNPUBLISHED` | No live version exists | `live_inode is null` | `+live:false` | +| `LOCKED` | A lock is held, by anyone | `locked_by is not null` | `+locked:true` | + +**Placement**: `com.dotcms.browser` rather than the REST package — `BrowserQuery` is the consumer, +and the browser layer must not depend on the REST layer. Sits alongside `FieldSearchCriteria`, which +plays the same query-shaping role. + +**Relationships**: none. The three are orthogonal facts about one row, which is exactly why they +combine with AND rather than OR (see [research.md R2](./research.md)). + +**Not a state machine**: an item can hold any subset of the three at once. Two subsets are worth +naming because they are user-visible oddities, not defects: + +- `{ARCHIVED, UNPUBLISHED}` is always equivalent to `{ARCHIVED}` — archiving removes the live + version (`ESContentletAPIImpl.java:3833`), so every archived item is already unpublished. +- `{ARCHIVED, LOCKED}` is reachable but rare: it needs a self-lock or a CMS-Admin archive + (`canLock` at `:10380`/`:10406`), and `internalArchive` never clears `locked_by`. + +--- + +## Transport shape: `DriveRequestForm.status` + +`dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/AbstractDriveRequestForm.java` + +```java +@JsonProperty("status") +@Value.Default +default List status() { return List.of(); } +``` + +| Property | Value | +|---|---| +| JSON key | `status` | +| Type on the wire | array of strings | +| Accepted values | `ARCHIVED`, `UNPUBLISHED`, `LOCKED` (case-insensitive on input, uppercased before lookup) | +| Default | `[]` — preserves today's behavior exactly (FR-002) | +| Duplicates | Collapsed; the parsed result is a `Set` | +| Unknown value | `400`, message naming the accepted values (FR-010) | + +**Declared as `List`, not `List`** — see [research.md R7](./research.md). The +helper owns the parse so the 400 is thrown explicitly, matching the `userSearchable` precedent +already in `ContentDriveHelper`. + +### Validation rules + +| Rule | Source | Enforced in | +|---|---|---| +| Empty is valid and means "no status filtering" | FR-001, FR-002 | `ContentDriveHelper` (block is skipped) | +| Every element must name a `ContentStatus` | FR-010 | `ContentDriveHelper.parseStatuses` → `BadRequestException` | +| Selection narrows (AND), never widens | FR-006 | `BrowserAPIImpl.appendContentStatusQuery` — independent `and` clauses | +| A non-empty selection excludes folders | FR-015 | `ContentDriveHelper` → `.showFolders(false)` | + +--- + +## Query shape: `BrowserQuery.contentStatuses` + +`dotCMS/src/main/java/com/dotcms/browser/BrowserQuery.java` + +```java +final Set contentStatuses; // never null; empty means no filtering +public Set getContentStatuses() // accessor, mirrors getFieldCriteria() +Builder withContentStatuses(@Nonnull Set) +``` + +Plumbed exactly like `workflowSchemeIds`: builder field (`LinkedHashSet`, insertion-ordered for +stable generated SQL), `Set.copyOf` in the constructor, a line in the copy-constructor, and a line +in `toString()`. + +**One derived field changes.** The constructor's + +```java +this.showWorking = builder.showWorking || builder.showArchived; +``` + +must also be true when the selection contains `ARCHIVED` or `UNPUBLISHED`. Both states imply no live +version, so without this the query joins on `live_inode` and can never match. See +[research.md R4](./research.md). + +--- + +## Frontend shape + +### Filter-bag entry + +`core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/models.ts` + +```ts +export type DotKnownContentDriveFilters = { + // … + status: string[]; // 'ARCHIVED' | 'UNPUBLISHED' | 'LOCKED' +}; +``` + +| Aspect | Behavior | Why | +|---|---|---| +| URL encoding | `status:ARCHIVED,LOCKED` | `encodeFilters` already comma-joins arrays — no change | +| URL decoding | `status: multiSelector` in `decodeByFilterKey` | one line; splits on comma | +| Seeded default | **No** | Empty genuinely means "off", unlike `languageId`/`sharedAssets` | +| "Clear all" | Cleared automatically | `clearFilters()` re-seeds only defaults, so `status` drops | +| Chip visibility | Automatic | `hasNonDefaultFilters` returns `true` for any non-default key | + +### Request field + +`core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts` + +```ts +export interface DotContentDriveSearchRequest { + // … + status?: string[]; +} +``` + +Sent only when non-empty. The `archived: false` pin is **removed** — the form's own `archived()` +already defaults to `false`, so omitting it produces an identical query while letting the status +selection own the archived decision (FR-019). + +### Option list + +`core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/constants.ts` + +```ts +export const STATUS_FILTER_KEY = 'status'; + +export const CONTENT_STATUS = { + ARCHIVED: 'ARCHIVED', + UNPUBLISHED: 'UNPUBLISHED', + LOCKED: 'LOCKED' +} as const; + +export const STATUS_FILTER_OPTIONS: { value: string; labelKey: string }[] = [ + { value: CONTENT_STATUS.ARCHIVED, labelKey: 'content-drive.status-filter.archived' }, + { value: CONTENT_STATUS.UNPUBLISHED, labelKey: 'content-drive.status-filter.unpublished' }, + { value: CONTENT_STATUS.LOCKED, labelKey: 'content-drive.status-filter.locked' } +]; +``` + +Shape follows `FOLDER_UPLOAD_BEHAVIOR_OPTIONS` in the same file. Order is display order: Archived +first because it is the capability people currently leave Content Drive to get (US1). + +--- + +## What this feature does **not** change + +- **No schema migration.** `deleted`, `live_inode` and `locked_by` all predate this work. +- **No index mapping change.** `deleted`, `live` and `locked` are already mapped + (`ESMappingAPIImpl.java:527` for `locked`). +- **No change to `BrowserQuery.showArchived`.** Its inclusive meaning and the legacy Site Browser + checkbox that depends on it are untouched (FR-008). +- **No new API surface.** One optional field on an existing request body; `openapi.yaml` is + regenerated, not hand-edited. From a4e63f1ebdb157f355a909032f100b291954bdbe Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 11:07:20 -0300 Subject: [PATCH 03/16] chore(speckit): commit plan.md, research.md and quickstart.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores stock Spec-Kit behavior for the three artifacts a reviewer needs to judge a design. Only tasks.md and checklists/ stay gitignored. The original policy in #36416 kept all five local on the "process artifact" test. That predates this repo adopting GitHub's native stacked pull requests (public preview 2026-07-30), which changes the calculation: the spec-driven flow now lands as a stack whose middle layer exists purely to review the design. Gitignoring plan.md makes that layer a PR with nothing to review. tasks.md and checklists/ stay ignored — both are regenerated by their own command, and a merged task list that no longer matches what was built is worse than no task list. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 7 +++---- .specify/CUSTOMIZATIONS.md | 28 +++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 9a1c952d10da..1bfd9501a3e9 100644 --- a/.gitignore +++ b/.gitignore @@ -225,9 +225,8 @@ dist/ /core-web/scratch/ # Spec-Kit working artifacts — process-only, kept local (see .specify/CUSTOMIZATIONS.md). -# spec.md (and data-model.md / contracts/ when they carry verified contracts) stay tracked. -specs/*/plan.md -specs/*/research.md +# Everything else under specs/ is tracked, as upstream Spec-Kit intends: spec.md, plan.md, +# research.md, quickstart.md, data-model.md and contracts/. Only the two artifacts that are +# regenerated by their own command, and go stale fastest, stay local. specs/*/tasks.md -specs/*/quickstart.md specs/*/checklists/ diff --git a/.specify/CUSTOMIZATIONS.md b/.specify/CUSTOMIZATIONS.md index c6ac8886de44..35bf2ba21ff3 100644 --- a/.specify/CUSTOMIZATIONS.md +++ b/.specify/CUSTOMIZATIONS.md @@ -39,12 +39,34 @@ process artifact**: | Artifact | Commit? | Why | |----------|---------|-----| | `spec.md` | Always | the reviewed contract (FRs, user stories, success criteria) | +| `plan.md` | Always | the reviewed *design* — approach, Legacy Impact, Constitution Check, ADR Alignment. A reviewer cannot judge a design they cannot see | +| `research.md` | Always | why each decision went the way it did, and what was rejected. This is the artifact a future dev most often wants and can least reconstruct from the code | +| `quickstart.md` | Always | how to build, test and verify the feature — useful long after merge | | `data-model.md` | When it carries verified contracts | concrete entity→field/type, relationships, validation rules, real payload/DB shapes confirmed while building — the field-level ground truth `spec.md` stays above | | `contracts/` | Same test as `data-model.md` | committed API specs are durable; scaffolding is not | -| `plan.md`, `research.md`, `tasks.md`, `quickstart.md`, `checklists/` | Never | pure process — how / what-order / decisions-in-flight | +| `tasks.md`, `checklists/` | Never | pure sequencing and in-flight bookkeeping; regenerated on demand by `/speckit-tasks` and `/speckit-checklist`, and stale within a day of implementation starting | -The never-commit set is enforced by `.gitignore` (`specs/*/plan.md`, `research.md`, -`tasks.md`, `quickstart.md`, `checklists/`). +The never-commit set is enforced by `.gitignore` (`specs/*/tasks.md`, `specs/*/checklists/`). + +### Why this changed (2026-08-24) + +The original policy kept `plan.md`, `research.md` and `quickstart.md` local too, on the +"process artifact" test. That was written before this repo adopted **stacked pull requests** +(GitHub's native stacks, public preview 2026-07-30, via the `github/gh-stack` extension). + +Stacks change the calculation. The spec-driven flow now lands as a stack — spec → design → +implementation — where each layer is reviewed on its own. A design layer whose `plan.md` is +gitignored is a PR with nothing in it to review, which is what surfaced the problem on +[#37066](https://github.com/dotCMS/core/issues/37066) (stack +[#37172](https://github.com/dotCMS/core/pull/37171)). The old rule was not wrong for the +one-spec-PR-plus-one-implementation-PR shape it was written for; it simply never considered +this one. + +Committing them also restores stock Spec-Kit behavior, whose premise is that every artifact is +version-controlled so there is a visible trail from original intent to shipped code. + +`tasks.md` and `checklists/` stay ignored: both are regenerated by their own command, and a +merged task list that no longer matches what was built is worse than no task list. **`data-model.md` commit-worthiness bar:** commit it only if a future dev would need it to know the shapes without reading the code. Its structure follows the plan template's From 4c6f547b2d9bed4dedf54298154c38bd453d97c7 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 11:07:23 -0300 Subject: [PATCH 04/16] docs(content-drive): add Status filter plan, research and quickstart (#37066) Now trackable after the preceding gitignore change. These are the artifacts /speckit-plan already generated for this feature; they were sitting on disk, invisible to review. - plan.md design, Legacy Impact, Constitution Check, ADR Alignment gate - research.md R1-R10, each verified against the code rather than the issue text - quickstart.md build/test commands and the end-to-end validation walkthrough Co-Authored-By: Claude Opus 5 (1M context) --- .../37066-content-drive-status-filter/plan.md | 282 ++++++++++++++++++ .../quickstart.md | 187 ++++++++++++ .../research.md | 256 ++++++++++++++++ 3 files changed, 725 insertions(+) create mode 100644 specs/37066-content-drive-status-filter/plan.md create mode 100644 specs/37066-content-drive-status-filter/quickstart.md create mode 100644 specs/37066-content-drive-status-filter/research.md diff --git a/specs/37066-content-drive-status-filter/plan.md b/specs/37066-content-drive-status-filter/plan.md new file mode 100644 index 000000000000..6442fb76b285 --- /dev/null +++ b/specs/37066-content-drive-status-filter/plan.md @@ -0,0 +1,282 @@ +# Implementation Plan: Content Drive Status Filter + +**Branch**: `issue-37066-content-drive-status-filter-plan` (spec on `issue-37066-content-drive-status-filter`) | **Date**: 2026-08-24 | **Spec**: [spec.md](./spec.md) + +**Input**: Feature specification from `/specs/37066-content-drive-status-filter/spec.md` + +**Issue**: [dotCMS/core#37066](https://github.com/dotCMS/core/issues/37066) (absorbed #37067; epic #33999) + +## Summary + +Add a **Status** filter (Archived, Unpublished, Locked) to Content Drive: a new optional `status` +array on `POST /api/v1/drive/search`, resolved as AND-combined predicates in the database, plus a +multiselect chip in the Content Drive toolbar that drives it and round-trips through the URL. + +The technical approach is a flat, additive one. The three statuses are independent boolean facts +about the same `contentlet_version_info` row, so each becomes one independent `and` clause — no +composed OR group, no new join. The plumbing mirrors the workflow filter (`beaf846d51`) end to end. +The only delicate part is that **two existing code paths already have an opinion about +`cvi.deleted`** and both must learn about `ARCHIVED`; that is detailed below and is where the +regression risk lives. + +## Technical Context + +**Language/Version**: Java 25 (`dotcms.core.compiler.release`); TypeScript 5.x / Angular 22+ (core-web) + +**Primary Dependencies**: JAX-RS + Immutables (`@Value.Immutable`) for the request form; `BrowserAPI`/`BrowserQuery` for the query layer; PrimeNG (`p-popover`, `p-listbox`, `p-checkbox`) and NgRx Signal Store on the frontend + +**Storage**: PostgreSQL / MS SQL (`contentlet_version_info`) as the system of record; Elasticsearch/OpenSearch for the non-default `PURE_ES` path. **No schema change** — all three columns already exist (`postgres.sql:550-552`) + +**Testing**: JUnit integration (`dotcms-integration`, `-Dit.test=`), JUnit unit (`dotCMS/src/test`), Jest/Spectator (core-web) + +**Target Platform**: dotCMS server (Docker) + the core-web SPA + +**Project Type**: Full-stack — REST form + shared browse query layer + Angular portlet library + +**Performance Goals**: No regression against today's drive search. Each status adds one indexed-column predicate to an existing `where`; no new join, no subquery, no extra round-trip. A non-empty selection also drops the folder query entirely (`showFolders(false)`), so filtered requests do strictly less work + +**Constraints**: Rollback-safe (no schema, no mapping, no breaking contract change). With no `status` sent, every generated query must be **byte-identical** to today. The legacy Site Browser's inclusive "Show Archived" behavior must not change + +**Scale/Scope**: ~4 backend files + 1 new enum; ~6 frontend files + 1 new component; 1 new integration test class, 1 unit test, 3 spec files + +## Legacy Impact + +- **Touches legacy?** No `com.dotmarketing.*` source is modified. `com.dotcms.browser.BrowserAPIImpl` + is long-lived and heavily-parameterized but sits in the modern package. The legacy Site Browser JSP + (`view_browser.jsp`) and the legacy Content Search portlet (`ContentletAjax`) are **read as + precedent and left untouched**. +- **Modern vs legacy placement**: everything new lands in `com.dotcms.*` — the enum in + `com.dotcms.browser` (next to `FieldSearchCriteria`, which plays the same query-shaping role), the + form field in `com.dotcms.rest.api.v1.drive`. +- **Backward compatibility / migration**: none required. No DB schema change, no ES/OpenSearch + mapping change, no serialized-state change. The REST change is one **optional** field with an empty + default, so existing callers are unaffected. Not rollback-unsafe under any category in + `docs/core/ROLLBACK_UNSAFE_CATEGORIES.md`. + - The one real compatibility risk is behavioral, not structural: `BrowserQuery.showArchived` keeps + its **inclusive** meaning (archived *plus* everything else) because `view_browser.jsp:145` + depends on it. The new exclusive behavior is added alongside. An integration assertion guards + this (FR-008). +- **Progressive enhancements** (in-scope, small, only in code already being touched): + - Javadoc on every new method and on the new enum, matching the density of `appendWorkflowQuery` + and `withWorkflowSchemeIds` around it. + - The new frontend component is written to current standards from the start — `@if`, `input()`, + signals, `#`-private members, `ChangeDetectionStrategy.OnPush` — and strict-mode clean, per the + in-flight core-web strict migration. + - No wholesale rewrite of `BrowserAPIImpl`. It is a 2700-line file; this change adds one private + method and touches three existing lines. + +## Test Strategy (TDD — mandatory) + +Constitution Principle V: no implementation code before tests are written, developer-approved, and +confirmed **failing** for the right reason. + +| Component / behavior | Test type(s) | Where | Notes | +|---|---|---|---| +| Status value parsing; unknown value → 400 | Unit (JUnit) | `dotCMS/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveHelperStatusTest.java` | Mirrors `ContentDriveFieldFilterResolverTest`. No container needed | +| Each status alone; each pair; all three; empty default | Integration | `dotcms-integration/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveStatusFilterTest.java` | Follows `ContentDriveWorkflowFilterTest`: dedicated site + folder + a purpose-built content type + unique id, `@AfterClass` cleanup. Never asserts against shared default content types | +| `ARCHIVED + UNPUBLISHED` == `ARCHIVED`; `ARCHIVED + LOCKED` reachable | Integration | same class | FR-007 and the second Edge Case — asserted as documented behavior | +| Parity with and without free text (default hybrid heuristic) | Integration | same class | FR-009 / SC-005 | +| Parity under `BROWSE_API_HEURISTIC_TYPE=PURE_ES` | Integration | same class | Config override + restore in the test; the only coverage `buildPureESQuery` gets | +| Status + archive-target workflow step | Integration | same class | FR-011. The regression this change is most likely to cause | +| Legacy inclusive `showArchived` unchanged | Integration | same class | FR-008 — a direct `BrowserQuery` assertion, not through the drive form | +| `ContentDriveWorkflowArchiveStepTest` still green | Integration (existing) | unchanged file | Byte-identical SQL when no status is sent | +| Status multiselect: single, multiple, clearing, testids | Jest/Spectator | `…/dot-content-drive-status-filter/dot-content-drive-status-filter.component.spec.ts` | Driven through the rendered checkbox's `(onChange)`, never protected members. No `if` in test bodies. No CSS-class assertions | +| Request carries `status`; `showFolders` false; `archived` pin gone | Jest | `…/store/dot-content-drive.store.spec.ts` | Asserts the built `$request` payload | +| `status` URL decode round-trip | Jest | `…/utils/functions.spec.ts` | Alongside the existing `workflow` decode cases | + +**Registration**: `ContentDriveStatusFilterTest` joins `MainSuite3a`, where the other drive +integration tests live. + +- **Tests that cannot be implemented**: **none**. Every layer is reachable. Postman is deliberately + omitted rather than "not possible" — the endpoint-level behavior is covered more precisely by the + integration tests, which can seed the exact archived/locked fixtures a Postman collection cannot + construct against a shared environment. If the reviewer wants a Postman smoke case for the 400, + that is a cheap addition. + +## Constitution Check + +*GATE: evaluated before Phase 0, re-evaluated after Phase 1 design. Result: **PASS**, no violations.* + +| Principle | Verdict | Evidence | +|---|---|---| +| **I. Legacy-Aware Development** | PASS | No `com.dotmarketing.*` changes. New code in `com.dotcms.browser` / `com.dotcms.rest.api.v1.drive`. Legacy Site Browser behavior explicitly preserved and asserted. Progressive enhancements scoped to touched code only — see Legacy Impact | +| **II. Config & Logging Discipline** | PASS | No new `System.*` calls. Reads `BROWSE_API_HEURISTIC_TYPE` only through the existing `Config`-backed `HEURISTIC_TYPE` lazy. No new dependency, so no `bom/application/pom.xml` change | +| **III. Security by Default** | PASS | No secrets. The only user input is a closed enum, validated against it and rejected with a 400 — it never reaches SQL as text, so there is no injection surface. Permission filtering is untouched: the status clauses narrow a candidate set that `permissionAPI` still filters downstream, so the filter grants no new visibility | +| **IV. Contract Correctness** | PASS | One optional field with an empty default; no `@Schema` return type changes. `openapi.yaml` regenerated from the annotations and committed alongside the Java change. Not rollback-unsafe: no schema, no mapping, no breaking contract change | +| **V. Test-First / TDD** | PASS | Test Strategy above covers every layer with no exceptions claimed. `/speckit-tasks` will order each user story as tests → approval GATE → Red GATE → implementation, and `/speckit-implement` must halt at each gate | +| **ADR consultation** | PASS | `/speckit-adr-context` ran as the mandatory `before_plan` hook; results in ADR Alignment below | + +## ADR Alignment (Gate) + +**Step 1 — Consult existing ADRs**: run automatically as the `before_plan` hook. + +```bash +.specify/scripts/bash/adr-context.sh content-drive search elasticsearch browser query rest angular filter permissions legacy +``` + +### Relevant existing ADRs + +| ADR | Title | Status | How it constrains / informs this plan | +|---|---|---|---| +| [ADR-0018](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0018-database-first-content-drive-search-with-index-deferred-text-filtering.md) | Database-First Search for Content Drive, with Text Filtering Deferred to the Search Index | proposed | **Directly governing.** Its routing table lists *"Archived / deleted, show-on-menu → **DB** (version-info flags)"*. All three status predicates are version-info flags, so all three are resolved in SQL. It also states the index must be used *only* for free-text and searchable-field matching, and that structural criteria "must **never** be silently re-routed to the index for speed" — which this plan honors. It further notes `PURE_ES` "remains available behind configuration… but is **not** the default": that is precisely why `buildPureESQuery` is patched, so a supported configuration cannot silently drop the filter | +| [ADR-0009](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0009-opensearch-migration-plan.md) | Migrate OpenSearch from 1.x to 3.x Using Environment-Based Migration Strategy | accepted | Constrains only the `PURE_ES` clauses. The three terms used (`deleted`, `live`, `locked`) are core version-info fields mapped identically under both backends, so no migration-phase divergence is introduced. ADR-0018's own rationale — "shrink the blast radius of the ES→OS migration" by hanging fewer correctness guarantees off the index — is served by keeping the DB as the authority here | +| [ADR-0020](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0020-deprecate-folder-bypath-endpoint.md) | Deprecate `POST /api/v1/folder/byPath` in favor of `GET /api/v1/folder/search` | accepted | Surfaced by the keyword search but **not applicable** — this feature adds no folder endpoint and calls neither | + +### Conflicts with accepted ADRs + +**None.** The one ADR that governs this work (ADR-0018) is `proposed` rather than `accepted`, so it +is directional rather than binding — but this plan complies with it fully anyway, so the distinction +does not need resolving. Its central rule is that structural and metadata predicates belong in the +database, and all three status predicates are resolved there. + +### Proposed ADRs + +**None proposed.** This feature adds a filter *within* an already-decided routing contract; it makes +no new architectural decision. The AND-vs-OR choice is a domain fact about independent boolean flags +(and matches long-standing behavior in the legacy Content Search portlet), not an architectural +decision worth recording. + +## Project Structure + +### Documentation (this feature) + +```text +specs/37066-content-drive-status-filter/ +├── spec.md # /speckit-specify output +├── plan.md # This file +├── research.md # Phase 0 — R1..R10, all questions closed +├── data-model.md # Phase 1 — enum, transport and filter-bag shapes +├── quickstart.md # Phase 1 — how to build, test and see it work +├── contracts/ +│ └── drive-search-status.md # Phase 1 — the `status` field contract +├── checklists/requirements.md # gitignored; local spec-quality record +└── tasks.md # Phase 2 — /speckit-tasks, NOT created here +``` + +### Source Code (repository root) + +```text +# Backend — query layer +dotCMS/src/main/java/com/dotcms/browser/ +├── ContentStatus.java # NEW — ARCHIVED | UNPUBLISHED | LOCKED +├── BrowserQuery.java # + field, builder method, copy-ctor, toString; +│ # showWorking derivation extended +└── BrowserAPIImpl.java # + appendContentStatusQuery; 3 touched lines + # (:1980 archiveStepIds, :2006 exclusion, :612 ES) + +# Backend — REST +dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ +├── AbstractDriveRequestForm.java # + status() : List, default List.of() +├── ContentDriveHelper.java # + parseStatuses + builder wiring + showFolders(false) +└── ContentDriveResource.java # @Operation description only +dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml # regenerated, committed + +# Backend — tests +dotCMS/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveHelperStatusTest.java # NEW +dotcms-integration/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveStatusFilterTest.java # NEW +dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java # + registration + +# Frontend — portlet +core-web/libs/portlets/dot-content-drive/portlet/src/lib/ +├── shared/constants.ts # + STATUS_FILTER_KEY, CONTENT_STATUS, STATUS_FILTER_OPTIONS +├── shared/models.ts # + status: string[] on DotKnownContentDriveFilters +├── utils/functions.ts # + status: multiSelector in decodeByFilterKey +├── store/dot-content-drive.store.ts # - archived: false; + status; + showFolders term +└── components/dot-content-drive-toolbar/ + ├── dot-content-drive-toolbar.component.{ts,html} # render the new chip + └── components/dot-content-drive-status-filter/ # NEW component + template + spec + +# Frontend — shared model + i18n +core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts # + status?: string[] +dotCMS/src/main/webapp/WEB-INF/messages/Language.properties # + 4 keys (near :7113) +``` + +**Structure Decision**: Full-stack, following the exact shape the workflow filter established in +`beaf846d51` — request form → `BrowserQuery` → `BrowserAPIImpl` on the backend, and filter bag → +store `$request` → toolbar chip on the frontend. Nothing new is introduced structurally; this +feature is a second instance of an already-proven pattern, which is why it should land well under +the archive-step work's footprint (`f92f939296`: 184 impl lines). + +--- + +## Implementation approach + +Full detail and verification for each decision is in [research.md](./research.md); this is the +summary a reviewer needs. + +### Backend + +**1. `ContentStatus` enum** — `ARCHIVED`, `UNPUBLISHED`, `LOCKED`, in `com.dotcms.browser`. + +**2. `AbstractDriveRequestForm.status()`** — `List`, defaulting to `List.of()`. Deliberately +strings rather than the enum, so `ContentDriveHelper` owns the parse and throws an explicit +`BadRequestException` naming the accepted values — the deterministic 400 FR-010 asks for, matching +the `userSearchable` precedent already in that class (R7). + +**3. `BrowserQuery`** — plumbed exactly like `workflowSchemeIds`. One derived line changes: + +```java +this.showWorking = builder.showWorking || builder.showArchived; // :151 +``` + +must also be true when `ARCHIVED` or `UNPUBLISHED` is selected. Both states mean *no live version*, +so without this the query joins `c.inode = cvi.live_inode` and can never match — the filter would +silently return nothing. The drive path is safe today only by coincidence (`live()` defaults false); +the flag has to be right for any caller (R4). + +**4. `BrowserAPIImpl` — the three clauses**, in a new private `appendContentStatusQuery`: + +| Status | SQL | +|---|---| +| `ARCHIVED` | `and cvi.deleted = ` | +| `UNPUBLISHED` | `and cvi.live_inode is null` | +| `LOCKED` | `and cvi.locked_by is not null` | + +Independent `and` clauses — that *is* the AND semantics, no combinator needed. + +**5. `BrowserAPIImpl` — the two `cvi.deleted` interactions.** This is the risk surface: + +- **The global exclusion** (`:2006`) gains a third term: + `if (!showArchived && archiveStepIds.isEmpty() && !statuses.contains(ARCHIVED))`. + Without it, `cvi.deleted = false` and `cvi.deleted = true` are both emitted and `ARCHIVED` always + returns nothing. With it, `UNPUBLISHED`/`LOCKED` alone still keep the exclusion — which is exactly + FR-004, for free. +- **Archive-target workflow steps** (`:1980`). `archiveStepIds` is already emptied when + `showArchived`, because `appendWorkflowQuery` otherwise owns `cvi.deleted` per branch and would + force `false` on the live branch. `ARCHIVED` needs identical treatment: + ```java + final boolean admitsArchived = browserQuery.showArchived || statuses.contains(ARCHIVED); + ``` + This reuses the mechanism the archive-step work already built rather than inventing a second + reconciliation. With no status sent, every generated query stays byte-identical (R5). + +**6. `buildPureESQuery`** — the hardcoded `+deleted:false` (`:612`) becomes conditional, plus +`+live:false` / `+locked:true`. Only runs under `BROWSE_API_HEURISTIC_TYPE=PURE_ES`, which is a +supported configuration where the filter would otherwise silently no-op (R6). + +**7. `ContentDriveHelper`** — a block mirroring the workflow block directly above it: parse, set the +statuses, and `showFolders(false)` because folders carry no status. + +### Frontend + +Reuse over invention — every piece already exists (R9): + +- **Constants / models / decode**: three small additions (`STATUS_FILTER_OPTIONS`, `status: string[]` + on the filter bag and on `DotContentDriveSearchRequest`, `status: multiSelector` in + `decodeByFilterKey`). Encoding needs **no** change: `encodeFilters` already comma-joins arrays. +- **Not seeded in `withFilterDefaults`.** Unlike `languageId` and `sharedAssets`, where "absent" is + not a neutral state, an empty status set genuinely means no filtering. Leaving it unseeded makes + "Clear all" appear and clear correctly with no new code. +- **Store `$request`**: drop `archived: false` (the server default already supplies it) and send + `status` instead; add `!filters()?.status?.length` to the `showFolders` conjunction. +- **New component**: `dot-chip-filter` (`mode="dropdown"`) + `p-popover` + `p-listbox` with a + `p-checkbox` per row and `dot-filter-list-item` for labels, over a static three-option list. + Modeled on the workflow filter but **without** its service, caches, request-id guard and reconcile + pass — those exist because workflow options are fetched and can vanish between loads. Three fixed + options need none of it. `data-testid` on the chip, panel and each option; `[attr.aria-label]` on + the chip. +- **i18n**: four `content-drive.status-filter.*` keys in `Language.properties`. + +## Complexity Tracking + +No Constitution Check or ADR Alignment violations, so this section is intentionally empty. diff --git a/specs/37066-content-drive-status-filter/quickstart.md b/specs/37066-content-drive-status-filter/quickstart.md new file mode 100644 index 000000000000..01213cc681ea --- /dev/null +++ b/specs/37066-content-drive-status-filter/quickstart.md @@ -0,0 +1,187 @@ +# Quickstart: Validating the Content Drive Status Filter + +**Feature**: [spec.md](./spec.md) | **Plan**: [plan.md](./plan.md) | **Date**: 2026-08-24 + +How to build, test and see this feature working. Shapes and semantics live in +[data-model.md](./data-model.md) and [contracts/drive-search-status.md](./contracts/drive-search-status.md); +this file is the run guide. + +--- + +## Prerequisites + +```bash +sdk env install # Java 25 via SDKMAN (.sdkmanrc) — build fails on the wrong version +nvm use # Node 22.22.3+ via nvm (.nvmrc) — frontend build fails on the wrong version +``` + +`nvm use` is also needed before `git commit`: the pre-commit hook runs under Node. + +--- + +## Build + +```bash +# Backend — core plus its in-project deps (~2-3 min) +./mvnw install -pl :dotcms-core --am -DskipTests +``` + +## Regenerate and verify the OpenAPI contract + +`openapi.yaml` is generated by `swagger-maven-plugin` at compile; CI fails if the committed file +does not match. No Docker needed. + +```bash +./mvnw compile -pl :dotcms-core -DskipTests +git diff --stat -- '*openapi.yaml' # expect the drive-search request schema to gain `status` +``` + +Commit the regenerated yaml **in the same commit** as the annotation change. + +--- + +## Tests + +### Red first + +Constitution Principle V: write the tests, get them developer-approved, and confirm they **fail for +the right reason** before any implementation. Each command below should be run once before +implementing (expect failure) and again after (expect green). + +### Unit — status parsing and the 400 + +```bash +./mvnw test -pl :dotcms-core -Dtest=ContentDriveHelperStatusTest +``` + +Expected: valid values map to `ContentStatus`; an unknown value raises `BadRequestException` naming +the accepted values. + +### Integration — the behavior that matters + +```bash +./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false \ + -Dit.test=ContentDriveStatusFilterTest +``` + +Needs a running PostgreSQL + Elasticsearch. For fast iteration: + +```bash +just test-integration-ide # start PostgreSQL + Elasticsearch + dotCMS +just test-integration-stop # stop when done +``` + +Single method while iterating: + +```bash +./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false \ + -Dit.test=ContentDriveStatusFilterTest#unpublishedAndLockedReturnsOnlyBoth +``` + +Coverage to expect in that class: + +| Case | Asserts | +|---|---| +| No status | Result set identical to today — archived hidden (FR-002) | +| Each status alone | Exactly the items in that state (FR-003/4/5) | +| Each pair, and all three | Exactly the items holding every selected state (FR-006) | +| `ARCHIVED + UNPUBLISHED` | Same set as `ARCHIVED` alone (FR-007) | +| `ARCHIVED + LOCKED` | Reachable — a self-locked archived item is returned | +| Status + free text | Same status semantics with a keyword present (FR-009) | +| `BROWSE_API_HEURISTIC_TYPE=PURE_ES` | Same results as the default heuristic (SC-005) | +| Status + archive-target workflow step | A coherent, non-empty result (FR-011) | +| Legacy inclusive `showArchived` | Still returns archived **plus** everything else (FR-008) | + +### Regression guard — do not skip + +```bash +./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false \ + -Dit.test=ContentDriveWorkflowArchiveStepTest +``` + +This is the test most likely to break: `appendWorkflowQuery` owns `cvi.deleted` per branch when an +archive-target step is selected. With no status sent, the generated SQL must be byte-identical to +before. + +> Never run the full integration suite to check this work — it takes 60+ minutes. + +### Frontend + +```bash +cd core-web +pnpm nx test portlets-content-drive --testPathPatterns=status-filter +pnpm nx test portlets-content-drive --testPathPatterns='dot-content-drive.store|functions' + +# Jest transpiles without typechecking — typecheck explicitly, or the dev server +# will quietly keep serving the last good bundle +npx tsc --noEmit -p libs/portlets/dot-content-drive/portlet/tsconfig.lib.json + +pnpm nx format:write +pnpm nx lint portlets-content-drive +``` + +`nx` is not global — always go through `pnpm`. + +--- + +## See it work end to end + +```bash +just dev-run # dotCMS in Docker +cd core-web && pnpm nx serve dotcms-ui # frontend dev server +``` + +Set up a fixture in one folder: + +1. Publish one item. +2. Create a second and leave it unpublished. +3. Archive a third. +4. Lock a fourth (open it and leave it checked out). +5. Leave one subfolder in place, to verify folders drop out. + +Then walk the acceptance scenarios: + +| Do | Expect | +|---|---| +| Open Content Drive on that folder, no filters | Live + unpublished + locked items **and** the subfolder. No archived item | +| Select **Archived** | Only the archived item. No folder | +| Select **Unpublished** | Only the unpublished item. Not the archived one | +| Select **Locked** | Only the locked item | +| Select **Unpublished + Locked** | Only an item that is both (create one to see a hit) | +| Select **Archived + Unpublished** | Same count as Archived alone — documented, not an error | +| Check the address bar | `…filters=…;status:UNPUBLISHED,LOCKED` | +| Reload the page | Same chips, same results | +| Open the URL in another browser | Same view for the recipient | +| Press **Clear all** | Status chip gone, folders back, archived hidden again | + +### Direct API check + +```bash +curl -sS -u admin@dotcms.com:admin \ + -H 'Content-Type: application/json' \ + -X POST http://localhost:8082/api/v1/drive/search \ + -d '{"assetPath":"//demo.dotcms.com/","status":["UNPUBLISHED","LOCKED"]}' | jq '.entity | {contentCount, folderCount}' +``` + +Expect `folderCount: 0` whenever a status is set. Then confirm the rejection path: + +```bash +curl -sS -o /dev/null -w '%{http_code}\n' -u admin@dotcms.com:admin \ + -H 'Content-Type: application/json' \ + -X POST http://localhost:8082/api/v1/drive/search \ + -d '{"assetPath":"//demo.dotcms.com/","status":["DRAFT"]}' +# → 400 +``` + +Verify query claims against this running instance rather than by reading the Java. + +--- + +## Definition of done + +- [ ] Every test above is green, and each was seen failing first +- [ ] `ContentDriveWorkflowArchiveStepTest` still passes +- [ ] `openapi.yaml` regenerated and committed with the annotation change +- [ ] Legacy Site Browser "Show Archived" checkbox verified unchanged +- [ ] `pnpm nx format:write` and `lint` clean; `tsc --noEmit` clean +- [ ] No status selected produces results identical to `main` for the same folder diff --git a/specs/37066-content-drive-status-filter/research.md b/specs/37066-content-drive-status-filter/research.md new file mode 100644 index 000000000000..8e4622d4a0be --- /dev/null +++ b/specs/37066-content-drive-status-filter/research.md @@ -0,0 +1,256 @@ +# Phase 0 Research: Content Drive Status Filter + +**Feature**: [spec.md](./spec.md) | **Issue**: [#37066](https://github.com/dotCMS/core/issues/37066) | **Date**: 2026-08-24 + +Every finding below was verified against the code at `origin/main` (`e46da2b187`), not inferred +from the issue text. Line numbers are from that commit. + +--- + +## R1: What exists today for each of the three statuses + +**Decision**: Two of the three are net-new; the third exists in a form that answers a different +question and must be left alone. + +**Findings**: + +| Status | Present in `BrowserQuery`? | Detail | +|---|---|---| +| Archived | Partially, and **inclusive** | `showArchived` (`BrowserQuery.java:55`, builder `:470`) merely *skips* `appendExcludeArchivedQuery` (`BrowserAPIImpl.java:2006`). It returns archived content **plus everything else** — the opposite of what an "Archived" chip means. | +| Unpublished | No | `showWorking` and the form's `live` flag select *which version* to show. That is not the inverse of "has a live version". | +| Locked | No | No field, no builder method, no SQL clause, no ES clause. Nothing under `com.dotcms.rest.api.v1.drive` mentions `locked`. | + +Confirmed against the full field list (`BrowserQuery.java:44-83`) and all builder methods. + +**There is no raw query passthrough to lean on**: `BrowserQuery.luceneQuery` (`:66`) is written +only by `withFilter` / `withFileName` and is never read by `BrowserAPIImpl`. The clauses must be +built explicitly. + +**Rationale for keeping `showArchived` as-is**: the legacy Site Browser's "Show Archived" checkbox +(`view_browser.jsp:145`) depends on the inclusive meaning. Both questions are legitimate and both +must stay expressible, so the exclusive behavior is added *alongside* rather than replacing. + +**Alternatives considered**: redefining `showArchived` to be exclusive and adding an +`includeArchived` for the legacy path. Rejected — it inverts the meaning of a flag with callers +outside this feature's blast radius, for no gain. + +--- + +## R2: AND, not OR — and why that differs from every other Content Drive filter + +**Decision**: Selected statuses combine with **AND**. + +**Rationale**: the three are independent boolean facts about the *same* `contentlet_version_info` +row. An item can be unpublished *and* locked at once, so intersecting them is meaningful and is the +driving use case (US4). The existing multiselects — base type, content type, language — are OR +because each is a *single-valued* attribute: an item has exactly one base type, so an AND across +two would always be empty. + +**Precedent**: the legacy Content Search portlet has always combined these three additively +(`ContentletAjax.java:1010-1021`): + +```java +if (!showDeleted) "+deleted:false" else "+deleted:true" +if (filterLocked) "+locked:true" +if (filterUnpublish) "+live:false" +``` + +Its UI exposes them as a mutually exclusive dropdown (`view_contentlets.jsp:695-698`), but that was +a presentation simplification, never a domain constraint. Content Drive keeps the additive backend +and gives it a control that can actually express it. + +**Consequence**: each status is a flat, independent SQL clause. No composed OR group is needed, so +this lands well under the footprint of the archive-step work (`f92f939296`, 184 impl lines). + +--- + +## R3: The DB predicates + +**Decision**: + +| Status | SQL clause | ES term | +|---|---|---| +| Archived | `cvi.deleted = ` | `+deleted:true` | +| Unpublished | `cvi.live_inode is null` | `+live:false` | +| Locked | `cvi.locked_by is not null` | `+locked:true` | + +**Verification**: all three columns exist on `contentlet_version_info` (`postgres.sql:550-552`: +`live_inode varchar(36)`, `deleted bool not null`, `locked_by varchar(100)`). The `locked` field is +mapped into the index (`ESMappingAPIImpl.java:527`). `cvi` is already the alias in +`buildSelectBaseQuery` (`BrowserAPIImpl.java:2040`), and `DbConnectionFactory.getDBTrue()/getDBFalse()` +is the established way to write a boolean literal in this file. + +**Alternatives considered**: `live_inode <> working_inode` for Unpublished. Rejected — that is +"has unpublished changes", a third distinct question, and it is false for content that was never +published at all. + +--- + +## R4: The `showWorking` derivation must learn about the new statuses + +**Decision**: `BrowserQuery`'s constructor line + +```java +this.showWorking = builder.showWorking || builder.showArchived; // :151 +``` + +must also be true when `ARCHIVED` or `UNPUBLISHED` is selected. + +**Rationale**: `selectQuery` (`:1947`) picks the joined inode column from this flag: + +```java +final String workingLiveInode = browserQuery.showWorking || browserQuery.showArchived + ? "working_inode" : "live_inode"; +``` + +and the base query joins `c.inode = cvi.` (`:2043`). Archived and unpublished rows have +**no live version by definition**, so under `live_inode` the join can never match and the filter +would silently return nothing. The same flag drives `buildPureESQuery`'s `+working:true` vs +`+live:true` (`:615`), where `+live:true` alongside `+live:false` would be self-contradicting. + +The Content Drive path happens to be safe today (the form's `live()` defaults to `false`), but the +flag has to be correct for **any** caller of `BrowserQuery`, and relying on a coincidence in one +caller is exactly the kind of drift ADR-0018 was written to stop. + +--- + +## R5: Two existing branches already have an opinion about `cvi.deleted` + +This is the only genuinely delicate part of the change. Both branches must learn about `ARCHIVED`. + +### R5a — The global archived exclusion + +Today (`BrowserAPIImpl.java:2006`): + +```java +if (!browserQuery.showArchived && archiveStepIds.isEmpty()) { + appendExcludeArchivedQuery(selectQuery); // and cvi.deleted = false +} +``` + +**Decision**: add `&& !statuses.contains(ARCHIVED)`. + +Without it, `cvi.deleted = false` **and** `cvi.deleted = true` would both be emitted and `ARCHIVED` +would return nothing, always. With it, `UNPUBLISHED`/`LOCKED` selected alone still keep the +exclusion — which is precisely FR-004's "excludes archived content unless Archived is also +selected". The requirement and the code shape line up exactly; no extra logic is needed to get it. + +### R5b — Archive-target workflow steps + +`archiveStepIds` is deliberately emptied when `showArchived` (`:1980`), and the existing comment +says why: `appendWorkflowQuery` otherwise **owns** `cvi.deleted` per branch (`:2342-2360`), forcing +`cvi.deleted = false` on the live branch and hiding the archived content the caller asked for. + +**Decision**: `ARCHIVED` gets identical treatment: + +```java +final boolean admitsArchived = browserQuery.showArchived || statuses.contains(ARCHIVED); +final Set archiveStepIds = admitsArchived + ? Set.of() + : resolveArchiveTargetSteps(browserQuery.workflowStepIds); +``` + +This is the FR-011 / `ContentDriveWorkflowArchiveStepTest` regression case. It reuses the mechanism +the archive-step work already built rather than inventing a second reconciliation, and with no +status selected every generated query stays **byte-identical**. + +**Alternatives considered**: folding the status clauses into `appendWorkflowQuery`'s branch +structure. Rejected — status and workflow are orthogonal filters, and coupling them would make each +one harder to reason about for no behavioral gain. + +--- + +## R6: Which query path actually runs, and why both must be patched + +**Decision**: implement in the SQL path (`selectQuery`) **and** in `buildPureESQuery`. + +**Findings**: `doElasticSearchTextFiltering` (`:479`) switches on `BROWSE_API_HEURISTIC_TYPE`, +defaulting to `HYBRID_SINGLE_CHUNKED_QUERY_ES` (`:697`). + +- Under the **default hybrid** heuristic the SQL query supplies the ordered candidate inode set and + the index only narrows each chunk by the text term. So the SQL clauses apply **with or without** + text in the search box — this is what satisfies FR-009's "identical with and without a keyword". +- `buildPureESQuery` — with its hardcoded `+deleted:false` at `:612` — runs **only** under + `BROWSE_API_HEURISTIC_TYPE=PURE_ES`. + +`PURE_ES` is not the default and ADR-0018 says it must not become one, but it is a **supported +configuration**. Leaving it unpatched would make all three filters silently no-op there, returning a +wider set than the user asked for. That is FR-009 and SC-005. + +--- + +## R7: Rejecting an invalid status value + +**Decision**: the form declares `List status()`; `ContentDriveHelper` parses it into the +enum and throws `BadRequestException` naming the accepted values. + +**Rationale**: FR-010 asks for a 400 "consistent with how the drive already rejects unknown +field-filter keys", and that precedent is an explicit `BadRequestException` thrown in +`ContentDriveHelper.driveSearch` (the `userSearchable` guard, ~line 180). Keeping the same shape +means one error path, one message style, and a status code we control directly. + +**Alternatives considered**: declaring the field as `List` and letting Jackson +reject. Rejected — deserialization failures surface as `InvalidFormatException` from the immutables +layer, whose mapping to a 400 with a useful message is less direct than throwing it ourselves. The +typed form field is marginally prettier; the deterministic error is worth more. + +--- + +## R8: Where the new enum lives + +**Decision**: `com.dotcms.browser.ContentStatus` — `ARCHIVED`, `UNPUBLISHED`, `LOCKED`. + +**Rationale**: `BrowserQuery` is the real consumer, and `com.dotcms.browser` already holds this +kind of query-shaping type (`FieldSearchCriteria`, with its own `RoutingBucket` enum). Placing it in +`com.dotcms.rest.api.v1.drive` would make the browser layer depend on the REST layer. + +Constitution Principle I is satisfied: entirely modern `com.dotcms.*`, nothing added to +`com.dotmarketing.*`. + +--- + +## R9: Frontend — reuse, don't rebuild + +**Decision**: model the control on the workflow filter, but single-column and static. + +**Findings** — everything needed already exists: + +| Need | Existing thing to reuse | +|---|---| +| Chip + active state + overflow label | `DotChipFilterComponent` (`@dotcms/ui`), `mode="dropdown"` | +| Popover + listbox styling | `CHIP_FILTER_POPOVER_PT`, `CHIP_FILTER_LISTBOX_PT`, `PANEL_SCROLL_HEIGHT` | +| Row label with truncation | `DotFilterListItemComponent` | +| Read / write / clear a filter | store `getFilterValue` / `patchFilters` / `removeFilter` | +| URL encode | `encodeFilters` — already comma-joins any array value, no change needed | +| URL decode | one line in `decodeByFilterKey`: `status: multiSelector` | +| "Clear all" visibility + clearing | `hasNonDefaultFilters` / `clearFilters` — both work unchanged | + +The workflow filter carries a service, two caches, a request-id guard and a reconcile pass, because +its options are fetched and can disappear between loads. The status options are a fixed set of +three. **None of that machinery should be copied** — the component is a `linkedSignal` over the +filter value plus a checkbox list. + +**Decision on defaults**: `status` is deliberately **not** added to `withFilterDefaults`. Unlike +`languageId` and `sharedAssets` — where "absent" is not a neutral state and must be seeded — an +empty status set genuinely means "no status filtering". Leaving it unseeded makes +`hasNonDefaultFilters` return `true` whenever a status is present (so "Clear all" appears) and makes +`clearFilters()` drop it, both for free. + +--- + +## R10: Removing the hardcoded `archived: false` + +**Decision**: delete the `archived: false` pin from `$request` in `dot-content-drive.store.ts:135` +and send `status` instead. + +**Verification**: safe to simply remove — `AbstractDriveRequestForm.archived()` is +`@Value.Default default boolean archived() { return false; }`, so omitting it from the payload +produces the identical query. This is FR-019, and it is what stops the request from contradicting +the user's Archived selection. + +--- + +## Open questions + +**None.** No `NEEDS CLARIFICATION` markers were carried out of the spec, and every technical +uncertainty above was closed by reading the code rather than by assumption. From b154169a905e8f9e322640b0a8d6d09be96928b9 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 11:12:11 -0300 Subject: [PATCH 05/16] docs(content-drive): address spec review on the archived baseline (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two non-blocking notes from the automated review on #37170, both verified against the code before acting: 1. FR-004 vs FR-006 read as a contradiction to a strict "pure AND of three independent flags" implementer. They are not: excluding archived content is the drive's pre-existing baseline (appendExcludeArchivedQuery runs on every request today), the statuses AND on top of it, and ARCHIVED is the only one that lifts it. data-model.md now spells this out with a per-selection predicate table, and contracts/ points at it. 2. LOCKED and version scoping compose, as the reviewer suspected. Confirmed in the legacy portlet: ContentletAjax.java:1018 appends +locked:true and :1035 unconditionally appends +working:true. Documented, including the one deliberate difference — legacy always scopes to working, whereas here only ARCHIVED and UNPUBLISHED force it, so live:true + LOCKED is a coherent "live content that is locked" query rather than a bug. Adds the two integration cases the review asked for, including the fixture note that the FR-004 assertion needs an item that is both archived and unpublished or it passes vacuously. Co-Authored-By: Claude Opus 5 (1M context) --- .../contracts/drive-search-status.md | 5 +++ .../data-model.md | 39 +++++++++++++++++++ .../37066-content-drive-status-filter/plan.md | 2 + 3 files changed, 46 insertions(+) diff --git a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md index 5aff4f2da52d..5d4374e81ed5 100644 --- a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md +++ b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md @@ -39,6 +39,11 @@ Resource: `dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ContentDriveResourc **AND, not OR.** Unlike `contentTypes`, `baseTypes` and `language` — single-valued attributes where an intersection is always empty — these are independent flags one item can hold at once. +**The archived exclusion is a baseline, not a fourth value.** Every drive request already excludes +archived content; the statuses AND on top of that, and `ARCHIVED` is the only one that lifts it. +That is why `["UNPUBLISHED"]` means "unpublished **and not archived**" without contradicting the AND +rule. See [../data-model.md](../data-model.md) for the per-selection predicate table. + ### Side effects on other fields | Field | Effect when `status` is non-empty | diff --git a/specs/37066-content-drive-status-filter/data-model.md b/specs/37066-content-drive-status-filter/data-model.md index 0c8327232b89..50baa6493fcd 100644 --- a/specs/37066-content-drive-status-filter/data-model.md +++ b/specs/37066-content-drive-status-filter/data-model.md @@ -67,8 +67,47 @@ already in `ContentDriveHelper`. | Empty is valid and means "no status filtering" | FR-001, FR-002 | `ContentDriveHelper` (block is skipped) | | Every element must name a `ContentStatus` | FR-010 | `ContentDriveHelper.parseStatuses` → `BadRequestException` | | Selection narrows (AND), never widens | FR-006 | `BrowserAPIImpl.appendContentStatusQuery` — independent `and` clauses | +| The archived baseline stands unless `ARCHIVED` is selected | FR-004 | `BrowserAPIImpl:2006` — the exclusion is skipped only when the selection contains `ARCHIVED` | | A non-empty selection excludes folders | FR-015 | `ContentDriveHelper` → `.showFolders(false)` | +### The archived baseline is not a fourth flag + +FR-006 says the statuses combine with AND, and FR-004 says `UNPUBLISHED` excludes archived content +unless `ARCHIVED` is also selected. Read as "a pure AND of three independent flags", those look like +they disagree. They don't, and an implementer who misses the distinction will get `UNPUBLISHED` +wrong. + +**Excluding archived content is the drive's pre-existing default, not a member of this set.** +`appendExcludeArchivedQuery` already emits `cvi.deleted = false` on every request today. The three +statuses are AND-ed *on top of* that baseline; `ARCHIVED` is the only one that lifts it. + +So the generated predicate is: + +| Selection | Baseline | Status clauses | Net | +|---|---|---|---| +| `[]` | `deleted = false` | — | today's behavior | +| `[UNPUBLISHED]` | `deleted = false` | `live_inode is null` | unpublished **and not archived** | +| `[LOCKED]` | `deleted = false` | `locked_by is not null` | locked **and not archived** | +| `[ARCHIVED]` | *lifted* | `deleted = true` | archived only | +| `[ARCHIVED, LOCKED]` | *lifted* | `deleted = true` + `locked_by is not null` | archived **and** locked | + +This falls out of the code shape rather than needing special handling: the baseline is skipped only +when the selection contains `ARCHIVED`, so `UNPUBLISHED`/`LOCKED` alone keep it automatically. + +*(Raised by the automated spec review on [#37170](https://github.com/dotCMS/core/pull/37170).)* + +### `LOCKED` and version scoping compose + +`LOCKED` does not constrain which version is joined, so it stacks on whatever `showWorking` already +selected. That is the same pairing the legacy portlet uses: `ContentletAjax.java:1018` appends +`+locked:true` and `:1035` unconditionally appends `+working:true`. + +One deliberate difference: legacy **always** scopes to the working version, whereas here the drive +scopes to working because `AbstractDriveRequestForm.live()` defaults to `false`. A caller that sets +`live: true` with `status: ["LOCKED"]` therefore gets "live content that is locked" — a coherent, +strictly more expressive query, not a bug. Only `ARCHIVED` and `UNPUBLISHED` force working-version +scoping, because neither state can have a live version at all. + --- ## Query shape: `BrowserQuery.contentStatuses` diff --git a/specs/37066-content-drive-status-filter/plan.md b/specs/37066-content-drive-status-filter/plan.md index 6442fb76b285..b7c2e3867257 100644 --- a/specs/37066-content-drive-status-filter/plan.md +++ b/specs/37066-content-drive-status-filter/plan.md @@ -75,6 +75,8 @@ confirmed **failing** for the right reason. | Status value parsing; unknown value → 400 | Unit (JUnit) | `dotCMS/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveHelperStatusTest.java` | Mirrors `ContentDriveFieldFilterResolverTest`. No container needed | | Each status alone; each pair; all three; empty default | Integration | `dotcms-integration/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveStatusFilterTest.java` | Follows `ContentDriveWorkflowFilterTest`: dedicated site + folder + a purpose-built content type + unique id, `@AfterClass` cleanup. Never asserts against shared default content types | | `ARCHIVED + UNPUBLISHED` == `ARCHIVED`; `ARCHIVED + LOCKED` reachable | Integration | same class | FR-007 and the second Edge Case — asserted as documented behavior | +| `UNPUBLISHED` alone excludes an archived item; `ARCHIVED + UNPUBLISHED` admits it | Integration | same class | FR-004's baseline carve-out. Fixture needs an item that is **both** archived and unpublished, or the assertion passes vacuously. Raised by the automated spec review on #37170 | +| `LOCKED` composes with working-version scoping | Integration | same class | FR-005. Assert a locked item is returned under the drive's default (`live: false`), matching the legacy `+locked:true` / `+working:true` pairing (`ContentletAjax.java:1018`/`:1035`). Raised by the same review | | Parity with and without free text (default hybrid heuristic) | Integration | same class | FR-009 / SC-005 | | Parity under `BROWSE_API_HEURISTIC_TYPE=PURE_ES` | Integration | same class | Config override + restore in the test; the only coverage `buildPureESQuery` gets | | Status + archive-target workflow step | Integration | same class | FR-011. The regression this change is most likely to cause | From fe40cd3405c9f84a89fe420db3c5d5a504c0611a Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 11:13:14 -0300 Subject: [PATCH 06/16] chore(speckit): stop tracking .specify/feature.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I added this to the spec PR by mistake. It is not a spec artifact: it is a per-developer pointer to the feature you are currently on, rewritten by /speckit-specify and read by get_feature_paths() so the downstream commands can locate the spec folder without depending on branch naming. Committing it would aim every teammate's next /speckit-plan at whatever feature last merged, and two concurrent feature branches would conflict on its single line every time. It has never been committed on main — the 36605 and 36834 features both shipped without it — but it was not gitignored either, so it was untracked by luck rather than by rule. Now it is a rule. Callers who want to target a feature explicitly should set SPECIFY_FEATURE_DIRECTORY, which takes precedence over the file. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 5 +++++ .specify/CUSTOMIZATIONS.md | 8 ++++++++ .specify/feature.json | 1 - 3 files changed, 13 insertions(+), 1 deletion(-) delete mode 100644 .specify/feature.json diff --git a/.gitignore b/.gitignore index 1bfd9501a3e9..dd78765c194d 100644 --- a/.gitignore +++ b/.gitignore @@ -230,3 +230,8 @@ dist/ # regenerated by their own command, and go stale fastest, stay local. specs/*/tasks.md specs/*/checklists/ + +# Per-developer pointer to the feature you are currently working on, rewritten by +# /speckit-specify. Committing it would aim everyone else's /speckit-plan at your +# feature, and every concurrent feature branch would collide on its single line. +.specify/feature.json diff --git a/.specify/CUSTOMIZATIONS.md b/.specify/CUSTOMIZATIONS.md index 35bf2ba21ff3..eb7df9ec43a1 100644 --- a/.specify/CUSTOMIZATIONS.md +++ b/.specify/CUSTOMIZATIONS.md @@ -48,6 +48,14 @@ process artifact**: The never-commit set is enforced by `.gitignore` (`specs/*/tasks.md`, `specs/*/checklists/`). +**`.specify/feature.json` is never committed either.** It is not a spec artifact — it is a +per-developer pointer to the feature *you* are currently on, rewritten by `/speckit-specify` and +read by `get_feature_paths()` so the downstream commands can find the spec folder without relying +on branch naming. Committing it would aim every teammate's next `/speckit-plan` at whatever feature +last merged, and two concurrent feature branches would conflict on its single line every time. It is +gitignored for that reason. To point the commands at a feature explicitly, set +`SPECIFY_FEATURE_DIRECTORY` instead — it takes precedence over the file. + ### Why this changed (2026-08-24) The original policy kept `plan.md`, `research.md` and `quickstart.md` local too, on the diff --git a/.specify/feature.json b/.specify/feature.json deleted file mode 100644 index 4081e8f69aec..000000000000 --- a/.specify/feature.json +++ /dev/null @@ -1 +0,0 @@ -{"feature_directory":"specs/37066-content-drive-status-filter"} From a6d2945366ac9eaf47df761c1460768634b2f7da Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 11:26:55 -0300 Subject: [PATCH 07/16] docs(content-drive): status filter combines with OR, not AND (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverses the ticket's original AND semantics after review. The case for AND was that these are independent flags one item can hold at once, so intersecting them is meaningful. It is — for one of four combinations. Under AND, ARCHIVED+UNPUBLISHED is redundant (archiving removes the live version, so every archived item is already unpublished), ARCHIVED+LOCKED is almost always empty, and all three is empty in practice. Only UNPUBLISHED+LOCKED says anything. Under OR all four are meaningful. The decisive point is consistency: every other chip in that toolbar row widens on selection, and no UI affordance can convey that one of them inverts the rule. A user who checks a second box and gets fewer results reads that as a bug. The other filters' OR-ness being forced by their single-valued nature is invisible to the user; all they learn is the pattern. Accepted cost, recorded in Assumptions: "unpublished AND locked" is no longer expressible in Content Drive. Implementation shape changes from N independent `and` clauses to one OR-ed group AND-ed against the archived baseline, which stays OUTSIDE the group — folding it in would make [UNPUBLISHED, LOCKED] read `(deleted = false or ...)` and match nearly every row. Also settles the toolbar position (between the shared-assets and content-type filters, so the row reads broadest-scope-first) and makes the navigation requirement explicit: status persists across deep link, reload, folder browsing, Back/Forward and an editor round-trip, exactly like every other filter, because it rides the shared filters bag rather than its own query param. Co-Authored-By: Claude Opus 5 (1M context) --- .../contracts/drive-search-status.md | 35 ++-- .../data-model.md | 81 +++++---- .../37066-content-drive-status-filter/spec.md | 156 ++++++++++-------- 3 files changed, 157 insertions(+), 115 deletions(-) diff --git a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md index 5d4374e81ed5..a6e8696aba94 100644 --- a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md +++ b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md @@ -21,7 +21,7 @@ Resource: `dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ContentDriveResourc | Field | Type | Required | Default | Description | |---|---|---|---|---| -| `status` | `string[]` | No | `[]` | Content states to narrow by. Accepted: `ARCHIVED`, `UNPUBLISHED`, `LOCKED`. Entries combine with **AND**. | +| `status` | `string[]` | No | `[]` | Content states to filter by. Accepted: `ARCHIVED`, `UNPUBLISHED`, `LOCKED`. Entries combine with **OR**. | ### Semantics @@ -31,18 +31,20 @@ Resource: `dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ContentDriveResourc | `["ARCHIVED"]` | Only archived content | | `["UNPUBLISHED"]` | Only content with no live version, archived excluded | | `["LOCKED"]` | Only content with a lock held, by anyone, archived excluded | -| `["UNPUBLISHED","LOCKED"]` | Content that is both — the driving multiselect use case | -| `["ARCHIVED","UNPUBLISHED"]` | Same set as `["ARCHIVED"]` alone. Redundant, not an error | -| `["ARCHIVED","LOCKED"]` | Archived content that still holds a lock. Reachable; often empty | -| all three | Archived **and** unpublished **and** locked | +| `["UNPUBLISHED","LOCKED"]` | Content that is unpublished **or** locked, archived excluded | +| `["ARCHIVED","UNPUBLISHED"]` | Everything with no live version — archived **or** unpublished | +| `["ARCHIVED","LOCKED"]` | Archived content **or** content with a lock held | +| all three | Anything not cleanly published — archived, unpublished **or** locked | -**AND, not OR.** Unlike `contentTypes`, `baseTypes` and `language` — single-valued attributes where -an intersection is always empty — these are independent flags one item can hold at once. +**OR, not AND.** Selecting more statuses returns *more* content, exactly like `contentTypes`, +`baseTypes` and `language`. Adding a status can never shrink the result set. -**The archived exclusion is a baseline, not a fourth value.** Every drive request already excludes -archived content; the statuses AND on top of that, and `ARCHIVED` is the only one that lifts it. -That is why `["UNPUBLISHED"]` means "unpublished **and not archived**" without contradicting the AND -rule. See [../data-model.md](../data-model.md) for the per-selection predicate table. +**The archived exclusion is a baseline, not a fourth value, and it sits outside the OR group.** +Every drive request already excludes archived content; the selected statuses are OR-ed together and +that group is AND-ed against the baseline, which only `ARCHIVED` lifts. That is why +`["UNPUBLISHED"]` means "unpublished and not archived" without contradicting the OR rule. Folding +the baseline into the group instead would make `["UNPUBLISHED","LOCKED"]` match nearly every row. +See [../data-model.md](../data-model.md) for the per-selection predicate table. ### Side effects on other fields @@ -61,7 +63,7 @@ supplies it. ### 200 — success Response shape is unchanged (`ResponseEntityView`). Only the contents of `list`, -and the counts, narrow. +and the counts, change. ### 400 — unrecognized status value @@ -119,12 +121,15 @@ CI verifies the committed file matches what the build produces. `DotContentDriveSearchRequest.status?: string[]` in `core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts`. -URL round-trip via the shared filter bag, so a filtered view is shareable and survives reload -(FR-016): +The value lives in the shared `filters` bag rather than its own query param, so it inherits every +navigation mechanism the other filters already use — deep link, reload, folder browsing, browser +Back/Forward, and the legacy-editor round-trip (FR-016): ``` …?filters=languageId:1;sharedAssets:true;status:UNPUBLISHED,LOCKED ``` Encoding needs no new code — `encodeFilters` already comma-joins array values. Decoding is one entry -in `decodeByFilterKey` (`status: multiSelector`). +in `decodeByFilterKey` (`status: multiSelector`), which is **required**: without it a single-value +URL (`status:ARCHIVED`) falls through to the comma sniff and decodes as a string rather than an +array. diff --git a/specs/37066-content-drive-status-filter/data-model.md b/specs/37066-content-drive-status-filter/data-model.md index 50baa6493fcd..d155b18b73b7 100644 --- a/specs/37066-content-drive-status-filter/data-model.md +++ b/specs/37066-content-drive-status-filter/data-model.md @@ -1,6 +1,11 @@ # Phase 1 Data Model: Content Drive Status Filter -**Feature**: [spec.md](./spec.md) | **Plan**: [plan.md](./plan.md) | **Date**: 2026-08-24 +**Feature**: [spec.md](./spec.md) | **Date**: 2026-08-24 + +> `plan.md`, `research.md` and `quickstart.md` are Spec-Kit process artifacts and are gitignored by +> policy (`.specify/CUSTOMIZATIONS.md`), so the `research.md` references below point at files that +> exist on the author's machine, not in this repo. Each one is summarized inline so this document +> stands on its own. No database schema changes. Every column this feature reads already exists on `contentlet_version_info` and is already indexed into the search index. This document describes the @@ -24,16 +29,19 @@ A closed enum of three independent states a contentlet version can hold. and the browser layer must not depend on the REST layer. Sits alongside `FieldSearchCriteria`, which plays the same query-shaping role. -**Relationships**: none. The three are orthogonal facts about one row, which is exactly why they -combine with AND rather than OR (see [research.md R2](./research.md)). +**Relationships**: none. The three are orthogonal facts about one row. -**Not a state machine**: an item can hold any subset of the three at once. Two subsets are worth -naming because they are user-visible oddities, not defects: +**Not a state machine**: an item can hold any subset of the three at once. The filter asks whether +an item is in *any* selected state, not all of them — selected statuses combine with **OR**, like +the Content Type and Language filters. AND was considered and rejected: under AND, +`{ARCHIVED, UNPUBLISHED}` is redundant, `{ARCHIVED, LOCKED}` is almost always empty and all three is +empty in practice, so only one of four combinations says anything — and the chip would be the sole +exception in a toolbar row where every other filter widens on selection (research.md R2). -- `{ARCHIVED, UNPUBLISHED}` is always equivalent to `{ARCHIVED}` — archiving removes the live - version (`ESContentletAPIImpl.java:3833`), so every archived item is already unpublished. -- `{ARCHIVED, LOCKED}` is reachable but rare: it needs a self-lock or a CMS-Admin archive - (`canLock` at `:10380`/`:10406`), and `internalArchive` never clears `locked_by`. +One overlap is worth knowing even though it no longer produces a degenerate result: every archived +item is also unpublished, because archiving removes the live version +(`ESContentletAPIImpl.java:3833`). Under OR that just means `{ARCHIVED, UNPUBLISHED}` reads as +"everything with no live version" rather than being redundant. --- @@ -56,9 +64,10 @@ default List status() { return List.of(); } | Duplicates | Collapsed; the parsed result is a `Set` | | Unknown value | `400`, message naming the accepted values (FR-010) | -**Declared as `List`, not `List`** — see [research.md R7](./research.md). The -helper owns the parse so the 400 is thrown explicitly, matching the `userSearchable` precedent -already in `ContentDriveHelper`. +**Declared as `List`, not `List`** (research.md R7): a typed field would +route an invalid value through Jackson's `InvalidFormatException`, whose mapping to a useful 400 is +less direct than throwing one ourselves. The helper owns the parse instead, matching the +`userSearchable` precedent already in `ContentDriveHelper`. ### Validation rules @@ -66,35 +75,37 @@ already in `ContentDriveHelper`. |---|---|---| | Empty is valid and means "no status filtering" | FR-001, FR-002 | `ContentDriveHelper` (block is skipped) | | Every element must name a `ContentStatus` | FR-010 | `ContentDriveHelper.parseStatuses` → `BadRequestException` | -| Selection narrows (AND), never widens | FR-006 | `BrowserAPIImpl.appendContentStatusQuery` — independent `and` clauses | -| The archived baseline stands unless `ARCHIVED` is selected | FR-004 | `BrowserAPIImpl:2006` — the exclusion is skipped only when the selection contains `ARCHIVED` | +| Selection widens (OR), never narrows | FR-006 | `BrowserAPIImpl.appendContentStatusQuery` — one OR-ed group | +| The archived baseline stands unless `ARCHIVED` is selected | FR-007 | `BrowserAPIImpl:2006` — the exclusion is skipped only when the selection contains `ARCHIVED` | | A non-empty selection excludes folders | FR-015 | `ContentDriveHelper` → `.showFolders(false)` | -### The archived baseline is not a fourth flag - -FR-006 says the statuses combine with AND, and FR-004 says `UNPUBLISHED` excludes archived content -unless `ARCHIVED` is also selected. Read as "a pure AND of three independent flags", those look like -they disagree. They don't, and an implementer who misses the distinction will get `UNPUBLISHED` -wrong. +### The archived baseline is not a fourth flag, and it lives outside the OR group -**Excluding archived content is the drive's pre-existing default, not a member of this set.** -`appendExcludeArchivedQuery` already emits `cvi.deleted = false` on every request today. The three -statuses are AND-ed *on top of* that baseline; `ARCHIVED` is the only one that lifts it. +Excluding archived content is the drive's **pre-existing default**, not a member of this set: +`appendExcludeArchivedQuery` already emits `cvi.deleted = false` on every request today. The status +group is OR-ed internally and AND-ed against that baseline; `ARCHIVED` is the only status that lifts +it. -So the generated predicate is: - -| Selection | Baseline | Status clauses | Net | +| Selection | Baseline | Status group | Net | |---|---|---|---| | `[]` | `deleted = false` | — | today's behavior | -| `[UNPUBLISHED]` | `deleted = false` | `live_inode is null` | unpublished **and not archived** | -| `[LOCKED]` | `deleted = false` | `locked_by is not null` | locked **and not archived** | -| `[ARCHIVED]` | *lifted* | `deleted = true` | archived only | -| `[ARCHIVED, LOCKED]` | *lifted* | `deleted = true` + `locked_by is not null` | archived **and** locked | +| `[UNPUBLISHED]` | `deleted = false` | `(live_inode is null)` | unpublished, not archived | +| `[LOCKED]` | `deleted = false` | `(locked_by is not null)` | locked, not archived | +| `[UNPUBLISHED, LOCKED]` | `deleted = false` | `(live_inode is null or locked_by is not null)` | either, still not archived | +| `[ARCHIVED]` | *lifted* | `(deleted = true)` | archived only | +| `[ARCHIVED, UNPUBLISHED]` | *lifted* | `(deleted = true or live_inode is null)` | everything with no live version | +| all three | *lifted* | `(deleted = true or live_inode is null or locked_by is not null)` | anything not cleanly published | + +**The bug to avoid** is folding the baseline into the group. `[UNPUBLISHED, LOCKED]` would then read +`(deleted = false or live_inode is null or locked_by is not null)`, which matches essentially every +row in the folder — a filter that silently stops filtering. -This falls out of the code shape rather than needing special handling: the baseline is skipped only -when the selection contains `ARCHIVED`, so `UNPUBLISHED`/`LOCKED` alone keep it automatically. +Note that a single-status selection produces a one-disjunct group, so `[ARCHIVED]` is still exactly +`cvi.deleted = true`. OR and AND only diverge from two statuses upward. -*(Raised by the automated spec review on [#37170](https://github.com/dotCMS/core/pull/37170).)* +*(The baseline-vs-flag distinction was raised by the automated spec review on +[#37170](https://github.com/dotCMS/core/pull/37170); the OR semantics were settled separately during +planning, see the OR rationale above.)* ### `LOCKED` and version scoping compose @@ -131,8 +142,8 @@ this.showWorking = builder.showWorking || builder.showArchived; ``` must also be true when the selection contains `ARCHIVED` or `UNPUBLISHED`. Both states imply no live -version, so without this the query joins on `live_inode` and can never match. See -[research.md R4](./research.md). +version, so without this `selectQuery` joins on `live_inode` — which those rows never have — and the +filter silently returns nothing (research.md R4). --- diff --git a/specs/37066-content-drive-status-filter/spec.md b/specs/37066-content-drive-status-filter/spec.md index 23cc81468dc0..f0b1ff74c189 100644 --- a/specs/37066-content-drive-status-filter/spec.md +++ b/specs/37066-content-drive-status-filter/spec.md @@ -10,7 +10,7 @@ **GitHub Issue**: [dotCMS/core#37066](https://github.com/dotCMS/core/issues/37066) (absorbed [#37067](https://github.com/dotCMS/core/issues/37067); parent epic [#33999](https://github.com/dotCMS/core/issues/33999)) -**Input**: User description: "Content Drive needs a Status filter (Archived, Unpublished, Locked): the search clauses on the drive search endpoint and the multiselect in the toolbar. None of the three predicates is expressible on the endpoint today. Selecting several narrows the results (AND), because they are independent states an item can hold at the same time. Nothing selected must mean exactly today's behavior: archived hidden, everything else returned." +**Input**: User description: "Content Drive needs a Status filter (Archived, Unpublished, Locked): the search clauses on the drive search endpoint and the multiselect in the toolbar. None of the three predicates is expressible on the endpoint today. Nothing selected must mean exactly today's behavior: archived hidden, everything else returned." --- @@ -20,12 +20,22 @@ This is **one vertical slice**: the search capability and the control that drive The ticket originally split them across two issues; #37067 was merged into #37066 because each half restated the same contract and duplicated contract text is where drift starts. -The Status filter is deliberately **not** the same shape as the existing Content Drive filters. -Content type, base type and language are single-valued attributes, so selecting several is an OR -("either of these types"). Status is a set of independent flags a single item can hold at once, so -selecting several is an AND ("both unpublished *and* locked"). This difference is the reason the -control is a multiselect rather than a dropdown, and it is the single most important thing for a -reader to carry into planning. +**Selected statuses combine additively (OR), exactly like the Content Type and Language filters +beside it.** Checking more boxes returns more content, never less. `Archived + Unpublished` means +"archived **or** unpublished" — everything with no live version — not the intersection of the two. + +This is a deliberate reversal of the ticket's original wording, which specified AND. The argument +for AND was that these are independent flags one item can hold at once, so intersecting them is +meaningful. It is — but only for one of the four possible combinations. Under AND, +`Archived + Unpublished` is redundant (archiving removes the live version, so every archived item is +already unpublished), `Archived + Locked` is almost always empty, and all three together is empty in +practice. Only `Unpublished + Locked` says something useful. Under OR every combination is +meaningful, and the control stops behaving as the sole exception in a row of filters that all widen +when you check more boxes — a difference no UI affordance can convey and that a user would read as +a bug. + +The cost is accepted and recorded in Assumptions: "unpublished **and** locked" is no longer +expressible in Content Drive. --- @@ -36,7 +46,7 @@ reader to carry into planning. An editor is looking for a page a colleague archived last week, to check what it said before deciding whether to restore it. Content Drive hides archived content by default, so today the only way to see it is to leave Content Drive for the legacy Content Search portlet. The editor selects -**Archived** in the Status filter and the drive lists archived items — and only archived items. +**Archived** and the drive lists archived items — and only archived items. **Why this priority**: This is the capability people currently leave Content Drive to get. It is also the only one of the three that is *partially* present today in a form that does the wrong @@ -77,8 +87,6 @@ Unpublished. Only the never-published item is listed. Unpublished, **Then** only the unpublished, non-archived items are listed. 2. **Given** an item that was published and then unpublished, **When** the editor selects Unpublished, **Then** that item is listed. -3. **Given** Unpublished is selected, **When** the editor also selects Archived, **Then** archived - items are admitted to the results (see Edge Cases for why this pair reads as it does). --- @@ -104,64 +112,71 @@ locked item is listed. --- -### User Story 4 - Combining statuses to ask a sharper question (Priority: P2) +### User Story 4 - Seeing everything that is not cleanly published (Priority: P2) -A manager wants "drafts currently checked out by someone" — work in progress that is both blocked and -unpublished. They select **Unpublished** and **Locked** together and the drive lists only content -that is both. +Before handing a section over, a lead wants one view of everything needing attention: drafts, +archived items and anything checked out. They select all three statuses and the drive lists content +in *any* of those states, rather than making them run three separate passes and reconcile the +results by hand. -**Why this priority**: This is the reason the control is a multiselect rather than a dropdown. A -single-select could not express it, and the combination is a question content teams actually ask. -It is P2 rather than P1 because it builds on stories 2 and 3 rather than standing alone. +**Why this priority**: This is the reason the control is a multiselect rather than a dropdown. Each +status alone is already useful (stories 1–3), so this builds on them rather than standing alone — +but the union is the question a lead actually asks at review time, and no single status answers it. -**Independent Test**: Create four items covering every unpublished/locked combination. Select both -statuses. Only the item that is both unpublished and locked is listed. +**Independent Test**: Seed one archived, one unpublished, one locked and one plain live item. Select +all three statuses. The first three are listed and the live one is not. **Acceptance Scenarios**: -1. **Given** items covering all four unpublished/locked combinations, **When** both statuses are - selected, **Then** exactly the item holding both states is listed. -2. **Given** both statuses are selected, **When** one is cleared, **Then** the results widen to the +1. **Given** items in each of the three states plus a clean live item, **When** all three statuses + are selected, **Then** every item except the clean live one is listed. +2. **Given** Unpublished is selected, **When** the editor also selects Locked, **Then** the results + *widen* to include locked content that is not unpublished. +3. **Given** two statuses are selected, **When** one is cleared, **Then** the results narrow to the remaining status alone. --- -### User Story 5 - A filtered view survives reload and can be shared (Priority: P3) +### User Story 5 - A filtered view survives navigation and can be shared (Priority: P3) An editor sends a colleague a link to "everything unpublished in Marketing". The colleague opens the -link and sees the same filtered view, with the Status selection shown as an active chip. Reloading -the page keeps it. Clearing all filters removes it along with everything else. +link and sees the same filtered view, with the Status selection shown as an active chip. Browsing +into a subfolder keeps it. Browser Back returns to the previous view with the right filters. Opening +an item in the editor and coming back keeps it. Reloading keeps it. Clearing all filters removes it +along with everything else. **Why this priority**: Every other Content Drive filter behaves this way, so a Status filter that did not would read as broken. It is P3 only because the filter is useful before it is shareable. -**Independent Test**: Select two statuses, copy the address, open it in a new session. The same two -statuses are selected and the same results are listed. +**Independent Test**: Select two statuses, navigate into a subfolder, press Back, then reload. The +selection and the results are the same at every step. **Acceptance Scenarios**: 1. **Given** a Status selection, **When** the page is reloaded, **Then** the selection and results are unchanged. -2. **Given** a Status selection, **When** "Clear all" is used, **Then** the Status selection is +2. **Given** a Status selection, **When** the editor browses into another folder, **Then** the + selection still applies in that folder. +3. **Given** a Status selection, **When** the editor uses browser Back or Forward, **Then** the + restored view carries the selection that URL had. +4. **Given** a Status selection, **When** the editor opens an item in the editor and returns, + **Then** the selection is still applied. +5. **Given** a Status selection, **When** "Clear all" is used, **Then** the Status selection is removed along with the other filters. -3. **Given** a Status selection, **When** the view is shared as a link, **Then** the recipient sees +6. **Given** a Status selection, **When** the view is shared as a link, **Then** the recipient sees the same selection. --- ### Edge Cases -- **Archived + Unpublished returns the same items as Archived alone.** Archiving an item removes its - live version, so every archived item is already unpublished. The pair is redundant, not broken. It - must not be presented as an error, an empty state, or a warning — it is simply a narrower question - whose answer happens to coincide with a wider one. -- **Archived + Locked is reachable but uncommon**, since it needs an item that was locked by its own - holder or archived by an administrator while a lock stood. An empty result there is legitimate and - shows the ordinary empty state, never an error. - **Folders have no status.** Whenever any status is selected the results are content only. This matches how the drive already behaves for the other narrowing filters. - **No status selected** must produce exactly the behavior that exists today — archived content hidden, everything else returned — with no change to result counts, ordering or pagination. +- **Archived is the only status that reveals archived content.** Selecting Unpublished or Locked + alone must not surface archived items, even though every archived item is technically also + unpublished. Hiding archived content is the drive's standing default, and only Archived lifts it. - **An unrecognized status value** submitted directly to the search endpoint is rejected with a clear client error naming the accepted values, rather than being silently ignored (which would return a wider result set than the caller asked for). @@ -173,6 +188,8 @@ statuses are selected and the same results are listed. keyword is present and how the environment is configured. Every strategy must apply the status filter identically, so the same selection never returns different results because of a configuration the user cannot see. +- **An empty result is still possible** — a folder with no content in any selected state. That shows + the standard empty state, never an error. ## Requirements *(mandatory)* @@ -182,15 +199,17 @@ statuses are selected and the same results are listed. Archived, Unpublished and Locked, defaulting to an empty set. - **FR-002**: An empty set MUST preserve today's behavior exactly: archived content excluded, all other content returned. -- **FR-003**: Archived MUST return only archived content, never archived content in addition to - everything else. -- **FR-004**: Unpublished MUST return only content with no live version, and MUST exclude archived - content unless Archived is also selected. -- **FR-005**: Locked MUST return only content on which a lock is held, regardless of who holds it. -- **FR-006**: Multiple selected statuses MUST combine with AND — the result is content holding every - selected state at once. -- **FR-007**: Selecting Archived together with Unpublished MUST return the same set as Archived - alone, and this MUST be documented rather than treated as a defect. +- **FR-003**: Archived alone MUST return only archived content, never archived content in addition + to everything else. +- **FR-004**: Unpublished alone MUST return only content with no live version, and MUST exclude + archived content. +- **FR-005**: Locked alone MUST return only content on which a lock is held, regardless of who holds + it, and MUST exclude archived content. +- **FR-006**: Multiple selected statuses MUST combine with **OR** — the result is content holding + *any* of the selected states. Adding a status MUST never reduce the result set. +- **FR-007**: Archived MUST be the only status that admits archived content into the results. + Selecting it alongside others widens the results to include archived content as well as content in + the other selected states. - **FR-008**: The existing inclusive "show archived" behavior relied on by the legacy Site Browser MUST be left unchanged; the exclusive Archived behavior is added alongside it. - **FR-009**: A status selection MUST produce identical results whether or not a keyword search is @@ -200,17 +219,18 @@ statuses are selected and the same results are listed. - **FR-011**: A status selection MUST NOT conflict with a workflow filter that also constrains archived content; the two MUST combine into one coherent result set. - **FR-012**: Users MUST be able to select any combination of the three statuses from a single - control in the Content Drive toolbar, alongside the existing filters. + control in the Content Drive toolbar, positioned between the shared-assets and content-type + filters so the row reads from broadest scope to narrowest. - **FR-013**: The control's labels MUST be localizable, following the Content Drive naming convention already used by the other filters. - **FR-014**: The active selection MUST be reflected as a chip, consistent with the other toolbar filters, and MUST be clearable from that chip. - **FR-015**: Folders MUST be excluded from the results whenever any status is selected. -- **FR-016**: A status selection MUST round-trip through the address bar like every other filter, so - a filtered view is shareable and survives a reload. +- **FR-016**: A status selection MUST persist across navigation exactly as every other Content Drive + filter does: deep link, page reload, browsing between folders, browser Back/Forward, and opening + an item in the editor and returning. - **FR-017**: The existing "Clear all" action MUST clear the status selection. -- **FR-018**: An empty result set arising from a legitimate combination MUST show the standard empty - state, never an error. +- **FR-018**: An empty result set MUST show the standard empty state, never an error. - **FR-019**: The Content Drive request MUST stop pinning archived content off unconditionally; that decision MUST come from the status selection instead. - **FR-020**: The control and each of its options MUST carry stable test identifiers, and the @@ -220,9 +240,10 @@ statuses are selected and the same results are listed. - **Content Status**: An independent state an item can hold, from a closed set of three — *Archived* (removed from circulation but recoverable), *Unpublished* (no live version), and *Locked* (checked - out by a user). An item may hold several at once, which is what makes the selection additive. + out by a user). An item may hold several at once, but the filter asks whether an item is in *any* + selected state, not all of them. - **Status Selection**: The set of statuses the user has chosen. Empty by default; every member - narrows the result set further. + widens the result set. ## Success Criteria *(mandatory)* @@ -232,26 +253,30 @@ statuses are selected and the same results are listed. without leaving for another part of the product. - **SC-002**: Each single status returns exactly the items in that state and no others, verified against a known fixture covering all three states plus unaffected content. -- **SC-003**: Every pair of statuses, and all three together, return exactly the items holding all - the selected states. +- **SC-003**: Every pair of statuses, and all three together, return exactly the union of the items + in the selected states — never fewer items than any one of them alone returns. - **SC-004**: With no status selected, result counts and ordering are identical to those produced before the filter existed, for the same folder and filters. - **SC-005**: The same status selection returns the same result set with and without a keyword search, and under every supported search strategy. -- **SC-006**: A shared link to a status-filtered view reproduces the same selection and the same - results for a second user. -- **SC-007**: No legitimate combination — including the redundant and the rarely-populated ones — - presents as an error to the user. +- **SC-006**: A status-filtered view reproduces the same selection and results after a reload, a + folder change, a Back/Forward, an editor round-trip, and when opened by a second user from a + shared link. +- **SC-007**: Selecting an additional status never returns fewer results than the selection did + before it was added. ## Legacy Considerations *(dotCMS-specific — mandatory)* - **Existing behavior touched**: The shared content-browsing capability behind both Content Drive (modern) and the Site Browser (legacy). The legacy Site Browser exposes a "Show Archived" checkbox whose meaning is *inclusive* — archived content **in addition to** everything else. The new - Archived status is *exclusive* — archived content **only**. These are different questions and both - must remain expressible; the new behavior is added alongside the old one rather than replacing it. - The legacy Content Search portlet already offers all three of these predicates and already combines - them additively, so this feature brings Content Drive to parity rather than inventing semantics. + Archived status is *exclusive* — archived content **only**, when selected alone. These are + different questions and both must remain expressible; the new behavior is added alongside the old + one rather than replacing it. + - The legacy Content Search portlet offers the same three predicates but combines them + **additively in the AND sense** on its backend, while exposing them as a mutually exclusive + dropdown in its UI. This spec deliberately follows neither: it keeps the multiselect but makes + it a union, because that is what matches the rest of the Content Drive toolbar. - **Backward-compatibility expectations**: The legacy Site Browser's "Show Archived" checkbox must behave exactly as before. Existing callers of the drive search capability that send no status must see byte-identical results. No content, stored configuration, or admin workflow changes. @@ -267,10 +292,11 @@ statuses are selected and the same results are listed. example "has a scheduled publish date") are out of scope. - "Locked" means a lock is held by anyone, not "locked by me". A per-user variant is not requested and would be a separate filter. +- **Intersective queries are out of scope.** "Unpublished **and** locked" — drafts currently checked + out — is not expressible through this control, and this is an accepted trade for a filter row with + one consistent mental model. If it proves to be a real need, it belongs in a later refinement that + makes the combining rule explicit in the UI rather than implicit and inverted. - Users of this filter already have permission to see the content it surfaces; the filter narrows a result set that permission checks have already constrained, and grants no new visibility. -- The redundancy of Archived + Unpublished is acceptable to expose rather than something to prevent - in the control. Disabling one option based on another would be a second, hidden rule the user has - to learn, and the combination is harmless. - The Shared Assets / System Host toggle is explicitly out of scope, tracked separately in [#34760](https://github.com/dotCMS/core/issues/34760). From c829fec9a2683482780ab154153fa86e8fcf6de5 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 11:27:13 -0300 Subject: [PATCH 08/16] Revert "chore(speckit): commit plan.md, research.md and quickstart.md" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores the commit policy from #36416: plan.md, research.md and quickstart.md are process artifacts and stay local. They remain on disk, so /speckit-implement and /speckit-converge are unaffected. I proposed un-ignoring them because a stacked design PR with a gitignored plan.md has nothing in it to review. That was solved differently instead — the spec and its design artifacts are now one PR, so there is no design layer needing its own diff. Keeps two things from the reverted change: - .specify/feature.json stays gitignored. It is a per-developer pointer, not a spec artifact, and had never been committed on main — untracked by luck rather than by rule. - data-model.md no longer links to the now-local research.md; each reference is summarized inline so the committed document stands on its own. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 7 +- .specify/CUSTOMIZATIONS.md | 40 +-- .../37066-content-drive-status-filter/plan.md | 284 ------------------ .../quickstart.md | 187 ------------ .../research.md | 256 ---------------- 5 files changed, 13 insertions(+), 761 deletions(-) delete mode 100644 specs/37066-content-drive-status-filter/plan.md delete mode 100644 specs/37066-content-drive-status-filter/quickstart.md delete mode 100644 specs/37066-content-drive-status-filter/research.md diff --git a/.gitignore b/.gitignore index dd78765c194d..41e94f07dd73 100644 --- a/.gitignore +++ b/.gitignore @@ -225,10 +225,11 @@ dist/ /core-web/scratch/ # Spec-Kit working artifacts — process-only, kept local (see .specify/CUSTOMIZATIONS.md). -# Everything else under specs/ is tracked, as upstream Spec-Kit intends: spec.md, plan.md, -# research.md, quickstart.md, data-model.md and contracts/. Only the two artifacts that are -# regenerated by their own command, and go stale fastest, stay local. +# spec.md (and data-model.md / contracts/ when they carry verified contracts) stay tracked. +specs/*/plan.md +specs/*/research.md specs/*/tasks.md +specs/*/quickstart.md specs/*/checklists/ # Per-developer pointer to the feature you are currently working on, rewritten by diff --git a/.specify/CUSTOMIZATIONS.md b/.specify/CUSTOMIZATIONS.md index eb7df9ec43a1..8a1dbca027b3 100644 --- a/.specify/CUSTOMIZATIONS.md +++ b/.specify/CUSTOMIZATIONS.md @@ -39,14 +39,18 @@ process artifact**: | Artifact | Commit? | Why | |----------|---------|-----| | `spec.md` | Always | the reviewed contract (FRs, user stories, success criteria) | -| `plan.md` | Always | the reviewed *design* — approach, Legacy Impact, Constitution Check, ADR Alignment. A reviewer cannot judge a design they cannot see | -| `research.md` | Always | why each decision went the way it did, and what was rejected. This is the artifact a future dev most often wants and can least reconstruct from the code | -| `quickstart.md` | Always | how to build, test and verify the feature — useful long after merge | | `data-model.md` | When it carries verified contracts | concrete entity→field/type, relationships, validation rules, real payload/DB shapes confirmed while building — the field-level ground truth `spec.md` stays above | | `contracts/` | Same test as `data-model.md` | committed API specs are durable; scaffolding is not | -| `tasks.md`, `checklists/` | Never | pure sequencing and in-flight bookkeeping; regenerated on demand by `/speckit-tasks` and `/speckit-checklist`, and stale within a day of implementation starting | +| `plan.md`, `research.md`, `tasks.md`, `quickstart.md`, `checklists/` | Never | pure process — how / what-order / decisions-in-flight | -The never-commit set is enforced by `.gitignore` (`specs/*/tasks.md`, `specs/*/checklists/`). +The never-commit set is enforced by `.gitignore` (`specs/*/plan.md`, `research.md`, +`tasks.md`, `quickstart.md`, `checklists/`). + +**`data-model.md` commit-worthiness bar:** commit it only if a future dev would need it +to know the shapes without reading the code. Its structure follows the plan template's +Phase 1 spec — *entity name, fields, relationships, validation rules from requirements*. +If a feature's `data-model.md` just restates entities already obvious from `spec.md`, it's +as ephemeral as the rest — don't commit it (same for `contracts/`). **`.specify/feature.json` is never committed either.** It is not a spec artifact — it is a per-developer pointer to the feature *you* are currently on, rewritten by `/speckit-specify` and @@ -56,32 +60,6 @@ last merged, and two concurrent feature branches would conflict on its single li gitignored for that reason. To point the commands at a feature explicitly, set `SPECIFY_FEATURE_DIRECTORY` instead — it takes precedence over the file. -### Why this changed (2026-08-24) - -The original policy kept `plan.md`, `research.md` and `quickstart.md` local too, on the -"process artifact" test. That was written before this repo adopted **stacked pull requests** -(GitHub's native stacks, public preview 2026-07-30, via the `github/gh-stack` extension). - -Stacks change the calculation. The spec-driven flow now lands as a stack — spec → design → -implementation — where each layer is reviewed on its own. A design layer whose `plan.md` is -gitignored is a PR with nothing in it to review, which is what surfaced the problem on -[#37066](https://github.com/dotCMS/core/issues/37066) (stack -[#37172](https://github.com/dotCMS/core/pull/37171)). The old rule was not wrong for the -one-spec-PR-plus-one-implementation-PR shape it was written for; it simply never considered -this one. - -Committing them also restores stock Spec-Kit behavior, whose premise is that every artifact is -version-controlled so there is a visible trail from original intent to shipped code. - -`tasks.md` and `checklists/` stay ignored: both are regenerated by their own command, and a -merged task list that no longer matches what was built is worse than no task list. - -**`data-model.md` commit-worthiness bar:** commit it only if a future dev would need it -to know the shapes without reading the code. Its structure follows the plan template's -Phase 1 spec — *entity name, fields, relationships, validation rules from requirements*. -If a feature's `data-model.md` just restates entities already obvious from `spec.md`, it's -as ephemeral as the rest — don't commit it (same for `contracts/`). - ## Customizations ### 1. Constitution — `.specify/memory/constitution.md` (AUTHORED) diff --git a/specs/37066-content-drive-status-filter/plan.md b/specs/37066-content-drive-status-filter/plan.md deleted file mode 100644 index b7c2e3867257..000000000000 --- a/specs/37066-content-drive-status-filter/plan.md +++ /dev/null @@ -1,284 +0,0 @@ -# Implementation Plan: Content Drive Status Filter - -**Branch**: `issue-37066-content-drive-status-filter-plan` (spec on `issue-37066-content-drive-status-filter`) | **Date**: 2026-08-24 | **Spec**: [spec.md](./spec.md) - -**Input**: Feature specification from `/specs/37066-content-drive-status-filter/spec.md` - -**Issue**: [dotCMS/core#37066](https://github.com/dotCMS/core/issues/37066) (absorbed #37067; epic #33999) - -## Summary - -Add a **Status** filter (Archived, Unpublished, Locked) to Content Drive: a new optional `status` -array on `POST /api/v1/drive/search`, resolved as AND-combined predicates in the database, plus a -multiselect chip in the Content Drive toolbar that drives it and round-trips through the URL. - -The technical approach is a flat, additive one. The three statuses are independent boolean facts -about the same `contentlet_version_info` row, so each becomes one independent `and` clause — no -composed OR group, no new join. The plumbing mirrors the workflow filter (`beaf846d51`) end to end. -The only delicate part is that **two existing code paths already have an opinion about -`cvi.deleted`** and both must learn about `ARCHIVED`; that is detailed below and is where the -regression risk lives. - -## Technical Context - -**Language/Version**: Java 25 (`dotcms.core.compiler.release`); TypeScript 5.x / Angular 22+ (core-web) - -**Primary Dependencies**: JAX-RS + Immutables (`@Value.Immutable`) for the request form; `BrowserAPI`/`BrowserQuery` for the query layer; PrimeNG (`p-popover`, `p-listbox`, `p-checkbox`) and NgRx Signal Store on the frontend - -**Storage**: PostgreSQL / MS SQL (`contentlet_version_info`) as the system of record; Elasticsearch/OpenSearch for the non-default `PURE_ES` path. **No schema change** — all three columns already exist (`postgres.sql:550-552`) - -**Testing**: JUnit integration (`dotcms-integration`, `-Dit.test=`), JUnit unit (`dotCMS/src/test`), Jest/Spectator (core-web) - -**Target Platform**: dotCMS server (Docker) + the core-web SPA - -**Project Type**: Full-stack — REST form + shared browse query layer + Angular portlet library - -**Performance Goals**: No regression against today's drive search. Each status adds one indexed-column predicate to an existing `where`; no new join, no subquery, no extra round-trip. A non-empty selection also drops the folder query entirely (`showFolders(false)`), so filtered requests do strictly less work - -**Constraints**: Rollback-safe (no schema, no mapping, no breaking contract change). With no `status` sent, every generated query must be **byte-identical** to today. The legacy Site Browser's inclusive "Show Archived" behavior must not change - -**Scale/Scope**: ~4 backend files + 1 new enum; ~6 frontend files + 1 new component; 1 new integration test class, 1 unit test, 3 spec files - -## Legacy Impact - -- **Touches legacy?** No `com.dotmarketing.*` source is modified. `com.dotcms.browser.BrowserAPIImpl` - is long-lived and heavily-parameterized but sits in the modern package. The legacy Site Browser JSP - (`view_browser.jsp`) and the legacy Content Search portlet (`ContentletAjax`) are **read as - precedent and left untouched**. -- **Modern vs legacy placement**: everything new lands in `com.dotcms.*` — the enum in - `com.dotcms.browser` (next to `FieldSearchCriteria`, which plays the same query-shaping role), the - form field in `com.dotcms.rest.api.v1.drive`. -- **Backward compatibility / migration**: none required. No DB schema change, no ES/OpenSearch - mapping change, no serialized-state change. The REST change is one **optional** field with an empty - default, so existing callers are unaffected. Not rollback-unsafe under any category in - `docs/core/ROLLBACK_UNSAFE_CATEGORIES.md`. - - The one real compatibility risk is behavioral, not structural: `BrowserQuery.showArchived` keeps - its **inclusive** meaning (archived *plus* everything else) because `view_browser.jsp:145` - depends on it. The new exclusive behavior is added alongside. An integration assertion guards - this (FR-008). -- **Progressive enhancements** (in-scope, small, only in code already being touched): - - Javadoc on every new method and on the new enum, matching the density of `appendWorkflowQuery` - and `withWorkflowSchemeIds` around it. - - The new frontend component is written to current standards from the start — `@if`, `input()`, - signals, `#`-private members, `ChangeDetectionStrategy.OnPush` — and strict-mode clean, per the - in-flight core-web strict migration. - - No wholesale rewrite of `BrowserAPIImpl`. It is a 2700-line file; this change adds one private - method and touches three existing lines. - -## Test Strategy (TDD — mandatory) - -Constitution Principle V: no implementation code before tests are written, developer-approved, and -confirmed **failing** for the right reason. - -| Component / behavior | Test type(s) | Where | Notes | -|---|---|---|---| -| Status value parsing; unknown value → 400 | Unit (JUnit) | `dotCMS/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveHelperStatusTest.java` | Mirrors `ContentDriveFieldFilterResolverTest`. No container needed | -| Each status alone; each pair; all three; empty default | Integration | `dotcms-integration/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveStatusFilterTest.java` | Follows `ContentDriveWorkflowFilterTest`: dedicated site + folder + a purpose-built content type + unique id, `@AfterClass` cleanup. Never asserts against shared default content types | -| `ARCHIVED + UNPUBLISHED` == `ARCHIVED`; `ARCHIVED + LOCKED` reachable | Integration | same class | FR-007 and the second Edge Case — asserted as documented behavior | -| `UNPUBLISHED` alone excludes an archived item; `ARCHIVED + UNPUBLISHED` admits it | Integration | same class | FR-004's baseline carve-out. Fixture needs an item that is **both** archived and unpublished, or the assertion passes vacuously. Raised by the automated spec review on #37170 | -| `LOCKED` composes with working-version scoping | Integration | same class | FR-005. Assert a locked item is returned under the drive's default (`live: false`), matching the legacy `+locked:true` / `+working:true` pairing (`ContentletAjax.java:1018`/`:1035`). Raised by the same review | -| Parity with and without free text (default hybrid heuristic) | Integration | same class | FR-009 / SC-005 | -| Parity under `BROWSE_API_HEURISTIC_TYPE=PURE_ES` | Integration | same class | Config override + restore in the test; the only coverage `buildPureESQuery` gets | -| Status + archive-target workflow step | Integration | same class | FR-011. The regression this change is most likely to cause | -| Legacy inclusive `showArchived` unchanged | Integration | same class | FR-008 — a direct `BrowserQuery` assertion, not through the drive form | -| `ContentDriveWorkflowArchiveStepTest` still green | Integration (existing) | unchanged file | Byte-identical SQL when no status is sent | -| Status multiselect: single, multiple, clearing, testids | Jest/Spectator | `…/dot-content-drive-status-filter/dot-content-drive-status-filter.component.spec.ts` | Driven through the rendered checkbox's `(onChange)`, never protected members. No `if` in test bodies. No CSS-class assertions | -| Request carries `status`; `showFolders` false; `archived` pin gone | Jest | `…/store/dot-content-drive.store.spec.ts` | Asserts the built `$request` payload | -| `status` URL decode round-trip | Jest | `…/utils/functions.spec.ts` | Alongside the existing `workflow` decode cases | - -**Registration**: `ContentDriveStatusFilterTest` joins `MainSuite3a`, where the other drive -integration tests live. - -- **Tests that cannot be implemented**: **none**. Every layer is reachable. Postman is deliberately - omitted rather than "not possible" — the endpoint-level behavior is covered more precisely by the - integration tests, which can seed the exact archived/locked fixtures a Postman collection cannot - construct against a shared environment. If the reviewer wants a Postman smoke case for the 400, - that is a cheap addition. - -## Constitution Check - -*GATE: evaluated before Phase 0, re-evaluated after Phase 1 design. Result: **PASS**, no violations.* - -| Principle | Verdict | Evidence | -|---|---|---| -| **I. Legacy-Aware Development** | PASS | No `com.dotmarketing.*` changes. New code in `com.dotcms.browser` / `com.dotcms.rest.api.v1.drive`. Legacy Site Browser behavior explicitly preserved and asserted. Progressive enhancements scoped to touched code only — see Legacy Impact | -| **II. Config & Logging Discipline** | PASS | No new `System.*` calls. Reads `BROWSE_API_HEURISTIC_TYPE` only through the existing `Config`-backed `HEURISTIC_TYPE` lazy. No new dependency, so no `bom/application/pom.xml` change | -| **III. Security by Default** | PASS | No secrets. The only user input is a closed enum, validated against it and rejected with a 400 — it never reaches SQL as text, so there is no injection surface. Permission filtering is untouched: the status clauses narrow a candidate set that `permissionAPI` still filters downstream, so the filter grants no new visibility | -| **IV. Contract Correctness** | PASS | One optional field with an empty default; no `@Schema` return type changes. `openapi.yaml` regenerated from the annotations and committed alongside the Java change. Not rollback-unsafe: no schema, no mapping, no breaking contract change | -| **V. Test-First / TDD** | PASS | Test Strategy above covers every layer with no exceptions claimed. `/speckit-tasks` will order each user story as tests → approval GATE → Red GATE → implementation, and `/speckit-implement` must halt at each gate | -| **ADR consultation** | PASS | `/speckit-adr-context` ran as the mandatory `before_plan` hook; results in ADR Alignment below | - -## ADR Alignment (Gate) - -**Step 1 — Consult existing ADRs**: run automatically as the `before_plan` hook. - -```bash -.specify/scripts/bash/adr-context.sh content-drive search elasticsearch browser query rest angular filter permissions legacy -``` - -### Relevant existing ADRs - -| ADR | Title | Status | How it constrains / informs this plan | -|---|---|---|---| -| [ADR-0018](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0018-database-first-content-drive-search-with-index-deferred-text-filtering.md) | Database-First Search for Content Drive, with Text Filtering Deferred to the Search Index | proposed | **Directly governing.** Its routing table lists *"Archived / deleted, show-on-menu → **DB** (version-info flags)"*. All three status predicates are version-info flags, so all three are resolved in SQL. It also states the index must be used *only* for free-text and searchable-field matching, and that structural criteria "must **never** be silently re-routed to the index for speed" — which this plan honors. It further notes `PURE_ES` "remains available behind configuration… but is **not** the default": that is precisely why `buildPureESQuery` is patched, so a supported configuration cannot silently drop the filter | -| [ADR-0009](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0009-opensearch-migration-plan.md) | Migrate OpenSearch from 1.x to 3.x Using Environment-Based Migration Strategy | accepted | Constrains only the `PURE_ES` clauses. The three terms used (`deleted`, `live`, `locked`) are core version-info fields mapped identically under both backends, so no migration-phase divergence is introduced. ADR-0018's own rationale — "shrink the blast radius of the ES→OS migration" by hanging fewer correctness guarantees off the index — is served by keeping the DB as the authority here | -| [ADR-0020](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0020-deprecate-folder-bypath-endpoint.md) | Deprecate `POST /api/v1/folder/byPath` in favor of `GET /api/v1/folder/search` | accepted | Surfaced by the keyword search but **not applicable** — this feature adds no folder endpoint and calls neither | - -### Conflicts with accepted ADRs - -**None.** The one ADR that governs this work (ADR-0018) is `proposed` rather than `accepted`, so it -is directional rather than binding — but this plan complies with it fully anyway, so the distinction -does not need resolving. Its central rule is that structural and metadata predicates belong in the -database, and all three status predicates are resolved there. - -### Proposed ADRs - -**None proposed.** This feature adds a filter *within* an already-decided routing contract; it makes -no new architectural decision. The AND-vs-OR choice is a domain fact about independent boolean flags -(and matches long-standing behavior in the legacy Content Search portlet), not an architectural -decision worth recording. - -## Project Structure - -### Documentation (this feature) - -```text -specs/37066-content-drive-status-filter/ -├── spec.md # /speckit-specify output -├── plan.md # This file -├── research.md # Phase 0 — R1..R10, all questions closed -├── data-model.md # Phase 1 — enum, transport and filter-bag shapes -├── quickstart.md # Phase 1 — how to build, test and see it work -├── contracts/ -│ └── drive-search-status.md # Phase 1 — the `status` field contract -├── checklists/requirements.md # gitignored; local spec-quality record -└── tasks.md # Phase 2 — /speckit-tasks, NOT created here -``` - -### Source Code (repository root) - -```text -# Backend — query layer -dotCMS/src/main/java/com/dotcms/browser/ -├── ContentStatus.java # NEW — ARCHIVED | UNPUBLISHED | LOCKED -├── BrowserQuery.java # + field, builder method, copy-ctor, toString; -│ # showWorking derivation extended -└── BrowserAPIImpl.java # + appendContentStatusQuery; 3 touched lines - # (:1980 archiveStepIds, :2006 exclusion, :612 ES) - -# Backend — REST -dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ -├── AbstractDriveRequestForm.java # + status() : List, default List.of() -├── ContentDriveHelper.java # + parseStatuses + builder wiring + showFolders(false) -└── ContentDriveResource.java # @Operation description only -dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml # regenerated, committed - -# Backend — tests -dotCMS/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveHelperStatusTest.java # NEW -dotcms-integration/src/test/java/com/dotcms/rest/api/v1/drive/ContentDriveStatusFilterTest.java # NEW -dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java # + registration - -# Frontend — portlet -core-web/libs/portlets/dot-content-drive/portlet/src/lib/ -├── shared/constants.ts # + STATUS_FILTER_KEY, CONTENT_STATUS, STATUS_FILTER_OPTIONS -├── shared/models.ts # + status: string[] on DotKnownContentDriveFilters -├── utils/functions.ts # + status: multiSelector in decodeByFilterKey -├── store/dot-content-drive.store.ts # - archived: false; + status; + showFolders term -└── components/dot-content-drive-toolbar/ - ├── dot-content-drive-toolbar.component.{ts,html} # render the new chip - └── components/dot-content-drive-status-filter/ # NEW component + template + spec - -# Frontend — shared model + i18n -core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts # + status?: string[] -dotCMS/src/main/webapp/WEB-INF/messages/Language.properties # + 4 keys (near :7113) -``` - -**Structure Decision**: Full-stack, following the exact shape the workflow filter established in -`beaf846d51` — request form → `BrowserQuery` → `BrowserAPIImpl` on the backend, and filter bag → -store `$request` → toolbar chip on the frontend. Nothing new is introduced structurally; this -feature is a second instance of an already-proven pattern, which is why it should land well under -the archive-step work's footprint (`f92f939296`: 184 impl lines). - ---- - -## Implementation approach - -Full detail and verification for each decision is in [research.md](./research.md); this is the -summary a reviewer needs. - -### Backend - -**1. `ContentStatus` enum** — `ARCHIVED`, `UNPUBLISHED`, `LOCKED`, in `com.dotcms.browser`. - -**2. `AbstractDriveRequestForm.status()`** — `List`, defaulting to `List.of()`. Deliberately -strings rather than the enum, so `ContentDriveHelper` owns the parse and throws an explicit -`BadRequestException` naming the accepted values — the deterministic 400 FR-010 asks for, matching -the `userSearchable` precedent already in that class (R7). - -**3. `BrowserQuery`** — plumbed exactly like `workflowSchemeIds`. One derived line changes: - -```java -this.showWorking = builder.showWorking || builder.showArchived; // :151 -``` - -must also be true when `ARCHIVED` or `UNPUBLISHED` is selected. Both states mean *no live version*, -so without this the query joins `c.inode = cvi.live_inode` and can never match — the filter would -silently return nothing. The drive path is safe today only by coincidence (`live()` defaults false); -the flag has to be right for any caller (R4). - -**4. `BrowserAPIImpl` — the three clauses**, in a new private `appendContentStatusQuery`: - -| Status | SQL | -|---|---| -| `ARCHIVED` | `and cvi.deleted = ` | -| `UNPUBLISHED` | `and cvi.live_inode is null` | -| `LOCKED` | `and cvi.locked_by is not null` | - -Independent `and` clauses — that *is* the AND semantics, no combinator needed. - -**5. `BrowserAPIImpl` — the two `cvi.deleted` interactions.** This is the risk surface: - -- **The global exclusion** (`:2006`) gains a third term: - `if (!showArchived && archiveStepIds.isEmpty() && !statuses.contains(ARCHIVED))`. - Without it, `cvi.deleted = false` and `cvi.deleted = true` are both emitted and `ARCHIVED` always - returns nothing. With it, `UNPUBLISHED`/`LOCKED` alone still keep the exclusion — which is exactly - FR-004, for free. -- **Archive-target workflow steps** (`:1980`). `archiveStepIds` is already emptied when - `showArchived`, because `appendWorkflowQuery` otherwise owns `cvi.deleted` per branch and would - force `false` on the live branch. `ARCHIVED` needs identical treatment: - ```java - final boolean admitsArchived = browserQuery.showArchived || statuses.contains(ARCHIVED); - ``` - This reuses the mechanism the archive-step work already built rather than inventing a second - reconciliation. With no status sent, every generated query stays byte-identical (R5). - -**6. `buildPureESQuery`** — the hardcoded `+deleted:false` (`:612`) becomes conditional, plus -`+live:false` / `+locked:true`. Only runs under `BROWSE_API_HEURISTIC_TYPE=PURE_ES`, which is a -supported configuration where the filter would otherwise silently no-op (R6). - -**7. `ContentDriveHelper`** — a block mirroring the workflow block directly above it: parse, set the -statuses, and `showFolders(false)` because folders carry no status. - -### Frontend - -Reuse over invention — every piece already exists (R9): - -- **Constants / models / decode**: three small additions (`STATUS_FILTER_OPTIONS`, `status: string[]` - on the filter bag and on `DotContentDriveSearchRequest`, `status: multiSelector` in - `decodeByFilterKey`). Encoding needs **no** change: `encodeFilters` already comma-joins arrays. -- **Not seeded in `withFilterDefaults`.** Unlike `languageId` and `sharedAssets`, where "absent" is - not a neutral state, an empty status set genuinely means no filtering. Leaving it unseeded makes - "Clear all" appear and clear correctly with no new code. -- **Store `$request`**: drop `archived: false` (the server default already supplies it) and send - `status` instead; add `!filters()?.status?.length` to the `showFolders` conjunction. -- **New component**: `dot-chip-filter` (`mode="dropdown"`) + `p-popover` + `p-listbox` with a - `p-checkbox` per row and `dot-filter-list-item` for labels, over a static three-option list. - Modeled on the workflow filter but **without** its service, caches, request-id guard and reconcile - pass — those exist because workflow options are fetched and can vanish between loads. Three fixed - options need none of it. `data-testid` on the chip, panel and each option; `[attr.aria-label]` on - the chip. -- **i18n**: four `content-drive.status-filter.*` keys in `Language.properties`. - -## Complexity Tracking - -No Constitution Check or ADR Alignment violations, so this section is intentionally empty. diff --git a/specs/37066-content-drive-status-filter/quickstart.md b/specs/37066-content-drive-status-filter/quickstart.md deleted file mode 100644 index 01213cc681ea..000000000000 --- a/specs/37066-content-drive-status-filter/quickstart.md +++ /dev/null @@ -1,187 +0,0 @@ -# Quickstart: Validating the Content Drive Status Filter - -**Feature**: [spec.md](./spec.md) | **Plan**: [plan.md](./plan.md) | **Date**: 2026-08-24 - -How to build, test and see this feature working. Shapes and semantics live in -[data-model.md](./data-model.md) and [contracts/drive-search-status.md](./contracts/drive-search-status.md); -this file is the run guide. - ---- - -## Prerequisites - -```bash -sdk env install # Java 25 via SDKMAN (.sdkmanrc) — build fails on the wrong version -nvm use # Node 22.22.3+ via nvm (.nvmrc) — frontend build fails on the wrong version -``` - -`nvm use` is also needed before `git commit`: the pre-commit hook runs under Node. - ---- - -## Build - -```bash -# Backend — core plus its in-project deps (~2-3 min) -./mvnw install -pl :dotcms-core --am -DskipTests -``` - -## Regenerate and verify the OpenAPI contract - -`openapi.yaml` is generated by `swagger-maven-plugin` at compile; CI fails if the committed file -does not match. No Docker needed. - -```bash -./mvnw compile -pl :dotcms-core -DskipTests -git diff --stat -- '*openapi.yaml' # expect the drive-search request schema to gain `status` -``` - -Commit the regenerated yaml **in the same commit** as the annotation change. - ---- - -## Tests - -### Red first - -Constitution Principle V: write the tests, get them developer-approved, and confirm they **fail for -the right reason** before any implementation. Each command below should be run once before -implementing (expect failure) and again after (expect green). - -### Unit — status parsing and the 400 - -```bash -./mvnw test -pl :dotcms-core -Dtest=ContentDriveHelperStatusTest -``` - -Expected: valid values map to `ContentStatus`; an unknown value raises `BadRequestException` naming -the accepted values. - -### Integration — the behavior that matters - -```bash -./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false \ - -Dit.test=ContentDriveStatusFilterTest -``` - -Needs a running PostgreSQL + Elasticsearch. For fast iteration: - -```bash -just test-integration-ide # start PostgreSQL + Elasticsearch + dotCMS -just test-integration-stop # stop when done -``` - -Single method while iterating: - -```bash -./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false \ - -Dit.test=ContentDriveStatusFilterTest#unpublishedAndLockedReturnsOnlyBoth -``` - -Coverage to expect in that class: - -| Case | Asserts | -|---|---| -| No status | Result set identical to today — archived hidden (FR-002) | -| Each status alone | Exactly the items in that state (FR-003/4/5) | -| Each pair, and all three | Exactly the items holding every selected state (FR-006) | -| `ARCHIVED + UNPUBLISHED` | Same set as `ARCHIVED` alone (FR-007) | -| `ARCHIVED + LOCKED` | Reachable — a self-locked archived item is returned | -| Status + free text | Same status semantics with a keyword present (FR-009) | -| `BROWSE_API_HEURISTIC_TYPE=PURE_ES` | Same results as the default heuristic (SC-005) | -| Status + archive-target workflow step | A coherent, non-empty result (FR-011) | -| Legacy inclusive `showArchived` | Still returns archived **plus** everything else (FR-008) | - -### Regression guard — do not skip - -```bash -./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false \ - -Dit.test=ContentDriveWorkflowArchiveStepTest -``` - -This is the test most likely to break: `appendWorkflowQuery` owns `cvi.deleted` per branch when an -archive-target step is selected. With no status sent, the generated SQL must be byte-identical to -before. - -> Never run the full integration suite to check this work — it takes 60+ minutes. - -### Frontend - -```bash -cd core-web -pnpm nx test portlets-content-drive --testPathPatterns=status-filter -pnpm nx test portlets-content-drive --testPathPatterns='dot-content-drive.store|functions' - -# Jest transpiles without typechecking — typecheck explicitly, or the dev server -# will quietly keep serving the last good bundle -npx tsc --noEmit -p libs/portlets/dot-content-drive/portlet/tsconfig.lib.json - -pnpm nx format:write -pnpm nx lint portlets-content-drive -``` - -`nx` is not global — always go through `pnpm`. - ---- - -## See it work end to end - -```bash -just dev-run # dotCMS in Docker -cd core-web && pnpm nx serve dotcms-ui # frontend dev server -``` - -Set up a fixture in one folder: - -1. Publish one item. -2. Create a second and leave it unpublished. -3. Archive a third. -4. Lock a fourth (open it and leave it checked out). -5. Leave one subfolder in place, to verify folders drop out. - -Then walk the acceptance scenarios: - -| Do | Expect | -|---|---| -| Open Content Drive on that folder, no filters | Live + unpublished + locked items **and** the subfolder. No archived item | -| Select **Archived** | Only the archived item. No folder | -| Select **Unpublished** | Only the unpublished item. Not the archived one | -| Select **Locked** | Only the locked item | -| Select **Unpublished + Locked** | Only an item that is both (create one to see a hit) | -| Select **Archived + Unpublished** | Same count as Archived alone — documented, not an error | -| Check the address bar | `…filters=…;status:UNPUBLISHED,LOCKED` | -| Reload the page | Same chips, same results | -| Open the URL in another browser | Same view for the recipient | -| Press **Clear all** | Status chip gone, folders back, archived hidden again | - -### Direct API check - -```bash -curl -sS -u admin@dotcms.com:admin \ - -H 'Content-Type: application/json' \ - -X POST http://localhost:8082/api/v1/drive/search \ - -d '{"assetPath":"//demo.dotcms.com/","status":["UNPUBLISHED","LOCKED"]}' | jq '.entity | {contentCount, folderCount}' -``` - -Expect `folderCount: 0` whenever a status is set. Then confirm the rejection path: - -```bash -curl -sS -o /dev/null -w '%{http_code}\n' -u admin@dotcms.com:admin \ - -H 'Content-Type: application/json' \ - -X POST http://localhost:8082/api/v1/drive/search \ - -d '{"assetPath":"//demo.dotcms.com/","status":["DRAFT"]}' -# → 400 -``` - -Verify query claims against this running instance rather than by reading the Java. - ---- - -## Definition of done - -- [ ] Every test above is green, and each was seen failing first -- [ ] `ContentDriveWorkflowArchiveStepTest` still passes -- [ ] `openapi.yaml` regenerated and committed with the annotation change -- [ ] Legacy Site Browser "Show Archived" checkbox verified unchanged -- [ ] `pnpm nx format:write` and `lint` clean; `tsc --noEmit` clean -- [ ] No status selected produces results identical to `main` for the same folder diff --git a/specs/37066-content-drive-status-filter/research.md b/specs/37066-content-drive-status-filter/research.md deleted file mode 100644 index 8e4622d4a0be..000000000000 --- a/specs/37066-content-drive-status-filter/research.md +++ /dev/null @@ -1,256 +0,0 @@ -# Phase 0 Research: Content Drive Status Filter - -**Feature**: [spec.md](./spec.md) | **Issue**: [#37066](https://github.com/dotCMS/core/issues/37066) | **Date**: 2026-08-24 - -Every finding below was verified against the code at `origin/main` (`e46da2b187`), not inferred -from the issue text. Line numbers are from that commit. - ---- - -## R1: What exists today for each of the three statuses - -**Decision**: Two of the three are net-new; the third exists in a form that answers a different -question and must be left alone. - -**Findings**: - -| Status | Present in `BrowserQuery`? | Detail | -|---|---|---| -| Archived | Partially, and **inclusive** | `showArchived` (`BrowserQuery.java:55`, builder `:470`) merely *skips* `appendExcludeArchivedQuery` (`BrowserAPIImpl.java:2006`). It returns archived content **plus everything else** — the opposite of what an "Archived" chip means. | -| Unpublished | No | `showWorking` and the form's `live` flag select *which version* to show. That is not the inverse of "has a live version". | -| Locked | No | No field, no builder method, no SQL clause, no ES clause. Nothing under `com.dotcms.rest.api.v1.drive` mentions `locked`. | - -Confirmed against the full field list (`BrowserQuery.java:44-83`) and all builder methods. - -**There is no raw query passthrough to lean on**: `BrowserQuery.luceneQuery` (`:66`) is written -only by `withFilter` / `withFileName` and is never read by `BrowserAPIImpl`. The clauses must be -built explicitly. - -**Rationale for keeping `showArchived` as-is**: the legacy Site Browser's "Show Archived" checkbox -(`view_browser.jsp:145`) depends on the inclusive meaning. Both questions are legitimate and both -must stay expressible, so the exclusive behavior is added *alongside* rather than replacing. - -**Alternatives considered**: redefining `showArchived` to be exclusive and adding an -`includeArchived` for the legacy path. Rejected — it inverts the meaning of a flag with callers -outside this feature's blast radius, for no gain. - ---- - -## R2: AND, not OR — and why that differs from every other Content Drive filter - -**Decision**: Selected statuses combine with **AND**. - -**Rationale**: the three are independent boolean facts about the *same* `contentlet_version_info` -row. An item can be unpublished *and* locked at once, so intersecting them is meaningful and is the -driving use case (US4). The existing multiselects — base type, content type, language — are OR -because each is a *single-valued* attribute: an item has exactly one base type, so an AND across -two would always be empty. - -**Precedent**: the legacy Content Search portlet has always combined these three additively -(`ContentletAjax.java:1010-1021`): - -```java -if (!showDeleted) "+deleted:false" else "+deleted:true" -if (filterLocked) "+locked:true" -if (filterUnpublish) "+live:false" -``` - -Its UI exposes them as a mutually exclusive dropdown (`view_contentlets.jsp:695-698`), but that was -a presentation simplification, never a domain constraint. Content Drive keeps the additive backend -and gives it a control that can actually express it. - -**Consequence**: each status is a flat, independent SQL clause. No composed OR group is needed, so -this lands well under the footprint of the archive-step work (`f92f939296`, 184 impl lines). - ---- - -## R3: The DB predicates - -**Decision**: - -| Status | SQL clause | ES term | -|---|---|---| -| Archived | `cvi.deleted = ` | `+deleted:true` | -| Unpublished | `cvi.live_inode is null` | `+live:false` | -| Locked | `cvi.locked_by is not null` | `+locked:true` | - -**Verification**: all three columns exist on `contentlet_version_info` (`postgres.sql:550-552`: -`live_inode varchar(36)`, `deleted bool not null`, `locked_by varchar(100)`). The `locked` field is -mapped into the index (`ESMappingAPIImpl.java:527`). `cvi` is already the alias in -`buildSelectBaseQuery` (`BrowserAPIImpl.java:2040`), and `DbConnectionFactory.getDBTrue()/getDBFalse()` -is the established way to write a boolean literal in this file. - -**Alternatives considered**: `live_inode <> working_inode` for Unpublished. Rejected — that is -"has unpublished changes", a third distinct question, and it is false for content that was never -published at all. - ---- - -## R4: The `showWorking` derivation must learn about the new statuses - -**Decision**: `BrowserQuery`'s constructor line - -```java -this.showWorking = builder.showWorking || builder.showArchived; // :151 -``` - -must also be true when `ARCHIVED` or `UNPUBLISHED` is selected. - -**Rationale**: `selectQuery` (`:1947`) picks the joined inode column from this flag: - -```java -final String workingLiveInode = browserQuery.showWorking || browserQuery.showArchived - ? "working_inode" : "live_inode"; -``` - -and the base query joins `c.inode = cvi.` (`:2043`). Archived and unpublished rows have -**no live version by definition**, so under `live_inode` the join can never match and the filter -would silently return nothing. The same flag drives `buildPureESQuery`'s `+working:true` vs -`+live:true` (`:615`), where `+live:true` alongside `+live:false` would be self-contradicting. - -The Content Drive path happens to be safe today (the form's `live()` defaults to `false`), but the -flag has to be correct for **any** caller of `BrowserQuery`, and relying on a coincidence in one -caller is exactly the kind of drift ADR-0018 was written to stop. - ---- - -## R5: Two existing branches already have an opinion about `cvi.deleted` - -This is the only genuinely delicate part of the change. Both branches must learn about `ARCHIVED`. - -### R5a — The global archived exclusion - -Today (`BrowserAPIImpl.java:2006`): - -```java -if (!browserQuery.showArchived && archiveStepIds.isEmpty()) { - appendExcludeArchivedQuery(selectQuery); // and cvi.deleted = false -} -``` - -**Decision**: add `&& !statuses.contains(ARCHIVED)`. - -Without it, `cvi.deleted = false` **and** `cvi.deleted = true` would both be emitted and `ARCHIVED` -would return nothing, always. With it, `UNPUBLISHED`/`LOCKED` selected alone still keep the -exclusion — which is precisely FR-004's "excludes archived content unless Archived is also -selected". The requirement and the code shape line up exactly; no extra logic is needed to get it. - -### R5b — Archive-target workflow steps - -`archiveStepIds` is deliberately emptied when `showArchived` (`:1980`), and the existing comment -says why: `appendWorkflowQuery` otherwise **owns** `cvi.deleted` per branch (`:2342-2360`), forcing -`cvi.deleted = false` on the live branch and hiding the archived content the caller asked for. - -**Decision**: `ARCHIVED` gets identical treatment: - -```java -final boolean admitsArchived = browserQuery.showArchived || statuses.contains(ARCHIVED); -final Set archiveStepIds = admitsArchived - ? Set.of() - : resolveArchiveTargetSteps(browserQuery.workflowStepIds); -``` - -This is the FR-011 / `ContentDriveWorkflowArchiveStepTest` regression case. It reuses the mechanism -the archive-step work already built rather than inventing a second reconciliation, and with no -status selected every generated query stays **byte-identical**. - -**Alternatives considered**: folding the status clauses into `appendWorkflowQuery`'s branch -structure. Rejected — status and workflow are orthogonal filters, and coupling them would make each -one harder to reason about for no behavioral gain. - ---- - -## R6: Which query path actually runs, and why both must be patched - -**Decision**: implement in the SQL path (`selectQuery`) **and** in `buildPureESQuery`. - -**Findings**: `doElasticSearchTextFiltering` (`:479`) switches on `BROWSE_API_HEURISTIC_TYPE`, -defaulting to `HYBRID_SINGLE_CHUNKED_QUERY_ES` (`:697`). - -- Under the **default hybrid** heuristic the SQL query supplies the ordered candidate inode set and - the index only narrows each chunk by the text term. So the SQL clauses apply **with or without** - text in the search box — this is what satisfies FR-009's "identical with and without a keyword". -- `buildPureESQuery` — with its hardcoded `+deleted:false` at `:612` — runs **only** under - `BROWSE_API_HEURISTIC_TYPE=PURE_ES`. - -`PURE_ES` is not the default and ADR-0018 says it must not become one, but it is a **supported -configuration**. Leaving it unpatched would make all three filters silently no-op there, returning a -wider set than the user asked for. That is FR-009 and SC-005. - ---- - -## R7: Rejecting an invalid status value - -**Decision**: the form declares `List status()`; `ContentDriveHelper` parses it into the -enum and throws `BadRequestException` naming the accepted values. - -**Rationale**: FR-010 asks for a 400 "consistent with how the drive already rejects unknown -field-filter keys", and that precedent is an explicit `BadRequestException` thrown in -`ContentDriveHelper.driveSearch` (the `userSearchable` guard, ~line 180). Keeping the same shape -means one error path, one message style, and a status code we control directly. - -**Alternatives considered**: declaring the field as `List` and letting Jackson -reject. Rejected — deserialization failures surface as `InvalidFormatException` from the immutables -layer, whose mapping to a 400 with a useful message is less direct than throwing it ourselves. The -typed form field is marginally prettier; the deterministic error is worth more. - ---- - -## R8: Where the new enum lives - -**Decision**: `com.dotcms.browser.ContentStatus` — `ARCHIVED`, `UNPUBLISHED`, `LOCKED`. - -**Rationale**: `BrowserQuery` is the real consumer, and `com.dotcms.browser` already holds this -kind of query-shaping type (`FieldSearchCriteria`, with its own `RoutingBucket` enum). Placing it in -`com.dotcms.rest.api.v1.drive` would make the browser layer depend on the REST layer. - -Constitution Principle I is satisfied: entirely modern `com.dotcms.*`, nothing added to -`com.dotmarketing.*`. - ---- - -## R9: Frontend — reuse, don't rebuild - -**Decision**: model the control on the workflow filter, but single-column and static. - -**Findings** — everything needed already exists: - -| Need | Existing thing to reuse | -|---|---| -| Chip + active state + overflow label | `DotChipFilterComponent` (`@dotcms/ui`), `mode="dropdown"` | -| Popover + listbox styling | `CHIP_FILTER_POPOVER_PT`, `CHIP_FILTER_LISTBOX_PT`, `PANEL_SCROLL_HEIGHT` | -| Row label with truncation | `DotFilterListItemComponent` | -| Read / write / clear a filter | store `getFilterValue` / `patchFilters` / `removeFilter` | -| URL encode | `encodeFilters` — already comma-joins any array value, no change needed | -| URL decode | one line in `decodeByFilterKey`: `status: multiSelector` | -| "Clear all" visibility + clearing | `hasNonDefaultFilters` / `clearFilters` — both work unchanged | - -The workflow filter carries a service, two caches, a request-id guard and a reconcile pass, because -its options are fetched and can disappear between loads. The status options are a fixed set of -three. **None of that machinery should be copied** — the component is a `linkedSignal` over the -filter value plus a checkbox list. - -**Decision on defaults**: `status` is deliberately **not** added to `withFilterDefaults`. Unlike -`languageId` and `sharedAssets` — where "absent" is not a neutral state and must be seeded — an -empty status set genuinely means "no status filtering". Leaving it unseeded makes -`hasNonDefaultFilters` return `true` whenever a status is present (so "Clear all" appears) and makes -`clearFilters()` drop it, both for free. - ---- - -## R10: Removing the hardcoded `archived: false` - -**Decision**: delete the `archived: false` pin from `$request` in `dot-content-drive.store.ts:135` -and send `status` instead. - -**Verification**: safe to simply remove — `AbstractDriveRequestForm.archived()` is -`@Value.Default default boolean archived() { return false; }`, so omitting it from the payload -produces the identical query. This is FR-019, and it is what stops the request from contradicting -the user's Archived selection. - ---- - -## Open questions - -**None.** No `NEEDS CLARIFICATION` markers were carried out of the spec, and every technical -uncertainty above was closed by reading the code rather than by assumption. From a74385f2aa90fa665b606dba0a165cf53a30abf6 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 11:37:34 -0300 Subject: [PATCH 09/16] docs(content-drive): make data-model.md self-contained (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review note on #37170: the design rationale cited research.md items (R2, R4, R7), but research.md is gitignored by the commit policy this PR reaffirms, so a reviewer could not verify the load-bearing claims. The R-numbers read as citations to something unreviewable. Fixed both ways the review suggested: the reasoning is now inlined in full and the R-references are gone, so nothing in the committed docs points outside the diff. The showWorking passage gained the most: it now names the two call sites that make the flag load-bearing (BrowserAPIImpl:1947 picking the joined inode column, :615 choosing +working:true vs +live:true), says why the join can never match for archived/unpublished rows, and notes that the drive path is safe today only by coincidence of live() defaulting to false — plus that LOCKED alone does not need the flag, since a locked item may have a live version. Co-Authored-By: Claude Opus 5 (1M context) --- .../data-model.md | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/specs/37066-content-drive-status-filter/data-model.md b/specs/37066-content-drive-status-filter/data-model.md index d155b18b73b7..d39190b549d8 100644 --- a/specs/37066-content-drive-status-filter/data-model.md +++ b/specs/37066-content-drive-status-filter/data-model.md @@ -2,10 +2,9 @@ **Feature**: [spec.md](./spec.md) | **Date**: 2026-08-24 -> `plan.md`, `research.md` and `quickstart.md` are Spec-Kit process artifacts and are gitignored by -> policy (`.specify/CUSTOMIZATIONS.md`), so the `research.md` references below point at files that -> exist on the author's machine, not in this repo. Each one is summarized inline so this document -> stands on its own. +This document is self-contained: every decision below carries its own reasoning rather than citing +the Spec-Kit process artifacts, which are gitignored by policy (`.specify/CUSTOMIZATIONS.md`) and so +are not reviewable from this diff. No database schema changes. Every column this feature reads already exists on `contentlet_version_info` and is already indexed into the search index. This document describes the @@ -36,7 +35,7 @@ an item is in *any* selected state, not all of them — selected statuses combin the Content Type and Language filters. AND was considered and rejected: under AND, `{ARCHIVED, UNPUBLISHED}` is redundant, `{ARCHIVED, LOCKED}` is almost always empty and all three is empty in practice, so only one of four combinations says anything — and the chip would be the sole -exception in a toolbar row where every other filter widens on selection (research.md R2). +exception in a toolbar row where every other filter widens on selection. One overlap is worth knowing even though it no longer produces a degenerate result: every archived item is also unpublished, because archiving removes the live version @@ -64,10 +63,14 @@ default List status() { return List.of(); } | Duplicates | Collapsed; the parsed result is a `Set` | | Unknown value | `400`, message naming the accepted values (FR-010) | -**Declared as `List`, not `List`** (research.md R7): a typed field would -route an invalid value through Jackson's `InvalidFormatException`, whose mapping to a useful 400 is -less direct than throwing one ourselves. The helper owns the parse instead, matching the -`userSearchable` precedent already in `ContentDriveHelper`. +**Declared as `List`, not `List`.** The requirement (FR-010) is a 400 on an +invalid value, "consistent with how `userSearchable` rejects unknown keys" — and that precedent is an +explicit `BadRequestException` thrown inside `ContentDriveHelper.driveSearch`. Declaring the field as +the enum would instead let the Immutables/Jackson layer reject it during deserialization, as an +`InvalidFormatException` whose mapping to a 400 with a message naming the accepted values is not +under this code's control. So the helper owns the parse: one error path, one message style, one +status code, matching the filter that already does this. The typed field would read better; the +deterministic error is worth more. ### Validation rules @@ -141,9 +144,19 @@ in `toString()`. this.showWorking = builder.showWorking || builder.showArchived; ``` -must also be true when the selection contains `ARCHIVED` or `UNPUBLISHED`. Both states imply no live -version, so without this `selectQuery` joins on `live_inode` — which those rows never have — and the -filter silently returns nothing (research.md R4). +must also be true when the selection contains `ARCHIVED` or `UNPUBLISHED`. + +`selectQuery` picks the joined inode column from this flag +(`BrowserAPIImpl.java:1947`: `showWorking || showArchived ? "working_inode" : "live_inode"`), and the +base query joins `c.inode = cvi.` (`:2043`). Archived and unpublished rows have **no +live version by definition**, so under `live_inode` the join can never match and the filter returns +nothing — silently, with no error. The same flag also drives `buildPureESQuery`'s `+working:true` vs +`+live:true` (`:615`), where emitting `+live:true` alongside a `live:false` disjunct would be +self-contradicting. + +The Content Drive path happens to be safe today because the form's `live()` defaults to `false`, but +that is a coincidence in one caller, not a property of `BrowserQuery`. `LOCKED` alone does not need +this: a locked item may well have a live version. --- From 3f0e315bac3d81946e68b5690c524773f270e98c Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 11:39:16 -0300 Subject: [PATCH 10/16] Revert the .specify/feature.json gitignore addition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comply with the existing policy rather than amending it. This PR now touches no policy files at all — .gitignore and .specify/CUSTOMIZATIONS.md are identical to main. feature.json stays out of the commit (it was never tracked on main, and 36605 and 36834 both shipped without it). It is simply untracked rather than ignored, which is exactly main's state today. If the team wants it ignored, that belongs in its own PR against the policy, not carried along inside a feature branch. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 5 ----- .specify/CUSTOMIZATIONS.md | 8 -------- 2 files changed, 13 deletions(-) diff --git a/.gitignore b/.gitignore index 41e94f07dd73..9a1c952d10da 100644 --- a/.gitignore +++ b/.gitignore @@ -231,8 +231,3 @@ specs/*/research.md specs/*/tasks.md specs/*/quickstart.md specs/*/checklists/ - -# Per-developer pointer to the feature you are currently working on, rewritten by -# /speckit-specify. Committing it would aim everyone else's /speckit-plan at your -# feature, and every concurrent feature branch would collide on its single line. -.specify/feature.json diff --git a/.specify/CUSTOMIZATIONS.md b/.specify/CUSTOMIZATIONS.md index 8a1dbca027b3..c6ac8886de44 100644 --- a/.specify/CUSTOMIZATIONS.md +++ b/.specify/CUSTOMIZATIONS.md @@ -52,14 +52,6 @@ Phase 1 spec — *entity name, fields, relationships, validation rules from requ If a feature's `data-model.md` just restates entities already obvious from `spec.md`, it's as ephemeral as the rest — don't commit it (same for `contracts/`). -**`.specify/feature.json` is never committed either.** It is not a spec artifact — it is a -per-developer pointer to the feature *you* are currently on, rewritten by `/speckit-specify` and -read by `get_feature_paths()` so the downstream commands can find the spec folder without relying -on branch naming. Committing it would aim every teammate's next `/speckit-plan` at whatever feature -last merged, and two concurrent feature branches would conflict on its single line every time. It is -gitignored for that reason. To point the commands at a feature explicitly, set -`SPECIFY_FEATURE_DIRECTORY` instead — it takes precedence over the file. - ## Customizations ### 1. Constitution — `.specify/memory/constitution.md` (AUTHORED) From dfb782570e016099a885b9cb8f4234295c88143f Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 16:59:13 -0300 Subject: [PATCH 11/16] docs(content-drive): place the Status chip after Content Type (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Was between shared-assets and content-type, on a broadest-scope-first reading. The row is not ordered by scope breadth — it is ordered by dependency. Content type is the pivot: the workflow filter loads its schemes via getSchemesByContentTypes, and the field-filter menu only appears when exactly one content type is selected. So the row reads precondition -> pivot -> things derived from the pivot -> version. Under that reading the old position was wrong twice: it split the shared-assets gate from the pivot it gates, and it separated status from workflow, which asks the same kind of question (where is this in its lifecycle, and is anyone holding it). Status now leads that pair — it is the coarser question, and unlike workflow its three options never depend on the content-type selection. Also the cheapest option for existing users: content type, almost certainly the most-used filter, keeps its position. The shared-assets toggle is deliberately left alone. Its stated rationale relies on the same scope-breadth reading, and there is a real frequency argument for demoting a toggle almost nobody flips out of the first slot — but that is a filter this ticket does not own, and it belongs in its own ticket. Co-Authored-By: Claude Opus 5 (1M context) --- specs/37066-content-drive-status-filter/spec.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/specs/37066-content-drive-status-filter/spec.md b/specs/37066-content-drive-status-filter/spec.md index f0b1ff74c189..dc85852f1a04 100644 --- a/specs/37066-content-drive-status-filter/spec.md +++ b/specs/37066-content-drive-status-filter/spec.md @@ -219,8 +219,11 @@ selection and the results are the same at every step. - **FR-011**: A status selection MUST NOT conflict with a workflow filter that also constrains archived content; the two MUST combine into one coherent result set. - **FR-012**: Users MUST be able to select any combination of the three statuses from a single - control in the Content Drive toolbar, positioned between the shared-assets and content-type - filters so the row reads from broadest scope to narrowest. + control in the Content Drive toolbar, positioned **after the content-type filter and immediately + before the workflow filter**. Status and workflow ask the same kind of question — where content + sits in its lifecycle, and whether anyone is holding it — so they read as a pair; and content type + keeps its position as the selection the workflow filter's options and the field-filter menu are + both derived from. - **FR-013**: The control's labels MUST be localizable, following the Content Drive naming convention already used by the other filters. - **FR-014**: The active selection MUST be reflected as a chip, consistent with the other toolbar From 17bf995b8ce7583cf78de080c6d5a7e290b89725 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Mon, 24 Aug 2026 17:02:06 -0300 Subject: [PATCH 12/16] docs(content-drive): keep Content Type and Workflow adjacent (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Corrects the previous commit. I argued the row is dependency-ordered, then put Status between Content Type and Workflow — breaking the only real dependency edge in the row. Verified edges: content-type -> workflow REAL (getSchemesByContentTypes) content-type -> field-filter menu REAL (only at exactly one content type) shared-assets -> content-type NOT REAL (CT reads only baseType/contentType) status -> anything none, its three options are fixed The gate->pivot relationship I used to justify keeping Status away from position 2 does not exist in the code. Content type and workflow are the pairing that is actually coupled, so they stay adjacent. Status depends on nothing, so it goes where it costs least: after workflow. The two lifecycle questions still read as a pair, content type keeps its position, and only locale shifts. What this gives up is "Status leads the pair because it is the coarser question" — an aesthetic preference that should not outrank a real dependency. Co-Authored-By: Claude Opus 5 (1M context) --- specs/37066-content-drive-status-filter/spec.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/specs/37066-content-drive-status-filter/spec.md b/specs/37066-content-drive-status-filter/spec.md index dc85852f1a04..75b9950ae3c1 100644 --- a/specs/37066-content-drive-status-filter/spec.md +++ b/specs/37066-content-drive-status-filter/spec.md @@ -219,11 +219,12 @@ selection and the results are the same at every step. - **FR-011**: A status selection MUST NOT conflict with a workflow filter that also constrains archived content; the two MUST combine into one coherent result set. - **FR-012**: Users MUST be able to select any combination of the three statuses from a single - control in the Content Drive toolbar, positioned **after the content-type filter and immediately - before the workflow filter**. Status and workflow ask the same kind of question — where content - sits in its lifecycle, and whether anyone is holding it — so they read as a pair; and content type - keeps its position as the selection the workflow filter's options and the field-filter menu are - both derived from. + control in the Content Drive toolbar, positioned **after the workflow filter and before the locale + filter**. Content type and workflow MUST stay adjacent: the workflow filter's options are derived + from the content-type selection, and that is the only such dependency in the row. Status depends on + nothing, so it sits beside workflow — the two ask the same kind of question, where content sits in + its lifecycle and whether anyone is holding it — without coming between workflow and the selection + it reads from. - **FR-013**: The control's labels MUST be localizable, following the Content Drive naming convention already used by the other filters. - **FR-014**: The active selection MUST be reflected as a chip, consistent with the other toolbar From c07bea474e27dcfd0ba12332c66add60917dc213 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Tue, 25 Aug 2026 09:53:07 -0300 Subject: [PATCH 13/16] docs(content-drive): fix Lucene AND bug and pin the empty-status guarantee (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings from @nollymar on #37170, plus @dario-daza's question. 1. The index-term table listed `+deleted:true`, `+live:false`, `+locked:true`. In Lucene `+` means REQUIRED, so concatenating those is an AND — the exact opposite of the OR semantics this spec just settled on, and it would have shipped silently. Terms are now bare, with the composed forms spelled out: `+deleted:false +(live:false OR locked:true)`. This is already the convention in the method being changed — BrowserAPIImpl:630 writes `+(conhost: OR conhost:SYSTEM_HOST)`. The legacy ContentletAjax reference keeps its `+` prefixes, since legacy genuinely does AND its flags, with a note not to copy that form. 2. An empty status must be skipped entirely, not translated into a vacuous clause: `and ( )` is a SQL syntax error and `+()` is invalid Lucene. FR-002 said "preserves today's behavior" without saying the field is ignored. It is also the default path, not an edge case — every search that exists today sends no status. 3. OR applies within the status filter only; status still ANDs with every other filter, as they already do with each other (BrowserAPIImpl:2338 has the workflow filter appending its own `and ( … )`). New FR-006a states the rule, which the spec got away with omitting while every filter was OR. 4. Success Criteria now names where the test types live and why Postman is deliberately omitted, so a reviewer who cannot see the gitignored plan does not have to ask. Co-Authored-By: Claude Opus 5 (1M context) --- .../contracts/drive-search-status.md | 30 ++++++++++++++ .../data-model.md | 40 +++++++++++++++++-- .../37066-content-drive-status-filter/spec.md | 28 ++++++++++--- 3 files changed, 89 insertions(+), 9 deletions(-) diff --git a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md index a6e8696aba94..4febd507c7f3 100644 --- a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md +++ b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md @@ -39,6 +39,11 @@ Resource: `dotCMS/src/main/java/com/dotcms/rest/api/v1/drive/ContentDriveResourc **OR, not AND.** Selecting more statuses returns *more* content, exactly like `contentTypes`, `baseTypes` and `language`. Adding a status can never shrink the result set. +**OR applies *within* `status` only. Separate filters still AND with each other**, exactly as they +do today: `{"workflow": [...], "status": ["UNPUBLISHED","LOCKED"]}` means *governed by that workflow* +**and** *(unpublished **or** locked)*. Each filter appends its own `and ( … )` clause server-side +(`BrowserAPIImpl:2338` for workflow), so adding `status` never loosens another filter. + **The archived exclusion is a baseline, not a fourth value, and it sits outside the OR group.** Every drive request already excludes archived content; the selected statuses are OR-ed together and that group is AND-ed against the baseline, which only `ARCHIVED` lifts. That is why @@ -94,6 +99,31 @@ environment runs (FR-009 / SC-005). Both paths are in scope: | `HYBRID_SINGLE_CHUNKED_QUERY_ES` (**default**) | `BrowserAPIImpl.selectQuery` supplies the candidate set; the index only narrows by text | SQL clauses — so they apply with **and** without a keyword | | `PURE_ES` | `BrowserAPIImpl.buildPureESQuery` | Index terms, replacing the hardcoded `+deleted:false` | +### An empty `status` MUST be ignored entirely + +An omitted or empty `status` changes **nothing** on either path — the field is skipped, not +translated into a vacuous clause. Both builders MUST return early on an empty set rather than +opening a group they have nothing to fill: `and ( )` is a SQL syntax error and `+()` is invalid +Lucene. This is what makes FR-002's "byte-identical to today" literally true, and it is the case to +assert first, because every existing caller of drive search sends no `status`. + +### Multiple statuses MUST be one explicit OR group + +In Lucene, `+` means REQUIRED. Emitting `+deleted:true +live:false` is an **AND** — the opposite of +this contract. Multiple statuses go in one explicit group, with the archived baseline left outside +it as its own required clause: + +| Selection | Index query | +|---|---| +| `[]` | *(no status terms at all)* | +| `["UNPUBLISHED"]` | `+deleted:false +(live:false)` | +| `["UNPUBLISHED","LOCKED"]` | `+deleted:false +(live:false OR locked:true)` | +| `["ARCHIVED"]` | `+(deleted:true)` | +| `["ARCHIVED","LOCKED"]` | `+(deleted:true OR locked:true)` | + +This is already the convention in the method being changed — `BrowserAPIImpl:630` writes +`+(conhost: OR conhost:SYSTEM_HOST)`. + Aligned with [ADR-0018](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0018-database-first-content-drive-search-with-index-deferred-text-filtering.md), which routes version-info flags (archived/deleted) to the **database**. `PURE_ES` is patched not to promote it, but because it is a supported configuration where the filter would otherwise silently diff --git a/specs/37066-content-drive-status-filter/data-model.md b/specs/37066-content-drive-status-filter/data-model.md index d39190b549d8..1d6456e51d3b 100644 --- a/specs/37066-content-drive-status-filter/data-model.md +++ b/specs/37066-content-drive-status-filter/data-model.md @@ -20,9 +20,16 @@ A closed enum of three independent states a contentlet version can hold. | Constant | Meaning | Backing column (`contentlet_version_info`) | Index term | |---|---|---|---| -| `ARCHIVED` | Removed from circulation, recoverable | `deleted = true` | `+deleted:true` | -| `UNPUBLISHED` | No live version exists | `live_inode is null` | `+live:false` | -| `LOCKED` | A lock is held, by anyone | `locked_by is not null` | `+locked:true` | +| `ARCHIVED` | Removed from circulation, recoverable | `deleted = true` | `deleted:true` | +| `UNPUBLISHED` | No live version exists | `live_inode is null` | `live:false` | +| `LOCKED` | A lock is held, by anyone | `locked_by is not null` | `locked:true` | + +> **The index terms above are bare on purpose — do not prefix them with `+`.** In Lucene syntax `+` +> means REQUIRED, so emitting `+deleted:true +live:false` is an **AND**, which is the exact opposite +> of this feature's semantics. Multiple statuses MUST be wrapped in one explicit group: +> `+(deleted:true OR live:false)`. This is already the convention in the method being changed — +> `BrowserAPIImpl:630` writes `+(conhost: OR conhost:SYSTEM_HOST)`. See the composed forms under +> [Query shape](#query-shape-browserquerycontentstatuses). **Placement**: `com.dotcms.browser` rather than the REST package — `BrowserQuery` is the consumer, and the browser layer must not depend on the REST layer. Sits alongside `FieldSearchCriteria`, which @@ -99,6 +106,29 @@ it. | `[ARCHIVED, UNPUBLISHED]` | *lifted* | `(deleted = true or live_inode is null)` | everything with no live version | | all three | *lifted* | `(deleted = true or live_inode is null or locked_by is not null)` | anything not cleanly published | +### Composed query forms + +The SQL group and the index group are the same shape: the selected statuses OR-ed inside one group, +AND-ed against the archived baseline that sits outside it. + +| Selection | SQL | Index | +|---|---|---| +| `[]` | *(no status clause at all)* | *(no status clause at all)* | +| `[UNPUBLISHED]` | `and cvi.deleted = false and ( cvi.live_inode is null )` | `+deleted:false +(live:false)` | +| `[UNPUBLISHED, LOCKED]` | `and cvi.deleted = false and ( cvi.live_inode is null or cvi.locked_by is not null )` | `+deleted:false +(live:false OR locked:true)` | +| `[ARCHIVED]` | `and ( cvi.deleted = true )` | `+(deleted:true)` | +| `[ARCHIVED, LOCKED]` | `and ( cvi.deleted = true or cvi.locked_by is not null )` | `+(deleted:true OR locked:true)` | + +**An empty selection must emit nothing at all** — not an empty group. `and ( )` is a SQL syntax +error and `+()` is invalid Lucene, so both builders MUST return early on an empty set rather than +opening a group they then have nothing to fill. This is what makes FR-002's "byte-identical to +today" literally true. + +**Filters AND with each other; only values within one filter OR.** A status selection combined with +the workflow filter means *governed by that workflow* **and** *in any of the selected states* — each +filter appends its own `and ( … )` clause (`BrowserAPIImpl:2338` for workflow), exactly as content +type and locale already compose today. + **The bug to avoid** is folding the baseline into the group. `[UNPUBLISHED, LOCKED]` would then read `(deleted = false or live_inode is null or locked_by is not null)`, which matches essentially every row in the folder — a filter that silently stops filtering. @@ -114,7 +144,9 @@ planning, see the OR rationale above.)* `LOCKED` does not constrain which version is joined, so it stacks on whatever `showWorking` already selected. That is the same pairing the legacy portlet uses: `ContentletAjax.java:1018` appends -`+locked:true` and `:1035` unconditionally appends `+working:true`. +`+locked:true` and `:1035` unconditionally appends `+working:true`. (Those legacy terms carry `+` +because legacy genuinely does AND its status flags — do **not** copy that form here; see the note +under the enum table.) One deliberate difference: legacy **always** scopes to the working version, whereas here the drive scopes to working because `AbstractDriveRequestForm.live()` defaults to `false`. A caller that sets diff --git a/specs/37066-content-drive-status-filter/spec.md b/specs/37066-content-drive-status-filter/spec.md index 75b9950ae3c1..bc1c44829253 100644 --- a/specs/37066-content-drive-status-filter/spec.md +++ b/specs/37066-content-drive-status-filter/spec.md @@ -180,10 +180,10 @@ selection and the results are the same at every step. - **An unrecognized status value** submitted directly to the search endpoint is rejected with a clear client error naming the accepted values, rather than being silently ignored (which would return a wider result set than the caller asked for). -- **Status combined with a workflow filter.** Content Drive can already filter by workflow, including - steps that archive content. A status selection combined with such a workflow filter must return a - coherent result, not an empty one caused by two rules contradicting each other about archived - content. +- **Status combined with a workflow filter.** The two combine with AND: *governed by that workflow* + **and** *in any selected state*. Content Drive can already filter by workflow, including steps that + archive content, so the pairing must return a coherent result rather than an empty one caused by + two rules contradicting each other about archived content. - **Text search plus status.** The drive uses different search strategies depending on whether a keyword is present and how the environment is configured. Every strategy must apply the status filter identically, so the same selection never returns different results because of a @@ -198,7 +198,9 @@ selection and the results are the same at every step. - **FR-001**: The drive search capability MUST accept a set of content statuses drawn from Archived, Unpublished and Locked, defaulting to an empty set. - **FR-002**: An empty set MUST preserve today's behavior exactly: archived content excluded, all - other content returned. + other content returned. The status filter MUST be **skipped entirely** in that case, not + translated into a vacuous "matches anything" condition — every search that exists today sends no + status, so this is the default path, not an edge case. - **FR-003**: Archived alone MUST return only archived content, never archived content in addition to everything else. - **FR-004**: Unpublished alone MUST return only content with no live version, and MUST exclude @@ -207,6 +209,10 @@ selection and the results are the same at every step. it, and MUST exclude archived content. - **FR-006**: Multiple selected statuses MUST combine with **OR** — the result is content holding *any* of the selected states. Adding a status MUST never reduce the result set. +- **FR-006a**: OR applies **within** the status filter only. Status MUST still combine with every + other filter by **AND**, as the existing filters already do with each other. Selecting a workflow + and two statuses means *governed by that workflow* **and** *in either of those states* — adding a + status MUST never loosen another filter. - **FR-007**: Archived MUST be the only status that admits archived content into the results. Selecting it alongside others widens the results to include archived content as well as content in the other selected states. @@ -251,6 +257,18 @@ selection and the results are the same at every step. ## Success Criteria *(mandatory)* +> **How these are verified.** This specification stays technology-agnostic by convention, so the +> concrete test types live in the plan: **unit** for status parsing and the 400, **integration** for +> the semantics matrix (each status, each pair, all three, the empty default, the never-shrinks +> property, `PURE_ES` parity, and the archive-step regression), and **Jest/Spectator** for the chip, +> the request payload and the URL round-trip. Postman is deliberately not used here — the behavior +> needs seeded archived/locked fixtures that a collection cannot construct against a shared +> environment, and the integration tests cover the same endpoint more precisely. +> +> Per Constitution Principle V these are non-negotiable and land **before** implementation: +> `/speckit-tasks` orders every user story as tests → developer-approval gate → confirmed-failing +> (Red) gate → implementation. + ### Measurable Outcomes - **SC-001**: An editor can locate archived content from within Content Drive in a single action, From 2f535242c1862fcce8b219c88d4af2490a4c4edb Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Tue, 25 Aug 2026 15:55:12 -0300 Subject: [PATCH 14/16] docs(content-drive): folder exclusion is a client rule, not an endpoint side effect (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FR-015 said folders "MUST be excluded from the results whenever any status is selected", which read as a requirement on the search capability. Implemented that way it becomes a silent side effect: the endpoint overrides an explicitly requested showFolders, so the response stops matching the request and the folder pagination describes a query the caller never received. Folders carry no status, so the Content Drive UI should stop asking for them — and it already does, in the store's $request. But that is the client's decision. The endpoint does what it is told. - spec.md FR-015: reworded as a client-side rule, with an explicit prohibition on the search capability overriding a requested folder setting. - contracts/: the side-effects table now records showFolders as unaffected, and states that `status` has no side effects on other fields. - data-model.md: validation rule points at the store rather than the helper, plus a section explaining why the endpoint stays obedient. Also noted: the pre-existing workflow filter still forces showFolders(false) server-side, so the two filters now differ. Left as-is — outside this feature — but flagged for a separate decision. Spec changed after sign-off, so #37170 needs re-approval per CLAUDE.md. Co-Authored-By: Claude Opus 5 (1M context) --- .../contracts/drive-search-status.md | 8 +++++++- .../data-model.md | 16 +++++++++++++++- specs/37066-content-drive-status-filter/spec.md | 7 ++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md index 4febd507c7f3..534254aa8f5a 100644 --- a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md +++ b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md @@ -55,9 +55,15 @@ See [../data-model.md](../data-model.md) for the per-selection predicate table. | Field | Effect when `status` is non-empty | |---|---| -| `showFolders` | Forced to `false`. Folders carry no status (FR-015), consistent with the workflow filter | +| `showFolders` | **Unaffected.** The endpoint honours whatever the caller sent | | `archived` | Unaffected and unchanged. The legacy inclusive flag keeps its meaning (FR-008) | +**`status` has no side effects on other fields.** Folders carry no status, so the Content Drive UI +sends `showFolders: false` once a status is selected (FR-015) — but that is the client's decision. +Overriding an explicit `showFolders: true` server-side would make the response stop matching the +request, and would leave `folderCursor` / `hasMoreFolders` describing a folder query the caller never +received. A caller that wants folders alongside a status gets them. + The Content Drive UI stops sending `archived: false` altogether (FR-019); the server default already supplies it. diff --git a/specs/37066-content-drive-status-filter/data-model.md b/specs/37066-content-drive-status-filter/data-model.md index 1d6456e51d3b..340c9a5e85c5 100644 --- a/specs/37066-content-drive-status-filter/data-model.md +++ b/specs/37066-content-drive-status-filter/data-model.md @@ -87,7 +87,21 @@ deterministic error is worth more. | Every element must name a `ContentStatus` | FR-010 | `ContentDriveHelper.parseStatuses` → `BadRequestException` | | Selection widens (OR), never narrows | FR-006 | `BrowserAPIImpl.appendContentStatusQuery` — one OR-ed group | | The archived baseline stands unless `ARCHIVED` is selected | FR-007 | `BrowserAPIImpl:2006` — the exclusion is skipped only when the selection contains `ARCHIVED` | -| A non-empty selection excludes folders | FR-015 | `ContentDriveHelper` → `.showFolders(false)` | +| A non-empty selection excludes folders **in the UI** | FR-015 | the store's `$request`, not the endpoint — see below | + +### Folders are the client's call, not the endpoint's + +Folders carry no status, so the Content Drive UI drops them once a status is selected — the +`showFolders` conjunction in the store's `$request` already does this alongside `baseType`, +`contentType` and `workflow`. + +The **endpoint deliberately does not enforce it.** Forcing `showFolders` to false server-side would +be a silent side effect: the response would stop matching the request, and `folderCursor` / +`hasMoreFolders` would report on a folder query the caller never received. A caller that asks for +folders alongside a status receives them. + +*(Note: the pre-existing workflow filter still forces `showFolders(false)` in `ContentDriveHelper`. +The two filters therefore differ. Reconciling that is outside this feature.)* ### The archived baseline is not a fourth flag, and it lives outside the OR group diff --git a/specs/37066-content-drive-status-filter/spec.md b/specs/37066-content-drive-status-filter/spec.md index bc1c44829253..30fda8891e43 100644 --- a/specs/37066-content-drive-status-filter/spec.md +++ b/specs/37066-content-drive-status-filter/spec.md @@ -235,7 +235,12 @@ selection and the results are the same at every step. convention already used by the other filters. - **FR-014**: The active selection MUST be reflected as a chip, consistent with the other toolbar filters, and MUST be clearable from that chip. -- **FR-015**: Folders MUST be excluded from the results whenever any status is selected. +- **FR-015**: Folders MUST be excluded from the Content Drive results whenever any status is + selected. This is a **client-side** rule: folders carry no status, so the Content Drive UI stops + requesting them. The search capability itself MUST NOT override an explicitly requested + folder setting — a caller that asks for folders alongside a status MUST receive them. Silently + overriding it would make the response stop matching the request, and would leave the folder + pagination describing a query the caller never received. - **FR-016**: A status selection MUST persist across navigation exactly as every other Content Drive filter does: deep link, page reload, browsing between folders, browser Back/Forward, and opening an item in the editor and returning. From 87d57f7011605605b42e52e12c698c43202cbf46 Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Tue, 25 Aug 2026 15:59:44 -0300 Subject: [PATCH 15/16] docs(content-drive): pin folder-rule flexibility as FR-015a (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FR-015 already says the folder rule is client-side. FR-015a makes the consequence a requirement rather than an implication: if folder visibility later becomes its own control, honouring it must stay a client-side change — no alteration to the search capability or its contract. That is the whole point of keeping the endpoint obedient. Writing it down stops the rule drifting back into the backend the next time someone reads FR-015 and assumes the API should enforce it. Spec changed after sign-off, so #37170 still needs re-approval. Co-Authored-By: Claude Opus 5 (1M context) --- specs/37066-content-drive-status-filter/spec.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/specs/37066-content-drive-status-filter/spec.md b/specs/37066-content-drive-status-filter/spec.md index 30fda8891e43..77b02f2ab3d1 100644 --- a/specs/37066-content-drive-status-filter/spec.md +++ b/specs/37066-content-drive-status-filter/spec.md @@ -241,6 +241,9 @@ selection and the results are the same at every step. folder setting — a caller that asks for folders alongside a status MUST receive them. Silently overriding it would make the response stop matching the request, and would leave the folder pagination describing a query the caller never received. +- **FR-015a**: Because the rule is client-side, changing it MUST remain a client-side change. If + folder visibility is later exposed as its own control, honouring it MUST NOT require altering the + search capability or its contract. - **FR-016**: A status selection MUST persist across navigation exactly as every other Content Drive filter does: deep link, page reload, browsing between folders, browser Back/Forward, and opening an item in the editor and returning. From 2c98e1bc878e0ab73cc12c5d53599f0758e2b06f Mon Sep 17 00:00:00 2001 From: Jalinson Diaz Date: Wed, 26 Aug 2026 12:15:36 -0300 Subject: [PATCH 16/16] docs(content-drive): accept the PURE_ES UNPUBLISHED divergence (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the open semantics question raised in review. Decision: accept and document, leaning on ADR-0018. UNPUBLISHED means "no live version exists" — a question about the content as a whole. The index stores `live` per VERSION, so a published item with newer unpublished edits has a working document carrying live:false, which the index query matches and `cvi.live_inode is null` does not. Under PURE_ES, UNPUBLISHED therefore returns that item too. The identifier-scoped meaning stays the definition. The index cannot express it: "does any version of this identifier have live:true" is not answerable from a single document, so there is no index-side fix short of redefining the status — which would change the DEFAULT path's behaviour to match a limitation of a strategy nobody runs. ADR-0018 already covers this. It routes structural predicates to the database precisely because the index cannot answer them reliably, and says PURE_ES forfeits that guarantee for every criterion and must not become the default. This is one instance of a limitation that decision already accepted. PURE_ES is opt-in and is not set in any config file in the repository. - FR-009 narrowed to the default strategy; new FR-009a records the exception and its ADR justification. - SC-005 scoped to match. - contracts/ gains a table contrasting the two predicates and the question each actually answers. ARCHIVED and LOCKED are unaffected, as is the default path. Spec changed after sign-off, so #37170 still needs re-approval. Co-Authored-By: Claude Opus 5 (1M context) --- .../contracts/drive-search-status.md | 23 +++++++++++++++++++ .../37066-content-drive-status-filter/spec.md | 17 ++++++++++++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md index 534254aa8f5a..dcd9722604fc 100644 --- a/specs/37066-content-drive-status-filter/contracts/drive-search-status.md +++ b/specs/37066-content-drive-status-filter/contracts/drive-search-status.md @@ -135,6 +135,29 @@ which routes version-info flags (archived/deleted) to the **database**. `PURE_ES promote it, but because it is a supported configuration where the filter would otherwise silently no-op. +### One accepted divergence under `PURE_ES` + +`UNPUBLISHED` does **not** mean quite the same thing on the two paths, and the difference is +accepted rather than fixed. + +| Path | Predicate | Question it answers | +|---|---|---| +| SQL (default) | `cvi.live_inode is null` | does this **content** have a live version anywhere? | +| Index (`PURE_ES`) | `live:false` | is **this version** the live one? | + +The index stores `live` per version, so a published item that also has newer unpublished edits has a +working document carrying `live:false` — which the index query matches and the SQL query does not. +Under `PURE_ES`, `UNPUBLISHED` therefore returns that item as well. + +**The identifier-scoped meaning is the definition** (see `ContentStatus.UNPUBLISHED`). The index +simply cannot express it: "does any version of this identifier have `live:true`?" is not answerable +from a single document. + +This is not a new limitation. ADR-0018 routes structural predicates to the database precisely +because the index cannot answer them reliably, and states that `PURE_ES` forfeits that guarantee for +*every* criterion and must not become the default. `PURE_ES` is opt-in and is not set in any config +in the repository. `ARCHIVED` and `LOCKED` are unaffected, as is the default path. + --- ## OpenAPI diff --git a/specs/37066-content-drive-status-filter/spec.md b/specs/37066-content-drive-status-filter/spec.md index 77b02f2ab3d1..99b9bfe1c45a 100644 --- a/specs/37066-content-drive-status-filter/spec.md +++ b/specs/37066-content-drive-status-filter/spec.md @@ -219,7 +219,19 @@ selection and the results are the same at every step. - **FR-008**: The existing inclusive "show archived" behavior relied on by the legacy Site Browser MUST be left unchanged; the exclusive Archived behavior is added alongside it. - **FR-009**: A status selection MUST produce identical results whether or not a keyword search is - active, and under every supported search strategy the environment can be configured to use. + active, under the default search strategy. +- **FR-009a**: Under the index-only strategy (`PURE_ES`, opt-in and not the default), *Unpublished* + MAY additionally return content that has a live version alongside newer unpublished edits. This is + an accepted divergence, not a defect: *Unpublished* means "no live version exists", which is a + question about the content as a whole, while the index records that flag per version — so a + published item's draft version reads as not-live. The identifier-scoped meaning is the definition; + the index cannot express it in a single-document query. + - This follows [ADR-0018](https://github.com/dotCMS/platform-adrs/blob/main/decisions/0018-database-first-content-drive-search-with-index-deferred-text-filtering.md), + which routes structural predicates to the database precisely because the index cannot answer + them reliably, and states that the index-only strategy forfeits that guarantee for *every* + criterion and must not become the default. This is one instance of a limitation that decision + already accepted, not a new one introduced here. + - Every other status is unaffected, and the default strategy is unaffected. - **FR-010**: An unrecognized status value MUST be rejected with a client error, consistent with how the drive already rejects unknown field-filter keys. - **FR-011**: A status selection MUST NOT conflict with a workflow filter that also constrains @@ -288,7 +300,8 @@ selection and the results are the same at every step. - **SC-004**: With no status selected, result counts and ordering are identical to those produced before the filter existed, for the same folder and filters. - **SC-005**: The same status selection returns the same result set with and without a keyword - search, and under every supported search strategy. + search under the default strategy. Under the opt-in index-only strategy, parity holds for every + status except the *Unpublished* case described in FR-009a. - **SC-006**: A status-filtered view reproduces the same selection and results after a reload, a folder change, a Back/Forward, an editor round-trip, and when opened by a second user from a shared link.