diff --git a/experimenter/experimenter/nimbus_ui/forms.py b/experimenter/experimenter/nimbus_ui/forms.py
index 5d904aeab1..ecef904c14 100644
--- a/experimenter/experimenter/nimbus_ui/forms.py
+++ b/experimenter/experimenter/nimbus_ui/forms.py
@@ -50,6 +50,10 @@ def get_changelog_message(self) -> str:
def save(self, *args, **kwargs):
experiment = super().save(*args, **kwargs)
+
+ if type(experiment) is NimbusBranchScreenshot:
+ experiment = self.instance.branch.experiment
+
generate_nimbus_changelog(
experiment, self.request.user, self.get_changelog_message()
)
@@ -1771,3 +1775,24 @@ def save(self, commit=True):
def get_changelog_message(self):
return f"{self.request.user} updated collaborators"
+
+
+class BranchLeadingScreenshotForm(NimbusChangeLogFormMixin, forms.ModelForm):
+ image = forms.ImageField(
+ required=False,
+ widget=forms.FileInput(attrs={"class": "form-control"}),
+ )
+ description = forms.CharField(
+ required=False,
+ widget=forms.TextInput(attrs={"class": "form-control"}),
+ )
+
+ class Meta:
+ model = NimbusBranchScreenshot
+ fields = ["image", "description"]
+
+ def get_changelog_message(self):
+ return (
+ f"{self.request.user} updated leading screenshot for "
+ f"{self.instance.branch.slug} branch"
+ )
diff --git a/experimenter/experimenter/nimbus_ui/templates/common/branch_card.html b/experimenter/experimenter/nimbus_ui/templates/common/branch_card.html
index e45ca9dd39..ece11fae18 100644
--- a/experimenter/experimenter/nimbus_ui/templates/common/branch_card.html
+++ b/experimenter/experimenter/nimbus_ui/templates/common/branch_card.html
@@ -1,17 +1,31 @@
{% load nimbus_extras %}
-
+
{% with shot=branch.screenshots.first %}
{% if shot and shot.image %}
-

+
+

+
+
{% else %}
-
No screenshot
+
{% endif %}
{% endwith %}
@@ -28,3 +42,63 @@
{{ branch.name }}
{{ branch.description|truncatechars:100 }}
+
diff --git a/experimenter/experimenter/nimbus_ui/templates/nimbus_experiments/results-new-inner.html b/experimenter/experimenter/nimbus_ui/templates/nimbus_experiments/results-new-inner.html
index 346d9aa8ca..b9bcb3737e 100644
--- a/experimenter/experimenter/nimbus_ui/templates/nimbus_experiments/results-new-inner.html
+++ b/experimenter/experimenter/nimbus_ui/templates/nimbus_experiments/results-new-inner.html
@@ -89,8 +89,12 @@ Hypothesis
{% for branch in branch_data %}
- {% include "common/branch_card.html" with branch=branch %}
+ {% for branch_slug, branch_leading_screenshot_form in branch_leading_screenshot_forms.items %}
+ {% if branch_slug == branch.slug %}
+ {% include "common/branch_card.html" with branch=branch experiment_slug=experiment.slug screenshot_form=branch_leading_screenshot_form %}
+ {% endif %}
+ {% endfor %}
{% endfor %}
diff --git a/experimenter/experimenter/nimbus_ui/tests/test_forms.py b/experimenter/experimenter/nimbus_ui/tests/test_forms.py
index ac9b5d9c77..25f10017c8 100644
--- a/experimenter/experimenter/nimbus_ui/tests/test_forms.py
+++ b/experimenter/experimenter/nimbus_ui/tests/test_forms.py
@@ -41,6 +41,7 @@
ApproveEndExperimentForm,
ApproveUpdateRolloutForm,
AudienceForm,
+ BranchLeadingScreenshotForm,
BranchScreenshotCreateForm,
BranchScreenshotDeleteForm,
CancelEndEnrollmentForm,
@@ -4411,3 +4412,49 @@ def test_feature_unsubscribe_form_removes_subscriber(self):
self.assertTrue(form.is_valid())
form.save()
self.assertNotIn(self.request.user, feature_config.subscribers.all())
+
+
+class ResultsEditBranchLeadingImageTests(RequestFormTestCase):
+ def test_edit_leading_image(self):
+ application = NimbusExperiment.Application.DESKTOP
+ experiment = NimbusExperimentFactory.create_with_lifecycle(
+ NimbusExperimentFactory.Lifecycles.CREATED,
+ application=application,
+ )
+ experiment.branches.all().delete()
+ experiment.changes.all().delete()
+
+ reference_branch = NimbusBranchFactory.create(experiment=experiment, ratio=1)
+ experiment.reference_branch = reference_branch
+ experiment.save()
+
+ reference_screenshot = reference_branch.screenshots.first()
+
+ image_bytes = io.BytesIO()
+ image = Image.new("RGB", (10, 10), color="red")
+ image.save(image_bytes, format="PNG")
+ image_bytes.seek(0)
+ dummy_image = SimpleUploadedFile(
+ "test.png", image_bytes.read(), content_type="image/png"
+ )
+
+ form = BranchLeadingScreenshotForm(
+ instance=reference_screenshot,
+ data={
+ "description": "Updated control screenshot",
+ },
+ files={
+ "image": dummy_image,
+ },
+ request=self.request,
+ )
+
+ form.save()
+ experiment.refresh_from_db()
+
+ self.assertEqual(
+ experiment.reference_branch.screenshots.get(
+ id=reference_screenshot.id
+ ).description,
+ "Updated control screenshot",
+ )
diff --git a/experimenter/experimenter/nimbus_ui/tests/test_views.py b/experimenter/experimenter/nimbus_ui/tests/test_views.py
index 89f74e18af..77036f22f9 100644
--- a/experimenter/experimenter/nimbus_ui/tests/test_views.py
+++ b/experimenter/experimenter/nimbus_ui/tests/test_views.py
@@ -1,11 +1,14 @@
import datetime
+import io
import json
from unittest.mock import patch
from django.conf import settings
+from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import TestCase
from django.urls import reverse
from parameterized import parameterized
+from PIL import Image
from experimenter.base.tests.factories import (
CountryFactory,
@@ -3028,6 +3031,62 @@ def test_get_redirects_to_next_step(self, current_url, next_url, data):
)
+@mock_valid_outcomes
+class TestResultsEditBranchImagesView(AuthTestCase):
+ def test_upload_updates_screenshot(self):
+ experiment = NimbusExperimentFactory.create(owner=self.user)
+ branch = NimbusBranchFactory.create(experiment=experiment, slug="t1")
+
+ url = reverse(
+ "nimbus-ui-branch-leading-screenshot-upload",
+ kwargs={"slug": experiment.slug, "branch_slug": branch.slug},
+ )
+
+ image_bytes = io.BytesIO()
+ image = Image.new("RGB", (10, 10), color="red")
+ image.save(image_bytes, format="PNG")
+ image_bytes.seek(0)
+ dummy_image = SimpleUploadedFile(
+ "test.png", image_bytes.read(), content_type="image/png"
+ )
+
+ response = self.client.post(url, {"image": dummy_image})
+
+ self.assertEqual(response.status_code, 200)
+
+ branch.refresh_from_db()
+ shot = branch.screenshots.first()
+ self.assertIsNotNone(shot)
+
+ def test_upload_creates_screenshot(self):
+ experiment = NimbusExperimentFactory.create(owner=self.user)
+ experiment.branches.all().delete()
+
+ branch = NimbusBranchFactory.create(experiment=experiment, slug="t1")
+ branch.screenshots.all().delete()
+
+ url = reverse(
+ "nimbus-ui-branch-leading-screenshot-upload",
+ kwargs={"slug": experiment.slug, "branch_slug": branch.slug},
+ )
+
+ image_bytes = io.BytesIO()
+ image = Image.new("RGB", (10, 10), color="red")
+ image.save(image_bytes, format="PNG")
+ image_bytes.seek(0)
+ dummy_image = SimpleUploadedFile(
+ "test.png", image_bytes.read(), content_type="image/png"
+ )
+
+ response = self.client.post(url, {"image": dummy_image})
+
+ self.assertEqual(response.status_code, 200)
+
+ branch.refresh_from_db()
+ shot = branch.screenshots.first()
+ self.assertIsNotNone(shot)
+
+
@mock_valid_outcomes
class TestResultsView(AuthTestCase):
def test_render_to_response(self):
diff --git a/experimenter/experimenter/nimbus_ui/urls.py b/experimenter/experimenter/nimbus_ui/urls.py
index a684147327..ae202b7cf5 100644
--- a/experimenter/experimenter/nimbus_ui/urls.py
+++ b/experimenter/experimenter/nimbus_ui/urls.py
@@ -10,6 +10,7 @@
BranchDeleteView,
BranchesPartialUpdateView,
BranchesUpdateView,
+ BranchLeadingScreenshotView,
BranchScreenshotCreateView,
BranchScreenshotDeleteView,
CancelEndEnrollmentView,
@@ -298,4 +299,9 @@
BranchScreenshotDeleteView.as_view(),
name="nimbus-ui-delete-branch-screenshot",
),
+ re_path(
+ r"^(?P[\w-]+)/branches/(?P[\w-]+)/leading-screenshot/",
+ BranchLeadingScreenshotView.as_view(),
+ name="nimbus-ui-branch-leading-screenshot-upload",
+ ),
]
diff --git a/experimenter/experimenter/nimbus_ui/views.py b/experimenter/experimenter/nimbus_ui/views.py
index 21e9bf0987..5a03644ee5 100644
--- a/experimenter/experimenter/nimbus_ui/views.py
+++ b/experimenter/experimenter/nimbus_ui/views.py
@@ -36,6 +36,7 @@
ApproveEndExperimentForm,
ApproveUpdateRolloutForm,
AudienceForm,
+ BranchLeadingScreenshotForm,
BranchScreenshotCreateForm,
BranchScreenshotDeleteForm,
CancelEndEnrollmentForm,
@@ -671,6 +672,38 @@ class ApproveUpdateRolloutView(StatusUpdateView):
form_class = ApproveUpdateRolloutForm
+class BranchLeadingScreenshotView(
+ NimbusExperimentViewMixin,
+ RequestFormMixin,
+ UpdateView,
+):
+ form_class = BranchLeadingScreenshotForm
+
+ def get_branch(self):
+ return (
+ self.get_object().branches.filter(slug=self.kwargs.get("branch_slug")).first()
+ )
+
+ def get_form_kwargs(self):
+ kwargs = super().get_form_kwargs()
+
+ if branch := self.get_branch():
+ if screenshot := branch.screenshots.first():
+ kwargs["instance"] = screenshot
+ else:
+ # create an unsaved placeholder instance of the correct model
+ screenshot_model = branch.screenshots.model
+ kwargs["instance"] = screenshot_model(branch=branch)
+
+ return kwargs
+
+ def form_valid(self, form):
+ form.save()
+ response = HttpResponse()
+ response.headers["HX-Refresh"] = "true"
+ return response
+
+
class NewResultsView(NimbusExperimentViewMixin, DetailView):
template_name = "nimbus_experiments/results-new.html"
@@ -710,6 +743,13 @@ def get_context_data(self, **kwargs):
analysis_basis, selected_segment, selected_reference_branch
)
+ branch_leading_screenshot_forms = {
+ branch.slug: BranchLeadingScreenshotForm(instance=branch.screenshots.first())
+ for branch in experiment.branches.all()
+ }
+
+ context["branch_leading_screenshot_forms"] = branch_leading_screenshot_forms
+
relative_metric_changes = {}
all_metrics = experiment.get_metric_data(