diff --git a/experimenter/experimenter/experiments/models.py b/experimenter/experimenter/experiments/models.py index b3713e5778..f0c96633bb 100644 --- a/experimenter/experimenter/experiments/models.py +++ b/experimenter/experimenter/experiments/models.py @@ -699,6 +699,28 @@ def can_preview_to_draft(self): def can_preview_to_review(self): return self.is_preview + def can_show_remote_settings_pending(self, reviewer): + + return self.publish_status in ( + self.PublishStatus.APPROVED, + self.PublishStatus.WAITING, + ) and self.can_review(reviewer) + + @property + def can_show_timeout_message(self): + return self.changes.latest_timeout() + + @property + def should_call_preview_task(self): + return self.status in { + NimbusExperiment.Status.PREVIEW, + NimbusExperiment.Status.DRAFT, + } + + @property + def should_call_push_task(self): + return self.publish_status == NimbusExperiment.PublishStatus.APPROVED + @property def draft_date(self): if change := self.changes.all().order_by("changed_on").first(): diff --git a/experimenter/experimenter/nimbus_ui_new/forms.py b/experimenter/experimenter/nimbus_ui_new/forms.py index 92498fb502..8f5e5cca9a 100644 --- a/experimenter/experimenter/nimbus_ui_new/forms.py +++ b/experimenter/experimenter/nimbus_ui_new/forms.py @@ -12,6 +12,10 @@ NimbusExperimentBranchThroughExcluded, NimbusExperimentBranchThroughRequired, ) +from experimenter.kinto.tasks import ( + nimbus_check_kinto_push_queue_by_collection, + nimbus_synchronize_preview_experiments_in_kinto, +) from experimenter.nimbus_ui_new.constants import NimbusUIConstants from experimenter.outcomes import Outcomes from experimenter.projects.models import Project @@ -535,6 +539,19 @@ def save(self, commit=True): experiment.status_next = self.status_next experiment.publish_status = self.publish_status experiment.save() + # Allocate bucket range if needed + if experiment.has_filter(NimbusExperiment.Filters.SHOULD_ALLOCATE_BUCKETS): + experiment.allocate_bucket_range() + + if experiment.should_call_preview_task: + nimbus_synchronize_preview_experiments_in_kinto.apply_async(countdown=5) + + if experiment.should_call_push_task: + collection = experiment.kinto_collection + nimbus_check_kinto_push_queue_by_collection.apply_async( + countdown=5, args=[collection] + ) + return experiment @@ -581,3 +598,25 @@ class ReviewToDraftForm(UpdateStatusForm): def get_changelog_message(self): return f"{self.request.user} cancelled the review" + + +class ReviewToApproveForm(UpdateStatusForm): + status = NimbusExperiment.Status.DRAFT + status_next = NimbusExperiment.Status.LIVE + publish_status = NimbusExperiment.PublishStatus.APPROVED + + def get_changelog_message(self): + return f"{self.request.user} approved the review." + + +class ReviewToRejectForm(UpdateStatusForm): + status = NimbusExperiment.Status.DRAFT + status_next = None + publish_status = NimbusExperiment.PublishStatus.IDLE + changelog_message = forms.CharField( + required=True, label="Reason for Rejection", max_length=1000 + ) + + def get_changelog_message(self): + changelog_message = self.cleaned_data.get("changelog_message", "") + return f"{self.request.user} rejected the review with reason: {changelog_message}" diff --git a/experimenter/experimenter/nimbus_ui_new/static/js/review_controls.js b/experimenter/experimenter/nimbus_ui_new/static/js/review_controls.js index 88faa90720..800312f3b9 100644 --- a/experimenter/experimenter/nimbus_ui_new/static/js/review_controls.js +++ b/experimenter/experimenter/nimbus_ui_new/static/js/review_controls.js @@ -12,3 +12,32 @@ window.toggleSubmitButton = function () { const submitButton = document.getElementById("request-launch-button"); submitButton.disabled = !(checkbox1.checked && checkbox2.checked); }; + +function initializeRejectApproveListeners() { + const rejectButton = document.getElementById("reject-button"); + const reviewControls = document.getElementById("review-controls"); + const rejectFormContainer = document.getElementById("reject-form-container"); + const cancelReject = document.getElementById("cancel-reject"); + + if (rejectButton) { + rejectButton.addEventListener("click", () => { + reviewControls.classList.add("d-none"); + rejectFormContainer.classList.remove("d-none"); + }); + } + + if (cancelReject) { + cancelReject.addEventListener("click", () => { + rejectFormContainer.classList.add("d-none"); + reviewControls.classList.remove("d-none"); + }); + } +} + +// Initialize listeners on initial load +document.addEventListener("DOMContentLoaded", initializeRejectApproveListeners); + +// Reinitialize listeners after HTMX content swaps +document.body.addEventListener("htmx:afterSwap", () => { + initializeRejectApproveListeners(); +}); diff --git a/experimenter/experimenter/nimbus_ui_new/templates/nimbus_experiments/launch_controls.html b/experimenter/experimenter/nimbus_ui_new/templates/nimbus_experiments/launch_controls.html index ab676d4b18..9caa085dc6 100644 --- a/experimenter/experimenter/nimbus_ui_new/templates/nimbus_experiments/launch_controls.html +++ b/experimenter/experimenter/nimbus_ui_new/templates/nimbus_experiments/launch_controls.html @@ -1,3 +1,5 @@ +{% load nimbus_extras %} +
{% csrf_token %} @@ -116,6 +118,45 @@
{% elif experiment.is_review %} + {% if experiment.can_show_timeout_message %} + + {% endif %} +
+

+ {{ experiment.changes.latest_review_request.changed_by.email }} requested to Launch this experiment. +

+ + +
+ +
+ + + + +

The experiment is currently under review. If you wish to cancel the review, click the button below:

+ {% elif experiment|can_show_remote_settings_pending:user %} +
+

+ Action required: Please review this change in Remote Settings to launch this experiment. +

+ Open Remote Settings +
{% endif %} diff --git a/experimenter/experimenter/nimbus_ui_new/templatetags/nimbus_extras.py b/experimenter/experimenter/nimbus_ui_new/templatetags/nimbus_extras.py index ef39d09aed..d4eb9270bf 100644 --- a/experimenter/experimenter/nimbus_ui_new/templatetags/nimbus_extras.py +++ b/experimenter/experimenter/nimbus_ui_new/templatetags/nimbus_extras.py @@ -81,3 +81,8 @@ def format_json(value): f'
{parsed_json}
' ) + + +@register.filter +def can_show_remote_settings_pending(experiment, reviewer): + return experiment.can_show_remote_settings_pending(reviewer) diff --git a/experimenter/experimenter/nimbus_ui_new/tests/test_forms.py b/experimenter/experimenter/nimbus_ui_new/tests/test_forms.py index 34b5ee338a..c2c1789323 100644 --- a/experimenter/experimenter/nimbus_ui_new/tests/test_forms.py +++ b/experimenter/experimenter/nimbus_ui_new/tests/test_forms.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + from django.test import RequestFactory, TestCase from django.urls import reverse @@ -15,6 +17,10 @@ NimbusDocumentationLinkFactory, NimbusExperimentFactory, ) +from experimenter.kinto.tasks import ( + nimbus_check_kinto_push_queue_by_collection, + nimbus_synchronize_preview_experiments_in_kinto, +) from experimenter.nimbus_ui_new.constants import NimbusUIConstants from experimenter.nimbus_ui_new.forms import ( AudienceForm, @@ -28,7 +34,9 @@ PreviewToDraftForm, PreviewToReviewForm, QAStatusForm, + ReviewToApproveForm, ReviewToDraftForm, + ReviewToRejectForm, SignoffForm, SubscribeForm, TakeawaysForm, @@ -337,6 +345,15 @@ def setUp(self): super().setUp() self.experiment = NimbusExperimentFactory.create() + self.mock_preview_task = patch.object( + nimbus_synchronize_preview_experiments_in_kinto, "apply_async" + ).start() + self.mock_push_task = patch.object( + nimbus_check_kinto_push_queue_by_collection, "apply_async" + ).start() + + self.addCleanup(patch.stopall) + def test_draft_to_preview_form(self): self.experiment.status = NimbusExperiment.Status.DRAFT self.experiment.status_next = None @@ -353,6 +370,7 @@ def test_draft_to_preview_form(self): changelog = experiment.changes.latest("changed_on") self.assertEqual(changelog.changed_by, self.user) self.assertIn("launched experiment to Preview", changelog.message) + self.mock_preview_task.assert_called_once_with(countdown=5) def test_draft_to_review_form(self): self.experiment.status = NimbusExperiment.Status.DRAFT @@ -376,6 +394,7 @@ def test_preview_to_review_form(self): self.experiment.status_next = NimbusExperiment.Status.PREVIEW self.experiment.publish_status = NimbusExperiment.PublishStatus.IDLE self.experiment.save() + form = PreviewToReviewForm( data={}, instance=self.experiment, request=self.request ) @@ -389,12 +408,14 @@ def test_preview_to_review_form(self): changelog = experiment.changes.latest("changed_on") self.assertEqual(changelog.changed_by, self.user) self.assertIn("requested launch from Preview", changelog.message) + self.mock_preview_task.assert_called_once_with(countdown=5) def test_preview_to_draft_form(self): self.experiment.status = NimbusExperiment.Status.PREVIEW self.experiment.status_next = NimbusExperiment.Status.PREVIEW self.experiment.publish_status = NimbusExperiment.PublishStatus.IDLE self.experiment.save() + form = PreviewToDraftForm(data={}, instance=self.experiment, request=self.request) self.assertTrue(form.is_valid(), form.errors) @@ -406,6 +427,7 @@ def test_preview_to_draft_form(self): changelog = experiment.changes.latest("changed_on") self.assertEqual(changelog.changed_by, self.user) self.assertIn("moved the experiment back to Draft", changelog.message) + self.mock_preview_task.assert_called_once_with(countdown=5) def test_review_to_draft_form(self): self.experiment.status = NimbusExperiment.Status.DRAFT @@ -425,6 +447,57 @@ def test_review_to_draft_form(self): self.assertEqual(changelog.changed_by, self.user) self.assertIn("cancelled the review", changelog.message) + def test_review_to_approve_form(self): + self.experiment.status = NimbusExperiment.Status.DRAFT + self.experiment.status_next = NimbusExperiment.Status.LIVE + self.experiment.publish_status = NimbusExperiment.PublishStatus.REVIEW + self.experiment.save() + + form = ReviewToApproveForm( + data={}, instance=self.experiment, request=self.request + ) + self.assertTrue(form.is_valid(), form.errors) + + experiment = form.save() + self.assertEqual(experiment.status, NimbusExperiment.Status.DRAFT) + self.assertEqual(experiment.status_next, NimbusExperiment.Status.LIVE) + self.assertEqual( + experiment.publish_status, NimbusExperiment.PublishStatus.APPROVED + ) + + changelog = experiment.changes.latest("changed_on") + self.assertEqual(changelog.changed_by, self.user) + self.assertIn(f"{self.user.email} approved the review.", changelog.message) + + self.mock_push_task.assert_called_once_with( + countdown=5, args=[experiment.kinto_collection] + ) + + def test_review_to_reject_form_with_reason(self): + self.experiment.status = NimbusExperiment.Status.DRAFT + self.experiment.status_next = NimbusExperiment.Status.LIVE + self.experiment.publish_status = NimbusExperiment.PublishStatus.REVIEW + self.experiment.save() + + form = ReviewToRejectForm( + data={"changelog_message": "Needs more work."}, + instance=self.experiment, + request=self.request, + ) + self.assertTrue(form.is_valid(), form.errors) + + experiment = form.save() + self.assertEqual(experiment.status, NimbusExperiment.Status.DRAFT) + self.assertEqual(experiment.status_next, None) + self.assertEqual(experiment.publish_status, NimbusExperiment.PublishStatus.IDLE) + + changelog = experiment.changes.latest("changed_on") + self.assertEqual(changelog.changed_by, self.user) + self.assertIn( + f"{self.user.email} rejected the review with reason: Needs more work.", + changelog.message, + ) + class TestOverviewForm(RequestFormTestCase): def test_valid_form_saves(self): diff --git a/experimenter/experimenter/nimbus_ui_new/tests/test_views.py b/experimenter/experimenter/nimbus_ui_new/tests/test_views.py index de7b2c73e5..5bcdd3adde 100644 --- a/experimenter/experimenter/nimbus_ui_new/tests/test_views.py +++ b/experimenter/experimenter/nimbus_ui_new/tests/test_views.py @@ -21,6 +21,10 @@ NimbusExperimentFactory, NimbusFeatureConfigFactory, ) +from experimenter.kinto.tasks import ( + nimbus_check_kinto_push_queue_by_collection, + nimbus_synchronize_preview_experiments_in_kinto, +) from experimenter.nimbus_ui_new.filtersets import SortChoices, TypeChoices from experimenter.nimbus_ui_new.forms import QAStatusForm, TakeawaysForm from experimenter.nimbus_ui_new.views import StatusChoices @@ -1293,6 +1297,15 @@ def setUp(self): super().setUp() self.experiment = NimbusExperimentFactory.create() + self.mock_preview_task = patch.object( + nimbus_synchronize_preview_experiments_in_kinto, "apply_async" + ).start() + self.mock_push_task = patch.object( + nimbus_check_kinto_push_queue_by_collection, "apply_async" + ).start() + + self.addCleanup(patch.stopall) + def test_draft_to_preview(self): self.experiment.status = NimbusExperiment.Status.DRAFT self.experiment.status_next = None @@ -1310,6 +1323,8 @@ def test_draft_to_preview(self): self.experiment.publish_status, NimbusExperiment.PublishStatus.IDLE ) + self.mock_preview_task.assert_called_once_with(countdown=5) + def test_draft_to_review(self): self.experiment.status = NimbusExperiment.Status.DRAFT self.experiment.status_next = None @@ -1346,6 +1361,8 @@ def test_preview_to_review(self): self.experiment.publish_status, NimbusExperiment.PublishStatus.REVIEW ) + self.mock_preview_task.assert_called_once_with(countdown=5) + def test_preview_to_draft(self): self.experiment.status = NimbusExperiment.Status.PREVIEW self.experiment.status_next = NimbusExperiment.Status.PREVIEW @@ -1363,6 +1380,8 @@ def test_preview_to_draft(self): self.experiment.publish_status, NimbusExperiment.PublishStatus.IDLE ) + self.mock_preview_task.assert_called_once_with(countdown=5) + def test_cancel_review(self): self.experiment.status = NimbusExperiment.Status.DRAFT self.experiment.status_next = NimbusExperiment.Status.LIVE @@ -1380,6 +1399,56 @@ def test_cancel_review(self): self.experiment.publish_status, NimbusExperiment.PublishStatus.IDLE ) + def test_review_to_approve_view(self): + self.experiment.status = NimbusExperiment.Status.DRAFT + self.experiment.status_next = NimbusExperiment.Status.LIVE + self.experiment.publish_status = NimbusExperiment.PublishStatus.REVIEW + self.experiment.save() + + response = self.client.post( + reverse("nimbus-new-review-to-approve", kwargs={"slug": self.experiment.slug}) + ) + self.assertEqual(response.status_code, 200) + self.experiment.refresh_from_db() + self.assertEqual(self.experiment.status, NimbusExperiment.Status.DRAFT) + self.assertEqual(self.experiment.status_next, NimbusExperiment.Status.LIVE) + self.assertEqual( + self.experiment.publish_status, NimbusExperiment.PublishStatus.APPROVED + ) + + changelog = self.experiment.changes.latest("changed_on") + self.assertEqual(changelog.changed_by, self.user) + self.assertIn(f"{self.user.email} approved the review.", changelog.message) + self.mock_push_task.assert_called_once_with( + countdown=5, args=[self.experiment.kinto_collection] + ) + + def test_review_to_reject_view_with_reason(self): + self.experiment.status = NimbusExperiment.Status.DRAFT + self.experiment.status_next = NimbusExperiment.Status.LIVE + self.experiment.publish_status = NimbusExperiment.PublishStatus.REVIEW + self.experiment.save() + + response = self.client.post( + reverse("nimbus-new-review-to-reject", kwargs={"slug": self.experiment.slug}), + data={"changelog_message": "Not ready for launch."}, + ) + + self.assertEqual(response.status_code, 200) + self.experiment.refresh_from_db() + self.assertEqual(self.experiment.status, NimbusExperiment.Status.DRAFT) + self.assertEqual(self.experiment.status_next, None) + self.assertEqual( + self.experiment.publish_status, NimbusExperiment.PublishStatus.IDLE + ) + + changelog = self.experiment.changes.latest("changed_on") + self.assertEqual(changelog.changed_by, self.user) + self.assertIn( + f"{self.user.email} rejected the review with reason: Not ready for launch.", + changelog.message, + ) + class TestAudienceUpdateView(AuthTestCase): def test_get_renders_page(self): diff --git a/experimenter/experimenter/nimbus_ui_new/urls.py b/experimenter/experimenter/nimbus_ui_new/urls.py index a12d730aec..4569edb9b4 100644 --- a/experimenter/experimenter/nimbus_ui_new/urls.py +++ b/experimenter/experimenter/nimbus_ui_new/urls.py @@ -15,7 +15,9 @@ PreviewToDraftView, PreviewToReviewView, QAStatusUpdateView, + ReviewToApproveView, ReviewToDraftView, + ReviewToRejectView, SignoffUpdateView, SubscribeView, TakeawaysUpdateView, @@ -118,4 +120,14 @@ ReviewToDraftView.as_view(), name="nimbus-new-review-to-draft", ), + re_path( + r"^(?P[\w-]+)/review-to-approve/$", + ReviewToApproveView.as_view(), + name="nimbus-new-review-to-approve", + ), + re_path( + r"^(?P[\w-]+)/review-to-reject/$", + ReviewToRejectView.as_view(), + name="nimbus-new-review-to-reject", + ), ] diff --git a/experimenter/experimenter/nimbus_ui_new/views.py b/experimenter/experimenter/nimbus_ui_new/views.py index e8f61475d4..d9043496f0 100644 --- a/experimenter/experimenter/nimbus_ui_new/views.py +++ b/experimenter/experimenter/nimbus_ui_new/views.py @@ -28,7 +28,9 @@ PreviewToDraftForm, PreviewToReviewForm, QAStatusForm, + ReviewToApproveForm, ReviewToDraftForm, + ReviewToRejectForm, SignoffForm, SubscribeForm, TakeawaysForm, @@ -303,3 +305,11 @@ class PreviewToReviewView(StatusUpdateView): class ReviewToDraftView(StatusUpdateView): form_class = ReviewToDraftForm + + +class ReviewToApproveView(StatusUpdateView): + form_class = ReviewToApproveForm + + +class ReviewToRejectView(StatusUpdateView): + form_class = ReviewToRejectForm