diff --git a/experimenter/experimenter/experiments/constants.py b/experimenter/experimenter/experiments/constants.py index f601607f2..02f18a2e5 100644 --- a/experimenter/experimenter/experiments/constants.py +++ b/experimenter/experimenter/experiments/constants.py @@ -915,7 +915,8 @@ class FirefoxLabsGroups(models.TextChoices): DAILY_ACTIVE_USERS = "client_level_daily_active_users_v2" DAYS_OF_USE = "days_of_use" RETENTION = "retained" - RETENTION_3_DAYS = "active_in_last_3_days_legacy" + RETENTION_3_DAYS = "active_in_last_3_days" + RETENTION_3_DAYS_DESKTOP = "active_in_last_3_days_legacy" SEARCH_COUNT = "search_count" DAU_METRIC = { diff --git a/experimenter/experimenter/jetstream/results_manager.py b/experimenter/experimenter/jetstream/results_manager.py index c68c5fc95..f26fc4e0e 100644 --- a/experimenter/experimenter/jetstream/results_manager.py +++ b/experimenter/experimenter/jetstream/results_manager.py @@ -316,16 +316,14 @@ def check_valid_point(point): def get_kpi_metrics( self, analysis_basis, segment, reference_branch, window="overall" ): + kpi_metrics = [m.copy() for m in NimbusConstants.KPI_METRICS] - kpi_metrics = NimbusConstants.KPI_METRICS.copy() - - # 3-day retention is only available for Desktop experiments - if self.experiment.application != self.experiment.Application.DESKTOP: - kpi_metrics = [ - m - for m in kpi_metrics - if m.get("slug") != NimbusConstants.RETENTION_3_DAYS - ] + # For desktop experiments, use legacy version of metric until Glean migration + if self.experiment.application == self.experiment.Application.DESKTOP: + for m in kpi_metrics: + if m.get("slug") == NimbusConstants.RETENTION_3_DAYS: + m["slug"] = NimbusConstants.RETENTION_3_DAYS_DESKTOP + break window_results = self.get_window_results(analysis_basis, segment, window) other_metrics = ( diff --git a/experimenter/experimenter/jetstream/tests/test_results_manager.py b/experimenter/experimenter/jetstream/tests/test_results_manager.py index 9950718a5..59a6a20bb 100644 --- a/experimenter/experimenter/jetstream/tests/test_results_manager.py +++ b/experimenter/experimenter/jetstream/tests/test_results_manager.py @@ -2642,33 +2642,53 @@ def test_metrics_have_data_different_windows(self, results_data, expected): expected, ) - def test_get_kpi_metrics_excludes_3day_retention_for_non_desktop(self): - fenix_outcome = Outcomes.get_by_slug_and_application( - "fenix_outcome", NimbusExperiment.Application.FENIX - ) - fenix_experiment = NimbusExperimentFactory.create( - application=NimbusExperiment.Application.FENIX, - primary_outcomes=[fenix_outcome.slug], - secondary_outcomes=[], + @parameterized.expand( + [ + ( + NimbusExperiment.Application.DESKTOP, + [ + "client_level_daily_active_users_v2", + "retained", + "search_count", + "active_in_last_3_days_legacy", + ], + ), + ( + NimbusExperiment.Application.FENIX, + [ + "client_level_daily_active_users_v2", + "retained", + "search_count", + "active_in_last_3_days", + ], + ), + ( + NimbusExperiment.Application.IOS, + [ + "client_level_daily_active_users_v2", + "retained", + "search_count", + "active_in_last_3_days", + ], + ), + ] + ) + def test_get_kpi_metrics_by_application(self, application, expected_metric_slugs): + experiment = NimbusExperimentFactory.create( + application=application, ) NimbusBranchFactory.create( - experiment=fenix_experiment, name="Branch A", slug="branch-a" + experiment=experiment, name="Branch A", slug="branch-a" ) NimbusBranchFactory.create( - experiment=fenix_experiment, name="Branch B", slug="branch-b" + experiment=experiment, name="Branch B", slug="branch-b" ) - fenix_results_manager = ExperimentResultsManager(fenix_experiment) - kpi_metrics = fenix_results_manager.get_kpi_metrics( - "enrollments", "all", "branch-a" - ) + results_manager = ExperimentResultsManager(experiment) + kpi_metrics = results_manager.get_kpi_metrics("enrollments", "all", "branch-a") slugs = [metric["slug"] for metric in kpi_metrics] - self.assertNotIn("active_in_last_3_days_legacy", slugs) - - # Verify base KPI metrics are still present - self.assertIn("retained", slugs) - self.assertIn("search_count", slugs) + self.assertEqual(set(slugs), set(expected_metric_slugs)) @parameterized.expand( [