Pārlūkot izejas kodu

[slack] Refactor for better end-user usability (#2202)

- Better error messages in message thread for end-user context
- Error message does not clutter chat now
- Detect SQL for Slack users who don't have Hue account but don't send gist
- Refactor unit tests
Harsh Gupta 4 gadi atpakaļ
vecāks
revīzija
41d8010e98

+ 2 - 2
desktop/core/src/desktop/lib/botserver/api.py

@@ -80,8 +80,8 @@ def generate_slack_install_link(request):
 
   return JsonResponse({'link': install_link})
 
-def _send_message(channel_info, message=None, block_element=None):
+def _send_message(channel_info, message=None, block_element=None, message_ts=None):
   try:
-    return slack_client.chat_postMessage(channel=channel_info, text=message, blocks=block_element)
+    return slack_client.chat_postMessage(channel=channel_info, text=message, blocks=block_element, thread_ts=message_ts)
   except Exception as e:
     raise PopupException(_("Error posting message in channel"), detail=e)

+ 1 - 1
desktop/core/src/desktop/lib/botserver/api_tests.py

@@ -81,7 +81,7 @@ class TestApi(object):
       data = json.loads(response.content)
 
       assert_equal(200, response.status_code)
-      chat_postMessage.assert_called_with(channel='channel-1', text='@api_user: message with link', blocks=None)
+      chat_postMessage.assert_called_with(channel='channel-1', text='@api_user: message with link', blocks=None, thread_ts=None)
       assert_true(data.get('ok'))
   
   def test_generate_slack_install_link(self):

+ 31 - 22
desktop/core/src/desktop/lib/botserver/views.py

@@ -85,23 +85,23 @@ def parse_events(request, event):
   message_element = event['blocks'][0].get('elements', []) if event.get('blocks') else []
 
   if event.get('type') == 'message':
-    handle_on_message(request.get_host(), request.is_secure(), channel_id, event.get('bot_id'), message_element, user_id)
+    handle_on_message(request.get_host(), request.is_secure(), channel_id, event.get('bot_id'), message_element, user_id, event.get('ts'))
 
   if event.get('type') == 'link_shared':
     handle_on_link_shared(request.get_host(), channel_id, event.get('message_ts'), event.get('links'), user_id)
 
   if event.get('type') == 'app_mention':
-    handle_on_app_mention(request.get_host(), channel_id, user_id, event.get('text'))
+    handle_on_app_mention(request.get_host(), channel_id, user_id, event.get('text'), event.get('ts'))
 
 
-def handle_on_app_mention(host_domain, channel_id, user_id, text):
+def handle_on_app_mention(host_domain, channel_id, user_id, text, message_ts):
   if text and 'help' in text:
-    msg_block = help_message(user_id)
-    _send_message(channel_id, block_element=msg_block)
+    help_msg_block = help_message(user_id)
+    _send_message(channel_id, block_element=help_msg_block)
 
   if text and 'queries' in text:
     slack_user = check_slack_user_permission(host_domain, user_id)
-    user = get_user(channel_id, slack_user)
+    user = get_user(channel_id, slack_user, message_ts)
 
     handle_query_bank(channel_id, user_id)
 
@@ -183,7 +183,7 @@ def handle_query_bank(channel_id, user_id):
   _send_message(channel_id, block_element=message_block)
 
 
-def handle_on_message(host_domain, is_http_secure, channel_id, bot_id, elements, user_id):
+def handle_on_message(host_domain, is_http_secure, channel_id, bot_id, elements, user_id, message_ts):
   # Ignore bot's own message since that will cause an infinite loop of messages if we respond.
   if bot_id is not None:
     return HttpResponse(status=200)
@@ -192,26 +192,30 @@ def handle_on_message(host_domain, is_http_secure, channel_id, bot_id, elements,
     text = element_block['elements'][0].get('text', '') if element_block.get('elements') else ''
 
     if 'hello hue' in text.lower():
-      msg_block = help_message(user_id)
-      _send_message(channel_id, block_element=msg_block)
+      help_msg_block = help_message(user_id)
+      _send_message(channel_id, block_element=help_msg_block)
 
     if text.lower().startswith('select'):
-      handle_select_statement(host_domain, is_http_secure, channel_id, user_id, text.lower())
+      handle_select_statement(host_domain, is_http_secure, channel_id, user_id, text.lower(), message_ts)
 
 
-def handle_select_statement(host_domain, is_http_secure, channel_id, user_id, statement):
+def handle_select_statement(host_domain, is_http_secure, channel_id, user_id, statement, message_ts):
+  msg = 'Hi <@{user}> \n Looks like you are copy/pasting SQL, instead now you can send Editor links which unfurls in a rich preview!'.format(user=user_id)
+  _send_message(channel_id, message=msg)
+
+  # Check Slack user perms to send gist link
   slack_user = check_slack_user_permission(host_domain, user_id)
-  user = get_user(channel_id, slack_user)
+  user = get_user(channel_id, slack_user, message_ts)
+
+  _make_select_statement_gist(host_domain, is_http_secure, user, channel_id, statement)
 
-  default_dialect = get_cluster_config(rewrite_user(user))['main_button_action']['dialect']
 
+def _make_select_statement_gist(host_domain, is_http_secure, user, channel_id, statement):
+  default_dialect = get_cluster_config(rewrite_user(user))['main_button_action']['dialect']
   gist_response = _gist_create(host_domain, is_http_secure, user, statement, default_dialect)
 
-  bot_message = ('Hi <@{user}>\n'
-  'Looks like you are copy/pasting SQL, instead now you can send Editor links which unfurls in a rich preview!\n'
-  'Here is the gist link\n {gist_link}'
-  ).format(user=user_id, gist_link=gist_response['link'])
-  _send_message(channel_id, bot_message)
+  msg = 'Here is the gist link\n {gist_link}'.format(gist_link=gist_response['link'])
+  _send_message(channel_id, message=msg)
 
 
 def handle_on_link_shared(host_domain, channel_id, message_ts, links, user_id):
@@ -228,14 +232,19 @@ def handle_on_link_shared(host_domain, channel_id, message_ts, links, user_id):
         doc = _get_gist_document(**query_id)
         doc_type = 'gist'
       else:
+        err_msg = 'Could not access the query, please check the link again.'
+        _send_message(channel_id, message=err_msg, message_ts=message_ts)
         raise SlackBotException(_("Cannot unfurl link"))
     except Document2.DoesNotExist:
+      err_msg = 'Query document not found or does not exist.'
+      _send_message(channel_id, message=err_msg, message_ts=message_ts)
+
       msg = "Document with {key} does not exist".format(key=query_id)
       raise SlackBotException(_(msg))
 
     # Permission check for Slack user to be Hue user
     slack_user = check_slack_user_permission(host_domain, user_id)
-    user = get_user(channel_id, slack_user) if not slack_user['is_bot'] else doc.owner
+    user = get_user(channel_id, slack_user, message_ts) if not slack_user['is_bot'] else doc.owner
     doc.can_read_or_exception(user)
 
     request = MockRequest(user=rewrite_user(user))
@@ -251,12 +260,12 @@ def handle_on_link_shared(host_domain, channel_id, message_ts, links, user_id):
       send_result_file(request, channel_id, message_ts, doc, 'xls')
 
 
-def get_user(channel_id, slack_user):
+def get_user(channel_id, slack_user, message_ts):
   try:
     return User.objects.get(username=slack_user.get('user_email_prefix'))
   except User.DoesNotExist:
-    bot_message = 'Corresponding Hue user not found or does not have access to the query'
-    _send_message(channel_id, bot_message)
+    err_msg = 'Corresponding Hue user not found or does not have access.'
+    _send_message(channel_id, message=err_msg, message_ts=message_ts)
     raise SlackBotException(_("Slack user does not have access to the query"))
 
 

+ 191 - 203
desktop/core/src/desktop/lib/botserver/views_tests.py

@@ -60,13 +60,15 @@ class TestBotServer(unittest.TestCase):
 
     self.email_domain = '.'.join(self.host_domain.split('.')[-2:])
 
+    self.channel_id = "channel"
+    self.message_ts = "12.1"
+    self.user_id = "user_id"
+
   def test_handle_on_message(self):
     with patch('desktop.lib.botserver.views._send_message') as _send_message:
       with patch('desktop.lib.botserver.views.handle_select_statement') as handle_select_statement:
-      
-        channel_id = "channel"
+
         bot_id = "bot_id"
-        user_id = "user_id"
         message_element = [{
           'elements': [{
             'text': 'hello hue test'
@@ -74,7 +76,7 @@ class TestBotServer(unittest.TestCase):
         }]
 
         # Bot sending message
-        response = handle_on_message(self.host_domain, self.is_http_secure, channel_id, bot_id, message_element, user_id)
+        response = handle_on_message(self.host_domain, self.is_http_secure, self.channel_id, bot_id, message_element, self.user_id, self.message_ts)
         assert_equal(response.status_code, 200)
         assert_false(_send_message.called)
 
@@ -122,8 +124,8 @@ class TestBotServer(unittest.TestCase):
         ]
 
         # Help message
-        handle_on_message(self.host_domain, self.is_http_secure, channel_id, None, message_element, user_id)
-        _send_message.assert_called_with(channel_id, block_element=help_block)
+        handle_on_message(self.host_domain, self.is_http_secure, self.channel_id, None, message_element, self.user_id, self.message_ts)
+        _send_message.assert_called_with(self.channel_id, block_element=help_block)
 
         # Detect SQL
         message_element = [
@@ -139,38 +141,32 @@ class TestBotServer(unittest.TestCase):
           },
         ]
 
-        handle_on_message(self.host_domain, self.is_http_secure, channel_id, None, message_element, user_id)
-        handle_select_statement.assert_called_with(self.host_domain, self.is_http_secure, channel_id, user_id, 'select 1')
+        handle_on_message(self.host_domain, self.is_http_secure, self.channel_id, None, message_element, self.user_id, self.message_ts)
+        handle_select_statement.assert_called_with(self.host_domain, self.is_http_secure, self.channel_id, self.user_id, 'select 1', self.message_ts)
 
   def test_handle_select_statement(self):
-    with patch('desktop.lib.botserver.views.check_slack_user_permission') as check_slack_user:
-      with patch('desktop.lib.botserver.views.get_user') as get_user:
-        with patch('desktop.lib.botserver.views.get_cluster_config') as get_cluster_config:
-          with patch('desktop.lib.botserver.views._send_message') as _send_message:
-            with patch('desktop.lib.botserver.views._gist_create') as _gist_create:
+    with patch('desktop.lib.botserver.views.check_slack_user_permission') as check_slack_user_permission:
+      with patch('desktop.lib.botserver.views._make_select_statement_gist') as _make_select_statement_gist:
+        with patch('desktop.lib.botserver.views._send_message') as _send_message:
+          with patch('desktop.lib.botserver.views.get_user') as get_user:
 
-              channel_id = "channel"
-              user_id = "user_id"
-              statement = 'select 1'
+            statement = 'select 1'
+            detect_msg = 'Hi <@user_id> \n Looks like you are copy/pasting SQL, instead now you can send Editor links which unfurls in a rich preview!'
 
-              get_user.return_value = self.user
+            # For Slack user not Hue user
+            get_user.side_effect = SlackBotException('Slack user does not have access to the query')
 
-              get_cluster_config.return_value = {
-                'main_button_action': {
-                  'dialect': 'hive'
-                },
-              }
-              _gist_create.return_value = {
-                'link': 'gist_link'
-              }
+            assert_raises(SlackBotException, handle_select_statement, self.host_domain, self.is_http_secure, self.channel_id, self.user_id, statement, self.message_ts)
+            _send_message.assert_called_with('channel', message=detect_msg)
+            assert_false(_make_select_statement_gist.called)
+
+            # For Slack user is Hue user
+            get_user.side_effect = None
+            get_user.return_value = self.user
 
-              handle_select_statement(self.host_domain, self.is_http_secure, channel_id, user_id, statement)
-              _send_message.assert_called_with(
-                channel_id, 
-                ('Hi <@user_id>\n'
-                'Looks like you are copy/pasting SQL, instead now you can send Editor links which unfurls in a rich preview!\n'
-                'Here is the gist link\n gist_link')
-              )
+            handle_select_statement(self.host_domain, self.is_http_secure, self.channel_id, self.user_id, statement, self.message_ts)
+            _send_message.assert_called_with('channel', message=detect_msg)
+            _make_select_statement_gist.assert_called_with(self.host_domain, self.is_http_secure, self.user, 'channel', 'select 1')
 
   def test_handle_query_history_link(self):
     with patch('desktop.lib.botserver.views.slack_client.chat_unfurl') as chat_unfurl:
@@ -179,226 +175,218 @@ class TestBotServer(unittest.TestCase):
           with patch('desktop.lib.botserver.views.slack_client.users_info') as users_info:
             with patch('desktop.lib.botserver.views._query_result') as query_result:
               with patch('desktop.lib.botserver.views._make_result_table') as result_table:
+                with patch('desktop.lib.botserver.views._send_message') as _send_message:
+
+                  doc_data = {
+                    "dialect": "mysql",
+                    "snippets": [{
+                      "statement_raw": "SELECT 5000",
+                    }],
+                    'uuid': 'doc uuid'
+                  }
+                  doc = Document2.objects.create(data=json.dumps(doc_data), owner=self.user)
+                  links = [{"url": "https://{host_domain}/hue/editor?editor=".format(host_domain=self.host_domain) + str(doc.id)}]
+
+                  # Slack user is Hue user but without read access sends link
+                  users_info.return_value = {
+                    "ok": True,
+                    "user": {
+                      "is_bot": False,
+                      "profile": {
+                        "email": "test_not_me@{domain}".format(domain=self.email_domain)
+                      }
+                    }
+                  }
+                  assert_raises(PopupException, handle_on_link_shared, self.host_domain, "channel", "12.1", links, "<@user_id>")
 
-                channel_id = "channel"
-                message_ts = "12.1"
-                user_id = "<@user_id>"
+                  # Slack user is Hue user with read access sends link
+                  doc.update_permission(self.user, is_link_on=True)
 
-                doc_data = {
-                  "dialect": "mysql",
-                  "snippets": [{
-                    "statement_raw": "SELECT 5000",
-                  }],
-                  'uuid': 'doc uuid'
-                }
-                doc = Document2.objects.create(data=json.dumps(doc_data), owner=self.user)
-                links = [{"url": "https://{host_domain}/hue/editor?editor=".format(host_domain=self.host_domain) + str(doc.id)}]
-
-                # Slack user is Hue user but without read access sends link
-                users_info.return_value = {
-                  "ok": True,
-                  "user": {
-                    "is_bot": False,
-                    "profile": {
-                      "email": "test_not_me@{domain}".format(domain=self.email_domain)
-                    }
+                  check_status.return_value = {'query_status': {'status': 'available'}, 'status': 0}
+                  query_result.return_value = {
+                    'data': [[5000]],
+                    'meta': [{'comment': '', 'name': '5000', 'type': 'INT_TYPE'}],
                   }
-                }
-                assert_raises(PopupException, handle_on_link_shared, self.host_domain, "channel", "12.1", links, "<@user_id>")
+                  result_table.return_value = '  Columns(1)\n------------  ----\n        5000  5000'
 
-                # Slack user is Hue user with read access sends link
-                doc.update_permission(self.user, is_link_on=True)
+                  handle_on_link_shared(self.host_domain, self.channel_id, self.message_ts, links, self.user_id)
 
-                check_status.return_value = {'query_status': {'status': 'available'}, 'status': 0}
-                query_result.return_value = {
-                  'data': [[5000]],
-                  'meta': [{'comment': '', 'name': '5000', 'type': 'INT_TYPE'}],
-                }
-                result_table.return_value = '  Columns(1)\n------------  ----\n        5000  5000'
-
-                handle_on_link_shared(self.host_domain, channel_id, message_ts, links, user_id)
-
-                query_preview = {
-                  links[0]['url']: {
-                  "color": "#025BA6",
-                  "blocks": [
-                    {
-                      "type": "section",
-                      "text": {
-                        "type": "mrkdwn",
-                        "text": "\n*<{url}|Open  query of mysql dialect created by test in Hue>*".format(url=links[0]['url']),
-                        }
-                    },
-                    {
-                      "type": "divider",
-                    },
-                    {
-                      "type": "section",
-                      "text": {
-                        "type": "mrkdwn",
-                        "text": "*Statement:*\n```SELECT 5000```",
-                        }
+                  query_preview = {
+                    links[0]['url']: {
+                    "color": "#025BA6",
+                    "blocks": [
+                      {
+                        "type": "section",
+                        "text": {
+                          "type": "mrkdwn",
+                          "text": "\n*<{url}|Open  query of mysql dialect created by test in Hue>*".format(url=links[0]['url']),
+                          }
                       },
                       {
-                        'type': 'section',
-                        'text': {
-                          'type': 'mrkdwn',
-                          'text': "*Query result:*\n```  Columns(1)\n------------  ----\n        5000  5000```",
+                        "type": "divider",
+                      },
+                      {
+                        "type": "section",
+                        "text": {
+                          "type": "mrkdwn",
+                          "text": "*Statement:*\n```SELECT 5000```",
+                          }
+                        },
+                        {
+                          'type': 'section',
+                          'text': {
+                            'type': 'mrkdwn',
+                            'text': "*Query result:*\n```  Columns(1)\n------------  ----\n        5000  5000```",
+                          }
                         }
-                      }
-                    ]
+                      ]
+                    }
                   }
-                }
 
-                chat_unfurl.assert_called_with(channel=channel_id, ts=message_ts, unfurls=query_preview)
-                assert_true(send_result_file.called)
+                  chat_unfurl.assert_called_with(channel=self.channel_id, ts=self.message_ts, unfurls=query_preview)
+                  assert_true(send_result_file.called)
 
-                # Document does not exist
-                qhistory_url = "https://{host_domain}/hue/editor?editor=109644".format(host_domain=self.host_domain)
-                assert_raises(PopupException, handle_on_link_shared, self.host_domain, "channel", "12.1", [{"url": qhistory_url}], "<@user_id>")
+                  # Document does not exist
+                  qhistory_url = "https://{host_domain}/hue/editor?editor=109644".format(host_domain=self.host_domain)
+                  assert_raises(SlackBotException, handle_on_link_shared, self.host_domain, "channel", "12.1", [{"url": qhistory_url}], "<@user_id>")
+                  _send_message.assert_called_with('channel', message='Query document not found or does not exist.', message_ts='12.1')
 
-                # Cannot unfurl link with invalid query link
-                inv_qhistory_url = "https://{host_domain}/hue/editor/?type=4".format(host_domain=self.host_domain)
-                assert_raises(PopupException, handle_on_link_shared, self.host_domain, "channel", "12.1", [{"url": inv_qhistory_url}], "<@user_id>")
+                  # Cannot unfurl link with invalid query link
+                  inv_qhistory_url = "https://{host_domain}/hue/editor/?type=4".format(host_domain=self.host_domain)
+                  assert_raises(SlackBotException, handle_on_link_shared, self.host_domain, "channel", "12.1", [{"url": inv_qhistory_url}], "<@user_id>")
+                  _send_message.assert_called_with('channel', message='Could not access the query, please check the link again.', message_ts='12.1')
 
   def test_handle_gist_link(self):
     with patch('desktop.lib.botserver.views.slack_client.chat_unfurl') as chat_unfurl:
       with patch('desktop.lib.botserver.views.slack_client.users_info') as users_info:
         with patch('desktop.lib.botserver.views.send_result_file') as send_result_file:
+          with patch('desktop.lib.botserver.views._send_message') as _send_message:
 
-          channel_id = "channel"
-          message_ts = "12.1"
-          user_id = "<@user_id>"
-
-          doc_data = {"statement_raw": "SELECT 98765"}
-          gist_doc = Document2.objects.create(
-            name='Mysql Query',
-            type='gist',
-            owner=self.user,
-            data=json.dumps(doc_data),
-            extra='mysql'
-          )
-          links = [{"url": "https://{host_domain}/hue/gist?uuid=".format(host_domain=self.host_domain) + gist_doc.uuid}]
-
-          # Slack user who is Hue user sends link
-          users_info.return_value = {
-            "ok": True,
-            "user": {
-              "is_bot": False,
-              "profile": {
-                "email": "test@{domain}".format(domain=self.email_domain)
+            doc_data = {"statement_raw": "SELECT 98765"}
+            gist_doc = Document2.objects.create(
+              name='Mysql Query',
+              type='gist',
+              owner=self.user,
+              data=json.dumps(doc_data),
+              extra='mysql'
+            )
+            links = [{"url": "https://{host_domain}/hue/gist?uuid=".format(host_domain=self.host_domain) + gist_doc.uuid}]
+
+            # Slack user who is Hue user sends link
+            users_info.return_value = {
+              "ok": True,
+              "user": {
+                "is_bot": False,
+                "profile": {
+                  "email": "test@{domain}".format(domain=self.email_domain)
+                }
               }
             }
-          }
-          handle_on_link_shared(self.host_domain, channel_id, message_ts, links, user_id)
-
-          gist_preview = {
-            links[0]['url']: {
-            "color": "#025BA6",
-            "blocks": [
-              {
-                "type": "section",
-                "text": {
-                  "type": "mrkdwn",
-                  "text": "\n*<{url}|Open Mysql Query gist of mysql dialect created by test in Hue>*".format(url=links[0]['url']),
-                  }
-              },
-              {
-                "type": "divider",
-              },
-              {
-                "type": "section",
-                "text": {
-                  "type": "mrkdwn",
-                  "text": "*Statement:*\n```SELECT 98765```",
-                  }
+            handle_on_link_shared(self.host_domain, self.channel_id, self.message_ts, links, self.user_id)
+
+            gist_preview = {
+              links[0]['url']: {
+              "color": "#025BA6",
+              "blocks": [
+                {
+                  "type": "section",
+                  "text": {
+                    "type": "mrkdwn",
+                    "text": "\n*<{url}|Open Mysql Query gist of mysql dialect created by test in Hue>*".format(url=links[0]['url']),
+                    }
+                },
+                {
+                  "type": "divider",
                 },
-              ]
+                {
+                  "type": "section",
+                  "text": {
+                    "type": "mrkdwn",
+                    "text": "*Statement:*\n```SELECT 98765```",
+                    }
+                  },
+                ]
+              }
             }
-          }
 
-          chat_unfurl.assert_called_with(channel=channel_id, ts=message_ts, unfurls=gist_preview)
-          assert_false(send_result_file.called)
+            chat_unfurl.assert_called_with(channel=self.channel_id, ts=self.message_ts, unfurls=gist_preview)
+            assert_false(send_result_file.called)
 
-          # Gist link sent directly from Hue to Slack via bot
-          users_info.return_value = {
-            "ok": True,
-            "user": {
-              "is_bot": True,
+            # Gist link sent directly from Hue to Slack via bot
+            users_info.return_value = {
+              "ok": True,
+              "user": {
+                "is_bot": True,
+              }
             }
-          }
-          handle_on_link_shared(self.host_domain, channel_id, message_ts, links, user_id)
+            handle_on_link_shared(self.host_domain, self.channel_id, self.message_ts, links, self.user_id)
 
-          chat_unfurl.assert_called_with(channel=channel_id, ts=message_ts, unfurls=gist_preview)
-          assert_false(send_result_file.called)
+            chat_unfurl.assert_called_with(channel=self.channel_id, ts=self.message_ts, unfurls=gist_preview)
+            assert_false(send_result_file.called)
 
-          # Gist document does not exist
-          gist_url = "https://{host_domain}/hue/gist?uuid=6d1c407b-d999-4dfd-ad23-d3a46c19a427".format(host_domain=self.host_domain)
-          assert_raises(PopupException, handle_on_link_shared, self.host_domain, "channel", "12.1", [{"url": gist_url}], "<@user_id>")
+            # Gist document does not exist
+            gist_url = "https://{host_domain}/hue/gist?uuid=6d1c407b-d999-4dfd-ad23-d3a46c19a427".format(host_domain=self.host_domain)
+            assert_raises(SlackBotException, handle_on_link_shared, self.host_domain, "channel", "12.1", [{"url": gist_url}], "<@user_id>")
+            _send_message.assert_called_with('channel', message='Query document not found or does not exist.', message_ts='12.1')
 
-          # Cannot unfurl with invalid gist link
-          inv_gist_url = "https://{host_domain}/hue/gist?uuids/=invalid_link".format(host_domain=self.host_domain)
-          assert_raises(PopupException, handle_on_link_shared, self.host_domain, "channel", "12.1", [{"url": inv_gist_url}], "<@user_id>")
+            # Cannot unfurl with invalid gist link
+            inv_gist_url = "https://{host_domain}/hue/gist?uuids/=invalid_link".format(host_domain=self.host_domain)
+            assert_raises(SlackBotException, handle_on_link_shared, self.host_domain, "channel", "12.1", [{"url": inv_gist_url}], "<@user_id>")
+            _send_message.assert_called_with('channel', message='Could not access the query, please check the link again.', message_ts='12.1')
 
   def test_slack_user_not_hue_user(self):
     with patch('desktop.lib.botserver.views.slack_client.users_info') as users_info:
-      with patch('desktop.lib.botserver.views._get_gist_document') as _get_gist_document:
-        with patch('desktop.lib.botserver.views.slack_client.chat_postMessage') as chat_postMessage:
-        
-          # Can be checked similarly with query link too
-          doc_data = {"statement_raw": "SELECT 98765"}
-          links = [{"url": "https://{host_domain}/hue/gist?uuid=some_uuid".format(host_domain=self.host_domain)}]
-          _get_gist_document.return_value = Mock(data=json.dumps(doc_data), owner=self.user, extra='mysql')
-
-          # Same domain but diff email prefix
-          users_info.return_value = {
-            "ok": True,
-            "user": {
-              "is_bot": False,
-              "profile": {
-                "email": "test_user_not_exist@{domain}".format(domain=self.email_domain)
-              }
+      with patch('desktop.lib.botserver.views._send_message') as _send_message:
+
+        # Same domain but diff email prefix
+        users_info.return_value = {
+          "ok": True,
+          "user": {
+            "is_bot": False,
+            "profile": {
+              "email": "test_user_not_exist@{domain}".format(domain=self.email_domain)
             }
           }
-          assert_raises(PopupException, handle_on_link_shared, self.host_domain, "channel", "12.1", links, "<@user_id>")
-
-          # Different domain but same email prefix
-          users_info.return_value = {
-            "ok": True,
-            "user": {
-              "is_bot": False,
-              "profile": {
-                "email": "test@example.com"
-              }
+        }
+        slack_user = check_slack_user_permission(self.host_domain, self.user_id)
+
+        assert_raises(SlackBotException, get_user, "channel", slack_user, "12.1")
+        _send_message.assert_called_with('channel', message='Corresponding Hue user not found or does not have access.', message_ts='12.1')
+
+        # Different domain but same email prefix
+        users_info.return_value = {
+          "ok": True,
+          "user": {
+            "is_bot": False,
+            "profile": {
+              "email": "test@example.com"
             }
           }
-          assert_raises(PopupException, handle_on_link_shared, self.host_domain, "channel", "12.1", links, "<@user_id>")
+        }
+        slack_user = check_slack_user_permission(self.host_domain, self.user_id)
+
+        assert_raises(SlackBotException, get_user, "channel", slack_user, "12.1")
+        _send_message.assert_called_with('channel', message='Corresponding Hue user not found or does not have access.', message_ts='12.1')
   
   def test_handle_on_app_mention(self):
     with patch('desktop.lib.botserver.views.check_slack_user_permission') as check_slack_user_permission:
       with patch('desktop.lib.botserver.views.get_user') as get_user:
         with patch('desktop.lib.botserver.views.handle_query_bank') as handle_query_bank:
 
-          channel_id = "channel"
-          user_id = "<@user_id>"
-
           text = '@hue some message'
-          handle_on_app_mention(self.host_domain, channel_id, user_id, text)
+          handle_on_app_mention(self.host_domain, self.channel_id, self.user_id, text, self.message_ts)
 
           assert_false(handle_query_bank.called)
 
           text = '@hue queries'
-          handle_on_app_mention(self.host_domain, channel_id, user_id, text)
+          handle_on_app_mention(self.host_domain, self.channel_id, self.user_id, text, self.message_ts)
 
-          handle_query_bank.assert_called_with(channel_id, user_id)
+          handle_query_bank.assert_called_with(self.channel_id, self.user_id)
 
   def test_handle_query_bank(self):
     with patch('desktop.lib.botserver.views.get_all_queries') as get_all_queries:
       with patch('desktop.lib.botserver.views._send_message') as _send_message:
 
-        channel_id = "channel"
-        user_id = "<@user_id>"
-
         get_all_queries.return_value = [
           {
             "name": "Test Query 1",
@@ -421,7 +409,7 @@ class TestBotServer(unittest.TestCase):
         test_query_block = [
           {
             'type': 'section',
-            'text': {'type': 'mrkdwn', 'text': 'Hi <@<@user_id>>, here is the list of all saved queries!'}
+            'text': {'type': 'mrkdwn', 'text': 'Hi <@user_id>, here is the list of all saved queries!'}
           }, 
           {
             'type': 'divider'
@@ -436,5 +424,5 @@ class TestBotServer(unittest.TestCase):
           }
         ]
         
-        handle_query_bank(channel_id, user_id)
-        _send_message.assert_called_with(channel_id, block_element=test_query_block)
+        handle_query_bank(self.channel_id, self.user_id)
+        _send_message.assert_called_with(self.channel_id, block_element=test_query_block)