From bb6d189fa80692b7bfc6c23cdabb855e82b87a5c Mon Sep 17 00:00:00 2001
From: Tomasz Kolek <tomasz-kolek@o2.pl>
Date: Mon, 14 Nov 2016 21:06:39 +0100
Subject: [PATCH] Add github dispatcher to have one elegant url for both github
 integrations.

Dispatcher avoids to create new URL for github_webhook.
---
 templates/zerver/integrations.html           |  2 +-
 zerver/lib/integrations.py                   | 14 +++++++++++---
 zerver/tests/webhooks/test_github_webhook.py |  2 +-
 zerver/views/webhooks/github_dispatcher.py   | 12 ++++++++++++
 zproject/urls.py                             |  9 ++++++++-
 5 files changed, 33 insertions(+), 6 deletions(-)
 create mode 100644 zerver/views/webhooks/github_dispatcher.py

diff --git a/templates/zerver/integrations.html b/templates/zerver/integrations.html
index 04111d58b7..6d15be2196 100644
--- a/templates/zerver/integrations.html
+++ b/templates/zerver/integrations.html
@@ -929,7 +929,7 @@
 
       <p>In the <b>"Payload URL"</b> field, enter a URL constructed like this:</p>
 
-      <p><code>{{ external_api_uri_subdomain }}/v1/external/webhook_github?api_key=abcdefgh&amp;stream=github</code></p>
+      <p><code>{{ external_api_uri_subdomain }}/v1/external/github?api_key=abcdefgh&amp;stream=github</code></p>
 
       <p>where <code>api_key</code> is the API key of your Zulip
         bot. Select the actions that you want to result in a
diff --git a/zerver/lib/integrations.py b/zerver/lib/integrations.py
index 5c07823285..c736c2c5d9 100644
--- a/zerver/lib/integrations.py
+++ b/zerver/lib/integrations.py
@@ -90,6 +90,15 @@ class HubotLozenge(Integration):
         self.git_url = git_url
         super(HubotLozenge, self).__init__(name, name, logo, display_name=display_name)
 
+class GithubIntegration(WebhookIntegration):
+    """
+    We need this class to don't creating url object for git integrations.
+    We want to have one generic url with dispatch function for github service and github webhook.
+    """
+    @property
+    def url_object(self):
+        # type: () -> None
+        return
 
 WEBHOOK_INTEGRATIONS = [
     WebhookIntegration('airbrake'),
@@ -102,16 +111,15 @@ WEBHOOK_INTEGRATIONS = [
     WebhookIntegration('crashlytics'),
     WebhookIntegration('deskdotcom', logo='static/images/integrations/logos/deskcom.png', display_name='Desk.com'),
     WebhookIntegration('freshdesk'),
-    WebhookIntegration(
+    GithubIntegration(
         'github',
         function='zerver.views.webhooks.github.api_github_landing',
         display_name='GitHub',
         secondary_line_text='(deprecated)'
     ),
-    WebhookIntegration(
+    GithubIntegration(
         'github_webhook',
         display_name='GitHub',
-        url='api/v1/external/webhook_github',
         logo='static/images/integrations/logos/github.png',
         secondary_line_text='(webhook)',
         function='zerver.views.webhooks.github_webhook.api_github_webhook'
diff --git a/zerver/tests/webhooks/test_github_webhook.py b/zerver/tests/webhooks/test_github_webhook.py
index a8c45eb0e9..4a21e199b7 100644
--- a/zerver/tests/webhooks/test_github_webhook.py
+++ b/zerver/tests/webhooks/test_github_webhook.py
@@ -7,7 +7,7 @@ from zerver.lib.test_classes import WebhookTestCase
 
 class GithubWebhookTest(WebhookTestCase):
     STREAM_NAME = 'github'
-    URL_TEMPLATE = "/api/v1/external/webhook_github?stream={stream}&api_key={api_key}"
+    URL_TEMPLATE = "/api/v1/external/github?stream={stream}&api_key={api_key}"
     FIXTURE_DIR_NAME = 'github_webhook'
     EXPECTED_SUBJECT_REPO_EVENTS = u"public-repo"
     EXPECTED_SUBJECT_ISSUE_EVENTS = u"public-repo / Issue #2 Spelling error in the README file"
diff --git a/zerver/views/webhooks/github_dispatcher.py b/zerver/views/webhooks/github_dispatcher.py
new file mode 100644
index 0000000000..27c450be30
--- /dev/null
+++ b/zerver/views/webhooks/github_dispatcher.py
@@ -0,0 +1,12 @@
+from __future__ import absolute_import
+from django.http import HttpRequest, HttpResponse
+from .github_webhook import api_github_webhook
+from .github import api_github_landing
+
+
+def api_github_webhook_dispatch(request):
+    # type: (HttpRequest) -> HttpResponse
+    if request.META.get('HTTP_X_GITHUB_EVENT'):
+        return api_github_webhook(request)
+    else:
+        return api_github_landing(request)
diff --git a/zproject/urls.py b/zproject/urls.py
index 9f2207d68f..01ebdde5eb 100644
--- a/zproject/urls.py
+++ b/zproject/urls.py
@@ -9,6 +9,8 @@ from zproject import dev_urls
 from zproject.legacy_urls import legacy_urls
 from zerver.views.integrations import IntegrationView, APIView, HelpView
 from zerver.lib.integrations import WEBHOOK_INTEGRATIONS
+from zerver.views.webhooks import github_dispatcher
+
 
 from django.contrib.auth.views import (login, password_reset,
                                        password_reset_done, password_reset_confirm, password_reset_complete)
@@ -303,8 +305,13 @@ urls += url(r'^user_uploads/(?P<realm_id_str>(\d*|unk))/(?P<filename>.*)',
                      {'override_api_url_scheme'})}),
 
 # Incoming webhook URLs
+# We don't create urls for particular git integrations here
+# because of generic one below
 for incoming_webhook in WEBHOOK_INTEGRATIONS:
-    urls.append(incoming_webhook.url_object)
+    if incoming_webhook.url_object:
+        urls.append(incoming_webhook.url_object)
+
+urls.append(url(r'^api/v1/external/github', github_dispatcher.api_github_webhook_dispatch))
 
 # Mobile-specific authentication URLs
 urls += [