From b40aec3a0951ff71d30dcf085f4b4c11c74303a9 Mon Sep 17 00:00:00 2001 From: Eeshan Garg Date: Tue, 25 Sep 2018 14:58:05 -0230 Subject: [PATCH] webhooks/freshdesk: Improve test coverage. Note that Freshdesk allows custom templating for outgoing payloads in their webhook UI. Therefore, the payloads added in this commit did not have to be official payloads from Freshdesk. --- tools/test-backend | 1 - ...atus_changed_fixture_with_missing_key.json | 13 ++++++++ .../freshdesk/fixtures/unknown_payload.json | 14 +++++++++ zerver/webhooks/freshdesk/tests.py | 31 +++++++++++++++++++ zerver/webhooks/freshdesk/view.py | 9 +++--- 5 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 zerver/webhooks/freshdesk/fixtures/status_changed_fixture_with_missing_key.json create mode 100644 zerver/webhooks/freshdesk/fixtures/unknown_payload.json diff --git a/tools/test-backend b/tools/test-backend index 012b3d3d96..88a2e787c1 100755 --- a/tools/test-backend +++ b/tools/test-backend @@ -100,7 +100,6 @@ not_yet_fully_covered = { 'zerver/data_import/gitter.py', 'zerver/data_import/import_util.py', # Webhook integrations with incomplete coverage - 'zerver/webhooks/freshdesk/view.py', 'zerver/webhooks/github/view.py', 'zerver/webhooks/github_legacy/view.py', 'zerver/webhooks/gitlab/view.py', diff --git a/zerver/webhooks/freshdesk/fixtures/status_changed_fixture_with_missing_key.json b/zerver/webhooks/freshdesk/fixtures/status_changed_fixture_with_missing_key.json new file mode 100644 index 0000000000..bb65b7ab4a --- /dev/null +++ b/zerver/webhooks/freshdesk/fixtures/status_changed_fixture_with_missing_key.json @@ -0,0 +1,13 @@ +{"freshdesk_webhook": + { + "ticket_id":"11", + "ticket_url":"http://test1234zzz.freshdesk.com/helpdesk/tickets/11", + "ticket_type":"Incident", + "ticket_subject":"Test ticket subject ☃", + "ticket_description":"
Test ticket description ☃.
", + "ticket_status":"Waiting on Customer", + "ticket_priority":"Low", + "requester_name":"Requester Bob", + "requester_email":"requester-bob@example.com", + } +} diff --git a/zerver/webhooks/freshdesk/fixtures/unknown_payload.json b/zerver/webhooks/freshdesk/fixtures/unknown_payload.json new file mode 100644 index 0000000000..7b6b3b9868 --- /dev/null +++ b/zerver/webhooks/freshdesk/fixtures/unknown_payload.json @@ -0,0 +1,14 @@ +{"freshdesk_webhook": + { + "triggered_event":"{unknown_event:{from:3,to:1}}", + "ticket_id":"11", + "ticket_url":"http://test1234zzz.freshdesk.com/helpdesk/tickets/11", + "ticket_type":"Incident", + "ticket_subject":"Test ticket subject", + "ticket_description":"
Test ticket description.
", + "ticket_status":"Resolved", + "ticket_priority":"Low", + "requester_name":"Requester Bob", + "requester_email":"requester-bob@example.com", + } +} diff --git a/zerver/webhooks/freshdesk/tests.py b/zerver/webhooks/freshdesk/tests.py index fab5695c42..8f1cb80bcd 100644 --- a/zerver/webhooks/freshdesk/tests.py +++ b/zerver/webhooks/freshdesk/tests.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- +from mock import MagicMock, patch + from zerver.lib.test_classes import WebhookTestCase class FreshdeskHookTests(WebhookTestCase): @@ -36,6 +38,19 @@ Status: **Resolved** => **Waiting on Customer**""" self.api_stream_message(self.TEST_USER_EMAIL, 'status_changed', expected_subject, expected_message, content_type="application/x-www-form-urlencoded") + def test_status_change_fixture_without_required_key(self) -> None: + """ + A fixture without the requisite keys should raise JsonableError. + """ + self.url = self.build_webhook_url() + payload = self.get_body('status_changed_fixture_with_missing_key') + kwargs = { + 'HTTP_AUTHORIZATION': self.encode_credentials(self.TEST_USER_EMAIL), + 'content_type': 'application/x-www-form-urlencoded', + } + result = self.client_post(self.url, payload, **kwargs) + self.assert_json_error(result, 'Missing key triggered_event in JSON') + def test_priority_change(self) -> None: """ Messages are generated when a ticket's priority changes through @@ -48,6 +63,22 @@ Priority: **High** => **Low**""" self.api_stream_message(self.TEST_USER_EMAIL, 'priority_changed', expected_subject, expected_message, content_type="application/x-www-form-urlencoded") + @patch('zerver.lib.webhooks.common.check_send_webhook_message') + def test_unknown_event_payload_ignore( + self, check_send_webhook_message_mock: MagicMock) -> None: + """ + Ignore unknown event payloads. + """ + self.url = self.build_webhook_url() + payload = self.get_body('unknown_payload') + kwargs = { + 'HTTP_AUTHORIZATION': self.encode_credentials(self.TEST_USER_EMAIL), + 'content_type': 'application/x-www-form-urlencoded', + } + result = self.client_post(self.url, payload, **kwargs) + self.assertFalse(check_send_webhook_message_mock.called) + self.assert_json_success(result) + def note_change(self, fixture: str, note_type: str) -> None: """ Messages are generated when a note gets added to a ticket through diff --git a/zerver/webhooks/freshdesk/view.py b/zerver/webhooks/freshdesk/view.py index b1439ebd06..9f587fe37a 100644 --- a/zerver/webhooks/freshdesk/view.py +++ b/zerver/webhooks/freshdesk/view.py @@ -38,12 +38,13 @@ def property_name(property: str, index: int) -> str: "Waiting on Customer", "Job Application", "Monthly"] priorities = ["", "Low", "Medium", "High", "Urgent"] + name = "" if property == "status": - return statuses[index] if index < len(statuses) else str(index) + name = statuses[index] if index < len(statuses) else str(index) elif property == "priority": - return priorities[index] if index < len(priorities) else str(index) - else: - raise ValueError("Unknown property") + name = priorities[index] if index < len(priorities) else str(index) + + return name def parse_freshdesk_event(event_string: str) -> List[str]: