Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ REDIS_PORT=
REDIS_DB=
OPENIDC_CLIENT_ID=
OPENIDC_CLIENT_SECRET=
EMAIL_RELEASE_DRIVERS=
EMAIL_REVIEW=
EMAIL_SHIP=
EMAIL_SENDER=
Expand Down
1 change: 1 addition & 0 deletions .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ REDIS_PORT=6379
REDIS_DB=0
OPENIDC_CLIENT_ID=
OPENIDC_CLIENT_SECRET=
EMAIL_RELEASE_DRIVERS=
EMAIL_REVIEW=
EMAIL_SHIP=
EMAIL_SENDER=
Expand Down
6 changes: 6 additions & 0 deletions app/experimenter/experiments/api_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
ExperimentDetailView,
ExperimentListView,
ExperimentRejectView,
ExperimentSendIntentToShipEmailView,
)


Expand All @@ -19,6 +20,11 @@
ExperimentRejectView.as_view(),
name="experiments-api-reject",
),
url(
r"^(?P<slug>[\w-]+)/intent-to-ship-email$",
ExperimentSendIntentToShipEmailView.as_view(),
name="experiments-api-send-intent-to-ship-email",
),
url(
r"^(?P<slug>[\w-]+)/$",
ExperimentDetailView.as_view(),
Expand Down
23 changes: 23 additions & 0 deletions app/experimenter/experiments/api_views.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from rest_framework.generics import ListAPIView, UpdateAPIView, RetrieveAPIView
from rest_framework.response import Response
from rest_framework import status

from experimenter.experiments.models import Experiment, ExperimentChangeLog
from experimenter.experiments.serializers import ExperimentSerializer
from experimenter.experiments import email


class ExperimentListView(ListAPIView):
Expand Down Expand Up @@ -60,3 +62,24 @@ def update(self, request, *args, **kwargs):
)

return Response()


class ExperimentSendIntentToShipEmailView(UpdateAPIView):
lookup_field = "slug"
queryset = Experiment.objects.filter(status=Experiment.STATUS_REVIEW)

def update(self, request, *args, **kwargs):
experiment = self.get_object()

if experiment.review_intent_to_ship:
return Response(
{"error": "email-already-sent"},
status=status.HTTP_409_CONFLICT,
)

email.send_intent_to_ship_email(experiment.id)
Comment thread
glasserc marked this conversation as resolved.

experiment.review_intent_to_ship = True
experiment.save()

return Response()
4 changes: 4 additions & 0 deletions app/experimenter/experiments/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,10 @@ class ExperimentConstants(object):
{attention}"""
)

INTENT_TO_SHIP_EMAIL_SUBJECT = (
"SHIELD Study Intent to ship: {name} {version} {channel}"
)

BUGZILLA_OVERVIEW_TEMPLATE = (
"""
{experiment.name}
Expand Down
37 changes: 37 additions & 0 deletions app/experimenter/experiments/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

from django.conf import settings
from django.core.mail import send_mail
from django.core.mail.message import EmailMessage
from django.template.loader import render_to_string

from experimenter.experiments.models import Experiment

Expand All @@ -22,3 +24,38 @@ def send_review_email(experiment_name, experiment_url, needs_attention):
[settings.EMAIL_REVIEW],
fail_silently=False,
)


def send_intent_to_ship_email(experiment_id):
experiment = Experiment.objects.get(id=experiment_id)

bug_url = settings.BUGZILLA_DETAIL_URL.format(id=experiment.bugzilla_id)

# Because that's how it's done in Experiment.population (property)
percent_of_population = f"{float(experiment.population_percent):g}%"

content = render_to_string(
"experiments/intent_to_ship.txt",
{
"experiment": experiment,
"bug_url": bug_url,
"percent_of_population": percent_of_population,
"locales": [str(l) for l in experiment.locales.all()],
"countries": [str(c) for c in experiment.countries.all()],
},
)
# Strip extra newlines from autoescape tag
content = content.strip() + "\n"

version = experiment.firefox_version
channel = experiment.firefox_channel
email = EmailMessage(
Experiment.INTENT_TO_SHIP_EMAIL_SUBJECT.format(
name=experiment.name, version=version, channel=channel
),
content,
settings.EMAIL_SENDER,
[settings.EMAIL_RELEASE_DRIVERS],
cc=[experiment.owner.email],
)
email.send(fail_silently=False)
2 changes: 2 additions & 0 deletions app/experimenter/experiments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ def get_queryset(self):
"owner",
"comments",
"comments__created_by",
"locales",
"countries",
)
.annotate(latest_change=Max("changes__changed_on"))
)
Expand Down
41 changes: 41 additions & 0 deletions app/experimenter/experiments/tests/test_api_views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json

from django.conf import settings
from django.core import mail
from django.test import TestCase
from django.urls import reverse

Expand Down Expand Up @@ -191,3 +192,43 @@ def test_post_to_reject_raises_404_for_non_pending_experiment(self):
)

self.assertEqual(response.status_code, 404)


class TestExperimentSendIntentToShipEmailView(TestCase):

def test_put_to_view_sends_email(self):
user_email = "user@example.com"

experiment = ExperimentFactory.create_with_variants(
review_intent_to_ship=False, status=Experiment.STATUS_REVIEW
)
old_outbox_len = len(mail.outbox)

response = self.client.put(
reverse(
"experiments-api-send-intent-to-ship-email",
kwargs={"slug": experiment.slug},
),
**{settings.OPENIDC_EMAIL_HEADER: user_email},
)

self.assertEqual(response.status_code, 200)

experiment = Experiment.objects.get(pk=experiment.pk)
self.assertEqual(experiment.review_intent_to_ship, True)
self.assertEqual(len(mail.outbox), old_outbox_len + 1)

def test_put_raises_409_if_email_already_sent(self):
experiment = ExperimentFactory.create_with_variants(
review_intent_to_ship=True, status=Experiment.STATUS_REVIEW
)

response = self.client.put(
reverse(
"experiments-api-send-intent-to-ship-email",
kwargs={"slug": experiment.slug},
),
**{settings.OPENIDC_EMAIL_HEADER: "user@example.com"},
)

self.assertEqual(response.status_code, 409)
141 changes: 140 additions & 1 deletion app/experimenter/experiments/tests/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
from django.conf import settings
from django.core import mail

from experimenter.experiments.email import send_review_email
from experimenter.experiments.email import (
send_review_email,
send_intent_to_ship_email,
)
from experimenter.experiments.tests.factories import ExperimentFactory


Expand Down Expand Up @@ -48,3 +51,139 @@ def test_send_review_email_with_needs_attention(self):
)
self.assertEqual(sent_email.from_email, settings.EMAIL_SENDER)
self.assertEqual(sent_email.recipients(), [settings.EMAIL_REVIEW])


class TestIntentToShipEmail(TestCase):
maxDiff = None

def test_send_intent_to_ship_email_with_risk_fields(self):
risk_description = "Hardcoded fictitious technical challenge"
experiment = ExperimentFactory.create(
name="Experiment",
slug="experiment",
risks="Hardcoded fictitious risk",
risk_technical_description=risk_description,
population_percent=10.0,
)
send_intent_to_ship_email(experiment.id)

sent_email = mail.outbox[-1]
self.verify_subject(experiment, sent_email)

bug_url = settings.BUGZILLA_DETAIL_URL.format(
id=experiment.bugzilla_id
)
expected_locales = self.format_locales(experiment)
expected_countries = self.format_countries(experiment)
expected_body = (
f"""
Hello Release Drivers,

This request is coming from information entered in Experimenter.
Please reach out to the person(s) on cc: with any questions, details,
or discussion. They will email an update if any of the key information
changes. Otherwise they will reach out once the study has fully passed
QA for Release Management sign-off.

Experimenter Bug: {bug_url}
Experimenter URL: {experiment.experiment_url}
Study owner: {experiment.owner.email}
Description: {experiment.short_description}
Timeline & Channel: {experiment.firefox_version} {experiment.firefox_channel}
Intended study dates: {experiment.dates}
Percent of Population: 10%
Platforms: {experiment.platform}
Locales: {expected_locales}; {expected_countries}
QA Status: {experiment.qa_status}
Meta Bug: {experiment.feature_bugzilla_url}
Related links: {experiment.related_work}
Risk: Hardcoded fictitious risk
Technical Complexity: Hardcoded fictitious technical challenge

Thank you!!
""".lstrip()
)

self.assertEqual(expected_body, sent_email.body)
self.assertEqual(sent_email.from_email, settings.EMAIL_SENDER)
self.assertEqual(
sent_email.recipients(),
[settings.EMAIL_RELEASE_DRIVERS, experiment.owner.email],
)

def test_send_intent_to_ship_email_without_risk_fields(self):
experiment = ExperimentFactory.create(
name="Experiment",
slug="experiment",
risks="",
risk_technical_description="",
population_percent=10.0,
)
send_intent_to_ship_email(experiment.id)

sent_email = mail.outbox[-1]
self.verify_subject(experiment, sent_email)

bug_url = settings.BUGZILLA_DETAIL_URL.format(
id=experiment.bugzilla_id
)
expected_locales = self.format_locales(experiment)
expected_countries = self.format_countries(experiment)
expected_body = (
f"""
Hello Release Drivers,

This request is coming from information entered in Experimenter.
Please reach out to the person(s) on cc: with any questions, details,
or discussion. They will email an update if any of the key information
changes. Otherwise they will reach out once the study has fully passed
QA for Release Management sign-off.

Experimenter Bug: {bug_url}
Experimenter URL: {experiment.experiment_url}
Study owner: {experiment.owner.email}
Description: {experiment.short_description}
Timeline & Channel: {experiment.firefox_version} {experiment.firefox_channel}
Intended study dates: {experiment.dates}
Percent of Population: 10%
Platforms: {experiment.platform}
Locales: {expected_locales}; {expected_countries}
QA Status: {experiment.qa_status}
Meta Bug: {experiment.feature_bugzilla_url}
Related links: {experiment.related_work}

Thank you!!
""".lstrip()
)

self.assertEqual(expected_body, sent_email.body)
self.assertEqual(sent_email.from_email, settings.EMAIL_SENDER)
self.assertEqual(
sent_email.recipients(),
[settings.EMAIL_RELEASE_DRIVERS, experiment.owner.email],
)

def format_locales(self, experiment):
locales = "All locales"
if experiment.locales.exists(): # pragma: no branch
locales = ", ".join(
str(locale) for locale in experiment.locales.all()
)
return locales

def format_countries(self, experiment):
countries = "All countries"
if experiment.countries.exists(): # pragma: no branch
countries = ", ".join(
str(country) for country in experiment.countries.all()
)
return countries

def verify_subject(self, experiment, email):
expected_subject = "".join(
[
"SHIELD Study Intent to ship: Experiment ",
f"{experiment.firefox_version} {experiment.firefox_channel}",
]
)
self.assertEqual(email.subject, expected_subject)
3 changes: 3 additions & 0 deletions app/experimenter/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,9 @@
# Email to send to when an experiment is ready to ship
EMAIL_SHIP = config("EMAIL_SHIP")

# Email to send to when an experiment is being signed-off
EMAIL_RELEASE_DRIVERS = config("EMAIL_RELEASE_DRIVERS")

# Bugzilla API Integration
BUGZILLA_HOST = config("BUGZILLA_HOST")
BUGZILLA_API_KEY = config("BUGZILLA_API_KEY")
Expand Down
1 change: 1 addition & 0 deletions app/experimenter/settings_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@
EMAIL_REVIEW = "testreview@example.com"
EMAIL_SHIP = "testship@example.com"
EMAIL_SENDER = "sender@example.com"
EMAIL_RELEASE_DRIVERS = "release.drivers@example.com"

USE_GOOGLE_ANALYTICS = True
25 changes: 25 additions & 0 deletions app/experimenter/static/js/detail-review.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Hook up intent-to-ship email
jQuery(function($) {
let sendUrl;
$("button.send-intent-to-ship").on("click", function(e) {
sendUrl = this.dataset.url;
// Let Bootstrap handle showing the modal itself
});

const modal = $("#send-intent-to-ship-modal");

modal.find("button.send").on("click", async function(e) {
this.innerHTML = "Sending...";
this.disabled = true;
const resp = await fetch(sendUrl, {
Comment thread
jaredlockhart marked this conversation as resolved.
method: "PUT",
});
if (resp.status == 200) {
// Rather than trying to update the DOM to match the new state,
// just refresh the page.
location.reload();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

} else {
modal.find(".sending-failed").toggleClass("d-none");
}
});
});
Loading