feat(nimbus): stricter status transitions (#14266)

Becuase

* With more experiments comes more collisions with multiple users
pressing buttons quickly
* We do not have strict requirements for each status transition form
* This means things can be approved multiple times by multiple users
concurrently
* This can leave experiments in broken states if users collide
* We should add strict conditions to every state transition form so a
state transition can only occur once

This commit

* Adds required entry states for every status transition form based on
the states defined in the diagrams
* Updates the UI to show more detailed information if a status collision
occurs
* Updates tests

fixes #14227

Author: jaredlockhart

experimenter/experimenter/nimbus_ui/constants.py

@@ -22,6 +22,10 @@ class NimbusUIConstants:
         "Name maps to a pre-existing slug, please choose another name."
     )
     ERROR_TAG_DUPLICATE_NAME = "Tag with this Name already exists."
+    ERROR_INVALID_STATE_TRANSITION = (
+        "Cannot perform this action: experiment must be in state {required_state}, "
+        "but is currently in state {current_state}."
+    )
 
     RISK_MESSAGE_URL = "https://mozilla-hub.atlassian.net/wiki/spaces/FIREFOX/pages/208308555/Message+Consult+Creation"
     REVIEW_URL = "https://experimenter.info/access"

experimenter/experimenter/nimbus_ui/forms.py

@@ -1366,10 +1366,51 @@ class UpdateStatusForm(NimbusChangeLogFormMixin, forms.ModelForm):
     publish_status = None
     is_paused = None
 
+    required_status = None
+    required_status_next = None
+    required_publish_status = None
+    required_is_paused = None
+
     class Meta:
         model = NimbusExperiment
         fields = []
 
+    def clean(self):
+        cleaned_data = super().clean()
+
+        required_state = (
+            self.required_status,
+            self.required_status_next,
+            self.required_publish_status,
+            self.required_is_paused,
+        )
+        current_state = (
+            self.instance.status,
+            self.instance.status_next,
+            self.instance.publish_status,
+            self.instance.is_paused,
+        )
+
+        state_mismatch = (
+            self.required_status != self.instance.status
+            or self.required_status_next != self.instance.status_next
+            or self.required_publish_status != self.instance.publish_status
+            or (
+                self.required_is_paused is not None
+                and self.required_is_paused != self.instance.is_paused
+            )
+        )
+
+        if state_mismatch:
+            raise forms.ValidationError(
+                NimbusUIConstants.ERROR_INVALID_STATE_TRANSITION.format(
+                    required_state=required_state,
+                    current_state=current_state,
+                )
+            )
+
+        return cleaned_data
+
     @transaction.atomic
     def save(self, commit=True):
         self.instance.status = self.status
@@ -1401,9 +1442,15 @@ def save(self, commit=True):
 
 
 class DraftToPreviewForm(UpdateStatusForm):
+    required_status = NimbusExperiment.Status.DRAFT
+    required_status_next = None
+    required_publish_status = NimbusExperiment.PublishStatus.IDLE
+    required_is_paused = False
+
     status = NimbusExperiment.Status.PREVIEW
-    status_next = NimbusExperiment.Status.PREVIEW
+    status_next = None
     publish_status = NimbusExperiment.PublishStatus.IDLE
+    is_paused = False
 
     def get_changelog_message(self):
         return f"{self.request.user} launched experiment to Preview"
@@ -1418,29 +1465,49 @@ def save(self, commit=True):
 
 
 class DraftToReviewForm(SlackNotificationMixin, UpdateStatusForm):
+    required_status = NimbusExperiment.Status.DRAFT
+    required_status_next = None
+    required_publish_status = NimbusExperiment.PublishStatus.IDLE
+    required_is_paused = False
+
     status = NimbusExperiment.Status.DRAFT
     status_next = NimbusExperiment.Status.LIVE
     publish_status = NimbusExperiment.PublishStatus.REVIEW
+    is_paused = False
+
     slack_action = NimbusConstants.SLACK_ACTION_LAUNCH_REQUEST
 
     def get_changelog_message(self):
         return f"{self.request.user} requested launch without Preview"
 
 
 class PreviewToReviewForm(SlackNotificationMixin, UpdateStatusForm):
+    required_status = NimbusExperiment.Status.PREVIEW
+    required_status_next = None
+    required_publish_status = NimbusExperiment.PublishStatus.IDLE
+    required_is_paused = False
+
     status = NimbusExperiment.Status.DRAFT
     status_next = NimbusExperiment.Status.LIVE
     publish_status = NimbusExperiment.PublishStatus.REVIEW
+    is_paused = False
+
     slack_action = NimbusConstants.SLACK_ACTION_LAUNCH_REQUEST
 
     def get_changelog_message(self):
         return f"{self.request.user} requested launch from Preview"
 
 
 class PreviewToDraftForm(UpdateStatusForm):
+    required_status = NimbusExperiment.Status.PREVIEW
+    required_status_next = None
+    required_publish_status = NimbusExperiment.PublishStatus.IDLE
+    required_is_paused = False
+
     status = NimbusExperiment.Status.DRAFT
-    status_next = NimbusExperiment.Status.DRAFT
+    status_next = None
     publish_status = NimbusExperiment.PublishStatus.IDLE
+    is_paused = False
 
     def get_changelog_message(self):
         return f"{self.request.user} moved the experiment back to Draft"
@@ -1453,9 +1520,16 @@ def save(self, commit=True):
 
 
 class ReviewToDraftForm(UpdateStatusForm):
+    required_status = NimbusExperiment.Status.DRAFT
+    required_status_next = NimbusExperiment.Status.LIVE
+    required_publish_status = NimbusExperiment.PublishStatus.REVIEW
+    required_is_paused = False
+
     status = NimbusExperiment.Status.DRAFT
-    status_next = NimbusExperiment.Status.DRAFT
+    status_next = None
     publish_status = NimbusExperiment.PublishStatus.IDLE
+    is_paused = False
+
     changelog_message = forms.CharField(
         required=False, label="Changelog Message", max_length=1000
     )
@@ -1474,9 +1548,15 @@ def get_changelog_message(self):
 
 
 class ReviewToApproveForm(UpdateStatusForm):
+    required_status = NimbusExperiment.Status.DRAFT
+    required_status_next = NimbusExperiment.Status.LIVE
+    required_publish_status = NimbusExperiment.PublishStatus.REVIEW
+    required_is_paused = False
+
     status = NimbusExperiment.Status.DRAFT
     status_next = NimbusExperiment.Status.LIVE
     publish_status = NimbusExperiment.PublishStatus.APPROVED
+    is_paused = False
 
     def get_changelog_message(self):
         return f"{self.request.user} approved the review."
@@ -1493,46 +1573,43 @@ def save(self, commit=True):
 
 
 class LiveToEndEnrollmentForm(SlackNotificationMixin, UpdateStatusForm):
+    required_status = NimbusExperiment.Status.LIVE
+    required_status_next = None
+    required_publish_status = NimbusExperiment.PublishStatus.IDLE
+    required_is_paused = False
+
     status = NimbusExperiment.Status.LIVE
     status_next = NimbusExperiment.Status.LIVE
     publish_status = NimbusExperiment.PublishStatus.REVIEW
     is_paused = True
+
     slack_action = NimbusConstants.SLACK_ACTION_END_ENROLLMENT_REQUEST
 
     def clean(self):
+        cleaned_data = super().clean()
+
         if self.instance and self.instance.is_rollout_dirty:
             raise forms.ValidationError(NimbusExperiment.ERROR_CANNOT_PAUSE_UNPUBLISHED)
 
-        if not self.instance.should_show_end_enrollment:
-            if (
-                self.instance.is_draft
-                or self.instance.is_preview
-                or self.instance.is_complete
-            ):
-                raise forms.ValidationError(NimbusExperiment.ERROR_CANNOT_PAUSE_NOT_LIVE)
-
-            if not self.instance.is_enrolling:
-                raise forms.ValidationError(NimbusExperiment.ERROR_CANNOT_PAUSE_PAUSED)
+        if self.instance.is_rollout and not self.instance.is_firefox_labs_opt_in:
+            raise forms.ValidationError(NimbusExperiment.ERROR_CANNOT_PAUSE_ROLLOUT)
 
-            if self.instance.is_rollout and not self.instance.is_firefox_labs_opt_in:
-                raise forms.ValidationError(NimbusExperiment.ERROR_CANNOT_PAUSE_ROLLOUT)
-
-            # The conditions for Experiment.should_show_enrollment have changed
-            # but this function has become out of sync.
-            raise forms.ValidationError(
-                NimbusExperiment.ERROR_CANNOT_PAUSE_INVALID
-            )  # pragma: no cover
-
-        return super().clean()
+        return cleaned_data
 
     def get_changelog_message(self):
         return f"{self.request.user} requested review to end enrollment"
 
 
 class ApproveEndEnrollmentForm(UpdateStatusForm):
+    required_status = NimbusExperiment.Status.LIVE
+    required_status_next = NimbusExperiment.Status.LIVE
+    required_publish_status = NimbusExperiment.PublishStatus.REVIEW
+    required_is_paused = True
+
     status = NimbusExperiment.Status.LIVE
     status_next = NimbusExperiment.Status.LIVE
     publish_status = NimbusExperiment.PublishStatus.APPROVED
+    is_paused = True
 
     def get_changelog_message(self):
         return f"{self.request.user} approved the end enrollment request"
@@ -1547,19 +1624,32 @@ def save(self, commit=True):
 
 
 class LiveToCompleteForm(SlackNotificationMixin, UpdateStatusForm):
+    required_status = NimbusExperiment.Status.LIVE
+    required_status_next = None
+    required_publish_status = NimbusExperiment.PublishStatus.IDLE
+    required_is_paused = False
+
     status = NimbusExperiment.Status.LIVE
     status_next = NimbusExperiment.Status.COMPLETE
     publish_status = NimbusExperiment.PublishStatus.REVIEW
+    is_paused = False
+
     slack_action = NimbusConstants.SLACK_ACTION_END_EXPERIMENT_REQUEST
 
     def get_changelog_message(self):
         return f"{self.request.user} requested review to end experiment"
 
 
 class ApproveEndExperimentForm(UpdateStatusForm):
+    required_status = NimbusExperiment.Status.LIVE
+    required_status_next = NimbusExperiment.Status.COMPLETE
+    required_publish_status = NimbusExperiment.PublishStatus.REVIEW
+    required_is_paused = None
+
     status = NimbusExperiment.Status.LIVE
     status_next = NimbusExperiment.Status.COMPLETE
     publish_status = NimbusExperiment.PublishStatus.APPROVED
+    is_paused = True
 
     def get_changelog_message(self):
         return f"{self.request.user} approved the end experiment request"
@@ -1574,10 +1664,16 @@ def save(self, commit=True):
 
 
 class CancelEndEnrollmentForm(UpdateStatusForm):
+    required_status = NimbusExperiment.Status.LIVE
+    required_status_next = NimbusExperiment.Status.LIVE
+    required_publish_status = NimbusExperiment.PublishStatus.REVIEW
+    required_is_paused = True
+
     status = NimbusExperiment.Status.LIVE
     status_next = None
     publish_status = NimbusExperiment.PublishStatus.IDLE
     is_paused = False
+
     changelog_message = forms.CharField(
         required=False, label="Changelog Message", max_length=1000
     )
@@ -1596,9 +1692,15 @@ def get_changelog_message(self):
 
 
 class CancelEndExperimentForm(UpdateStatusForm):
+    required_status = NimbusExperiment.Status.LIVE
+    required_status_next = NimbusExperiment.Status.COMPLETE
+    required_publish_status = NimbusExperiment.PublishStatus.REVIEW
+    required_is_paused = None
+
     status = NimbusExperiment.Status.LIVE
     status_next = None
     publish_status = NimbusExperiment.PublishStatus.IDLE
+
     changelog_message = forms.CharField(
         required=False, label="Changelog Message", max_length=1000
     )
@@ -1621,19 +1723,33 @@ def get_changelog_message(self):
 
 
 class LiveToUpdateRolloutForm(SlackNotificationMixin, UpdateStatusForm):
+    required_status = NimbusExperiment.Status.LIVE
+    required_status_next = None
+    required_publish_status = NimbusExperiment.PublishStatus.IDLE
+    required_is_paused = False
+
     status = NimbusExperiment.Status.LIVE
     status_next = NimbusExperiment.Status.LIVE
     publish_status = NimbusExperiment.PublishStatus.REVIEW
+    is_paused = False
+
     slack_action = NimbusConstants.SLACK_ACTION_UPDATE_REQUEST
 
     def get_changelog_message(self):
         return f"{self.request.user} requested review to update Audience"
 
 
 class CancelUpdateRolloutForm(UpdateStatusForm):
+    required_status = NimbusExperiment.Status.LIVE
+    required_status_next = NimbusExperiment.Status.LIVE
+    required_publish_status = NimbusExperiment.PublishStatus.REVIEW
+    required_is_paused = False
+
     status = NimbusExperiment.Status.LIVE
     status_next = None
     publish_status = NimbusExperiment.PublishStatus.IDLE
+    is_paused = False
+
     changelog_message = forms.CharField(
         required=False, label="Changelog Message", max_length=1000
     )
@@ -1652,9 +1768,15 @@ def get_changelog_message(self):
 
 
 class ApproveUpdateRolloutForm(UpdateStatusForm):
+    required_status = NimbusExperiment.Status.LIVE
+    required_status_next = NimbusExperiment.Status.LIVE
+    required_publish_status = NimbusExperiment.PublishStatus.REVIEW
+    required_is_paused = False
+
     status = NimbusExperiment.Status.LIVE
     status_next = NimbusExperiment.Status.LIVE
     publish_status = NimbusExperiment.PublishStatus.APPROVED
+    is_paused = False
 
     def get_changelog_message(self):
         return f"{self.request.user} approved the update review request"

experimenter/experimenter/nimbus_ui/templates/nimbus_experiments/launch_controls.html

@@ -29,11 +29,16 @@
   {% endif %}
   {% if update_status_form_errors %}
     <div class="alert alert-danger" role="alert">
-      <p class="mb-1">Could not request an update for this experiment:</p>
-      {{ update_status_form_errors }}
-      <p class="mb-0">
-        Please fix the above issues or ask in <code>#ask-experimenter</code>.
+      <p class="mb-2">
+        That action could not be completed because the experiment has changed.
+        <a class="text-decoration-none"
+           data-bs-toggle="collapse"
+           href="#error-details"
+           role="button"
+           aria-expanded="false"
+           aria-controls="error-details">See more <i class="fa-solid fa-chevron-down"></i></a>
       </p>
+      <div class="collapse mt-2" id="error-details">{{ update_status_form_errors }}</div>
     </div>
   {% endif %}
   {% with rejection=experiment.rejection_block %}