Auto Sync and Re-Sync for Manually Created Integrations

readthedocs/oauth/services/base.py

@@ -226,12 +226,14 @@ def get_paginated_results(self, response):
         """
         raise NotImplementedError
 
-    def setup_webhook(self, project):
+    def setup_webhook(self, project, integration=None):
         """
         Setup webhook for project.
 
         :param project: project to set up webhook for
         :type project: Project
+        :param integration: Integration for the project
+        :type integration: Integration
         :returns: boolean based on webhook set up success, and requests Response object
         :rtype: (Bool, Response)
         """

readthedocs/oauth/services/bitbucket.py

@@ -206,21 +206,24 @@ def get_webhook_data(self, project, integration):
             'events': ['repo:push'],
         })
 
-    def setup_webhook(self, project):
+    def setup_webhook(self, project, integration=None):
         """
         Set up Bitbucket project webhook for project.
 
         :param project: project to set up webhook for
         :type project: Project
+        :param integration: Integration for the project
+        :type integration: Integration
         :returns: boolean based on webhook set up success, and requests Response object
         :rtype: (Bool, Response)
         """
         session = self.get_session()
         owner, repo = build_utils.get_bitbucket_username_repo(url=project.repo)
-        integration, _ = Integration.objects.get_or_create(
-            project=project,
-            integration_type=Integration.BITBUCKET_WEBHOOK,
-        )
+        if not integration:
+            integration, _ = Integration.objects.get_or_create(
+                project=project,
+                integration_type=Integration.BITBUCKET_WEBHOOK,
+            )
         data = self.get_webhook_data(project, integration)
         resp = None
         try:
@@ -308,11 +311,12 @@ def update_webhook(self, project, integration):
                 return self.setup_webhook(project)
 
         # Catch exceptions with request or deserializing JSON
-        except (KeyError, RequestException, ValueError):
+        except (KeyError, RequestException, TypeError, ValueError):
             log.exception(
                 'Bitbucket webhook update failed for project: %s',
                 project,
             )
+            return (False, resp)
         else:
             log.error(
                 'Bitbucket webhook update failed for project: %s',

readthedocs/oauth/services/github.py

@@ -190,21 +190,26 @@ def get_webhook_data(self, project, integration):
             'events': ['push', 'pull_request', 'create', 'delete'],
         })
 
-    def setup_webhook(self, project):
+    def setup_webhook(self, project, integration=None):
         """
         Set up GitHub project webhook for project.
 
         :param project: project to set up webhook for
         :type project: Project
+        :param integration: Integration for the project
+        :type integration: Integration
         :returns: boolean based on webhook set up success, and requests Response object
         :rtype: (Bool, Response)
         """
         session = self.get_session()
         owner, repo = build_utils.get_github_username_repo(url=project.repo)
-        integration, _ = Integration.objects.get_or_create(
-            project=project,
-            integration_type=Integration.GITHUB_WEBHOOK,
-        )
+        if integration:
+            integration.recreate_secret()
+        else:
+            integration, _ = Integration.objects.get_or_create(
+                project=project,
+                integration_type=Integration.GITHUB_WEBHOOK,
+            )
         data = self.get_webhook_data(project, integration)
         resp = None
         try:
@@ -233,6 +238,9 @@ def setup_webhook(self, project):
                     'permissions: project=%s',
                     project,
                 )
+                # Set the secret to None so that the integration can be used manually.
+                integration.secret = None
+                integration.save()
                 return (False, resp)
         # Catch exceptions with request or deserializing JSON
         except (RequestException, ValueError):
@@ -271,9 +279,9 @@ def update_webhook(self, project, integration):
         session = self.get_session()
         integration.recreate_secret()
         data = self.get_webhook_data(project, integration)
-        url = integration.provider_data.get('url')
         resp = None
         try:
+            url = integration.provider_data.get('url')
             resp = session.patch(
                 url,
                 data=data,
@@ -296,11 +304,12 @@ def update_webhook(self, project, integration):
                 return self.setup_webhook(project)
 
         # Catch exceptions with request or deserializing JSON
-        except (RequestException, ValueError):
+        except (AttributeError, RequestException, ValueError):
             log.exception(
                 'GitHub webhook update failed for project: %s',
                 project,
             )
+            return (False, resp)
         else:
             log.error(
                 'GitHub webhook update failed for project: %s',

readthedocs/oauth/services/gitlab.py

@@ -262,21 +262,29 @@ def get_webhook_data(self, repo_id, project, integration):
             'wiki_events': False,
         })
 
-    def setup_webhook(self, project):
+    def setup_webhook(self, project, integration=None):
         """
         Set up GitLab project webhook for project.
 
         :param project: project to set up webhook for
         :type project: Project
+        :param integration: Integration for a project
+        :type integration: Integration
         :returns: boolean based on webhook set up success
         :rtype: bool
         """
-        integration, _ = Integration.objects.get_or_create(
-            project=project,
-            integration_type=Integration.GITLAB_WEBHOOK,
-        )
+        if integration:
+            integration.recreate_secret()
+        else:
+            integration, _ = Integration.objects.get_or_create(
+                project=project,
+                integration_type=Integration.GITLAB_WEBHOOK,
+            )
         repo_id = self._get_repo_id(project)
         if repo_id is None:
+            # Set the secret to None so that the integration can be used manually.
+            integration.secret = None
+            integration.save()
             return (False, None)
 
         data = self.get_webhook_data(repo_id, project, integration)
@@ -306,13 +314,17 @@ def setup_webhook(self, project):
                     'permissions: project=%s',
                     project,
                 )
+                # Set the secret to None so that the integration can be used manually.
+                integration.secret = None
+                integration.save()
                 return (False, resp)
 
         except (RequestException, ValueError):
             log.exception(
                 'GitLab webhook creation failed for project: %s',
                 project,
             )
+            return (False, resp)
         else:
             log.error(
                 'GitLab webhook creation failed for project: %s',

readthedocs/oauth/tasks.py

@@ -11,6 +11,7 @@
     AttachWebhookNotification,
     InvalidProjectWebhookNotification,
 )
+from readthedocs.oauth.utils import SERVICE_MAP
 from readthedocs.oauth.services.base import SyncServiceError
 from readthedocs.projects.models import Project
 from readthedocs.worker import app
@@ -43,7 +44,7 @@ def sync_remote_repositories(user_id):
 
 
 @app.task(queue='web')
-def attach_webhook(project_pk, user_pk):
+def attach_webhook(project_pk, user_pk, integration=None):
     """
     Add post-commit hook on project import.
 
@@ -59,15 +60,22 @@ def attach_webhook(project_pk, user_pk):
         user=user,
         success=False,
     )
+    if integration:
+        service = SERVICE_MAP.get(integration.integration_type)
 
-    for service_cls in registry:
-        if service_cls.is_project_service(project):
-            service = service_cls
-            break
+        if not service:
+            log.warning('There are no registered services in the application.')
+            project_notification.send()
+            return None
     else:
-        log.warning('There are no registered services in the application.')
-        project_notification.send()
-        return None
+        for service_cls in registry:
+            if service_cls.is_project_service(project):
+                service = service_cls
+                break
+        else:
+            log.warning('There are no registered services in the application.')
+            project_notification.send()
+            return None
 
     provider = allauth_registry.by_id(service.adapter.provider_id)
     notification = AttachWebhookNotification(
@@ -79,7 +87,7 @@ def attach_webhook(project_pk, user_pk):
 
     user_accounts = service.for_user(user)
     for account in user_accounts:
-        success, __ = account.setup_webhook(project)
+        success, __ = account.setup_webhook(project, integration=integration)
         if success:
             notification.success = True
             notification.send()

readthedocs/projects/views/private.py

@@ -781,6 +781,16 @@ class IntegrationList(IntegrationMixin, ListView):
 
 class IntegrationCreate(IntegrationMixin, CreateView):
 
+    def form_valid(self, form):
+        self.object = form.save()
+        if self.object.has_sync:
+            attach_webhook(
+                project_pk=self.get_project().pk,
+                user_pk=self.request.user.pk,
+                integration=self.object
+            )
+        return HttpResponseRedirect(self.get_success_url())
+
     def get_success_url(self):
         return reverse(
             'projects_integrations_detail',

readthedocs/rtd_tests/tests/test_project_views.py

@@ -13,6 +13,7 @@
 
 from readthedocs.builds.constants import EXTERNAL, LATEST
 from readthedocs.builds.models import Build, Version
+from readthedocs.integrations.models import GenericAPIWebhook, GitHubWebhook
 from readthedocs.oauth.models import RemoteRepository
 from readthedocs.projects import tasks
 from readthedocs.projects.constants import PUBLIC
@@ -497,6 +498,44 @@ def test_subproject_create(self):
                 args=[project.pk],
             )
 
+    @patch('readthedocs.projects.views.private.attach_webhook')
+    def test_integration_create(self, attach_webhook):
+        project = get(Project, slug='pip', users=[self.user])
+
+        response = self.client.post(
+            reverse('projects_integrations_create', args=[project.slug]),
+            data={
+                'project': project.pk,
+                'integration_type': GitHubWebhook.GITHUB_WEBHOOK
+            },
+        )
+        integration = GitHubWebhook.objects.filter(project=project)
+
+        self.assertTrue(integration.exists())
+        self.assertEqual(response.status_code, 302)
+        attach_webhook.assert_called_once_with(
+            project_pk=project.pk,
+            user_pk=self.user.pk,
+            integration=integration.first()
+        )
+
+    @patch('readthedocs.projects.views.private.attach_webhook')
+    def test_integration_create_generic_webhook(self, attach_webhook):
+        project = get(Project, slug='pip', users=[self.user])
+
+        response = self.client.post(
+            reverse('projects_integrations_create', args=[project.slug]),
+            data={
+                'project': project.pk,
+                'integration_type': GenericAPIWebhook.API_WEBHOOK
+            },
+        )
+        integration = GenericAPIWebhook.objects.filter(project=project)
+
+        self.assertTrue(integration.exists())
+        self.assertEqual(response.status_code, 302)
+        attach_webhook.assert_not_called()
+
 
 class TestPrivateMixins(MockBuildTestCase):
 

readthedocs/templates/projects/integration_webhook_detail.html

@@ -19,15 +19,15 @@
   {% if integration.has_sync and integration.can_sync %}
     <p>
       {% blocktrans trimmed %}
-        This webhook was configured when this project was imported. If this
-        integration is not functioning correctly, try resyncing the webhook:
+        This webhook was configured when this project was imported
+          or it was automatically created with the correct configuration. If this
+        integration is not functioning correctly, try re-syncing the webhook:
       {% endblocktrans %}
     </p>
-
-    <form method="post" action="{% url 'projects_integrations_webhooks_sync' project_slug=project.slug integration_pk=integration.pk %}">
-      {% csrf_token %}
-      <input type="submit" value="{% trans "Resync webhook" %}">
-    </form>
+    <p>
+      {% url 'api_webhook' project_slug=project.slug integration_pk=integration.pk as webhook_url %}
+      <a href="//{{ PRODUCTION_DOMAIN }}{{ webhook_url }}">{{ PRODUCTION_DOMAIN }}{{ webhook_url }}</a>
+    </p>
   {% else %}
     {% comment %}
       Display information for manual webhook set up if either case is true:
@@ -40,10 +40,11 @@
     {% if integration.has_sync and not integration.can_sync %}
       <p>
         {% blocktrans trimmed %}
-          This integration was created automatically from an existing webhook
-          configured on your repository. To make any changes to this webhook,
-          you'll need to update the configuration there. You can use the
-          following address to manually configure this webhook.
+          This webhook was created automatically from an existing webhook
+          configured on your repository. If this integration is not functioning correctly,
+          you can try re-syncing it. Otherwise you'll need to update
+          the configuration on your repository.
+          You can use the following address to manually configure this webhook.
         {% endblocktrans %}
       </p>
     {% else %}
@@ -72,6 +73,13 @@
     </p>
   {% endif %}
 
+  {% if integration.has_sync %}
+    <form method="post" action="{% url 'projects_integrations_webhooks_sync' project_slug=project.slug integration_pk=integration.pk %}">
+      {% csrf_token %}
+      <input type="submit" value="{% trans "Resync webhook" %}">
+    </form>
+  {% endif %}
+
   <h3>{% trans "Recent Activity" %}</h3>
 
   <div class="module-list-wrapper httpexchanges">