fix(webhooks): prevent unnecessary organisation webhook tasks (#3365)

api/audit/signals.py

@@ -1,5 +1,6 @@
 import logging
 
+from django.conf import settings
 from django.db.models.signals import post_save
 from django.dispatch import receiver
 
@@ -9,28 +10,35 @@
 from integrations.dynatrace.dynatrace import DynatraceWrapper
 from integrations.new_relic.new_relic import NewRelicWrapper
 from integrations.slack.slack import SlackWrapper
+from organisations.models import OrganisationWebhook
 from webhooks.webhooks import WebhookEventType, call_organisation_webhooks
 
 logger = logging.getLogger(__name__)
 
 
 @receiver(post_save, sender=AuditLog)
 def call_webhooks(sender, instance, **kwargs):
-    data = AuditLogListSerializer(instance=instance).data
+    if settings.DISABLE_WEBHOOKS:
+        return
 
-    if not (instance.project or instance.environment):
+    if not (instance.project_id or instance.environment_id):
         logger.warning("Audit log without project or environment. Not sending webhook.")
         return
 
-    organisation = (
-        instance.project.organisation
+    organisation_id = (
+        instance.project.organisation_id
         if instance.project
-        else instance.environment.project.organisation
-    )
-    call_organisation_webhooks.delay(
-        args=(organisation.id, data, WebhookEventType.AUDIT_LOG_CREATED.value)
+        else instance.environment.project.organisation_id
     )
 
+    if OrganisationWebhook.objects.filter(
+        organisation_id=organisation_id, enabled=True
+    ).exists():
+        data = AuditLogListSerializer(instance=instance).data
+        call_organisation_webhooks.delay(
+            args=(organisation_id, data, WebhookEventType.AUDIT_LOG_CREATED.value)
+        )
+
 
 def _get_integration_config(instance, integration_name):
     if hasattr(instance.project, integration_name):

api/tests/unit/audit/test_unit_audit_models.py

@@ -1,16 +1,29 @@
+from pytest_mock import MockerFixture
+
 from audit.models import AuditLog
 from audit.related_object_type import RelatedObjectType
 from audit.serializers import AuditLogListSerializer
 from integrations.datadog.models import DataDogConfiguration
+from organisations.models import Organisation, OrganisationWebhook
+from projects.models import Project
 from webhooks.webhooks import WebhookEventType
 
 
-def test_organisation_webhooks_are_called_when_audit_log_saved(project, mocker):
+def test_organisation_webhooks_are_called_when_audit_log_saved(
+    project: Project, mocker: MockerFixture, organisation: Organisation
+) -> None:
     # Given
     mock_call_webhooks = mocker.patch("audit.signals.call_organisation_webhooks")
 
     audit_log = AuditLog(project=project, log="Some audit log")
 
+    OrganisationWebhook.objects.create(
+        organisation=organisation,
+        url="http://example.com/webhook",
+        enabled=True,
+        name="example webhook",
+    )
+
     # When
     audit_log.save()
 

api/tests/unit/audit/test_unit_audit_signals.py

@@ -0,0 +1,83 @@
+from pytest_django.fixtures import SettingsWrapper
+from pytest_mock import MockerFixture
+
+from audit.models import AuditLog
+from audit.signals import call_webhooks
+from organisations.models import Organisation, OrganisationWebhook
+from projects.models import Project
+from webhooks.webhooks import WebhookEventType
+
+
+def test_call_webhooks_does_not_create_task_if_webhooks_disabled(
+    organisation: Organisation,
+    project: Project,
+    settings: SettingsWrapper,
+    mocker: MockerFixture,
+) -> None:
+    # Given
+    audit_log = AuditLog(project=project)
+    settings.DISABLE_WEBHOOKS = True
+
+    mocked_call_organisation_webhooks = mocker.patch(
+        "audit.signals.call_organisation_webhooks"
+    )
+
+    # When
+    call_webhooks(sender=AuditLog, instance=audit_log)
+
+    # Then
+    mocked_call_organisation_webhooks.delay.assert_not_called()
+
+
+def test_call_webhooks_does_not_create_task_if_organisation_has_no_webhooks(
+    organisation: Organisation,
+    project: Project,
+    settings: SettingsWrapper,
+    mocker: MockerFixture,
+) -> None:
+    # Given
+    audit_log = AuditLog(project=project)
+    settings.DISABLE_WEBHOOKS = False
+
+    assert organisation.webhooks.count() == 0
+
+    mocked_call_organisation_webhooks = mocker.patch(
+        "audit.signals.call_organisation_webhooks"
+    )
+
+    # When
+    call_webhooks(sender=AuditLog, instance=audit_log)
+
+    # Then
+    mocked_call_organisation_webhooks.delay.assert_not_called()
+
+
+def test_call_webhooks_creates_task_if_organisation_has_webhooks(
+    organisation: Organisation,
+    project: Project,
+    settings: SettingsWrapper,
+    mocker: MockerFixture,
+) -> None:
+    # Given
+    audit_log = AuditLog.objects.create(project=project)
+    OrganisationWebhook.objects.create(
+        organisation=organisation,
+        name="Example webhook",
+        url="http://example.com/webhook",
+        enabled=True,
+    )
+    settings.DISABLE_WEBHOOKS = False
+
+    mocked_call_organisation_webhooks = mocker.patch(
+        "audit.signals.call_organisation_webhooks"
+    )
+
+    # When
+    call_webhooks(sender=AuditLog, instance=audit_log)
+
+    # Then
+    mocked_call_organisation_webhooks.delay.assert_called_once()
+    mock_call = mocked_call_organisation_webhooks.delay.call_args.kwargs
+    assert mock_call["args"][0] == organisation.id
+    assert mock_call["args"][1]["id"] == audit_log.id
+    assert mock_call["args"][2] == WebhookEventType.AUDIT_LOG_CREATED.name