risk rel pop wording and risk mapping exclusion fixes #2261 (#2286)

* risk rel pop wording and risk mapping exclusion fixes #2261 * formatting Co-authored-by: Jared Lockhart <[email protected]>
mozilla · Feb 18, 2020 · 57b96cc · 57b96cc
1 parent 89e7f81
commit 57b96cc
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 22 deletions.
diff --git a/app/experimenter/experiments/constants.py b/app/experimenter/experiments/constants.py
@@ -220,9 +220,7 @@ class ExperimentConstants(object):
     RISK_CONFIDENTIAL_LABEL = (
         "Is this delivery confidential to Mozilla, sensitive, or internal only?"
     )
-    RISK_RELEASE_POPULATION_LABEL = (
-        "Does this delivery affect 1% or more of Release users?"
-    )
+    RISK_RELEASE_POPULATION_LABEL = "Does this delivery affect 1 million Release users?"
     RISK_REVENUE_LABEL = """Does this delivery have possible negative impact on revenue
          (ex: search, pocket revenue, New Tab page experience)?"""
 
@@ -242,6 +240,7 @@ class ExperimentConstants(object):
     RISK_HIGHER_RISK_LABEL = """I have been advised that this delivery design creates a
         higher risk of errors due to complexity or timing requirements."""
 
+    RISK_EXCLUSIONS = {TYPE_ROLLOUT: ["risk_release_population"]}
     SURVEY_REQUIRED_LABEL = "Is a Survey Required?"
     SURVEY_INSTRUCTIONS_LABEL = "Survey Launch Instructions"
 

diff --git a/app/experimenter/experiments/models.py b/app/experimenter/experiments/models.py
@@ -616,23 +616,26 @@ def completed_results(self):
 
     @property
     def _risk_questions(self):
-        return (
-            self.risk_partner_related,
-            self.risk_brand,
-            self.risk_fast_shipped,
-            self.risk_confidential,
-            self.risk_release_population,
-            self.risk_revenue,
-            self.risk_data_category,
-            self.risk_external_team_impact,
-            self.risk_telemetry_data,
-            self.risk_ux,
-            self.risk_security,
-            self.risk_revision,
-            self.risk_technical,
-            self.risk_higher_risk,
+        risk_questions = (
+            "risk_partner_related",
+            "risk_brand",
+            "risk_fast_shipped",
+            "risk_confidential",
+            "risk_release_population",
+            "risk_revenue",
+            "risk_data_category",
+            "risk_external_team_impact",
+            "risk_telemetry_data",
+            "risk_ux",
+            "risk_security",
+            "risk_revision",
+            "risk_technical",
+            "risk_higher_risk",
         )
 
+        exclusions = ExperimentConstants.RISK_EXCLUSIONS.get(self.type, [])
+        return [getattr(self, risk) for risk in risk_questions if risk not in exclusions]
+
     @property
     def completed_risks(self):
         completed = None not in self._risk_questions

diff --git a/app/experimenter/experiments/tests/test_models.py b/app/experimenter/experiments/tests/test_models.py
@@ -1003,7 +1003,7 @@ def test_risk_questions_returns_a_tuple(self):
         )
         self.assertEqual(
             experiment._risk_questions,
-            (
+            [
                 False,
                 True,
                 False,
@@ -1018,7 +1018,44 @@ def test_risk_questions_returns_a_tuple(self):
                 True,
                 False,
                 True,
-            ),
+            ],
+        )
+
+    def test_risk_questions_returns_a_tuple_rollout(self):
+        experiment = ExperimentFactory.create(
+            type=Experiment.TYPE_ROLLOUT,
+            risk_partner_related=False,
+            risk_brand=True,
+            risk_fast_shipped=False,
+            risk_confidential=True,
+            risk_release_population=None,
+            risk_revenue=True,
+            risk_data_category=False,
+            risk_external_team_impact=True,
+            risk_telemetry_data=False,
+            risk_ux=True,
+            risk_security=False,
+            risk_revision=True,
+            risk_technical=False,
+            risk_higher_risk=True,
+        )
+        self.assertEqual(
+            experiment._risk_questions,
+            [
+                False,
+                True,
+                False,
+                True,
+                True,
+                False,
+                True,
+                False,
+                True,
+                False,
+                True,
+                False,
+                True,
+            ],
         )
 
     def test_risk_not_completed_when_risk_questions_not_answered(self):

diff --git a/app/experimenter/templates/experiments/edit_risks.html b/app/experimenter/templates/experiments/edit_risks.html
@@ -22,8 +22,9 @@
     {% include "experiments/radio_field_inline.html" with field=form.risk_fast_shipped %}
 
     {% include "experiments/radio_field_inline.html" with field=form.risk_confidential %}
-
-    {% include "experiments/radio_field_inline.html" with field=form.risk_release_population %}
+    {% if not experiment.is_rollout %}
+      {% include "experiments/radio_field_inline.html" with field=form.risk_release_population %}
+    {% endif %}
 
     {% include "experiments/radio_field_inline.html" with field=form.risk_revenue %}
 

diff --git a/app/experimenter/templates/experiments/section_risks.html b/app/experimenter/templates/experiments/section_risks.html
@@ -33,12 +33,14 @@
     </strong>
   </p>
 
+  {% if not experiment.is_rollout %}
   <p>
     {{ experiment.RISK_RELEASE_POPULATION_LABEL }}
     <strong class="{% if experiment.risk_release_population %}text-danger{% endif %}">
       {{ experiment.risk_release_population|yesno:"Yes,No" }}
     </strong>
   </p>
+  {% endif %}
 
   <p>
     {{ experiment.RISK_REVENUE_LABEL }}