From 4769934ba20d1f293b76fd579b52f4e00300bdc6 Mon Sep 17 00:00:00 2001
From: Peter Bengtsson <mail@peterbe.com>
Date: Fri, 22 Mar 2019 12:38:00 -0400
Subject: [PATCH] risk_technical and risk_technical_description invariance
 fixes #1082

Fixes #1082
---
 app/experimenter/experiments/forms.py         | 21 ++++++++++
 .../experiments/tests/test_forms.py           | 40 +++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/app/experimenter/experiments/forms.py b/app/experimenter/experiments/forms.py
index 9bddbd92c6..fafab2afdc 100644
--- a/app/experimenter/experiments/forms.py
+++ b/app/experimenter/experiments/forms.py
@@ -689,6 +689,27 @@ class Meta:
             "qa_status",
         )
 
+    def clean(self):
+        cleaned_data = super().clean()
+        if (
+            "risk_technical" in cleaned_data
+            and "risk_technical_description" in cleaned_data
+        ):
+            # Both checked, now we need to do an invariance check on these
+            # two. This is to match what's done with jQuery in the form:
+            # the 'risk_technical_description' needs to be required
+            # if 'risk_technical' is truthy.
+            if cleaned_data.get("risk_technical") == "True":
+                if not cleaned_data["risk_technical_description"]:
+                    msg = (
+                        f"This is required if "
+                        f"'{Experiment.RISK_TECHNICAL_LABEL}' is true."
+                    )
+                    raise forms.ValidationError(
+                        {"risk_technical_description": msg}
+                    )
+        return cleaned_data
+
 
 class ExperimentReviewForm(
     ExperimentConstants, ChangeLogMixin, forms.ModelForm
diff --git a/app/experimenter/experiments/tests/test_forms.py b/app/experimenter/experiments/tests/test_forms.py
index 4fb4389c31..cd79d2342b 100644
--- a/app/experimenter/experiments/tests/test_forms.py
+++ b/app/experimenter/experiments/tests/test_forms.py
@@ -1081,6 +1081,46 @@ def test_form_saves_risks(self):
         self.assertEqual(experiment.test_builds, data["test_builds"])
         self.assertEqual(experiment.qa_status, data["qa_status"])
 
+    def test_form_risk_technical_invariance(self):
+        created_experiment = ExperimentFactory.create_with_status(
+            Experiment.STATUS_DRAFT
+        )
+
+        data = {
+            "risk_internal_only": True,
+            "risk_partner_related": True,
+            "risk_brand": True,
+            "risk_fast_shipped": True,
+            "risk_confidential": True,
+            "risk_release_population": True,
+            "risk_technical": True,
+            "risk_technical_description": "",  # Note!
+            "risks": "There are some risks",
+            "testing": "Always be sure to test!",
+            "test_builds": "Latest build",
+            "qa_status": "It ain't easy being green",
+        }
+
+        form = ExperimentRisksForm(
+            request=self.request, data=data, instance=created_experiment
+        )
+        self.assertFalse(form.is_valid())
+
+        # It would be okey if 'risk_technical' was falsy.
+        data["risk_technical"] = False
+        form = ExperimentRisksForm(
+            request=self.request, data=data, instance=created_experiment
+        )
+        self.assertTrue(form.is_valid())
+
+        # And definitely OK if both at truthy.
+        data["risk_technical"] = True
+        data["risk_technical_description"] = "Some text"
+        form = ExperimentRisksForm(
+            request=self.request, data=data, instance=created_experiment
+        )
+        self.assertTrue(form.is_valid())
+
 
 class TestExperimentReviewForm(
     MockRequestMixin, MockBugzillaMixin, MockTasksMixin, TestCase