diff --git a/app/experimenter/experiments/constants.py b/app/experimenter/experiments/constants.py index 4613254349..4cddd9fa3c 100644 --- 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 index 897fdad39a..300bf9e834 100644 --- 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 index 612989f4b4..cb74ec48ef 100644 --- 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 index aa7be3ba4e..308fa155bb 100644 --- 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 index e976fdf4ab..2ef373beb9 100644 --- a/app/experimenter/templates/experiments/section_risks.html +++ b/app/experimenter/templates/experiments/section_risks.html @@ -33,12 +33,14 @@
+ {% if not experiment.is_rollout %}{{ experiment.RISK_RELEASE_POPULATION_LABEL }} {{ experiment.risk_release_population|yesno:"Yes,No" }}
+ {% endif %}{{ experiment.RISK_REVENUE_LABEL }}