diff --git a/zerver/lib/outgoing_webhook.py b/zerver/lib/outgoing_webhook.py index f0bcc23d29..70e9cd072b 100644 --- a/zerver/lib/outgoing_webhook.py +++ b/zerver/lib/outgoing_webhook.py @@ -41,12 +41,12 @@ class OutgoingWebhookServiceInterface: raise NotImplementedError() # Given a successful outgoing webhook REST operation, returns two-element tuple - # whose left-hand value contains a success message + # whose left-hand value contains a success dictionary # to sent back to the user (or None if no success message should be sent) # and right-hand value contains a failure message # to sent back to the user (or None if no failure message should be sent) def process_success(self, response: Response, - event: Dict[str, Any]) -> Tuple[Optional[str], Optional[str]]: + event: Dict[str, Any]) -> Tuple[Optional[Dict[str, Any]], Optional[str]]: raise NotImplementedError() class GenericOutgoingWebhookService(OutgoingWebhookServiceInterface): @@ -64,13 +64,16 @@ class GenericOutgoingWebhookService(OutgoingWebhookServiceInterface): return rest_operation, json.dumps(request_data) def process_success(self, response: Response, - event: Dict[str, Any]) -> Tuple[Optional[str], Optional[str]]: + event: Dict[str, Any]) -> Tuple[Optional[Dict[str, Any]], Optional[str]]: response_json = json.loads(response.text) if "response_not_required" in response_json and response_json['response_not_required']: return None, None if "response_string" in response_json: - return str(response_json['response_string']), None + success_data = dict( + response_string=str(response_json['response_string']), + ) + return success_data, None else: return None, None @@ -103,10 +106,12 @@ class SlackOutgoingWebhookService(OutgoingWebhookServiceInterface): return rest_operation, request_data def process_success(self, response: Response, - event: Dict[str, Any]) -> Tuple[Optional[str], Optional[str]]: + event: Dict[str, Any]) -> Tuple[Optional[Dict[str, Any]], Optional[str]]: response_json = json.loads(response.text) if "text" in response_json: - return response_json["text"], None + response_string = response_json['text'] + success_data = dict(response_string=response_string) + return success_data, None else: return None, None @@ -225,9 +230,16 @@ def request_retry(event: Dict[str, Any], def process_success_response(event: Dict[str, Any], service_handler: Any, response: Response) -> None: - success_message, _ = service_handler.process_success(response, event) - if success_message is not None: - succeed_with_message(event, success_message) + success_data, _ = service_handler.process_success(response, event) + + if success_data is None: + return + + success_message = success_data.get('response_string') + if success_message is None: + return + + succeed_with_message(event, success_message) def do_rest_call(rest_operation: Dict[str, Any], request_data: Optional[Dict[str, Any]], diff --git a/zerver/tests/test_outgoing_webhook_interfaces.py b/zerver/tests/test_outgoing_webhook_interfaces.py index 4d3cb1d53b..65f30d9385 100644 --- a/zerver/tests/test_outgoing_webhook_interfaces.py +++ b/zerver/tests/test_outgoing_webhook_interfaces.py @@ -43,7 +43,7 @@ class TestGenericOutgoingWebhookService(ZulipTestCase): response.text = json.dumps({"response_string": 'test_content'}) success_response, _ = self.handler.process_success(response, self.event) - self.assertEqual(success_response, 'test_content') + self.assertEqual(success_response, dict(response_string='test_content')) response.text = json.dumps({}) success_response, _ = self.handler.process_success(response, self.event) @@ -130,4 +130,4 @@ class TestSlackOutgoingWebhookService(ZulipTestCase): response.text = json.dumps({"text": 'test_content'}) success_response, _ = self.handler.process_success(response, self.stream_message_event) - self.assertEqual(success_response, 'test_content') + self.assertEqual(success_response, dict(response_string='test_content')) diff --git a/zerver/tests/test_outgoing_webhook_system.py b/zerver/tests/test_outgoing_webhook_system.py index 4d64f63c75..eef3ab55e9 100644 --- a/zerver/tests/test_outgoing_webhook_system.py +++ b/zerver/tests/test_outgoing_webhook_system.py @@ -27,8 +27,13 @@ def timeout_error(http_method: Any, final_url: Any, data: Any, **request_kwargs: raise requests.exceptions.Timeout("Time is up!") class MockServiceHandler(OutgoingWebhookServiceInterface): - def process_success(self, response: Response, event: Dict[str, Any]) -> Tuple[Optional[str], Optional[str]]: - return "Success!", None + def process_success(self, response: Response, event: Dict[str, Any]) -> Tuple[Optional[Dict[str, Any]], Optional[str]]: + # Our tests don't really look at the content yet. + # They just ensure we use the "success" codepath. + success_data = dict( + response_string="whatever", + ) + return success_data, None service_handler = MockServiceHandler(None, None, None, None)