From 132b7f1f5e0850e7d0f0e2615ece76b2666ef2d1 Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Mon, 3 Mar 2025 12:09:52 -0800 Subject: [PATCH 1/6] Create API for deleting workflows --- .../endpoints/organization_workflow_details.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_details.py b/src/sentry/workflow_engine/endpoints/organization_workflow_details.py index 230ab7b2d78319..2b97e0097962fe 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_details.py @@ -2,6 +2,7 @@ from rest_framework.request import Request from rest_framework.response import Response +from sentry import audit_log from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint @@ -15,7 +16,9 @@ RESPONSE_UNAUTHORIZED, ) from sentry.apidocs.parameters import GlobalParams, WorkflowParams +from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion from sentry.models.organization import Organization +from sentry.utils.audit import create_audit_entry from sentry.workflow_engine.endpoints.serializers import WorkflowSerializer from sentry.workflow_engine.models import Workflow @@ -75,4 +78,12 @@ def delete(self, request: Request, organization: Organization, workflow: Workflo """ Delete a workflow """ - pass + RegionScheduledDeletion.schedule(workflow, days=0, actor=request.user) + create_audit_entry( + request=request, + organization=organization, + target_object=workflow.id, + event=audit_log.get_event_id("WORKFLOW_REMOVE"), + data=workflow.get_audit_log_data(), + ) + return Response(status=204) From 95255021997e0084234f09e6a5e85ead9cd3595e Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Mon, 3 Mar 2025 12:17:36 -0800 Subject: [PATCH 2/6] setup test scaffold --- .../organization_workflow_details.py | 1 + .../test_organization_workflow_details.py | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_details.py b/src/sentry/workflow_engine/endpoints/organization_workflow_details.py index 2b97e0097962fe..82fb66259d0514 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_details.py @@ -86,4 +86,5 @@ def delete(self, request: Request, organization: Organization, workflow: Workflo event=audit_log.get_event_id("WORKFLOW_REMOVE"), data=workflow.get_audit_log_data(), ) + return Response(status=204) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py index 1c7f31d9dd4072..d2a5078a70c4dd 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py @@ -1,6 +1,7 @@ from sentry.api.serializers import serialize from sentry.testutils.cases import APITestCase from sentry.testutils.silo import region_silo_test +from tests.sentry.workflow_engine.test_base import BaseWorkflowTest class OrganizationWorkflowDetailsBaseTest(APITestCase): @@ -20,3 +21,31 @@ def test_simple(self): def test_does_not_exist(self): self.get_error_response(self.organization.slug, 3, status_code=404) + + +@region_silo_test +class OrganizationDeleteWorkflowTest(OrganizationWorkflowDetailsBaseTest, BaseWorkflowTest): + def setUp(self): + super().setUp() + self.workflow = self.create_workflow(organization_id=self.organization.id) + + def test_simple(self): + # delete the workflow + pass + + def test_does_not_exist(self): + # delete a workflow that does not exist, -1 id + pass + + def test_delete_configured_workflow(self): + # add data conditions + # add actions + pass + + def test_audit_entry(self): + # ensure there is an audit entry for the deleted workflow + pass + + def test_without_permissions(self): + # delete a workflow that does not belong to this organization + pass From 0be643e6ca69a77ec8e9e72fdfaf9a002758d32c Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Mon, 3 Mar 2025 12:55:10 -0800 Subject: [PATCH 3/6] Fix registry entries for worfklows => workflows --- src/sentry/audit_log/register.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sentry/audit_log/register.py b/src/sentry/audit_log/register.py index c9442162da2680..8f6ae5617e6818 100644 --- a/src/sentry/audit_log/register.py +++ b/src/sentry/audit_log/register.py @@ -589,7 +589,7 @@ default_manager.add( AuditLogEvent( event_id=213, - name="WORFKLOW_ADD", + name="WORKFLOW_ADD", api_name="workflow.add", template="added workflow {name}", ) @@ -597,7 +597,7 @@ default_manager.add( AuditLogEvent( event_id=214, - name="WORFKLOW_EDIT", + name="WORKFLOW_EDIT", api_name="workflow.edit", template="edited workflow {name}", ) @@ -605,7 +605,7 @@ default_manager.add( AuditLogEvent( event_id=215, - name="WORFKLOW_REMOVE", + name="WORKFLOW_REMOVE", api_name="workflow.remove", template="removed workflow {name}", ) From 0b5d724f5123a4246bce74a546dea997772011ec Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Mon, 3 Mar 2025 12:55:39 -0800 Subject: [PATCH 4/6] while i'm here, quickly add an audit entry for the put request --- .../endpoints/organization_workflow_details.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/sentry/workflow_engine/endpoints/organization_workflow_details.py b/src/sentry/workflow_engine/endpoints/organization_workflow_details.py index 82fb66259d0514..fdb48a105c5bad 100644 --- a/src/sentry/workflow_engine/endpoints/organization_workflow_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_workflow_details.py @@ -72,6 +72,14 @@ def put(self, request: Request, organization: Organization, workflow: Workflow): """ Updates a workflow """ + create_audit_entry( + request=request, + organization=organization, + target_object=workflow.id, + event=audit_log.get_event_id("WORKFLOW_EDIT"), + data=workflow.get_audit_log_data(), + ) + pass def delete(self, request: Request, organization: Organization, workflow: Workflow): From 00e750264c76b1fe275b0b3a73332aa982c4b465 Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Mon, 3 Mar 2025 12:55:57 -0800 Subject: [PATCH 5/6] Setup a simple test case and the audit entry test case --- .../test_organization_workflow_details.py | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py index d2a5078a70c4dd..2d1dbe51b401f1 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py @@ -1,6 +1,11 @@ +from sentry import audit_log from sentry.api.serializers import serialize +from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion +from sentry.models.auditlogentry import AuditLogEntry +from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase -from sentry.testutils.silo import region_silo_test +from sentry.testutils.outbox import outbox_runner +from sentry.testutils.silo import assume_test_silo_mode, region_silo_test from tests.sentry.workflow_engine.test_base import BaseWorkflowTest @@ -25,13 +30,31 @@ def test_does_not_exist(self): @region_silo_test class OrganizationDeleteWorkflowTest(OrganizationWorkflowDetailsBaseTest, BaseWorkflowTest): + method = "DELETE" + def setUp(self): super().setUp() self.workflow = self.create_workflow(organization_id=self.organization.id) def test_simple(self): - # delete the workflow - pass + with outbox_runner(): + self.get_success_response(self.organization.slug, self.workflow.id) + + assert RegionScheduledDeletion.objects.filter( + model_name="Workflow", + object_id=self.workflow.id, + ).exists() + + def test_audit_entry(self): + with outbox_runner(): + self.get_success_response(self.organization.slug, self.workflow.id) + + with assume_test_silo_mode(SiloMode.CONTROL): + assert AuditLogEntry.objects.filter( + target_object=self.workflow.id, + event=audit_log.get_event_id("WORKFLOW_REMOVE"), + actor=self.user, + ).exists() def test_does_not_exist(self): # delete a workflow that does not exist, -1 id @@ -42,10 +65,6 @@ def test_delete_configured_workflow(self): # add actions pass - def test_audit_entry(self): - # ensure there is an audit entry for the deleted workflow - pass - def test_without_permissions(self): # delete a workflow that does not belong to this organization pass From 7d9a7269ecd5061b98afb3dbcc54327feb29b053 Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Mon, 3 Mar 2025 15:21:23 -0800 Subject: [PATCH 6/6] Add more complex test conditions, ensure that nested models are removed and test permissions --- .../test_organization_workflow_details.py | 66 ++++++++++++++++--- 1 file changed, 58 insertions(+), 8 deletions(-) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py index 2d1dbe51b401f1..79e937f436ae59 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py @@ -1,11 +1,14 @@ from sentry import audit_log from sentry.api.serializers import serialize from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion +from sentry.deletions.tasks.scheduled import run_scheduled_deletions from sentry.models.auditlogentry import AuditLogEntry from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase +from sentry.testutils.helpers import TaskRunner from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import assume_test_silo_mode, region_silo_test +from sentry.workflow_engine.models import Action, DataConditionGroup from tests.sentry.workflow_engine.test_base import BaseWorkflowTest @@ -32,6 +35,9 @@ def test_does_not_exist(self): class OrganizationDeleteWorkflowTest(OrganizationWorkflowDetailsBaseTest, BaseWorkflowTest): method = "DELETE" + def tasks(self): + return TaskRunner() + def setUp(self): super().setUp() self.workflow = self.create_workflow(organization_id=self.organization.id) @@ -57,14 +63,58 @@ def test_audit_entry(self): ).exists() def test_does_not_exist(self): - # delete a workflow that does not exist, -1 id - pass + with outbox_runner(): + response = self.get_error_response(self.organization.slug, -1) + assert response.status_code == 404 + + # Ensure it wasn't deleted + assert not RegionScheduledDeletion.objects.filter( + model_name="Workflow", + object_id=self.workflow.id, + ).exists() + + def test_delete_configured_workflow__action(self): + action_condition_group, action = self.create_workflow_action(workflow=self.workflow) + + with outbox_runner(): + self.get_success_response(self.organization.slug, self.workflow.id) + + # Ensure the workflow is scheduled for deletion + assert RegionScheduledDeletion.objects.filter( + model_name="Workflow", + object_id=self.workflow.id, + ).exists() - def test_delete_configured_workflow(self): - # add data conditions - # add actions - pass + # Delete the workflow + with self.tasks(): + run_scheduled_deletions() + + # Ensure action is removed + assert not Action.objects.filter(id=action.id).exists() + + def test_delete_configured_workflow__action_condition(self): + action_condition_group, action = self.create_workflow_action(workflow=self.workflow) + + with outbox_runner(): + self.get_success_response(self.organization.slug, self.workflow.id) + + # Ensure the workflow is scheduled for deletion + assert RegionScheduledDeletion.objects.filter( + model_name="Workflow", + object_id=self.workflow.id, + ).exists() + + # Actually delete the workflow + with self.tasks(): + run_scheduled_deletions() + + assert not DataConditionGroup.objects.filter(id=action_condition_group.id).exists() def test_without_permissions(self): - # delete a workflow that does not belong to this organization - pass + # Create a workflow with a different organization + new_org = self.create_organization() + workflow = self.create_workflow(organization_id=new_org.id) + + with outbox_runner(): + response = self.get_error_response(self.organization.slug, workflow.id) + assert response.status_code == 404