feat(experiments): allow changing the page of a draft experiment (#37176)

[oidacra]

Backend for #37176 — epic #36763 (Experiments: A/B Testing v2).

The problem

The page an experiment tests was frozen the moment the draft was created. A submitted pageId was
accepted by the request contract and then discarded, so an editor who picked the wrong page had one
remedy: delete the experiment and rebuild it — name, description, goals and traffic split included.

The restriction was broader than the data required. The real constraint is variants: creating one
copies the page's layout into it, so the copy only means anything for the page it came from. The
control is exempt — it owns no duplicated layout, it is the page. So the swap is safe in exactly
one case: still a draft, and the control is the only variant.

What changed

File
ExperimentsAPIImpl	The eligibility rule, and regeneration of the control variant's url from the new page
ExperimentsFactoryImpl	page_id added to UPDATE_EXPERIMENT
ExperimentsResource	Carries the submitted pageId across

A pageId equal to the stored one stays a no-op whatever the status or variant count. Anything else
that does not qualify is refused with HTTP 400 naming the rule that blocked it, rather than being
dropped in silence — that silent drop was the more dangerous half of the defect, because the caller
was told its change had succeeded when it had not.

The root cause was not where the issue said it was

The issue attributed the bug to patchExperiment never reading pageId. True, but only half of it.
UPDATE_EXPERIMENT did not include page_id at all — the column appeared only in the INSERT, so the
page was immutable at the persistence layer. Implemented exactly as written, the fix would not
have worked.

It surfaced as a split result: with the resource and API changes in place, shouldChangeThePage still
returned the old page while shouldRegenerateControlVariantUrl passed. traffic_proportion is in the
UPDATE statement and page_id was not, so the new control url persisted and the new page did not.

Worth a careful look in review: this writes a column that the UPDATE never wrote before. It is
safe — every existing path writes back the value it already had — but it deserves a second pair of
eyes.

Review feedback

• @freddyDOTCMS — "move these validations to the API level": done, and it was more than a layering
  preference. The rule and the url regeneration were split across two layers, leaving save()
  half-guarded: a caller reaching experimentsAPI.save() directly with a changed pageId on a
  non-draft experiment could move page_id while leaving non-control variants' urls on the old page.
  The rule now sits next to the regeneration it protects. Only a changed page reaches the check, so
  addVariant, deleteVariant, promoteVariant, start and end pay nothing.

Tests

8 integration tests in a new ExperimentsResourceIntegrationTest, covering every value of the status
enum — DRAFT with only the control, DRAFT with a real variant, RUNNING, SCHEDULED, ENDED, ARCHIVED.

Two of them are not in the issue's list and were added deliberately:

• the unchanged-pageId no-op on ineligible experiments — the compatibility guarantee that keeps
  clients echoing the whole experiment back on every save working, and the reason the equality check
  must run before the eligibility check rather than after it;
• SCHEDULED / ENDED / ARCHIVED — RUNNING was the only non-DRAFT status covered, leaving three
  quarters of the status rule unguarded. SCHEDULED matters most: Allow changing the page of a SCHEDULED experiment that has not started #37214 proposes allowing it later,
  and this is what will make that change announce itself.

The class also had to be registered in MainSuite1a. The suites enumerate their classes explicitly,
so without that the 8 tests would have passed locally and never run in CI — a regression could
have merged with no signal.

Postman gains two requests asserting the 400 itself. Driving the resource directly bypasses the
JAX-RS exception mappers, so integration tests can assert the exception type but not the status code.

Not in this PR

The frontend half (FR-008–013, User Stories 1, 3, 4) lands in
#37135, whose branch already carries the files this
change would need. Until it ships the capability is not reachable by a user — the page picker
stays disabled by #37064's blanket rule — so this should not be demoed as "editors can now change the
page". It is verified by tests, not by clicking.

Also surfaced and filed separately rather than fixed here:

• Allow changing the page of a SCHEDULED experiment that has not started #37214 — allowing the page change on a SCHEDULED experiment
• Spike: verify the real Experiment state machine, what happens to the data, and what is validated per transition #37215 — a spike to establish the real experiment state machine, what happens to the data, and what
  is validated per transition
• the control variant is identified two different ways in ExperimentsAPIImpl (save() keys off the
  Original description, addVariant() off the DEFAULT id). This change uses the id, because that
  is what gates the layout copy. Unifying them would change behaviour for existing experiments.

Verification status

• Integration: 8/8 green locally, against the committed code.
• Postman: inconclusive locally — the collection aborted on an ECONNRESET in an earlier folder, so
  the two new requests never ran. No dotCMS-side exception at the abort point; not yet attributed.
  PR Test / Postman Tests - Experiment in CI is the cheap comparison.
• Draft on purpose until CI is green and the frontend half lands.

This PR fixes: #37176

This PR fixes: #37176

[claude]

Claude finished @oidacra's task in 1m 52s —— View job

---

Code Review

• Read the diff against origin/main
• Review Java changes (API, Factory, Resource)
• Review tests + Postman
• Post review findings

Reviewed the backend page-change logic across ExperimentsAPIImpl, ExperimentsFactoryImpl, and ExperimentsResource. The design is sound: the eligibility rule and control-URL regeneration now live together in save() so they hold for every caller, and the page_id column is now written by UPDATE_EXPERIMENT. I traced the guard ordering and confirmed the important properties below.

Verified correct:

• Permission on the new page — validatePermissionToEdit(experiment, user) (ExperimentsAPIImpl.java:183) runs against the incoming experiment, i.e. the new pageId, so a user cannot repoint an experiment onto a page they lack EDIT on. No permission bypass.
• New page existence — getHtmlPageAsset(experiment) (:177) rejects a missing/blank pageId with DotStateException before the change rule runs, so an empty-string pageId from the form is refused rather than persisted.
• No-op ordering — the !pageId.equals(...) gate (:214-215) runs before validatePageChange, so clients echoing back an unchanged pageId on a normal save never hit the DRAFT/variant checks. This matches the intent and is covered by the dedicated no-op test.
• Control-URL format — regenerateControlVariantUrl (:1075) builds getURI() + "?variantName=" + DEFAULT_VARIANT.name(), byte-identical to createExperimentVariant (:1113), and only rewrites the control variant.
• UPDATE parameter alignment — the added dc.addParam(experiment.pageId()) (ExperimentsFactoryImpl.java:166) is prepended to match the new leading page_id=?; the parameter order still lines up with the statement.

New Issues

• 🟡 Medium: ExperimentsAPIImpl.java:1071 — regenerateControlVariantUrl re-fetches the page via findContentletByIdentifierAnyLanguage(experiment.pageId(), false) even though save() already loaded htmlPageAsset for the same pageId at line 177. This is a second content lookup inside the transaction on every page change. It is not a bug — the 2-arg fetch here intentionally mirrors createExperimentVariant, and getURI() should be identical — so this is optional. If you consolidate, verify the URI is unaffected by the live/variant fetch difference between the two call sites before reusing the existing asset. Fix this →

No blocking issues. The page_id write in UPDATE_EXPERIMENT is the change most worth a second reviewer's eye (as the author notes), but it is safe: every pre-existing save path writes back the pageId it already read, since the resource only overrides pageId when the form field is non-null and the API no-ops on an unchanged value.

· issue-37176-draft-experiment-page-change

[oidacra]

Closing — #37180 is back open and is the PR to use.

This one only existed because renaming the branch orphaned #37180's head ref and GitHub closed it. Restoring the old ref made it reopenable, so the review discussion (8 comments across 3 threads from @erickgonzalez and @freddyDOTCMS) stays where it happened, along with the full history. Nothing is lost here: both PRs pointed at the same commits, and #37180 already carries them.

done