Skip to content

Commit

Permalink
fix #7440 feat(nimbus): migrate sticky targeting configs
Browse files Browse the repository at this point in the history
Because

* Now that the sticky targeting clause is moved right into the NimbusExperiment targeting method
* There's no need for the sticky clause to be included in individual targeting configs

This commit

* Removes the sticky clauses from all desktop and mobile targeting configs
* Removes the now redundant 'pip_never_used_sticky' targeting config and migrates experiments that used it to 'pip_never_used'
* Migrates all existing experiments that use a previously sticky targeting config to have is_sticky=True
  • Loading branch information
jaredlockhart committed Jun 22, 2022
1 parent 064e853 commit 03ccecb
Show file tree
Hide file tree
Showing 5 changed files with 362 additions and 235 deletions.
55 changes: 55 additions & 0 deletions app/experimenter/experiments/migrations/0214_migrate_sticky.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Generated by Django 3.2.13 on 2022-06-21 20:01

from django.db import migrations


def migrate_sticky_experiments(apps, schema_editor):
NimbusExperiment = apps.get_model("experiments", "NimbusExperiment")

NimbusExperiment.objects.filter(
targeting_config_slug__in=[
"first_run",
"first_run_chrome",
"first_run_win1903",
"not_tcp_study_first_run",
"windows_userchoice_first_run",
"infrequent_user_uris",
"infrequent_user_need_pin",
"infrequent_user_need_default",
"infrequent_user_need_default_has_pin",
"infrequent_user_has_default_need_pin",
"infrequent_windows_user_need_pin",
"infrequent_win_user_uris",
"infrequent_user_5_bookmarks",
"casual_user_uris",
"casual_user_need_pin",
"casual_user_need_default",
"casual_user_need_default_has_pin",
"casual_user_has_default_need_pin",
"regular_user_uris",
"regular_user_need_pin",
"regular_user_need_default",
"regular_user_need_default_has_pin",
"regular_user_has_default_need_pin",
"regular_user_uses_fxa",
"core_user_uris",
"pip_never_used_sticky",
"mobile_new_users",
"mobile_recently_updated_users",
]
).update(is_sticky=True)

NimbusExperiment.objects.filter(targeting_config_slug="pip_never_used_sticky").update(
targeting_config_slug="pip_never_used"
)


class Migration(migrations.Migration):

dependencies = [
("experiments", "0213_alter_nimbusexperiment_languages_field_for_mobile_client"),
]

operations = [
migrations.RunPython(migrate_sticky_experiments),
]
200 changes: 142 additions & 58 deletions app/experimenter/experiments/tests/test_migrations.py
Original file line number Diff line number Diff line change
@@ -1,105 +1,189 @@
from django_test_migrations.contrib.unittest_case import MigratorTestCase

from experimenter.base.models import Language, Locale
from experimenter.experiments.constants import NimbusConstants
from experimenter.experiments.models import NimbusExperiment as Experiment

DESKTOP_STICKY_TARGETING_SLUGS = [
"first_run",
"first_run_chrome",
"first_run_win1903",
"not_tcp_study_first_run",
"windows_userchoice_first_run",
"infrequent_user_uris",
"infrequent_user_need_pin",
"infrequent_user_need_default",
"infrequent_user_need_default_has_pin",
"infrequent_user_has_default_need_pin",
"infrequent_windows_user_need_pin",
"infrequent_win_user_uris",
"infrequent_user_5_bookmarks",
"casual_user_uris",
"casual_user_need_pin",
"casual_user_need_default",
"casual_user_need_default_has_pin",
"casual_user_has_default_need_pin",
"regular_user_uris",
"regular_user_need_pin",
"regular_user_need_default",
"regular_user_need_default_has_pin",
"regular_user_has_default_need_pin",
"regular_user_uses_fxa",
"core_user_uris",
]

MOBILE_STICKY_TARGETING_SLUGS = [
"mobile_new_users",
"mobile_recently_updated_users",
]

NON_STICKY_TARGETING_SLUGS = [
"core_user_has_default_need_pin",
"core_user_need_default",
"core_user_need_default_has_pin",
"core_user_need_pin",
"fx95_desktop_users",
"homepage_google_dot_com",
"mac_only",
"no_enterprise_or_last_30d_vpn_use",
"no_enterprise_or_past_vpn",
"no_enterprise_users",
"no_targeting",
"not_tcp_study",
"pocket_common",
"rally_core_addon_user",
"urlbar_firefox_suggest",
"windows_userchoice",
]


class TestMigration(MigratorTestCase):
migrate_from = ("experiments", "0212_nimbusexperiment_is_sticky")
migrate_to = (
migrate_from = (
"experiments",
"0213_alter_nimbusexperiment_languages_field_for_mobile_client",
)
migrate_to = (
"experiments",
"0214_migrate_sticky",
)

def prepare(self):
"""Prepare some data before the migration."""
User = self.old_state.apps.get_model("auth", "User")
Locale = self.old_state.apps.get_model("base", "Locale")
Language = self.old_state.apps.get_model("base", "Language")
NimbusExperiment = self.old_state.apps.get_model(
"experiments", "NimbusExperiment"
)
user = User.objects.create(email="test@example.com")
locale_en = Locale.objects.create(code="en", name="English")
Language.objects.create(code="en", name="English")
locale_ts = Locale.objects.create(code="ts", name="does not exist")
locale_ca = Locale.objects.create(code="en-CA", name="English Canada")

desktop_experiment = NimbusExperiment.objects.create(
owner=user,
name="desktop experiment",
slug="desktop_experiment",
application=Experiment.Application.DESKTOP,
status=NimbusConstants.Status.DRAFT,
)
desktop_experiment.locales.add(locale_en.id)
mobile_experiments_with_locales = NimbusExperiment.objects.create(
owner=user,
name="mobile experiment with locales",
slug="mobile_experiment_with_locales",
application=Experiment.Application.FOCUS_ANDROID,
status=NimbusConstants.Status.DRAFT,
)
mobile_experiments_with_locales.locales.add(locale_en.id, locale_ts.id, locale_ca)
# Desktop experiments that will keep the same targeting slug
# but have is_sticky set to True
for desktop_sticky_targeting in DESKTOP_STICKY_TARGETING_SLUGS:
NimbusExperiment.objects.create(
owner=user,
name=desktop_sticky_targeting,
slug=desktop_sticky_targeting,
application=Experiment.Application.DESKTOP,
status=NimbusConstants.Status.DRAFT,
targeting_config_slug=desktop_sticky_targeting,
is_sticky=False,
)

# Targeting slug 'pip_never_used_sticky' will get replaced with 'pip_never_used'
# and is_sticky set to True
NimbusExperiment.objects.create(
owner=user,
name="mobile experiment without locales",
slug="mobile_experiment_without_locales",
application=Experiment.Application.FOCUS_IOS,
name="pip_never_used_sticky",
slug="pip_never_used_sticky",
application=Experiment.Application.DESKTOP,
status=NimbusConstants.Status.DRAFT,
targeting_config_slug="pip_never_used_sticky",
is_sticky=False,
)

mobile_experiments_with_locales_but_not_in_draft = (
# Mobile experiments that will keep the same targeting slug
# but have is_sticky set to True
for mobile_sticky_targeting in MOBILE_STICKY_TARGETING_SLUGS:
NimbusExperiment.objects.create(
owner=user,
name="mobile experiment not in draft with locales",
slug="mobile_experiment_not_in_draft_with_locales",
application=Experiment.Application.IOS,
status=NimbusConstants.Status.COMPLETE,
name=mobile_sticky_targeting,
slug=mobile_sticky_targeting,
application=Experiment.Application.FOCUS_ANDROID,
status=NimbusConstants.Status.DRAFT,
targeting_config_slug=mobile_sticky_targeting,
is_sticky=False,
)

# Desktop experiments that will keep the same targeting slug
# and keep is_sticky set to False
for non_sticky_targeting in NON_STICKY_TARGETING_SLUGS:
NimbusExperiment.objects.create(
owner=user,
name=non_sticky_targeting,
slug=non_sticky_targeting,
application=Experiment.Application.DESKTOP,
status=NimbusConstants.Status.DRAFT,
targeting_config_slug=non_sticky_targeting,
is_sticky=False,
)
)
mobile_experiments_with_locales_but_not_in_draft.locales.add(locale_en.id)

def test_migration(self):
"""Run the test itself."""
NimbusExperiment = self.new_state.apps.get_model(
"experiments", "NimbusExperiment"
)

# get desktop experiment
desktop_experiment = NimbusExperiment.objects.get(
application=Experiment.Application.DESKTOP
# Desktop experiments that keep the same sticky targeting slug
# now have is_sticky set to True
self.assertEqual(
NimbusExperiment.objects.filter(
slug__in=DESKTOP_STICKY_TARGETING_SLUGS, is_sticky=True
).count(),
len(DESKTOP_STICKY_TARGETING_SLUGS),
)

self.assertEqual(
desktop_experiment.locales.all()[0].code, Locale.objects.get(code="en").code
NimbusExperiment.objects.filter(
slug__in=DESKTOP_STICKY_TARGETING_SLUGS, is_sticky=False
).count(),
0,
)

# get mobile experiment included locale field
mobile_experiment_with_locales = NimbusExperiment.objects.get(
application=Experiment.Application.FOCUS_ANDROID
# 'pip_never_used_sticky' becomes 'pip_never_used' with is_sticky set to True
self.assertFalse(
NimbusExperiment.objects.filter(
targeting_config_slug="pip_never_used_sticky"
).exists()
)
self.assertEqual(mobile_experiment_with_locales.locales.count(), 0)
self.assertEqual(
mobile_experiment_with_locales.languages.all()[0].code,
Language.objects.get(code="en").code,
self.assertTrue(
NimbusExperiment.objects.filter(
targeting_config_slug="pip_never_used", is_sticky=True
).exists()
)

# get mobile experiment without locale field
mobile_experiment_with_locales = NimbusExperiment.objects.get(
application=Experiment.Application.FOCUS_IOS
# Mobile experiments that keep the same sticky targeting slug
# now have is_sticky set to True
self.assertEqual(
NimbusExperiment.objects.filter(
slug__in=MOBILE_STICKY_TARGETING_SLUGS, is_sticky=True
).count(),
len(MOBILE_STICKY_TARGETING_SLUGS),
)
self.assertEqual(mobile_experiment_with_locales.locales.count(), 0)
self.assertEqual(mobile_experiment_with_locales.languages.count(), 0)

# get mobile experiment without locale field which is not in draft
mobile_experiments_with_locales_but_not_in_draft = NimbusExperiment.objects.get(
application=Experiment.Application.IOS
self.assertEqual(
NimbusExperiment.objects.filter(
slug__in=MOBILE_STICKY_TARGETING_SLUGS, is_sticky=False
).count(),
0,
)

# Desktop experiments that keep the same non-sticky targeting slug
# still have is_sticky set to False
self.assertEqual(
NimbusExperiment.objects.filter(
slug__in=NON_STICKY_TARGETING_SLUGS, is_sticky=True
).count(),
0,
)
self.assertEqual(
mobile_experiments_with_locales_but_not_in_draft.locales.all()[0].code,
Locale.objects.get(code="en").code,
NimbusExperiment.objects.filter(
slug__in=NON_STICKY_TARGETING_SLUGS, is_sticky=False
).count(),
len(NON_STICKY_TARGETING_SLUGS),
)
9 changes: 3 additions & 6 deletions app/experimenter/experiments/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,10 +565,7 @@ def test_targeting_with_languages_mobile(self):
)
self.assertEqual(
experiment.targeting,
(
"(is_already_enrolled || days_since_install < 7) "
"&& (language in ['en', 'es', 'fr'])"
),
"(days_since_install < 7) && (language in ['en', 'es', 'fr'])",
)
JEXLParser().parse(experiment.targeting)

Expand Down Expand Up @@ -632,7 +629,7 @@ def test_targeting_with_sticky_mobile(self):
"(is_already_enrolled) "
"|| "
"("
"(is_already_enrolled || days_since_install < 7) "
"(days_since_install < 7) "
"&& (app_version|versionCompare('100.!') >= 0) "
"&& (language in ['en']) "
"&& (region in ['CA'])"
Expand Down Expand Up @@ -668,7 +665,7 @@ def test_targeting_with_locales_languages_mobile(self):
self.assertEqual(
experiment.targeting,
(
"(is_already_enrolled || days_since_install < 7) "
"(days_since_install < 7) "
"&& ('de' in locale || 'en' in locale || 'es' in locale "
"|| 'ro' in locale) "
"&& (language in ['en', 'es', 'fr'])"
Expand Down
Loading

0 comments on commit 03ccecb

Please sign in to comment.