Skip to content

Commit

Permalink
Merge pull request #162 from monzo/slack-by-email
Browse files Browse the repository at this point in the history
Support looking up slack users by email and caching them
  • Loading branch information
Chris authored Oct 3, 2019
2 parents af9ed05 + 4e7e759 commit edaa12a
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 1 deletion.
43 changes: 43 additions & 0 deletions response/slack/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,46 @@ def get_user_profile(external_id):

logger.info(f"Got user {external_id} from Slack and cached in DB")
return user_profile


def get_user_profile_by_email(email):
"""
Gets a slack user profile:
- from the DB cache if available
- or else from the Slack API
"""
if not email:
return None

try:
external_user = ExternalUser.objects.get(email=email)
logger.info(f"Got user with email {email} from DB cache")

return {
"id": external_user.external_id,
"name": external_user.display_name,
"fullname": external_user.full_name,
"email": external_user.email,
}
except ExternalUser.DoesNotExist:
# profile from slack
try:
user_profile = settings.SLACK_CLIENT.get_user_profile_by_email(email)
except SlackError:
logger.error(
f"Failed to get user with email {email} from DB cache or Slack"
)
raise

# store it in the DB
ExternalUser.objects.get_or_create_slack(
external_id=user_profile["id"],
defaults={
"display_name": user_profile["name"],
"full_name": user_profile["fullname"],
"email": user_profile["email"],
},
)

logger.info(f"Got user with email {email} from Slack and cached in DB")
return user_profile
13 changes: 13 additions & 0 deletions response/slack/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,19 @@ def get_user_profile(self, user_id):
"email": response["user"]["profile"].get("email", None),
}

def get_user_profile_by_email(self, email):
if not email:
return None

response = self.api_call("users.lookupByEmail", email=email)

return {
"id": response["user"]["id"],
"name": response["user"]["name"],
"fullname": response["user"]["profile"]["real_name"],
"email": email,
}

def rename_channel(self, channel_id, new_name):
prefix = ""
if not (new_name.startswith("inc-") or new_name.startswith("#inc-")):
Expand Down
41 changes: 41 additions & 0 deletions tests/slack/slack_payloads.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,44 @@
"cache_ts": 1498777272,
"response_metadata": {"next_cursor": ""},
}

user_by_email = {
"ok": True,
"user": {
"id": "W012A3CDE",
"team_id": "T012AB3C4",
"name": "spengler",
"deleted": False,
"color": "9f69e7",
"real_name": "Egon Spengler",
"tz": "America/Los_Angeles",
"tz_label": "Pacific Daylight Time",
"tz_offset": -25200,
"profile": {
"avatar_hash": "ge3b51ca72de",
"status_text": "Print is dead",
"status_emoji": ":books:",
"real_name": "Egon Spengler",
"display_name": "spengler",
"real_name_normalized": "Egon Spengler",
"display_name_normalized": "spengler",
"email": "spengler@ghostbusters.example.com",
"image_24": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
"image_32": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
"image_48": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
"image_72": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
"image_192": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
"image_512": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg",
"team": "T012AB3C4",
},
"is_admin": True,
"is_owner": False,
"is_primary_owner": False,
"is_restricted": False,
"is_ultra_restricted": False,
"is_bot": False,
"updated": 1502138686,
"is_app_user": False,
"has_2fa": False,
},
}
61 changes: 60 additions & 1 deletion tests/slack/test_cache.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import pytest

from response.core.models import ExternalUser
from response.slack.cache import get_user_profile, update_user_cache
from response.slack.cache import (
get_user_profile,
get_user_profile_by_email,
update_user_cache,
)
from tests.slack.slack_payloads import (
users_list_new,
users_list_page_1,
Expand Down Expand Up @@ -111,3 +115,58 @@ def test_get_user_profile_in_cache(mock_slack):

# check cache is unchanged
assert len(ExternalUser.objects.all()) == 1


@pytest.mark.django_db
def test_get_user_profile_by_email_not_in_cache(mock_slack):
mock_slack.get_user_profile_by_email.return_value = {
"id": "U12345678",
"name": "spengler",
"fullname": "Egon Spengler",
"email": "spengler@ghostbusters.example.com",
}

# check cache is empty at start
assert len(ExternalUser.objects.all()) == 0

# request a user from cache
user = get_user_profile_by_email("spengler@ghostbusters.example.com")

# check we get back the user from slack
mock_slack.get_user_profile_by_email.assert_called()
assert user["id"] == "U12345678"

# check user is now in cache
assert len(ExternalUser.objects.all()) == 1

# and that it has the right details populated
cache_user = ExternalUser.objects.get(email="spengler@ghostbusters.example.com")
assert cache_user.display_name == "spengler"
assert cache_user.full_name == "Egon Spengler"
assert cache_user.email == "spengler@ghostbusters.example.com"


@pytest.mark.django_db
def test_get_user_profile_by_email_in_cache(mock_slack):
# create cache entry for user
slack_user = ExternalUser(
external_id="U12345678",
display_name="spengler",
full_name="Egon Spengler",
email="spengler@ghostbusters.example.com",
)
slack_user.save()
assert len(ExternalUser.objects.all()) == 1

# request a user from cache
user = get_user_profile_by_email("spengler@ghostbusters.example.com")

# check we get back the user from cache (i.e. not call to Slack API)
mock_slack.get_user_profile_by_email.assert_not_called()
assert user["id"] == "U12345678"
assert user["name"] == "spengler"
assert user["fullname"] == "Egon Spengler"
assert user["email"] == "spengler@ghostbusters.example.com"

# check cache is unchanged
assert len(ExternalUser.objects.all()) == 1
18 changes: 18 additions & 0 deletions tests/slack/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.conf import settings

from response.slack import client
from tests.slack.slack_payloads import user_by_email


@pytest.fixture
Expand Down Expand Up @@ -64,3 +65,20 @@ def test_slack_backoff_rate_limit_max_retry_attempts(slack_client, slack_api_moc
with pytest.raises(client.SlackError) as e:
slack_client.api_call("test_endpoint", "arg1", kwarg2="foo")
assert e.slack_error == "test_error"


def test_get_user_profile_by_email(slack_client, slack_api_mock):
slack_api_mock.api_call.return_value = user_by_email

# request a user by email
user = slack_client.get_user_profile_by_email("spengler@ghostbusters.example.com")

slack_api_mock.api_call.assert_called_with(
"users.lookupByEmail", email="spengler@ghostbusters.example.com"
)

# check we get back the expected user profile
assert user["id"] == "W012A3CDE"
assert user["name"] == "spengler"
assert user["fullname"] == "Egon Spengler"
assert user["email"] == "spengler@ghostbusters.example.com"

0 comments on commit edaa12a

Please sign in to comment.