feat(backend): fix notifiaction topic leakage (#104)

app/core/factories.py

@@ -10,6 +10,14 @@
 from accounts.models import User
 from emails.models import Email, EmailState, EmailAttachment
 from core.models import Client, FileUpload, Issue, Person, Tenancy
+from core.models.issue import CaseStage
+from notify.models import (
+    Notification,
+    NOTIFY_TOPIC_CHOICES,
+    NotifyEvent,
+    NotifyChannel,
+    NotifyTarget,
+)
 
 TINY_PNG = b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\tpHYs\x00\x00\x0e\xc4\x00\x00\x0e\xc4\x01\x95+\x0e\x1b\x00\x00\x00\x19tEXtSoftware\x00gnome-screenshot\xef\x03\xbf>\x00\x00\x00\rIDAT\x08\x99c```\xf8\x0f\x00\x01\x04\x01\x00}\xb2\xc8\xdf\x00\x00\x00\x00IEND\xaeB`\x82"
 
@@ -35,7 +43,6 @@ def username(self):
 
 
 class TimestampedModelFactory(factory.django.DjangoModelFactory):
-
     modified_at = factory.Faker(
         "date_time_between", tzinfo=timezone.utc, start_date="-1m", end_date="now"
     )
@@ -158,3 +165,28 @@ def file(self, create, extracted, **kwargs):
 def get_dummy_file(name):
     f = io.BytesIO(TINY_PNG)
     return InMemoryUploadedFile(f, None, name, "image/png", len(TINY_PNG), None)
+
+
+@factory.django.mute_signals(post_save)
+class NotificationFactory(factory.django.DjangoModelFactory):
+    class Meta:
+        model = Notification
+
+    name = factory.Faker("color_name")
+    topic = factory.Faker(
+        "random_element", elements=[c[0] for c in NOTIFY_TOPIC_CHOICES]
+    )
+    event = factory.Faker(
+        "random_element", elements=[c[0] for c in NotifyEvent.choices]
+    )
+    channel = factory.Faker(
+        "random_element", elements=[c[0] for c in NotifyChannel.choices]
+    )
+    target = factory.Faker(
+        "random_element", elements=[c[0] for c in NotifyTarget.choices]
+    )
+    event_stage = factory.Faker(
+        "random_element", elements=[c[0] for c in CaseStage.CHOICES]
+    )
+    raw_text = factory.Faker("sentence")
+    message_text = factory.Faker("sentence")

app/notify/admin.py

@@ -4,7 +4,7 @@
 
 
 @admin.register(Notification)
-class EmailTemplateAdmin(admin.ModelAdmin):
+class NotificationAdmin(admin.ModelAdmin):
     list_display = (
         "id",
         "name",

app/notify/signals.py

@@ -41,6 +41,9 @@ def on_issue_stage_change(issue_pk, old_stage: str, new_stage: str):
         event=NotifyEvent.STAGE_CHANGE, event_stage=new_stage
     )
     for notification in notifications:
+        if notification.topic not in (issue.topic, "GENERAL"):
+            continue
+
         if not notification.channel == NotifyChannel.SLACK:
             logger.error("Notification[%s] has unsupported channel", notification)
             continue
@@ -75,5 +78,9 @@ def on_issue_stage_change(issue_pk, old_stage: str, new_stage: str):
             )
             continue
 
-        message_text = f"*Notification for case <{issue.url}|{issue.fileref}>*\n{notification.message_text}"
+        message_text = get_notification_message_text(issue, notification)
         send_slack_direct_message(message_text, slack_user["id"])
+
+
+def get_notification_message_text(issue: Issue, notification: Notification):
+    return f"*Notification for case <{issue.url}|{issue.fileref}>*\n{notification.message_text}"

app/notify/tests/test_notifications.py

@@ -0,0 +1,115 @@
+from unittest.mock import patch, call
+
+import pytest
+
+from core.factories import IssueFactory, NotificationFactory, UserFactory
+from notify.signals import get_notification_message_text
+
+TEST_SLACK_USER_ID = "some-slack-id"
+
+
+def mock_get_slack_user_by_email(email: str):
+    return {"id": f"slack-{email}"}
+
+
+@pytest.mark.django_db
+@patch("notify.signals.get_slack_user_by_email", mock_get_slack_user_by_email)
+@patch("notify.signals.send_slack_direct_message")
+def test_send_notifcation__with_same_issue_topic(mock_send_message):
+    paralegal = UserFactory()
+    issue = IssueFactory(topic="REPAIRS", stage="UNSTARTED", paralegal=paralegal)
+    notification = NotificationFactory(
+        topic="REPAIRS",
+        channel="SLACK",
+        event="STAGE_CHANGE",
+        event_stage="CLIENT_AGREEMENT",
+        target="PARALEGAL",
+    )
+
+    assert mock_send_message.call_count == 0
+    issue.stage = "CLIENT_AGREEMENT"
+    issue.save()
+    assert mock_send_message.call_count == 1
+    mock_send_message.assert_has_calls(
+        [
+            call(
+                get_notification_message_text(issue, notification),
+                f"slack-{paralegal.email}",
+            )
+        ]
+    )
+
+
+@pytest.mark.django_db
+@patch("notify.signals.get_slack_user_by_email", mock_get_slack_user_by_email)
+@patch("notify.signals.send_slack_direct_message")
+def test_send_notifcation__with_different_issue_topic(mock_send_message):
+    issue = IssueFactory(topic="BONDS", stage="UNSTARTED", paralegal=UserFactory())
+    NotificationFactory(
+        topic="REPAIRS",
+        channel="SLACK",
+        event="STAGE_CHANGE",
+        event_stage="CLIENT_AGREEMENT",
+        target="PARALEGAL",
+    )
+
+    assert mock_send_message.call_count == 0
+    issue.stage = "CLIENT_AGREEMENT"
+    issue.save()
+    assert mock_send_message.call_count == 0
+
+
+@pytest.mark.django_db
+@patch("notify.signals.get_slack_user_by_email", mock_get_slack_user_by_email)
+@patch("notify.signals.send_slack_direct_message")
+def test_send_notifcation__with_general_issue_topic(mock_send_message):
+    paralegal = UserFactory()
+    issue = IssueFactory(topic="BONDS", stage="UNSTARTED", paralegal=paralegal)
+    notification = NotificationFactory(
+        topic="GENERAL",
+        channel="SLACK",
+        event="STAGE_CHANGE",
+        event_stage="CLIENT_AGREEMENT",
+        target="PARALEGAL",
+    )
+
+    assert mock_send_message.call_count == 0
+    issue.stage = "CLIENT_AGREEMENT"
+    issue.save()
+    assert mock_send_message.call_count == 1
+    mock_send_message.assert_has_calls(
+        [
+            call(
+                get_notification_message_text(issue, notification),
+                f"slack-{paralegal.email}",
+            )
+        ]
+    )
+
+
+@pytest.mark.django_db
+@patch("notify.signals.get_slack_user_by_email", mock_get_slack_user_by_email)
+@patch("notify.signals.send_slack_direct_message")
+def test_send_notifcation__with_lawyer_target(mock_send_message):
+    lawyer = UserFactory()
+    issue = IssueFactory(topic="BONDS", stage="UNSTARTED", lawyer=lawyer)
+    notification = NotificationFactory(
+        topic="GENERAL",
+        channel="SLACK",
+        event="STAGE_CHANGE",
+        event_stage="CLIENT_AGREEMENT",
+        target="LAWYER",
+    )
+
+    assert mock_send_message.call_count == 0
+    issue.stage = "CLIENT_AGREEMENT"
+    issue.save()
+    assert mock_send_message.call_count == 1
+    mock_send_message.assert_has_calls(
+        [
+            call(
+                get_notification_message_text(issue, notification),
+                f"slack-{lawyer.email}",
+            )
+        ]
+    )

app/pyproject.toml

@@ -4,3 +4,4 @@ filterwarnings = [
     "ignore::django.utils.deprecation.RemovedInDjango50Warning",
     "ignore::wagtail.utils.deprecation.RemovedInWagtail217Warning",
 ]
+addopts = "--reuse-db --ds=clerk.settings.test"