Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nimbus): Approval and rejection flow #12090

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
2ab1754
feat(nimbus): Summary page actions
yashikakhurana Nov 21, 2024
611c393
feat(nimbus): Cancel review flow
yashikakhurana Dec 11, 2024
cf977c6
feat(nimbus): Cancel review flow
yashikakhurana Dec 11, 2024
e1260b9
feat(nimbus): Cancel review flow
yashikakhurana Dec 11, 2024
9696f4a
feat(nimbus): Update urls
yashikakhurana Dec 11, 2024
0c770c3
test(nimbus): Test cases
yashikakhurana Dec 11, 2024
eefc8e1
test(nimbus): Test cases
yashikakhurana Dec 11, 2024
9d4eea5
Merge branch 'main' into 11757/draft_preview_flow
yashikakhurana Dec 11, 2024
0064d56
test(nimbus): Test cases
yashikakhurana Dec 11, 2024
47137d1
test(nimbus): Test cases
yashikakhurana Dec 11, 2024
84d22e5
test(nimbus): Test cases
yashikakhurana Dec 12, 2024
aa6e85d
test(nimbus): Test cases
yashikakhurana Dec 12, 2024
1c9e951
test(nimbus): Test cases
yashikakhurana Dec 12, 2024
6067e0b
Merge branch 'main' into 11757/draft_preview_flow
yashikakhurana Dec 12, 2024
fab606b
Some ideas from jared
jaredlockhart Dec 12, 2024
c98ad23
feat(nimbus): Launch control flow
yashikakhurana Dec 24, 2024
7cb83f2
Merge branch 'main' into 11757/draft_preview_flow
yashikakhurana Dec 24, 2024
30cd3f1
feat(nimbus): Launch control flow
yashikakhurana Dec 24, 2024
5fa331d
feat(nimbus): Launch control flow
yashikakhurana Dec 24, 2024
8e26539
feat(nimbus): Launch control flow
yashikakhurana Dec 24, 2024
b4a0153
feat(nimbus): Launch control flow
yashikakhurana Dec 24, 2024
c83735d
feat(nimbus): format
yashikakhurana Dec 24, 2024
9dcf31b
feat(nimbus)
yashikakhurana Jan 14, 2025
58f0c26
Merge branch 'main' into 11757/draft_preview_flow
yashikakhurana Jan 14, 2025
7b6eb7c
feat(nimbus): fix formating
yashikakhurana Jan 14, 2025
2e6f4ae
Merge branch 'main' into 11757/draft_preview_flow
yashikakhurana Jan 16, 2025
598a79c
feat(nimbus): Approval rejection flow
yashikakhurana Jan 22, 2025
3819ef9
feat(nimbus): Test approve and reject
yashikakhurana Jan 22, 2025
783d53b
feat(nimbus): Test approve and reject
yashikakhurana Jan 22, 2025
842d00d
Merge branch 'main' into 12072/approval_rejection
yashikakhurana Jan 28, 2025
cdb149d
feat(nimbus): Update status
yashikakhurana Jan 29, 2025
b98fa23
feat(nimbus): Sync remote settings
yashikakhurana Feb 3, 2025
8c81c43
Merge branch 'main' into 12072/approval_rejection
yashikakhurana Feb 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions experimenter/experimenter/experiments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
39 changes: 39 additions & 0 deletions experimenter/experimenter/nimbus_ui_new/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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}"
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
{% load nimbus_extras %}

<div id="launch-controls">
<form>
{% csrf_token %}
Expand Down Expand Up @@ -116,6 +118,45 @@
</div>
<!-- Review Mode Controls -->
{% elif experiment.is_review %}
{% if experiment.can_show_timeout_message %}
<div class="alert alert-danger" role="alert">
<p>
<span role="img" aria-label="red X emoji">❌</span>
Remote Settings request has timed out, please go through the approval flow to launch this experiment again.
</p>
</div>
{% endif %}
<div id="review-controls" class="alert alert-primary">
<p>
<strong>{{ experiment.changes.latest_review_request.changed_by.email }}</strong> requested to Launch this experiment.
</p>
<button type="button"
class="btn btn-success"
hx-post="{% url 'nimbus-new-review-to-approve' slug=experiment.slug %}"
hx-select="#content"
hx-target="#content"
hx-swap="outerHTML">Approve and Launch Experiment</button>
<button type="button" class="btn btn-danger" id="reject-button">Reject</button>
</div>
<!-- Rejection Form -->
<div id="reject-form-container" class="d-none alert alert-warning">
<label for="changelog_message">
<strong>You are rejecting the request to launch this experiment.</strong> Please add some comments:
</label>
<textarea id="changelog_message"
name="changelog_message"
class="form-control"
rows="4"
required></textarea>
<button type="submit"
class="btn btn-danger mt-2"
hx-post="{% url 'nimbus-new-review-to-reject' slug=experiment.slug %}"
hx-select="#content"
hx-target="#content"
hx-swap="outerHTML"
hx-include="#changelog_message">Submit</button>
<button type="button" class="btn btn-secondary mt-2" id="cancel-reject">Cancel</button>
</div>
<div class="alert alert-warning">
<p>The experiment is currently under review. If you wish to cancel the review, click the button below:</p>
<button type="button"
Expand All @@ -125,6 +166,16 @@
hx-target="#content"
hx-swap="outerHTML">Cancel Review</button>
</div>
{% elif experiment|can_show_remote_settings_pending:user %}
<div class="alert alert-primary">
<p>
<strong>Action required:</strong> Please review this change in Remote Settings to launch this experiment.
</p>
<a href="{{ experiment.review_url }}"
class="btn btn-primary"
target="_blank"
rel="noopener noreferrer">Open Remote Settings</a>
</div>
{% endif %}
</form>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,8 @@ def format_json(value):
f'<pre class="text-monospace" style="white-space: pre-wrap; '
f'word-wrap: break-word;">{parsed_json}</pre>'
)


@register.filter
def can_show_remote_settings_pending(experiment, reviewer):
return experiment.can_show_remote_settings_pending(reviewer)
73 changes: 73 additions & 0 deletions experimenter/experimenter/nimbus_ui_new/tests/test_forms.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest.mock import patch

from django.test import RequestFactory, TestCase
from django.urls import reverse

Expand All @@ -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,
Expand All @@ -28,7 +34,9 @@
PreviewToDraftForm,
PreviewToReviewForm,
QAStatusForm,
ReviewToApproveForm,
ReviewToDraftForm,
ReviewToRejectForm,
SignoffForm,
SubscribeForm,
TakeawaysForm,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
)
Expand All @@ -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)

Expand All @@ -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
Expand All @@ -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):
Expand Down
Loading