devtools: Add support for non-json fixtures for the integrations tool.

Note: If you're going to send fixtures which are not JSON or of the
text/plain content type, make sure you set the correct content type
in the custom headers.

E.g. For the wordpress fixtures the "Content-Type" should be set to
"application/x-www-form-urlencoded".

static/js/integrations_dev_panel.js

@@ -62,6 +62,11 @@ function get_selected_integration_name() {
     return $("#integration_name").children("option:selected").val();
 }
 
+function get_fixture_format(fixture_name) {
+    var pieces = fixture_name.split(".");
+    return pieces[pieces.length - 1];
+}
+
 function get_custom_http_headers() {
     var custom_headers = $("#custom_http_headers").val();
     if (custom_headers !== "") {
@@ -80,13 +85,14 @@ function get_custom_http_headers() {
 function load_fixture_body(fixture_name) {
     /* Given a fixture name, use the loaded_fixtures dictionary to set the fixture body field. */
     var integration_name = get_selected_integration_name();
-    var element = loaded_fixtures[integration_name][fixture_name];
-    var fixture_body = JSON.stringify(element, null, 4); // 4 is for the pretty print indent factor.
-
+    var fixture_body = loaded_fixtures[integration_name][fixture_name];
     if (fixture_body === undefined) {
         set_message("Fixture does not have a body.", "warning");
         return;
     }
+    if (get_fixture_format(fixture_name) === "json") {
+        fixture_body = JSON.stringify(fixture_body, null, 4);// 4 is for pretty print .
+    }
     $("#fixture_body")[0].value = fixture_body;
 
     return;
@@ -166,7 +172,7 @@ function get_fixtures(integration_name) {
         return;
     }
 
-    // We don't have the fixutures for this integration; fetch them using Zulip's channel library.
+    // We don't have the fixtures for this integration; fetch them using Zulip's channel library.
     // Relative url pattern: /devtools/integrations/(?P<integration_name>.+)/fixtures
     channel.get({
         url: "/devtools/integrations/" + integration_name + "/fixtures",
@@ -198,19 +204,24 @@ function send_webhook_fixture_message() {
     }
 
     var body = $("#fixture_body").val();
-    try {
-        // Let JavaScript validate the JSON for us.
-        body = JSON.stringify(JSON.parse(body));
-    } catch (err) {
-        set_message("Invalid JSON in fixture body.", "warning");
-        return;
+    var fixture_name = $("#fixture_name").val();
+    var is_json = false;
+    if (fixture_name && get_fixture_format(fixture_name) === "json") {
+        try {
+            // Let JavaScript validate the JSON for us.
+            body = JSON.stringify(JSON.parse(body));
+            is_json = true;
+        } catch (err) {
+            set_message("Invalid JSON in fixture body.", "warning");
+            return;
+        }
     }
 
     var custom_headers = get_custom_http_headers();
 
     channel.post({
         url: "/devtools/integrations/check_send_webhook_fixture_message",
-        data: {url: url, body: body, custom_headers: custom_headers},
+        data: {url: url, body: body, custom_headers: custom_headers, is_json: is_json},
         beforeSend: function (xhr) {xhr.setRequestHeader('X-CSRFToken', csrftoken);},
         success: function () {
             // If the previous fixture body was sent successfully, then we should change the success

templates/zerver/integrations/development/dev_panel.html

@@ -75,7 +75,7 @@
         </tr>
 
         <tr>
-            <td colspan="2" class="center-text pad-top"><b>JSON Body</b></td>
+            <td colspan="2" class="center-text pad-top"><b>Fixture Body</b></td>
         </tr>
         <tr>
             <td colspan="2" class="form-group">

zerver/tests/test_integrations_dev_panel.py

@@ -17,6 +17,7 @@ class TestIntegrationsDevPanel(ZulipTestCase):
             "url": url,
             "body": body,
             "custom_headers": "",
+            "is_json": True
         }
 
         response = self.client_post(target_url, data)
@@ -36,6 +37,7 @@ class TestIntegrationsDevPanel(ZulipTestCase):
             "url": url,
             "body": body,
             "custom_headers": "",
+            "is_json": True
         }
 
         response = self.client_post(target_url, data)
@@ -58,6 +60,7 @@ class TestIntegrationsDevPanel(ZulipTestCase):
             "url": url,
             "body": body,
             "custom_headers": ujson.dumps({"X_GITHUB_EVENT": "ping"}),
+            "is_json": True
         }
 
         response = self.client_post(target_url, data)
@@ -69,6 +72,29 @@ class TestIntegrationsDevPanel(ZulipTestCase):
         self.assertEqual(Stream.objects.get(id=latest_msg.recipient.type_id).name, "Denmark")
         self.assertEqual(latest_msg.topic_name(), "GitHub Notifications")
 
+    def test_check_send_webhook_fixture_message_for_success_with_headers_and_non_json_fixtures(self) -> None:
+        bot = get_user('webhook-bot@zulip.com', self.zulip_realm)
+        url = "/api/v1/external/wordpress?api_key={key}&stream=Denmark&topic=Wordpress Notifications".format(key=bot.api_key)
+        target_url = "/devtools/integrations/check_send_webhook_fixture_message"
+        with open("zerver/webhooks/wordpress/fixtures/publish_post_no_data_provided.txt", "r") as f:
+            body = f.read()
+
+        data = {
+            "url": url,
+            "body": body,
+            "custom_headers": ujson.dumps({"Content-Type": "application/x-www-form-urlencoded"}),
+            "is_json": False,
+        }
+
+        response = self.client_post(target_url, data)
+        self.assertEqual(response.status_code, 200)
+
+        latest_msg = Message.objects.latest('id')
+        expected_message = "New post published:\n* [New WordPress Post](WordPress Post URL)"
+        self.assertEqual(latest_msg.content, expected_message)
+        self.assertEqual(Stream.objects.get(id=latest_msg.recipient.type_id).name, "Denmark")
+        self.assertEqual(latest_msg.topic_name(), "Wordpress Notifications")
+
     def test_get_fixtures_for_nonexistant_integration(self) -> None:
         target_url = "/devtools/integrations/somerandomnonexistantintegration/fixtures"
         response = self.client_get(target_url)
@@ -85,13 +111,6 @@ class TestIntegrationsDevPanel(ZulipTestCase):
         self.assertEqual(response.status_code, 404)
         self.assertEqual(ujson.loads(response.content), expected_response)
 
-    def test_get_fixtures_for_integration_without_json_fixtures(self) -> None:
-        target_url = "/devtools/integrations/deskdotcom/fixtures"
-        response = self.client_get(target_url)
-        expected_response = {'msg': 'The integration "deskdotcom" has non-JSON fixtures.', 'result': 'error'}
-        self.assertEqual(response.status_code, 400)
-        self.assertEqual(ujson.loads(response.content), expected_response)
-
     def test_get_fixtures_for_success(self) -> None:
         target_url = "/devtools/integrations/airbrake/fixtures"
         response = self.client_get(target_url)
@@ -148,6 +167,74 @@ class TestIntegrationsDevPanel(ZulipTestCase):
             self.assertEqual(Stream.objects.get(id=msg.recipient.type_id).name, "Denmark")
             self.assertEqual(msg.topic_name(), "Appfollow Bulk Notifications")
 
+    def test_send_all_webhook_fixture_messages_for_success_with_non_json_fixtures(self) -> None:
+        bot = get_user('webhook-bot@zulip.com', self.zulip_realm)
+        url = "/api/v1/external/wordpress?api_key={key}&stream=Denmark&topic=Wordpress Bulk Notifications".format(key=bot.api_key)
+        target_url = "/devtools/integrations/send_all_webhook_fixture_messages"
+
+        data = {
+            "url": url,
+            "custom_headers": "",
+            "integration_name": "wordpress",
+        }
+
+        response = self.client_post(target_url, data)
+        expected_responses = [
+            {
+                "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'},
+                "fixture_name": "user_register.txt",
+                "status_code": 400
+            },
+            {
+                "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'},
+                "fixture_name": "publish_post_no_data_provided.txt",
+                "status_code": 400
+            },
+            {
+                "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'},
+                "fixture_name": "unknown_action_no_data.txt",
+                "status_code": 400
+            },
+            {
+                "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'},
+                "fixture_name": "publish_page.txt",
+                "status_code": 400
+            },
+            {
+                "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'},
+                "fixture_name": "unknown_action_no_hook_provided.txt",
+                "status_code": 400
+            },
+            {
+                "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'},
+                "fixture_name": "publish_post_type_not_provided.txt",
+                "status_code": 400
+            },
+            {
+                "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'},
+                "fixture_name": "wp_login.txt",
+                "status_code": 400
+            },
+            {
+                "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'},
+                "fixture_name": "publish_post.txt",
+                "status_code": 400
+            }
+        ]
+        responses = ujson.loads(response.content)["responses"]
+        for r in responses:
+            r["message"] = ujson.loads(r["message"])
+        self.assertEqual(response.status_code, 200)
+        for r in responses:
+            # We have to use this roundabout manner since the order may vary each time. This is not
+            # an issue. Basically, we're trying to compare 2 lists and since we're not resorting to
+            # using sets or a sorted order, we're sticking with O(n*m) time complexity for this
+            # comparision (where n and m are the lengths of the two lists respectively). But since
+            # this is just a unit test and more importantly n = m = some-low-number we don't really
+            # care about the time complexity being what it is.
+            self.assertTrue(r in expected_responses)
+            expected_responses.remove(r)
+
     @patch("zerver.views.development.integrations.os.path.exists")
     def test_send_all_webhook_fixture_messages_for_missing_fixtures(self, os_path_exists_mock: MagicMock) -> None:
         os_path_exists_mock.return_value = False
@@ -162,16 +249,3 @@ class TestIntegrationsDevPanel(ZulipTestCase):
         expected_response = {'msg': 'The integration "appfollow" does not have fixtures.', 'result': 'error'}
         self.assertEqual(response.status_code, 404)
         self.assertEqual(ujson.loads(response.content), expected_response)
-
-    def test_send_all_webhook_fixture_messages_for_non_json_fixtures(self) -> None:
-        bot = get_user('webhook-bot@zulip.com', self.zulip_realm)
-        url = "/api/v1/external/appfollow?api_key={key}&stream=Denmark&topic=Desktdotcom Bulk Notifications".format(key=bot.api_key)
-        data = {
-            "url": url,
-            "custom_headers": "",
-            "integration_name": "deskdotcom",
-        }
-        response = self.client_post("/devtools/integrations/send_all_webhook_fixture_messages", data)
-        expected_response = {'msg': 'The integration "deskdotcom" has non-JSON fixtures.', 'result': 'error'}
-        self.assertEqual(response.status_code, 400)
-        self.assertEqual(ujson.loads(response.content), expected_response)

zerver/views/development/integrations.py

@@ -26,9 +26,9 @@ def dev_panel(request: HttpRequest) -> HttpResponse:
     context = {"integrations": integrations, "bots": bots}
     return render(request, "zerver/integrations/development/dev_panel.html", context)
 
-
 def send_webhook_fixture_message(url: str=REQ(),
                                  body: str=REQ(),
+                                 is_json: bool=REQ(),
                                  custom_headers: str=REQ()) -> HttpResponse:
     client = Client()
     realm = get_realm("zulip")
@@ -39,10 +39,12 @@ def send_webhook_fixture_message(url: str=REQ(),
     if not headers:
         headers = {}
     http_host = headers.pop("HTTP_HOST", realm.host)
-    content_type = headers.pop("HTTP_CONTENT_TYPE", "application/json")
+    if is_json:
+        content_type = headers.pop("HTTP_CONTENT_TYPE", "application/json")
+    else:
+        content_type = headers.pop("HTTP_CONTENT_TYPE", "text/plain")
     return client.post(url, body, content_type=content_type, HTTP_HOST=http_host, **headers)
 
-
 @has_request_variables
 def get_fixtures(request: HttpResponse,
                  integration_name: str=REQ()) -> HttpResponse:
@@ -61,13 +63,12 @@ def get_fixtures(request: HttpResponse,
 
     for fixture in os.listdir(fixtures_dir):
         fixture_path = os.path.join(fixtures_dir, fixture)
+        content = open(fixture_path).read()
         try:
-            json_data = ujson.loads(open(fixture_path).read())
+            content = ujson.loads(content)
         except ValueError:
-            msg = ("The integration \"{integration_name}\" has non-JSON fixtures.").format(
-                integration_name=integration_name)
-            return json_error(msg)
-        fixtures[fixture] = json_data
+            pass  # The file extension will be used to determine the type.
+        fixtures[fixture] = content
 
     return json_success({"fixtures": fixtures})
 
@@ -76,8 +77,9 @@ def get_fixtures(request: HttpResponse,
 def check_send_webhook_fixture_message(request: HttpRequest,
                                        url: str=REQ(),
                                        body: str=REQ(),
+                                       is_json: bool=REQ(),
                                        custom_headers: str=REQ()) -> HttpResponse:
-    response = send_webhook_fixture_message(url, body, custom_headers)
+    response = send_webhook_fixture_message(url, body, is_json, custom_headers)
     if response.status_code == 200:
         return json_success()
     else:
@@ -99,14 +101,13 @@ def send_all_webhook_fixture_messages(request: HttpRequest,
     responses = []
     for fixture in os.listdir(fixtures_dir):
         fixture_path = os.path.join(fixtures_dir, fixture)
-        try:
-            # Just a quick check to make sure that the fixtures are of a valid JSON format.
-            json_data = ujson.dumps(ujson.loads(open(fixture_path).read()))
-        except ValueError:
-            msg = ("The integration \"{integration_name}\" has non-JSON fixtures.").format(
-                integration_name=integration_name)
-            return json_error(msg)
-        response = send_webhook_fixture_message(url, json_data, custom_headers)
+        content = open(fixture_path).read()
+        fixture_format = fixture.split(".")[-1]
+        if fixture_format == "json":
+            is_json = True
+        else:
+            is_json = False
+        response = send_webhook_fixture_message(url, content, is_json, custom_headers)
         responses.append({"status_code": response.status_code,
                           "fixture_name": fixture,
                           "message": response.content})