Skip to content

Commit 6fda473

Browse files
fix(nimbus): omit monitoring data when cloning experiments (#15248)
Because * Cloned experiments carried over monitoring data from the parent * Monitoring data is specific to the parent experiment's lifecycle and is not relevant to the new clone This commit * Nulls out `monitoring_data` during the clone step * Adds a data migration to clear `monitoring_data` on non-Live/Complete experiments * Updates clone test to set `monitoring_data` on parent and assert it is `None` on the child * Updates migration test to cover the new data migration Fixes #15247
1 parent 2717275 commit 6fda473

4 files changed

Lines changed: 43 additions & 73 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
from django.db import migrations
2+
3+
4+
def clear_monitoring_data(apps, schema_editor):
5+
NimbusExperiment = apps.get_model("experiments", "NimbusExperiment")
6+
NimbusExperiment.objects.exclude(status__in=["Live", "Complete"]).update(
7+
monitoring_data=None
8+
)
9+
10+
11+
class Migration(migrations.Migration):
12+
dependencies = [
13+
("experiments", "0326_alter_nimbusexperiment_next_steps_and_more"),
14+
]
15+
16+
operations = [
17+
migrations.RunPython(clear_monitoring_data),
18+
]

experimenter/experimenter/experiments/models.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2079,6 +2079,7 @@ def clone(self, name, user, rollout_branch_slug=None, changed_on=None):
20792079
cloned.published_dto = None
20802080
cloned.published_date = None
20812081
cloned.results_data = None
2082+
cloned.monitoring_data = None
20822083
cloned.takeaways_summary = None
20832084
cloned.next_steps = None
20842085
cloned.project_impact = None
Lines changed: 22 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,104 +1,53 @@
11
from django_test_migrations.contrib.unittest_case import MigratorTestCase
22

33

4-
class TestRemoveVpnApplicationsMigration(MigratorTestCase):
4+
class TestClearMonitoringDataMigration(MigratorTestCase):
55
migrate_from = (
66
"experiments",
7-
"0318_alter_nimbusexperiment_application_and_more",
7+
"0326_alter_nimbusexperiment_next_steps_and_more",
88
)
99
migrate_to = (
1010
"experiments",
11-
"0319_delete_vpn_features",
11+
"0327_clear_monitoring_data_non_live_complete",
1212
)
1313

1414
def prepare(self):
1515
User = self.old_state.apps.get_model("auth", "User")
16-
NimbusFeatureConfig = self.old_state.apps.get_model(
17-
"experiments", "NimbusFeatureConfig"
18-
)
1916
NimbusExperiment = self.old_state.apps.get_model(
2017
"experiments", "NimbusExperiment"
2118
)
22-
NimbusIsolationGroup = self.old_state.apps.get_model(
23-
"experiments", "NimbusIsolationGroup"
24-
)
2519

2620
owner, _ = User.objects.get_or_create(
2721
username="test@example.com",
2822
defaults={"email": "test@example.com"},
2923
)
3024

31-
NimbusFeatureConfig.objects.get_or_create(
32-
slug="no-feature-vpn-web",
33-
defaults={
34-
"name": "No Feature VPN Web",
35-
"application": "vpn-web",
36-
},
37-
)
38-
39-
NimbusFeatureConfig.objects.get_or_create(
40-
slug="no-feature-fenix",
41-
defaults={
42-
"name": "No Feature Fenix",
43-
"application": "fenix",
44-
},
45-
)
46-
47-
NimbusExperiment.objects.get_or_create(
48-
slug="test-vpn-web",
49-
defaults={
50-
"name": "Test VPN Web",
51-
"application": "vpn-web",
52-
"owner": owner,
53-
},
54-
)
25+
self.monitoring_data = {"total_enrollments": 100, "total_unenrollments": 5}
5526

56-
NimbusExperiment.objects.get_or_create(
57-
slug="test-fenix",
58-
defaults={
59-
"name": "Test Fenix",
60-
"application": "fenix",
61-
"owner": owner,
62-
},
63-
)
64-
65-
NimbusIsolationGroup.objects.get_or_create(
66-
application="vpn-web",
67-
name="test-group",
68-
instance=1,
69-
)
70-
71-
NimbusIsolationGroup.objects.get_or_create(
72-
application="fenix",
73-
name="test-group",
74-
instance=1,
75-
)
27+
for status in ("Draft", "Preview", "Live", "Complete"):
28+
NimbusExperiment.objects.create(
29+
slug=f"test-{status.lower()}",
30+
name=f"Test {status}",
31+
application="firefox-desktop",
32+
owner=owner,
33+
status=status,
34+
monitoring_data=self.monitoring_data,
35+
)
7636

7737
def test_migration(self):
78-
NimbusFeatureConfig = self.new_state.apps.get_model(
79-
"experiments", "NimbusFeatureConfig"
80-
)
8138
NimbusExperiment = self.new_state.apps.get_model(
8239
"experiments", "NimbusExperiment"
8340
)
84-
NimbusIsolationGroup = self.new_state.apps.get_model(
85-
"experiments", "NimbusIsolationGroup"
86-
)
8741

88-
self.assertFalse(
89-
NimbusFeatureConfig.objects.filter(slug="no-feature-vpn-web").exists()
42+
self.assertIsNone(NimbusExperiment.objects.get(slug="test-draft").monitoring_data)
43+
self.assertIsNone(
44+
NimbusExperiment.objects.get(slug="test-preview").monitoring_data
9045
)
91-
92-
self.assertTrue(
93-
NimbusFeatureConfig.objects.filter(slug="no-feature-fenix").exists()
46+
self.assertEqual(
47+
NimbusExperiment.objects.get(slug="test-live").monitoring_data,
48+
self.monitoring_data,
9449
)
95-
96-
self.assertFalse(NimbusExperiment.objects.filter(application="vpn-web").exists())
97-
98-
self.assertTrue(NimbusExperiment.objects.filter(application="fenix").exists())
99-
100-
self.assertFalse(
101-
NimbusIsolationGroup.objects.filter(application="vpn-web").exists()
50+
self.assertEqual(
51+
NimbusExperiment.objects.get(slug="test-complete").monitoring_data,
52+
self.monitoring_data,
10253
)
103-
104-
self.assertTrue(NimbusIsolationGroup.objects.filter(application="fenix").exists())

experimenter/experimenter/experiments/tests/test_models.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4093,6 +4093,7 @@ def test_clone_created_experiment(self):
40934093
application=NimbusExperiment.Application.DESKTOP,
40944094
conclusion_recommendations=["RERUN", "STOP"],
40954095
takeaways_summary="takeaway",
4096+
monitoring_data={"total_enrollments": 500, "total_unenrollments": 10},
40964097
)
40974098
NimbusExperimentBranchThroughRequired.objects.create(
40984099
parent_experiment=parent,
@@ -4216,6 +4217,7 @@ def _clone_experiment_and_assert_common_expectations(
42164217
self.assertEqual(child.published_dto, None)
42174218
self.assertEqual(child.published_date, None)
42184219
self.assertEqual(child.results_data, None)
4220+
self.assertEqual(child.monitoring_data, None)
42194221
self.assertEqual(child.takeaways_gain_amount, None)
42204222
self.assertEqual(child.takeaways_metric_gain, False)
42214223
self.assertEqual(child.takeaways_qbr_learning, False)

0 commit comments

Comments
 (0)