Skip to content

Commit b4bdd3d

Browse files
stsewdstsewd
authored
GitHub App: fix PR builds and import (#12126)
- Update admin to make it more usable for remote repos/orgs - When implementing the signature of the base service class I forgot to return a tuple, the tuple contains a boolean and the response, which is weird, because all other method return only a boolean, and we never use the response. So I went ahead and removed the need to return a tuple from all services. - Allow activating PR previews for projects connected to a GH app from the form. - Build from PRs for projects that explicitly have the option enabled, since now with the GH app we always listen to all events. - Tests tests tests
1 parent 9174344 commit b4bdd3d

File tree

14 files changed

+294
-48
lines changed

14 files changed

+294
-48
lines changed

‎readthedocs/oauth/admin.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class RemoteRepositoryAdmin(admin.ModelAdmin):
2626
"created",
2727
"modified",
2828
)
29-
raw_id_fields = ("organization",)
29+
raw_id_fields = ("organization", "github_app_installation")
3030
list_select_related = ("organization",)
3131
list_filter = (
3232
"vcs_provider",
@@ -86,8 +86,25 @@ class RemoteRepositoryRelationAdmin(admin.ModelAdmin):
8686
list_select_related = (
8787
"remote_repository",
8888
"user",
89+
"account",
90+
)
91+
list_display = (
92+
"id",
93+
"remote_repository",
94+
"user",
95+
"account",
96+
"vcs_provider",
97+
"admin",
98+
)
99+
list_filter = (
100+
"remote_repository__vcs_provider",
101+
"admin",
89102
)
90103

104+
def vcs_provider(self, obj):
105+
"""Get the display name for the VCS provider."""
106+
return obj.remote_repository.vcs_provider
107+
91108

92109
@admin.register(RemoteOrganizationRelation)
93110
class RemoteOrganizationRelationAdmin(admin.ModelAdmin):
@@ -101,4 +118,17 @@ class RemoteOrganizationRelationAdmin(admin.ModelAdmin):
101118
list_select_related = (
102119
"remote_organization",
103120
"user",
121+
"account",
122+
)
123+
list_display = (
124+
"id",
125+
"remote_organization",
126+
"user",
127+
"account",
128+
"vcs_provider",
104129
)
130+
list_filter = ("remote_organization__vcs_provider",)
131+
132+
def vcs_provider(self, obj):
133+
"""Get the display name for the VCS provider."""
134+
return obj.remote_organization.vcs_provider

‎readthedocs/oauth/services/base.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,20 +65,19 @@ def sync(self):
6565
"""
6666
raise NotImplementedError
6767

68-
def setup_webhook(self, project, integration=None):
68+
def setup_webhook(self, project, integration=None) -> bool:
6969
"""
7070
Setup webhook for project.
7171
7272
:param project: project to set up webhook for
7373
:type project: Project
7474
:param integration: Integration for the project
7575
:type integration: Integration
76-
:returns: boolean based on webhook set up success, and requests Response object
77-
:rtype: (Bool, Response)
76+
:returns: boolean based on webhook set up success
7877
"""
7978
raise NotImplementedError
8079

81-
def update_webhook(self, project, integration):
80+
def update_webhook(self, project, integration) -> bool:
8281
"""
8382
Update webhook integration.
8483
@@ -87,7 +86,6 @@ def update_webhook(self, project, integration):
8786
:param integration: Webhook integration to update
8887
:type integration: Integration
8988
:returns: boolean based on webhook update success, and requests Response object
90-
:rtype: (Bool, Response)
9189
"""
9290
raise NotImplementedError
9391

‎readthedocs/oauth/services/bitbucket.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ def get_provider_data(self, project, integration):
273273

274274
return integration.provider_data
275275

276-
def setup_webhook(self, project, integration=None):
276+
def setup_webhook(self, project, integration=None) -> bool:
277277
"""
278278
Set up Bitbucket project webhook for project.
279279
@@ -282,7 +282,6 @@ def setup_webhook(self, project, integration=None):
282282
:param integration: Integration for the project
283283
:type integration: Integration
284284
:returns: boolean based on webhook set up success, and requests Response object
285-
:rtype: (Bool, Response)
286285
"""
287286
owner, repo = build_utils.get_bitbucket_username_repo(url=project.repo)
288287
url = f"{self.base_api_url}/2.0/repositories/{owner}/{repo}/hooks"
@@ -313,7 +312,7 @@ def setup_webhook(self, project, integration=None):
313312
log.debug(
314313
"Bitbucket webhook creation successful for project.",
315314
)
316-
return (True, resp)
315+
return True
317316

318317
if resp.status_code in [401, 403, 404]:
319318
log.info(
@@ -333,9 +332,9 @@ def setup_webhook(self, project, integration=None):
333332
except (RequestException, ValueError):
334333
log.exception("Bitbucket webhook creation failed for project.")
335334

336-
return (False, resp)
335+
return False
337336

338-
def update_webhook(self, project, integration):
337+
def update_webhook(self, project, integration) -> bool:
339338
"""
340339
Update webhook integration.
341340
@@ -344,7 +343,6 @@ def update_webhook(self, project, integration):
344343
:param integration: Webhook integration to update
345344
:type integration: Integration
346345
:returns: boolean based on webhook set up success, and requests Response object
347-
:rtype: (Bool, Response)
348346
"""
349347
log.bind(project_slug=project.slug)
350348
provider_data = self.get_provider_data(project, integration)
@@ -370,7 +368,7 @@ def update_webhook(self, project, integration):
370368
integration.provider_data = recv_data
371369
integration.save()
372370
log.info("Bitbucket webhook update successful for project.")
373-
return (True, resp)
371+
return True
374372

375373
# Bitbucket returns 404 when the webhook doesn't exist. In this
376374
# case, we call ``setup_webhook`` to re-configure it from scratch
@@ -391,4 +389,4 @@ def update_webhook(self, project, integration):
391389
except (KeyError, RequestException, TypeError, ValueError):
392390
log.exception("Bitbucket webhook update failed for project.")
393391

394-
return (False, resp)
392+
return False

‎readthedocs/oauth/services/github.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ def get_provider_data(self, project, integration):
279279

280280
return integration.provider_data
281281

282-
def setup_webhook(self, project, integration=None):
282+
def setup_webhook(self, project, integration=None) -> bool:
283283
"""
284284
Set up GitHub project webhook for project.
285285
@@ -288,7 +288,6 @@ def setup_webhook(self, project, integration=None):
288288
:param integration: Integration for the project
289289
:type integration: Integration
290290
:returns: boolean based on webhook set up success, and requests Response object
291-
:rtype: (Bool, Response)
292291
"""
293292
owner, repo = build_utils.get_github_username_repo(url=project.repo)
294293

@@ -320,7 +319,7 @@ def setup_webhook(self, project, integration=None):
320319
integration.provider_data = recv_data
321320
integration.save()
322321
log.debug("GitHub webhook creation successful for project.")
323-
return (True, resp)
322+
return True
324323

325324
if resp.status_code in [401, 403, 404]:
326325
log.warning("GitHub project does not exist or user does not have permissions.")
@@ -339,9 +338,9 @@ def setup_webhook(self, project, integration=None):
339338
except (RequestException, ValueError):
340339
log.exception("GitHub webhook creation failed for project.")
341340

342-
return (False, resp)
341+
return False
343342

344-
def update_webhook(self, project, integration):
343+
def update_webhook(self, project, integration) -> bool:
345344
"""
346345
Update webhook integration.
347346
@@ -350,7 +349,6 @@ def update_webhook(self, project, integration):
350349
:param integration: Webhook integration to update
351350
:type integration: Integration
352351
:returns: boolean based on webhook update success, and requests Response object
353-
:rtype: (Bool, Response)
354352
"""
355353
data = self.get_webhook_data(project, integration)
356354
resp = None
@@ -384,7 +382,7 @@ def update_webhook(self, project, integration):
384382
integration.provider_data = recv_data
385383
integration.save()
386384
log.info("GitHub webhook update successful for project.")
387-
return (True, resp)
385+
return True
388386

389387
# GitHub returns 404 when the webhook doesn't exist. In this case,
390388
# we call ``setup_webhook`` to re-configure it from scratch
@@ -405,7 +403,7 @@ def update_webhook(self, project, integration):
405403
except (AttributeError, RequestException, ValueError):
406404
log.exception("GitHub webhook update failed for project.")
407405

408-
return (False, resp)
406+
return False
409407

410408
def remove_webhook(self, project):
411409
"""

‎readthedocs/oauth/services/githubapp.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
class GitHubAppService(Service):
3434
vcs_provider_slug = GITHUB_APP
3535
allauth_provider = GitHubAppProvider
36+
supports_build_status = True
3637

3738
def __init__(self, installation: GitHubAppInstallation):
3839
self.installation = installation
@@ -481,10 +482,10 @@ def get_clone_token(self, project):
481482
)
482483
return None
483484

484-
def setup_webhook(self, project, integration=None):
485+
def setup_webhook(self, project, integration=None) -> bool:
485486
"""When using a GitHub App, we don't need to set up a webhook."""
486487
return True
487488

488-
def update_webhook(self, project, integration=None):
489+
def update_webhook(self, project, integration=None) -> bool:
489490
"""When using a GitHub App, we don't need to set up a webhook."""
490491
return True

‎readthedocs/oauth/services/gitlab.py

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ def get_provider_data(self, project, integration):
361361

362362
return integration.provider_data
363363

364-
def setup_webhook(self, project, integration=None):
364+
def setup_webhook(self, project, integration=None) -> bool:
365365
"""
366366
Set up GitLab project webhook for project.
367367
@@ -384,7 +384,7 @@ def setup_webhook(self, project, integration=None):
384384
url = f"{self.base_api_url}/api/v4/projects/{repo_id}/hooks"
385385

386386
if repo_id is None:
387-
return (False, resp)
387+
return False
388388

389389
log.bind(
390390
project_slug=project.slug,
@@ -404,7 +404,7 @@ def setup_webhook(self, project, integration=None):
404404
integration.provider_data = resp.json()
405405
integration.save()
406406
log.debug("GitLab webhook creation successful for project.")
407-
return (True, resp)
407+
return True
408408

409409
if resp.status_code in [401, 403, 404]:
410410
log.info("Gitlab project does not exist or user does not have permissions.")
@@ -414,9 +414,9 @@ def setup_webhook(self, project, integration=None):
414414
except Exception:
415415
log.exception("GitLab webhook creation failed.")
416416

417-
return (False, resp)
417+
return False
418418

419-
def update_webhook(self, project, integration):
419+
def update_webhook(self, project, integration) -> bool:
420420
"""
421421
Update webhook integration.
422422
@@ -428,8 +428,6 @@ def update_webhook(self, project, integration):
428428
429429
:returns: boolean based on webhook update success, and requests Response
430430
object
431-
432-
:rtype: (Bool, Response)
433431
"""
434432
provider_data = self.get_provider_data(project, integration)
435433

@@ -442,7 +440,7 @@ def update_webhook(self, project, integration):
442440
repo_id = self._get_repo_id(project)
443441

444442
if repo_id is None:
445-
return (False, resp)
443+
return False
446444

447445
data = self.get_webhook_data(repo_id, project, integration)
448446

@@ -467,7 +465,7 @@ def update_webhook(self, project, integration):
467465
integration.provider_data = recv_data
468466
integration.save()
469467
log.info("GitLab webhook update successful for project.")
470-
return (True, resp)
468+
return True
471469

472470
# GitLab returns 404 when the webhook doesn't exist. In this case,
473471
# we call ``setup_webhook`` to re-configure it from scratch
@@ -486,7 +484,7 @@ def update_webhook(self, project, integration):
486484
debug_data=debug_data,
487485
)
488486

489-
return (False, resp)
487+
return False
490488

491489
def send_build_status(self, *, build, commit, status):
492490
"""

‎readthedocs/oauth/tasks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ def attach_webhook(project_pk, user_pk=None, integration=None, **kwargs):
210210
return False
211211

212212
for service in services:
213-
success, _ = service.setup_webhook(project, integration=integration)
213+
success = service.setup_webhook(project, integration=integration)
214214
if success:
215215
project.has_valid_webhook = True
216216
project.save()

0 commit comments

Comments
 (0)