Merge pull request #158 from monzo/use-slack-profile-cache

To save on slack calls, use the cached user profile where we can

response/slack/dialog_handlers.py

@@ -5,6 +5,7 @@
 from django.conf import settings
 
 from response.core.models import ExternalUser, Incident
+from response.slack.cache import get_user_profile
 from response.slack.client import channel_reference
 from response.slack.decorators import dialog_handler
 from response.slack.settings import INCIDENT_EDIT_DIALOG, INCIDENT_REPORT_DIALOG
@@ -27,14 +28,14 @@ def report_incident(
     else:
         report_only = False
 
-    name = settings.SLACK_CLIENT.get_user_profile(user_id)["name"]
+    name = get_user_profile(user_id)["name"]
     reporter, _ = ExternalUser.objects.get_or_create_slack(
         external_id=user_id, display_name=name
     )
 
     lead = None
     if lead_id:
-        lead_name = settings.SLACK_CLIENT.get_user_profile(lead_id)["name"]
+        lead_name = get_user_profile(lead_id)["name"]
         lead, _ = ExternalUser.objects.get_or_create_slack(
             external_id=lead_id, display_name=lead_name
         )
@@ -74,7 +75,7 @@ def edit_incident(
 
     lead = None
     if lead_id:
-        lead_name = settings.SLACK_CLIENT.get_user_profile(lead_id)["name"]
+        lead_name = get_user_profile(lead_id)["name"]
         lead, _ = ExternalUser.objects.get_or_create_slack(
             external_id=lead_id, display_name=lead_name
         )

response/slack/incident_commands.py

@@ -1,8 +1,7 @@
 from datetime import datetime
 
-from django.conf import settings
-
 from response.core.models import Action, ExternalUser, Incident
+from response.slack.cache import get_user_profile
 from response.slack.client import SlackError, reference_to_id
 from response.slack.decorators.incident_command import (
     __default_incident_command,
@@ -35,7 +34,7 @@ def update_impact(incident: Incident, user_id: str, message: str):
 @__default_incident_command(["lead"], helptext="Assign someone as the incident lead")
 def set_incident_lead(incident: Incident, user_id: str, message: str):
     assignee = reference_to_id(message) or user_id
-    name = settings.SLACK_CLIENT.get_user_profile(assignee)["name"]
+    name = get_user_profile(assignee)["name"]
     user, _ = ExternalUser.objects.get_or_create_slack(
         external_id=assignee, display_name=name
     )
@@ -101,7 +100,7 @@ def close_incident(incident: Incident, user_id: str, message: str):
 
 @__default_incident_command(["action"], helptext="Log a follow up action")
 def set_action(incident: Incident, user_id: str, message: str):
-    name = settings.SLACK_CLIENT.get_user_profile(user_id)["name"]
+    name = get_user_profile(user_id)["name"]
     action_reporter, _ = ExternalUser.objects.get_or_create_slack(
         external_id=user_id, display_name=name
     )

response/slack/models/pinned_message.py

@@ -1,15 +1,15 @@
 from datetime import datetime
 
-from django.conf import settings
 from django.db import models
 
 from response.core.models import ExternalUser, Incident, TimelineEvent
 from response.core.serializers import ExternalUserSerializer
+from response.slack.cache import get_user_profile
 
 
 class PinnedMessageManager(models.Manager):
     def add_pin(self, incident, message_ts, author_id, text):
-        name = settings.SLACK_CLIENT.get_user_profile(author_id)["name"]
+        name = get_user_profile(author_id)["name"]
         author, _ = ExternalUser.objects.get_or_create_slack(
             external_id=author_id, display_name=name
         )

response/slack/models/user_stats.py

@@ -1,9 +1,9 @@
 from datetime import datetime
 
-from django.conf import settings
 from django.db import models
 
 from response.core.models import ExternalUser, Incident
+from response.slack.cache import get_user_profile
 
 
 class UserStats(models.Model):
@@ -22,7 +22,7 @@ class Meta:
 
     @staticmethod
     def increment_message_count(incident, user_id):
-        name = settings.SLACK_CLIENT.get_user_profile(user_id)["name"]
+        name = get_user_profile(user_id)["name"]
         user, _ = ExternalUser.objects.get_or_create_slack(
             external_id=user_id, display_name=name
         )

response/templatetags/unslackify.py

@@ -2,8 +2,8 @@
 
 import emoji_data_python
 from django import template
-from django.conf import settings
 
+from response.slack.cache import get_user_profile
 from response.slack.client import slack_to_human_readable
 
 register = template.Library()
@@ -25,6 +25,6 @@ def unslackify(value):
 
 @register.filter
 def slack_id_to_fullname(value):
-    profile = settings.SLACK_CLIENT.get_user_profile(value)
+    profile = get_user_profile(value)
     if profile:
         return profile["fullname"]

tests/slack/test_pin_message.py

@@ -1,4 +1,6 @@
 import datetime
+import unittest
+from unittest.mock import patch
 
 import pytest
 from faker import Faker
@@ -11,32 +13,34 @@
 faker = Faker()
 
 
-@pytest.mark.django_db
-def test_add_and_remove_pin(mock_slack):
-    incident = IncidentFactory.create()
-    user = ExternalUserFactory.create()
+class TestPinnedMessage(unittest.TestCase):
+    @pytest.mark.django_db
+    @patch("response.slack.models.pinned_message.get_user_profile")
+    def test_add_and_remove_pin(self, get_user_profile):
+        incident = IncidentFactory.create()
+        user = ExternalUserFactory.create()
 
-    mock_slack.get_user_profile.return_value = {"name": user.display_name}
+        get_user_profile.return_value = {"name": user.display_name}
 
-    text = faker.paragraph(nb_sentences=2, variable_nb_sentences=True)
-    handle_pin_added(
-        incident,
-        {"item": {"message": {"user": user.external_id, "ts": 123, "text": text}}},
-    )
-    mock_slack.get_user_profile.assert_called_with(user.external_id)
+        text = faker.paragraph(nb_sentences=2, variable_nb_sentences=True)
+        handle_pin_added(
+            incident,
+            {"item": {"message": {"user": user.external_id, "ts": 123, "text": text}}},
+        )
+        get_user_profile.assert_called_with(user.external_id)
 
-    message = PinnedMessage.objects.get(incident=incident, message_ts=123)
-    assert message.text == text
+        message = PinnedMessage.objects.get(incident=incident, message_ts=123)
+        assert message.text == text
 
-    handle_pin_removed(
-        incident,
-        {"item": {"message": {"user": user.external_id, "ts": 123, "text": text}}},
-    )
+        handle_pin_removed(
+            incident,
+            {"item": {"message": {"user": user.external_id, "ts": 123, "text": text}}},
+        )
 
-    mock_slack.get_user_profile.assert_called_with(user.external_id)
+        get_user_profile.assert_called_with(user.external_id)
 
-    with pytest.raises(PinnedMessage.DoesNotExist):
-        PinnedMessage.objects.get(incident=incident, message_ts=123)
-        TimelineEvent.objects.get(
-            incident=incident, timestamp=datetime.fromtimestamp(123)
-        )
+        with pytest.raises(PinnedMessage.DoesNotExist):
+            PinnedMessage.objects.get(incident=incident, message_ts=123)
+            TimelineEvent.objects.get(
+                incident=incident, timestamp=datetime.fromtimestamp(123)
+            )