add decision point while paused, pendingActivation property added#3117
add decision point while paused, pendingActivation property added#3117danoswaltCL wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a pendingActivation flag for decision points so paused experiments can accept new/editable decision points without immediately serving them in assignment responses.
Changes:
- Adds
pendingActivationto backend model/DTO/frontend models and database migration. - Updates assignment and experiment state handling to suppress pending decision points and clear the flag when resuming/running.
- Updates frontend decision point restrictions/tooltips to allow paused-state edits only for pending decision points.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/frontend/projects/upgrade/src/assets/i18n/en.json |
Adds paused decision point tooltip copy. |
packages/frontend/.../experiment-decision-points-table.component.ts |
Adds per-row action disabled/tooltip logic based on pending activation. |
packages/frontend/.../experiment-decision-points-table.component.html |
Wires per-row tooltip and disabled state helpers. |
packages/frontend/.../experiment-decision-points-section-card.component.html |
Passes experiment state into the table. |
packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.selectors.ts |
Changes paused decision point card restriction behavior. |
packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.model.ts |
Adds pending activation fields to frontend experiment decision point DTOs. |
packages/backend/test/unit/services/ExperimentService.test.ts |
Adds tests for pending activation creation/state update behavior. |
packages/backend/test/unit/services/ExperimentAssignmentService.test.ts |
Adds tests for excluding pending decision points from assignments. |
packages/backend/test/unit/repositories/DecisionPointRepository.test.ts |
Adds repository tests for clearing pending activation. |
packages/backend/src/database/migrations/1778284800000-addPendingActivationToDecisionPoint.ts |
Adds and backfills the decision point column. |
packages/backend/src/api/services/ExperimentService.ts |
Sets new/imported decision points pending and clears pending on running transition. |
packages/backend/src/api/services/ExperimentAssignmentService.ts |
Filters pending decision points out of assignment responses. |
packages/backend/src/api/repositories/ExperimentRepository.ts |
Excludes pending decision points in context/decision point lookup queries. |
packages/backend/src/api/repositories/DecisionPointRepository.ts |
Adds bulk update helper for pending activation. |
packages/backend/src/api/models/DecisionPoint.ts |
Adds persisted pendingActivation column. |
packages/backend/src/api/DTO/ExperimentDTO.ts |
Adds DTO validation for optional pendingActivation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| decisionPoint.id = decisionPoint.id || crypto.randomUUID(); | ||
| return { ...decisionPoint, experiment: experimentDoc }; | ||
| return { ...decisionPoint, experiment: experimentDoc, pendingActivation: true }; | ||
| })) || |
There was a problem hiding this comment.
this is worth adding, i'm updating and doing some tests
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
|
||
| private getActivePartitions(experiment: Experiment): DecisionPoint[] { | ||
| return experiment.partitions.filter((dp) => !dp.pendingActivation); | ||
| } |
There was a problem hiding this comment.
Do we want to be adding new methods that use the old 'partitions' terminology?
There was a problem hiding this comment.
:) no, we don't
| } | ||
|
|
||
| private getActivePartitions(experiment: Experiment): DecisionPoint[] { | ||
| return experiment.partitions.filter((dp) => !dp.pendingActivation); |
There was a problem hiding this comment.
This is removing experiments from pools even when they are in preview mode and the user is a preview user, which is inconsistent with the test a few lines lower in mapDecisionPoints. That's why the preview user integration tests weren't passing. If this is fixed we can add them back in and support the feature.
| // creating decision point docs | ||
| // DPs are pending unless the experiment starts directly in an enrolling state | ||
| const internalCreationState = EXPERIMENT_STATE_INTERNAL_NAME_OVERRIDES[experiment.state] || experiment.state; | ||
| const startsPending = internalCreationState !== EXPERIMENT_STATE.ENROLLING; |
There was a problem hiding this comment.
If an experiment is created directly in COMPLETED ('cancelled'), pendingActivation will be true, which is inconsistent. Even though editing is disallowed in the frontend for 'cancelled' experiments, we may rely in the future on pendingActivation only being true when the dps are actually pending activation.
pendingActivationproperty to decision pointspendingActivationDPs tofalsewhen an experiment is set to RUNNINGpendingActivation=truependingActivation, allows adding decision point while paused.