Project: allow connecting a project to a remote repository after it has been created (#11498)

* Project: allow connecting a project to a remote repo after it has been created

Close #9437

* Add migration

* Mark migration as safe

* Tests

* Fix tests

* Updates from review

Author: stsewd

readthedocs/oauth/models.py

@@ -186,7 +186,7 @@ class Meta:
         db_table = "oauth_remoterepository_2020"
 
     def __str__(self):
-        return self.html_url
+        return self.html_url or self.full_name
 
     def matches(self, user):
         """Existing projects connected to this RemoteRepository."""

readthedocs/projects/forms.py

@@ -52,13 +52,62 @@ def __init__(self, *args, **kwargs):
         self.user = kwargs.pop("user", None)
         super().__init__(*args, **kwargs)
 
+        self.fields["repo"].widget.attrs["placeholder"] = self.placehold_repo()
+        self.fields["repo"].widget.attrs["required"] = True
+
+        queryset = RemoteRepository.objects.for_project_linking(self.user)
+        current_remote_repo = (
+            self.instance.remote_repository if self.instance.pk else None
+        )
+        # If there is a remote repo attached to the project, add it to the queryset,
+        # since the current user might not have access to it.
+        if current_remote_repo:
+            queryset |= RemoteRepository.objects.filter(
+                pk=current_remote_repo.pk
+            ).distinct()
+        self.fields["remote_repository"].queryset = queryset
+        self.fields["remote_repository"].empty_label = _("No connected repository")
+
     def save(self, commit=True):
         project = super().save(commit)
         if commit:
             if self.user and not project.users.filter(pk=self.user.pk).exists():
                 project.users.add(self.user)
         return project
 
+    def clean_name(self):
+        name = self.cleaned_data.get("name", "")
+        if not self.instance.pk:
+            potential_slug = slugify(name)
+            if Project.objects.filter(slug=potential_slug).exists():
+                raise forms.ValidationError(
+                    _("Invalid project name, a project already exists with that name"),
+                )  # yapf: disable # noqa
+            if not potential_slug:
+                # Check the generated slug won't be empty
+                raise forms.ValidationError(
+                    _("Invalid project name"),
+                )
+
+        return name
+
+    def clean_repo(self):
+        repo = self.cleaned_data.get("repo", "")
+        return repo.rstrip("/")
+
+    def placehold_repo(self):
+        return choice(
+            [
+                "https://bitbucket.org/cherrypy/cherrypy",
+                "https://bitbucket.org/birkenfeld/sphinx",
+                "https://bitbucket.org/hpk42/tox",
+                "https://github.com/zzzeek/sqlalchemy.git",
+                "https://github.com/django/django.git",
+                "https://github.com/fabric/fabric.git",
+                "https://github.com/ericholscher/django-kong.git",
+            ]
+        )
+
 
 class ProjectTriggerBuildMixin:
 
@@ -313,73 +362,13 @@ class ProjectBasicsForm(ProjectForm):
 
     class Meta:
         model = Project
-        fields = ("name", "repo", "default_branch", "language")
-
-    remote_repository = forms.IntegerField(
-        widget=forms.HiddenInput(),
-        required=False,
-    )
+        fields = ("name", "repo", "default_branch", "language", "remote_repository")
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.fields["repo"].widget.attrs["placeholder"] = self.placehold_repo()
         self.fields["repo"].widget.attrs["required"] = True
-
-    def save(self, commit=True):
-        """Add remote repository relationship to the project instance."""
-        instance = super().save(commit)
-        remote_repo = self.cleaned_data.get("remote_repository", None)
-        if remote_repo:
-            if commit:
-                remote_repo.projects.add(self.instance)
-                remote_repo.save()
-            else:
-                instance.remote_repository = remote_repo
-        return instance
-
-    def clean_name(self):
-        name = self.cleaned_data.get("name", "")
-        if not self.instance.pk:
-            potential_slug = slugify(name)
-            if Project.objects.filter(slug=potential_slug).exists():
-                raise forms.ValidationError(
-                    _("Invalid project name, a project already exists with that name"),
-                )  # yapf: disable # noqa
-            if not potential_slug:
-                # Check the generated slug won't be empty
-                raise forms.ValidationError(
-                    _("Invalid project name"),
-                )
-
-        return name
-
-    def clean_repo(self):
-        repo = self.cleaned_data.get("repo", "")
-        return repo.rstrip("/")
-
-    def clean_remote_repository(self):
-        remote_repo = self.cleaned_data.get("remote_repository", None)
-        if not remote_repo:
-            return None
-        try:
-            return RemoteRepository.objects.for_project_linking(self.user).get(
-                pk=remote_repo,
-            )
-        except RemoteRepository.DoesNotExist as exc:
-            raise forms.ValidationError(_("Repository invalid")) from exc
-
-    def placehold_repo(self):
-        return choice(
-            [
-                "https://bitbucket.org/cherrypy/cherrypy",
-                "https://bitbucket.org/birkenfeld/sphinx",
-                "https://bitbucket.org/hpk42/tox",
-                "https://github.com/zzzeek/sqlalchemy.git",
-                "https://github.com/django/django.git",
-                "https://github.com/fabric/fabric.git",
-                "https://github.com/ericholscher/django-kong.git",
-            ]
-        )
+        self.fields["remote_repository"].widget = forms.HiddenInput()
 
 
 class ProjectConfigForm(forms.Form):
@@ -394,7 +383,7 @@ def __init__(self, *args, **kwargs):
 
 class UpdateProjectForm(
     ProjectTriggerBuildMixin,
-    ProjectBasicsForm,
+    ProjectForm,
     ProjectPRBuildsMixin,
 ):
 
@@ -406,6 +395,7 @@ class Meta:
             # Basics and repo settings
             "name",
             "repo",
+            "remote_repository",
             "language",
             "default_version",
             "privacy_level",

readthedocs/projects/migrations/0126_alter_remote_repository_description.py

@@ -0,0 +1,43 @@
+# Generated by Django 4.2.13 on 2024-07-24 21:57
+
+import django.db.models.deletion
+from django.db import migrations, models
+from django_safemigrate import Safe
+
+
+class Migration(migrations.Migration):
+    safe = Safe.before_deploy
+    dependencies = [
+        ("oauth", "0016_deprecate_old_vcs"),
+        ("projects", "0125_update_naming"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="historicalproject",
+            name="remote_repository",
+            field=models.ForeignKey(
+                blank=True,
+                db_constraint=False,
+                verbose_name="Connected repository",
+                help_text="Repository connected to this project",
+                null=True,
+                on_delete=django.db.models.deletion.DO_NOTHING,
+                related_name="+",
+                to="oauth.remoterepository",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="project",
+            name="remote_repository",
+            field=models.ForeignKey(
+                blank=True,
+                verbose_name="Connected repository",
+                help_text="Repository connected to this project",
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name="projects",
+                to="oauth.remoterepository",
+            ),
+        ),
+    ]

readthedocs/projects/models.py

@@ -540,6 +540,8 @@ class Project(models.Model):
 
     remote_repository = models.ForeignKey(
         "oauth.RemoteRepository",
+        verbose_name=_("Connected repository"),
+        help_text=_("Repository connected to this project"),
         on_delete=models.SET_NULL,
         related_name="projects",
         null=True,

readthedocs/projects/views/private.py

@@ -200,6 +200,10 @@ class ProjectUpdate(ProjectMixin, UpdateView):
     def get_success_url(self):
         return reverse("projects_detail", args=[self.object.slug])
 
+    def get_form(self, data=None, files=None, **kwargs):
+        kwargs["user"] = self.request.user
+        return super().get_form(data, files, **kwargs)
+
 
 class ProjectDelete(UpdateChangeReasonPostView, ProjectMixin, DeleteViewWithMessage):
     success_message = _("Project deleted")

readthedocs/rtd_tests/tests/test_project_forms.py

@@ -11,6 +11,7 @@
 from readthedocs.builds.constants import EXTERNAL, LATEST, STABLE
 from readthedocs.builds.models import Version
 from readthedocs.core.forms import RichValidationError
+from readthedocs.oauth.models import RemoteRepository, RemoteRepositoryRelation
 from readthedocs.organizations.models import Organization, Team
 from readthedocs.projects.constants import (
     ADDONS_FLYOUT_SORTING_CALVER,
@@ -156,7 +157,8 @@ def test_strip_repo_url(self):
 
 class TestProjectAdvancedForm(TestCase):
     def setUp(self):
-        self.project = get(Project, privacy_level=PUBLIC)
+        self.user = get(User)
+        self.project = get(Project, privacy_level=PUBLIC, users=[self.user])
         get(
             Version,
             project=self.project,
@@ -333,6 +335,55 @@ def test_trigger_build_on_save(self, trigger_build):
             version=default_branch,
         )
 
+    def test_set_remote_repository(self):
+        data = {
+            "name": "Project",
+            "repo": "https://github.com/readthedocs/readthedocs.org/",
+            "repo_type": self.project.repo_type,
+            "default_version": LATEST,
+            "language": self.project.language,
+            "versioning_scheme": self.project.versioning_scheme,
+        }
+
+        remote_repository = get(
+            RemoteRepository,
+            full_name="rtfd/template",
+            clone_url="https://github.com/rtfd/template",
+            html_url="https://github.com/rtfd/template",
+            ssh_url="[email protected]:rtfd/template.git",
+            private=False,
+        )
+
+        # No remote repository attached.
+        form = UpdateProjectForm(data, instance=self.project, user=self.user)
+        self.assertTrue(form.is_valid())
+
+        # Remote repository attached, but it doesn't belong to the user.
+        data["remote_repository"] = remote_repository.pk
+        form = UpdateProjectForm(data, instance=self.project, user=self.user)
+        self.assertFalse(form.is_valid())
+        self.assertIn("remote_repository", form.errors)
+
+        # Remote repository attached, it belongs to the user now.
+        remote_repository_rel = get(
+            RemoteRepositoryRelation,
+            remote_repository=remote_repository,
+            user=self.user,
+            admin=True,
+        )
+        data["remote_repository"] = remote_repository.pk
+        form = UpdateProjectForm(data, instance=self.project, user=self.user)
+        self.assertTrue(form.is_valid())
+
+        # The project has the remote repository attached.
+        # And the user doesn't have access to it anymore, but still can use it.
+        self.project.remote_repository = remote_repository
+        self.project.save()
+        remote_repository_rel.delete()
+        data["remote_repository"] = remote_repository.pk
+        form = UpdateProjectForm(data, instance=self.project, user=self.user)
+        self.assertTrue(form.is_valid())
+
 
 class TestProjectAdvancedFormDefaultBranch(TestCase):
     def setUp(self):