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 34 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
34 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
b51b5eb
feat(nimbus): Change prefix
yashikakhurana Feb 22, 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
Prev Previous commit
Next Next commit
feat(nimbus): Test approve and reject
  • Loading branch information
yashikakhurana committed Jan 22, 2025
commit 3819ef93a8c061e82d290ea45d4fc76e211965c2
8 changes: 6 additions & 2 deletions experimenter/experimenter/nimbus_ui_new/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -594,8 +594,12 @@ def get_changelog_message(self):

class ReviewToRejectForm(UpdateStatusForm):
status = NimbusExperiment.Status.DRAFT
status_next = 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):
return f"{self.request.user} rejected the review."
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,4 +1,5 @@
{% load nimbus_extras %}

<div id="launch-controls">
<form>
{% csrf_token %}
Expand Down Expand Up @@ -117,37 +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"
hx-post="{% url 'nimbus-new-review-to-reject' slug=experiment.slug %}"
hx-select="#content"
hx-target="#content"
hx-swap="outerHTML">
Reject
</button>
</div>


{% 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 @@ -157,11 +166,13 @@
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">Open Remote Settings</a>
</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>
46 changes: 46 additions & 0 deletions experimenter/experimenter/nimbus_ui_new/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
PreviewToDraftForm,
PreviewToReviewForm,
QAStatusForm,
ReviewToApproveForm,
ReviewToDraftForm,
ReviewToRejectForm,
SignoffForm,
SubscribeForm,
TakeawaysForm,
Expand Down Expand Up @@ -413,6 +415,50 @@ 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.publish_status = NimbusExperiment.PublishStatus.REVIEW
self.experiment.status_next = NimbusExperiment.Status.LIVE
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)

def test_review_to_reject_form_with_reason(self):
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
47 changes: 47 additions & 0 deletions experimenter/experimenter/nimbus_ui_new/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1380,6 +1380,53 @@ 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)

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