diff --git a/app/experimenter/experiments/forms.py b/app/experimenter/experiments/forms.py index 3fbe6e9ac9..fe24e375e0 100644 --- a/app/experimenter/experiments/forms.py +++ b/app/experimenter/experiments/forms.py @@ -1,4 +1,3 @@ -import decimal import json import re @@ -6,13 +5,10 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth import get_user_model -from django.forms.models import ModelChoiceIterator -from django.utils import timezone from django.utils.html import strip_tags from django.utils.safestring import mark_safe from django.utils.text import slugify -from experimenter.base.models import Country, Locale from experimenter.projects.models import Project from experimenter.bugzilla import get_bugzilla_id from experimenter.experiments import tasks @@ -246,220 +242,6 @@ def clean(self): return cleaned_data -class CustomModelChoiceIterator(ModelChoiceIterator): - - def __iter__(self): - yield (CustomModelMultipleChoiceField.ALL_KEY, self.field.all_label) - for choice in super().__iter__(): - yield choice - - -class CustomModelMultipleChoiceField(forms.ModelMultipleChoiceField): - """Return a ModelMultipleChoiceField but with the exception that - there's one extra "All" choice inserted as the first choice. - And when submitted, if "All" was one of the choices, reset - it to chose nothing.""" - - ALL_KEY = "__all__" - - def __init__(self, *args, **kwargs): - self.all_label = kwargs.pop("all_label") - super().__init__(*args, **kwargs) - - def clean(self, value): - if value is not None: - if self.ALL_KEY in value: - value = [] - return super().clean(value) - - iterator = CustomModelChoiceIterator - - -class ExperimentTimelinePopulationForm(ChangeLogMixin, forms.ModelForm): - proposed_start_date = forms.DateField( - required=False, - label="Proposed Start Date", - help_text=Experiment.PROPOSED_START_DATE_HELP_TEXT, - widget=forms.DateInput(attrs={"type": "date", "class": "form-control"}), - ) - proposed_duration = forms.IntegerField( - required=False, - min_value=1, - label="Proposed Total Duration (days)", - help_text=Experiment.PROPOSED_DURATION_HELP_TEXT, - widget=forms.NumberInput(attrs={"class": "form-control"}), - ) - proposed_enrollment = forms.IntegerField( - required=False, - min_value=1, - label="Proposed Enrollment Duration (days)", - help_text=Experiment.PROPOSED_ENROLLMENT_HELP_TEXT, - widget=forms.NumberInput(attrs={"class": "form-control"}), - ) - rollout_playbook = forms.ChoiceField( - required=False, - label="Rollout Playbook", - choices=Experiment.ROLLOUT_PLAYBOOK_CHOICES, - widget=forms.Select(attrs={"class": "form-control"}), - help_text=Experiment.ROLLOUT_PLAYBOOK_HELP_TEXT, - ) - population_percent = forms.DecimalField( - required=False, - label="Population Percentage", - help_text=Experiment.POPULATION_PERCENT_HELP_TEXT, - initial="0.00", - widget=forms.NumberInput(attrs={"class": "form-control"}), - ) - firefox_min_version = forms.ChoiceField( - required=False, - choices=Experiment.MIN_VERSION_CHOICES, - widget=forms.Select(attrs={"class": "form-control"}), - help_text=Experiment.VERSION_HELP_TEXT, - ) - firefox_max_version = forms.ChoiceField( - required=False, - choices=Experiment.MAX_VERSION_CHOICES, - widget=forms.Select(attrs={"class": "form-control"}), - ) - firefox_channel = forms.ChoiceField( - required=False, - choices=Experiment.CHANNEL_CHOICES, - widget=forms.Select(attrs={"class": "form-control"}), - label="Firefox Channel", - help_text=Experiment.CHANNEL_HELP_TEXT, - ) - locales = CustomModelMultipleChoiceField( - required=False, - label="Locales", - all_label="All locales", - help_text="Applicable only if you don't select All", - queryset=Locale.objects.all(), - to_field_name="code", - ) - countries = CustomModelMultipleChoiceField( - required=False, - label="Countries", - all_label="All countries", - help_text="Applicable only if you don't select All", - queryset=Country.objects.all(), - to_field_name="code", - ) - platform = forms.ChoiceField( - required=False, - label="Platform", - help_text=Experiment.PLATFORM_HELP_TEXT, - choices=Experiment.PLATFORM_CHOICES, - widget=forms.Select(attrs={"class": "form-control"}), - ) - client_matching = forms.CharField( - required=False, - label="Population Filtering", - help_text=Experiment.CLIENT_MATCHING_HELP_TEXT, - widget=forms.Textarea(attrs={"class": "form-control", "rows": 10}), - ) - # See https://developer.snapappointments.com/bootstrap-select/examples/ - # for more options that relate to the initial rendering of the HTML - # as a way to customize how it works. - locales.widget.attrs.update({"data-live-search": "true"}) - countries.widget.attrs.update({"data-live-search": "true"}) - - class Meta: - model = Experiment - fields = [ - "proposed_start_date", - "proposed_duration", - "proposed_enrollment", - "rollout_playbook", - "population_percent", - "firefox_min_version", - "firefox_max_version", - "firefox_channel", - "locales", - "countries", - "platform", - "client_matching", - ] - - def __init__(self, *args, **kwargs): - data = kwargs.pop("data", None) - instance = kwargs.pop("instance", None) - if instance: - # The reason we must do this is because the form fields - # for locales and countries don't know about the instance - # not having anything set, and we want the "All" option to - # appear in the generated HTML widget. - kwargs.setdefault("initial", {}) - if not instance.locales.all().exists(): - kwargs["initial"]["locales"] = [CustomModelMultipleChoiceField.ALL_KEY] - if not instance.countries.all().exists(): - kwargs["initial"]["countries"] = [CustomModelMultipleChoiceField.ALL_KEY] - super().__init__(data=data, instance=instance, *args, **kwargs) - - def clean_population_percent(self): - population_percent = self.cleaned_data.get("population_percent") - - if population_percent and not (0 < population_percent <= 100): - raise forms.ValidationError( - "The population size must be between 0 and 100 percent." - ) - - return population_percent - - def clean_firefox_max_version(self): - firefox_min_version = self.cleaned_data.get("firefox_min_version") - firefox_max_version = self.cleaned_data.get("firefox_max_version") - - if firefox_min_version and firefox_max_version: - if firefox_max_version < firefox_min_version: - raise forms.ValidationError( - "The max version must be larger than or equal to the min version." - ) - - return firefox_max_version - - def clean_proposed_start_date(self): - start_date = self.cleaned_data.get("proposed_start_date") - - if start_date and start_date < timezone.now().date(): - raise forms.ValidationError( - "The delivery start date must be no earlier than the current date." - ) - - return start_date - - def clean(self): - cleaned_data = super().clean() - - # enrollment may be None - enrollment = cleaned_data.get("proposed_enrollment") - duration = cleaned_data.get("proposed_duration") - - if (enrollment and duration) and enrollment > duration: - msg = ( - "Enrollment duration is optional, but if set, " - "must be lower than the delivery duration. " - "If enrollment duration is not specified - users " - "are enrolled for the entire delivery." - ) - self._errors["proposed_enrollment"] = [msg] - - return cleaned_data - - def save(self, *args, **kwargs): - experiment = super().save(*args, **kwargs) - - if self.instance.is_rollout: - rollout_size = self.instance.rollout_dates.get( - "first_increase" - ) or self.instance.rollout_dates.get("final_increase") - - if rollout_size: - experiment.population_percent = decimal.Decimal(rollout_size["percent"]) - experiment.save() - - return experiment - - class RadioWidget(forms.widgets.RadioSelect): template_name = "experiments/radio_widget.html" diff --git a/app/experimenter/experiments/tests/serializers/test_timeline_population.py b/app/experimenter/experiments/tests/serializers/test_timeline_population.py index 0efccda163..4010b60896 100644 --- a/app/experimenter/experiments/tests/serializers/test_timeline_population.py +++ b/app/experimenter/experiments/tests/serializers/test_timeline_population.py @@ -41,7 +41,8 @@ def setUp(self): super().setUp() self.locale = LocaleFactory.create() self.country = CountryFactory.create() - self.experiment = ExperimentFactory.create( + self.experiment = ExperimentFactory.create_with_status( + ExperimentConstants.STATUS_DRAFT, type=ExperimentConstants.TYPE_PREF, locales=[self.locale], countries=[self.country], @@ -49,7 +50,6 @@ def setUp(self): ) def test_serializer_outputs_expected_schema_pref(self): - serializer = ExperimentTimelinePopSerializer(self.experiment) self.assertEqual( @@ -74,9 +74,7 @@ def test_serializer_outputs_expected_schema_pref(self): def test_all_values_are_optional(self): data = {} - serializer = ExperimentTimelinePopSerializer(instance=self.experiment, data=data) - self.assertTrue(serializer.is_valid()) def test_serializer_saves_values(self): @@ -107,7 +105,7 @@ def test_serializer_saves_values(self): experiment = serializer.save() - self.assertEqual(experiment.changes.count(), 1) + self.assertEqual(experiment.changes.count(), 2) self.assertEqual(experiment.proposed_start_date, data["proposed_start_date"]) self.assertEqual(experiment.proposed_duration, data["proposed_duration"]) self.assertEqual(experiment.proposed_enrollment, data["proposed_enrollment"]) diff --git a/app/experimenter/experiments/tests/test_forms.py b/app/experimenter/experiments/tests/test_forms.py index 953550b9fd..874f49530e 100644 --- a/app/experimenter/experiments/tests/test_forms.py +++ b/app/experimenter/experiments/tests/test_forms.py @@ -1,12 +1,9 @@ -import datetime -import decimal import json from django import forms from django.conf import settings from django.core.exceptions import ValidationError from django.test import TestCase, override_settings -from django.utils import timezone from faker import Factory as FakerFactory from django.contrib.auth.models import Permission from django.contrib.contenttypes.models import ContentType @@ -15,7 +12,6 @@ BugzillaURLField, DSIssueURLField, ChangeLogMixin, - CustomModelMultipleChoiceField, ExperimentArchiveForm, ExperimentCommentForm, ExperimentObjectivesForm, @@ -27,15 +23,12 @@ ExperimentResultsForm, JSONField, NormandyIdForm, - ExperimentTimelinePopulationForm, ExperimentOrderingForm, ) from experimenter.experiments.models import Experiment from experimenter.experiments.tests.factories import ( ExperimentFactory, UserFactory, - CountryFactory, - LocaleFactory, ProjectFactory, ) from experimenter.bugzilla.tests.mixins import MockBugzillaMixin @@ -175,25 +168,28 @@ def test_changelog_values(self): experiment = Experiment() experiment.save() - country1 = CountryFactory(code="CA", name="Canada") - country2 = CountryFactory(code="US", name="United States") - locale1 = LocaleFactory(code="da", name="Danish") - locale2 = LocaleFactory(code="de", name="German") + ds_url = "{base}DS-123".format(base=settings.DS_ISSUE_HOST) + bug_url = "{base}show_bug.cgi?id=123".format(base=settings.BUGZILLA_HOST) + related_exp = ExperimentFactory.create() + project = ProjectFactory.create() data = { - "proposed_start_date": timezone.now().date(), - "proposed_duration": 20, - "population_percent": "10", - "firefox_min_version": "56.0", - "firefox_max_version": "58.0", - "firefox_channel": Experiment.CHANNEL_BETA, - "client_matching": "en-us", - "platform": Experiment.PLATFORM_WINDOWS, - "locales": [locale1, locale2], - "countries": [country1, country2], + "type": Experiment.TYPE_PREF, + "name": "A new experiment!", + "short_description": "Let us learn new things", + "data_science_issue_url": ds_url, + "owner": self.user.id, + "analysis_owner": self.user.id, + "engineering_owner": "Lisa the Engineer", + "public_name": "A new public experiment!", + "public_description": "Let us learn new public things", + "related_to": [related_exp], + "feature_bugzilla_url": bug_url, + "related_work": "Designs: https://www.example.com/myproject/", + "projects": [project], } - form = ExperimentTimelinePopulationForm( + form = ExperimentOverviewForm( request=self.request, data=data, instance=experiment ) self.assertTrue(form.is_valid()) @@ -201,58 +197,68 @@ def test_changelog_values(self): latest_changes = experiment.changes.latest() expected_data = { - "locales": { - "new_value": ["da", "de"], + "analysis_owner": { + "display_name": "Data Science Owner", + "new_value": experiment.analysis_owner.id, + "old_value": None, + }, + "data_science_issue_url": { + "display_name": "Data Science Issue URL", + "new_value": "https://jira.example.com/browse/DS-123", + "old_value": None, + }, + "engineering_owner": { + "display_name": "Engineering Owner", + "new_value": "Lisa the Engineer", "old_value": None, - "display_name": "Locales", }, - "platform": { - "new_value": "All Windows", + "feature_bugzilla_url": { + "display_name": "Feature Bugzilla URL", + "new_value": "https://bugzilla.allizom.org/show_bug.cgi?id=123", "old_value": None, - "display_name": "Platform", }, - "countries": { - "new_value": ["CA", "US"], + "name": { + "display_name": "Name", + "new_value": "A new experiment!", "old_value": None, - "display_name": "Countries", }, - "client_matching": { - "new_value": "en-us", + "owner": { + "display_name": "Delivery Owner", + "new_value": experiment.owner.id, "old_value": None, - "display_name": "Population Filtering", }, - "firefox_channel": { - "new_value": "Beta", + "projects": { + "display_name": "Related Projects", + "new_value": [{"slug": project.slug}], "old_value": None, - "display_name": "Firefox Channel", }, - "proposed_duration": { - "new_value": 20, + "public_description": { + "display_name": "Public Description", + "new_value": "Let us learn new public things", "old_value": None, - "display_name": "Proposed Total Duration (days)", }, - "population_percent": { - "new_value": "10.0000", + "public_name": { + "display_name": "Public Name", + "new_value": "A new public experiment!", "old_value": None, - "display_name": "Population Percentage", }, - "firefox_min_version": { - "new_value": "56.0", + "related_to": { + "display_name": "Related Deliveries", + "new_value": [related_exp.id], "old_value": None, - "display_name": "Firefox Min Version", }, - "firefox_max_version": { - "new_value": "58.0", + "related_work": { + "display_name": "Related Work URLs", + "new_value": "Designs: https://www.example.com/myproject/", "old_value": None, - "display_name": "Firefox Max Version", }, - "proposed_start_date": { - "new_value": timezone.now().date().strftime("%Y-%m-%d"), + "short_description": { + "display_name": "Description", + "new_value": "Let us learn new things", "old_value": None, - "display_name": "Proposed Start Date", }, } - + self.maxDiff = None self.assertEqual(expected_data, latest_changes.changed_values) @@ -386,328 +392,6 @@ def test_invalid_bugzilla_url(self): self.assertFalse(form.is_valid()) -class TestExperimentTimelinePopulationForm(MockRequestMixin, TestCase): - - def setUp(self): - super().setUp() - - self.data = { - "proposed_start_date": timezone.now().date(), - "proposed_duration": 20, - "proposed_enrollment": 10, - "population_percent": "10.0", - "firefox_channel": Experiment.CHANNEL_NIGHTLY, - "firefox_min_version": "67.0", - "firefox_max_version": "69.0", - "locales": [], - "countries": [], - "platform": Experiment.PLATFORM_WINDOWS, - "client_matching": "en-us", - } - - def test_no_fields_required(self): - experiment = ExperimentFactory.create() - data = { - "proposed_start_date": "", - "proposed_duration": "", - "proposed_enrollment": "", - "population_percent": "", - "firefox_channel": "", - "firefox_min_version": "", - "firefox_max_version": "", - "locales": [], - "countries": [], - "platform": "", - "client_matching": "", - } - form = ExperimentTimelinePopulationForm( - request=self.request, data=data, instance=experiment - ) - self.assertTrue(form.is_valid()) - experiment = form.save() - - def test_form_saves_experiment_timeline_population_data(self): - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, countries=[], locales=[] - ) - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - - self.assertEqual(experiment.changes.count(), 1) - - experiment = form.save() - - self.assertEqual(experiment.proposed_start_date, self.data["proposed_start_date"]) - self.assertEqual(experiment.proposed_duration, self.data["proposed_duration"]) - self.assertEqual(experiment.proposed_enrollment, self.data["proposed_enrollment"]) - self.assertEqual(experiment.population_percent, decimal.Decimal("10.000")) - self.assertEqual(experiment.firefox_channel, self.data["firefox_channel"]) - self.assertEqual(experiment.firefox_min_version, self.data["firefox_min_version"]) - self.assertEqual(experiment.firefox_max_version, self.data["firefox_max_version"]) - self.assertEqual(experiment.platform, self.data["platform"]) - self.assertEqual(experiment.client_matching, self.data["client_matching"]) - - self.assertEqual(experiment.changes.count(), 2) - - def test_form_saves_rollout_timeline_population_data(self): - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, - type=Experiment.TYPE_ROLLOUT, - countries=[], - locales=[], - ) - - data = { - "proposed_start_date": timezone.now().date(), - "proposed_duration": 20, - "rollout_playbook": Experiment.ROLLOUT_PLAYBOOK_LOW_RISK, - "firefox_channel": Experiment.CHANNEL_NIGHTLY, - "firefox_min_version": "67.0", - "firefox_max_version": "69.0", - "locales": [], - "countries": [], - } - - form = ExperimentTimelinePopulationForm( - request=self.request, data=data, instance=experiment - ) - - self.assertEqual(experiment.changes.count(), 1) - - experiment = form.save() - - self.assertEqual(experiment.proposed_start_date, data["proposed_start_date"]) - self.assertEqual(experiment.firefox_min_version, data["firefox_min_version"]) - self.assertEqual(experiment.firefox_max_version, data["firefox_max_version"]) - self.assertEqual(experiment.firefox_channel, data["firefox_channel"]) - self.assertEqual(experiment.population_percent, decimal.Decimal("25.000")) - - self.assertEqual(experiment.changes.count(), 2) - - def test_enrollment_must_be_less_or_equal_duration(self): - self.data["proposed_enrollment"] = 2 - self.data["proposed_duration"] = 1 - - form = ExperimentTimelinePopulationForm(request=self.request, data=self.data) - self.assertFalse(form.is_valid()) - - def test_large_duration_is_invalid(self): - self.data["proposed_duration"] = Experiment.MAX_DURATION + 1 - - form = ExperimentTimelinePopulationForm(request=self.request, data=self.data) - self.assertFalse(form.is_valid()) - - def test_large_enrollment_duration_is_invalid(self): - self.data["proposed_enrollment"] = Experiment.MAX_DURATION + 1 - - form = ExperimentTimelinePopulationForm(request=self.request, data=self.data) - self.assertFalse(form.is_valid()) - - def test_start_date_must_be_greater_or_equal_to_current_date(self): - experiment = ExperimentFactory.create_with_status(Experiment.STATUS_DRAFT) - self.data["proposed_start_date"] = timezone.now().date() - datetime.timedelta( - days=1 - ) - - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertFalse(form.is_valid()) - - def test_locales_choices(self): - locale1 = LocaleFactory(code="sv-SE", name="Swedish") - locale2 = LocaleFactory(code="fr", name="French") - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, countries=[], locales=[] - ) - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertEqual( - list(form.fields["locales"].choices), - [ - (CustomModelMultipleChoiceField.ALL_KEY, "All locales"), - (locale2.code, str(locale2)), - (locale1.code, str(locale1)), - ], - ) - - def test_locales_initials(self): - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, locales=[] - ) - locale1 = LocaleFactory(code="sv-SE", name="Swedish") - locale2 = LocaleFactory(code="fr", name="French") - experiment.locales.add(locale1) - experiment.locales.add(locale2) - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertEqual(form.initial["locales"], [locale2, locale1]) - - def test_locales_initials_all_locales(self): - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, locales=[] - ) - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertEqual( - form.initial["locales"], [CustomModelMultipleChoiceField.ALL_KEY] - ) - - def test_clean_locales(self): - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, num_variants=0 - ) - locale1 = LocaleFactory(code="sv-SE", name="Swedish") - locale2 = LocaleFactory(code="fr", name="French") - self.data["locales"] = [locale2.code, locale1.code] - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertTrue(form.is_valid()) - self.assertEqual(set(form.cleaned_data["locales"]), set([locale2, locale1])) - - def test_clean_locales_all(self): - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, num_variants=0 - ) - locale1 = LocaleFactory(code="sv-SE", name="Swedish") - locale2 = LocaleFactory(code="fr", name="French") - self.data["locales"] = [ - locale2.code, - CustomModelMultipleChoiceField.ALL_KEY, - locale1.code, - ] - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertTrue(form.is_valid()) - self.assertEqual(list(form.cleaned_data["locales"]), []) - - def test_clean_unrecognized_locales(self): - experiment = ExperimentFactory.create_with_status(Experiment.STATUS_DRAFT) - self.data["locales"] = ["xxx"] - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertTrue(not form.is_valid()) - self.assertTrue(form.errors["locales"]) - - def test_countries_choices(self): - country1 = CountryFactory(code="SV", name="Sweden") - country2 = CountryFactory(code="FR", name="France") - - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, countries=[], locales=[] - ) - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertEqual( - list(form.fields["countries"].choices), - [ - (CustomModelMultipleChoiceField.ALL_KEY, "All countries"), - (country2.code, str(country2)), - (country1.code, str(country1)), - ], - ) - - def test_countries_initials(self): - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, countries=[] - ) - country1 = CountryFactory(code="SV", name="Sweden") - country2 = CountryFactory(code="FR", name="France") - experiment.countries.add(country1) - experiment.countries.add(country2) - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertEqual(form.initial["countries"], [country2, country1]) - - def test_countries_initials_all(self): - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, num_variants=0, countries=[] - ) - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertEqual( - form.initial["countries"], [CustomModelMultipleChoiceField.ALL_KEY] - ) - - def test_clean_countries(self): - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, num_variants=0 - ) - country1 = CountryFactory(code="SV", name="Sweden") - country2 = CountryFactory(code="FR", name="France") - self.data["countries"] = [country1.code, country2.code] - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertTrue(form.is_valid()) - self.assertEqual( - # form.cleaned_data["countries"] is a QuerySet to exhaust it. - list(form.cleaned_data["countries"]), - [country2, country1], - ) - - def test_clean_countries_all(self): - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, num_variants=0 - ) - country1 = CountryFactory(code="SV", name="Sweden") - country2 = CountryFactory(code="FR", name="France") - self.data["countries"] = [ - country1.code, - CustomModelMultipleChoiceField.ALL_KEY, - country2.code, - ] - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertTrue(form.is_valid()) - self.assertEqual(list(form.cleaned_data["countries"]), []) - - def test_clean_unrecognized_countries(self): - experiment = ExperimentFactory.create_with_status(Experiment.STATUS_DRAFT) - self.data["countries"] = ["xxx"] - form = ExperimentTimelinePopulationForm( - request=self.request, data=self.data, instance=experiment - ) - self.assertTrue(not form.is_valid()) - self.assertTrue(form.errors["countries"]) - - def test_form_is_invalid_if_population_percent_below_0(self): - self.data["population_percent"] = "-1" - form = ExperimentTimelinePopulationForm(request=self.request, data=self.data) - self.assertFalse(form.is_valid()) - self.assertIn("population_percent", form.errors) - - def test_form_is_invalid_if_population_percent_above_100(self): - self.data["population_percent"] = "101" - form = ExperimentTimelinePopulationForm(request=self.request, data=self.data) - self.assertFalse(form.is_valid()) - self.assertIn("population_percent", form.errors) - - def test_form_is_invalid_if_firefox_max_is_lower_than_min(self): - self.data["firefox_min_version"] = "66.0" - self.data["firefox_max_version"] = "64.0" - form = ExperimentTimelinePopulationForm(request=self.request, data=self.data) - self.assertFalse(form.is_valid()) - self.assertIn("firefox_max_version", form.errors) - - def test_form_is_valid_if_firefox_max_is_equal_to_min(self): - self.data["firefox_min_version"] = "66.0" - self.data["firefox_max_version"] = "66.0" - form = ExperimentTimelinePopulationForm(request=self.request, data=self.data) - self.assertTrue(form.is_valid()) - - class TestExperimentObjectivesForm(MockRequestMixin, TestCase): def test_no_fields_required(self): diff --git a/app/experimenter/experiments/tests/test_views.py b/app/experimenter/experiments/tests/test_views.py index f33499cdde..a8ef43c826 100644 --- a/app/experimenter/experiments/tests/test_views.py +++ b/app/experimenter/experiments/tests/test_views.py @@ -1,5 +1,3 @@ -import datetime -import decimal import random import re import json @@ -10,7 +8,6 @@ from django.conf import settings from django.test import TestCase, override_settings from django.urls import reverse -from django.utils import timezone from experimenter.experiments.forms import NormandyIdForm from experimenter.experiments.models import Experiment, Country, Locale @@ -398,60 +395,6 @@ def test_get_view_returns_context(self): self.assertEqual(json.loads(context["countries"]), countries) self.assertEqual(json.loads(context["locales"]), locales) - def test_view_saves_experiment(self): - user_email = "user@example.com" - experiment = ExperimentFactory.create_with_status( - Experiment.STATUS_DRAFT, proposed_enrollment=1, proposed_duration=2 - ) - locale = LocaleFactory() - country = CountryFactory() - - new_start_date = timezone.now().date() + datetime.timedelta( - days=random.randint(1, 100) - ) - new_enrollment = experiment.proposed_enrollment + 1 - new_duration = experiment.proposed_duration + 1 - - data = { - "action": "continue", - "proposed_start_date": new_start_date, - "proposed_enrollment": new_enrollment, - "proposed_duration": new_duration, - "population_percent": "11", - "firefox_min_version": Experiment.VERSION_CHOICES[-2][0], - "firefox_max_version": Experiment.VERSION_CHOICES[-1][0], - "firefox_channel": Experiment.CHANNEL_NIGHTLY, - "client_matching": "New matching!", - "platform": Experiment.PLATFORM_WINDOWS, - "locales": [locale.code], - "countries": [country.code], - } - - response = self.client.post( - reverse("experiments-timeline-pop-update", kwargs={"slug": experiment.slug}), - data, - **{settings.OPENIDC_EMAIL_HEADER: user_email}, - ) - self.assertEqual(response.status_code, 302) - - experiment = Experiment.objects.get() - - self.assertEqual(experiment.proposed_start_date, new_start_date) - self.assertEqual(experiment.proposed_enrollment, new_enrollment) - self.assertEqual(experiment.proposed_duration, new_duration) - self.assertEqual( - experiment.population_percent, decimal.Decimal(data["population_percent"]) - ) - self.assertEqual(experiment.firefox_min_version, data["firefox_min_version"]) - self.assertEqual(experiment.firefox_max_version, data["firefox_max_version"]) - - self.assertEqual(experiment.firefox_channel, data["firefox_channel"]) - self.assertEqual(experiment.platform, data["platform"]) - - self.assertTrue(locale in experiment.locales.all()) - - self.assertTrue(country in experiment.countries.all()) - class TestExperimentDesignUpdateView(TestCase): diff --git a/app/experimenter/experiments/views.py b/app/experimenter/experiments/views.py index f7a5c6b8c9..021f061989 100644 --- a/app/experimenter/experiments/views.py +++ b/app/experimenter/experiments/views.py @@ -17,7 +17,6 @@ ExperimentRisksForm, ExperimentStatusForm, ExperimentSubscribedForm, - ExperimentTimelinePopulationForm, NormandyIdForm, ExperimentOrderingForm, ) @@ -94,9 +93,8 @@ class ExperimentOverviewUpdateView(ExperimentFormMixin, UpdateView): template_name = "experiments/edit_overview.html" -class ExperimentTimelinePopulationUpdateView(ExperimentFormMixin, UpdateView): - form_class = ExperimentTimelinePopulationForm - next_view_name = "experiments-design-update" +class ExperimentTimelinePopulationUpdateView(DetailView): + model = Experiment template_name = "experiments/edit_timeline_population.html" def get_context_data(self, **kwargs):