Browse Source

chore(issues): Revert-revert of deleting streamline-targeting-context (#65160)

Revert-revert of #64876, with the addition of a bug fix caught in canary
with https://github.com/getsentry/sentry/pull/65104

The canary error came from refreshing the schemas after a
ProjectOwnership is created, but then `raw` is set to None. Now it
should accept None just fine, espcially since thats a valid value for
that field on the model.

**todo**
- [x] Add tests
Leander Rodrigues 1 year ago
parent
commit
5f4c34cf48

+ 5 - 13
src/sentry/api/endpoints/codeowners/__init__.py

@@ -69,20 +69,12 @@ class ProjectCodeOwnerSerializer(CamelSnakeModelSerializer):
         )
 
         # Convert IssueOwner syntax into schema syntax
-        has_targeting_context = features.has(
-            "organizations:streamline-targeting-context", self.context["project"].organization
-        )
         try:
-            if has_targeting_context:
-                validated_data = create_schema_from_issue_owners(
-                    issue_owners=issue_owner_rules,
-                    project_id=self.context["project"].id,
-                    add_owner_ids=True,
-                )
-            else:
-                validated_data = create_schema_from_issue_owners(
-                    issue_owners=issue_owner_rules, project_id=self.context["project"].id
-                )
+            validated_data = create_schema_from_issue_owners(
+                issue_owners=issue_owner_rules,
+                project_id=self.context["project"].id,
+                add_owner_ids=True,
+            )
             return {
                 **attrs,
                 "schema": validated_data,

+ 28 - 36
src/sentry/api/endpoints/codeowners/index.py

@@ -3,7 +3,7 @@ from rest_framework.exceptions import PermissionDenied
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry import analytics, features
+from sentry import analytics
 from sentry.api.api_owners import ApiOwner
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
@@ -26,28 +26,31 @@ class ProjectCodeOwnersEndpoint(ProjectEndpoint, ProjectCodeOwnersMixin):
         "POST": ApiPublishStatus.PRIVATE,
     }
 
-    def add_owner_id_to_schema(self, codeowner: ProjectCodeOwners, project: Project) -> None:
-        if not hasattr(codeowner, "schema") or (
-            codeowner.schema
-            and codeowner.schema.get("rules")
-            and "id" not in codeowner.schema["rules"][0]["owners"][0].keys()
+    def refresh_codeowners_schema(self, codeowner: ProjectCodeOwners, project: Project) -> None:
+        if hasattr(codeowner, "schema") and (
+            codeowner.schema is None or codeowner.schema.get("rules") is None
         ):
-            # Convert raw to issue owners syntax so that the schema can be created
-            raw = codeowner.raw
-            associations, _ = validate_codeowners_associations(codeowner.raw, project)
-            codeowner.raw = convert_codeowners_syntax(
-                codeowner.raw,
-                associations,
-                codeowner.repository_project_path_config,
-            )
-            codeowner.schema = create_schema_from_issue_owners(
-                codeowner.raw, project.id, add_owner_ids=True, remove_deleted_owners=True
-            )
+            return
+
+        # Convert raw to issue owners syntax so that the schema can be created
+        raw = codeowner.raw
+        associations, _ = validate_codeowners_associations(codeowner.raw, project)
+        codeowner.raw = convert_codeowners_syntax(
+            codeowner.raw,
+            associations,
+            codeowner.repository_project_path_config,
+        )
+        codeowner.schema = create_schema_from_issue_owners(
+            project_id=project.id,
+            issue_owners=codeowner.raw,
+            add_owner_ids=True,
+            remove_deleted_owners=True,
+        )
 
-            # Convert raw back to codeowner type to be saved
-            codeowner.raw = raw
+        # Convert raw back to codeowner type to be saved
+        codeowner.raw = raw
 
-            codeowner.save()
+        codeowner.save()
 
     def get(self, request: Request, project: Project) -> Response:
         """
@@ -65,17 +68,11 @@ class ProjectCodeOwnersEndpoint(ProjectEndpoint, ProjectCodeOwnersMixin):
         expand = request.GET.getlist("expand", [])
         expand.append("errors")
 
-        has_targeting_context = features.has(
-            "organizations:streamline-targeting-context", project.organization
-        )
-
         codeowners = list(ProjectCodeOwners.objects.filter(project=project).order_by("-date_added"))
-
-        if has_targeting_context and codeowners:
-            for codeowner in codeowners:
-                self.add_owner_id_to_schema(codeowner, project)
-            expand.append("renameIdentifier")
-            expand.append("hasTargetingContext")
+        for codeowner in codeowners:
+            self.refresh_codeowners_schema(codeowner, project)
+        expand.append("renameIdentifier")
+        expand.append("hasTargetingContext")
 
         return Response(
             serialize(
@@ -115,12 +112,7 @@ class ProjectCodeOwnersEndpoint(ProjectEndpoint, ProjectCodeOwnersMixin):
                 codeowners_id=project_codeowners.id,
             )
 
-            expand = ["ownershipSyntax", "errors"]
-            has_targeting_context = features.has(
-                "organizations:streamline-targeting-context", project.organization
-            )
-            if has_targeting_context:
-                expand.append("hasTargetingContext")
+            expand = ["ownershipSyntax", "errors", "hasTargetingContext"]
 
             return Response(
                 serialize(

+ 23 - 36
src/sentry/api/endpoints/project_ownership.py

@@ -92,19 +92,13 @@ class ProjectOwnershipRequestSerializer(serializers.Serializer):
                 {"raw": f"Raw needs to be <= {max_length} characters in length"}
             )
 
-        if features.has(
-            "organizations:streamline-targeting-context",
-            self.context["ownership"].project.organization,
-        ):
-            schema = create_schema_from_issue_owners(
-                attrs["raw"], self.context["ownership"].project_id, add_owner_ids=True
-            )
-        else:
-            schema = create_schema_from_issue_owners(
-                attrs["raw"], self.context["ownership"].project_id
-            )
-
-        self._validate_no_codeowners(schema["rules"])
+        schema = create_schema_from_issue_owners(
+            project_id=self.context["ownership"].project_id,
+            issue_owners=attrs["raw"],
+            add_owner_ids=True,
+        )
+        if schema:
+            self._validate_no_codeowners(schema["rules"])
 
         attrs["schema"] = schema
         return attrs
@@ -202,16 +196,19 @@ class ProjectOwnershipEndpoint(ProjectEndpoint):
                 last_updated=None,
             )
 
-    def add_owner_id_to_schema(self, ownership: ProjectOwnership, project: Project) -> None:
-        if not hasattr(ownership, "schema") or (
-            ownership.schema
-            and ownership.schema.get("rules")
-            and "id" not in ownership.schema["rules"][0]["owners"][0].keys()
+    def refresh_ownership_schema(self, ownership: ProjectOwnership, project: Project) -> None:
+        if hasattr(ownership, "schema") and (
+            ownership.schema is None or ownership.schema.get("rules") is None
         ):
-            ownership.schema = create_schema_from_issue_owners(
-                ownership.raw, project.id, add_owner_ids=True, remove_deleted_owners=True
-            )
-            ownership.save()
+            return
+
+        ownership.schema = create_schema_from_issue_owners(
+            project_id=project.id,
+            issue_owners=ownership.raw,
+            add_owner_ids=True,
+            remove_deleted_owners=True,
+        )
+        ownership.save()
 
     def rename_schema_identifier_for_parsing(self, ownership: ProjectOwnership) -> None:
         """
@@ -240,17 +237,12 @@ class ProjectOwnershipEndpoint(ProjectEndpoint):
         Returns details on a project's ownership configuration.
         """
         ownership = self.get_ownership(project)
-        should_return_schema = features.has(
-            "organizations:streamline-targeting-context", project.organization
-        )
 
-        if should_return_schema and ownership:
-            self.add_owner_id_to_schema(ownership, project)
+        if ownership:
+            self.refresh_ownership_schema(ownership, project)
             self.rename_schema_identifier_for_parsing(ownership)
 
-        return Response(
-            serialize(ownership, request.user, should_return_schema=should_return_schema)
-        )
+        return Response(serialize(ownership, request.user))
 
     @extend_schema(
         operation_id="Update Ownership Configuration for a Project",
@@ -280,9 +272,6 @@ class ProjectOwnershipEndpoint(ProjectEndpoint):
         if list(request.data) != ["raw"] and not has_project_write:
             raise PermissionDenied
 
-        should_return_schema = features.has(
-            "organizations:streamline-targeting-context", project.organization
-        )
         serializer = ProjectOwnershipRequestSerializer(
             data=request.data, partial=True, context={"ownership": self.get_ownership(project)}
         )
@@ -305,7 +294,5 @@ class ProjectOwnershipEndpoint(ProjectEndpoint):
                 data={**change_data, **project.get_audit_log_data()},
             )
             ownership_rule_created.send_robust(project=project, sender=self.__class__)
-            return Response(
-                serialize(ownership, request.user, should_return_schema=should_return_schema)
-            )
+            return Response(serialize(ownership, request.user))
         return Response(serializer.errors, status=400)

+ 7 - 9
src/sentry/api/serializers/models/projectownership.py

@@ -23,15 +23,15 @@ class ProjectOwnershipResponse(ProjectOwnershipResponseOptional):
 
 @register(ProjectOwnership)
 class ProjectOwnershipSerializer(Serializer):
-    def serialize(
-        self, obj, attrs, user, should_return_schema=False, **kwargs
-    ) -> ProjectOwnershipResponse:
+    def serialize(self, obj, attrs, user, **kwargs) -> ProjectOwnershipResponse:
         assignment = (
             "Auto Assign to Suspect Commits"
             if obj.auto_assignment and obj.suspect_committer_auto_assignment
-            else "Auto Assign to Issue Owner"
-            if obj.auto_assignment and not obj.suspect_committer_auto_assignment
-            else "Turn off Auto-Assignment"
+            else (
+                "Auto Assign to Issue Owner"
+                if obj.auto_assignment and not obj.suspect_committer_auto_assignment
+                else "Turn off Auto-Assignment"
+            )
         )
 
         project_ownership_data: ProjectOwnershipResponse = {
@@ -43,8 +43,6 @@ class ProjectOwnershipSerializer(Serializer):
             "autoAssignment": assignment,
             "codeownersAutoSync": obj.codeowners_auto_sync,
         }
-
-        if should_return_schema:
-            project_ownership_data["schema"] = obj.schema
+        project_ownership_data["schema"] = obj.schema
 
         return project_ownership_data

+ 0 - 1
src/sentry/apidocs/examples/project_examples.py

@@ -213,7 +213,6 @@ DETAILED_PROJECT = {
             "performance-metrics-backed-transaction-summary",
             "performance-db-main-thread-detector",
             "issue-platform",
-            "streamline-targeting-context",
             "performance-consecutive-db-issue",
             "performance-consecutive-http-post-process-group",
             "performance-n-plus-one-api-calls-detector",

+ 0 - 3
src/sentry/conf/server.py

@@ -1436,7 +1436,6 @@ SENTRY_EARLY_FEATURES = {
     "organizations:source-maps-debugger-blue-thunder-edition": "Enable source maps debugger",
     "organizations:sourcemaps-bundle-flat-file-indexing": "Enable the new flat file indexing system for sourcemaps.",
     "organizations:sourcemaps-upload-release-as-artifact-bundle": "Upload release bundles as artifact bundles",
-    "organizations:streamline-targeting-context": "Enable the new suggested assignees feature",
     "organizations:user-feedback-ui": "Enable User Feedback v2 UI",
 }
 
@@ -1909,8 +1908,6 @@ SENTRY_FEATURES: dict[str, bool | None] = {
     "organizations:starfish-view": False,
     # Enable starfish dropdown on the webservice view for switching chart visualization
     "organizations:starfish-wsv-chart-dropdown": False,
-    # Enable the new suggested assignees feature
-    "organizations:streamline-targeting-context": False,
     # Allow organizations to configure all symbol sources.
     "organizations:symbol-sources": True,
     # Enable team insights page

+ 0 - 1
src/sentry/features/__init__.py

@@ -271,7 +271,6 @@ default_manager.add("organizations:starfish-mobile-appstart", OrganizationFeatur
 default_manager.add("organizations:starfish-test-endpoint", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
 default_manager.add("organizations:starfish-view", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
 default_manager.add("organizations:starfish-wsv-chart-dropdown", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
-default_manager.add("organizations:streamline-targeting-context", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
 default_manager.add("organizations:symbol-sources", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
 default_manager.add("organizations:team-workflow-notifications", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
 default_manager.add("organizations:trace-view-load-more", OrganizationFeature, FeatureHandlerStrategy.REMOTE)

+ 7 - 10
src/sentry/integrations/slack/message_builder/issues.py

@@ -336,16 +336,13 @@ def get_suggested_assignees(
     ):  # we don't want every user in the project to be a suggested assignee
         resolved_owners = ActorTuple.resolve_many(issue_owners)
         suggested_assignees = RpcActor.many_from_object(resolved_owners)
-    if features.has("organizations:streamline-targeting-context", project.organization):
-        try:
-            suspect_commit_users = RpcActor.many_from_object(
-                get_suspect_commit_users(project, event)
-            )
-            suggested_assignees.extend(suspect_commit_users)
-        except (Release.DoesNotExist, Commit.DoesNotExist):
-            logger.info("Skipping suspect committers because release does not exist.")
-        except Exception:
-            logger.exception("Could not get suspect committers. Continuing execution.")
+    try:
+        suspect_commit_users = RpcActor.many_from_object(get_suspect_commit_users(project, event))
+        suggested_assignees.extend(suspect_commit_users)
+    except (Release.DoesNotExist, Commit.DoesNotExist):
+        logger.info("Skipping suspect committers because release does not exist.")
+    except Exception:
+        logger.exception("Could not get suspect committers. Continuing execution.")
     if suggested_assignees:
         suggested_assignees = dedupe_suggested_assignees(suggested_assignees)
         assignee_texts = []

+ 1 - 1
src/sentry/models/projectcodeowners.py

@@ -120,7 +120,7 @@ class ProjectCodeOwners(Model):
         # Convert IssueOwner syntax into schema syntax
         try:
             schema = create_schema_from_issue_owners(
-                issue_owners=issue_owner_rules, project_id=self.project.id
+                project_id=self.project.id, issue_owners=issue_owner_rules
             )
             # Convert IssueOwner syntax into schema syntax
             if schema:

+ 15 - 13
src/sentry/notifications/utils/participants.py

@@ -338,23 +338,25 @@ def determine_eligible_recipients(
             suggested_assignees.append(assignee_actor)
 
         suspect_commit_users = None
-        if features.has("organizations:streamline-targeting-context", project.organization):
-            try:
-                suspect_commit_users = RpcActor.many_from_object(
-                    get_suspect_commit_users(project, event)
-                )
-                suggested_assignees.extend(suspect_commit_users)
-            except (Release.DoesNotExist, Commit.DoesNotExist):
-                logger.info("Skipping suspect committers because release does not exist.")
-            except Exception:
-                logger.exception("Could not get suspect committers. Continuing execution.")
+
+        try:
+            suspect_commit_users = RpcActor.many_from_object(
+                get_suspect_commit_users(project, event)
+            )
+            suggested_assignees.extend(suspect_commit_users)
+        except (Release.DoesNotExist, Commit.DoesNotExist):
+            logger.info("Skipping suspect committers because release does not exist.")
+        except Exception:
+            logger.exception("Could not get suspect committers. Continuing execution.")
 
         metrics.incr(
             "features.owners.send_to",
             tags={
-                "outcome": outcome
-                if outcome == "match" or fallthrough_choice is None
-                else fallthrough_choice.value,
+                "outcome": (
+                    outcome
+                    if outcome == "match" or fallthrough_choice is None
+                    else fallthrough_choice.value
+                ),
                 "hasSuspectCommitters": str(bool(suspect_commit_users)),
             },
         )

Some files were not shown because too many files changed in this diff