Browse Source

[slack] Update unit tests and refactor views.py (#1923)

* [slack] Refactor code and update UTs

- Changed None check according to PEP8
- Try/except for Slack API calls
- Separate method for result file upload
- Better PopupException 'message' in UTs
- Updated UTs
Harsh Gupta 4 years ago
parent
commit
80ade93312

+ 37 - 27
desktop/core/src/desktop/lib/botserver/views.py

@@ -92,15 +92,12 @@ def parse_events(event):
 
 def handle_on_message(channel_id, bot_id, text, user_id):
   # Ignore bot's own message since that will cause an infinite loop of messages if we respond.
-  if bot_id:
+  if bot_id is not None:
     return HttpResponse(status=200)
   
-  if slack_client:
+  if slack_client is not None:
     if text and 'hello hue' in text.lower():
-      response = say_hi_user(channel_id, user_id)
-
-      if not response['ok']:
-        raise PopupException(_("Error posting message"), detail=response["error"])
+      send_hi_user(channel_id, user_id)
 
 
 def handle_on_link_shared(channel_id, message_ts, links):
@@ -126,28 +123,38 @@ def handle_on_link_shared(channel_id, message_ts, links):
     request = MockRequest(user=user)
 
     payload = _make_unfurl_payload(request, item['url'], id_type, doc, doc_type)
-    response = slack_client.chat_unfurl(channel=channel_id, ts=message_ts, unfurls=payload['payload'])
-    if not response['ok']:
-      raise PopupException(_("Cannot unfurl link"), detail=response["error"])
+    try:
+      slack_client.chat_unfurl(channel=channel_id, ts=message_ts, unfurls=payload['payload'])
+    except Exception as e:
+      raise PopupException(_("Cannot unfurl link"), detail=e)
     
     # Generate and upload result xlsx file only if result available
     if payload['file_status']:
-      notebook = json.loads(doc.data)
-      snippet = notebook['snippets'][0]
-      snippet['statement'] = notebook['snippets'][0]['statement_raw']
-      file_format = 'xlsx'
-      file_name = _get_snippet_name(notebook)
-
-      content_generator = get_api(request, snippet).download(notebook, snippet, file_format='xls')
-
-      response = slack_client.files_upload(
-        channels=channel_id, 
-        file=next(content_generator), 
-        thread_ts=message_ts,
-        filetype=file_format,
-        filename='{}.{}'.format(file_name, file_format),
-        initial_comment='Here is your result file!'
-        )
+      send_result_file(request, channel_id, message_ts, doc, 'xls')
+
+
+def send_result_file(request, channel_id, message_ts, doc, file_format):
+  notebook = json.loads(doc.data)
+  snippet = notebook['snippets'][0]
+  snippet['statement'] = notebook['snippets'][0]['statement_raw']
+
+  content_generator = get_api(request, snippet).download(notebook, snippet, file_format)
+
+  file_format = 'xlsx'
+  file_name = _get_snippet_name(notebook)
+
+  try:
+    slack_client.files_upload(
+      channels=channel_id,
+      file=next(content_generator), 
+      thread_ts=message_ts,
+      filetype=file_format,
+      filename='{}.{}'.format(file_name, file_format),
+      initial_comment='Here is your result file!'
+    )
+  except Exception as e:
+    raise PopupException(_("Cannot upload result file"), detail=e)
+
 
 def _query_result(request, notebook, max_rows):
   snippet = notebook['snippets'][0]
@@ -256,10 +263,13 @@ def _make_unfurl_payload(request, url, id_type, doc, doc_type):
   return {'payload': payload, 'file_status': file_status}
 
 
-def say_hi_user(channel_id, user_id):
+def send_hi_user(channel_id, user_id):
   """
   Sends Hi<user_id> message in a specific channel.
 
   """
   bot_message = 'Hi <@{}> :wave:'.format(user_id)
-  return slack_client.api_call(api_method='chat.postMessage', json={'channel': channel_id, 'text': bot_message})
+  try:
+    slack_client.api_call(api_method='chat.postMessage', json={'channel': channel_id, 'text': bot_message})
+  except Exception as e:
+    raise PopupException(_("Error posting message"), detail=e)

+ 62 - 51
desktop/core/src/desktop/lib/botserver/views_tests.py

@@ -45,17 +45,19 @@ class TestBotServer(unittest.TestCase):
     if not conf.SLACK.IS_ENABLED.get():
       raise SkipTest
 
-  def test_say_hi_user(self):
+  def test_send_hi_user(self):
     with patch('desktop.lib.botserver.views.slack_client.api_call') as api_call:
       api_call.return_value = {
         "ok": True
       }
-      response = say_hi_user("channel", "user_id")
+      send_hi_user("channel", "user_id")
       api_call.assert_called_with(api_method='chat.postMessage', json={'channel': 'channel', 'text': 'Hi <@user_id> :wave:'})
-      assert_true(response['ok'])
-  
+
+      api_call.side_effect = PopupException('message')
+      assert_raises(PopupException, send_hi_user, "channel", "user_id")
+
   def test_handle_on_message(self):
-    with patch('desktop.lib.botserver.views.say_hi_user') as say_hi_user:
+    with patch('desktop.lib.botserver.views.send_hi_user') as say_hi_user:
       
       response = handle_on_message("channel", "bot_id", "text", "user_id")
       assert_equal(response.status_code, 200)
@@ -75,49 +77,58 @@ class TestBotServer(unittest.TestCase):
       with patch('desktop.lib.botserver.views._make_unfurl_payload') as mock_unfurl_payload:
         with patch('desktop.lib.botserver.views.Document2.objects.get') as document2_objects_get:
           with patch('desktop.lib.botserver.views._get_gist_document') as _get_gist_document:
-        
-            client = make_logged_in_client(username="test", groupname="default", recreate=True, is_superuser=False)
-            user = User.objects.get(username="test")
-            channel_id = "channel"
-            message_ts = "12.1"
-
-            # qhistory link
-            links = [{"url": "https://demo.gethue.com/hue/editor?editor=123456"}]
-            doc_data = {
-              "dialect": "mysql",
-              "snippets": [{
-                "database": "hue",
-                "statement_raw": "SELECT 5000",
-              }]
-            }
-            document2_objects_get.return_value = Mock(data=json.dumps(doc_data), owner=user)
-
-            handle_on_link_shared(channel_id, message_ts, links)
-            
-            mock_unfurl_payload.assert_called_with(links[0]["url"], "SELECT 5000", "Mysql", "test")
-            assert_true(chat_unfurl.called)
-
-            # gist link
-            doc_data = {"statement_raw": "SELECT 98765"}
-            _get_gist_document.return_value = Mock(data=json.dumps(doc_data), owner=user, extra='mysql')
-            links = [{"url": "http://demo.gethue.com/hue/gist?uuid=random"}]
-
-            handle_on_link_shared(channel_id, message_ts, links)
-            
-            mock_unfurl_payload.assert_called_with(links[0]["url"], "SELECT 98765", "Mysql", "test")
-            assert_true(chat_unfurl.called)
-
-            # Cannot unfurl link with invalid links
-            inv_qhistory_url = "https://demo.gethue.com/hue/editor/?type=4"
-            inv_gist_url = "http://demo.gethue.com/hue/gist?uuids/=xyz"
-            assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": inv_qhistory_url}])
-            assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": inv_gist_url}])
-
-            # Document does not exist
-            document2_objects_get.side_effect = PopupException('message')
-            _get_gist_document.side_effect = PopupException('message')
-            
-            qhistory_url = "https://demo.gethue.com/hue/editor?editor=109644"
-            gist_url = "https://demo.gethue.com/hue/gist?uuid=6d1c407b-d999-4dfd-ad23-d3a46c19a427"
-            assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": qhistory_url}])
-            assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": gist_url}])
+            with patch('desktop.lib.botserver.views.send_result_file') as send_result_file:
+
+              client = make_logged_in_client(username="test", groupname="default", recreate=True, is_superuser=False)
+              user = User.objects.get(username="test")
+              channel_id = "channel"
+              message_ts = "12.1"
+
+              # qhistory link
+              links = [{"url": "https://demo.gethue.com/hue/editor?editor=123456"}]
+              doc_data = {
+                "dialect": "mysql",
+                "snippets": [{
+                  "database": "hue",
+                  "statement_raw": "SELECT 5000",
+                }]
+              }
+              document2_objects_get.return_value = Mock(data=json.dumps(doc_data), owner=user)
+              mock_unfurl_payload.return_value = {
+                'payload': {},
+                'file_status': True,
+              }
+              handle_on_link_shared(channel_id, message_ts, links)
+              assert_true(chat_unfurl.called)
+              assert_true(send_result_file.called)
+
+              # gist link
+              doc_data = {"statement_raw": "SELECT 98765"}
+              _get_gist_document.return_value = Mock(data=json.dumps(doc_data), owner=user, extra='mysql')
+              links = [{"url": "http://demo.gethue.com/hue/gist?uuid=random"}]
+
+              mock_unfurl_payload.return_value = {
+                'payload': {},
+                'file_status': False,
+              }
+              handle_on_link_shared(channel_id, message_ts, links)
+              assert_true(chat_unfurl.called)
+
+              # Cannot unfurl link with invalid links
+              inv_qhistory_url = "https://demo.gethue.com/hue/editor/?type=4"
+              inv_gist_url = "http://demo.gethue.com/hue/gist?uuids/=xyz"
+              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": inv_qhistory_url}])
+              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": inv_gist_url}])
+
+              # Document does not exist
+              document2_objects_get.side_effect = PopupException('Query document does not exist')
+              _get_gist_document.side_effect = PopupException('Gist does not exist')
+
+              qhistory_url = "https://demo.gethue.com/hue/editor?editor=109644"
+              gist_url = "https://demo.gethue.com/hue/gist?uuid=6d1c407b-d999-4dfd-ad23-d3a46c19a427"
+              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": qhistory_url}])
+              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": gist_url}])
+
+              # chat_unfurl exception
+              chat_unfurl.side_effect = PopupException('Cannot unfurl link')
+              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", links)