feat(edit-content): refactor the relationship field onto the shared search surface (#37192) - #37515
Draft
adrianjm-dotCMS wants to merge 6 commits into
Draft
feat(edit-content): refactor the relationship field onto the shared search surface (#37192)#37515adrianjm-dotCMS wants to merge 6 commits into
adrianjm-dotCMS wants to merge 6 commits into
Conversation
…ated-list fixes Spec-Kit PR 1 for #37192. Covers two independent bodies of work in the Relationship field of the new Edit Content: - The "Add Relationships" picker moves onto the shared search surface (full-width search + filter chips, no folder-tree sidebar) and searches through api/v1/drive/search. It is a selection editor: it opens with the already-related items checked, unchecking removes, and the confirm action is never disabled. - The related-content list in the form loses its 6-row paging and gains a "Load more" control at 40 rows, so drag-reorder reaches the whole set. Plus hover-only drag handle, right-aligned Status, and single-column form max width. 36 functional requirements, 43 acceptance scenarios across 6 user stories. The issue's acceptance criteria were updated to match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings from the PR #37482 review, all accepted. - FR-011 (new): the picker's selection is an accumulated set, independent of the result view. It survives paging, search and filter changes; already- related items enter it in full even when outside the current view; confirm reconciles against the set, never against the rows on screen. Today the picker fetches once and pages in the browser, so this holds by accident; api/v1/drive/search pages on the server and ends that accident. An edge case records that the current pre-selection filters only the first response, and hands the latent-defect question to /speckit-plan. - FR-016: states that it bounds FR-007 rather than contradicting it — exchanging the item on a single-cardinality field is a remove-then-add, not an in-picker swap. - US4 scenario 1: reworded so the retired 6-row page size stops reading as a live threshold. 37 FRs, 45 acceptance scenarios. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…earch surface (#37192) Implements the spec approved in PR #37482. **A — the "Add Relationships" picker.** Replaces the bespoke dialog with the shared surface: `dot-filter-bar` chips over a picker-owned `DOT_FILTER_FACADE`, `dot-folder-list-view` in multiple-selection mode, the shared site/folder browser behind a filter chip, and `api/v1/drive/search` pinned to the relationship's target content type. No folder-tree sidebar — the folder scope is a chip, per the design. The selection is an accumulated set keyed by identifier and seeded from the contentlets the field already holds, never rebuilt from a search response. That is what makes server-side paging safe: the dialog it replaces filtered its first response to decide what was pre-checked, so an already-related item that response did not contain was silently dropped on confirm. It is a selection editor, not an add-only dialog: it opens with the related items checked, unchecking removes, and the confirm action is never disabled — confirming an empty selection empties the relationship. **B — the related-content list.** Paging removed; 40 rows plus "Load more", following the Key/Value precedent. Hover-only drag handle, right-aligned Status, single-column form max width. Content Drive's field-filter chip opens the same dialog through DOT_RELATIONSHIP_PICKER; its contract spec passes unmodified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ationship E2E suite (#37192) Two gaps found by running the three save flows, the cardinality guard and Content Drive's field filter against a live backend. **The claimed-identifier set had no reader in the template.** `loadConstrained` filled it and `toggleSelection` refused the pick, but the row rendered like any other: the control toggled under the cursor, the editor confirmed, and the pick was dropped with nothing on screen explaining it. The store's own tests passed throughout, because they asserted `$isConstrained` rather than anything rendered. Fixed with an `unselectable` input on `dot-folder-list-view`, bound to a new `$constrainedList`. Refused and allowed are separate template branches, not one element with a bound `[disabled]`: PrimeNG's table controls drive their checkbox through `[(ngModel)]`, whose `setDisabledState` latches on init, so a row that was refused and then allowed again keeps a dead control. **The whole relationship E2E suite drove the deleted dialog.** All six spec files imported the page object for "Select Existing Content", and the table spec drove the paginator FR-021 removed — so this change was breaking CI. Rewritten as `add-relationships-dialog.ts`, with two tests that could not simply be ported: - the confirm-disabled assertion is inverted per FR-013; - the two paging tests are now Load more (FR-021/FR-022). Adds the unrelate-and-save coverage that never existed. Relating and reordering each had a test that survived a save; removing did not — the direction where a change that never reached the payload looks identical on screen until a reload. Verified server-side through `GET /api/v1/content/{id}?depth=1`, not just in the rendered list: relate, unrelate, drag-reorder, the cardinality guard's positive path (a ONE_TO_ONE child already held by another parent, which no test had ever exercised against real data), and Content Drive's chip round-tripping to `us.rel:<identifier>`. E2E 51 passed · ui 2031 · edit-content 2274 · content-drive 1366 · lint · AOT Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ships component Removed unused empty state keys and the associated clear filters functionality from the Add Relationships component. Updated the component to rely on the store's reset method for clearing filters and scope, ensuring a more consistent user experience. Enhanced the filter facade to account for changes in the browsed site, allowing for clearer state management when navigating between sites and folders. This change streamlines the code and improves the clarity of the empty state behavior in the UI.
…psSiteChipComponent Enhanced the unit tests for the AddRelationshipsSiteChipComponent to cover scenarios where the component's scope is echoed back from the `dot-host-folder-field`. Added checks to ensure that the component does not reset paging when the echoed value matches the current site being searched, and that it correctly re-scopes when a genuinely different value is provided. This addresses potential issues with duplicate search requests on throttled connections.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 2 of 2 — implementation. Spec approved in #37482; this branches off that branch, so its diff shrinks once PR 1 merges.
Closes #37192.
What changed
A — the "Add Relationships" picker. The bespoke dialog is replaced by the shared surface:
dot-filter-barchips over a picker-ownedDOT_FILTER_FACADE,dot-folder-list-viewin multiple-selection mode, the shared site/folder browser behind a filter chip, andapi/v1/drive/searchpinned to the relationship's target content type. No folder-tree sidebar — the folder scope is a chip, per the design.B — the related-content list. Paging removed; 40 rows plus Load more, following the Key/Value field's precedent (#37191). Hover-only drag handle, right-aligned Status, single-column form max width.
The one design decision worth reading
The selection is an accumulated set keyed by identifier, seeded from the contentlets the field already holds — never rebuilt from a search response.
The dialog this replaces filtered its first search response to decide what was pre-checked, so an already-related item that response did not contain was never checked and was silently dropped on confirm. That was survivable only because the old dialog fetched once and paged in the browser.
api/v1/drive/searchpages on the server, so the accident would have become a reliable way to lose relationships. Per ADR-0018 the free-text filter is index-backed and lags, which is a second, independent reason the selection cannot be derived from search results.Second consumer — please don't miss this
DotSelectExistingContentComponentwas not only the relationship field's. Content Drive's shared field-filter chip opens the same dialog throughDOT_RELATIONSHIP_PICKER. That provider is re-pointed here and its contract spec passes with only presentation assertions updated (which component and geometry it opens — C5 explicitly allows that). Contract:specs/37192-relationship-field-assetpicker/contracts/relationship-picker.contract.md.Verified live: the chip opens the dialog with
selectionMode: 'single'and theApplylabel, and round-trips tous.rel:<identifier>in the URL with the result list filtered to match.Its replacement footer is deleted: it reached the old store across PrimeNG's
templates.footerboundary only because that store wasprovidedIn: 'root'. The new store is per-dialog on purpose, so the two footers collapsed into one with a configurable label.What a live backend found that the green suite did not
Three things only showed up once the flows were run against a rebuilt instance. They are the most useful part of this PR to review.
1. The cardinality refusal had no reader in the template. The store loaded the identifiers already claimed by another parent and
toggleSelectionrefused them — and the row rendered like every other. The control toggled under the cursor, the editor confirmed, and the pick was dropped with nothing on screen explaining it. The store's own tests passed throughout, because they asserted$isConstrainedrather than anything rendered.Fixed with an
unselectableinput ondot-folder-list-view. Refused and allowed are separate template branches rather than one element with a bound[disabled]: PrimeNG's table controls drive their checkbox through[(ngModel)], whosesetDisabledStatelatches on init, so a row that was refused and then allowed again keeps a dead control. The reset direction is what caught it.2. The entire relationship E2E suite drove the deleted dialog. All six spec files imported the page object for "Select Existing Content", and the table spec drove the paginator FR-021 removed — this change was breaking CI and I had not noticed. Rewritten as
add-relationships-dialog.ts. Two tests could not simply be ported: the confirm-disabled assertion is inverted per FR-013, and the two paging tests are now Load more.3. A test I wrote contradicted FR-016. "Confirm an empty selection to unrelate the last item" was written against single cardinality, where FR-016 keeps the picker shut once the field holds its item. It only means anything on a multi-cardinality field. Running it is the only reason I know.
Open points — the reason this is a draft
1. Six
libs/uiedits, against my own plan. The plan named touching that library the signal to stop and re-scope; I did not stop. All additive with defaults,uigreen at 2031 tests:dot-chip-filter— optionalicon; a titleless chip renders its value as its labeldot-folder-list-view— configurable empty-state copy plus an optional actiondot-folder-list-view—unselectablerows (finding 1 above; not optional, without it the cardinality guard is invisible)dot-asset-picker— full-screen behaviour extracted to the sharedfullscreen-dialogmodule and now used by both (this one reduces duplication)Either justify them here or tell me which to revert.
2. Columns are Content Drive's, not the design's. Shown: Name, Status, Locale, Type, Edited By, Last Edited. The artboard asks for Title, URL Title, Posting Date, Locale, Status, Last Modified. Changing them reaches into
dot-folder-list-view— a scope decision, not a tweak.3. New i18n keys need a backend rebuild. They are in
Language.properties; a server serving the deployed copy shows raw keys.Also worth a reviewer's eye
Site scoping changed.
/drive/searchresolves exactly one host and has no all-sites mode, so the picker now browses one site where the old dialog searched across all of them by default. The chip names the site being searched, so the UI no longer implies otherwise — but it is a behaviour change and belongs to product, not to me. Shared Assets is skipped without a request, since its hostname cannot be parsed as a URI authority (it used to surface as a 500).The "Show Selected" toggle is restored, not new. The old dialog had it; the rewrite lost it and
/speckit-convergereported the absence as a gap, which I initially read as a missing feature rather than as my own regression. Only the(N)counter is new — and FR-011 is what makes it necessary, since the selection accumulates across pages, searches and sites.Store duplication left deliberately undone.
withAssetBrowsealready does this store's browse work. Generalising it means changing a shared feature with five shipped consumers (File field, Image field, three Story Block nodes, WYSIWYG,openBrowserModal) with no automated safety net for the behaviour that matters. Its two documented traps were fixed inside this store instead — per-page cursor bookmarks, and a row count that is notcontentCount. De-duplication deserves its own issue.One pre-existing bug recorded, not fixed. Routing between two contentlets whose relationship fields differ in cardinality opens the picker with the previous field's selection control. It self-corrects on reopen, and the read is the same one the deleted dialog made — out of scope here, noted so it is not mistaken for a regression from this change.
Verification
relationship-field)edit-contentuiportlets-content-driveVerified server-side, not just on screen — each through
GET /api/v1/content/{id}?depth=1: relate → save, unrelate → save, drag-reorder → save (order preserved), and the cardinality guard's positive path, a ONE_TO_ONE child already held by another parent, which no test had ever exercised against real data.No Integration or Postman: frontend-only against unchanged contracts (SC-007, ADR-0013).
The pattern is the thing to review, not the individual fixes. Every bug in this PR that reached a human was a test asserting what I wrote rather than what the system needs —
objectContainingon one field, adata-testidthat survives on an unresolved element, a computed with no reader, a mocked call asserted instead of its effect. The fixes now assert the outgoing request or the rendered input. Where I rebuilt from the spec and the design, anything the spec did not name was lost in silence; the 252-line page object I deleted turned out to be the only inventory of what the old dialog actually did.🤖 Generated with Claude Code