From dc173397476c514c116f52d5b9439535a252dac7 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Fri, 30 Aug 2019 04:47:32 +0600 Subject: [PATCH 01/22] fix Integration resync bug --- readthedocs/oauth/services/bitbucket.py | 2 +- readthedocs/oauth/services/github.py | 21 +++++++++++++++++++-- readthedocs/oauth/services/gitlab.py | 23 +++++++++++++++++++++-- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py index 5af74cf2a12..df4d3ebcfeb 100644 --- a/readthedocs/oauth/services/bitbucket.py +++ b/readthedocs/oauth/services/bitbucket.py @@ -308,7 +308,7 @@ def update_webhook(self, project, integration): # Bitbucket returns 404 when the webhook doesn't exist. In this # case, we call ``setup_webhook`` to re-configure it from scratch if resp.status_code == 404: - return self.setup_webhook(project) + return self.setup_webhook(project, integration) # Catch exceptions with request or deserializing JSON except (KeyError, RequestException, TypeError, ValueError): diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index c5b1f2de8ce..133e8ba60c2 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -221,6 +221,7 @@ def setup_webhook(self, project, integration=None): data=data, headers={'content-type': 'application/json'}, ) + # GitHub will return 200 if already synced if resp.status_code in [200, 201]: recv_data = resp.json() @@ -244,11 +245,17 @@ def setup_webhook(self, project, integration=None): return (False, resp) # Catch exceptions with request or deserializing JSON except (RequestException, ValueError): + integration.secret = None + integration.save() + log.exception( 'GitHub webhook creation failed for project: %s', project, ) else: + integration.secret = None + integration.save() + log.error( 'GitHub webhook creation failed for project: %s', project, @@ -287,6 +294,7 @@ def update_webhook(self, project, integration): data=data, headers={'content-type': 'application/json'}, ) + # GitHub will return 200 if already synced if resp.status_code in [200, 201]: recv_data = resp.json() @@ -301,16 +309,25 @@ def update_webhook(self, project, integration): # GitHub returns 404 when the webhook doesn't exist. In this case, # we call ``setup_webhook`` to re-configure it from scratch if resp.status_code == 404: - return self.setup_webhook(project) + return self.setup_webhook(project, integration) + except AttributeError: + # We get AttributeError when the provider_data does not have anything + # it only happens if the webhook attachment was not successful in the first place + return self.setup_webhook(project, integration) # Catch exceptions with request or deserializing JSON - except (AttributeError, RequestException, ValueError): + except (RequestException, ValueError): log.exception( 'GitHub webhook update failed for project: %s', project, ) + # Set the secret to None so that the integration can be used manually. + integration.secret = None + integration.save() return (False, resp) else: + integration.secret = None + integration.save() log.error( 'GitHub webhook update failed for project: %s', project, diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py index c22202375f2..4592c9ea217 100644 --- a/readthedocs/oauth/services/gitlab.py +++ b/readthedocs/oauth/services/gitlab.py @@ -304,6 +304,7 @@ def setup_webhook(self, project, integration=None): data=data, headers={'content-type': 'application/json'}, ) + if resp.status_code == 201: integration.provider_data = resp.json() integration.save() @@ -325,12 +326,18 @@ def setup_webhook(self, project, integration=None): return (False, resp) except (RequestException, ValueError): + integration.secret = None + integration.save() + log.exception( 'GitLab webhook creation failed for project: %s', project, ) return (False, resp) else: + integration.secret = None + integration.save() + log.error( 'GitLab webhook creation failed for project: %s', project, @@ -360,9 +367,10 @@ def update_webhook(self, project, integration): integration.recreate_secret() data = self.get_webhook_data(repo_id, project, integration) - hook_id = integration.provider_data.get('id') + resp = None try: + hook_id = integration.provider_data.get('id') resp = session.put( '{url}/api/v4/projects/{repo_id}/hooks/{hook_id}'.format( url=self.adapter.provider_base_url, @@ -372,6 +380,7 @@ def update_webhook(self, project, integration): data=data, headers={'content-type': 'application/json'}, ) + if resp.status_code == 200: recv_data = resp.json() integration.provider_data = recv_data @@ -385,15 +394,25 @@ def update_webhook(self, project, integration): # GitLab returns 404 when the webhook doesn't exist. In this case, # we call ``setup_webhook`` to re-configure it from scratch if resp.status_code == 404: - return self.setup_webhook(project) + return self.setup_webhook(project, integration) + except AttributeError: + # We get AttributeError when the provider_data does not have anything + # it only happens if the webhook attachment was not successful in the first place + return self.setup_webhook(project, integration) # Catch exceptions with request or deserializing JSON except (RequestException, ValueError): + integration.secret = None + integration.save() + log.exception( 'GitLab webhook update failed for project: %s', project, ) else: + integration.secret = None + integration.save() + log.error( 'GitLab webhook update failed for project: %s', project, From 00c110ac44476bbaef618bafda8465c4a1482a50 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Fri, 30 Aug 2019 18:00:12 +0600 Subject: [PATCH 02/22] add remove_secret method --- readthedocs/integrations/models.py | 4 ++++ readthedocs/oauth/services/github.py | 15 +++++---------- readthedocs/oauth/services/gitlab.py | 19 +++++++------------ 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/readthedocs/integrations/models.py b/readthedocs/integrations/models.py index 55059853e55..bef588535ec 100644 --- a/readthedocs/integrations/models.py +++ b/readthedocs/integrations/models.py @@ -274,6 +274,10 @@ def recreate_secret(self): self.secret = get_secret() self.save(update_fields=['secret']) + def remove_secret(self): + self.secret = None + self.save(update_fields=['secret']) + def __str__(self): return ( _('{0} for {1}') diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index 133e8ba60c2..c8d0d3eb938 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -240,21 +240,18 @@ def setup_webhook(self, project, integration=None): project, ) # Set the secret to None so that the integration can be used manually. - integration.secret = None - integration.save() + integration.remove_secret() return (False, resp) # Catch exceptions with request or deserializing JSON except (RequestException, ValueError): - integration.secret = None - integration.save() + integration.remove_secret() log.exception( 'GitHub webhook creation failed for project: %s', project, ) else: - integration.secret = None - integration.save() + integration.remove_secret() log.error( 'GitHub webhook creation failed for project: %s', @@ -322,12 +319,10 @@ def update_webhook(self, project, integration): project, ) # Set the secret to None so that the integration can be used manually. - integration.secret = None - integration.save() + integration.remove_secret() return (False, resp) else: - integration.secret = None - integration.save() + integration.remove_secret() log.error( 'GitHub webhook update failed for project: %s', project, diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py index 4592c9ea217..2ac9f7a011d 100644 --- a/readthedocs/oauth/services/gitlab.py +++ b/readthedocs/oauth/services/gitlab.py @@ -288,8 +288,7 @@ def setup_webhook(self, project, integration=None): 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() + integration.remove_secret() return (False, None) data = self.get_webhook_data(repo_id, project, integration) @@ -321,13 +320,12 @@ def setup_webhook(self, project, integration=None): project, ) # Set the secret to None so that the integration can be used manually. - integration.secret = None - integration.save() + integration.remove_secret() + return (False, resp) except (RequestException, ValueError): - integration.secret = None - integration.save() + integration.remove_secret() log.exception( 'GitLab webhook creation failed for project: %s', @@ -335,8 +333,7 @@ def setup_webhook(self, project, integration=None): ) return (False, resp) else: - integration.secret = None - integration.save() + integration.remove_secret() log.error( 'GitLab webhook creation failed for project: %s', @@ -402,16 +399,14 @@ def update_webhook(self, project, integration): return self.setup_webhook(project, integration) # Catch exceptions with request or deserializing JSON except (RequestException, ValueError): - integration.secret = None - integration.save() + integration.remove_secret() log.exception( 'GitLab webhook update failed for project: %s', project, ) else: - integration.secret = None - integration.save() + integration.remove_secret() log.error( 'GitLab webhook update failed for project: %s', From 35faa9a22990baa0d2be8265259060614250c84e Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Fri, 30 Aug 2019 18:41:35 +0600 Subject: [PATCH 03/22] Update Integration details page messsage --- .../templates/projects/integration_webhook_detail.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/readthedocs/templates/projects/integration_webhook_detail.html b/readthedocs/templates/projects/integration_webhook_detail.html index d8911172884..b7d3c3292d8 100644 --- a/readthedocs/templates/projects/integration_webhook_detail.html +++ b/readthedocs/templates/projects/integration_webhook_detail.html @@ -20,7 +20,7 @@

{% blocktrans trimmed %} This webhook was configured when this project was imported - or it was automatically created with the correct configuration. If this + or it was manually created with the correct configuration. If this integration is not functioning correctly, try re-syncing the webhook: {% endblocktrans %}

@@ -40,7 +40,7 @@ {% if integration.has_sync and not integration.can_sync %}

{% blocktrans trimmed %} - This webhook was created automatically from an existing webhook + This webhook was created automatically or manually 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. From 9271ed413355145ede2e406d7c19383d2c19a8d6 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Fri, 30 Aug 2019 19:29:10 +0600 Subject: [PATCH 04/22] GitHub setup_webhook and update_webhook tests added --- readthedocs/rtd_tests/tests/test_oauth.py | 117 ++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index a2e4f07f9f7..aa61ee635b7 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -9,6 +9,7 @@ from readthedocs.builds.constants import EXTERNAL, BUILD_STATUS_SUCCESS from readthedocs.builds.models import Version, Build +from readthedocs.integrations.models import GitHubWebhook from readthedocs.oauth.models import RemoteOrganization, RemoteRepository from readthedocs.oauth.services import ( BitbucketService, @@ -34,6 +35,13 @@ def setUp(self): self.external_build = get( Build, project=self.project, version=self.external_version ) + self.integration = get( + GitHubWebhook, + project=self.project, + provider_data={ + 'url': 'https://github.com/' + } + ) def test_make_project_pass(self): repo_json = { @@ -216,6 +224,115 @@ def test_make_private_project(self): repo = self.service.create_repository(repo_json, organization=self.org) self.assertIsNotNone(repo) + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_setup_webhook_successful(self, session, mock_logger): + session().post.return_value.status_code = 201 + session().post.return_value.json.return_value = {} + success, _ = self.service.setup_webhook( + self.project, + self.integration + ) + + self.assertTrue(success) + self.assertTrue(self.integration.secret) + mock_logger.info.assert_called_with( + "GitHub webhook creation successful for project: %s", + self.project, + ) + + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_setup_webhook_404_error(self, session, mock_logger): + session().post.return_value.status_code = 404 + success, _ = self.service.setup_webhook( + self.project, + self.integration + ) + + self.assertFalse(success) + self.assertFalse(self.integration.secret) + mock_logger.info.assert_called_with( + 'GitHub project does not exist or user does not have ' + 'permissions: project=%s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_setup_webhook_value_error(self, session, mock_logger): + session().post.side_effect = ValueError + success = self.service.setup_webhook( + self.project, + self.integration + ) + + self.assertFalse(self.integration.secret) + mock_logger.exception.assert_called_with( + 'GitHub webhook creation failed for project: %s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_update_webhook_successful(self, session, mock_logger): + session().patch.return_value.status_code = 201 + session().patch.return_value.json.return_value = {} + success, _ = self.service.update_webhook( + self.project, + self.integration + ) + + self.assertTrue(success) + self.assertTrue(self.integration.secret) + mock_logger.info.assert_called_with( + "GitHub webhook creation successful for project: %s", + self.project, + ) + + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + @mock.patch('readthedocs.oauth.services.github.GitHubService.setup_webhook') + def test_update_webhook_404_error(self, setup_webhook, session): + session().patch.return_value.status_code = 404 + self.service.update_webhook( + self.project, + self.integration + ) + + setup_webhook.assert_called_once_with( + self.project, + self.integration + ) + + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + @mock.patch('readthedocs.oauth.services.github.GitHubService.setup_webhook') + def test_update_webhook_attribute_error(self, setup_webhook, session): + session().patch.side_effect = AttributeError + self.service.update_webhook( + self.project, + self.integration + ) + + setup_webhook.assert_called_once_with( + self.project, + self.integration + ) + + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_update_webhook_value_error(self, session, mock_logger): + session().patch.side_effect = ValueError + self.service.update_webhook( + self.project, + self.integration + ) + + self.assertFalse(self.integration.secret) + mock_logger.exception.assert_called_with( + 'GitHub webhook update failed for project: %s', + self.project, + ) + class BitbucketOAuthTests(TestCase): From c19128a00a2081d5d3c40d3ef345ae697b2c47d4 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Fri, 30 Aug 2019 20:03:51 +0600 Subject: [PATCH 05/22] GitLab setup_webhook and update_webhook tests added --- readthedocs/oauth/services/github.py | 4 +- readthedocs/rtd_tests/tests/test_oauth.py | 137 +++++++++++++++++++++- 2 files changed, 133 insertions(+), 8 deletions(-) diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index c8d0d3eb938..64ff748acf3 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -298,7 +298,7 @@ def update_webhook(self, project, integration): integration.provider_data = recv_data integration.save() log.info( - 'GitHub webhook creation successful for project: %s', + 'GitHub webhook update successful for project: %s', project, ) return (True, resp) @@ -332,7 +332,7 @@ def update_webhook(self, project, integration): except ValueError: debug_data = resp.content log.debug( - 'GitHub webhook creation failure response: %s', + 'GitHub webhook update failure response: %s', debug_data, ) return (False, resp) diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index aa61ee635b7..57a1472ceb9 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -9,7 +9,11 @@ from readthedocs.builds.constants import EXTERNAL, BUILD_STATUS_SUCCESS from readthedocs.builds.models import Version, Build -from readthedocs.integrations.models import GitHubWebhook +from readthedocs.integrations.models import ( + GitHubWebhook, + GitLabWebhook, + BitbucketWebhook +) from readthedocs.oauth.models import RemoteOrganization, RemoteRepository from readthedocs.oauth.services import ( BitbucketService, @@ -605,6 +609,8 @@ def setUp(self): self.client.login(username='eric', password='test') self.user = User.objects.get(pk=1) self.project = Project.objects.get(slug='pip') + self.project.repo = 'https://gitlab.com/testorga/testrepo' + self.project.save() self.org = RemoteOrganization.objects.create(slug='testorga', json='') self.privacy = self.project.version_privacy_level self.service = GitLabService(user=self.user, account=None) @@ -612,6 +618,13 @@ def setUp(self): self.external_build = get( Build, project=self.project, version=self.external_version ) + self.integration = get( + GitLabWebhook, + project=self.project, + provider_data={ + 'id': '999999999' + } + ) def get_private_repo_data(self): """Manipulate repo response data to get private repo data.""" @@ -685,11 +698,6 @@ def test_make_private_project(self): repo = self.service.create_repository(data, organization=self.org) self.assertIsNotNone(repo) - def test_setup_webhook(self): - success, response = self.service.setup_webhook(self.project) - self.assertFalse(success) - self.assertIsNone(response) - @mock.patch('readthedocs.oauth.services.gitlab.log') @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') @mock.patch('readthedocs.oauth.services.gitlab.GitLabService._get_repo_id') @@ -746,3 +754,120 @@ def test_send_build_status_value_error(self, repo_id, session, mock_logger): 'GitLab commit status creation failed for project: %s', self.project, ) + + @mock.patch('readthedocs.oauth.services.gitlab.log') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') + def test_setup_webhook_successful(self, session, mock_logger): + session().post.return_value.status_code = 201 + session().post.return_value.json.return_value = {} + success, _ = self.service.setup_webhook( + self.project, + self.integration + ) + + self.assertTrue(success) + self.assertTrue(self.integration.secret) + mock_logger.info.assert_called_with( + "GitLab webhook creation successful for project: %s", + self.project, + ) + + @mock.patch('readthedocs.oauth.services.gitlab.log') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') + def test_setup_webhook_404_error(self, session, mock_logger): + session().post.return_value.status_code = 404 + success, _ = self.service.setup_webhook( + self.project, + self.integration + ) + + self.assertFalse(success) + self.assertFalse(self.integration.secret) + mock_logger.info.assert_called_with( + 'Gitlab project does not exist or user does not have ' + 'permissions: project=%s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.gitlab.log') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') + def test_setup_webhook_value_error(self, session, mock_logger): + session().post.side_effect = ValueError + success = self.service.setup_webhook( + self.project, + self.integration + ) + + self.assertFalse(self.integration.secret) + mock_logger.exception.assert_called_with( + 'GitLab webhook creation failed for project: %s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.gitlab.log') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService._get_repo_id') + def test_update_webhook_successful(self, repo_id, session, mock_logger): + repo_id.return_value = '9999' + session().put.return_value.status_code = 200 + session().put.return_value.json.return_value = {} + success, _ = self.service.update_webhook( + self.project, + self.integration + ) + + self.assertTrue(success) + self.assertTrue(self.integration.secret) + mock_logger.info.assert_called_with( + "GitLab webhook update successful for project: %s", + self.project, + ) + + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.setup_webhook') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService._get_repo_id') + def test_update_webhook_404_error(self, repo_id, setup_webhook, session): + repo_id.return_value = '9999' + session().put.return_value.status_code = 404 + self.service.update_webhook( + self.project, + self.integration + ) + + setup_webhook.assert_called_once_with( + self.project, + self.integration + ) + + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.setup_webhook') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService._get_repo_id') + def test_update_webhook_attribute_error(self, repo_id, setup_webhook, session): + repo_id.return_value = '9999' + session().put.side_effect = AttributeError + self.service.update_webhook( + self.project, + self.integration + ) + + setup_webhook.assert_called_once_with( + self.project, + self.integration + ) + + @mock.patch('readthedocs.oauth.services.gitlab.log') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService._get_repo_id') + def test_update_webhook_value_error(self, repo_id, session, mock_logger): + repo_id.return_value = '9999' + session().put.side_effect = ValueError + self.service.update_webhook( + self.project, + self.integration + ) + + self.assertFalse(self.integration.secret) + mock_logger.exception.assert_called_with( + 'GitLab webhook update failed for project: %s', + self.project, + ) From 557fd4bd4e5af342f6dab6e53a8dae68b046c8ea Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Fri, 30 Aug 2019 20:20:30 +0600 Subject: [PATCH 06/22] update webhook creation --- readthedocs/oauth/services/bitbucket.py | 5 +++++ readthedocs/oauth/services/github.py | 16 +++++++++------- readthedocs/oauth/services/gitlab.py | 11 ++++++----- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py index df4d3ebcfeb..8a3ac2eb379 100644 --- a/readthedocs/oauth/services/bitbucket.py +++ b/readthedocs/oauth/services/bitbucket.py @@ -312,6 +312,11 @@ def update_webhook(self, project, integration): # Catch exceptions with request or deserializing JSON except (KeyError, RequestException, TypeError, ValueError): + # We get TypeError when the provider_data is None + # it only happens if the webhook attachment was not successful in the first place + if not integration.provider_data: + return self.setup_webhook(project, integration) + log.exception( 'Bitbucket webhook update failed for project: %s', project, diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index 64ff748acf3..ae1a90ecb60 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -308,18 +308,20 @@ def update_webhook(self, project, integration): if resp.status_code == 404: return self.setup_webhook(project, integration) - except AttributeError: - # We get AttributeError when the provider_data does not have anything - # it only happens if the webhook attachment was not successful in the first place - return self.setup_webhook(project, integration) # Catch exceptions with request or deserializing JSON - except (RequestException, ValueError): + except (AttributeError, RequestException, ValueError): + # We get AttributeError when the provider_data is None + # it only happens if the webhook attachment was not successful in the first place + if not integration.provider_data: + return self.setup_webhook(project, integration) + + # Set the secret to None so that the integration can be used manually. + integration.remove_secret() + log.exception( 'GitHub webhook update failed for project: %s', project, ) - # Set the secret to None so that the integration can be used manually. - integration.remove_secret() return (False, resp) else: integration.remove_secret() diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py index 2ac9f7a011d..bceb0fccec9 100644 --- a/readthedocs/oauth/services/gitlab.py +++ b/readthedocs/oauth/services/gitlab.py @@ -393,12 +393,13 @@ def update_webhook(self, project, integration): if resp.status_code == 404: return self.setup_webhook(project, integration) - except AttributeError: - # We get AttributeError when the provider_data does not have anything - # it only happens if the webhook attachment was not successful in the first place - return self.setup_webhook(project, integration) # Catch exceptions with request or deserializing JSON - except (RequestException, ValueError): + except (AttributeError, RequestException, ValueError): + # We get AttributeError when the provider_data is None + # it only happens if the webhook attachment was not successful in the first place + if not integration.provider_data: + return self.setup_webhook(project, integration) + integration.remove_secret() log.exception( From 3e0dc6c3fe2d578614e3f2375588dd0c59e70bb1 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Fri, 30 Aug 2019 20:27:27 +0600 Subject: [PATCH 07/22] fix tests --- readthedocs/rtd_tests/tests/test_oauth.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index 57a1472ceb9..ecc395d3abb 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -290,7 +290,7 @@ def test_update_webhook_successful(self, session, mock_logger): self.assertTrue(success) self.assertTrue(self.integration.secret) mock_logger.info.assert_called_with( - "GitHub webhook creation successful for project: %s", + "GitHub webhook update successful for project: %s", self.project, ) @@ -310,7 +310,10 @@ def test_update_webhook_404_error(self, setup_webhook, session): @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') @mock.patch('readthedocs.oauth.services.github.GitHubService.setup_webhook') - def test_update_webhook_attribute_error(self, setup_webhook, session): + def test_update_webhook_no_provider_data(self, setup_webhook, session): + self.integration.provider_data = None + self.integration.save() + session().patch.side_effect = AttributeError self.service.update_webhook( self.project, @@ -842,7 +845,10 @@ def test_update_webhook_404_error(self, repo_id, setup_webhook, session): @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.setup_webhook') @mock.patch('readthedocs.oauth.services.gitlab.GitLabService._get_repo_id') - def test_update_webhook_attribute_error(self, repo_id, setup_webhook, session): + def test_update_webhook_no_provider_data(self, repo_id, setup_webhook, session): + self.integration.provider_data = None + self.integration.save() + repo_id.return_value = '9999' session().put.side_effect = AttributeError self.service.update_webhook( From defd05f76956c60a4b6ecfb6007564bd03aa10d6 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Fri, 30 Aug 2019 20:41:38 +0600 Subject: [PATCH 08/22] BitBucket setup_webhook and update_webhook tests added --- readthedocs/rtd_tests/tests/test_oauth.py | 121 ++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index ecc395d3abb..c82522f38ce 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -444,9 +444,22 @@ def setUp(self): self.client.login(username='eric', password='test') self.user = User.objects.get(pk=1) self.project = Project.objects.get(slug='pip') + self.project.repo = 'https://bitbucket.org/testuser/testrepo/' + self.project.save() self.org = RemoteOrganization.objects.create(slug='rtfd', json='') self.privacy = self.project.version_privacy_level self.service = BitbucketService(user=self.user, account=None) + self.integration = get( + GitHubWebhook, + project=self.project, + provider_data={ + 'links': { + 'self': { + 'href': 'https://bitbucket.org/' + } + } + } + ) def test_make_project_pass(self): repo = self.service.create_repository( @@ -514,6 +527,114 @@ def test_import_with_no_token(self): services = BitbucketService.for_user(self.user) self.assertEqual(services, []) + @mock.patch('readthedocs.oauth.services.bitbucket.log') + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.get_session') + def test_setup_webhook_successful(self, session, mock_logger): + session().post.return_value.status_code = 201 + session().post.return_value.json.return_value = {} + success, _ = self.service.setup_webhook( + self.project, + self.integration + ) + + self.assertTrue(success) + mock_logger.info.assert_called_with( + "Bitbucket webhook creation successful for project: %s", + self.project, + ) + + @mock.patch('readthedocs.oauth.services.bitbucket.log') + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.get_session') + def test_setup_webhook_404_error(self, session, mock_logger): + session().post.return_value.status_code = 404 + success, _ = self.service.setup_webhook( + self.project, + self.integration + ) + + self.assertFalse(success) + mock_logger.info.assert_called_with( + 'Bitbucket project does not exist or user does not have ' + 'permissions: project=%s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.bitbucket.log') + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.get_session') + def test_setup_webhook_value_error(self, session, mock_logger): + session().post.side_effect = ValueError + success = self.service.setup_webhook( + self.project, + self.integration + ) + + mock_logger.exception.assert_called_with( + 'Bitbucket webhook creation failed for project: %s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.bitbucket.log') + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.get_session') + def test_update_webhook_successful(self, session, mock_logger): + session().put.return_value.status_code = 200 + session().put.return_value.json.return_value = {} + success, _ = self.service.update_webhook( + self.project, + self.integration + ) + + self.assertTrue(success) + self.assertTrue(self.integration.secret) + mock_logger.info.assert_called_with( + "Bitbucket webhook update successful for project: %s", + self.project, + ) + + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.get_session') + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.setup_webhook') + def test_update_webhook_404_error(self, setup_webhook, session): + session().put.return_value.status_code = 404 + self.service.update_webhook( + self.project, + self.integration + ) + + setup_webhook.assert_called_once_with( + self.project, + self.integration + ) + + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.get_session') + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.setup_webhook') + def test_update_webhook_no_provider_data(self, setup_webhook, session): + self.integration.provider_data = None + self.integration.save() + + session().put.side_effect = AttributeError + self.service.update_webhook( + self.project, + self.integration + ) + + setup_webhook.assert_called_once_with( + self.project, + self.integration + ) + + @mock.patch('readthedocs.oauth.services.bitbucket.log') + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.get_session') + def test_update_webhook_value_error(self, session, mock_logger): + session().put.side_effect = ValueError + self.service.update_webhook( + self.project, + self.integration + ) + + mock_logger.exception.assert_called_with( + 'Bitbucket webhook update failed for project: %s', + self.project, + ) + class GitLabOAuthTests(TestCase): From 79cdefb45610cabe9fa4c7ccf4ecd53845a9bbb8 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Wed, 4 Sep 2019 18:59:26 -0700 Subject: [PATCH 09/22] Cleanup the logic around secret clearing --- readthedocs/oauth/services/bitbucket.py | 8 +-- readthedocs/oauth/services/github.py | 40 ++++++------- readthedocs/oauth/services/gitlab.py | 59 ++++++++----------- readthedocs/oauth/utils.py | 1 + .../projects/integration_webhook_detail.html | 9 ++- 5 files changed, 54 insertions(+), 63 deletions(-) diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py index 8a3ac2eb379..2a008dc19bf 100644 --- a/readthedocs/oauth/services/bitbucket.py +++ b/readthedocs/oauth/services/bitbucket.py @@ -251,7 +251,6 @@ def setup_webhook(self, project, integration=None): 'permissions: project=%s', project, ) - return (False, resp) # Catch exceptions with request or deserializing JSON except (RequestException, ValueError): @@ -271,7 +270,8 @@ def setup_webhook(self, project, integration=None): ) except ValueError: pass - return (False, resp) + + return (False, resp) def update_webhook(self, project, integration): """ @@ -321,7 +321,6 @@ def update_webhook(self, project, integration): 'Bitbucket webhook update failed for project: %s', project, ) - return (False, resp) else: log.error( 'Bitbucket webhook update failed for project: %s', @@ -336,4 +335,5 @@ def update_webhook(self, project, integration): 'Bitbucket webhook update failure response: %s', debug_data, ) - return (False, resp) + + return (False, resp) diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index ae1a90ecb60..aed63eac6ce 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -203,7 +203,7 @@ def setup_webhook(self, project, integration=None): """ session = self.get_session() owner, repo = build_utils.get_github_username_repo(url=project.repo) - if integration: + if integration and not integration.secret: integration.recreate_secret() else: integration, _ = Integration.objects.get_or_create( @@ -239,20 +239,16 @@ def setup_webhook(self, project, integration=None): 'permissions: project=%s', project, ) - # Set the secret to None so that the integration can be used manually. - integration.remove_secret() - return (False, resp) + + # All other status codes will flow to the `else` clause below + # Catch exceptions with request or deserializing JSON except (RequestException, ValueError): - integration.remove_secret() - log.exception( 'GitHub webhook creation failed for project: %s', project, ) else: - integration.remove_secret() - log.error( 'GitHub webhook creation failed for project: %s', project, @@ -267,7 +263,10 @@ def setup_webhook(self, project, integration=None): 'GitHub webhook creation failure response: %s', debug_data, ) - return (False, resp) + + # Always remove the secret and return False if we don't return True above + integration.remove_secret() + return (False, resp) def update_webhook(self, project, integration): """ @@ -281,9 +280,16 @@ def update_webhook(self, project, integration): :rtype: (Bool, Response) """ session = self.get_session() - integration.recreate_secret() + if not integration.secret: + integration.recreate_secret() data = self.get_webhook_data(project, integration) resp = None + + # Handle the case where we don't have a proper provider_data set + # This happens with a user-managed webhook previously + if not integration.provider_data: + return self.setup_webhook(project, integration) + try: url = integration.provider_data.get('url') resp = session.patch( @@ -310,21 +316,11 @@ def update_webhook(self, project, integration): # Catch exceptions with request or deserializing JSON except (AttributeError, RequestException, ValueError): - # We get AttributeError when the provider_data is None - # it only happens if the webhook attachment was not successful in the first place - if not integration.provider_data: - return self.setup_webhook(project, integration) - - # Set the secret to None so that the integration can be used manually. - integration.remove_secret() - log.exception( 'GitHub webhook update failed for project: %s', project, ) - return (False, resp) else: - integration.remove_secret() log.error( 'GitHub webhook update failed for project: %s', project, @@ -337,7 +333,9 @@ def update_webhook(self, project, integration): 'GitHub webhook update failure response: %s', debug_data, ) - return (False, resp) + + integration.remove_secret() + return (False, resp) def send_build_status(self, build, commit, state): """ diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py index bceb0fccec9..667945e7ff3 100644 --- a/readthedocs/oauth/services/gitlab.py +++ b/readthedocs/oauth/services/gitlab.py @@ -14,7 +14,7 @@ RTD_BUILD_STATUS_API_NAME, SELECT_BUILD_STATUS, ) -from readthedocs.builds.utils import get_gitlab_username_repo +from readthedocs.builds import utils as build_utils from readthedocs.integrations.models import Integration from readthedocs.projects.models import Project @@ -52,11 +52,11 @@ def _get_repo_id(self, project): # https://docs.gitlab.com/ce/api/README.html#namespaced-path-encoding try: repo_id = json.loads(project.remote_repository.json).get('id') - except Project.remote_repository.RelatedObjectDoesNotExist: + except Exception: # Handle "Manual Import" when there is no RemoteRepository # associated with the project. It only works with gitlab.com at the # moment (doesn't support custom gitlab installations) - username, repo = get_gitlab_username_repo(project.repo) + username, repo = build_utils.get_gitlab_username_repo(project.repo) if (username, repo) == (None, None): return None @@ -278,7 +278,8 @@ def setup_webhook(self, project, integration=None): :returns: boolean based on webhook set up success :rtype: bool """ - if integration: + resp = None + if integration and not integration.secret: integration.recreate_secret() else: integration, _ = Integration.objects.get_or_create( @@ -286,14 +287,14 @@ def setup_webhook(self, project, integration=None): 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.remove_secret() - return (False, None) + return (False, resp) data = self.get_webhook_data(repo_id, project, integration) session = self.get_session() - resp = None try: resp = session.post( '{url}/api/v4/projects/{repo_id}/hooks'.format( @@ -319,27 +320,21 @@ def setup_webhook(self, project, integration=None): 'permissions: project=%s', project, ) - # Set the secret to None so that the integration can be used manually. - integration.remove_secret() - - return (False, resp) except (RequestException, ValueError): - integration.remove_secret() - log.exception( 'GitLab webhook creation failed for project: %s', project, ) - return (False, resp) else: - integration.remove_secret() - log.error( 'GitLab webhook creation failed for project: %s', project, ) - return (False, resp) + + # Always remove secret and return False if we don't return True above + integration.remove_secret() + return (False, resp) def update_webhook(self, project, integration): """ @@ -356,16 +351,22 @@ def update_webhook(self, project, integration): :rtype: (Bool, Response) """ + resp = None session = self.get_session() - repo_id = self._get_repo_id(project) + if repo_id is None: - return (False, None) + return (False, resp) + + # When we don't have provider_data, we aren't managing this webhook so setup a new one + if not integration.provider_data: + return self.setup_webhook(project, integration) + + if not integration.secret: + integration.recreate_secret() - integration.recreate_secret() data = self.get_webhook_data(repo_id, project, integration) - resp = None try: hook_id = integration.provider_data.get('id') resp = session.put( @@ -395,20 +396,11 @@ def update_webhook(self, project, integration): # Catch exceptions with request or deserializing JSON except (AttributeError, RequestException, ValueError): - # We get AttributeError when the provider_data is None - # it only happens if the webhook attachment was not successful in the first place - if not integration.provider_data: - return self.setup_webhook(project, integration) - - integration.remove_secret() - log.exception( 'GitLab webhook update failed for project: %s', project, ) else: - integration.remove_secret() - log.error( 'GitLab webhook update failed for project: %s', project, @@ -418,7 +410,9 @@ def update_webhook(self, project, integration): except ValueError: debug_data = resp.content log.debug('GitLab webhook update failure response: %s', debug_data) - return (False, resp) + + integration.remove_secret() + return (False, resp) def send_build_status(self, build, commit, state): """ @@ -433,13 +427,14 @@ def send_build_status(self, build, commit, state): :returns: boolean based on commit status creation was successful or not. :rtype: Bool """ + resp = None session = self.get_session() project = build.project repo_id = self._get_repo_id(project) if repo_id is None: - return (False, None) + return (False, resp) # select the correct state and description. gitlab_build_state = SELECT_BUILD_STATUS[state]['gitlab'] @@ -458,8 +453,6 @@ def send_build_status(self, build, commit, state): } url = self.adapter.provider_base_url - resp = None - try: resp = session.post( f'{url}/api/v4/projects/{repo_id}/statuses/{commit}', diff --git a/readthedocs/oauth/utils.py b/readthedocs/oauth/utils.py index 2370918b712..babb766488b 100644 --- a/readthedocs/oauth/utils.py +++ b/readthedocs/oauth/utils.py @@ -49,6 +49,7 @@ def update_webhook(project, integration, request=None): project.has_valid_webhook = True project.save() return True + messages.error( request, _( diff --git a/readthedocs/templates/projects/integration_webhook_detail.html b/readthedocs/templates/projects/integration_webhook_detail.html index b7d3c3292d8..4fdebcd4090 100644 --- a/readthedocs/templates/projects/integration_webhook_detail.html +++ b/readthedocs/templates/projects/integration_webhook_detail.html @@ -19,9 +19,8 @@ {% if integration.has_sync and integration.can_sync %}

{% blocktrans trimmed %} - This webhook was configured when this project was imported - or it was manually created with the correct configuration. If this - integration is not functioning correctly, try re-syncing the webhook: + This integration is being managed automatically by Read the Docs. If + it isn't functioning correctly, try re-syncing the webhook: {% endblocktrans %}

@@ -40,8 +39,8 @@ {% if integration.has_sync and not integration.can_sync %}

{% blocktrans trimmed %} - This webhook was created automatically or manually from an existing webhook - configured on your repository. If this integration is not functioning correctly, + This integration is not managed by Read the Docs currently. + 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. From 06e9d24f31561b49e8aa653361434fe4694a27ba Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 5 Sep 2019 12:36:20 +0600 Subject: [PATCH 10/22] fix tests --- readthedocs/rtd_tests/tests/test_oauth.py | 31 ++++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index c82522f38ce..6d487ae7c12 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -238,6 +238,8 @@ def test_setup_webhook_successful(self, session, mock_logger): self.integration ) + self.integration.refresh_from_db() + self.assertTrue(success) self.assertTrue(self.integration.secret) mock_logger.info.assert_called_with( @@ -253,9 +255,10 @@ def test_setup_webhook_404_error(self, session, mock_logger): self.project, self.integration ) + self.integration.refresh_from_db() self.assertFalse(success) - self.assertFalse(self.integration.secret) + self.assertIsNone(self.integration.secret) mock_logger.info.assert_called_with( 'GitHub project does not exist or user does not have ' 'permissions: project=%s', @@ -271,7 +274,9 @@ def test_setup_webhook_value_error(self, session, mock_logger): self.integration ) - self.assertFalse(self.integration.secret) + self.integration.refresh_from_db() + + self.assertIsNone(self.integration.secret) mock_logger.exception.assert_called_with( 'GitHub webhook creation failed for project: %s', self.project, @@ -287,6 +292,8 @@ def test_update_webhook_successful(self, session, mock_logger): self.integration ) + self.integration.refresh_from_db() + self.assertTrue(success) self.assertTrue(self.integration.secret) mock_logger.info.assert_called_with( @@ -334,7 +341,9 @@ def test_update_webhook_value_error(self, session, mock_logger): self.integration ) - self.assertFalse(self.integration.secret) + self.integration.refresh_from_db() + + self.assertIsNone(self.integration.secret) mock_logger.exception.assert_called_with( 'GitHub webhook update failed for project: %s', self.project, @@ -889,6 +898,8 @@ def test_setup_webhook_successful(self, session, mock_logger): self.integration ) + self.integration.refresh_from_db() + self.assertTrue(success) self.assertTrue(self.integration.secret) mock_logger.info.assert_called_with( @@ -905,8 +916,10 @@ def test_setup_webhook_404_error(self, session, mock_logger): self.integration ) + self.integration.refresh_from_db() + self.assertFalse(success) - self.assertFalse(self.integration.secret) + self.assertIsNone(self.integration.secret) mock_logger.info.assert_called_with( 'Gitlab project does not exist or user does not have ' 'permissions: project=%s', @@ -922,7 +935,9 @@ def test_setup_webhook_value_error(self, session, mock_logger): self.integration ) - self.assertFalse(self.integration.secret) + self.integration.refresh_from_db() + + self.assertIsNone(self.integration.secret) mock_logger.exception.assert_called_with( 'GitLab webhook creation failed for project: %s', self.project, @@ -940,6 +955,8 @@ def test_update_webhook_successful(self, repo_id, session, mock_logger): self.integration ) + self.integration.refresh_from_db() + self.assertTrue(success) self.assertTrue(self.integration.secret) mock_logger.info.assert_called_with( @@ -993,7 +1010,9 @@ def test_update_webhook_value_error(self, repo_id, session, mock_logger): self.integration ) - self.assertFalse(self.integration.secret) + self.integration.refresh_from_db() + + self.assertIsNone(self.integration.secret) mock_logger.exception.assert_called_with( 'GitLab webhook update failed for project: %s', self.project, From 16c9d09df9b3eabfe44f6779ce502f6e67c58410 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 5 Sep 2019 12:48:44 +0600 Subject: [PATCH 11/22] integration bug fix --- readthedocs/oauth/services/github.py | 9 ++++++--- readthedocs/oauth/services/gitlab.py | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index aed63eac6ce..3e88bc2d150 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -203,13 +203,16 @@ def setup_webhook(self, project, integration=None): """ session = self.get_session() owner, repo = build_utils.get_github_username_repo(url=project.repo) - if integration and not integration.secret: - integration.recreate_secret() - else: + + if not integration: integration, _ = Integration.objects.get_or_create( project=project, integration_type=Integration.GITHUB_WEBHOOK, ) + + if not integration.secret: + integration.recreate_secret() + data = self.get_webhook_data(project, integration) resp = None try: diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py index 667945e7ff3..fd43204b7ad 100644 --- a/readthedocs/oauth/services/gitlab.py +++ b/readthedocs/oauth/services/gitlab.py @@ -279,13 +279,16 @@ def setup_webhook(self, project, integration=None): :rtype: bool """ resp = None - if integration and not integration.secret: - integration.recreate_secret() - else: + + if not integration: integration, _ = Integration.objects.get_or_create( project=project, integration_type=Integration.GITLAB_WEBHOOK, ) + + if not integration.secret: + integration.recreate_secret() + repo_id = self._get_repo_id(project) if repo_id is None: From 76467d85f32a10ffc5e936c36a7aee81200171a0 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 5 Sep 2019 17:49:33 +0600 Subject: [PATCH 12/22] Get provider data from GitHub Webhooks API --- readthedocs/oauth/services/base.py | 13 +++++ readthedocs/oauth/services/github.py | 72 +++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py index 98a12b58f3e..98e3dcd170a 100644 --- a/readthedocs/oauth/services/base.py +++ b/readthedocs/oauth/services/base.py @@ -226,6 +226,19 @@ def get_paginated_results(self, response): """ raise NotImplementedError + def get_provider_data(self, project, integration): + """ + Gets provider data from GitHub Webhooks API. + + :param project: project + :type project: Project + :param integration: Integration for the project + :type integration: Integration + :returns: Dictionary containing provider data from the API or None + :rtype: dict + """ + raise NotImplementedError + def setup_webhook(self, project, integration=None): """ Setup webhook for project. diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index 3e88bc2d150..b32ff2c000d 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -190,6 +190,71 @@ def get_webhook_data(self, project, integration): 'events': ['push', 'pull_request', 'create', 'delete'], }) + def get_provider_data(self, project, integration): + """ + Gets provider data from GitHub Webhooks API. + + :param project: project + :type project: Project + :param integration: Integration for the project + :type integration: Integration + :returns: Dictionary containing provider data from the API or None + :rtype: dict + """ + + if integration.provider_data: + return integration.provider_data + + session = self.get_session() + owner, repo = build_utils.get_github_username_repo(url=project.repo) + + rtd_webhook_url = 'https://{domain}{path}'.format( + domain=settings.PRODUCTION_DOMAIN, + path=reverse( + 'api_webhook', + kwargs={ + 'project_slug': project.slug, + 'integration_pk': integration.pk, + }, + ) + ) + + try: + resp = session.get( + ( + 'https://api.github.com/repos/{owner}/{repo}/hooks' + .format(owner=owner, repo=repo) + ), + ) + + if resp.status_code == 200: + recv_data = resp.json() + + for webhook_data in recv_data: + if webhook_data["config"]["url"] == rtd_webhook_url: + integration.provider_data = webhook_data + integration.save() + + log.info( + 'GitHub integration updated with provider data for project: %s', + project, + ) + break + else: + log.info( + 'GitHub project does not exist or user does not have ' + 'permissions: project=%s', + project, + ) + + except Exception: + log.exception( + 'GitHub webhook Listing failed for project: %s', + project, + ) + + return integration.provider_data + def setup_webhook(self, project, integration=None): """ Set up GitHub project webhook for project. @@ -288,13 +353,16 @@ def update_webhook(self, project, integration): data = self.get_webhook_data(project, integration) resp = None + provider_data = self.get_provider_data(project, integration) + # Handle the case where we don't have a proper provider_data set # This happens with a user-managed webhook previously - if not integration.provider_data: + if not provider_data: return self.setup_webhook(project, integration) try: - url = integration.provider_data.get('url') + url = provider_data.get('url') + resp = session.patch( url, data=data, From 81d82929f736f2a6522444c09590e3b1f20fcce6 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 5 Sep 2019 18:15:19 +0600 Subject: [PATCH 13/22] GitHub Provider Data tests added --- readthedocs/rtd_tests/tests/test_oauth.py | 97 +++++++++++++++++++++-- 1 file changed, 92 insertions(+), 5 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index 6d487ae7c12..06f5b02d116 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -4,6 +4,7 @@ from django.contrib.auth.models import User from django.test import TestCase from django.test.utils import override_settings +from django.urls import reverse from django_dynamic_fixture import get @@ -46,6 +47,14 @@ def setUp(self): 'url': 'https://github.com/' } ) + self.provider_data = [ + { + "config": { + "url": "https://example.com/webhook" + }, + "url": "https://api.github.com/repos/test/Hello-World/hooks/12345678", + } + ] def test_make_project_pass(self): repo_json = { @@ -241,7 +250,7 @@ def test_setup_webhook_successful(self, session, mock_logger): self.integration.refresh_from_db() self.assertTrue(success) - self.assertTrue(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.info.assert_called_with( "GitHub webhook creation successful for project: %s", self.project, @@ -295,7 +304,7 @@ def test_update_webhook_successful(self, session, mock_logger): self.integration.refresh_from_db() self.assertTrue(success) - self.assertTrue(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.info.assert_called_with( "GitHub webhook update successful for project: %s", self.project, @@ -349,6 +358,84 @@ def test_update_webhook_value_error(self, session, mock_logger): self.project, ) + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_get_provider_data_successful(self, session, mock_logger): + self.integration.provider_data = {} + self.integration.save() + + webhook_data = self.provider_data + rtd_webhook_url = 'https://{domain}{path}'.format( + domain=settings.PRODUCTION_DOMAIN, + path=reverse( + 'api_webhook', + kwargs={ + 'project_slug': self.project.slug, + 'integration_pk': self.integration.pk, + }, + ) + ) + webhook_data[0]["config"]["url"] = rtd_webhook_url + + session().get.return_value.status_code = 200 + session().get.return_value.json.return_value = webhook_data + + self.service.get_provider_data( + self.project, + self.integration + ) + + self.integration.refresh_from_db() + + self.assertEqual(self.integration.provider_data, webhook_data[0]) + mock_logger.info.assert_called_with( + 'GitHub integration updated with provider data for project: %s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_get_provider_data_404_error(self, session, mock_logger): + self.integration.provider_data = {} + self.integration.save() + + session().get.return_value.status_code = 404 + + self.service.get_provider_data( + self.project, + self.integration + ) + + self.integration.refresh_from_db() + + self.assertEqual(self.integration.provider_data, {}) + mock_logger.info.assert_called_with( + 'GitHub project does not exist or user does not have ' + 'permissions: project=%s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_get_provider_data_attribute_error(self, session, mock_logger): + self.integration.provider_data = {} + self.integration.save() + + session().get.side_effect = AttributeError + + self.service.get_provider_data( + self.project, + self.integration + ) + + self.integration.refresh_from_db() + + self.assertEqual(self.integration.provider_data, {}) + mock_logger.exception.assert_called_with( + 'GitHub webhook Listing failed for project: %s', + self.project, + ) + class BitbucketOAuthTests(TestCase): @@ -593,7 +680,7 @@ def test_update_webhook_successful(self, session, mock_logger): ) self.assertTrue(success) - self.assertTrue(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.info.assert_called_with( "Bitbucket webhook update successful for project: %s", self.project, @@ -901,7 +988,7 @@ def test_setup_webhook_successful(self, session, mock_logger): self.integration.refresh_from_db() self.assertTrue(success) - self.assertTrue(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.info.assert_called_with( "GitLab webhook creation successful for project: %s", self.project, @@ -958,7 +1045,7 @@ def test_update_webhook_successful(self, repo_id, session, mock_logger): self.integration.refresh_from_db() self.assertTrue(success) - self.assertTrue(self.integration.secret) + self.assertIsNotNone(self.integration.secret) mock_logger.info.assert_called_with( "GitLab webhook update successful for project: %s", self.project, From a9cc473639028b93e6f9113fda35cdd2f532ef77 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 5 Sep 2019 18:22:53 +0600 Subject: [PATCH 14/22] lint fix --- readthedocs/oauth/services/github.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index b32ff2c000d..406c413a421 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -223,7 +223,7 @@ def get_provider_data(self, project, integration): resp = session.get( ( 'https://api.github.com/repos/{owner}/{repo}/hooks' - .format(owner=owner, repo=repo) + .format(owner=owner, repo=repo) ), ) From 55e27ee2f4f3e3a6875b55991ea97da076d32d11 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 5 Sep 2019 22:09:57 +0600 Subject: [PATCH 15/22] GitLab get_provider_data method added --- readthedocs/oauth/services/base.py | 2 +- readthedocs/oauth/services/gitlab.py | 82 ++++++++++++++++++++++++++-- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py index 98e3dcd170a..e093ae24408 100644 --- a/readthedocs/oauth/services/base.py +++ b/readthedocs/oauth/services/base.py @@ -228,7 +228,7 @@ def get_paginated_results(self, response): def get_provider_data(self, project, integration): """ - Gets provider data from GitHub Webhooks API. + Gets provider data from Git Providers Webhooks API. :param project: project :type project: Project diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py index fd43204b7ad..04a841786a0 100644 --- a/readthedocs/oauth/services/gitlab.py +++ b/readthedocs/oauth/services/gitlab.py @@ -267,6 +267,75 @@ def get_webhook_data(self, repo_id, project, integration): 'wiki_events': False, }) + def get_provider_data(self, project, integration): + """ + Gets provider data from GitLab Webhooks API. + + :param project: project + :type project: Project + :param integration: Integration for the project + :type integration: Integration + :returns: Dictionary containing provider data from the API or None + :rtype: dict + """ + + if integration.provider_data: + return integration.provider_data + + repo_id = self._get_repo_id(project) + + if repo_id is None: + return None + + session = self.get_session() + + rtd_webhook_url = 'https://{domain}{path}'.format( + domain=settings.PRODUCTION_DOMAIN, + path=reverse( + 'api_webhook', + kwargs={ + 'project_slug': project.slug, + 'integration_pk': integration.pk, + }, + ) + ) + + try: + resp = session.get( + '{url}/api/v4/projects/{repo_id}/hooks'.format( + url=self.adapter.provider_base_url, + repo_id=repo_id, + ), + ) + + if resp.status_code == 200: + recv_data = resp.json() + + for webhook_data in recv_data: + if webhook_data["url"] == rtd_webhook_url: + integration.provider_data = webhook_data + integration.save() + + log.info( + 'GitLab integration updated with provider data for project: %s', + project, + ) + break + else: + log.info( + 'GitLab project does not exist or user does not have ' + 'permissions: project=%s', + project, + ) + + except Exception: + log.exception( + 'GitLab webhook Listing failed for project: %s', + project, + ) + + return integration.provider_data + def setup_webhook(self, project, integration=None): """ Set up GitLab project webhook for project. @@ -354,6 +423,13 @@ def update_webhook(self, project, integration): :rtype: (Bool, Response) """ + provider_data = self.get_provider_data(project, integration) + + # Handle the case where we don't have a proper provider_data set + # This happens with a user-managed webhook previously + if not provider_data: + return self.setup_webhook(project, integration) + resp = None session = self.get_session() repo_id = self._get_repo_id(project) @@ -361,17 +437,13 @@ def update_webhook(self, project, integration): if repo_id is None: return (False, resp) - # When we don't have provider_data, we aren't managing this webhook so setup a new one - if not integration.provider_data: - return self.setup_webhook(project, integration) - if not integration.secret: integration.recreate_secret() data = self.get_webhook_data(repo_id, project, integration) try: - hook_id = integration.provider_data.get('id') + hook_id = provider_data.get('id') resp = session.put( '{url}/api/v4/projects/{repo_id}/hooks/{hook_id}'.format( url=self.adapter.provider_base_url, From e8239c69252d3fb79f7df3139df535b443df0748 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 5 Sep 2019 22:21:35 +0600 Subject: [PATCH 16/22] Gitlab get_provider_data tests added --- readthedocs/rtd_tests/tests/test_oauth.py | 85 +++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index 06f5b02d116..1e8e17e3a36 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -845,6 +845,13 @@ def setUp(self): 'id': '999999999' } ) + self.provider_data = [ + { + 'id': 1084320, + 'url': 'https://readthedocs.io/api/v2/webhook/test/99999999/', + } + + ] def get_private_repo_data(self): """Manipulate repo response data to get private repo data.""" @@ -1104,3 +1111,81 @@ def test_update_webhook_value_error(self, repo_id, session, mock_logger): 'GitLab webhook update failed for project: %s', self.project, ) + + @mock.patch('readthedocs.oauth.services.gitlab.log') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') + def test_get_provider_data_successful(self, session, mock_logger): + self.integration.provider_data = {} + self.integration.save() + + webhook_data = self.provider_data + rtd_webhook_url = 'https://{domain}{path}'.format( + domain=settings.PRODUCTION_DOMAIN, + path=reverse( + 'api_webhook', + kwargs={ + 'project_slug': self.project.slug, + 'integration_pk': self.integration.pk, + }, + ) + ) + webhook_data[0]["url"] = rtd_webhook_url + + session().get.return_value.status_code = 200 + session().get.return_value.json.return_value = webhook_data + + self.service.get_provider_data( + self.project, + self.integration + ) + + self.integration.refresh_from_db() + + self.assertEqual(self.integration.provider_data, webhook_data[0]) + mock_logger.info.assert_called_with( + 'GitLab integration updated with provider data for project: %s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.gitlab.log') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') + def test_get_provider_data_404_error(self, session, mock_logger): + self.integration.provider_data = {} + self.integration.save() + + session().get.return_value.status_code = 404 + + self.service.get_provider_data( + self.project, + self.integration + ) + + self.integration.refresh_from_db() + + self.assertEqual(self.integration.provider_data, {}) + mock_logger.info.assert_called_with( + 'GitLab project does not exist or user does not have ' + 'permissions: project=%s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.gitlab.log') + @mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session') + def test_get_provider_data_attribute_error(self, session, mock_logger): + self.integration.provider_data = {} + self.integration.save() + + session().get.side_effect = AttributeError + + self.service.get_provider_data( + self.project, + self.integration + ) + + self.integration.refresh_from_db() + + self.assertEqual(self.integration.provider_data, {}) + mock_logger.exception.assert_called_with( + 'GitLab webhook Listing failed for project: %s', + self.project, + ) \ No newline at end of file From cb5fc4b47a547092d8c48adc5a203e2d4bea20d5 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 5 Sep 2019 22:50:44 +0600 Subject: [PATCH 17/22] BitBucket get_provider_data method added --- readthedocs/oauth/services/bitbucket.py | 86 +++++++++++++++++++++-- readthedocs/rtd_tests/tests/test_oauth.py | 5 +- 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py index 2a008dc19bf..e72fc0ba672 100644 --- a/readthedocs/oauth/services/bitbucket.py +++ b/readthedocs/oauth/services/bitbucket.py @@ -206,6 +206,78 @@ def get_webhook_data(self, project, integration): 'events': ['repo:push'], }) + def get_provider_data(self, project, integration): + """ + Gets provider data from BitBucket Webhooks API. + + :param project: project + :type project: Project + :param integration: Integration for the project + :type integration: Integration + :returns: Dictionary containing provider data from the API or None + :rtype: dict + """ + + if integration.provider_data: + return integration.provider_data + + session = self.get_session() + owner, repo = build_utils.get_bitbucket_username_repo(url=project.repo) + + if not integration: + integration, _ = Integration.objects.get_or_create( + project=project, + integration_type=Integration.BITBUCKET_WEBHOOK, + ) + data = self.get_webhook_data(project, integration) + + rtd_webhook_url = 'https://{domain}{path}'.format( + domain=settings.PRODUCTION_DOMAIN, + path=reverse( + 'api_webhook', + kwargs={ + 'project_slug': project.slug, + 'integration_pk': integration.pk, + }, + ) + ) + + try: + resp = session.get( + ( + 'https://api.bitbucket.org/2.0/repositories/{owner}/{repo}/hooks' + .format(owner=owner, repo=repo) + ), + ) + + if resp.status_code == 200: + recv_data = resp.json() + + for webhook_data in recv_data["values"]: + if webhook_data["url"] == rtd_webhook_url: + integration.provider_data = webhook_data + integration.save() + + log.info( + 'Bitbucket integration updated with provider data for project: %s', + project, + ) + break + else: + log.info( + 'Bitbucket project does not exist or user does not have ' + 'permissions: project=%s', + project, + ) + + except Exception: + log.exception( + 'Bitbucket webhook Listing failed for project: %s', + project, + ) + + return integration.provider_data + def setup_webhook(self, project, integration=None): """ Set up Bitbucket project webhook for project. @@ -219,6 +291,7 @@ def setup_webhook(self, project, integration=None): """ session = self.get_session() owner, repo = build_utils.get_bitbucket_username_repo(url=project.repo) + if not integration: integration, _ = Integration.objects.get_or_create( project=project, @@ -284,6 +357,13 @@ def update_webhook(self, project, integration): :returns: boolean based on webhook set up success, and requests Response object :rtype: (Bool, Response) """ + provider_data = self.get_provider_data(project, integration) + + # Handle the case where we don't have a proper provider_data set + # This happens with a user-managed webhook previously + if not provider_data: + return self.setup_webhook(project, integration) + session = self.get_session() data = self.get_webhook_data(project, integration) resp = None @@ -295,6 +375,7 @@ def update_webhook(self, project, integration): data=data, headers={'content-type': 'application/json'}, ) + if resp.status_code == 200: recv_data = resp.json() integration.provider_data = recv_data @@ -312,11 +393,6 @@ def update_webhook(self, project, integration): # Catch exceptions with request or deserializing JSON except (KeyError, RequestException, TypeError, ValueError): - # We get TypeError when the provider_data is None - # it only happens if the webhook attachment was not successful in the first place - if not integration.provider_data: - return self.setup_webhook(project, integration) - log.exception( 'Bitbucket webhook update failed for project: %s', project, diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index 1e8e17e3a36..9ab271c3005 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -50,7 +50,7 @@ def setUp(self): self.provider_data = [ { "config": { - "url": "https://example.com/webhook" + "url": "https://example.com/webhook" }, "url": "https://api.github.com/repos/test/Hello-World/hooks/12345678", } @@ -850,7 +850,6 @@ def setUp(self): 'id': 1084320, 'url': 'https://readthedocs.io/api/v2/webhook/test/99999999/', } - ] def get_private_repo_data(self): @@ -1188,4 +1187,4 @@ def test_get_provider_data_attribute_error(self, session, mock_logger): mock_logger.exception.assert_called_with( 'GitLab webhook Listing failed for project: %s', self.project, - ) \ No newline at end of file + ) From 4b9e2b5bbfedea2fd0d78dc3a95c2715b8b9fc62 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 5 Sep 2019 23:04:44 +0600 Subject: [PATCH 18/22] Bitbucket get_provider_data method tests added --- readthedocs/oauth/services/bitbucket.py | 2 +- readthedocs/rtd_tests/tests/test_oauth.py | 89 +++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py index e72fc0ba672..080883fea84 100644 --- a/readthedocs/oauth/services/bitbucket.py +++ b/readthedocs/oauth/services/bitbucket.py @@ -369,7 +369,7 @@ def update_webhook(self, project, integration): resp = None try: # Expect to throw KeyError here if provider_data is invalid - url = integration.provider_data['links']['self']['href'] + url = provider_data['links']['self']['href'] resp = session.put( url, data=data, diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index 9ab271c3005..b0c915de8be 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -556,6 +556,17 @@ def setUp(self): } } ) + self.provider_data = { + 'values': [{ + 'links': { + 'self': { + 'href': 'https://bitbucket.org/' + } + }, + 'url': 'https://readthedocs.io/api/v2/webhook/test/99999999/', + + },] + } def test_make_project_pass(self): repo = self.service.create_repository( @@ -731,6 +742,84 @@ def test_update_webhook_value_error(self, session, mock_logger): self.project, ) + @mock.patch('readthedocs.oauth.services.bitbucket.log') + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.get_session') + def test_get_provider_data_successful(self, session, mock_logger): + self.integration.provider_data = {} + self.integration.save() + + webhook_data = self.provider_data + rtd_webhook_url = 'https://{domain}{path}'.format( + domain=settings.PRODUCTION_DOMAIN, + path=reverse( + 'api_webhook', + kwargs={ + 'project_slug': self.project.slug, + 'integration_pk': self.integration.pk, + }, + ) + ) + webhook_data['values'][0]["url"] = rtd_webhook_url + + session().get.return_value.status_code = 200 + session().get.return_value.json.return_value = webhook_data + + self.service.get_provider_data( + self.project, + self.integration + ) + + self.integration.refresh_from_db() + + self.assertEqual(self.integration.provider_data, webhook_data['values'][0]) + mock_logger.info.assert_called_with( + 'Bitbucket integration updated with provider data for project: %s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.bitbucket.log') + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.get_session') + def test_get_provider_data_404_error(self, session, mock_logger): + self.integration.provider_data = {} + self.integration.save() + + session().get.return_value.status_code = 404 + + self.service.get_provider_data( + self.project, + self.integration + ) + + self.integration.refresh_from_db() + + self.assertEqual(self.integration.provider_data, {}) + mock_logger.info.assert_called_with( + 'Bitbucket project does not exist or user does not have ' + 'permissions: project=%s', + self.project, + ) + + @mock.patch('readthedocs.oauth.services.bitbucket.log') + @mock.patch('readthedocs.oauth.services.bitbucket.BitbucketService.get_session') + def test_get_provider_data_attribute_error(self, session, mock_logger): + self.integration.provider_data = {} + self.integration.save() + + session().get.side_effect = AttributeError + + self.service.get_provider_data( + self.project, + self.integration + ) + + self.integration.refresh_from_db() + + self.assertEqual(self.integration.provider_data, {}) + mock_logger.exception.assert_called_with( + 'Bitbucket webhook Listing failed for project: %s', + self.project, + ) + class GitLabOAuthTests(TestCase): From 0c0ad42ac87cfd5df1248f9dccbada603c11e5da Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 5 Sep 2019 23:08:10 +0600 Subject: [PATCH 19/22] lint fix --- readthedocs/rtd_tests/tests/test_oauth.py | 1 - 1 file changed, 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index b0c915de8be..e73382367dd 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -564,7 +564,6 @@ def setUp(self): } }, 'url': 'https://readthedocs.io/api/v2/webhook/test/99999999/', - },] } From ea0e15d4454d018a7367b3dfeaf85b1952ec8954 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 9 Sep 2019 23:43:35 -0500 Subject: [PATCH 20/22] Remove dead code --- readthedocs/oauth/services/bitbucket.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py index 080883fea84..144b5e4508c 100644 --- a/readthedocs/oauth/services/bitbucket.py +++ b/readthedocs/oauth/services/bitbucket.py @@ -224,13 +224,6 @@ def get_provider_data(self, project, integration): session = self.get_session() owner, repo = build_utils.get_bitbucket_username_repo(url=project.repo) - if not integration: - integration, _ = Integration.objects.get_or_create( - project=project, - integration_type=Integration.BITBUCKET_WEBHOOK, - ) - data = self.get_webhook_data(project, integration) - rtd_webhook_url = 'https://{domain}{path}'.format( domain=settings.PRODUCTION_DOMAIN, path=reverse( @@ -239,7 +232,7 @@ def get_provider_data(self, project, integration): 'project_slug': project.slug, 'integration_pk': integration.pk, }, - ) + ), ) try: From 52d839b2872265c5b547e7534e87f1ba6eed5cb1 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 10 Sep 2019 00:08:18 -0500 Subject: [PATCH 21/22] Update docs --- docs/webhooks.rst | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/webhooks.rst b/docs/webhooks.rst index 4e5bc241946..f5119c30262 100644 --- a/docs/webhooks.rst +++ b/docs/webhooks.rst @@ -36,7 +36,13 @@ After you have added the integration, you'll see a link to information about the As an example, the URL pattern looks like this: *https://readthedocs.org/api/v2/webhook///*. -Use this URL when setting up a new webhook with your provider -- these steps vary depending on the provider: +Use this URL when setting up a new webhook with your provider -- these steps vary depending on the provider. + +.. note:: + + If your account is connected to the provider, + we'll try to setup the webhook automatically. + If something fails, you can still setup the webhook manually. .. _webhook-integration-github: From b5cd50abd175d5ad040186533ede56db7cb81381 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 10 Sep 2019 11:05:35 -0500 Subject: [PATCH 22/22] Update docs --- docs/webhooks.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/webhooks.rst b/docs/webhooks.rst index f5119c30262..cfeb604591d 100644 --- a/docs/webhooks.rst +++ b/docs/webhooks.rst @@ -167,8 +167,6 @@ we create a secret for every integration that offers a way to verify that a webh Currently, `GitHub `__ and `GitLab `__ offer a way to check this. -When :ref:`resyncing the webhook `, the secret is changed too. - Troubleshooting ---------------