Skip to content

Remote repository: Keep in sync when repos are moved to another org/users #9178

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

Merged
merged 10 commits into from
May 31, 2022
Merged

Remote repository: Keep in sync when repos are moved to another org/users #9178

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8c7c362
Remote repository: Keep in sync when repos are moved to another org/u…
stsewd May 10, 2022
eb8d042
It's a number, not a string!
stsewd May 10, 2022
bdb921f
Merge branch 'main' into keep-in-sync-remote-repos
stsewd May 25, 2022
b82c3c3
Always create an organization
stsewd May 26, 2022
99d43a1
Don't check for organization
stsewd May 26, 2022
4797d7c
Make explicit the creation of a relationship user->organization
stsewd May 26, 2022
b7ae87f
Update tests
stsewd May 26, 2022
a59150c
Update docstring
stsewd May 26, 2022
d19240e
Updates from review
stsewd May 31, 2022
bde5d43
Linter
stsewd May 31, 2022
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
21 changes: 0 additions & 21 deletions readthedocs/oauth/services/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,27 +221,6 @@ def sync(self):
.delete()
)

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

:param fields: dictionary of response data from API
:param privacy: privacy level to support
:param organization: remote organization to associate with
:type organization: RemoteOrganization
:rtype: RemoteRepository
"""
raise NotImplementedError

def create_organization(self, fields):
"""
Update or create remote organization from API response.

:param fields: dictionary response of data from API
:rtype: RemoteOrganization
"""
raise NotImplementedError

Comment on lines -224 to -244
Copy link
Member Author

Choose a reason for hiding this comment

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

the signature is different for the github provider now, we don't make any reference to these methods outside the class (we use the sync method) so we are fine having a different signature.

Copy link
Member

Choose a reason for hiding this comment

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

We should implement the same logic for the other providers as well if that's technically possible. Otherwise, we will have different behavior between GH SSO and the others.

def get_next_url_to_paginate(self, response):
"""
Return the next url to feed the `paginate` method.
Expand Down
85 changes: 42 additions & 43 deletions readthedocs/oauth/services/github.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,20 @@ def sync_organizations(self):
try:
orgs = self.paginate("https://api.github.com/user/orgs", per_page=100)
for org in orgs:
org_details = self.get_session().get(org['url']).json()
remote_organization = self.create_organization(org_details)
org_details = self.get_session().get(org["url"]).json()
remote_organization = self.create_organization(
org_details,
create_relationship=True,
)
remote_organizations.append(remote_organization)

org_url = org["url"]
org_repos = self.paginate(
f"{org_url}/repos",
per_page=100,
)

remote_organizations.append(remote_organization)

for repo in org_repos:
remote_repository = self.create_repository(
repo,
organization=remote_organization,
)
remote_repository = self.create_repository(repo)
remote_repositories.append(remote_repository)

except (TypeError, ValueError):
Expand All @@ -83,7 +82,7 @@ def sync_organizations(self):

return remote_organizations, remote_repositories

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

Expand All @@ -100,37 +99,30 @@ def create_repository(self, fields, privacy=None, organization=None):
]):

repo, _ = RemoteRepository.objects.get_or_create(
remote_id=fields['id'],
vcs_provider=self.vcs_provider_slug
)
remote_repository_relation = repo.get_remote_repository_relation(
self.user, self.account
remote_id=str(fields["id"]),
vcs_provider=self.vcs_provider_slug,
)

# It's possible that a user has access to a repository from within
# an organization without being member of that organization
# (external contributor). In this case, the repository will be
# listed under the ``/repos`` endpoint but not under ``/orgs``
# endpoint. Then, when calling this method (``create_repository``)
# we will have ``organization=None`` but we don't have to skip the
# creation of the ``RemoteRepositoryRelation``.
if repo.organization and organization and repo.organization != organization:
log.debug(
'Not importing repository because mismatched orgs.',
repository=fields['name'],
owner_type = fields["owner"]["type"]
organization = None
if owner_type == "Organization":
# NOTE: We shouldn't create a remote relationship with the current user here,
# since the user can have access to the repository, but not to the organization.
organization = self.create_organization(
fields=fields["owner"],
create_relationship=False,
)
return None

if any([
# There is an organization associated with this repository:
# attach the organization to the repository
organization is not None,
# There is no organization and the repository belongs to a
# user: removes the organization linked to the repository
not organization and fields['owner']['type'] == 'User',
]):

# If there is an organization associated with this repository,
# attach the organization to the repository.
if organization and owner_type == "Organization":
repo.organization = organization

# If the repository belongs to a user,
# remove the organization linked to the repository.
if owner_type == "User":
repo.organization = None

repo.name = fields['name']
repo.full_name = fields['full_name']
repo.description = fields['description']
Expand All @@ -151,7 +143,12 @@ def create_repository(self, fields, privacy=None, organization=None):

repo.save()

remote_repository_relation.admin = fields.get('permissions', {}).get('admin', False)
remote_repository_relation = repo.get_remote_repository_relation(
self.user, self.account
)
remote_repository_relation.admin = fields.get("permissions", {}).get(
"admin", False
)
remote_repository_relation.save()

return repo
Expand All @@ -161,20 +158,22 @@ def create_repository(self, fields, privacy=None, organization=None):
repository=fields['name'],
)

def create_organization(self, fields):
def create_organization(self, fields, create_relationship=False):
"""
Update or create remote organization from GitHub API response.

:param fields: dictionary response of data from API
:param bool create_relationship: If a remote relationship should be created for the
organization and the current user, if `False`, only the RemoteOrganization object
will be created/updated.
:rtype: RemoteOrganization
"""
organization, _ = RemoteOrganization.objects.get_or_create(
remote_id=fields['id'],
vcs_provider=self.vcs_provider_slug
)
organization.get_remote_organization_relation(
self.user, self.account
remote_id=str(fields["id"]),
vcs_provider=self.vcs_provider_slug,
)
if create_relationship:
organization.get_remote_organization_relation(self.user, self.account)

organization.url = fields.get('html_url')
# fields['login'] contains GitHub Organization slug
Expand Down
Loading