Add Pull Request builds page to settings (#10656)

* Add Pull Request builds page to settings

This is a basic form for now,
but we can spruce it up before it goes live.

* Rebase to fix tests, and add placeholder template for now

* Add validation logic to form

* Try to use clean_prevalidation

* Update common

* Use proper validation logic

* Cleanup merge

* Apply suggestions from code review

Co-authored-by: Anthony <[email protected]>

* Fix whitespace

* Remove old code

* Continue to disbale external_builds_enabled field

* Attempt to keep backwards compat

* Update readthedocs/projects/forms.py

Co-authored-by: Anthony <[email protected]>

---------

Co-authored-by: Anthony Johnson <[email protected]>

readthedocs/projects/forms.py

@@ -1,4 +1,5 @@
 """Project forms."""
+
 import json
 from random import choice
 from re import fullmatch
@@ -97,6 +98,109 @@ class ProjectBackendForm(forms.Form):
     backend = forms.CharField()
 
 
+class ProjectPRBuildsMixin(PrevalidatedForm):
+
+    """
+    Mixin that provides a method to setup the external builds option.
+
+    TODO: Remove this once we migrate to the new dashboard,
+    and we don't need to support the old project settings form.
+    """
+
+    def has_supported_integration(self, integrations):
+        supported_types = {Integration.GITHUB_WEBHOOK, Integration.GITLAB_WEBHOOK}
+        for integration in integrations:
+            if integration.integration_type in supported_types:
+                return True
+        return False
+
+    def can_build_external_versions(self, integrations):
+        """
+        Check if external versions can be enabled for this project.
+
+        A project can build external versions if:
+
+        - They are using GitHub or GitLab.
+        - The repository's webhook is setup to send pull request events.
+
+        If the integration's provider data isn't set,
+        it could mean that the user created the integration manually,
+        and doesn't have an account connected.
+        So we don't know for sure if the webhook is sending pull request events.
+        """
+        for integration in integrations:
+            provider_data = integration.provider_data
+            if integration.integration_type == Integration.GITHUB_WEBHOOK and (
+                not provider_data or "pull_request" in provider_data.get("events", [])
+            ):
+                return True
+            if integration.integration_type == Integration.GITLAB_WEBHOOK and (
+                not provider_data or provider_data.get("merge_requests_events")
+            ):
+                return True
+        return False
+
+    def setup_external_builds_option(self):
+        """Disable the external builds option if the project doesn't meet the requirements."""
+        if (
+            settings.ALLOW_PRIVATE_REPOS
+            and self.instance.remote_repository
+            and not self.instance.remote_repository.private
+        ):
+            self.fields["external_builds_privacy_level"].disabled = True
+            # TODO use a proper error/warning instead of help text for error states
+            help_text = _(
+                "We have detected that this project is public, pull request previews are set to public."
+            )
+            self.fields["external_builds_privacy_level"].help_text = help_text
+
+    def clean_prevalidation(self):
+        """Disable the external builds option if the project doesn't meet the requirements."""
+        integrations = list(self.instance.integrations.all())
+        has_supported_integration = self.has_supported_integration(integrations)
+        can_build_external_versions = self.can_build_external_versions(integrations)
+
+        # External builds are supported for this project,
+        # don't disable the option.
+        if has_supported_integration and can_build_external_versions:
+            return
+
+        msg = None
+        url = reverse("projects_integrations", args=[self.instance.slug])
+
+        if not has_supported_integration:
+            msg = _(
+                "To build from pull requests you need a "
+                f'GitHub or GitLab <a href="{url}">integration</a>.'
+            )
+
+        if has_supported_integration and not can_build_external_versions:
+            # If there is only one integration, link directly to it.
+            if len(integrations) == 1:
+                url = reverse(
+                    "projects_integrations_detail",
+                    args=[self.instance.slug, integrations[0].pk],
+                )
+            msg = _(
+                "To build from pull requests your repository's webhook "
+                "needs to send pull request events. "
+                f'Try to <a href="{url}">resync your integration</a>.'
+            )
+
+        if msg:
+            # TODO use a proper error/warning instead of help text for error states
+            field = self.fields["external_builds_enabled"]
+            field.disabled = True
+            field.help_text = f"{msg} {field.help_text}"
+            # Don't raise an error on the Update form,
+            # to keep backwards compat
+            if not self.fields.get("name"):
+                raise RichValidationError(
+                    msg,
+                    header=_("Pull request builds not supported"),
+                )
+
+
 class ProjectFormPrevalidateMixin:
 
     """Provides shared logic between the automatic and manual create forms."""
@@ -292,6 +396,7 @@ def __init__(self, *args, **kwargs):
 class UpdateProjectForm(
     ProjectTriggerBuildMixin,
     ProjectBasicsForm,
+    ProjectPRBuildsMixin,
 ):
 
     """Main project settings form."""
@@ -383,83 +488,6 @@ def __init__(self, *args, **kwargs):
 
         self.setup_external_builds_option()
 
-    def setup_external_builds_option(self):
-        """Disable the external builds option if the project doesn't meet the requirements."""
-        if (
-            settings.ALLOW_PRIVATE_REPOS
-            and self.instance.remote_repository
-            and not self.instance.remote_repository.private
-        ):
-            self.fields["external_builds_privacy_level"].disabled = True
-            help_text = _(
-                "We have detected that this project is public, pull request previews are set to public."
-            )
-            self.fields["external_builds_privacy_level"].help_text = help_text
-
-        integrations = list(self.instance.integrations.all())
-        has_supported_integration = self.has_supported_integration(integrations)
-        can_build_external_versions = self.can_build_external_versions(integrations)
-
-        # External builds are supported for this project,
-        # don't disable the option.
-        if has_supported_integration and can_build_external_versions:
-            return
-
-        msg = None
-        url = reverse("projects_integrations", args=[self.instance.slug])
-        if not has_supported_integration:
-            msg = _(
-                f'To build from pull requests you need a GitHub or GitLab <a href="{url}">integration</a>.'
-            )
-        if has_supported_integration and not can_build_external_versions:
-            # If there is only one integration, link directly to it.
-            if len(integrations) == 1:
-                url = reverse(
-                    "projects_integrations_detail",
-                    args=[self.instance.slug, integrations[0].pk],
-                )
-            msg = _(
-                f'To build from pull requests your repository\'s webhook needs to send pull request events. Try to <a href="{url}">resync your integration</a>.'
-            )
-
-        if msg:
-            field = self.fields["external_builds_enabled"]
-            field.disabled = True
-            field.help_text = f"{msg} {field.help_text}"
-
-    def has_supported_integration(self, integrations):
-        supported_types = {Integration.GITHUB_WEBHOOK, Integration.GITLAB_WEBHOOK}
-        for integration in integrations:
-            if integration.integration_type in supported_types:
-                return True
-        return False
-
-    def can_build_external_versions(self, integrations):
-        """
-        Check if external versions can be enabled for this project.
-
-        A project can build external versions if:
-
-        - They are using GitHub or GitLab.
-        - The repository's webhook is setup to send pull request events.
-
-        If the integration's provider data isn't set,
-        it could mean that the user created the integration manually,
-        and doesn't have an account connected.
-        So we don't know for sure if the webhook is sending pull request events.
-        """
-        for integration in integrations:
-            provider_data = integration.provider_data
-            if integration.integration_type == Integration.GITHUB_WEBHOOK and (
-                not provider_data or "pull_request" in provider_data.get("events", [])
-            ):
-                return True
-            if integration.integration_type == Integration.GITLAB_WEBHOOK and (
-                not provider_data or provider_data.get("merge_requests_events")
-            ):
-                return True
-        return False
-
     def clean_readthedocs_yaml_path(self):
         """
         Validate user input to help user.
@@ -571,6 +599,24 @@ def clean_alias(self):
         return alias
 
 
+class ProjectPullRequestForm(forms.ModelForm, ProjectPRBuildsMixin):
+
+    """Project pull requests configuration form."""
+
+    class Meta:
+        model = Project
+        fields = ["external_builds_enabled", "external_builds_privacy_level"]
+
+    def __init__(self, *args, **kwargs):
+        self.project = kwargs.pop("project", None)
+        super().__init__(*args, **kwargs)
+
+        self.setup_external_builds_option()
+
+        if not settings.ALLOW_PRIVATE_REPOS:
+            self.fields.pop("external_builds_privacy_level")
+
+
 class AddonsConfigForm(forms.ModelForm):
 
     """Form to opt-in into new beta addons."""

readthedocs/projects/urls/private.py

@@ -33,6 +33,7 @@
     ProjectEmailNotificationsCreate,
     ProjectNotifications,
     ProjectNotificationsDelete,
+    ProjectPullRequestsUpdate,
     ProjectRedirectsCreate,
     ProjectRedirectsDelete,
     ProjectRedirectsInsert,
@@ -181,6 +182,11 @@
         ProjectAdvertisingUpdate.as_view(),
         name="projects_advertising",
     ),
+    re_path(
+        r"^(?P<project_slug>[-\w]+)/pull-requests/$",
+        ProjectPullRequestsUpdate.as_view(),
+        name="projects_pull_requests",
+    ),
     re_path(
         r"^(?P<project_slug>[-\w]+)/search-analytics/$",
         SearchAnalytics.as_view(),

readthedocs/projects/views/private.py

@@ -1,4 +1,5 @@
 """Project views for authenticated users."""
+
 import structlog
 from allauth.socialaccount.models import SocialAccount
 from django.conf import settings
@@ -60,6 +61,7 @@
     ProjectBasicsForm,
     ProjectConfigForm,
     ProjectManualForm,
+    ProjectPullRequestForm,
     ProjectRelationshipForm,
     RedirectForm,
     TranslationForm,
@@ -1212,3 +1214,18 @@ def _get_csv_data(self):
 
     def _get_feature(self, project):
         return get_feature(project, feature_type=self.feature_type)
+
+
+class ProjectPullRequestsUpdate(PrivateViewMixin, UpdateView):
+    model = Project
+    form_class = ProjectPullRequestForm
+    success_message = _("Pull request settings have been updated")
+    template_name = "projects/pull_requests_form.html"
+    lookup_url_kwarg = "project_slug"
+    lookup_field = "slug"
+
+    def get_queryset(self):
+        return self.model.objects.for_admin_user(self.request.user)
+
+    def get_success_url(self):
+        return reverse("projects_pull_requests", args=[self.object.slug])

readthedocs/templates/projects/pull_requests_form.html

@@ -0,0 +1,3 @@
+{% extends "projects/project_edit_base.html" %}
+
+{# This is just a placeholder to satisfy tests. This template is only implemented in ext-theme #}