diff --git a/readthedocs/builds/tasks.py b/readthedocs/builds/tasks.py index fc076af9118..211bcdf436c 100644 --- a/readthedocs/builds/tasks.py +++ b/readthedocs/builds/tasks.py @@ -30,12 +30,10 @@ ) from readthedocs.builds.models import Build, Version from readthedocs.builds.utils import memcache_lock -from readthedocs.core.permissions import AdminPermission from readthedocs.core.utils import send_email, trigger_build from readthedocs.integrations.models import HttpExchange from readthedocs.notifications.models import Notification from readthedocs.oauth.notifications import MESSAGE_OAUTH_BUILD_STATUS_FAILURE -from readthedocs.projects.constants import GITHUB_BRAND, GITLAB_BRAND from readthedocs.projects.models import Project, WebHookEvent from readthedocs.storage import build_commands_storage from readthedocs.worker import app @@ -385,24 +383,16 @@ def sync_versions_task(project_pk, tags_data, branches_data, **kwargs): @app.task(max_retries=3, default_retry_delay=60, queue="web") def send_build_status(build_pk, commit, status): """ - Send Build Status to Git Status API for project external versions. - - It tries using these services' account in order: - - 1. user's account that imported the project - 2. each user's account from the project's maintainers + Send build status to GitHub/GitLab for a given build/commit. :param build_pk: Build primary key :param commit: commit sha of the pull/merge request :param status: build status failed, pending, or success to be sent. """ - # TODO: Send build status for Bitbucket. build = Build.objects.filter(pk=build_pk).first() if not build: return - provider_name = build.project.git_provider_name - log.bind( build_id=build.pk, project_slug=build.project.slug, @@ -412,76 +402,42 @@ def send_build_status(build_pk, commit, status): log.debug("Sending build status.") - if provider_name in [GITHUB_BRAND, GITLAB_BRAND]: - # get the service class for the project e.g: GitHubService. - service_class = build.project.git_service_class() - users = AdminPermission.admins(build.project) - - if build.project.remote_repository: - remote_repository = build.project.remote_repository - remote_repository_relations = ( - remote_repository.remote_repository_relations.filter( - account__isnull=False, - # Use ``user_in=`` instead of ``user__projects=`` here - # because User's are not related to Project's directly in - # Read the Docs for Business - user__in=AdminPermission.members(build.project), - ) - .select_related("account", "user") - .only("user", "account") - ) + # Get the service class for the project e.g: GitHubService. + # We fallback to guess the service from the repo, + # in the future we should only consider projects that have a remote repository. + service_class = build.project.get_git_service_class(fallback_to_clone_url=True) + if not service_class: + log.info("Project isn't connected to a Git service, not sending build status.") + return False - # Try using any of the users' maintainer accounts - # Try to loop through all remote repository relations for the projects users - for relation in remote_repository_relations: - service = service_class(relation.user, relation.account) - # Send status report using the API. - success = service.send_build_status( - build, - commit, - status, - ) + if not service_class.supports_build_status: + log.info("Git service doesn't support build status.") + return False - if success: - log.debug( - "Build status report sent correctly.", - user_username=relation.user.username, - ) - return True - else: - log.warning("Project does not have a RemoteRepository.") - # Try to send build status for projects with no RemoteRepository - for user in users: - services = service_class.for_user(user) - # Try to loop through services for users all social accounts - # to send successful build status - for service in services: - success = service.send_build_status( - build, - commit, - status, - ) - if success: - log.debug( - "Build status report sent correctly using an user account.", - user_username=user.username, - ) - return True - - # NOTE: this notifications was attached to every user. - # Now, I'm attaching it to the project itself since it's a problem at project level. - Notification.objects.add( - message_id=MESSAGE_OAUTH_BUILD_STATUS_FAILURE, - attached_to=build.project, - format_values={ - "provider_name": provider_name, - "url_connect_account": reverse("socialaccount_connections"), - }, - dismissable=True, + for service in service_class.for_project(build.project): + success = service.send_build_status( + build, + commit, + status, ) + if success: + log.debug("Build status report sent correctly.") + return True + + Notification.objects.add( + message_id=MESSAGE_OAUTH_BUILD_STATUS_FAILURE, + attached_to=build.project, + format_values={ + "provider_name": service_class.provider_name, + "url_connect_account": reverse("socialaccount_connections"), + }, + dismissable=True, + ) - log.info("No social account or repository permission available.") - return False + log.info( + "No social account or repository permission available, no build status sent." + ) + return False @app.task(queue="web") diff --git a/readthedocs/oauth/models.py b/readthedocs/oauth/models.py index 4810943d046..b236fb5737f 100644 --- a/readthedocs/oauth/models.py +++ b/readthedocs/oauth/models.py @@ -1,5 +1,5 @@ """OAuth service models.""" - +import structlog from allauth.socialaccount.models import SocialAccount from django.contrib.auth.models import User from django.core.validators import URLValidator @@ -14,6 +14,8 @@ from .constants import VCS_PROVIDER_CHOICES from .querysets import RemoteOrganizationQuerySet, RemoteRepositoryQuerySet +log = structlog.get_logger(__name__) + class RemoteOrganization(TimeStampedModel): @@ -224,6 +226,19 @@ def get_remote_repository_relation(self, user, social_account): ) return remote_repository_relation + def get_service_class(self): + from readthedocs.oauth.services import registry + + for service_cls in registry: + if service_cls.vcs_provider_slug == self.vcs_provider: + return service_cls + + # NOTE: this should never happen, but we log it just in case + log.exception( + "Service not found for the VCS provider", vcs_provider=self.vcs_provider + ) + return None + class RemoteRepositoryRelation(TimeStampedModel): remote_repository = models.ForeignKey( diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py index 63c5007859b..b2c30a78f62 100644 --- a/readthedocs/oauth/services/base.py +++ b/readthedocs/oauth/services/base.py @@ -1,10 +1,9 @@ """OAuth utility functions.""" - +import re from datetime import datetime import structlog from allauth.socialaccount.models import SocialAccount -from allauth.socialaccount.providers import registry from allauth.socialaccount.providers.oauth2.views import OAuth2Adapter from django.conf import settings from django.urls import reverse @@ -14,6 +13,8 @@ from requests.exceptions import RequestException from requests_oauthlib import OAuth2Session +from readthedocs.core.permissions import AdminPermission + log = structlog.get_logger(__name__) @@ -29,19 +30,104 @@ class SyncServiceError(Exception): class Service: + """Base class for service that interacts with a VCS provider and a project.""" + + vcs_provider_slug: str + url_pattern: re.Pattern | None + provider_name: str + default_user_avatar_url = settings.OAUTH_AVATAR_USER_DEFAULT_URL + default_org_avatar_url = settings.OAUTH_AVATAR_ORG_DEFAULT_URL + supports_build_status = False + + @classmethod + def for_project(self, project): + """Return an iterator of services that can be used for the project.""" + raise NotImplementedError + + @classmethod + def for_user(self, user): + """Return an iterator of services that belong to the user.""" + raise NotImplementedError + + def sync(self): + """ + Sync remote repositories and organizations. + + - Creates a new RemoteRepository/Organization per new repository + - Updates fields for existing RemoteRepository/Organization + - Deletes old RemoteRepository/Organization that are no longer present + in this provider. + """ + raise NotImplementedError + + 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) + """ + raise NotImplementedError + + def update_webhook(self, project, integration): + """ + Update webhook integration. + + :param project: project to set up webhook for + :type project: Project + :param integration: Webhook integration to update + :type integration: Integration + :returns: boolean based on webhook update success, and requests Response object + :rtype: (Bool, Response) + """ + raise NotImplementedError + + def send_build_status(self, build, commit, status): + """ + Create commit status for project. + + :param build: Build to set up commit status for + :type build: Build + :param commit: commit sha of the pull/merge request + :type commit: str + :param status: build state failure, pending, or success. + :type status: str + :returns: boolean based on commit status creation was successful or not. + :rtype: Bool + """ + raise NotImplementedError + + @classmethod + def is_project_service(cls, project): + """ + Determine if this is the service the project is using. + + .. note:: + + This should be deprecated in favor of attaching the + :py:class:`RemoteRepository` to the project instance. This is a + slight improvement on the legacy check for webhooks + """ + return ( + cls.url_pattern is not None + and cls.url_pattern.search(project.repo) is not None + ) + + +class UserService(Service): + """ - Service mapping for local accounts. + Subclass of Service that interacts with a VCS provider using the user's OAuth token. :param user: User to use in token lookup and session creation :param account: :py:class:`SocialAccount` instance for user """ adapter = None - url_pattern = None - vcs_provider_slug = None - - default_user_avatar_url = settings.OAUTH_AVATAR_USER_DEFAULT_URL - default_org_avatar_url = settings.OAUTH_AVATAR_ORG_DEFAULT_URL def __init__(self, user, account): self.session = None @@ -53,17 +139,20 @@ def __init__(self, user, account): social_account_id=self.account.pk, ) + @classmethod + def for_project(cls, project): + users = AdminPermission.admins(project) + for user in users: + yield from cls.for_user(user) + @classmethod def for_user(cls, user): - """Return list of instances if user has an account for the provider.""" - try: - accounts = SocialAccount.objects.filter( - user=user, - provider=cls.adapter.provider_id, - ) - return [cls(user=user, account=account) for account in accounts] - except SocialAccount.DoesNotExist: - return [] + accounts = SocialAccount.objects.filter( + user=user, + provider=cls.adapter.provider_id, + ) + for account in accounts: + yield cls(user=user, account=account) def get_adapter(self) -> type[OAuth2Adapter]: return self.adapter @@ -72,10 +161,6 @@ def get_adapter(self) -> type[OAuth2Adapter]: def provider_id(self): return self.get_adapter().provider_id - @property - def provider_name(self): - return registry.get_class(self.provider_id).name - def get_session(self): if self.session is None: self.create_session() @@ -300,60 +385,8 @@ def get_provider_data(self, project, integration): """ raise NotImplementedError - 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) - """ - raise NotImplementedError - - def update_webhook(self, project, integration): - """ - Update webhook integration. - - :param project: project to set up webhook for - :type project: Project - :param integration: Webhook integration to update - :type integration: Integration - :returns: boolean based on webhook update success, and requests Response object - :rtype: (Bool, Response) - """ + def sync_repositories(self): raise NotImplementedError - def send_build_status(self, build, commit, status): - """ - Create commit status for project. - - :param build: Build to set up commit status for - :type build: Build - :param commit: commit sha of the pull/merge request - :type commit: str - :param status: build state failure, pending, or success. - :type status: str - :returns: boolean based on commit status creation was successful or not. - :rtype: Bool - """ + def sync_organizations(self): raise NotImplementedError - - @classmethod - def is_project_service(cls, project): - """ - Determine if this is the service the project is using. - - .. note:: - - This should be deprecated in favor of attaching the - :py:class:`RemoteRepository` to the project instance. This is a - slight improvement on the legacy check for webhooks - """ - # TODO Replace this check by keying project to remote repos - return ( - cls.url_pattern is not None - and cls.url_pattern.search(project.repo) is not None - ) diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py index 7ffab66f3e8..193c11ba03c 100644 --- a/readthedocs/oauth/services/bitbucket.py +++ b/readthedocs/oauth/services/bitbucket.py @@ -15,12 +15,12 @@ from ..constants import BITBUCKET from ..models import RemoteOrganization, RemoteRepository, RemoteRepositoryRelation -from .base import Service, SyncServiceError +from .base import SyncServiceError, UserService log = structlog.get_logger(__name__) -class BitbucketService(Service): +class BitbucketService(UserService): """Provider service for Bitbucket.""" @@ -29,6 +29,7 @@ class BitbucketService(Service): url_pattern = re.compile(r"bitbucket.org") https_url_pattern = re.compile(r"^https:\/\/[^@]+@bitbucket.org/") vcs_provider_slug = BITBUCKET + provider_name = "Bitbucket" def sync_repositories(self): """Sync repositories from Bitbucket API.""" diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index fb95492a81d..e0f22457bd7 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -15,12 +15,12 @@ from ..constants import GITHUB from ..models import RemoteOrganization, RemoteRepository -from .base import Service, SyncServiceError +from .base import SyncServiceError, UserService log = structlog.get_logger(__name__) -class GitHubService(Service): +class GitHubService(UserService): """Provider service for GitHub.""" @@ -28,6 +28,8 @@ class GitHubService(Service): # TODO replace this with a less naive check url_pattern = re.compile(r"github\.com") vcs_provider_slug = GITHUB + provider_name = "GitHub" + supports_build_status = True def sync_repositories(self): """Sync repositories from GitHub API.""" diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py index ee809f92efb..9050bcfabbb 100644 --- a/readthedocs/oauth/services/gitlab.py +++ b/readthedocs/oauth/services/gitlab.py @@ -16,12 +16,12 @@ from ..constants import GITLAB from ..models import RemoteOrganization, RemoteRepository -from .base import Service, SyncServiceError +from .base import SyncServiceError, UserService log = structlog.get_logger(__name__) -class GitLabService(Service): +class GitLabService(UserService): """ Provider service for GitLab. @@ -31,7 +31,9 @@ class GitLabService(Service): - https://docs.gitlab.com/ce/api/oauth2.html """ + provider_name = "GitLab" adapter = GitLabOAuth2Adapter + supports_build_status = True # Just use the network location to determine if it's a GitLab project # because private repos have another base url, eg. git@gitlab.example.com url_pattern = re.compile( diff --git a/readthedocs/oauth/tasks.py b/readthedocs/oauth/tasks.py index c98ba45ce80..13a6185c1f1 100644 --- a/readthedocs/oauth/tasks.py +++ b/readthedocs/oauth/tasks.py @@ -3,7 +3,6 @@ import datetime import structlog -from allauth.socialaccount.providers import registry as allauth_registry from django.contrib.auth.models import User from django.db.models.functions import ExtractIsoWeekDay from django.urls import reverse @@ -153,8 +152,9 @@ def sync_active_users_remote_repositories(): log.exception("There was a problem re-syncing RemoteRepository.") +# TODO: remove user_pk from the signature on the next release. @app.task(queue="web") -def attach_webhook(project_pk, user_pk, integration=None): +def attach_webhook(project_pk, user_pk=None, integration=None, **kwargs): """ Add post-commit hook on project import. @@ -162,86 +162,67 @@ def attach_webhook(project_pk, user_pk, integration=None): all accounts until we set up a webhook. This should remain around for legacy connections -- that is, projects that do not have a remote repository them and were not set up with a VCS provider. + + :param project_pk: Project primary key + :param integration: Integration instance. If used, this function should + be called directly, not as a task. """ project = Project.objects.filter(pk=project_pk).first() - user = User.objects.filter(pk=user_pk).first() - - if not project or not user: + if not project: return False if integration: - service = SERVICE_MAP.get(integration.integration_type) - - if not service: - log.warning("There are no registered services in the application.") - Notification.objects.add( - message_id=MESSAGE_OAUTH_WEBHOOK_INVALID, - attached_to=project, - dismissable=True, - format_values={ - "url_integrations": reverse( - "projects_integrations", - args=[project.slug], - ), - }, - ) - return None + service_class = SERVICE_MAP.get(integration.integration_type) else: - 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.") - Notification.objects.add( - message_id=MESSAGE_OAUTH_WEBHOOK_INVALID, - attached_to=project, - dismissable=True, - format_values={ - "url_integrations": reverse( - "projects_integrations", - args=[project.slug], - ), - }, - ) - return None - - provider_class = allauth_registry.get_class(service.adapter.provider_id) - - user_accounts = service.for_user(user) - for account in user_accounts: - success, __ = account.setup_webhook(project, integration=integration) - if success: - # NOTE: do we want to communicate that we connect the webhook here? - # messages.add_message(request, "Webhook successfully added.") + # Get the service class for the project e.g: GitHubService. + # We fallback to guess the service from the repo, + # in the future we should only consider projects that have a remote repository. + service_class = project.get_git_service_class(fallback_to_clone_url=True) - project.has_valid_webhook = True - project.save() - return True - - # No valid account found - if user_accounts: + if not service_class: Notification.objects.add( - message_id=MESSAGE_OAUTH_WEBHOOK_NO_PERMISSIONS, - dismissable=True, + message_id=MESSAGE_OAUTH_WEBHOOK_INVALID, attached_to=project, + dismissable=True, format_values={ - "provider_name": provider_class.name, - "url_docs_webhook": "https://docs.readthedocs.io/page/webhooks.html", + "url_integrations": reverse( + "projects_integrations", + args=[project.slug], + ), }, ) - else: + return False + + services = list(service_class.for_project(project)) + if not services: Notification.objects.add( message_id=MESSAGE_OAUTH_WEBHOOK_NO_ACCOUNT, dismissable=True, attached_to=project, format_values={ - "provider_name": provider_class.name, + "provider_name": service_class.provider_name, "url_connect_account": reverse( "projects_integrations", args=[project.slug], ), }, ) + return False + for service in services: + success, _ = service.setup_webhook(project, integration=integration) + if success: + project.has_valid_webhook = True + project.save() + return True + + Notification.objects.add( + message_id=MESSAGE_OAUTH_WEBHOOK_NO_PERMISSIONS, + dismissable=True, + attached_to=project, + format_values={ + "provider_name": service_class.provider_name, + "url_docs_webhook": "https://docs.readthedocs.io/page/webhooks.html", + }, + ) return False diff --git a/readthedocs/oauth/utils.py b/readthedocs/oauth/utils.py index 7d527e1a066..544376ac7af 100644 --- a/readthedocs/oauth/utils.py +++ b/readthedocs/oauth/utils.py @@ -21,43 +21,35 @@ def update_webhook(project, integration, request=None): # FIXME: this method supports ``request=None`` on its definition. # However, it does not work when passing ``request=None`` as # it uses that object without checking if it's ``None`` or not. - service_cls = SERVICE_MAP.get(integration.integration_type) - if service_cls is None: + service_class = SERVICE_MAP.get(integration.integration_type) + if service_class is None: return None # TODO: remove after integrations without a secret are removed. if not integration.secret: integration.save() - updated = False - if project.remote_repository: - remote_repository_relations = ( - project.remote_repository.remote_repository_relations.filter( - account__isnull=False, user=request.user - ).select_related("account") + # If the integration's service class is different from the project's + # git service class, we skip the update, as the webhook is not valid + # (we can't create a GitHub webhook for a GitLab project, for example). + if service_class != project.get_git_service_class(fallback_to_clone_url=True): + messages.error( + request, + _( + "This integration type is not compatible with the project's Git provider." + ), ) - - for relation in remote_repository_relations: - service = service_cls(request.user, relation.account) - updated, __ = service.update_webhook(project, integration) - - if updated: - break - else: - # The project was imported manually and doesn't have a RemoteRepository - # attached. We do brute force over all the accounts registered for this - # service - service_accounts = service_cls.for_user(request.user) - for service in service_accounts: - updated, __ = service.update_webhook(project, integration) - if updated: - break - - if updated: - messages.success(request, _("Webhook activated")) - project.has_valid_webhook = True + project.has_valid_webhook = False project.save() - return True + return False + + for service in service_class.for_project(project): + updated, __ = service.update_webhook(project, integration) + if updated: + messages.success(request, _("Webhook activated")) + project.has_valid_webhook = True + project.save() + return True messages.error( request, diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index a2ea85c862d..7b4a33d0cbd 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -7,7 +7,6 @@ from urllib.parse import urlparse import structlog -from allauth.socialaccount.providers import registry as allauth_registry from django.conf import settings from django.contrib.auth.models import User from django.contrib.contenttypes.fields import GenericRelation @@ -1025,28 +1024,36 @@ def vcs_class(self): """ return backend_cls.get(self.repo_type) - def git_service_class(self): - """Get the service class for project. e.g: GitHubService, GitLabService.""" + def _guess_service_class(self): from readthedocs.oauth.services import registry for service_cls in registry: if service_cls.is_project_service(self): - service = service_cls - break - else: - log.warning("There are no registered services in the application.") - service = None + return service_cls + return None - return service + def get_git_service_class(self, fallback_to_clone_url=False): + """ + Get the service class for project. e.g: GitHubService, GitLabService. + + :param fallback_to_clone_url: If the project doesn't have a remote repository, + we try to guess the service class based on the clone URL. + """ + service_cls = None + if self.has_feature(Feature.DONT_SYNC_WITH_REMOTE_REPO): + return self._guess_service_class() + service_cls = ( + self.remote_repository and self.remote_repository.get_service_class() + ) + if not service_cls and fallback_to_clone_url: + return self._guess_service_class() + return service_cls @property def git_provider_name(self): """Get the provider name for project. e.g: GitHub, GitLab, Bitbucket.""" - service = self.git_service_class() - if service: - provider_class = allauth_registry.get_class(service.adapter.provider_id) - return provider_class.name - return None + service_class = self.get_git_service_class(fallback_to_clone_url=True) + return service_class.provider_name if service_class else None def find(self, filename, version): """ diff --git a/readthedocs/projects/views/mixins.py b/readthedocs/projects/views/mixins.py index 66c0fa92623..1a112299fab 100644 --- a/readthedocs/projects/views/mixins.py +++ b/readthedocs/projects/views/mixins.py @@ -140,6 +140,8 @@ def trigger_initial_build(self, project, user): from readthedocs.oauth.tasks import attach_webhook task_promise = chain( + # TODO: Remove user_pk on the next release, + # it's used just to keep backward compatibility with the old task signature. attach_webhook.si(project.pk, user.pk), update_docs, ) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 56812ec4d3a..aeacb38c81c 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -999,8 +999,10 @@ def form_valid(self, form): if self.object.has_sync: attach_webhook( project_pk=self.get_project().pk, - user_pk=self.request.user.pk, integration=self.object, + # TODO: Remove user_pk on the next release, + # it's used just to keep backward compatibility with the old task signature. + user_pk=None, ) return HttpResponseRedirect(self.get_success_url()) @@ -1056,7 +1058,9 @@ def post(self, request, *args, **kwargs): # the per-integration sync instead. attach_webhook( project_pk=self.get_project().pk, - user_pk=request.user.pk, + # TODO: Remove user_pk on the next release, + # it's used just to keep backward compatibility with the old task signature. + user_pk=None, ) return HttpResponseRedirect(self.get_success_url()) diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py index b64e91233fe..cd0bcf7f366 100644 --- a/readthedocs/rtd_tests/tests/test_celery.py +++ b/readthedocs/rtd_tests/tests/test_celery.py @@ -10,6 +10,7 @@ from readthedocs.builds.constants import BUILD_STATUS_SUCCESS, EXTERNAL, LATEST from readthedocs.builds.models import Build, Version from readthedocs.notifications.models import Notification +from readthedocs.oauth.constants import GITHUB, GITLAB from readthedocs.oauth.models import RemoteRepository, RemoteRepositoryRelation from readthedocs.oauth.notifications import MESSAGE_OAUTH_BUILD_STATUS_FAILURE from readthedocs.projects.models import Project @@ -85,8 +86,8 @@ def test_send_build_status_with_remote_repo_github(self, send_build_status): self.project.repo = "https://github.com/test/test/" self.project.save() - social_account = get(SocialAccount, user=self.eric, provider="gitlab") - remote_repo = get(RemoteRepository) + social_account = get(SocialAccount, user=self.eric, provider="github") + remote_repo = get(RemoteRepository, vcs_provider=GITHUB) remote_repo.projects.add(self.project) get( RemoteRepositoryRelation, @@ -159,7 +160,7 @@ def test_send_build_status_with_remote_repo_gitlab(self, send_build_status): self.project.save() social_account = get(SocialAccount, user=self.eric, provider="gitlab") - remote_repo = get(RemoteRepository) + remote_repo = get(RemoteRepository, vcs_provider=GITLAB) remote_repo.projects.add(self.project) get( RemoteRepositoryRelation, diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index 9e84886e0b5..ba206f12a7a 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -254,7 +254,7 @@ def test_make_organization(self): def test_import_with_no_token(self): """User without a GitHub SocialToken does not return a service.""" - services = GitHubService.for_user(get(User)) + services = list(GitHubService.for_user(get(User))) self.assertEqual(services, []) def test_multiple_users_same_repo(self): @@ -776,7 +776,7 @@ def test_make_organization(self): def test_import_with_no_token(self): """User without a Bitbucket SocialToken does not return a service.""" - services = BitbucketService.for_user(get(User)) + services = list(BitbucketService.for_user(get(User))) self.assertEqual(services, []) @mock.patch("readthedocs.oauth.services.bitbucket.log") diff --git a/readthedocs/rtd_tests/tests/test_oauth_sync.py b/readthedocs/rtd_tests/tests/test_oauth_sync.py index 43bc308de5c..5773f6e6357 100644 --- a/readthedocs/rtd_tests/tests/test_oauth_sync.py +++ b/readthedocs/rtd_tests/tests/test_oauth_sync.py @@ -70,7 +70,7 @@ def setUp(self): SocialToken, account=self.socialaccount, ) - self.service = GitHubService.for_user(self.user)[0] + self.service = list(GitHubService.for_user(self.user))[0] @requests_mock.Mocker(kw="mock_request") def test_sync_delete_stale(self, mock_request): diff --git a/readthedocs/rtd_tests/tests/test_project.py b/readthedocs/rtd_tests/tests/test_project.py index 7a1e238f302..befa60bccb1 100644 --- a/readthedocs/rtd_tests/tests/test_project.py +++ b/readthedocs/rtd_tests/tests/test_project.py @@ -239,7 +239,10 @@ def test_git_provider_name_github(self): def test_git_service_class_github(self): self.pip.repo = "https://github.com/pypa/pip" self.pip.save() - self.assertEqual(self.pip.git_service_class(), GitHubService) + self.assertEqual(self.pip.get_git_service_class(), None) + self.assertEqual( + self.pip.get_git_service_class(fallback_to_clone_url=True), GitHubService + ) def test_git_provider_name_gitlab(self): self.pip.repo = "https://gitlab.com/pypa/pip" @@ -249,7 +252,10 @@ def test_git_provider_name_gitlab(self): def test_git_service_class_gitlab(self): self.pip.repo = "https://gitlab.com/pypa/pip" self.pip.save() - self.assertEqual(self.pip.git_service_class(), GitLabService) + self.assertEqual(self.pip.get_git_service_class(), None) + self.assertEqual( + self.pip.get_git_service_class(fallback_to_clone_url=True), GitLabService + ) @mock.patch("readthedocs.projects.forms.trigger_build", mock.MagicMock()) diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index e760c64a690..2f78690af71 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -455,8 +455,8 @@ def test_integration_create(self, attach_webhook): self.assertEqual(response.status_code, 302) attach_webhook.assert_called_once_with( project_pk=self.project.pk, - user_pk=self.user.pk, integration=integration.first(), + user_pk=None, ) @mock.patch("readthedocs.projects.views.private.attach_webhook")