outgoing webhooks: Get rid of "Success!" prefix.

The earlier code here, if it got a payload with
"response_string" as a key, would prefix the
corresponding value with "Success!".  We just
want the bot to set its own content.

The code is reorganized here so that process_success()
always produces a value keyed by "content" from
incoming data, and then process_success_response()
doesn't do any fancy munging of the data.
This commit is contained in:
Steve Howell 2018-10-09 23:36:31 +00:00 committed by Tim Abbott
parent 0a751567a3
commit 229dd5d861
3 changed files with 10 additions and 14 deletions

View File

@ -71,8 +71,9 @@ class GenericOutgoingWebhookService(OutgoingWebhookServiceInterface):
return None
if "response_string" in response_json:
response_string = str(response_json['response_string'])
success_data = dict(response_string=response_string)
# We are deprecating response_string.
content = str(response_json['response_string'])
success_data = dict(content=content)
return success_data
if "content" in response_json:
@ -116,7 +117,7 @@ class SlackOutgoingWebhookService(OutgoingWebhookServiceInterface):
if "text" in response_json:
content = response_json['text']
success_data = dict(response_string=content)
success_data = dict(content=content)
return success_data
return None
@ -262,12 +263,7 @@ def process_success_response(event: Dict[str, Any],
if success_data is None:
return
response_string = success_data.get('response_string')
if response_string:
# For legacy reasons, we prepend "Success!"
content = "Success! " + response_string
else:
content = success_data.get('content')
content = success_data.get('content')
if content is None:
return

View File

@ -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, dict(response_string='test_content'))
self.assertEqual(success_response, dict(content='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, dict(response_string='test_content'))
self.assertEqual(success_response, dict(content='test_content'))

View File

@ -31,7 +31,7 @@ class MockServiceHandler(OutgoingWebhookServiceInterface):
# Our tests don't really look at the content yet.
# They just ensure we use the "success" codepath.
success_data = dict(
response_string="whatever",
content="whatever",
)
return success_data
@ -136,7 +136,7 @@ class TestOutgoingWebhookMessaging(ZulipTestCase):
self.send_personal_message(self.user_profile.email, self.bot_profile.email,
content="foo")
last_message = self.get_last_message()
self.assertEqual(last_message.content, "Success! Hidley ho, I'm a webhook responding!")
self.assertEqual(last_message.content, "Hidley ho, I'm a webhook responding!")
self.assertEqual(last_message.sender_id, self.bot_profile.id)
display_recipient = get_display_recipient(last_message.recipient)
# The next two lines error on mypy because the display_recipient is of type Union[str, List[Dict[str, Any]]].
@ -151,7 +151,7 @@ class TestOutgoingWebhookMessaging(ZulipTestCase):
content="@**{}** foo".format(self.bot_profile.full_name),
topic_name="bar")
last_message = self.get_last_message()
self.assertEqual(last_message.content, "Success! Hidley ho, I'm a webhook responding!")
self.assertEqual(last_message.content, "Hidley ho, I'm a webhook responding!")
self.assertEqual(last_message.sender_id, self.bot_profile.id)
self.assertEqual(last_message.subject, "bar")
display_recipient = get_display_recipient(last_message.recipient)