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/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) 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/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") 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/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/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); } 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 = ( 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) 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