Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to import public repositories on corporate site #3537

Merged
merged 4 commits into from
Jan 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions readthedocs/oauth/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@

from .querysets import RemoteOrganizationQuerySet, RemoteRepositoryQuerySet

DEFAULT_PRIVACY_LEVEL = getattr(settings, 'DEFAULT_PRIVACY_LEVEL', 'public')


@python_2_unicode_compatible
class RemoteOrganization(models.Model):
Expand Down
5 changes: 1 addition & 4 deletions readthedocs/oauth/services/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
from requests.exceptions import RequestException
from requests_oauthlib import OAuth2Session

DEFAULT_PRIVACY_LEVEL = getattr(settings, 'DEFAULT_PRIVACY_LEVEL', 'public')

log = logging.getLogger(__name__)


Expand Down Expand Up @@ -158,8 +156,7 @@ def paginate(self, url, **kwargs):
def sync(self):
raise NotImplementedError

def create_repository(
self, fields, privacy=DEFAULT_PRIVACY_LEVEL, organization=None):
def create_repository(self, fields, privacy=None, organization=None):
raise NotImplementedError

def create_organization(self, fields):
Expand Down
12 changes: 6 additions & 6 deletions readthedocs/oauth/services/bitbucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
from .base import Service


DEFAULT_PRIVACY_LEVEL = getattr(settings, 'DEFAULT_PRIVACY_LEVEL', 'public')

log = logging.getLogger(__name__)


Expand Down Expand Up @@ -87,8 +85,7 @@ def sync_teams(self):
raise Exception('Could not sync your Bitbucket team repositories, '
'try reconnecting your account')

def create_repository(self, fields, privacy=DEFAULT_PRIVACY_LEVEL,
organization=None):
def create_repository(self, fields, privacy=None, organization=None):
"""
Update or create a repository from Bitbucket API response.

Expand All @@ -103,8 +100,11 @@ def create_repository(self, fields, privacy=DEFAULT_PRIVACY_LEVEL,
:type organization: RemoteOrganization
:rtype: RemoteRepository
"""
if (fields['is_private'] is True and privacy == 'private' or
fields['is_private'] is False and privacy == 'public'):
privacy = privacy or settings.DEFAULT_PRIVACY_LEVEL
if (
(privacy == 'private') or
(fields['is_private'] is False and privacy == 'public')
):
repo, _ = RemoteRepository.objects.get_or_create(
full_name=fields['full_name'],
account=self.account,
Expand Down
10 changes: 4 additions & 6 deletions readthedocs/oauth/services/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@
from ..models import RemoteOrganization, RemoteRepository
from .base import Service


DEFAULT_PRIVACY_LEVEL = getattr(settings, 'DEFAULT_PRIVACY_LEVEL', 'public')

log = logging.getLogger(__name__)


Expand Down Expand Up @@ -70,8 +67,7 @@ def sync_organizations(self):
raise Exception('Could not sync your GitHub organizations, '
'try reconnecting your account')

def create_repository(self, fields, privacy=DEFAULT_PRIVACY_LEVEL,
organization=None):
def create_repository(self, fields, privacy=None, organization=None):
"""
Update or create a repository from GitHub API response.

Expand All @@ -81,9 +77,11 @@ def create_repository(self, fields, privacy=DEFAULT_PRIVACY_LEVEL,
:type organization: RemoteOrganization
:rtype: RemoteRepository
"""
privacy = privacy or settings.DEFAULT_PRIVACY_LEVEL
if (
(privacy == 'private') or
(fields['private'] is False and privacy == 'public')):
(fields['private'] is False and privacy == 'public')
):
try:
repo = RemoteRepository.objects.get(
full_name=fields['full_name'],
Expand Down
6 changes: 3 additions & 3 deletions readthedocs/oauth/services/gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from readthedocs.integrations.models import Integration

from ..models import RemoteOrganization, RemoteRepository
from .base import DEFAULT_PRIVACY_LEVEL, Service
from .base import Service

try:
from urlparse import urljoin, urlparse
Expand Down Expand Up @@ -108,8 +108,7 @@ def sync_organizations(self):
def is_owned_by(self, owner_id):
return self.account.extra_data['id'] == owner_id

def create_repository(
self, fields, privacy=DEFAULT_PRIVACY_LEVEL, organization=None):
def create_repository(self, fields, privacy=None, organization=None):
"""
Update or create a repository from GitLab API response.

Expand All @@ -119,6 +118,7 @@ def create_repository(
:type organization: RemoteOrganization
:rtype: RemoteRepository
"""
privacy = privacy or settings.DEFAULT_PRIVACY_LEVEL
repo_is_public = fields['visibility'] == 'public'
if privacy == 'private' or (repo_is_public and privacy == 'public'):
try:
Expand Down
41 changes: 41 additions & 0 deletions readthedocs/rtd_tests/tests/test_oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.conf import settings
from django.contrib.auth.models import User
from django.test import TestCase
from django.test.utils import override_settings

from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
from readthedocs.oauth.services import (
Expand Down Expand Up @@ -131,6 +132,24 @@ def test_multiple_users_same_repo(self):
self.assertEqual(github_project, github_project_5)
self.assertEqual(github_project_2, github_project_6)

@override_settings(DEFAULT_PRIVACY_LEVEL='private')
def test_make_private_project(self):
"""
Test ability to import ``public`` repositories under ``private`` level.
"""
repo_json = {
'name': 'testrepo',
'full_name': 'testuser/testrepo',
'description': 'Test Repo',
'git_url': 'git://github.com/testuser/testrepo.git',
'private': False,
'ssh_url': 'ssh://[email protected]:testuser/testrepo.git',
'html_url': 'https://github.com/testuser/testrepo',
'clone_url': 'https://github.com/testuser/testrepo.git',
}
repo = self.service.create_repository(repo_json, organization=self.org)
self.assertIsNotNone(repo)


class BitbucketOAuthTests(TestCase):

Expand Down Expand Up @@ -270,6 +289,16 @@ def test_make_project_fail(self):
data, organization=self.org, privacy=self.privacy)
self.assertIsNone(repo)

@override_settings(DEFAULT_PRIVACY_LEVEL='private')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most important part of the test.

The problem we have here is how are the method defined and how they use this setting:

The test passes because it's a public repo under a public default privacy level.

What it should be the best way to handle this? Should we make privacy as None by default and access the settings object inside the create_repository method?

cc @agjohnson

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this probably makes the most sense. 👍

def test_make_private_project(self):
"""
Test ability to import ``public`` repositories under ``private`` level.
"""
data = self.repo_response_data.copy()
data['is_private'] = False
repo = self.service.create_repository(data, organization=self.org)
self.assertIsNotNone(repo)

def test_make_organization(self):
org = self.service.create_organization(self.team_response_data)
self.assertIsInstance(org, RemoteOrganization)
Expand Down Expand Up @@ -444,3 +473,15 @@ def test_make_organization(self):
'https://secure.gravatar.com/avatar/test',
)
self.assertEqual(org.url, 'https://gitlab.com/testorga')

@override_settings(DEFAULT_PRIVACY_LEVEL='private')
def test_make_private_project(self):
"""
Test ability to import ``public`` repositories under ``private`` level.
"""
data = self.repo_response_data.copy()
data['visibility'] = 'public'
with mock.patch('readthedocs.oauth.services.gitlab.GitLabService.is_owned_by') as m: # yapf: disable
m.return_value = True
repo = self.service.create_repository(data, organization=self.org)
self.assertIsNotNone(repo)