feat(experiments): create/update experiment screen (/experiments/new + /:id/configuration) - #37064
Open
oidacra wants to merge 41 commits into
Open
feat(experiments): create/update experiment screen (/experiments/new + /:id/configuration)#37064oidacra wants to merge 41 commits into
oidacra wants to merge 41 commits into
Conversation
10 tasks
Contributor
|
Claude finished @oidacra's task in 3m 12s —— View job Code Review — PR #37064I reviewed the store/autosave core, the data-access layer (Lucene safety, PATCH shape), the form utilities, the shell + cards, routing, and the shared UI changes against This is a large but exceptionally well-reasoned change: the autosave diff/settle logic ( Resolved
Notes (non-blocking, no change required)
No new bugs, security issues, or convention violations found in the diff. Nothing blocking merge.
|
github-actions
Bot
added
the
Area : Frontend
oidacra
force-pushed
the
oidacra/experiments-portlet-screen-2-create-update-exper
branch
from
August 18, 2026 15:13
a992c18 to
dc6ecad
Compare
oidacra
marked this pull request as ready for review
August 18, 2026 16:10
oidacra
added
the
PR: docker image
Contributor
🐳 PR Docker test imageLatest build for commit docker pull dotcms/dotcms-test:pr-37064-oidacra-experiments-portlet-screen-2-create-update-exper
docker pull dotcms/dotcms-test:pr-37064-oidacra-experiments-portlet-screen-2-create-update-exper_f5a6212 |
Base automatically changed from
oidacra/experiments-portlet-screen-1-portlet-base-site-w
to
main
August 18, 2026 19:52
…e endpoint DotPagesBrowserService wraps /api/v1/page/search, folder search and an ES-search page-lock lookup for the standalone Configure screen, which has no UVE resolver chain to lean on. DotExperimentsService gains setName, mirroring setDescription over the generic experiments PATCH.
…ed autosave Signal Store Events, same shape as the list store: page-intent and API-outcome event groups, reducers per card, and one debounced switchMap handler per PATCH field group so edits collapse per group without cross-group merging. Creation is the autosave: one POST as soon as name+page exist, then a relative replaceUrl swap to /:id/configuration. targetingConditions never enters an outgoing payload. Skipped autosaves settle through an explicit event so $isAutosaving cannot stick. The 8 start-gated validation rules, transitions (start/stop/cancelSchedule/ abort over the same endpoint) and non-DRAFT locking, SCHEDULED included, are lifted from the legacy configuration store.
…wo dialogs Routed screen per the approved design: fixed header (back, title, status, kebab gated by AllowedActionsByExperimentStatus), locked banner, scrolling 1040px card column and pinned footer whose Start button is never disabled. One component per card - Details, Goal (radio types plus backend-validated condition operators), Page (immutable after creation, prefill error inline), Variants (inplace rename, copy variant URL, lock awareness, split evenly, n/max cap) and Scheduling (p-datepicker with time plus min/max duration bounds). Select A Page (folder tree, page table, pages with a live experiment greyed) and Add Variant dialogs. All form state is Angular signal forms; Jest specs accompany every component.
…ide list actions One UrlMatcher-based route config serves both paths so the post-creation replaceUrl swap reuses the shell instance instead of tearing it down mid-autosave; the experiments mount point drops reuseRoute: false for the same reason, mirroring the /content precedent. The list's New Experiment button comes alive and Configure leads each row's kebab.
89 keys covering the five cards, both dialogs, header/footer, locked banners and validation copy, following the list screen's naming.
The matcher, the list's navigation commands and the store's post-creation replaceUrl all spelled 'new'/'configuration' independently; one rename now touches one file.
…tants Pure functions move to util/dot-experiments-configure.util.ts and the i18n keys plus weight constants to shared/constants.ts, matching the list's layout. Kills three duplicate '/experiments' literals and a second TOTAL_WEIGHT along the way.
…PATCH
PATCH /api/v1/experiments/{id} applies every key of its body atomically,
so the per-field-group event pairs collapse into formEdited plus one
save triple over a generic DotExperimentsService.patch. The reducer
accumulates the diff; a single debounced switchMap flushes it, holding
back what the backend would reject (weights not summing 100 construct-
time-throw on TrafficProportion) and settling only the keys each
response actually carried, so mid-fix weights never snap back. SAVING
now marks the flight alone via saveRequested. Validation docs cite the
verified backend enforcement points.
…dTree slices One ConfigureFormModel and one form() with the schema inline; the cards take their subtree as input.required<FieldTree<Slice>> and bind leaves with [formField], with goal/scheduling rules composed via apply() from schemas the cards export. Hydration happens once per experiment in the shell, and a single bindFormAutosave diffs the root model. required() stays out of the schema deliberately: it reaches the DOM as the native attribute and would paint empty fields red on first render (AC28). Also the visual review pass: screen-enter fade on both screens, the traffic p-slider freed from a full-width input group, textarea styled via pInputText (bare p-textarea classes load no CSS without the directive, which crashes against signal forms), circular back button, wider control gaps, a synthetic Original/CONTROL row while the draft does not exist yet, and UVE's 3px indeterminate bar for the flight of each save.
… as validate()
variantWeights becomes a form slice with applyEach bounds per row and a
cross-field validate() reporting {kind: weightsTotal} — the same literal
the Start rules use, so the live warning bar and the reveal-on-start read
one truth. The card's warning and total read the slice; splitEvenly stops
being a store event (the card writes splitWeightsEvenly into the slice,
keeping SPLIT_EVENLY on the wire so addVariant still redistributes
server-side). The slice reseeds only when the variant identity set
changes, so save echoes never clobber weights mid-edit, and the wire
guard holding back sums that are not 100 stays untouched. Weight is
number|null: a cleared native number input writes null, not 0.
FormValueControl<string> per the custom-controls guide: value is the group's selection (the contract reserves that name, so the card's own option rides an option input), checked derives from their equality, and '' is the no-selection sentinel. Host is the radio — role, aria-checked, tabindex and keyboard selection — with the goal card's approved visuals. Signal forms only by decision: a ControlValueAccessor would even shadow the signal-native path, since FormField resolves CVA first. The goal card renders its four types with it, keeping the atomic slice write on select.
The circle is now p-radioButton's, rebuilt rather than reused: a 1.5rem box with a 1px border that fills with primary when checked, holding a 1rem primary.contrast dot that scales between 0.1 and 1 over form.field.transition.duration. Those are a real radio's rendered values, measured in the running app, so a radio drawn as a card now looks like every other radio in dotCMS — and gaining or losing the selection animates in both directions instead of switching in a single frame. p-radioButton itself cannot be reused here: it resolves NgControl from the injector without `optional`, so mounting one with no reactive or template form binding throws NG0201. Reusing it would drag NgControl into a control that is signal forms only, and nest a second focusable radio inside a host that is already role="radio". The card's own outline follows form.field as well, because a radio card is a control and not a container: the same 6px radius and border colour as the pInputText beside it. Nothing points at --p-radiobutton-* or --p-card-* — PrimeNG emits a component's variables together with its stylesheet, so both families are undefined on a screen with no radio button and no card mounted.
The surface, radius, border, padding and shadow are no longer restated in the component: they come from `components.card` in the preset, and follow it when it changes. The card now renders at the theme's 8px radius with its 1px gray.300 border and 1rem body padding, and mounting it also brings the `--p-card-*` tokens and the `.p-card` stylesheet onto the screen, which were absent before. `p-card` puts `.p-card` on its own host element, so the radio semantics stay on `<dot-radio-card>` one level out and the host is left a bare block the card fills. The selection accent is the only thing painted over the theme, and it needs `!` to land: PrimeNG injects component CSS outside any cascade layer while Tailwind's utilities live in `@layer utilities`, and unlayered declarations win regardless of specificity. The radio circle stays hand-built — `p-radioButton` resolves `NgControl` without `optional` and throws NG0201 with no form binding — but it is built from a real radio's measured values, so it reads as a PrimeNG radio.
The reason was buried next to the class constants it justifies. It belongs in the component doc, where a reader looks before reaching for p-radioButton again: its `onInit` calls `this.injector.get(NgControl)` with no `optional`, so one rendered with no ngModel, formControl or formControlName throws NG0201 — confirmed by mounting a bare one. Reusing it would mean an internal reactive-forms control existing only to paint a circle, inside a control whose contract is signal forms, plus a second focusable radio nested in a host that is already role="radio". Names the condition that would make it worth revisiting, so the decision does not have to be re-derived.
None of the five sections draws its own card any more: Details, Goal, Page, Variants and Scheduling each dropped `rounded-lg border border-surface-200 bg-white p-6` for a `p-card`, so their surface, radius, border, padding and shadow all come from `components.card` in the preset and follow it when it changes. They were rendering at 7px with a `surface-200` border while the theme's card is 8px with a `gray.300` one, which is exactly the drift this removes — the interior padding is now the theme's `1rem` rather than the template's `1.5rem`. The `<section>` element stays, one level in, so the card is chrome and the sections keep their grouping semantics and their `data-testid`.
Cuts the usage example, the card-chrome and animation paragraphs and the per-member restatements — the template and the class names carry those. Keeps the four things a reader cannot derive: that each card is one radio rather than a select, that it is signal forms only, why the circle is not a p-radioButton (NG0201), and why the accent needs the important suffix.
…s strings The class no longer decides how anything looks. `aria-checked` and `aria-disabled` were already on the host as the radio's semantics, so the stylesheet keys off them and the six class-name constants, the three class computeds and the host `[class]` binding all go away — including the two `!important` utilities: a component stylesheet is unlayered and scoped, so `p-card[_ngcontent]` outranks PrimeNG's `.p-card` on specificity where a Tailwind utility, sitting in `@layer utilities`, could not. Verified in the browser: the accent, the circle and the 0.1→1 dot all still land, sampled mid-transition. Real CSS also states the radio's geometry in the units the theme uses, so the comments explaining `border-(--p-form-field-border-color)`-style strings are gone with the strings. The template keeps Tailwind for layout and type, where nothing competes with it. Three assertions on class strings go with them: the appearance they stood for is now CSS keyed on an attribute the tests already assert, and jsdom computes no styles.
…ither forms API One implementation now serves every caller: reactive forms with `[formControl]`, templates with `[(ngModel)]`, signal forms with `[formField]` through the interop bridge, and no form at all with `[(value)]`. It replaces `FormValueControl`, which was signal-forms-only — and the two cannot be combined, because `FormField` resolves a value accessor before a custom control, so a CVA replaces the native path rather than adding to it. `disabled` is now the union of the input and what a bound control writes through `setDisabledState`, and blur reports through `registerOnTouched` — which is what the `touch` output existed for, so it goes. The spec gains a reactive-forms host alongside the signal-forms one: value both ways, disable(), and touched, asserted against each API rather than against the interface members. The signal-forms host is now the guard against the interop path regressing.
…ligned Preview / Edit Content and Split Evenly were outlined buttons competing with the weight input for attention, and Split Evenly sat wherever its column started. Both are now text buttons — no border, no fill — pinned to the right edge of their column, so the three rows end on the same line and the only bordered control in the area is the one you type into.
…thing A list that fits in the smallest page size had a working rows-per-page select above it: picking 25 or 50 rendered the same single page either way. PrimeNG only disables that select at zero records, so it stayed live on four experiments. It is now offered only when the total exceeds the smallest option, through `rowsPerPageOptions` — PrimeNG renders it only when that is set, and it already disables the page arrows on a single page, so the whole bar goes inert together and reads "Page 1" with nothing to press. Verified in the browser on a four-experiment list. Dropped rather than greyed out on purpose: keeping it visible and disabled means CSS that a keyboard can walk straight past, since the select is a component and not a native control. This is the framework's own way of saying there is nothing to choose.
The weights only ever travel as a set adding up to 100 — `TrafficProportion` rejects anything else on construction — so leaving the arithmetic to the user meant typing 20 and then working out that the rest is 80, or with three variants, which two numbers add up to the remainder. The row just committed now decides its own share and the rest is spread over the others in the proportion they already had: 20 on a two-row split gives the other 80, and 50/30/20 with 20 committed on the first becomes 20/48/32. The rounding drift lands on the largest adjusted row, so three rows still add up to exactly 100. On commit — blur or Enter — and not on every keystroke, which is what separates this from a UI that fights back: typing 2 on the way to 25 would otherwise send the other rows to 98 and back. Typing alone changes nothing but its own row, and the amber total warning stays for what a commit cannot produce: an experiment arriving from the server at anything but 100. Three cases are deliberately left alone, since there is nothing to spread from: a cleared row, a value the range rules already reject, and a total that adds up already. Rows holding nothing between them share the remainder evenly, having no proportions to keep. Verified in the browser: 20/80 typed to 30 shows 30/80 while typing and settles at 30/70 on blur.
… set Spreading the remainder over every other row undid decisions as they were made: setting 15 on Original and then 50 on a variant moved the 15, which is the one number the user had just been explicit about. The remainder now goes to the rows they have not set. With 15 and then 50 committed, the third row takes the 35 that is left and the 15 stays put. Only when every other row has been set does one have to move, and then it is the one set longest ago — and only as far as the arithmetic demands, widening to the next oldest if that one cannot absorb it alone. Split Evenly forgets every decision, since it overrides every weight: after a split, the rows are all fair game again. Verified in the browser on a three-variant experiment: 27/15/58, commit 15 on Original for 15/17/68 (the two untouched rows share the 85 in proportion), then commit 50 for 15/50/35 — with the 15 untouched.
It appeared and disappeared in a single frame, which is most noticeable exactly when it happens most — while a split is being completed and the total crosses 100. It now enters over 150ms, settling from 2px up so it reads as the strip taking its place, and leaves over 120ms. Same short duration family as the screen's own transition and the content-drive toolbar, through the same `animate.enter` / `animate.leave` mechanism, and dropped entirely under `prefers-reduced-motion`.
… beside it The row already stretches its children and the design asks for exactly that (`align-items: stretch`), but PrimeNG's button keeps its natural height one element in, so it sat 8px short and read as floating above the field. `h-full` on it lets it fill the row. Measured both states: 48px against 48px with a page selected, 41px against 41px while none is, top and bottom flush in each.
oidacra
force-pushed
the
oidacra/experiments-portlet-screen-2-create-update-exper
branch
from
August 18, 2026 19:52
7bc67f0 to
c1da055
Compare
`setName(id, name)` was `patch(id, { name })` under another name: same endpoint,
same verb, same mapping. It was added here to complete the legacy family of
single-key setters (`setGoal`, `setDescription`, `setScheduling`, ...) that the
Configure screen exists to replace with one accumulated multi-key call, and
nothing ever called it.
… why it overlaps The folder shapes went with the dialog that used them: `getFolderChildren` was removed when the Page card moved to the shared site browser, but `DotPageBrowserFolder`, `DotPageBrowserFolderChildren` and `DotPageBrowserFolderParams` stayed behind, exported from the barrel and referenced by nothing. Both docblocks still advertised folder navigation as a capability of a service that has two methods, neither of which browses folders. `DotPageSelectorService` already calls the same two endpoints, so the overlap is real and worth explaining rather than leaving for the next reader to find: a library cannot depend on an app, that service answers with the label/value shape an autocomplete needs rather than table rows, and it knows nothing about a page's lock state. Pointers now sit on both sides.
…e old screen does The display/edit swap was hand-rolled here with an `@if` and an `$isEditing` signal, while the old screen builds the same affordance on PrimeNG's `p-inplace` — which the theme already styles (`dotcms-theme/components/_inplace.scss`) and which gives the read state the `role="button"` and keydown handling the `@if` had no equivalent for. What was worth not reusing is the old `dot-experiments-inplace-edit-text` wrapper, which is ReactiveForms-based; the editor stays on signal forms. The docblock claimed that as the reason for hand-rolling the swap too, which it never was. `p-inplace` now owns the swap through `pInplaceDisplay`/`pInplaceContent`, Save and Cancel close it with `deactivate()`, and `onActivate` reseeds the draft from the persisted name so the editor opens the same way whichever affordance opened it. The sixteen existing tests pass untouched.
… fixed Five review findings on the Configure store and its cards. `validationErrors` was a snapshot frozen by the Start press, and only the Details card re-checked its own field against the live value — so seven of the eight rules stayed red after the user fixed them. The state now latches the *reveal* (`validationRevealed`) and the errors derive from the form, which drops the per-card re-checks and makes every rule clear itself. The scroll-to-first-error moves off the error list onto the press that caused it, or it would jump the screen on every keystroke that changed a rule. Autosave could lose an edit outright: `withoutSentKeys` settled a pending key by *name*, and `switchMap` only cancels a flight when the next debounce emits, so an edit made while the response was travelling was dropped with nothing left to resend it. Type, pause, keep typing, stop — and the server kept the older value while the form still showed the newer one. Keys now settle only while their pending value is still the one that went out. `enterExisting` reset the pending diff but not the validation state, so a URL pasted for another experiment carried the previous one's errors over. It now drops everything, as `enterNew` already did. A rejected page lookup was reported as "page not found" and never reached `DotHttpErrorManagerService`: a backend outage told the user the page did not exist and left ops with no signal. It gets its own event, its own copy, and the shared error manager. The variants counter set a static `aria-label` over its own text, so a screen reader heard "Variants used" and never the numbers. Two displaced JSDoc blocks move onto what they describe, and the Details card no longer claims a `required` rule the form never declares.
A PATCH against a local backend answers in a handful of milliseconds, so the indeterminate bar appeared and vanished inside a frame or two: the eye caught that something blue flickered under the header without ever resolving it into "saved", which reads as a glitch rather than as feedback. Once shown the bar now stays for at least `MIN_PROGRESS_BAR_VISIBLE_MS`, timed from when it appeared, so a fast save still gets a legible beat of it and a slow one keeps it up for as long as it takes. A save starting while a hide is pending cancels that hide and leaves the bar where it is, so back-to-back saves read as one continuous bar instead of a blink between them. The store is untouched: `$isSaving` still reports exactly when a request is on the wire, and how long that is worth showing stays with the screen that draws it. Its docblock also picks up a correction — `$isSaving` covers every request the screen makes, not just the autosave.
…tered `validateConfigure` read the goal and the weights off the persisted experiment only, which on `/experiments/new` is still `null`: a goal the user had just filled in went on being counted as missing until picking a page created the draft. Pressing Start on an empty form and working down the list — which is what the reveal is for — left the Goal card contradicting itself. The values were never lost, just unread. `formEdited` merges every key into the pending diff whether or not an experiment exists, and `applyPatchToExperiment` has nothing to apply an edit to before the POST answers, so the goal lives only there. The rules now read the diff first and fall back to the experiment, which is the same shape `draftName` and `selectedPage` already had for the two fields that could be filled in before creation. `minVariants` stays unsatisfiable until the draft exists, and that is not this: adding a variant needs an experiment id, so the card keeps `Add new variant` disabled until the POST answers.
…query
`?pageId=` is whatever the address bar carries, and both page lookups concatenate
it straight into a query string: `+identifier:${pageId}` in the prefill search
and in `getPageLockState`. A value carrying spaces or Lucene operators widens the
query instead of naming an id, so a crafted URL could prefill the Page card with
another contentlet, or answer with another contentlet's lock state. The endpoints
are permission-scoped and read-only, so this is query correctness rather than
privilege escalation — but it is a user-input entry point this screen opened.
`isDotIdentifier` states the shape once, next to the other shared helpers.
Nothing outside a 36-character UUID can name a page, so both call sites answer as
they would for a page that is not there — the prefill reports not-found, the lock
lookup reports unlocked — without spending a request on it.
The fixtures said `page-1`, which is not an identifier any dotCMS install would
produce; a fake that shape is why nothing here looked risky. They are UUIDs now,
which is also what makes the two guard tests meaningful.
Reported by claude[bot] on the PR.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces the new routed Experiments “Configure” experience so experiment creation/editing happens on dedicated routes (/experiments/new and /experiments/:id/configuration) with autosave-driven POST/PATCH flows, plus supporting UI primitives and data-access to enable page picking, locking awareness, and per-card editing.
Changes:
- Adds Configure routing (single
UrlMatcherroute), screen shell/cards/dialogs, and autosave event/store plumbing for create/update. - Extends shared UI/utilities (radio-card UI, identifier validation utility, browser-selector upload visibility + initial selection).
- Adds/extends data-access and models to support multi-key PATCH and page browsing/lock lookup.
Reviewed changes
Copilot reviewed 70 out of 70 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| dotCMS/src/main/webapp/WEB-INF/messages/Language.properties | Adds i18n keys for the new Experiments Configure screen and list “Configure” action. |
| core-web/libs/utils/src/lib/dot-utils.ts | Adds isDotIdentifier UUID-shape guard for safe identifier use in query strings. |
| core-web/libs/utils/src/lib/dot-utils.spec.ts | Adds unit tests for isDotIdentifier. |
| core-web/libs/ui/src/lib/components/dot-radio-card/dot-radio-card.component.ts | New reusable radio-as-card component for signal forms + PrimeNG. |
| core-web/libs/ui/src/lib/components/dot-radio-card/dot-radio-card.component.html | Template for the radio-card component. |
| core-web/libs/ui/src/lib/components/dot-radio-card/dot-radio-card.component.scss | Styling for selected/hover/disabled states (including :has). |
| core-web/libs/ui/src/lib/components/dot-radio-card/dot-radio-card.component.spec.ts | Spectator tests validating radio-card behavior in a real form host. |
| core-web/libs/ui/src/lib/components/dot-browser-selector/dot-browser-selector.component.ts | Adds $canUpload and selection plumbing for the shared browser selector. |
| core-web/libs/ui/src/lib/components/dot-browser-selector/dot-browser-selector.component.html | Wires selected folder id into sidebar and upload visibility into dataview. |
| core-web/libs/ui/src/lib/components/dot-browser-selector/components/dot-sidebar/dot-sidebar.component.ts | Adds selectedId input and updates selected-node derivation. |
| core-web/libs/ui/src/lib/components/dot-browser-selector/components/dot-dataview/dot-dataview.component.ts | Adds showUpload input to allow hiding upload affordances for unuploadable browsers. |
| core-web/libs/ui/src/lib/components/dot-browser-selector/components/dot-dataview/dot-dataview.component.html | Conditionally renders file input + upload button based on showUpload. |
| core-web/libs/ui/src/index.ts | Exports the new dot-radio-card component from the UI library barrel. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/util/dot-experiments-autosave.util.ts | Adds a single-root-form autosave binding that dispatches diffs to the store. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/store/dot-experiments-configure-page.events.ts | Defines Configure “page intent” events (enter/load/edit/variants/transitions/prefill). |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/store/dot-experiments-configure-api.events.ts | Defines Configure “API response” events for load/create/save/variants/transitions/prefill/lock. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/shared/models.ts | Adds Configure screen view state, form model slices, validation rules, and view models. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/shared/constants.ts | Adds Configure routing constants, autosave timings, copy keys, and picker/dialog constants. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/lib.routes.ts | Wires Configure route via a single matcher and adds config resolvers/data requirements. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/lib.routes.spec.ts | Adds matcher tests and resolver/provider/route-reuse assertions. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-list/dot-experiments-list.component.ts | Enables New Experiment navigation and adds “Configure” as a leading row action. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-list/dot-experiments-list.component.html | Enables New Experiment button, adjusts paginator rows-per-page options rendering. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-list/dot-experiments-list.component.scss | Adds screen-enter fade and reduced-motion handling for the list view. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-list/dot-experiments-list.component.spec.ts | Updates list specs for Configure action, New Experiment navigation, and pagination UI behavior. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/dot-experiments-configure.component.html | Adds the Configure shell layout (header/body/footer, progress bar, locked banner, states). |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/dot-experiments-configure.component.scss | Adds Configure screen enter animation and progress-bar overlay wrapper. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-variants/dot-experiments-variant-name-inplace.component.ts | Adds signal-forms-based inplace variant rename control. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-variants/dot-experiments-variant-name-inplace.component.html | Template for inplace variant rename (p-inplace + save/cancel). |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-variants/dot-experiments-variant-name-inplace.component.spec.ts | Tests rename UX (open/close/trim/disable/max length). |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-variants/dot-experiments-configure-variants.component.html | Implements Variants card UI (weights, copy URL, split evenly, add/delete, warnings). |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-variants/dot-experiments-configure-variants.component.scss | Adds animated enter/leave transitions for the weights warning strip. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-scheduling/dot-experiments-configure-scheduling.component.ts | Implements Scheduling card logic over a signal-form slice and backend-provided bounds. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-scheduling/dot-experiments-configure-scheduling.component.html | Implements Scheduling card UI with date+time pickers, bounds, note, and Clear Schedule. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-scheduling/dot-experiments-configure-scheduling.component.spec.ts | Tests scheduling UX and bounds behavior (including time picker rendering). |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-page/dot-experiments-configure-page.component.ts | Implements Page card (select page dialog, immutability after create, traffic allocation field). |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-page/dot-experiments-configure-page.component.html | Implements Page card UI with summary, Select button, prefill/required errors, slider+input. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-header/dot-experiments-configure-header.component.ts | Implements fixed header with back/title/status/subline and allowed-actions kebab. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-header/dot-experiments-configure-header.component.html | Header template including disabled “View Results” placeholder and Stop button. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-goal/dot-experiments-configure-goal.component.ts | Implements Goal card using radio cards + conditional panels and start-gated validation display. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-goal/dot-experiments-configure-goal.component.html | Goal card template: name input, radio-card type selection, condition panel, errors/hints. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-footer/dot-experiments-configure-footer.component.ts | Implements pinned footer hint precedence and Start/Schedule dispatching. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-footer/dot-experiments-configure-footer.component.html | Footer template with hint, back button, and Start/Schedule button. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-footer/dot-experiments-configure-footer.component.spec.ts | Tests footer hint precedence and start/back behaviors. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-details/dot-experiments-configure-details.component.ts | Implements Details card for name/description leaves and start-gated required messaging. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-details/dot-experiments-configure-details.component.html | Details card template with name/description inputs and errors/hints. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-configure-details/dot-experiments-configure-details.component.spec.ts | Tests details editing, validation reveal behavior, and locked state. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-add-variant-dialog/dot-experiments-add-variant-dialog.component.ts | Adds Add Variant dialog using signal forms and deterministic fallback naming. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-add-variant-dialog/dot-experiments-add-variant-dialog.component.html | Add Variant dialog template with optional name field and submit/cancel. |
| core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-configure/components/dot-experiments-add-variant-dialog/dot-experiments-add-variant-dialog.component.spec.ts | Tests typed vs generated names, trimming, validation, and cancel behavior. |
| core-web/libs/dotcms-models/src/lib/dot-experiments.model.ts | Adds DotExperimentPatchBody contract for multi-key PATCH without pageId/targetingConditions. |
| core-web/libs/data-access/src/lib/dot-pages/dot-pages-browser.service.ts | Adds page browsing + lock-state lookup service, with Lucene-injection-safe identifier checks. |
| core-web/libs/data-access/src/lib/dot-pages/dot-pages-browser.models.ts | Adds models for page browsing rows, query params, and lock-info shapes. |
| core-web/libs/data-access/src/lib/dot-experiments/dot-experiments.service.ts | Adds patch() method to support multi-key experiment PATCH in one request. |
| core-web/libs/data-access/src/lib/dot-experiments/dot-experiments.service.spec.ts | Adds tests for multi-key patch, schedule clearing, and payload shape expectations. |
| core-web/libs/data-access/src/index.ts | Re-exports the new DotPagesBrowser service/models from the data-access barrel. |
| core-web/apps/dotcms-ui/src/app/view/components/_common/dot-page-selector/service/dot-page-selector.service.ts | Adds documentation clarifying overlap with the new page browser service. |
| core-web/apps/dotcms-ui/src/app/app.routes.ts | Removes inherited reuseRoute: false from the experiments mount to preserve Configure reuse behavior. |
| core-web/apps/dotcms-ui/src/app/app.routes.spec.ts | Updates routing spec to assert experiments subtree doesn’t force reuseRoute: false. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+52
to
+55
| /** Nothing to clear before a date is set, and nothing may be cleared once locked. */ | ||
| protected readonly $canClearSchedule = computed<boolean>( | ||
| () => !this.$isLocked() && !!this.$startDate() | ||
| ); |
`$canClearSchedule` keyed off the start date, but an end date alone is a schedule
the backend keeps: `toRange` sends `{ startDate: null, endDate }`, which reads as
"start when Start is pressed, stop then". Setting only an end date therefore hid
the control for a schedule that was really there — and its own docblock already
said "before a date is set", not "before a start date is set".
Either date now counts. `clearSchedule` needed nothing: it always dropped both.
Not a dead end before this — each picker carries `showClear`, so the end date
could still be cleared on its own. What was missing was the affordance that drops
the schedule as one choice, in a state where there was a schedule to drop.
Reported by Copilot on the PR.
oidacra
added
PR: docker image
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.
Summary
Creation becomes a routed screen, not a dialog:
/experiments/newrenders the Configure screen with an empty draft, and/experiments/:experimentId/configurationis the same component in edit mode. Autosave is the creation mechanism — one POST as soon as name + page exist, then areplaceUrlswap to/:id/configuration; every later change is a debounced PATCH per field group. Functional parity with the legacy UVE configuration screen, with the approved design and 100% Angular Signal Forms (no ReactiveForms in new code).Closes #37003
CleanShot.2026-08-18.at.16.21.05.mp4
What's included
dot-experiments-configure.store.ts): Signal Store Events pattern (same shape as the list store), debouncedswitchMapautosave per field group, one-shot creation guard,?pageId=/?url=prefill, page-lock awareness, the 8 start-gated validation rules, transitions (start/schedule/stop/cancelSchedule/abort) and non-DRAFT locking (SCHEDULED included, matching the legacy screen).p-datepicker [showTime]+ min/max duration bounds fromDotExperimentsConfigResolver).Variant {n}fallback).UrlMatcherroute config serves both paths so the post-creationreplaceUrlswap reuses the component instance (no autosave loss); theexperimentsmount dropsreuseRoute: false(mirrors the/contentprecedent).DotPagesBrowserService(page search, folder tree, page-lock lookup) +DotExperimentsService.setName.Acceptance criteria
All 10 issue ACs verified by an AC-coverage pass (48 refined criteria, all covered) — routing/creation, debounced autosave with
targetingConditionsomitted from every payload, prefill with inline error, the five cards with ported behaviors, Select A Page grey-out, reveal-on-Start validation with scroll + footer count, transitions with toasts, kebab perAllowedActionsByExperimentStatus(no enterprise gates), old UVE screen untouched, Jest specs per card + autosave store with fake timers.Test plan
nx lintclean on the three touched projects;nx build dotcms-ui(production AOT) passes.nx affectedalso pulls inblock-editor/dotcms-block-editor, which fail lint/tests on the baseline without any change from this branch (zero files touched there; additive-only data-access exports) — pre-existing failures, not introduced here.Notes and follow-ups
PATCH /api/v1/experiments/{id}does not acceptpageId— the page is set at creation only; the UI disables Select afterwards with a tooltip (legacy parity: the page was never changeable).ExperimentForm.BuilderresetstargetingConditionsto[]on every PATCH regardless of the client payload — belongs to the Experiments API gaps for the standalone portlet: archive/restore, PATCH side effects, goal operators, variant cap #36988 scope.targetingConditionsin payloads) is enforced by spec.