outgoing webhooks: Warn user that PMs are not supported in Slack-format webhook.

zerver/lib/outgoing_webhook.py

@@ -83,7 +83,9 @@ def process_event(self, event: Dict[str, Any]) -> Tuple[Dict[str, Any], Any]:
                           'request_kwargs': {}}
 
         if event['message']['type'] == 'private':
-            raise NotImplementedError("Private messaging service not supported.")
+            failure_message = "Private messaging service not supported."
+            fail_with_message(event, failure_message)
+            return None, None
 
         request_data = [("token", self.token),
                         ("team_id", event['message']['sender_realm_str']),

zerver/tests/test_outgoing_webhook_interfaces.py

@@ -54,7 +54,7 @@ def test_process_success(self) -> None:
 class TestSlackOutgoingWebhookService(ZulipTestCase):
 
     def setUp(self) -> None:
-        self.event = {
+        self.stream_message_event = {
             u'command': '@**test**',
             u'user_profile_id': 12,
             u'service_name': 'test-service',
@@ -71,13 +71,32 @@ def setUp(self) -> None:
                 'sender_full_name': 'Sample User',
             }
         }
+
+        self.private_message_event = {
+            u'user_profile_id': 24,
+            u'service_name': 'test-service',
+            u'command': 'test content',
+            u'trigger': 'private_message',
+            u'message': {
+                'sender_id': 3,
+                'sender_realm_str': 'zulip',
+                'timestamp': 1529821610,
+                'sender_email': 'cordelia@zulip.com',
+                'type': 'private',
+                'sender_realm_id': 1,
+                'id': 219,
+                'subject': 'test',
+                'content': 'test content',
+            }
+        }
+
         self.handler = SlackOutgoingWebhookService(base_url='http://example.domain.com',
                                                    token="abcdef",
                                                    user_profile=None,
                                                    service_name='test-service')
 
-    def test_process_event(self) -> None:
-        rest_operation, request_data = self.handler.process_event(self.event)
+    def test_process_event_stream_message(self) -> None:
+        rest_operation, request_data = self.handler.process_event(self.stream_message_event)
 
         self.assertEqual(rest_operation['base_url'], 'http://example.domain.com')
         self.assertEqual(rest_operation['method'], 'POST')
@@ -93,12 +112,22 @@ def test_process_event(self) -> None:
         self.assertEqual(request_data[9][1], "mention")  # trigger_word
         self.assertEqual(request_data[10][1], 12)  # user_profile_id
 
+    @mock.patch('zerver.lib.outgoing_webhook.get_service_profile', return_value=mock_service)
+    @mock.patch('zerver.lib.outgoing_webhook.fail_with_message')
+    def test_process_event_private_message(self, mock_fail_with_message: mock.Mock,
+                                           mock_get_service_profile: mock.Mock) -> None:
+
+        rest_operation, request_data = self.handler.process_event(self.private_message_event)
+        self.assertIsNone(request_data)
+        self.assertIsNone(rest_operation)
+        self.assertTrue(mock_fail_with_message.called)
+
     def test_process_success(self) -> None:
         response = mock.Mock(spec=Response)
         response.text = json.dumps({"response_not_required": True})
-        success_response, _ = self.handler.process_success(response, self.event)
+        success_response, _ = self.handler.process_success(response, self.stream_message_event)
         self.assertEqual(success_response, None)
 
         response.text = json.dumps({"text": 'test_content'})
-        success_response, _ = self.handler.process_success(response, self.event)
+        success_response, _ = self.handler.process_success(response, self.stream_message_event)
         self.assertEqual(success_response, 'test_content')

zerver/worker/queue_processors.py

@@ -479,7 +479,8 @@ def consume(self, event: Mapping[str, Any]) -> None:
             dup_event['service_name'] = str(service.name)
             service_handler = get_outgoing_webhook_service_handler(service)
             rest_operation, request_data = service_handler.process_event(dup_event)
-            do_rest_call(rest_operation, request_data, dup_event, service_handler)
+            if rest_operation:
+                do_rest_call(rest_operation, request_data, dup_event, service_handler)
 
 @assign_queue('embedded_bots')
 class EmbeddedBotWorker(QueueProcessingWorker):