From 8c7c362db82fecc2d36f6057884f2ebdd1120e43 Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@protonmail.com>
Date: Mon, 9 May 2022 19:03:56 -0500
Subject: [PATCH 1/9] Remote repository: Keep in sync when repos are moved to
 another org/user.

We were doing some checks to avoid removing the remote organization
from a remote repo when this is listed from the /users/repos/ endpoint
or changing the organization when the project was moved.

But, since we are hitting the GitHub API directly,
we can always trust the result from there,
which includes the type of owner of the repository,
we can use that to decide if the repository belongs to an organization or not.

Fixes https://github.com/readthedocs/readthedocs.org/issues/8715
---
 readthedocs/oauth/services/github.py          |  46 ++--
 readthedocs/rtd_tests/tests/test_oauth.py     | 221 +++++++++++++-----
 .../rtd_tests/tests/test_oauth_sync.py        |  11 +-
 3 files changed, 186 insertions(+), 92 deletions(-)

diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
index ad070f358cb..b05e7e058a4 100644
--- a/readthedocs/oauth/services/github.py
+++ b/readthedocs/oauth/services/github.py
@@ -103,34 +103,23 @@ def create_repository(self, fields, privacy=None, organization=None):
                 remote_id=fields['id'],
                 vcs_provider=self.vcs_provider_slug
             )
-            remote_repository_relation = repo.get_remote_repository_relation(
-                self.user, self.account
-            )
 
-            # 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'],
-                )
-                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',
-            ]):
+            owner_type = fields["owner"]["type"]
+
+            # If there is an organization associated with this repository,
+            # attach the organization to the repository.
+            if (
+                organization
+                and owner_type == "Organization"
+                and organization.remote_id == fields["owner"]["id"]
+            ):
                 repo.organization = organization
 
+            # If there is no organization and the repository belongs to a user,
+            # remove the organization linked to the repository.
+            if not organization and owner_type == "User":
+                repo.organization = None
+
             repo.name = fields['name']
             repo.full_name = fields['full_name']
             repo.description = fields['description']
@@ -151,7 +140,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
diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py
index 2e246423d3a..828740b2807 100644
--- a/readthedocs/rtd_tests/tests/test_oauth.py
+++ b/readthedocs/rtd_tests/tests/test_oauth.py
@@ -13,11 +13,7 @@
 from readthedocs.integrations.models import GitHubWebhook, GitLabWebhook
 from readthedocs.oauth.constants import BITBUCKET, GITHUB, GITLAB
 from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
-from readthedocs.oauth.services import (
-    BitbucketService,
-    GitHubService,
-    GitLabService,
-)
+from readthedocs.oauth.services import BitbucketService, GitHubService, GitLabService
 from readthedocs.projects import constants
 from readthedocs.projects.models import Project
 
@@ -29,8 +25,8 @@ class GitHubOAuthTests(TestCase):
     def setUp(self):
         self.client.login(username='eric', password='test')
         self.user = User.objects.get(pk=1)
-        self.project = Project.objects.get(slug='pip')
-        self.org = RemoteOrganization.objects.create(slug='rtfd')
+        self.project = Project.objects.get(slug="pip")
+        self.org = RemoteOrganization.objects.create(slug="rtfd", remote_id=1234)
         self.privacy = settings.DEFAULT_PRIVACY_LEVEL
         self.service = GitHubService(
             user=self.user,
@@ -55,9 +51,7 @@ def setUp(self):
                 "url": "https://api.github.com/repos/test/Hello-World/hooks/12345678",
             }
         ]
-
-    def test_make_project_pass(self):
-        repo_json = {
+        self.repo_response_data = {
             'name': 'testrepo',
             'full_name': 'testuser/testrepo',
             'id': '12345678',
@@ -67,9 +61,45 @@ def test_make_project_pass(self):
             'ssh_url': 'ssh://git@github.com:testuser/testrepo.git',
             'html_url': 'https://github.com/testuser/testrepo',
             'clone_url': 'https://github.com/testuser/testrepo.git',
+            "owner": {
+                "type": "User",
+                "id": 1234,
+            },
         }
+
+    def test_create_remote_repository(self):
+        repo = self.service.create_repository(
+            self.repo_response_data,
+            privacy=self.privacy,
+        )
+        self.assertIsInstance(repo, RemoteRepository)
+        self.assertEqual(repo.name, "testrepo")
+        self.assertEqual(repo.full_name, "testuser/testrepo")
+        self.assertEqual(repo.remote_id, "12345678")
+        self.assertEqual(repo.vcs_provider, GITHUB)
+        self.assertEqual(repo.description, "Test Repo")
+        self.assertEqual(
+            repo.avatar_url,
+            settings.OAUTH_AVATAR_USER_DEFAULT_URL,
+        )
+        self.assertIn(self.user, repo.users.all())
+        self.assertEqual(repo.organization, None)
+        self.assertEqual(
+            repo.clone_url,
+            "https://github.com/testuser/testrepo.git",
+        )
+        self.assertEqual(
+            repo.ssh_url,
+            "ssh://git@github.com:testuser/testrepo.git",
+        )
+        self.assertEqual(repo.html_url, "https://github.com/testuser/testrepo")
+
+    def test_create_remote_repository_with_organization(self):
+        self.repo_response_data["owner"]["type"] = "Organization"
         repo = self.service.create_repository(
-            repo_json, organization=self.org, privacy=self.privacy,
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
         )
         self.assertIsInstance(repo, RemoteRepository)
         self.assertEqual(repo.name, 'testrepo')
@@ -91,23 +121,103 @@ def test_make_project_pass(self):
         )
         self.assertEqual(repo.html_url, 'https://github.com/testuser/testrepo')
 
-    def test_make_project_fail(self):
-        repo_json = {
-            'name': '',
-            'full_name': '',
-            'id': '',
-            'description': '',
-            'git_url': '',
-            'private': True,
-            'ssh_url': '',
-            'html_url': '',
-            'clone_url': '',
-        }
+    def test_skip_creation_remote_repository_on_private_repos(self):
+        self.repo_response_data["private"] = True
         github_project = self.service.create_repository(
-            repo_json, organization=self.org, privacy=self.privacy,
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
         )
         self.assertIsNone(github_project)
 
+    def test_project_was_moved_from_a_personal_account_to_an_organization(self):
+        github_project = self.service.create_repository(
+            self.repo_response_data,
+            privacy=self.privacy,
+        )
+        self.assertEqual(github_project.organization, None)
+
+        # Project belongs has been moved to an organization.
+        self.repo_response_data["owner"]["type"] = "Organization"
+
+        self.service.create_repository(
+            self.repo_response_data,
+            privacy=self.privacy,
+        )
+        github_project.refresh_from_db()
+        self.assertEqual(github_project.organization, None)
+
+        self.service.create_repository(
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
+        )
+        github_project.refresh_from_db()
+        self.assertEqual(github_project.organization, self.org)
+
+    def test_project_was_moved_from_an_organization_to_a_personal_account(self):
+        # Project belongs to an organization.
+        self.repo_response_data["owner"]["type"] = "Organization"
+        github_project = self.service.create_repository(
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
+        )
+        self.assertEqual(github_project.organization, self.org)
+
+        # Project belongs has been moved to a personal account.
+        self.repo_response_data["owner"]["type"] = "User"
+
+        # This operation doesn't have any effect.
+        self.service.create_repository(
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
+        )
+        github_project.refresh_from_db()
+        self.assertEqual(github_project.organization, self.org)
+
+        self.service.create_repository(
+            self.repo_response_data,
+            privacy=self.privacy,
+        )
+        github_project.refresh_from_db()
+        self.assertEqual(github_project.organization, None)
+
+    def test_project_was_moved_to_another_organization(self):
+        another_remote_organization = RemoteOrganization.objects.create(
+            slug="another", remote_id=4321
+        )
+
+        # Project belongs to an organization.
+        self.repo_response_data["owner"]["type"] = "Organization"
+        github_project = self.service.create_repository(
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
+        )
+        self.assertEqual(github_project.organization, self.org)
+
+        # Project was moved to another organization.
+        self.repo_response_data["owner"]["id"] = 4321
+
+        self.service.create_repository(
+            self.repo_response_data,
+            organization=another_remote_organization,
+            privacy=self.privacy,
+        )
+        github_project.refresh_from_db()
+        self.assertEqual(github_project.organization, another_remote_organization)
+
+        # The remote ID doesn't match, the project isn't moved to the organization.
+        self.service.create_repository(
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
+        )
+        github_project.refresh_from_db()
+        self.assertEqual(github_project.organization, another_remote_organization)
+
     def test_make_organization(self):
         org_json = {
             'id': 12345,
@@ -131,20 +241,11 @@ def test_import_with_no_token(self):
         self.assertEqual(services, [])
 
     def test_multiple_users_same_repo(self):
-        repo_json = {
-            'name': '',
-            'full_name': 'testrepo/multiple',
-            'id': '12345678',
-            'description': '',
-            'git_url': '',
-            'private': False,
-            'ssh_url': '',
-            'html_url': '',
-            'clone_url': '',
-        }
-
+        self.repo_response_data["owner"]["type"] = "Organization"
         github_project = self.service.create_repository(
-            repo_json, organization=self.org, privacy=self.privacy,
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
         )
 
         user2 = User.objects.get(pk=2)
@@ -153,17 +254,23 @@ def test_multiple_users_same_repo(self):
             account=get(SocialAccount, user=self.user)
         )
         github_project_2 = service.create_repository(
-            repo_json, organization=self.org, privacy=self.privacy,
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
         )
         self.assertIsInstance(github_project, RemoteRepository)
         self.assertIsInstance(github_project_2, RemoteRepository)
         self.assertEqual(github_project_2, github_project)
 
         github_project_3 = self.service.create_repository(
-            repo_json, organization=self.org, privacy=self.privacy,
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
         )
         github_project_4 = service.create_repository(
-            repo_json, organization=self.org, privacy=self.privacy,
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
         )
         self.assertIsInstance(github_project_3, RemoteRepository)
         self.assertIsInstance(github_project_4, RemoteRepository)
@@ -171,13 +278,19 @@ def test_multiple_users_same_repo(self):
         self.assertEqual(github_project_2, github_project_4)
 
         github_project_5 = self.service.create_repository(
-            repo_json, organization=self.org, privacy=self.privacy,
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
         )
         github_project_6 = service.create_repository(
-            repo_json, organization=self.org, privacy=self.privacy,
+            self.repo_response_data,
+            organization=self.org,
+            privacy=self.privacy,
         )
 
+        self.assertIsNotNone(github_project)
         self.assertEqual(github_project, github_project_5)
+        self.assertIsNotNone(github_project_2)
         self.assertEqual(github_project_2, github_project_6)
 
     @mock.patch('readthedocs.oauth.services.github.log')
@@ -232,23 +345,13 @@ def test_send_build_status_value_error(self, session, mock_logger):
         )
 
     @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',
-            'id': '12345678',
-            'description': 'Test Repo',
-            'git_url': 'git://github.com/testuser/testrepo.git',
-            'private': False,
-            'ssh_url': 'ssh://git@github.com: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)
+    def test_create_public_repo_when_private_projects_are_enabled(self):
+        """Test ability to import ``public`` repositories under ``private`` level."""
+        repo = self.service.create_repository(
+            self.repo_response_data, organization=self.org
+        )
+        self.assertEqual(repo.organization, self.org)
+        self.assertEqual(repo.remote_id, self.repo_response_data["id"])
 
     @mock.patch('readthedocs.oauth.services.github.log')
     @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session')
diff --git a/readthedocs/rtd_tests/tests/test_oauth_sync.py b/readthedocs/rtd_tests/tests/test_oauth_sync.py
index d0d803c571c..e85972f1dd8 100644
--- a/readthedocs/rtd_tests/tests/test_oauth_sync.py
+++ b/readthedocs/rtd_tests/tests/test_oauth_sync.py
@@ -1,8 +1,7 @@
-from allauth.socialaccount.models import SocialAccount, SocialToken
-from allauth.socialaccount.providers.github.views import GitHubOAuth2Adapter
 import django_dynamic_fixture as fixture
 import requests_mock
-
+from allauth.socialaccount.models import SocialAccount, SocialToken
+from allauth.socialaccount.providers.github.views import GitHubOAuth2Adapter
 from django.contrib.auth.models import User
 from django.test import TestCase
 
@@ -13,9 +12,7 @@
     RemoteRepository,
     RemoteRepositoryRelation,
 )
-from readthedocs.oauth.services import (
-    GitHubService,
-)
+from readthedocs.oauth.services import GitHubService
 from readthedocs.projects.models import Project
 
 
@@ -173,7 +170,7 @@ def test_sync_repositories_relation_with_organization(self, mock_request):
         """
         Sync repositories relations for a user where the RemoteRepository and RemoteOrganization already exist.
 
-        Note that ``repository.owner.type == 'Organzation'`` in the GitHub response.
+        Note that ``repository.owner.type == 'Organization'`` in the GitHub response.
         """
         self.payload_user_repos[0]['owner']['type'] = 'Organization'
         mock_request.get('https://api.github.com/user/repos', json=self.payload_user_repos)

From eb8d042f6d3e3b348a4816b5b4afc8026b9b9a19 Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@protonmail.com>
Date: Tue, 10 May 2022 15:53:47 -0500
Subject: [PATCH 2/9] It's a number, not a string!

---
 readthedocs/oauth/services/github.py      |  6 +++---
 readthedocs/rtd_tests/tests/test_oauth.py | 25 ++++++++++++-----------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
index b05e7e058a4..987af9c8021 100644
--- a/readthedocs/oauth/services/github.py
+++ b/readthedocs/oauth/services/github.py
@@ -100,8 +100,8 @@ 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_id=str(fields["id"]),
+                vcs_provider=self.vcs_provider_slug,
             )
 
             owner_type = fields["owner"]["type"]
@@ -111,7 +111,7 @@ def create_repository(self, fields, privacy=None, organization=None):
             if (
                 organization
                 and owner_type == "Organization"
-                and organization.remote_id == fields["owner"]["id"]
+                and organization.remote_id == str(fields["owner"]["id"])
             ):
                 repo.organization = organization
 
diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py
index 828740b2807..ac3d61ccb59 100644
--- a/readthedocs/rtd_tests/tests/test_oauth.py
+++ b/readthedocs/rtd_tests/tests/test_oauth.py
@@ -26,7 +26,7 @@ def setUp(self):
         self.client.login(username='eric', password='test')
         self.user = User.objects.get(pk=1)
         self.project = Project.objects.get(slug="pip")
-        self.org = RemoteOrganization.objects.create(slug="rtfd", remote_id=1234)
+        self.org = RemoteOrganization.objects.create(slug="rtfd", remote_id="1234")
         self.privacy = settings.DEFAULT_PRIVACY_LEVEL
         self.service = GitHubService(
             user=self.user,
@@ -52,15 +52,15 @@ def setUp(self):
             }
         ]
         self.repo_response_data = {
-            'name': 'testrepo',
-            'full_name': 'testuser/testrepo',
-            'id': '12345678',
-            'description': 'Test Repo',
-            'git_url': 'git://github.com/testuser/testrepo.git',
-            'private': False,
-            'ssh_url': 'ssh://git@github.com:testuser/testrepo.git',
-            'html_url': 'https://github.com/testuser/testrepo',
-            'clone_url': 'https://github.com/testuser/testrepo.git',
+            "name": "testrepo",
+            "full_name": "testuser/testrepo",
+            "id": 12345678,
+            "description": "Test Repo",
+            "git_url": "git://github.com/testuser/testrepo.git",
+            "private": False,
+            "ssh_url": "ssh://git@github.com:testuser/testrepo.git",
+            "html_url": "https://github.com/testuser/testrepo",
+            "clone_url": "https://github.com/testuser/testrepo.git",
             "owner": {
                 "type": "User",
                 "id": 1234,
@@ -186,7 +186,7 @@ def test_project_was_moved_from_an_organization_to_a_personal_account(self):
 
     def test_project_was_moved_to_another_organization(self):
         another_remote_organization = RemoteOrganization.objects.create(
-            slug="another", remote_id=4321
+            slug="another", remote_id="4321"
         )
 
         # Project belongs to an organization.
@@ -347,11 +347,12 @@ def test_send_build_status_value_error(self, session, mock_logger):
     @override_settings(DEFAULT_PRIVACY_LEVEL='private')
     def test_create_public_repo_when_private_projects_are_enabled(self):
         """Test ability to import ``public`` repositories under ``private`` level."""
+        self.repo_response_data["owner"]["type"] = "Organization"
         repo = self.service.create_repository(
             self.repo_response_data, organization=self.org
         )
         self.assertEqual(repo.organization, self.org)
-        self.assertEqual(repo.remote_id, self.repo_response_data["id"])
+        self.assertEqual(repo.remote_id, str(self.repo_response_data["id"]))
 
     @mock.patch('readthedocs.oauth.services.github.log')
     @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session')

From b82c3c3c343311d1dcc4a7b18998575dd88aa4ba Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@protonmail.com>
Date: Wed, 25 May 2022 19:15:14 -0500
Subject: [PATCH 3/9] Always create an organization

---
 readthedocs/oauth/services/github.py | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
index 987af9c8021..0702420c9ee 100644
--- a/readthedocs/oauth/services/github.py
+++ b/readthedocs/oauth/services/github.py
@@ -59,19 +59,15 @@ def sync_organizations(self):
             for org in orgs:
                 org_details = self.get_session().get(org['url']).json()
                 remote_organization = self.create_organization(org_details)
+                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):
@@ -83,7 +79,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.
 
@@ -105,14 +101,16 @@ def create_repository(self, fields, privacy=None, organization=None):
             )
 
             owner_type = fields["owner"]["type"]
+            organization = None
+            if owner_type == "Organization":
+                # NOTE: create_organization will always create a remote relationship
+                # with the current user, but the sync method will take care of removing
+                # the relationship for organizations where the user isn't a member.
+                organization = self.create_organization(fields=fields["owner"])
 
             # If there is an organization associated with this repository,
             # attach the organization to the repository.
-            if (
-                organization
-                and owner_type == "Organization"
-                and organization.remote_id == str(fields["owner"]["id"])
-            ):
+            if organization and owner_type == "Organization":
                 repo.organization = organization
 
             # If there is no organization and the repository belongs to a user,

From 99d43a15a28bff78b34e9a8a3516f9a60db601f2 Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@protonmail.com>
Date: Wed, 25 May 2022 19:17:25 -0500
Subject: [PATCH 4/9] Don't check for organization

---
 readthedocs/oauth/services/github.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
index 0702420c9ee..01c43558fc2 100644
--- a/readthedocs/oauth/services/github.py
+++ b/readthedocs/oauth/services/github.py
@@ -113,9 +113,9 @@ def create_repository(self, fields, privacy=None):
             if organization and owner_type == "Organization":
                 repo.organization = organization
 
-            # If there is no organization and the repository belongs to a user,
+            # If the repository belongs to a user,
             # remove the organization linked to the repository.
-            if not organization and owner_type == "User":
+            if owner_type == "User":
                 repo.organization = None
 
             repo.name = fields['name']

From 4797d7ce68866c2071b908e9dbc6da48c2867a9c Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@protonmail.com>
Date: Wed, 25 May 2022 19:24:53 -0500
Subject: [PATCH 5/9] Make explicit the creation of a relationship
 user->organization

---
 readthedocs/oauth/services/github.py | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
index 01c43558fc2..1260cdab77f 100644
--- a/readthedocs/oauth/services/github.py
+++ b/readthedocs/oauth/services/github.py
@@ -57,8 +57,11 @@ 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"]
@@ -103,10 +106,12 @@ def create_repository(self, fields, privacy=None):
             owner_type = fields["owner"]["type"]
             organization = None
             if owner_type == "Organization":
-                # NOTE: create_organization will always create a remote relationship
-                # with the current user, but the sync method will take care of removing
-                # the relationship for organizations where the user isn't a member.
-                organization = self.create_organization(fields=fields["owner"])
+                # 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,
+                )
 
             # If there is an organization associated with this repository,
             # attach the organization to the repository.
@@ -153,7 +158,7 @@ def create_repository(self, fields, privacy=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.
 
@@ -164,9 +169,8 @@ def create_organization(self, fields):
             remote_id=fields['id'],
             vcs_provider=self.vcs_provider_slug
         )
-        organization.get_remote_organization_relation(
-            self.user, self.account
-        )
+        if create_relationship:
+            organization.get_remote_organization_relation(self.user, self.account)
 
         organization.url = fields.get('html_url')
         # fields['login'] contains GitHub Organization slug

From b7ae87fdc87faf2956731e09aeda7b251c143c1e Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@protonmail.com>
Date: Thu, 26 May 2022 12:08:15 -0500
Subject: [PATCH 6/9] Update tests

---
 readthedocs/oauth/services/base.py        |  21 ---
 readthedocs/oauth/services/github.py      |   4 +-
 readthedocs/rtd_tests/tests/test_oauth.py | 154 ++++++++++++----------
 3 files changed, 86 insertions(+), 93 deletions(-)

diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py
index 38669f2f830..943110086c8 100644
--- a/readthedocs/oauth/services/base.py
+++ b/readthedocs/oauth/services/base.py
@@ -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
-
     def get_next_url_to_paginate(self, response):
         """
         Return the next url to feed the `paginate` method.
diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
index 1260cdab77f..4f9401bf073 100644
--- a/readthedocs/oauth/services/github.py
+++ b/readthedocs/oauth/services/github.py
@@ -166,8 +166,8 @@ def create_organization(self, fields, create_relationship=False):
         :rtype: RemoteOrganization
         """
         organization, _ = RemoteOrganization.objects.get_or_create(
-            remote_id=fields['id'],
-            vcs_provider=self.vcs_provider_slug
+            remote_id=str(fields["id"]),
+            vcs_provider=self.vcs_provider_slug,
         )
         if create_relationship:
             organization.get_remote_organization_relation(self.user, self.account)
diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py
index ac3d61ccb59..e7545b8f0c8 100644
--- a/readthedocs/rtd_tests/tests/test_oauth.py
+++ b/readthedocs/rtd_tests/tests/test_oauth.py
@@ -1,3 +1,4 @@
+import copy
 from unittest import mock
 
 from allauth.socialaccount.models import SocialAccount
@@ -26,7 +27,11 @@ def setUp(self):
         self.client.login(username='eric', password='test')
         self.user = User.objects.get(pk=1)
         self.project = Project.objects.get(slug="pip")
-        self.org = RemoteOrganization.objects.create(slug="rtfd", remote_id="1234")
+        self.org = RemoteOrganization.objects.create(
+            slug="organization",
+            remote_id="1234",
+            vcs_provider=GITHUB,
+        )
         self.privacy = settings.DEFAULT_PRIVACY_LEVEL
         self.service = GitHubService(
             user=self.user,
@@ -66,6 +71,24 @@ def setUp(self):
                 "id": 1234,
             },
         }
+        self.repo_with_org_response_data = copy.deepcopy(self.repo_response_data)
+        self.repo_with_org_response_data["owner"] = {
+            "login": "organization",
+            "id": 1234,
+            "node_id": "a1b2c3",
+            "url": "https://api.github.com/orgs/organization",
+            "description": "",
+            "name": "Organization",
+            "company": None,
+            "blog": "http://organization.org",
+            "location": "Portland, Oregon & Worldwide. ",
+            "email": None,
+            "is_verified": False,
+            "html_url": "https://github.com/organization",
+            "created_at": "2010-08-16T19:17:46Z",
+            "updated_at": "2020-08-12T14:26:39Z",
+            "type": "Organization",
+        }
 
     def test_create_remote_repository(self):
         repo = self.service.create_repository(
@@ -95,10 +118,8 @@ def test_create_remote_repository(self):
         self.assertEqual(repo.html_url, "https://github.com/testuser/testrepo")
 
     def test_create_remote_repository_with_organization(self):
-        self.repo_response_data["owner"]["type"] = "Organization"
         repo = self.service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
         self.assertIsInstance(repo, RemoteRepository)
@@ -121,11 +142,42 @@ def test_create_remote_repository_with_organization(self):
         )
         self.assertEqual(repo.html_url, 'https://github.com/testuser/testrepo')
 
+    def test_create_remote_repository_with_new_organization(self):
+        self.org.delete()
+        repo = self.service.create_repository(
+            self.repo_with_org_response_data,
+            privacy=self.privacy,
+        )
+        self.assertIsInstance(repo, RemoteRepository)
+        self.assertEqual(repo.name, "testrepo")
+        self.assertEqual(repo.full_name, "testuser/testrepo")
+        self.assertEqual(repo.remote_id, "12345678")
+        self.assertEqual(repo.vcs_provider, GITHUB)
+        self.assertEqual(repo.description, "Test Repo")
+        self.assertEqual(
+            repo.avatar_url,
+            settings.OAUTH_AVATAR_USER_DEFAULT_URL,
+        )
+        self.assertIn(self.user, repo.users.all())
+        self.assertEqual(
+            repo.clone_url,
+            "https://github.com/testuser/testrepo.git",
+        )
+        self.assertEqual(
+            repo.ssh_url,
+            "ssh://git@github.com:testuser/testrepo.git",
+        )
+        self.assertEqual(repo.html_url, "https://github.com/testuser/testrepo")
+        org = repo.organization
+
+        self.assertEqual(org.remote_id, "1234")
+        self.assertEqual(org.slug, "organization")
+        self.assertEqual(org.url, "https://github.com/organization")
+
     def test_skip_creation_remote_repository_on_private_repos(self):
         self.repo_response_data["private"] = True
         github_project = self.service.create_repository(
             self.repo_response_data,
-            organization=self.org,
             privacy=self.privacy,
         )
         self.assertIsNone(github_project)
@@ -137,19 +189,9 @@ def test_project_was_moved_from_a_personal_account_to_an_organization(self):
         )
         self.assertEqual(github_project.organization, None)
 
-        # Project belongs has been moved to an organization.
-        self.repo_response_data["owner"]["type"] = "Organization"
-
-        self.service.create_repository(
-            self.repo_response_data,
-            privacy=self.privacy,
-        )
-        github_project.refresh_from_db()
-        self.assertEqual(github_project.organization, None)
-
+        # Project has been moved to an organization.
         self.service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
         github_project.refresh_from_db()
@@ -157,26 +199,13 @@ def test_project_was_moved_from_a_personal_account_to_an_organization(self):
 
     def test_project_was_moved_from_an_organization_to_a_personal_account(self):
         # Project belongs to an organization.
-        self.repo_response_data["owner"]["type"] = "Organization"
         github_project = self.service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
         self.assertEqual(github_project.organization, self.org)
 
-        # Project belongs has been moved to a personal account.
-        self.repo_response_data["owner"]["type"] = "User"
-
-        # This operation doesn't have any effect.
-        self.service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
-            privacy=self.privacy,
-        )
-        github_project.refresh_from_db()
-        self.assertEqual(github_project.organization, self.org)
-
+        # Project has been moved to a personal account.
         self.service.create_repository(
             self.repo_response_data,
             privacy=self.privacy,
@@ -186,33 +215,22 @@ def test_project_was_moved_from_an_organization_to_a_personal_account(self):
 
     def test_project_was_moved_to_another_organization(self):
         another_remote_organization = RemoteOrganization.objects.create(
-            slug="another", remote_id="4321"
+            slug="another",
+            remote_id="4321",
+            vcs_provider=GITHUB,
         )
 
         # Project belongs to an organization.
-        self.repo_response_data["owner"]["type"] = "Organization"
         github_project = self.service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
         self.assertEqual(github_project.organization, self.org)
 
         # Project was moved to another organization.
-        self.repo_response_data["owner"]["id"] = 4321
-
+        self.repo_with_org_response_data["owner"]["id"] = 4321
         self.service.create_repository(
-            self.repo_response_data,
-            organization=another_remote_organization,
-            privacy=self.privacy,
-        )
-        github_project.refresh_from_db()
-        self.assertEqual(github_project.organization, another_remote_organization)
-
-        # The remote ID doesn't match, the project isn't moved to the organization.
-        self.service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
         github_project.refresh_from_db()
@@ -241,10 +259,8 @@ def test_import_with_no_token(self):
         self.assertEqual(services, [])
 
     def test_multiple_users_same_repo(self):
-        self.repo_response_data["owner"]["type"] = "Organization"
         github_project = self.service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
 
@@ -254,8 +270,7 @@ def test_multiple_users_same_repo(self):
             account=get(SocialAccount, user=self.user)
         )
         github_project_2 = service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
         self.assertIsInstance(github_project, RemoteRepository)
@@ -263,13 +278,11 @@ def test_multiple_users_same_repo(self):
         self.assertEqual(github_project_2, github_project)
 
         github_project_3 = self.service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
         github_project_4 = service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
         self.assertIsInstance(github_project_3, RemoteRepository)
@@ -278,13 +291,11 @@ def test_multiple_users_same_repo(self):
         self.assertEqual(github_project_2, github_project_4)
 
         github_project_5 = self.service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
         github_project_6 = service.create_repository(
-            self.repo_response_data,
-            organization=self.org,
+            self.repo_with_org_response_data,
             privacy=self.privacy,
         )
 
@@ -345,14 +356,11 @@ def test_send_build_status_value_error(self, session, mock_logger):
         )
 
     @override_settings(DEFAULT_PRIVACY_LEVEL='private')
-    def test_create_public_repo_when_private_projects_are_enabled(self):
+    def test_create_public_repo_when_private_projects_are_enabled_gh(self):
         """Test ability to import ``public`` repositories under ``private`` level."""
-        self.repo_response_data["owner"]["type"] = "Organization"
-        repo = self.service.create_repository(
-            self.repo_response_data, organization=self.org
-        )
+        repo = self.service.create_repository(self.repo_with_org_response_data)
         self.assertEqual(repo.organization, self.org)
-        self.assertEqual(repo.remote_id, str(self.repo_response_data["id"]))
+        self.assertEqual(repo.remote_id, str(self.repo_with_org_response_data["id"]))
 
     @mock.patch('readthedocs.oauth.services.github.log')
     @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session')
@@ -574,7 +582,10 @@ def setUp(self):
         self.project = Project.objects.get(slug='pip')
         self.project.repo = 'https://bitbucket.org/testuser/testrepo/'
         self.project.save()
-        self.org = RemoteOrganization.objects.create(slug='rtfd')
+        self.org = RemoteOrganization.objects.create(
+            slug="rtfd",
+            vcs_provider=BITBUCKET,
+        )
         self.privacy = settings.DEFAULT_PRIVACY_LEVEL
         self.service = BitbucketService(
             user=self.user,
@@ -1115,7 +1126,10 @@ def setUp(self):
         self.project = Project.objects.get(slug='pip')
         self.project.repo = 'https://gitlab.com/testorga/testrepo'
         self.project.save()
-        self.org = RemoteOrganization.objects.create(slug='testorga')
+        self.org = RemoteOrganization.objects.create(
+            slug="testorga",
+            vcs_provider=GITLAB,
+        )
         self.privacy = settings.DEFAULT_PRIVACY_LEVEL
         self.service = GitLabService(
             user=self.user,

From a59150c674fbb3bdc426183c623fe1ac37e64ec0 Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@protonmail.com>
Date: Thu, 26 May 2022 12:16:08 -0500
Subject: [PATCH 7/9] Update docstring

---
 readthedocs/oauth/services/github.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
index 4f9401bf073..781ce08b674 100644
--- a/readthedocs/oauth/services/github.py
+++ b/readthedocs/oauth/services/github.py
@@ -163,6 +163,9 @@ 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(

From d19240e4b4b11a906c1a2e0c6fb3ea7de8b80aa4 Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@protonmail.com>
Date: Tue, 31 May 2022 11:36:03 -0500
Subject: [PATCH 8/9] Updates from review

---
 readthedocs/oauth/services/github.py      | 14 +++++++-------
 readthedocs/rtd_tests/tests/test_oauth.py |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
index 781ce08b674..f5b84be8b35 100644
--- a/readthedocs/oauth/services/github.py
+++ b/readthedocs/oauth/services/github.py
@@ -60,7 +60,7 @@ def sync_organizations(self):
                 org_details = self.get_session().get(org["url"]).json()
                 remote_organization = self.create_organization(
                     org_details,
-                    create_relationship=True,
+                    create_user_relationship=True,
                 )
                 remote_organizations.append(remote_organization)
 
@@ -106,11 +106,11 @@ def create_repository(self, fields, privacy=None):
             owner_type = fields["owner"]["type"]
             organization = None
             if owner_type == "Organization":
-                # NOTE: We shouldn't create a remote relationship with the current user here,
+                # We aren't creating a remote relationship between the current user and the organization,
                 # since the user can have access to the repository, but not to the organization.
                 organization = self.create_organization(
                     fields=fields["owner"],
-                    create_relationship=False,
+                    create_user_relationship=False,
                 )
 
             # If there is an organization associated with this repository,
@@ -158,13 +158,13 @@ def create_repository(self, fields, privacy=None):
             repository=fields['name'],
         )
 
-    def create_organization(self, fields, create_relationship=False):
+    def create_organization(self, fields, create_user_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
+        :param bool create_relationship: Whether to create a remote relationship between the
+         organization and the current user. If `False`, only the `RemoteOrganization` object
          will be created/updated.
         :rtype: RemoteOrganization
         """
@@ -172,7 +172,7 @@ def create_organization(self, fields, create_relationship=False):
             remote_id=str(fields["id"]),
             vcs_provider=self.vcs_provider_slug,
         )
-        if create_relationship:
+        if create_user_relationship:
             organization.get_remote_organization_relation(self.user, self.account)
 
         organization.url = fields.get('html_url')
diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py
index e7545b8f0c8..8e0a171526f 100644
--- a/readthedocs/rtd_tests/tests/test_oauth.py
+++ b/readthedocs/rtd_tests/tests/test_oauth.py
@@ -356,7 +356,7 @@ def test_send_build_status_value_error(self, session, mock_logger):
         )
 
     @override_settings(DEFAULT_PRIVACY_LEVEL='private')
-    def test_create_public_repo_when_private_projects_are_enabled_gh(self):
+    def test_create_public_repo_when_private_projects_are_enabled(self):
         """Test ability to import ``public`` repositories under ``private`` level."""
         repo = self.service.create_repository(self.repo_with_org_response_data)
         self.assertEqual(repo.organization, self.org)

From bde5d43edc1469acc693f88ad8a719041247ed8e Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@protonmail.com>
Date: Tue, 31 May 2022 15:03:56 -0500
Subject: [PATCH 9/9] Linter

---
 readthedocs/oauth/services/github.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
index f5b84be8b35..16dc30a6402 100644
--- a/readthedocs/oauth/services/github.py
+++ b/readthedocs/oauth/services/github.py
@@ -106,8 +106,9 @@ def create_repository(self, fields, privacy=None):
             owner_type = fields["owner"]["type"]
             organization = None
             if owner_type == "Organization":
-                # We aren't creating a remote relationship between the current user and the organization,
-                # since the user can have access to the repository, but not to the organization.
+                # We aren't creating a remote relationship between the current user
+                # and the organization, since the user can have access to the repository,
+                # but not to the organization.
                 organization = self.create_organization(
                     fields=fields["owner"],
                     create_user_relationship=False,