mirror of https://github.com/zulip/zulip.git
outgoing_webhook: Fix broken way of accessing realm.uri.
Previously, this accessed realm.uri via trying to use zulip_default_context. That doesn't make any sense, because zulip_default_context expects an HttpRequest object, and those are nowhere in sight in the code path. We do, however, have the outgoing webhook bot user involved in the event, and that's the object to access realm.uri from here.
This commit is contained in:
parent
a2243378ea
commit
4674af0894
|
@ -13,7 +13,6 @@ from requests import Response
|
|||
|
||||
from django.utils.translation import ugettext as _
|
||||
|
||||
from zerver.context_processors import zulip_default_context
|
||||
from zerver.models import Realm, UserProfile, get_user_profile_by_id, get_client, \
|
||||
GENERIC_INTERFACE, Service, SLACK_INTERFACE, email_to_domain, get_service_profile
|
||||
from zerver.lib.actions import check_send_message
|
||||
|
@ -186,6 +185,8 @@ def do_rest_call(rest_operation, request_data, event, service_handler, timeout=N
|
|||
if error:
|
||||
raise JsonableError(error)
|
||||
|
||||
bot_user = get_user_profile_by_id(event['user_profile_id'])
|
||||
|
||||
http_method = rest_operation['method']
|
||||
final_url = urllib.parse.urljoin(rest_operation['base_url'], rest_operation['relative_url_path'])
|
||||
request_kwargs = rest_operation['request_kwargs']
|
||||
|
@ -198,9 +199,8 @@ def do_rest_call(rest_operation, request_data, event, service_handler, timeout=N
|
|||
if response_message is not None:
|
||||
succeed_with_message(event, response_message)
|
||||
else:
|
||||
context = zulip_default_context(request_data)
|
||||
message_url = ("%(server)s/#narrow/stream/%(stream)s/subject/%(subject)s/near/%(id)s"
|
||||
% {'server': context['realm_uri'],
|
||||
% {'server': bot_user.realm.uri,
|
||||
'stream': event['message']['display_recipient'],
|
||||
'subject': event['message']['subject'],
|
||||
'id': str(event['message']['id'])})
|
||||
|
|
|
@ -9,19 +9,9 @@ from requests import Response
|
|||
|
||||
from zerver.lib.outgoing_webhook import do_rest_call, OutgoingWebhookServiceInterface
|
||||
from zerver.lib.test_classes import ZulipTestCase
|
||||
from zerver.models import get_realm, get_user
|
||||
from builtins import object
|
||||
|
||||
mock_event = {'message': {'display_recipient': '',
|
||||
'subject': '',
|
||||
'id': ''},
|
||||
'command': '',
|
||||
'service_name': ''}
|
||||
|
||||
rest_operation = {'method': "POST",
|
||||
'relative_url_path': "",
|
||||
'request_kwargs': {},
|
||||
'base_url': ""}
|
||||
|
||||
class ResponseMock(object):
|
||||
def __init__(self, status_code, data, content):
|
||||
# type: (int, Any, str) -> None
|
||||
|
@ -45,12 +35,28 @@ class MockServiceHandler(OutgoingWebhookServiceInterface):
|
|||
service_handler = MockServiceHandler(None, None, None, None)
|
||||
|
||||
class DoRestCallTests(ZulipTestCase):
|
||||
def setUp(self):
|
||||
# type: () -> None
|
||||
realm = get_realm("zulip")
|
||||
user_profile = get_user("outgoing-webhook@zulip.com", realm)
|
||||
self.mock_event = {'message': {'display_recipient': '',
|
||||
'subject': '',
|
||||
'id': ''},
|
||||
'user_profile_id': user_profile.id,
|
||||
'command': '',
|
||||
'service_name': ''}
|
||||
|
||||
self.rest_operation = {'method': "POST",
|
||||
'relative_url_path': "",
|
||||
'request_kwargs': {},
|
||||
'base_url': ""}
|
||||
|
||||
@mock.patch('zerver.lib.outgoing_webhook.succeed_with_message')
|
||||
def test_successful_request(self, mock_succeed_with_message):
|
||||
# type: (mock.Mock) -> None
|
||||
response = ResponseMock(200, {"message": "testing"}, '')
|
||||
with mock.patch('requests.request', return_value=response):
|
||||
do_rest_call(rest_operation, None, None, service_handler, None)
|
||||
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
|
||||
self.assertTrue(mock_succeed_with_message.called)
|
||||
|
||||
@mock.patch('zerver.lib.outgoing_webhook.request_retry')
|
||||
|
@ -58,7 +64,7 @@ class DoRestCallTests(ZulipTestCase):
|
|||
# type: (mock.Mock) -> None
|
||||
response = ResponseMock(500, {"message": "testing"}, '')
|
||||
with mock.patch('requests.request', return_value=response):
|
||||
do_rest_call(rest_operation, None, mock_event, service_handler, None)
|
||||
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
|
||||
self.assertTrue(mock_request_retry.called)
|
||||
|
||||
@mock.patch('zerver.lib.outgoing_webhook.fail_with_message')
|
||||
|
@ -66,7 +72,7 @@ class DoRestCallTests(ZulipTestCase):
|
|||
# type: (mock.Mock) -> None
|
||||
response = ResponseMock(400, {"message": "testing"}, '')
|
||||
with mock.patch('requests.request', return_value=response):
|
||||
do_rest_call(rest_operation, None, mock_event, service_handler, None)
|
||||
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
|
||||
self.assertTrue(mock_fail_with_message.called)
|
||||
|
||||
@mock.patch('logging.info')
|
||||
|
@ -74,7 +80,7 @@ class DoRestCallTests(ZulipTestCase):
|
|||
@mock.patch('zerver.lib.outgoing_webhook.request_retry')
|
||||
def test_timeout_request(self, mock_request_retry, mock_requests_request, mock_logger):
|
||||
# type: (mock.Mock, mock.Mock, mock.Mock) -> None
|
||||
do_rest_call(rest_operation, None, mock_event, service_handler, None)
|
||||
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
|
||||
self.assertTrue(mock_request_retry.called)
|
||||
|
||||
@mock.patch('logging.exception')
|
||||
|
@ -82,5 +88,5 @@ class DoRestCallTests(ZulipTestCase):
|
|||
@mock.patch('zerver.lib.outgoing_webhook.fail_with_message')
|
||||
def test_request_exception(self, mock_fail_with_message, mock_requests_request, mock_logger):
|
||||
# type: (mock.Mock, mock.Mock, mock.Mock) -> None
|
||||
do_rest_call(rest_operation, None, mock_event, service_handler, None)
|
||||
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
|
||||
self.assertTrue(mock_fail_with_message.called)
|
||||
|
|
Loading…
Reference in New Issue