feat(nimbus): add a ready for review debug api field (#11783)

Because * We previously disabled the ready for review serializer after an experiment is live for performance reasons * We now need to inspect the ready for review serliaizer output for live experiments that may have launched without validation This commit * Moves the live check out of the ready for review serializer and into the graphql field * Adds a second graphql field that is not used by the frontend but can be manually invoked to run the ready for review serializer fixes #11782
2024-11-15 18:29:29 -05:00 · 2024-11-15 18:29:29 -05:00 · d534f5197d
--- a/experimenter/experimenter/experiments/api/v5/serializers.py
+++ b/experimenter/experimenter/experiments/api/v5/serializers.py
@ -2217,28 +2217,27 @@ class NimbusReviewSerializer(serializers.ModelSerializer):
        return data

    def validate(self, data):
-        if self.instance.status == self.instance.Status.DRAFT:
-            application = data.get("application")
-            channel = data.get("channel")
-            if application != NimbusExperiment.Application.DESKTOP and not channel:
-                raise serializers.ValidationError(
-                    {"channel": "Channel is required for this application."}
-                )
-            data = super().validate(data)
-            data = self._validate_versions(data)
-            data = self._validate_localizations(data)
-            data = self._validate_feature_configs(data)
-            data = self._validate_enrollment_targeting(data)
-            data = self._validate_sticky_enrollment(data)
-            data = self._validate_rollout_version_support(data)
-            data = self._validate_bucket_duplicates(data)
-            data = self._validate_proposed_release_date(data)
-            if application == NimbusExperiment.Application.DESKTOP:
-                data = self._validate_desktop_pref_rollouts(data)
-                data = self._validate_desktop_pref_flips(data)
-            else:
-                data = self._validate_languages_versions(data)
-                data = self._validate_countries_versions(data)
+        application = data.get("application")
+        channel = data.get("channel")
+        if application != NimbusExperiment.Application.DESKTOP and not channel:
+            raise serializers.ValidationError(
+                {"channel": "Channel is required for this application."}
+            )
+        data = super().validate(data)
+        data = self._validate_versions(data)
+        data = self._validate_localizations(data)
+        data = self._validate_feature_configs(data)
+        data = self._validate_enrollment_targeting(data)
+        data = self._validate_sticky_enrollment(data)
+        data = self._validate_rollout_version_support(data)
+        data = self._validate_bucket_duplicates(data)
+        data = self._validate_proposed_release_date(data)
+        if application == NimbusExperiment.Application.DESKTOP:
+            data = self._validate_desktop_pref_rollouts(data)
+            data = self._validate_desktop_pref_flips(data)
+        else:
+            data = self._validate_languages_versions(data)
+            data = self._validate_countries_versions(data)
        return data


--- a/experimenter/experimenter/experiments/api/v5/types.py
+++ b/experimenter/experimenter/experiments/api/v5/types.py
@ -573,6 +573,7 @@ class NimbusExperimentType(DjangoObjectType):
    qa_signoff = graphene.NonNull(graphene.Boolean)
    qa_status = NimbusExperimentQAStatusEnum()
    ready_for_review = graphene.Field(NimbusReviewType)
+    ready_for_review_debug = graphene.Field(NimbusReviewType)
    recipe_json = graphene.String()
    reference_branch = graphene.Field(NimbusBranchType)
    rejection = graphene.Field(NimbusChangeLogType)
@ -658,6 +659,7 @@ class NimbusExperimentType(DjangoObjectType):
            "qa_comment",
            "qa_status",
            "ready_for_review",
+            "ready_for_review_debug",
            "recipe_json",
            "reference_branch",
            "rejection",
@ -708,6 +710,25 @@ class NimbusExperimentType(DjangoObjectType):
        return [NimbusBranch(name=NimbusConstants.DEFAULT_TREATMENT_BRANCH_NAME)]

    def resolve_ready_for_review(self, info):
+        if self.status == self.Status.DRAFT:
+            serializer = NimbusReviewSerializer(
+                self,
+                data=NimbusReviewSerializer(self).data,
+            )
+            ready = serializer.is_valid()
+            return NimbusReviewType(
+                message=serializer.errors,
+                warnings=serializer.warnings,
+                ready=ready,
+            )
+        else:
+            return NimbusReviewType(
+                message={},
+                warnings={},
+                ready=True,
+            )
+
+    def resolve_ready_for_review_debug(self, info):
        serializer = NimbusReviewSerializer(
            self,
            data=NimbusReviewSerializer(self).data,
--- a/experimenter/experimenter/experiments/tests/api/v5/test_queries.py
+++ b/experimenter/experimenter/experiments/tests/api/v5/test_queries.py
@ -600,6 +600,12 @@ class TestNimbusExperimentBySlugQuery(GraphQLTestCase):
            data=NimbusReviewSerializer(experiment).data,
        )
        review_ready = review_serializer.is_valid()
+        review_errors = review_serializer.errors
+        review_warnings = review_serializer.warnings
+        if experiment.status != NimbusExperiment.Status.DRAFT:
+            review_ready = True
+            review_errors = {}
+            review_warnings = {}

        response = self.query(
            """
@ -722,6 +728,12 @@ class TestNimbusExperimentBySlugQuery(GraphQLTestCase):
                        warnings
                    }

+                    readyForReviewDebug {
+                        ready
+                        message
+                        warnings
+                    }
+
                    startDate
                    computedDurationDays
                    computedEndDate
@ -933,8 +945,13 @@ class TestNimbusExperimentBySlugQuery(GraphQLTestCase):
                ).name,
                "qaStatus": NimbusExperiment.QAStatus(experiment.qa_status).name,
                "readyForReview": {
-                    "message": review_serializer.errors,
+                    "message": review_errors,
                    "ready": review_ready,
+                    "warnings": review_warnings,
+                },
+                "readyForReviewDebug": {
+                    "message": review_serializer.errors,
+                    "ready": review_serializer.is_valid(),
                    "warnings": review_serializer.warnings,
                },
                "recipeJson": json.dumps(
--- a/experimenter/experimenter/experiments/tests/api/v5/test_serializers/test_nimbus_ready_for_review_serializer.py
+++ b/experimenter/experimenter/experiments/tests/api/v5/test_serializers/test_nimbus_ready_for_review_serializer.py
@ -4734,62 +4734,6 @@ class TestNimbusReviewSerializerMultiFeature(MockFmlErrorMixin, TestCase):
                },
            )

-    @parameterized.expand(
-        [
-            (NimbusExperimentFactory.Lifecycles.CREATED, False),
-            (NimbusExperimentFactory.Lifecycles.PREVIEW, True),
-            (NimbusExperimentFactory.Lifecycles.LIVE_APPROVE_APPROVE, True),
-            (NimbusExperimentFactory.Lifecycles.ENDING_APPROVE_APPROVE, True),
-        ]
-    )
-    def test_review_failures_are_skipped_for_non_draft(self, lifecycle, expected_valid):
-        experiment = NimbusExperimentFactory.create_with_lifecycle(
-            lifecycle,
-            application=NimbusExperiment.Application.FENIX,
-            channel=NimbusExperiment.Channel.RELEASE,
-            feature_configs=[
-                NimbusFeatureConfigFactory.create(
-                    application=NimbusExperiment.Application.FENIX,
-                    schemas=[
-                        NimbusVersionedSchemaFactory.build(
-                            version=None,
-                            schema=None,
-                        )
-                    ],
-                ),
-                NimbusFeatureConfigFactory.create(
-                    application=NimbusExperiment.Application.IOS,
-                    schemas=[
-                        NimbusVersionedSchemaFactory.build(
-                            version=None,
-                            schema=None,
-                        )
-                    ],
-                ),
-            ],
-            is_sticky=True,
-            firefox_min_version=NimbusExperiment.MIN_REQUIRED_VERSION,
-        )
-
-        serializer = NimbusReviewSerializer(
-            experiment,
-            data=NimbusReviewSerializer(
-                experiment,
-                context={"user": self.user},
-            ).data,
-            context={"user": self.user},
-        )
-
-        self.assertEqual(serializer.is_valid(), expected_valid)
-        if not expected_valid:
-            self.assertEqual(
-                serializer.errors["feature_configs"],
-                [
-                    "Feature Config application ios does not "
-                    "match experiment application fenix."
-                ],
-            )
-
    @parameterized.expand(
        [
            ({"feature-1": "bogus-collection"},),
--- a/experimenter/experimenter/nimbus-ui/schema.graphql
+++ b/experimenter/experimenter/nimbus-ui/schema.graphql
@ -87,6 +87,7 @@ type NimbusExperimentType {
  monitoringDashboardUrl: String
  qaSignoff: Boolean!
  readyForReview: NimbusReviewType
+  readyForReviewDebug: NimbusReviewType
  recipeJson: String
  rejection: NimbusChangeLogType
  requiredExperimentsBranches: [NimbusExperimentBranchThroughRequiredType!]!