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.

Sign up for GitHub

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

Jump to bottom

Allow to import public repositories on corporate site #3537

Merged
merged 4 commits into from
Jan 30, 2018
Merged

Allow to import public repositories on corporate site #3537

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f5e8987
Allow to import public repositories on corporate site
humitos Jan 22, 2018
4151207
Test for importing public repos under private default level
humitos Jan 22, 2018
7f4467c
Refactor the use of DEFAULT_PRIVACY_LEVEL to be able to override
humitos Jan 29, 2018
b258d35
Added the same test for GitLab and GitHub
humitos Jan 29, 2018
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
6 changes: 4 additions & 2 deletions readthedocs/oauth/services/bitbucket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,10 @@ 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'):
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
12 changes: 12 additions & 0 deletions readthedocs/rtd_tests/tests/test_oauth.py
View file
Open in desktop
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 @@ -270,6 +271,17 @@ 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, privacy=self.privacy)
self.assertIsNotNone(repo)

def test_make_organization(self):
org = self.service.create_organization(self.team_response_data)
self.assertIsInstance(org, RemoteOrganization)
Expand Down