Updating Slack API usage (#751)

* Updating Slack API usage

* Code review

* Because tests

orchestra/communication/slack.py

@@ -53,7 +53,7 @@ def __init__(self, api_key=None):
         if not api_key:
             api_key = settings.SLACK_EXPERTS_API_KEY
         self._service = Slacker(api_key)
-        for attr_name in ('chat', 'groups', 'users'):
+        for attr_name in ('chat', 'conversations', 'users'):
             setattr(self, attr_name, getattr(self._service, attr_name))
 
 
@@ -69,17 +69,14 @@ def add_worker_to_project_team(worker, project):
     slack = OrchestraSlackService()
     try:
         slack_user_id = worker.slack_user_id
-        response = slack.groups.invite(
+        slack.conversations.invite(
             project.slack_group_id, slack_user_id)
-        if not response.body.get('already_in_group'):
-            welcome_message = (
-                '<@{}> has been added to the team. '
-                'Welcome aboard!').format(slack_user_id)
-            slack.chat.post_message(project.slack_group_id, welcome_message)
+        welcome_message = (
+            '<@{}> has been added to the team. '
+            'Welcome aboard!').format(slack_user_id)
+        slack.chat.post_message(project.slack_group_id, welcome_message)
     except SlackError:
         logger.exception('Slack API Error')
-        # TODO(jrbotros): for now, using slack on a per-worker basis is
-        # optional; we'll want to rethink this in the future
 
 
 @run_if('ORCHESTRA_SLACK_EXPERTS_ENABLED')
@@ -88,12 +85,16 @@ def create_project_slack_group(project):
     Create slack channel for project team communication
     """
     slack = OrchestraSlackService()
-    response = slack.groups.create(_project_slack_group_name(project))
-    project.slack_group_id = response.body['group']['id']
-    slack.groups.set_topic(project.slack_group_id, project.short_description)
-    slack.groups.set_purpose(project.slack_group_id,
-                             'Discussing work on `{}`'.format(
-                                 project.short_description))
+    response = slack.conversations.create(
+        _project_slack_group_name(project),
+        is_private=True)
+    project.slack_group_id = response.body['channel']['id']
+    slack.conversations.set_topic(
+        project.slack_group_id, project.short_description)
+    slack.conversations.set_purpose(
+        project.slack_group_id,
+        'Discussing work on `{}`'.format(
+            project.short_description))
     project.save()
 
     # Message out project folder id.
@@ -113,7 +114,7 @@ def archive_project_slack_group(project):
     """
     slack = OrchestraSlackService()
     try:
-        response = slack.groups.archive(project.slack_group_id)
+        response = slack.conversations.archive(project.slack_group_id)
         if response:
             is_archived = response.body.get('ok')
             if not is_archived:
@@ -130,10 +131,10 @@ def unarchive_project_slack_group(project):
     """
     slack = OrchestraSlackService()
     try:
-        group_info = slack.groups.info(project.slack_group_id)
-        is_archived = group_info.body.get('group', {}).get('is_archived')
+        group_info = slack.conversations.info(project.slack_group_id)
+        is_archived = group_info.body.get('channel', {}).get('is_archived')
         if is_archived:
-            response = slack.groups.unarchive(project.slack_group_id)
+            response = slack.conversations.unarchive(project.slack_group_id)
             if response:
                 is_unarchived = response.body.get('ok')
                 if not is_unarchived:
@@ -160,11 +161,13 @@ def _project_slack_group_name(project):
     # slugifying the project short description.
     descriptor = slugify(project.short_description)[:16].strip('-')
     slack = OrchestraSlackService()
-    groups = {group['name'] for group in slack.groups.list().body['groups']}
+    channels = {
+        channel['name']
+        for channel in slack.conversations.list().body['channels']}
     while True:
         # Add 4 characters of randomness (~1.68 million permutations).
         name = '{}-{}'.format(descriptor, _random_string())
-        if name not in groups:
+        if name not in channels:
             break
     return name
 

orchestra/communication/tests/helpers/slack.py

@@ -8,7 +8,7 @@
 
 
 MOCK_SLACK_API_DATA = {
-    'groups': {},
+    'channels': {},
     'users': {},
     'invited': [],
 }
@@ -18,20 +18,20 @@ class MockSlacker(MagicMock):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.data = MOCK_SLACK_API_DATA
-        self.groups = Groups()
+        self.conversations = Conversations()
         self.chat = Chat()
         self.users = Users()
         self.populate_preexisting_groups()
 
     def populate_preexisting_groups(self):
         for group_name in PREEXISTING_GROUPS:
-            self.groups.create(group_name)
+            self.conversations.create(group_name, is_private=True)
 
     def get_messages(self, group_id):
-        return MOCK_SLACK_API_DATA['groups'][group_id]['messages']
+        return MOCK_SLACK_API_DATA['channels'][group_id]['messages']
 
     def clear(self):
-        MOCK_SLACK_API_DATA['groups'].clear()
+        MOCK_SLACK_API_DATA['channels'].clear()
         MOCK_SLACK_API_DATA['users'].clear()
         MOCK_SLACK_API_DATA['invited'][:] = []
         self.populate_preexisting_groups()
@@ -44,7 +44,7 @@ def __init__(self, body):
             self.body = body
 
     def _group_exists(self, group_id):
-        return MOCK_SLACK_API_DATA['groups'].get(group_id, None) is not None
+        return MOCK_SLACK_API_DATA['channels'].get(group_id, None) is not None
 
     def _user_exists(self, user_id):
         return MOCK_SLACK_API_DATA['users'].get(user_id, None) is not None
@@ -79,10 +79,10 @@ def accept_invite(self, email, username):
         }
 
 
-class Groups(BaseAPI):
-    def create(self, group_name):
-        group_id = str(len(MOCK_SLACK_API_DATA['groups']))
-        MOCK_SLACK_API_DATA['groups'][group_id] = {
+class Conversations(BaseAPI):
+    def create(self, group_name, is_private=False):
+        group_id = str(len(MOCK_SLACK_API_DATA['channels']))
+        MOCK_SLACK_API_DATA['channels'][group_id] = {
             'id': group_id,
             'users': [],
             'messages': [],
@@ -91,25 +91,22 @@ def create(self, group_name):
             'name': group_name.strip('#'),
         }
         return self.Response({
-            'group': MOCK_SLACK_API_DATA['groups'][group_id]
+            'channel': MOCK_SLACK_API_DATA['channels'][group_id]
         })
 
     def invite(self, group_id, user_id):
         self._validate_group(group_id=group_id)
         self._validate_user(user_id=user_id)
-
-        already_in_group = True
-        if user_id not in MOCK_SLACK_API_DATA['groups'][group_id]['users']:
+        if user_id not in MOCK_SLACK_API_DATA['channels'][group_id]['users']:
             # Slacker API does not raise an error if user already present
-            MOCK_SLACK_API_DATA['groups'][group_id]['users'].append(user_id)
-            already_in_group = False
-        return self.Response({'already_in_group': already_in_group})
+            MOCK_SLACK_API_DATA['channels'][group_id]['users'].append(user_id)
+        return self.Response({})
 
     def info(self, group_id):
         self._validate_group(group_id=group_id)
         return self.Response({
             'ok': True,
-            'group': {
+            'channel': {
                 'id': group_id,
                 'is_archived': True
             }
@@ -119,26 +116,26 @@ def kick(self, group_id, user_id):
         self._validate_group(group_id=group_id)
         self._validate_user(user_id=user_id)
 
-        if user_id not in MOCK_SLACK_API_DATA['groups'][group_id]['users']:
+        if user_id not in MOCK_SLACK_API_DATA['channels'][group_id]['users']:
             raise slacker.Error('User does not belong to group.')
 
         # Slacker API does not raise an error if user already present
-        MOCK_SLACK_API_DATA['groups'][group_id]['users'].remove(user_id)
+        MOCK_SLACK_API_DATA['channels'][group_id]['users'].remove(user_id)
 
     def set_topic(self, group_id, topic):
         self._validate_group(group_id=group_id)
-        MOCK_SLACK_API_DATA['groups'][group_id]['topic'] = topic
+        MOCK_SLACK_API_DATA['channels'][group_id]['topic'] = topic
 
     def set_purpose(self, group_id, purpose):
         if not self._group_exists(group_id):
             raise slacker.Error('Group not found.')
 
-        MOCK_SLACK_API_DATA['groups'][group_id]['purpose'] = purpose
+        MOCK_SLACK_API_DATA['channels'][group_id]['purpose'] = purpose
 
     def list(self):
         return self.Response({
             'ok': True,
-            'groups': list(MOCK_SLACK_API_DATA['groups'].values())
+            'channels': list(MOCK_SLACK_API_DATA['channels'].values())
         })
 
     def archive(self, group_id):
@@ -158,12 +155,12 @@ class Chat(BaseAPI):
     def post_message(self, group_identifier, text, parse='none'):
         if group_identifier.startswith('#'):
             groups = [
-                group for group in MOCK_SLACK_API_DATA['groups'].values()
+                group for group in MOCK_SLACK_API_DATA['channels'].values()
                 if group['name'] == group_identifier.strip('#')]
             group_identifier = groups[0]['id']
         self._validate_group(group_id=group_identifier)
         MOCK_SLACK_API_DATA[
-            'groups'][group_identifier]['messages'].append(text)
+            'channels'][group_identifier]['messages'].append(text)
 
 
 class Users(BaseAPI):

orchestra/interface_api/project_management/project_management.py

@@ -55,8 +55,8 @@ def edit_slack_membership(project_id, username, action):
     slack_user_id = Worker.objects.get(user__username=username).slack_user_id
     slack_group_id = Project.objects.get(id=project_id).slack_group_id
     if action == 'add':
-        slack.groups.invite(slack_group_id, slack_user_id)
+        slack.conversations.invite(slack_group_id, slack_user_id)
     elif action == 'remove':
-        slack.groups.kick(slack_group_id, slack_user_id)
+        slack.conversations.kick(slack_group_id, slack_user_id)
     else:
         raise Exception('Action not found.')

orchestra/interface_api/project_management/tests/test_project_management_api.py

@@ -260,8 +260,9 @@ def test_complete_and_skip_task_api(self):
         self.assertEqual(task.status, Task.Status.COMPLETE)
 
     @override_settings(ORCHESTRA_SLACK_EXPERTS_ENABLED=True)
-    @patch('orchestra.communication.tests.helpers.slack.Groups.unarchive')
-    @patch('orchestra.communication.tests.helpers.slack.Groups.archive')
+    @patch(
+        'orchestra.communication.tests.helpers.slack.Conversations.unarchive')
+    @patch('orchestra.communication.tests.helpers.slack.Conversations.archive')
     def test_unarchive_slack_channel_api(self, mock_archive, mock_unarchive):
         project = self.projects['single_human_step']
         create_subsequent_tasks(project)

orchestra/tests/test_project.py

@@ -39,7 +39,7 @@ def test_create_project_google_folder(self):
 
     @override_settings(ORCHESTRA_SLACK_EXPERTS_ENABLED=True)
     def test_create_project_slack_group(self):
-        groups = self.slack.data['groups']
+        groups = self.slack.data['channels']
         num_groups = len(groups)
         project = ProjectFactory(
             workflow_version=self.workflow_versions['test_workflow'])
@@ -51,7 +51,7 @@ def test_create_project_slack_group(self):
         self.assertEqual(group_id, project.slack_group_id)
 
     @override_settings(ORCHESTRA_SLACK_EXPERTS_ENABLED=True)
-    @patch('orchestra.communication.tests.helpers.slack.Groups.archive')
+    @patch('orchestra.communication.tests.helpers.slack.Conversations.archive')
     def test_complete_all_tasks_slack_annoucement(self, mock_slack_archive):
         project = self.projects['single_human_step']
         create_subsequent_tasks(project)
@@ -67,7 +67,7 @@ def test_complete_all_tasks_slack_annoucement(self, mock_slack_archive):
         self.assertTrue(mock_slack_archive.called)
 
     @override_settings(ORCHESTRA_SLACK_EXPERTS_ENABLED=True)
-    @patch('orchestra.communication.tests.helpers.slack.Groups.archive')
+    @patch('orchestra.communication.tests.helpers.slack.Conversations.archive')
     def test_completion_ends_project_true(self, mock_slack_archive):
         project = self.projects['test_human_and_machine']
         create_subsequent_tasks(project)
@@ -91,7 +91,7 @@ def test_completion_ends_project_true(self, mock_slack_archive):
         self.assertTrue(mock_slack_archive.called)
 
     @override_settings(ORCHESTRA_SLACK_EXPERTS_ENABLED=True)
-    @patch('orchestra.communication.tests.helpers.slack.Groups.archive')
+    @patch('orchestra.communication.tests.helpers.slack.Conversations.archive')
     def test_completion_ends_project_false(self, mock_slack_archive):
         project = self.projects['test_human_and_machine']
         create_subsequent_tasks(project)

orchestra/utils/tests/test_notifications.py

@@ -31,7 +31,7 @@ def test_notify_status_change(self):
         project = self.projects['empty_project']
         internal_name = settings.SLACK_INTERNAL_NOTIFICATION_CHANNEL.strip('#')
         internal_groups = [
-            group for group in self.slack.groups.list().body['groups']
+            group for group in self.slack.conversations.list().body['channels']
             if group['name'] == internal_name]
         internal_group_id = internal_groups[0]['id']
         internal_slack_messages = self.slack.get_messages(internal_group_id)
@@ -262,7 +262,7 @@ def fake_random_string():
 
         for group_name in ('ketchup-3-sales-1', 'bongo-bash723581-3',
                            'bongo-bash723581-4'):
-            self.slack.groups.create(group_name)
+            self.slack.conversations.create(group_name, is_private=True)
 
         project = self.projects['reject_rev_proj']