Merge pull request #7430 from readthedocs/humitos/sync-orgs-github

humitos · web-flow · commit c3eee5daa4dd · 2020-09-08T11:08:39.000+02:00
diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py
@@ -194,12 +194,13 @@ def sync(self):
         - deletes old RemoteRepository/Organization that are not present
           for this user in the current provider
         """
-        repos = self.sync_repositories()
-        organizations, organization_repos = self.sync_organizations()
+        remote_repositories = self.sync_repositories()
+        remote_organizations, remote_repositories_organizations = self.sync_organizations()
 
         # Delete RemoteRepository where the user doesn't have access anymore
         # (skip RemoteRepository tied to a Project on this user)
-        repository_full_names = self.get_repository_full_names(repos + organization_repos)
+        all_remote_repositories = remote_repositories + remote_repositories_organizations
+        repository_full_names = [r.full_name for r in all_remote_repositories if r is not None]
         (
             self.user.oauth_repositories
             .exclude(
@@ -210,10 +211,10 @@ def sync(self):
         )
 
         # Delete RemoteOrganization where the user doesn't have access anymore
-        organization_names = self.get_organization_names(organizations)
+        organization_slugs = [o.slug for o in remote_organizations if o is not None]
         (
             self.user.oauth_organizations
-            .exclude(name__in=organization_names)
+            .exclude(slug__in=organization_slugs)
             .filter(account=self.account)
             .delete()
         )
diff --git a/readthedocs/oauth/services/bitbucket.py b/readthedocs/oauth/services/bitbucket.py
@@ -32,15 +32,16 @@ class BitbucketService(Service):
 
     def sync_repositories(self):
         """Sync repositories from Bitbucket API."""
-        repos = []
+        remote_repositories = []
 
         # Get user repos
         try:
             repos = self.paginate(
                 'https://bitbucket.org/api/2.0/repositories/?role=member',
             )
             for repo in repos:
-                self.create_repository(repo)
+                remote_repository = self.create_repository(repo)
+                remote_repositories.append(remote_repository)
 
         except (TypeError, ValueError):
             log.warning('Error syncing Bitbucket repositories')
@@ -69,26 +70,29 @@ def sync_repositories(self):
         except (TypeError, ValueError):
             pass
 
-        return repos
+        return remote_repositories
 
     def sync_organizations(self):
         """Sync Bitbucket teams (our RemoteOrganization) and team repositories."""
-        teams = []
-        repositories = []
+        remote_organizations = []
+        remote_repositories = []
 
         try:
             teams = self.paginate(
                 'https://api.bitbucket.org/2.0/teams/?role=member',
             )
             for team in teams:
-                org = self.create_organization(team)
+                remote_organization = self.create_organization(team)
                 repos = self.paginate(team['links']['repositories']['href'])
 
-                # Add organization's repositories to the result
-                repositories.extend(repos)
+                remote_organizations.append(remote_organization)
 
                 for repo in repos:
-                    self.create_repository(repo, organization=org)
+                    remote_repository = self.create_repository(
+                        repo,
+                        organization=remote_organization,
+                    )
+                    remote_repositories.append(remote_repository)
 
         except ValueError:
             log.warning('Error syncing Bitbucket organizations')
@@ -97,7 +101,7 @@ def sync_organizations(self):
                 'try reconnecting your account',
             )
 
-        return teams, repositories
+        return remote_organizations, remote_repositories
 
     def create_repository(self, fields, privacy=None, organization=None):
         """
@@ -190,12 +194,6 @@ def create_organization(self, fields):
         organization.save()
         return organization
 
-    def get_repository_full_names(self, repositories):
-        return {repository.get('full_name') for repository in repositories}
-
-    def get_organization_names(self, organizations):
-        return {organization.get('display_name') for organization in organizations}
-
     def get_next_url_to_paginate(self, response):
         return response.json().get('next')
 
diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
@@ -36,41 +36,45 @@ class GitHubService(Service):
 
     def sync_repositories(self):
         """Sync repositories from GitHub API."""
-        repos = []
+        remote_repositories = []
 
         try:
             repos = self.paginate('https://api.github.com/user/repos?per_page=100')
             for repo in repos:
-                self.create_repository(repo)
+                remote_repository = self.create_repository(repo)
+                remote_repositories.append(remote_repository)
         except (TypeError, ValueError):
             log.warning('Error syncing GitHub repositories')
             raise SyncServiceError(
                 'Could not sync your GitHub repositories, '
                 'try reconnecting your account'
             )
-        return repos
+        return remote_repositories
 
     def sync_organizations(self):
         """Sync organizations from GitHub API."""
-        orgs = []
-        repositories = []
+        remote_organizations = []
+        remote_repositories = []
 
         try:
             orgs = self.paginate('https://api.github.com/user/orgs')
             for org in orgs:
-                org_resp = self.get_session().get(org['url'])
-                org_obj = self.create_organization(org_resp.json())
+                org_details = self.get_session().get(org['url']).json()
+                remote_organization = self.create_organization(org_details)
                 # Add repos
                 # TODO ?per_page=100
                 org_repos = self.paginate(
                     '{org_url}/repos'.format(org_url=org['url']),
                 )
 
-                # Add all the repositories for this organization to the result
-                repositories.extend(org_repos)
+                remote_organizations.append(remote_organization)
 
                 for repo in org_repos:
-                    self.create_repository(repo, organization=org_obj)
+                    remote_repository = self.create_repository(
+                        repo,
+                        organization=remote_organization,
+                    )
+                    remote_repositories.append(remote_repository)
 
         except (TypeError, ValueError):
             log.warning('Error syncing GitHub organizations')
@@ -79,7 +83,7 @@ def sync_organizations(self):
                 'try reconnecting your account'
             )
 
-        return orgs, repositories
+        return remote_organizations, remote_repositories
 
     def create_repository(self, fields, privacy=None, organization=None):
         """
@@ -170,12 +174,6 @@ def create_organization(self, fields):
         organization.save()
         return organization
 
-    def get_repository_full_names(self, repositories):
-        return {repository.get('full_name') for repository in repositories}
-
-    def get_organization_names(self, organizations):
-        return {organization.get('name') for organization in organizations}
-
     def get_next_url_to_paginate(self, response):
         return response.links.get('next', {}).get('url')
 
diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py
@@ -67,7 +67,7 @@ def get_paginated_results(self, response):
         return response.json()
 
     def sync_repositories(self):
-        repos = []
+        remote_repositories = []
         try:
             repos = self.paginate(
                 '{url}/api/v4/projects'.format(url=self.adapter.provider_base_url),
@@ -79,19 +79,20 @@ def sync_repositories(self):
             )
 
             for repo in repos:
-                self.create_repository(repo)
+                remote_repository = self.create_repository(repo)
+                remote_repositories.append(remote_repository)
         except (TypeError, ValueError):
             log.warning('Error syncing GitLab repositories')
             raise SyncServiceError(
                 'Could not sync your GitLab repositories, '
                 'try reconnecting your account'
             )
 
-        return repos
+        return remote_repositories
 
     def sync_organizations(self):
-        orgs = []
-        repositories = []
+        remote_organizations = []
+        remote_repositories = []
 
         try:
             orgs = self.paginate(
@@ -102,7 +103,7 @@ def sync_organizations(self):
                 sort='asc',
             )
             for org in orgs:
-                org_obj = self.create_organization(org)
+                remote_organization = self.create_organization(org)
                 org_repos = self.paginate(
                     '{url}/api/v4/groups/{id}/projects'.format(
                         url=self.adapter.provider_base_url,
@@ -114,19 +115,22 @@ def sync_organizations(self):
                     sort='asc',
                 )
 
-                # Add organization's repositories to the result
-                repositories.extend(org_repos)
+                remote_organizations.append(remote_organization)
 
                 for repo in org_repos:
-                    self.create_repository(repo, organization=org_obj)
+                    remote_repository = self.create_repository(
+                        repo,
+                        organization=remote_organization,
+                    )
+                    remote_repositories.append(remote_repository)
         except (TypeError, ValueError):
             log.warning('Error syncing GitLab organizations')
             raise SyncServiceError(
                 'Could not sync your GitLab organization, '
                 'try reconnecting your account'
             )
 
-        return orgs, repositories
+        return remote_organizations, remote_repositories
 
     def is_owned_by(self, owner_id):
         return self.account.extra_data['id'] == owner_id
@@ -232,12 +236,6 @@ def create_organization(self, fields):
         organization.save()
         return organization
 
-    def get_repository_full_names(self, repositories):
-        return {repository.get('name_with_namespace') for repository in repositories}
-
-    def get_organization_names(self, organizations):
-        return {organization.get('name') for organization in organizations}
-
     def get_webhook_data(self, repo_id, project, integration):
         """
         Get webhook JSON data to post to the API.
diff --git a/readthedocs/rtd_tests/tests/test_oauth_sync.py b/readthedocs/rtd_tests/tests/test_oauth_sync.py
diff --git a/requirements/testing.txt b/requirements/testing.txt
