From aabdbb71d934a20f3129f7063df8d110976c457e Mon Sep 17 00:00:00 2001 From: Alissa Sobo Date: Thu, 19 Mar 2020 12:39:11 -0700 Subject: [PATCH] Fix rollout timeline/pop page fixes #2403 (#2406) * Fix rollout timeline/pop page fixes #2403 * Split into two methods of enrollment and playbook * Add values to options Co-authored-by: Jared Lockhart <119884+jaredlockhart@users.noreply.github.com> --- app/experimenter/docs/openapi-schema.json | 36 +++++++++ app/experimenter/docs/swagger-ui.html | 36 +++++++++ .../serializers/timeline_population.py | 4 + .../serializers/test_timeline_population.py | 57 +++++++++++++- .../static/js/components/TimelinePopForm.js | 75 ++++++++++++++----- .../static/js/components/constants.js | 11 +++ app/experimenter/static/js/index.js | 2 + .../static/js/tests/TimelinePopForm.test.js | 37 ++++++++- .../experiments/edit_timeline_population.html | 1 + 9 files changed, 236 insertions(+), 23 deletions(-) diff --git a/app/experimenter/docs/openapi-schema.json b/app/experimenter/docs/openapi-schema.json index fe3aacec86..f67b12f389 100644 --- a/app/experimenter/docs/openapi-schema.json +++ b/app/experimenter/docs/openapi-schema.json @@ -4556,6 +4556,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -4662,6 +4666,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -4744,6 +4752,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -4826,6 +4838,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -4913,6 +4929,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -5019,6 +5039,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -5101,6 +5125,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -5183,6 +5211,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -5270,6 +5302,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true diff --git a/app/experimenter/docs/swagger-ui.html b/app/experimenter/docs/swagger-ui.html index 4ffdada71d..1f0fa337b5 100644 --- a/app/experimenter/docs/swagger-ui.html +++ b/app/experimenter/docs/swagger-ui.html @@ -4568,6 +4568,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -4674,6 +4678,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -4756,6 +4764,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -4838,6 +4850,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -4925,6 +4941,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -5031,6 +5051,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -5113,6 +5137,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -5195,6 +5223,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true @@ -5282,6 +5314,10 @@ "type": "integer", "nullable": true }, + "rollout_playbook": { + "type": "string", + "nullable": true + }, "proposed_duration": { "type": "integer", "nullable": true diff --git a/app/experimenter/experiments/serializers/timeline_population.py b/app/experimenter/experiments/serializers/timeline_population.py index bebc2b8f74..49a96b7a7e 100644 --- a/app/experimenter/experiments/serializers/timeline_population.py +++ b/app/experimenter/experiments/serializers/timeline_population.py @@ -36,6 +36,9 @@ class ExperimentTimelinePopSerializer( proposed_enrollment = serializers.IntegerField( required=False, allow_null=True, default=None ) + rollout_playbook = serializers.CharField( + required=False, allow_null=True, default=None + ) population_percent = serializers.DecimalField( required=False, max_digits=7, @@ -65,6 +68,7 @@ class Meta: fields = ( "proposed_start_date", "proposed_enrollment", + "rollout_playbook", "proposed_duration", "population_percent", "firefox_channel", diff --git a/app/experimenter/experiments/tests/serializers/test_timeline_population.py b/app/experimenter/experiments/tests/serializers/test_timeline_population.py index 03318c4a08..fd494e22f0 100644 --- a/app/experimenter/experiments/tests/serializers/test_timeline_population.py +++ b/app/experimenter/experiments/tests/serializers/test_timeline_population.py @@ -48,7 +48,7 @@ def setUp(self): population_percent="30.0000", ) - def test_serializer_outputs_expected_schema(self): + def test_serializer_outputs_expected_schema_pref(self): serializer = ExperimentTimelinePopSerializer(self.experiment) @@ -58,6 +58,7 @@ def test_serializer_outputs_expected_schema(self): "proposed_start_date": self.experiment.proposed_start_date.strftime( "%Y-%m-%d" ), + "rollout_playbook": None, "proposed_enrollment": self.experiment.proposed_enrollment, "proposed_duration": self.experiment.proposed_duration, "population_percent": self.experiment.population_percent, @@ -152,3 +153,57 @@ def test_serializer_rejects_enrollment_greater_duration(self): self.assertFalse(serializer.is_valid()) self.assertIn("proposed_enrollment", serializer.errors) + + def test_serializer_outputs_expected_schema_rollout(self): + experiment = ExperimentFactory.create( + type=ExperimentConstants.TYPE_ROLLOUT, + rollout_playbook="Low Risk Schedule", + locales=[self.locale], + countries=[self.country], + population_percent="30.0000", + proposed_enrollment=None, + ) + + serializer = ExperimentTimelinePopSerializer(experiment) + + self.assertEqual( + serializer.data, + { + "proposed_start_date": experiment.proposed_start_date.strftime( + "%Y-%m-%d" + ), + "proposed_enrollment": experiment.proposed_enrollment, + "rollout_playbook": experiment.rollout_playbook, + "proposed_duration": experiment.proposed_duration, + "population_percent": experiment.population_percent, + "firefox_channel": experiment.firefox_channel, + "firefox_min_version": experiment.firefox_min_version, + "firefox_max_version": experiment.firefox_max_version, + "locales": [{"value": self.locale.id, "label": self.locale.name}], + "countries": [{"value": self.country.id, "label": self.country.name}], + "platform": experiment.platform, + "client_matching": experiment.client_matching, + }, + ) + + def test_serializer_saves_rollout_playbook_field(self): + experiment = ExperimentFactory.create( + type=ExperimentConstants.TYPE_ROLLOUT, + rollout_playbook="Low Risk Schedule", + locales=[self.locale], + countries=[self.country], + population_percent="30.0000", + proposed_enrollment=None, + ) + + data = {"rollout_playbook": "High Risk Schedule", "countries": [], "locales": []} + + serializer = ExperimentTimelinePopSerializer( + instance=experiment, data=data, context={"request": self.request} + ) + + self.assertTrue(serializer.is_valid()) + + experiment = serializer.save() + + self.assertEqual(experiment.rollout_playbook, "High Risk Schedule") diff --git a/app/experimenter/static/js/components/TimelinePopForm.js b/app/experimenter/static/js/components/TimelinePopForm.js index 87c5555356..1d8119f3b3 100644 --- a/app/experimenter/static/js/components/TimelinePopForm.js +++ b/app/experimenter/static/js/components/TimelinePopForm.js @@ -10,6 +10,7 @@ import LabeledMultiSelect from "experimenter/components/LabeledMultiSelect"; import { makeApiRequest } from "experimenter/utils/api"; import { VERSION_CHOICES, + PLAYBOOK_CHOICES, PROPOSED_START_DATE_HELP, PROPOSED_DURATION_HELP, PROPOSED_ENROLLMENT_HELP, @@ -19,6 +20,7 @@ import { PLATFORM_HELP, CLIENT_MATCHING_HELP, COUNTRIES_LOCALES_HELP, + ROLLOUT_PLAYBOOK_HELP, } from "experimenter/components/constants"; @boundClass @@ -28,6 +30,7 @@ class TimelinePopForm extends React.PureComponent { shouldHavePopPercent: PropTypes.string, allCountries: PropTypes.array, allLocales: PropTypes.array, + experimentType: PropTypes.string, }; constructor(props) { @@ -94,14 +97,57 @@ class TimelinePopForm extends React.PureComponent { } } - displayVersionsOptions() { - let versionsJSX = []; + displayEnrollmentDuration() { + if (this.props.experimentType != "rollout") { + return ( + { + this.handleDataChange("proposed_enrollment", value); + }} + value={this.state.data.get("proposed_enrollment")} + error={this.state.errors.get("proposed_enrollment", "")} + helpContent={PROPOSED_ENROLLMENT_HELP} + labelColumnWidth={2} + optional={true} + /> + ); + } + } - for (const version of VERSION_CHOICES) { - versionsJSX.push(); + displayRolloutPlaybook() { + if (this.props.experimentType === "rollout") { + return ( + { + this.handleDataChange("rollout_playbook", value); + }} + value={this.state.data.get("rollout_playbook")} + error={this.state.errors.get("rollout_playbook", "")} + labelColumnWidth={2} + helpContent={ROLLOUT_PLAYBOOK_HELP} + helpIsExternalLink={true} + > + {this.displayOptions(PLAYBOOK_CHOICES)} + + ); + } + } + + displayOptions(choices) { + let choicesJSX = []; + + for (const choice of choices) { + choicesJSX.push(); } - return versionsJSX; + return choicesJSX; } async handleSubmit(event, redirectUrl) { @@ -186,19 +232,8 @@ class TimelinePopForm extends React.PureComponent { helpContent={PROPOSED_DURATION_HELP} labelColumnWidth={2} /> - { - this.handleDataChange("proposed_enrollment", value); - }} - value={this.state.data.get("proposed_enrollment")} - error={this.state.errors.get("proposed_enrollment", "")} - helpContent={PROPOSED_ENROLLMENT_HELP} - labelColumnWidth={2} - optional={true} - /> + {this.displayEnrollmentDuration()} + {this.displayRolloutPlaybook()}

Delivery Population

@@ -239,7 +274,7 @@ class TimelinePopForm extends React.PureComponent { labelColumnWidth={4} helpIsExternalLink={true} > - {this.displayVersionsOptions()} + {this.displayOptions(VERSION_CHOICES)} @@ -256,7 +291,7 @@ class TimelinePopForm extends React.PureComponent { labelColumnWidth={4} noHelpLink={true} > - {this.displayVersionsOptions()} + {this.displayOptions(VERSION_CHOICES)} diff --git a/app/experimenter/static/js/components/constants.js b/app/experimenter/static/js/components/constants.js index 2a854afee5..fec5d4020c 100644 --- a/app/experimenter/static/js/components/constants.js +++ b/app/experimenter/static/js/components/constants.js @@ -212,6 +212,14 @@ export const VERSION_CHOICES = [ ["80.0", "Firefox 80.0"], ]; +export const PLAYBOOK_CHOICES = [ + [null, "Rollout Playbook"], + ["low_risk", "Low Risk Schedule"], + ["high_risk", "High Risk Schedule"], + ["marketing", "Marketing Launch Schedule"], + ["custom", "Custom Schedule"], +]; + export const PROPOSED_START_DATE_HELP = (

Choose the date you expect the delivery to be launched to users. This date @@ -274,6 +282,9 @@ export const POPULATION_PERCENT_HELP = ( export const VERSION_HELP = "https://wiki.mozilla.org/Release_Management/Calendar"; +export const ROLLOUT_PLAYBOOK_HELP = + "https://mana.mozilla.org/wiki/pages/viewpage.action?pageId=90737068#StagedRollouts/GradualRollouts-Playbooks"; + export const PLATFORM_HELP = (

Select the target platform for this delivery.

); diff --git a/app/experimenter/static/js/index.js b/app/experimenter/static/js/index.js index b492e42cef..a7a3862f25 100644 --- a/app/experimenter/static/js/index.js +++ b/app/experimenter/static/js/index.js @@ -37,12 +37,14 @@ if (branchesDiv) { } if (timelinePopDiv) { + const experimentType = timelinePopDiv.dataset.experimentType; const slug = timelinePopDiv.dataset.experimentSlug; const shouldHavePopPercent = timelinePopDiv.dataset.shouldHavePopPercent; const allCountries = JSON.parse(timelinePopDiv.dataset.allCountries); const allLocales = JSON.parse(timelinePopDiv.dataset.allLocales); ReactDOM.render( { const apiResponse = setup(); const { getByLabelText, getByText, container } = await render( - , + , ); expect(Api.makeApiRequest).toHaveBeenCalledTimes(1); @@ -116,7 +120,7 @@ describe("The TimelinePopForm component for experiments", () => { window.HTMLElement.prototype.scrollIntoView = scrollIntoViewMock; const { getByLabelText, getByText, container } = await render( - , + , ); await waitForFormToLoad(container); @@ -150,4 +154,33 @@ describe("The TimelinePopForm component for experiments", () => { expect(Api.makeApiRequest).toHaveBeenCalled(); expect(console.error).toHaveBeenCalled(); }); + + it("displays and edit rollout playbook on rollout delivery", async () => { + setup(); + + const { getByLabelText, getByText, container } = await render( + , + ); + + expect(Api.makeApiRequest).toHaveBeenCalledTimes(1); + + await waitForFormToLoad(container); + + const rolloutPlaybookInput = getByLabelText(/Rollout Playbook/); + + location.replace = () => {}; + + fireEvent.change(rolloutPlaybookInput, { + target: { value: "low_risk" }, + }); + fireEvent.click(getByText("Save Draft")); + + const [url, { data }] = Api.makeApiRequest.mock.calls[1]; + expect(url).toBe("experiments/the-slug/timeline-population/"); + expect(data.rollout_playbook).toBe("low_risk"); + }); }); diff --git a/app/experimenter/templates/experiments/edit_timeline_population.html b/app/experimenter/templates/experiments/edit_timeline_population.html index b56e4f7550..b8c9bb2b4f 100644 --- a/app/experimenter/templates/experiments/edit_timeline_population.html +++ b/app/experimenter/templates/experiments/edit_timeline_population.html @@ -22,6 +22,7 @@ data-all-countries="{{ countries }}" data-all-locales="{{ locales }}" id="react-timelinepop-form" + data-experiment-type="{{ object.type }}" >