feat(nimbus): Show invalid fields errors

Because

- There is no warning message displayed on the summary page for pages with invalid fields
- The user has no way of knowing which fields are invalid and why

This commit

- Adds a warning message on the summary page if there are pages with invalid fields
- Includes links to the pages with invalid fields in the warning message
- Highlights the invalid fields and displays the reasons for the errors on the form when the link in the warning message is clicked

Fixes #12009

experimenter/experimenter/experiments/models.py

@@ -1307,6 +1307,47 @@ def audience_overlap_warnings(self):
 
         return warnings
 
+    def get_invalid_fields_errors(self, data=None):
+        from experimenter.experiments.api.v5.serializers import (
+            NimbusReviewSerializer,
+        )
+
+        temp_data = NimbusReviewSerializer(self).data
+        if data:
+            temp_data.update(data)
+
+        serializer = NimbusReviewSerializer(self, data=temp_data)
+
+        if serializer.is_valid():
+            return []
+        else:
+            field_errors = []
+            for field, error_list in serializer.errors.items():
+                for error in error_list:
+                    if field == "excluded_experiments":
+                        field_errors.append(("excluded_experiments_branches", str(error)))
+                    elif field == "required_experiments":
+                        field_errors.append(("required_experiments_branches", str(error)))
+                    else:
+                        field_errors.append((field, str(error)))
+            return field_errors
+
+    @property
+    def get_invalid_pages(self):
+        field_errors = self.get_invalid_fields_errors()
+
+        if not field_errors:
+            return []
+
+        error_fields = [field for field, _ in field_errors]
+        pages_with_errors = []
+
+        for page, fields in NimbusUIConstants.FIELD_PAGE_MAP.items():
+            if any(field in error_fields for field in fields):
+                pages_with_errors.append(page)
+
+        return pages_with_errors
+
     def clone(self, name, user, rollout_branch_slug=None, changed_on=None):
         # Inline import to prevent circular import
         from experimenter.experiments.changelog_utils import generate_nimbus_changelog

experimenter/experimenter/experiments/tests/test_models.py

@@ -3542,6 +3542,67 @@ def test_recipe_json_renders_nimbus_experiment_serializer(self):
         recipe_json_keys = sorted(json.loads(experiment.recipe_json.replace("\n", "")))
         self.assertEqual(serialized_keys, recipe_json_keys)
 
+    def test_get_invalid_pages(self):
+        experiment_1 = NimbusExperimentFactory.create(
+            name="test-experiment-1",
+            public_description="",
+        )
+        invalid_pages = experiment_1.get_invalid_pages
+        self.assertEqual(invalid_pages, ["overview"])
+
+        experiment_2 = NimbusExperimentFactory.create(
+            name="test-experiment-2",
+            feature_configs=[],
+            population_percent=0,
+        )
+        invalid_pages = experiment_2.get_invalid_pages
+        self.assertEqual(invalid_pages, ["branches", "audience"])
+
+        experiment_3 = NimbusExperimentFactory.create(
+            name="test-experiment-5",
+            public_description="",
+            population_percent=0,
+        )
+        invalid_pages = experiment_3.get_invalid_pages
+        self.assertEqual(invalid_pages, ["overview", "audience"])
+
+    def test_get_invalid_fields_errors(self):
+        experiment_1 = NimbusExperimentFactory.create(
+            name="test-experiment-3",
+            public_description="",
+        )
+        errors = experiment_1.get_invalid_fields_errors()
+        self.assertIn(("public_description", "This field may not be blank."), errors)
+
+        experiment_2 = NimbusExperimentFactory.create(
+            name="test-experiment-4",
+            feature_configs=[],
+            population_percent=0,
+        )
+        errors = experiment_2.get_invalid_fields_errors()
+        self.assertIn(
+            (
+                "feature_configs",
+                "You must select a feature configuration from the drop down.",
+            ),
+            errors,
+        )
+        self.assertIn(
+            (
+                "population_percent",
+                "Ensure this value is greater than or equal to 0.0001.",
+            ),
+            errors,
+        )
+
+        experiment_3 = NimbusExperimentFactory.create(
+            name="test-experiment-5",
+            public_description="",
+            population_percent=0,
+        )
+        errors = experiment_3.get_invalid_fields_errors()
+        self.assertIn(("public_description", "This field may not be blank."), errors)
+
 
 class TestNimbusBranch(TestCase):
     def test_str(self):

experimenter/experimenter/nimbus_ui_new/constants.py

@@ -49,3 +49,35 @@ class NimbusUIConstants:
             "period ended."
         ),
     }
+
+    FIELD_PAGE_MAP = {
+        "overview": [
+            "projects",
+            "public_description",
+            "risk_brand",
+            "risk_message",
+            "risk_revenue",
+            "risk_partner_related",
+        ],
+        "branches": [
+            "reference_branch",
+            "treatment_branches",
+            "feature_configs",
+            "is_rollout",
+            "localizations",
+        ],
+        "audience": [
+            "channel",
+            "firefox_min_version",
+            "languages",
+            "locales",
+            "countries",
+            "targeting_config_slug",
+            "proposed_enrollment",
+            "proposed_duration",
+            "population_percent",
+            "total_enrolled_clients",
+            "required_experiments",
+            "excluded_experiments",
+        ],
+    }

experimenter/experimenter/nimbus_ui_new/forms.py

@@ -1,5 +1,6 @@
 from django import forms
 from django.contrib.auth.models import User
+from django.db import models
 from django.forms import inlineformset_factory
 from django.http import HttpRequest
 from django.utils.text import slugify
@@ -35,6 +36,45 @@ def save(self, *args, **kwargs):
         return experiment
 
 
+class NimbusSerializerErrorMixin:
+    def clean(self):
+        if self.show_validation_errors:
+            cleaned_data = super().clean()
+
+            for key, value in cleaned_data.items():
+                if isinstance(value, models.QuerySet):
+                    cleaned_data[key] = [item.pk for item in value]
+
+            for field_name in [
+                "excluded_experiments_branches",
+                "required_experiments_branches",
+            ]:
+                if field_name in cleaned_data:
+                    slugs = cleaned_data[field_name]
+                    pks = []
+                    for slug in slugs:
+                        experiment_slug = slug.split(":")[0]
+                        try:
+                            experiment = NimbusExperiment.objects.get(
+                                slug=experiment_slug
+                            )
+                            pks.append(experiment.pk)
+                        except NimbusExperiment.DoesNotExist:
+                            pass
+                    cleaned_data[field_name.replace("branches", "")] = pks
+
+            errors = self.instance.get_invalid_fields_errors(data=cleaned_data)
+
+            if errors:
+                for field, error in errors:
+                    if field in self.fields:
+                        self.add_error(field, error)
+
+            return cleaned_data
+        else:
+            return super().clean()
+
+
 class NimbusExperimentCreateForm(NimbusChangeLogFormMixin, forms.ModelForm):
     owner = forms.ModelChoiceField(
         User.objects.all(),
@@ -185,7 +225,7 @@ class Meta:
         fields = ("title", "link")
 
 
-class OverviewForm(NimbusChangeLogFormMixin, forms.ModelForm):
+class OverviewForm(NimbusChangeLogFormMixin, NimbusSerializerErrorMixin, forms.ModelForm):
     YES_NO_CHOICES = (
         (True, "Yes"),
         (False, "No"),
@@ -242,6 +282,11 @@ class Meta:
         ]
 
     def __init__(self, *args, **kwargs):
+        request = kwargs.get("request")
+        if request:
+            self.show_validation_errors = request.GET.get("show_errors", "") == "true"
+        else:
+            self.show_validation_errors = False
         super().__init__(*args, **kwargs)
         self.NimbusDocumentationLinkFormSet = inlineformset_factory(
             NimbusExperiment,
@@ -337,7 +382,7 @@ def get_changelog_message(self):
         return f"{self.request.user} updated metrics"
 
 
-class AudienceForm(NimbusChangeLogFormMixin, forms.ModelForm):
+class AudienceForm(NimbusChangeLogFormMixin, NimbusSerializerErrorMixin, forms.ModelForm):
     def get_experiment_branch_choices():
         return sorted(
             [
@@ -454,6 +499,11 @@ class Meta:
         ]
 
     def __init__(self, *args, **kwargs):
+        request = kwargs.get("request")
+        if request:
+            self.show_validation_errors = request.GET.get("show_errors", "") == "true"
+        else:
+            self.show_validation_errors = False
         super().__init__(*args, **kwargs)
         self.setup_initial_experiments_branches("required_experiments_branches")
         self.setup_initial_experiments_branches("excluded_experiments_branches")

experimenter/experimenter/nimbus_ui_new/static/js/show_errors.js

@@ -0,0 +1,13 @@
+document.addEventListener("DOMContentLoaded", function () {
+  const urlParams = new URLSearchParams(window.location.search);
+  if (urlParams.get("show_errors")) {
+    const formIds = ["audience-form", "metrics-form"];
+
+    formIds.forEach(function (formId) {
+      const form = document.getElementById(formId);
+      if (form) {
+        form.requestSubmit();
+      }
+    });
+  }
+});

experimenter/experimenter/nimbus_ui_new/static/webpack.config.js

@@ -6,6 +6,7 @@ module.exports = {
     app: "./js/index.js",
     experiment_list: "./js/experiment_list.js",
     edit_audience: "./js/edit_audience.js",
+    show_errors: "./js/show_errors.js",
   },
   output: {
     filename: "[name].bundle.js",

experimenter/experimenter/nimbus_ui_new/templates/nimbus_experiments/detail.html

@@ -6,6 +6,40 @@
 {% block title %}{{ experiment.name }}{% endblock %}
 
 {% block main_content %}
+  <!-- Invalid Pages Warnings Card -->
+  {% if experiment.get_invalid_pages %}
+    <div class="alert alert-danger" role="alert">
+      <p class="mb-1">
+        Before this experiment can be reviewed or launched, all required fields must be completed. Fields on the
+        <strong>
+          {% for page in experiment.get_invalid_pages %}
+            {% if page == "overview" %}
+              <a href="{{ experiment.get_update_overview_url }}?show_errors=true"
+                 rel="noopener noreferrer">Overview</a>
+            {% elif page == "branches" %}
+              <a href="{{ experiment.get_update_overview_url }}?show_errors=true"
+                 rel="noopener noreferrer">Branches</a>
+            {% elif page == "metrics" %}
+              <a href="{{ experiment.get_update_metrics_url }}?show_errors=true"
+                 rel="noopener noreferrer">Metrics</a>
+            {% elif page == "audience" %}
+              <a href="{{ experiment.get_update_audience_url }}?show_errors=true"
+                 rel="noopener noreferrer">Audience</a>
+            {% else %}
+              {{ page|capfirst }}
+            {% endif %}
+            {% if not forloop.last %},{% endif %}
+          {% endfor %}
+        </strong>
+        {% if experiment.get_invalid_pages|length == 1 %}
+          page
+        {% else %}
+          pages
+        {% endif %}
+        are missing details.
+      </p>
+    </div>
+  {% endif %}
   <!-- Audience Overlap Warnings Card -->
   {% if experiment.audience_overlap_warnings %}
     {% for warning in experiment.audience_overlap_warnings %}