From bc6205d73fe10d46c8c9d0db86974f363ab96221 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Fri, 27 Mar 2026 15:30:46 -0700 Subject: [PATCH 1/6] feat(ACI): Release rule stats and group history wfe endpoints (#111737) Add a flag to release the rule stats and group history workflow engine versions of the endpoints. These complement the `ProjectRuleDetailsEndpoint` which is [about to be released to GA](https://github.com/getsentry/sentry-options-automator/pull/7008) as they provide additional context for rendering the page. --- src/sentry/features/temporary.py | 3 +++ .../rules/history/endpoints/project_rule_group_history.py | 3 +++ src/sentry/rules/history/endpoints/project_rule_stats.py | 3 +++ tests/sentry/rules/history/backends/test_postgres.py | 4 ++++ 4 files changed, 13 insertions(+) diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index ebfd7086146fc2..5fed3691df84d1 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -436,6 +436,9 @@ def register_temporary_features(manager: FeatureManager) -> None: # Use workflow engine exclusively for OrganizationCombinedRuleIndexEndpoint.get results. # See src/sentry/workflow_engine/docs/legacy_backport.md for context. manager.add("organizations:workflow-engine-combinedruleindex-get", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) + # Use workflow engine exclusively for ProjectRuleGroupHistoryIndexEndpoint.get and ProjectRuleStatsIndexEndpoint.get results. + # See src/sentry/workflow_engine/docs/legacy_backport.md for context. + manager.add("organizations:workflow-engine-projectrulegroupstats-get", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False) # Enable metric detector limits by plan type manager.add("organizations:workflow-engine-metric-detector-limit", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable EventUniqueUserFrequencyConditionWithConditions special alert condition diff --git a/src/sentry/rules/history/endpoints/project_rule_group_history.py b/src/sentry/rules/history/endpoints/project_rule_group_history.py index b4bed3c291982a..800fa7a196a326 100644 --- a/src/sentry/rules/history/endpoints/project_rule_group_history.py +++ b/src/sentry/rules/history/endpoints/project_rule_group_history.py @@ -57,6 +57,9 @@ def serialize( @extend_schema(tags=["issue_alerts"]) @cell_silo_endpoint class ProjectRuleGroupHistoryIndexEndpoint(WorkflowEngineRuleEndpoint): + workflow_engine_method_flags = { + "GET": "organizations:workflow-engine-projectrulegroupstats-get", + } publish_status = { "GET": ApiPublishStatus.EXPERIMENTAL, } diff --git a/src/sentry/rules/history/endpoints/project_rule_stats.py b/src/sentry/rules/history/endpoints/project_rule_stats.py index 9f45aca88a1086..3738dee4ab8042 100644 --- a/src/sentry/rules/history/endpoints/project_rule_stats.py +++ b/src/sentry/rules/history/endpoints/project_rule_stats.py @@ -40,6 +40,9 @@ def serialize( @extend_schema(tags=["issue_alerts"]) @cell_silo_endpoint class ProjectRuleStatsIndexEndpoint(WorkflowEngineRuleEndpoint): + workflow_engine_method_flags = { + "GET": "organizations:workflow-engine-projectrulegroupstats-get", + } publish_status = { "GET": ApiPublishStatus.EXPERIMENTAL, } diff --git a/tests/sentry/rules/history/backends/test_postgres.py b/tests/sentry/rules/history/backends/test_postgres.py index 347112bea52e06..173b1343672c06 100644 --- a/tests/sentry/rules/history/backends/test_postgres.py +++ b/tests/sentry/rules/history/backends/test_postgres.py @@ -197,6 +197,7 @@ def test_event_id(self) -> None: ) @with_feature("organizations:workflow-engine-rule-serializers") + @with_feature("organizations:workflow-engine-projectrulegroupstats-get") def test_single_written_workflow_history(self) -> None: """Test using WorkflowFireHistory when feature flag is enabled""" workflow_triggers = self.create_data_condition_group() @@ -276,6 +277,7 @@ def test_single_written_workflow_history(self) -> None: ) @with_feature("organizations:workflow-engine-rule-serializers") + @with_feature("organizations:workflow-engine-projectrulegroupstats-get") def test_combined_rule_and_workflow_history(self) -> None: """Test combining RuleFireHistory and WorkflowFireHistory when both exist""" rule = self.create_project_rule(project=self.event.project) @@ -416,6 +418,7 @@ def test(self) -> None: assert [r.count for r in results[-5:]] == [0, 0, 1, 1, 0] @with_feature("organizations:workflow-engine-rule-serializers") + @with_feature("organizations:workflow-engine-projectrulegroupstats-get") def test_single_written_workflow_history(self) -> None: """Test using WorkflowFireHistory when feature flag is enabled""" workflow_triggers = self.create_data_condition_group() @@ -452,6 +455,7 @@ def test_single_written_workflow_history(self) -> None: assert [r.count for r in results[-5:]] == [0, 2, 0, 1, 0] @with_feature("organizations:workflow-engine-rule-serializers") + @with_feature("organizations:workflow-engine-projectrulegroupstats-get") def test_combined_rule_and_workflow_history(self) -> None: """Test combining RuleFireHistory and WorkflowFireHistory when both exist""" rule = self.create_project_rule(project=self.event.project) From f870b10b8eec62c86dcddb0be2c5da785f158df8 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Fri, 27 Mar 2026 15:51:36 -0700 Subject: [PATCH 2/6] chore(ACI): Move functions to utils file (#111761) To prep for https://github.com/getsentry/sentry/pull/111760 this PR moves a bunch of functions into a utils file from a file that will soon be deleted. --- .../organization_detector_details.py | 4 +- .../endpoints/organization_detector_index.py | 8 +- .../endpoints/validators/detector_workflow.py | 185 +----------------- .../endpoints/validators/utils.py | 185 +++++++++++++++++- .../test_organization_workflow_index.py | 4 +- 5 files changed, 199 insertions(+), 187 deletions(-) diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index 7a25c0a37a9e64..b4bf4556ae024a 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -35,10 +35,12 @@ from sentry.workflow_engine.endpoints.validators.base import BaseDetectorTypeValidator from sentry.workflow_engine.endpoints.validators.detector_workflow import ( BulkDetectorWorkflowsValidator, +) +from sentry.workflow_engine.endpoints.validators.utils import ( can_delete_detector, can_edit_detector, + get_unknown_detector_type_error, ) -from sentry.workflow_engine.endpoints.validators.utils import get_unknown_detector_type_error from sentry.workflow_engine.models import Detector diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_index.py b/src/sentry/workflow_engine/endpoints/organization_detector_index.py index 2eef8b4ae79742..d236ef4645cbf0 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_index.py @@ -57,13 +57,15 @@ from sentry.workflow_engine.endpoints.validators.base import BaseDetectorTypeValidator from sentry.workflow_engine.endpoints.validators.detector_workflow import ( BulkDetectorWorkflowsValidator, - can_delete_detectors, - can_edit_detectors, ) from sentry.workflow_engine.endpoints.validators.detector_workflow_mutation import ( DetectorWorkflowMutationValidator, ) -from sentry.workflow_engine.endpoints.validators.utils import get_unknown_detector_type_error +from sentry.workflow_engine.endpoints.validators.utils import ( + can_delete_detectors, + can_edit_detectors, + get_unknown_detector_type_error, +) from sentry.workflow_engine.models import Detector from sentry.workflow_engine.models.detector_group import DetectorGroup diff --git a/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py b/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py index 42ed3f589eb709..df5f98f5442904 100644 --- a/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py +++ b/src/sentry/workflow_engine/endpoints/validators/detector_workflow.py @@ -1,189 +1,14 @@ -from collections.abc import Sequence from typing import Any, Literal -from django.db import router, transaction -from django.db.models import QuerySet from rest_framework import serializers -from rest_framework.exceptions import PermissionDenied -from rest_framework.request import Request -from sentry import audit_log from sentry.api.serializers.rest_framework.base import CamelSnakeSerializer -from sentry.grouping.grouptype import ErrorGroupType -from sentry.issue_detection.performance_detection import PERFORMANCE_WFE_DETECTOR_TYPES -from sentry.models.organization import Organization -from sentry.models.project import Project -from sentry.utils.audit import create_audit_entry -from sentry.workflow_engine.models.detector import Detector +from sentry.workflow_engine.endpoints.validators.utils import ( + perform_bulk_detector_workflow_operations, + validate_detectors_exist_and_have_permissions, + validate_workflows_exist, +) from sentry.workflow_engine.models.detector_workflow import DetectorWorkflow -from sentry.workflow_engine.models.workflow import Workflow -from sentry.workflow_engine.typings.grouptype import IssueStreamGroupType - -# Only those with organization write permissions can edit system-created detectors (e.g. error detectors). -SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES = {"org:write"} -USER_CREATED_DETECTOR_REQUIRED_SCOPES = {"org:write", "alerts:write"} - - -def is_system_created_detector(detector: Detector) -> bool: - return ( - detector.type in (ErrorGroupType.slug, IssueStreamGroupType.slug) - or detector.type in PERFORMANCE_WFE_DETECTOR_TYPES - ) - - -def can_edit_system_created_detectors(request: Request, project: Project) -> bool: - return request.access.has_any_project_scope(project, SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES) - - -def can_edit_user_created_detectors(request: Request, project: Project) -> bool: - return request.access.has_any_project_scope(project, USER_CREATED_DETECTOR_REQUIRED_SCOPES) - - -def can_edit_detectors(detectors: QuerySet[Detector], request: Request) -> bool: - """ - Determine if the requesting user has access to edit the given detectors. - System created detectors lock edit access to org:write, while user created detectors - are more permissive. - """ - required_scopes = ( - SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES - if any(is_system_created_detector(detector) for detector in detectors) - else USER_CREATED_DETECTOR_REQUIRED_SCOPES - ) - - projects = Project.objects.filter( - id__in=detectors.values_list("project_id", flat=True).distinct() - ) - - return all( - request.access.has_any_project_scope(project, required_scopes) for project in projects - ) - - -def can_edit_detector(detector: Detector, request: Request) -> bool: - """ - Determine if the requesting user has access to detector edit. If the request does not have the "alerts:write" - permission, then we must verify that the user is a team admin with "alerts:write" access to the project(s) - in their request. - """ - if is_system_created_detector(detector) and not can_edit_system_created_detectors( - request, detector.project - ): - return False - - return can_edit_user_created_detectors(request, detector.project) - - -def can_delete_detectors(detectors: QuerySet[Detector], request: Request) -> bool: - """ - Determine if the requesting user has access to delete the given detectors. - Only user-created detectors can be deleted, and require "alerts:write" permission. - """ - if any(is_system_created_detector(detector) for detector in detectors): - return False - - projects = Project.objects.filter( - id__in=detectors.values_list("project_id", flat=True).distinct() - ) - return all(can_edit_user_created_detectors(request, project) for project in projects) - - -def can_delete_detector(detector: Detector, request: Request) -> bool: - """ - Determine if the requesting user has access to delete the given detector. - Only user-created detectors can be deleted, and require "alerts:write" permission. - """ - if is_system_created_detector(detector): - return False - - return can_edit_user_created_detectors(request, detector.project) - - -def can_edit_detector_workflow_connections(detector: Detector, request: Request) -> bool: - """ - Anyone with alert write access to the project can connect/disconnect detectors of any type, - which is slightly different from full edit access which differs by detector type. - """ - return request.access.has_any_project_scope( - detector.project, USER_CREATED_DETECTOR_REQUIRED_SCOPES - ) - - -def validate_detectors_exist_and_have_permissions( - detector_ids: list[int], organization: Organization, request: Request -) -> QuerySet[Detector]: - detectors = Detector.objects.filter( - project__organization=organization, - id__in=detector_ids, - ) - found_detector_ids = set(detectors.values_list("id", flat=True)) - missing_detector_ids = set(detector_ids) - found_detector_ids - - if missing_detector_ids: - raise serializers.ValidationError(f"Some detectors do not exist: {missing_detector_ids}") - - if not all(can_edit_detector_workflow_connections(detector, request) for detector in detectors): - raise PermissionDenied - - return detectors - - -def validate_workflows_exist( - workflow_ids: list[int], organization: Organization -) -> QuerySet[Workflow]: - workflows = Workflow.objects.filter(organization=organization, id__in=workflow_ids) - found_workflow_ids = set(workflows.values_list("id", flat=True)) - missing_workflow_ids = set(workflow_ids) - found_workflow_ids - - if missing_workflow_ids: - raise serializers.ValidationError(f"Some workflows do not exist: {missing_workflow_ids}") - - return workflows - - -def perform_bulk_detector_workflow_operations( - detector_workflows_to_add: list[dict[Literal["detector_id", "workflow_id"], int]], - detector_workflows_to_remove: Sequence[DetectorWorkflow], - request: Request, - organization: Organization, -) -> list[DetectorWorkflow]: - created_detector_workflows: list[DetectorWorkflow] = [] - - with transaction.atomic(router.db_for_write(DetectorWorkflow)): - if detector_workflows_to_remove: - DetectorWorkflow.objects.filter( - id__in=[detector_workflow.id for detector_workflow in detector_workflows_to_remove] - ).delete() - - if detector_workflows_to_add: - created_detector_workflows = DetectorWorkflow.objects.bulk_create( - [ - DetectorWorkflow( - detector_id=pair["detector_id"], workflow_id=pair["workflow_id"] - ) - for pair in detector_workflows_to_add - ] - ) - - for detector_workflow in detector_workflows_to_remove: - create_audit_entry( - request=request, - organization=organization, - target_object=detector_workflow.id, - event=audit_log.get_event_id("DETECTOR_WORKFLOW_REMOVE"), - data=detector_workflow.get_audit_log_data(), - ) - - for detector_workflow in created_detector_workflows: - create_audit_entry( - request=request, - organization=organization, - target_object=detector_workflow.id, - event=audit_log.get_event_id("DETECTOR_WORKFLOW_ADD"), - data=detector_workflow.get_audit_log_data(), - ) - - return created_detector_workflows class BulkDetectorWorkflowsValidator(CamelSnakeSerializer[Any]): diff --git a/src/sentry/workflow_engine/endpoints/validators/utils.py b/src/sentry/workflow_engine/endpoints/validators/utils.py index fa42d2b3d69580..60a4fa1d32b493 100644 --- a/src/sentry/workflow_engine/endpoints/validators/utils.py +++ b/src/sentry/workflow_engine/endpoints/validators/utils.py @@ -1,20 +1,203 @@ import logging -from typing import Any +from collections.abc import Sequence +from typing import Any, Literal +from django.db import router, transaction +from django.db.models import QuerySet from django.forms import ValidationError from jsonschema import ValidationError as JsonValidationError from jsonschema import validate +from rest_framework import serializers +from rest_framework.exceptions import PermissionDenied +from rest_framework.request import Request +from sentry import audit_log from sentry.issues import grouptype from sentry.models.organization import Organization +from sentry.models.project import Project from sentry.types.actor import Actor from sentry.users.models.user import User from sentry.users.services.user import RpcUser from sentry.utils import metrics +from sentry.utils.audit import create_audit_entry from sentry.workflow_engine.models.detector import Detector +from sentry.workflow_engine.models.detector_workflow import DetectorWorkflow +from sentry.workflow_engine.models.workflow import Workflow logger = logging.getLogger(__name__) +# Only those with organization write permissions can edit system-created detectors (e.g. error detectors). +SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES = {"org:write"} +USER_CREATED_DETECTOR_REQUIRED_SCOPES = {"org:write", "alerts:write"} + + +def is_system_created_detector(detector: Detector) -> bool: + # Lazy imports to avoid circular imports: grouptype imports from validators/base + # which imports from this module. + from sentry.grouping.grouptype import ErrorGroupType + from sentry.issue_detection.performance_detection import PERFORMANCE_WFE_DETECTOR_TYPES + from sentry.workflow_engine.typings.grouptype import IssueStreamGroupType + + return ( + detector.type in (ErrorGroupType.slug, IssueStreamGroupType.slug) + or detector.type in PERFORMANCE_WFE_DETECTOR_TYPES + ) + + +def can_edit_system_created_detectors(request: Request, project: Project) -> bool: + return request.access.has_any_project_scope(project, SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES) + + +def can_edit_user_created_detectors(request: Request, project: Project) -> bool: + return request.access.has_any_project_scope(project, USER_CREATED_DETECTOR_REQUIRED_SCOPES) + + +def can_edit_detectors(detectors: QuerySet[Detector], request: Request) -> bool: + """ + Determine if the requesting user has access to edit the given detectors. + System created detectors lock edit access to org:write, while user created detectors + are more permissive. + """ + required_scopes = ( + SYSTEM_CREATED_DETECTOR_REQUIRED_SCOPES + if any(is_system_created_detector(detector) for detector in detectors) + else USER_CREATED_DETECTOR_REQUIRED_SCOPES + ) + + projects = Project.objects.filter( + id__in=detectors.values_list("project_id", flat=True).distinct() + ) + + return all( + request.access.has_any_project_scope(project, required_scopes) for project in projects + ) + + +def can_edit_detector(detector: Detector, request: Request) -> bool: + """ + Determine if the requesting user has access to detector edit. If the request does not have the "alerts:write" + permission, then we must verify that the user is a team admin with "alerts:write" access to the project(s) + in their request. + """ + if is_system_created_detector(detector) and not can_edit_system_created_detectors( + request, detector.project + ): + return False + + return can_edit_user_created_detectors(request, detector.project) + + +def can_delete_detectors(detectors: QuerySet[Detector], request: Request) -> bool: + """ + Determine if the requesting user has access to delete the given detectors. + Only user-created detectors can be deleted, and require "alerts:write" permission. + """ + if any(is_system_created_detector(detector) for detector in detectors): + return False + + projects = Project.objects.filter( + id__in=detectors.values_list("project_id", flat=True).distinct() + ) + return all(can_edit_user_created_detectors(request, project) for project in projects) + + +def can_delete_detector(detector: Detector, request: Request) -> bool: + """ + Determine if the requesting user has access to delete the given detector. + Only user-created detectors can be deleted, and require "alerts:write" permission. + """ + if is_system_created_detector(detector): + return False + + return can_edit_user_created_detectors(request, detector.project) + + +def can_edit_detector_workflow_connections(detector: Detector, request: Request) -> bool: + """ + Anyone with alert write access to the project can connect/disconnect detectors of any type, + which is slightly different from full edit access which differs by detector type. + """ + return request.access.has_any_project_scope( + detector.project, USER_CREATED_DETECTOR_REQUIRED_SCOPES + ) + + +def validate_detectors_exist_and_have_permissions( + detector_ids: list[int], organization: Organization, request: Request +) -> QuerySet[Detector]: + detectors = Detector.objects.filter( + project__organization=organization, + id__in=detector_ids, + ) + found_detector_ids = set(detectors.values_list("id", flat=True)) + missing_detector_ids = set(detector_ids) - found_detector_ids + + if missing_detector_ids: + raise serializers.ValidationError(f"Some detectors do not exist: {missing_detector_ids}") + + if not all(can_edit_detector_workflow_connections(detector, request) for detector in detectors): + raise PermissionDenied + + return detectors + + +def validate_workflows_exist( + workflow_ids: list[int], organization: Organization +) -> QuerySet[Workflow]: + workflows = Workflow.objects.filter(organization=organization, id__in=workflow_ids) + found_workflow_ids = set(workflows.values_list("id", flat=True)) + missing_workflow_ids = set(workflow_ids) - found_workflow_ids + + if missing_workflow_ids: + raise serializers.ValidationError(f"Some workflows do not exist: {missing_workflow_ids}") + + return workflows + + +def perform_bulk_detector_workflow_operations( + detector_workflows_to_add: list[dict[Literal["detector_id", "workflow_id"], int]], + detector_workflows_to_remove: Sequence[DetectorWorkflow], + request: Request, + organization: Organization, +) -> list[DetectorWorkflow]: + created_detector_workflows: list[DetectorWorkflow] = [] + + with transaction.atomic(router.db_for_write(DetectorWorkflow)): + if detector_workflows_to_remove: + DetectorWorkflow.objects.filter( + id__in=[detector_workflow.id for detector_workflow in detector_workflows_to_remove] + ).delete() + + if detector_workflows_to_add: + created_detector_workflows = DetectorWorkflow.objects.bulk_create( + [ + DetectorWorkflow( + detector_id=pair["detector_id"], workflow_id=pair["workflow_id"] + ) + for pair in detector_workflows_to_add + ] + ) + + for detector_workflow in detector_workflows_to_remove: + create_audit_entry( + request=request, + organization=organization, + target_object=detector_workflow.id, + event=audit_log.get_event_id("DETECTOR_WORKFLOW_REMOVE"), + data=detector_workflow.get_audit_log_data(), + ) + + for detector_workflow in created_detector_workflows: + create_audit_entry( + request=request, + organization=organization, + target_object=detector_workflow.id, + event=audit_log.get_event_id("DETECTOR_WORKFLOW_ADD"), + data=detector_workflow.get_audit_log_data(), + ) + + return created_detector_workflows + def update_owner(owner: Actor | None) -> tuple[int | None, int | None]: if owner: diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py index 6ca40934b42386..2b61ccb4145121 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py @@ -876,7 +876,7 @@ def test_create_workflow__invalid_actions(self) -> None: assert response.status_code == 400 - @mock.patch("sentry.workflow_engine.endpoints.validators.detector_workflow.create_audit_entry") + @mock.patch("sentry.workflow_engine.endpoints.validators.utils.create_audit_entry") def test_create_workflow_with_detector_ids(self, mock_audit: mock.MagicMock) -> None: detector_1 = self.create_detector() detector_2 = self.create_detector() @@ -906,7 +906,7 @@ def test_create_workflow_with_detector_ids(self, mock_audit: mock.MagicMock) -> ] assert len(detector_workflow_audit_calls) == 2 - @mock.patch("sentry.workflow_engine.endpoints.validators.detector_workflow.create_audit_entry") + @mock.patch("sentry.workflow_engine.endpoints.validators.utils.create_audit_entry") def test_create_workflow_connected_to_error_detector(self, mock_audit: mock.MagicMock) -> None: """ Tests that a member can create workflows with connections to a system-created detector From cb2ed4cd6f5eaa38dbe8600dccd5a11a4df9476e Mon Sep 17 00:00:00 2001 From: geoffg-sentry <165922362+geoffg-sentry@users.noreply.github.com> Date: Fri, 27 Mar 2026 19:03:58 -0400 Subject: [PATCH 3/6] Remove migrateRepository function (#111747) front end change pending deploy of https://github.com/getsentry/sentry/pull/111739 originally added in https://github.com/getsentry/sentry/pull/10008 --- static/app/actionCreators/integrations.tsx | 26 -------------- .../components/useScmRepoSelection.ts | 2 -- .../integrationRepos.spec.tsx | 36 +++++++++++-------- .../integrationReposAddRepository.tsx | 28 +++++++++++---- 4 files changed, 43 insertions(+), 49 deletions(-) diff --git a/static/app/actionCreators/integrations.tsx b/static/app/actionCreators/integrations.tsx index ca6da8c9c2b959..c608c0c0365a63 100644 --- a/static/app/actionCreators/integrations.tsx +++ b/static/app/actionCreators/integrations.tsx @@ -76,32 +76,6 @@ function applyRepositoryAddComplete(promise: Promise) { return promise; } -/** - * Migrate a repository to a new integration. - * - * @param client ApiClient - * @param orgSlug Organization Slug - * @param repositoryId Repository ID - * @param integration Integration provider data. - */ -export function migrateRepository( - client: Client, - orgSlug: string, - repositoryId: string, - integration: Integration -): Promise { - const data = {integrationId: integration.id}; - addLoadingMessage(); - const promise = client.requestPromise( - `/organizations/${orgSlug}/repos/${repositoryId}/`, - { - data, - method: 'PUT', - } - ); - return applyRepositoryAddComplete(promise); -} - /** * Add a repository * diff --git a/static/app/views/onboarding/components/useScmRepoSelection.ts b/static/app/views/onboarding/components/useScmRepoSelection.ts index fa0ae9424693f6..807ddf295deee7 100644 --- a/static/app/views/onboarding/components/useScmRepoSelection.ts +++ b/static/app/views/onboarding/components/useScmRepoSelection.ts @@ -102,8 +102,6 @@ export function useScmRepoSelection({ // Lookup missed (e.g., repo was hidden). Fall through to re-add it. } - // Note: for project creation (non-onboarding), we'll also need to handle - // migrateRepository for repos previously connected via legacy plugins. setBusy(true); try { const created = await fetchMutation({ diff --git a/static/app/views/settings/organizationIntegrations/integrationRepos.spec.tsx b/static/app/views/settings/organizationIntegrations/integrationRepos.spec.tsx index c807ee22db12e0..62f4a62b9c37b8 100644 --- a/static/app/views/settings/organizationIntegrations/integrationRepos.spec.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationRepos.spec.tsx @@ -207,7 +207,7 @@ describe('IntegrationRepos', () => { }); describe('migratable repo', () => { - it('associates repository with integration', async () => { + it('adds repository via POST even when existing repo matches', async () => { MockApiClient.addMockResponse({ url: `/organizations/${org.slug}/repos/`, body: [ @@ -225,9 +225,9 @@ describe('IntegrationRepos', () => { url: `/organizations/${org.slug}/integrations/${integration.id}/repos/`, body: {repos: [{identifier: 'example/repo-name', name: 'repo-name'}]}, }); - const updateRepo = MockApiClient.addMockResponse({ - method: 'PUT', - url: `/organizations/${org.slug}/repos/4/`, + const createRepo = MockApiClient.addMockResponse({ + method: 'POST', + url: `/organizations/${org.slug}/repos/`, body: {id: 244}, }); render(); @@ -235,16 +235,20 @@ describe('IntegrationRepos', () => { await userEvent.click(await screen.findByText('Add Repository')); await userEvent.type(screen.getByRole('textbox'), 'repo-name'); await userEvent.click(screen.getByText('repo-name')); - expect(updateRepo).toHaveBeenCalledWith( - `/organizations/${org.slug}/repos/4/`, + expect(createRepo).toHaveBeenCalledWith( + `/organizations/${org.slug}/repos/`, expect.objectContaining({ - data: {integrationId: '1'}, + data: { + installation: integration.id, + identifier: 'example/repo-name', + provider: `integrations:${integration.provider.key}`, + }, }) ); await waitFor(() => expect(resetReposSpy).toHaveBeenCalled()); }); - it('uses externalSlug not name for comparison', async () => { + it('uses POST path regardless of externalSlug match', async () => { MockApiClient.addMockResponse({ url: `/organizations/${org.slug}/repos/`, method: 'GET', @@ -257,9 +261,9 @@ describe('IntegrationRepos', () => { repos: [{identifier: '9876', name: 'repo-name'}], }, }); - const updateRepo = MockApiClient.addMockResponse({ - method: 'PUT', - url: `/organizations/${org.slug}/repos/4/`, + const createRepo = MockApiClient.addMockResponse({ + method: 'POST', + url: `/organizations/${org.slug}/repos/`, body: {id: 4320}, }); render(); @@ -269,10 +273,14 @@ describe('IntegrationRepos', () => { await userEvent.click(screen.getByText('repo-name')); expect(getItems).toHaveBeenCalled(); - expect(updateRepo).toHaveBeenCalledWith( - `/organizations/${org.slug}/repos/4/`, + expect(createRepo).toHaveBeenCalledWith( + `/organizations/${org.slug}/repos/`, expect.objectContaining({ - data: {integrationId: '1'}, + data: { + installation: integration.id, + identifier: '9876', + provider: `integrations:${integration.provider.key}`, + }, }) ); }); diff --git a/static/app/views/settings/organizationIntegrations/integrationReposAddRepository.tsx b/static/app/views/settings/organizationIntegrations/integrationReposAddRepository.tsx index 0c4d0f10def2a4..27c165e10383e5 100644 --- a/static/app/views/settings/organizationIntegrations/integrationReposAddRepository.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationReposAddRepository.tsx @@ -1,9 +1,10 @@ import {useMemo, useState} from 'react'; +import * as Sentry from '@sentry/react'; import {CompactSelect} from '@sentry/scraps/compactSelect'; import {OverlayTrigger} from '@sentry/scraps/overlayTrigger'; -import {addRepository, migrateRepository} from 'sentry/actionCreators/integrations'; +import {addRepository} from 'sentry/actionCreators/integrations'; import {DropdownButton} from 'sentry/components/dropdownButton'; import {t} from 'sentry/locale'; import {RepositoryStore} from 'sentry/stores/repositoryStore'; @@ -78,19 +79,32 @@ export function IntegrationReposAddRepository({ return selection.value === item.externalSlug; }); - let promise: Promise; if (migratableRepo) { - promise = migrateRepository(api, organization.slug, migratableRepo.id, integration); - } else { - promise = addRepository(api, organization.slug, selection.value, integration); + Sentry.captureException( + new Error( + 'Attempted to migrate repository integration — this code path is disabled' + ), + { + extra: { + repositoryId: migratableRepo.id, + integrationId: integration.id, + orgSlug: organization.slug, + }, + } + ); } try { - const repo = await promise; + const repo = await addRepository( + api, + organization.slug, + selection.value, + integration + ); onAddRepository(repo); RepositoryStore.resetRepositories(); } catch { - // Error feedback is handled by addRepository/migrateRepository + // Error feedback is handled by addRepository } finally { setAdding(false); } From 4db5947aaa4329237861f9f752bc66e9f9a51046 Mon Sep 17 00:00:00 2001 From: Kyle Consalus Date: Fri, 27 Mar 2026 16:14:02 -0700 Subject: [PATCH 4/6] fix(workflow-engine): Handle non-numeric DataCondition comparisons in combined-rules serializer (#111743) The WorkflowEngineDataConditionSerializer assumed all DataCondition comparison values in workflow DCGs were numeric, which holds for dual-written rules but not for single-written rules where anomaly detection conditions can store dict comparisons. Filter to numeric values only when building the comparison lookup. Fixes SENTRY-5MHW. --- .../workflow_engine_data_condition.py | 8 +++-- .../test_workflow_engine_data_condition.py | 32 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/sentry/incidents/endpoints/serializers/workflow_engine_data_condition.py b/src/sentry/incidents/endpoints/serializers/workflow_engine_data_condition.py index 8dc4eee14473df..e6de3a7444ccec 100644 --- a/src/sentry/incidents/endpoints/serializers/workflow_engine_data_condition.py +++ b/src/sentry/incidents/endpoints/serializers/workflow_engine_data_condition.py @@ -80,10 +80,12 @@ def get_attrs( # Map (condition_group_id, comparison) → action-filter DC exists in that DCG # We need: for a given detector's DCGs + priority level → matching DCG IDs # NOTE: Assumes DataConditions are limited to what would be dual written. - dcg_comparison_pairs: dict[int, set[int]] = defaultdict(set) + dcg_comparison_pairs: dict[int, set[int | float]] = defaultdict(set) for dc in DataCondition.objects.filter(condition_group__in=all_dcg_ids): - # Map comparison value → set of DCG IDs that have an action filter at that level - dcg_comparison_pairs[dc.condition_group_id].add(dc.comparison) + # Only collect numeric comparison values; non-numeric values (e.g. dicts + # from anomaly detection conditions) don't match condition_result levels. + if isinstance(dc.comparison, (int, float)): + dcg_comparison_pairs[dc.condition_group_id].add(dc.comparison) # Bulk-fetch all DCG → action mappings dcg_to_action_ids: dict[int, list[int]] = defaultdict(list) diff --git a/tests/sentry/incidents/serializers/test_workflow_engine_data_condition.py b/tests/sentry/incidents/serializers/test_workflow_engine_data_condition.py index 6da3cb5da77daa..96a489cdafe203 100644 --- a/tests/sentry/incidents/serializers/test_workflow_engine_data_condition.py +++ b/tests/sentry/incidents/serializers/test_workflow_engine_data_condition.py @@ -15,6 +15,8 @@ migrate_metric_data_conditions, migrate_resolve_threshold_data_condition, ) +from sentry.workflow_engine.models import DataCondition, WorkflowDataConditionGroup +from sentry.workflow_engine.models.data_condition import Condition from tests.sentry.incidents.serializers.test_workflow_engine_base import ( TestWorkflowEngineSerializer, ) @@ -153,6 +155,36 @@ def test_anomaly_detection(self) -> None: assert serialized_data_condition["alertThreshold"] == 0 assert serialized_data_condition["resolveThreshold"] is None + def test_anomaly_detection_with_workflow_actions(self) -> None: + dynamic_rule = self.create_dynamic_alert() + critical_trigger = self.create_alert_rule_trigger( + alert_rule=dynamic_rule, label="critical", alert_threshold=0 + ) + trigger_action = self.create_alert_rule_trigger_action(alert_rule_trigger=critical_trigger) + _, _, _, detector, _, _, _, _ = migrate_alert_rule(dynamic_rule) + detector_trigger, _, _ = migrate_metric_data_conditions(critical_trigger) + migrate_metric_action(trigger_action) + + workflow_dcg = WorkflowDataConditionGroup.objects.filter( + workflow__detectorworkflow__detector=detector, + ).first() + assert workflow_dcg is not None + DataCondition.objects.create( + type=Condition.ANOMALY_DETECTION, + comparison={"sensitivity": "high", "seasonality": "auto", "threshold_type": 2}, + condition_result=True, + condition_group=workflow_dcg.condition_group, + ) + + serialized = serialize( + detector_trigger, + self.user, + WorkflowEngineDataConditionSerializer(), + ) + assert serialized["thresholdType"] == AlertRuleThresholdType.ABOVE_AND_BELOW.value + assert serialized["alertThreshold"] == 0 + assert serialized["resolveThreshold"] is None + def test_multiple_rules(self) -> None: # create another comprehensive alert rule in the DB alert_rule, critical_trigger, warning_trigger, critical_action, warning_action = ( From b3a1c722cb5da1e579b690dd82cc4acb41e7cf39 Mon Sep 17 00:00:00 2001 From: Kyle Consalus Date: Fri, 27 Mar 2026 16:46:48 -0700 Subject: [PATCH 5/6] perf(workflows): Batch ActionAlertRuleTriggerAction lookup in WorkflowEngineActionSerializer (#111763) Avoids a query per action, and makes further refactors to improve batching more effective. --- .../serializers/workflow_engine_action.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/sentry/incidents/endpoints/serializers/workflow_engine_action.py b/src/sentry/incidents/endpoints/serializers/workflow_engine_action.py index cbd4bbbe3218cb..55f5139e6cf28e 100644 --- a/src/sentry/incidents/endpoints/serializers/workflow_engine_action.py +++ b/src/sentry/incidents/endpoints/serializers/workflow_engine_action.py @@ -1,5 +1,5 @@ from collections.abc import Mapping -from typing import Any +from typing import Any, Sequence from django.contrib.auth.models import AnonymousUser @@ -20,6 +20,17 @@ class WorkflowEngineActionSerializer(Serializer): + def get_attrs( + self, item_list: Sequence[Action], user: User | RpcUser | AnonymousUser, **kwargs: Any + ) -> dict[Action, dict[str, Any]]: + aarta_by_action_id = { + aarta.action_id: aarta + for aarta in ActionAlertRuleTriggerAction.objects.filter( + action__in=[item.id for item in item_list] + ) + } + return {item: {"aarta": aarta_by_action_id.get(item.id)} for item in item_list} + def serialize( self, obj: Action, attrs: Mapping[str, Any], user: User | RpcUser | AnonymousUser, **kwargs ) -> dict[str, Any]: @@ -30,10 +41,7 @@ def serialize( alert_rule_trigger_id = kwargs.get("alert_rule_trigger_id", -1) - try: - aarta = ActionAlertRuleTriggerAction.objects.get(action=obj.id) - except ActionAlertRuleTriggerAction.DoesNotExist: - aarta = None + aarta = attrs.get("aarta") priority = obj.data.get("priority") type_value = ActionService.get_value(obj.type) target = MetricAlertRegistryHandler.target(obj) From 89ef2bd45575ccfca5637f1702c6fa34ba283db7 Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Fri, 27 Mar 2026 16:52:09 -0700 Subject: [PATCH 6/6] ref(aci): Remove a try/except block for more detailed errors (#111439) # Description When investigating an issue, it was difficult to determine the source of the issue because we hit the exception block and lost which of the ValidationError was bubbling up. This will just allow the more specific validation error to bubble up and reformat them to all be ValidationErrors that can be handled in the API. The code still catches the exception and retriggers it to ensure the control flow is consistent and the validation error types are correct. --- src/sentry/incidents/metric_issue_detector.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 4f951549fa3dc1..2ca139ebd2b79b 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -1,7 +1,10 @@ from datetime import timedelta from typing import Any +from django.core.exceptions import ValidationError +from parsimonious.exceptions import ParseError from rest_framework import serializers +from urllib3.exceptions import MaxRetryError, TimeoutError from sentry import features, quotas from sentry.constants import ObjectStatus @@ -325,11 +328,9 @@ def update_data_source( validated_data_source: dict[str, Any] = {"data_sources": [data_source]} if not seer_updated: update_detector_data(instance, validated_data_source) - except Exception: + except (TimeoutError, MaxRetryError, ParseError, ValidationError) as e: # don't update the snuba query if we failed to send data to Seer - raise serializers.ValidationError( - "Failed to send data to Seer, cannot update detector" - ) + raise serializers.ValidationError(str(e)) extrapolation_mode = format_extrapolation_mode( data_source.get("extrapolation_mode", snuba_query.extrapolation_mode) @@ -367,11 +368,9 @@ def update_anomaly_detection(self, instance: Detector, validated_data: dict[str, try: update_detector_data(instance, validated_data) seer_updated = True - except Exception: + except (TimeoutError, MaxRetryError, ParseError, ValidationError) as e: # Don't update if we failed to send data to Seer - raise serializers.ValidationError( - "Failed to send data to Seer, cannot update detector" - ) + raise serializers.ValidationError(str(e)) elif ( validated_data.get("config")