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

@@ -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',

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":"<div>Test ticket description ☃.</div>",
+        "ticket_status":"Waiting on Customer",
+        "ticket_priority":"Low",
+    "requester_name":"Requester Bob",
+        "requester_email":"requester-bob@example.com",
+    }
+}

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":"<div>Test ticket description.</div>",
+        "ticket_status":"Resolved",
+        "ticket_priority":"Low",
+    "requester_name":"Requester Bob",
+        "requester_email":"requester-bob@example.com",
+    }
+}

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

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]: