PR previews: Warn users when enabling the feature on incompatible projects

stsewd · stsewd · commit 47546f5cb133 · 2022-06-02T12:50:27.000-05:00
Closes #9176
diff --git a/docs/user/integrations.rst b/docs/user/integrations.rst
@@ -85,7 +85,8 @@ GitLab
 * Go to the :guilabel:`Settings` > :guilabel:`Webhooks` page for your project
 * For **URL**, use the URL of the integration on Read the Docs,
   found on the :guilabel:`Admin` > :guilabel:`Integrations`  page
-* Leave the default **Push events** selected and mark **Tag push events** also
+* Leave the default **Push events** selected,
+  additionally mark **Tag push events** and **Merge request events**.
 * Finish by clicking **Add Webhook**
 
 Gitea
diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py
@@ -10,6 +10,7 @@
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.template.loader import render_to_string
+from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
 
 from readthedocs.builds.constants import INTERNAL
@@ -272,6 +273,67 @@ def __init__(self, *args, **kwargs):
         else:
             self.fields['default_version'].widget.attrs['readonly'] = True
 
+        self.setup_external_builds_option()
+
+    def setup_external_builds_option(self):
+        """Disable the external builds option if the project doesn't meet the requirements."""
+        msg = None
+        url = reverse("projects_integrations", args=[self.instance.slug])
+        integrations = list(self.instance.integrations.all())
+        if not self.has_supported_integration(integrations):
+            msg = _(
+                f'To build from pull requests you need a GitHub or GitLab <a href="{url}">integration</a>.'
+            )
+        elif not self.can_build_external_versions(integrations):
+            # 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. "
+                f'Try to <a href="{url}">resync your integration</a>.'
+            )
+
+        if msg:
+            field = self.fields["external_builds_enabled"]
+            field.disabled = True
+            field.help_text = 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_conf_py_file(self):
         filename = self.cleaned_data.get('conf_py_file', '').strip()
         if filename and 'conf.py' not in filename:
diff --git a/readthedocs/projects/migrations/0089_update_help_text.py b/readthedocs/projects/migrations/0089_update_help_text.py
@@ -0,0 +1,31 @@
+# Generated by Django 3.2.13 on 2022-06-02 17:49
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("projects", "0088_domain_field_edits"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="historicalproject",
+            name="external_builds_enabled",
+            field=models.BooleanField(
+                default=False,
+                help_text='More information in <a href="https://docs.readthedocs.io/page/guides/autobuild-docs-for-pull-requests.html">our docs</a>.',
+                verbose_name="Build pull requests for this project",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="project",
+            name="external_builds_enabled",
+            field=models.BooleanField(
+                default=False,
+                help_text='More information in <a href="https://docs.readthedocs.io/page/guides/autobuild-docs-for-pull-requests.html">our docs</a>.',
+                verbose_name="Build pull requests for this project",
+            ),
+        ),
+    ]
diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py
@@ -218,7 +218,9 @@ class Project(models.Model):
     external_builds_enabled = models.BooleanField(
         _('Build pull requests for this project'),
         default=False,
-        help_text=_('More information in <a href="https://docs.readthedocs.io/page/guides/autobuild-docs-for-pull-requests.html">our docs</a>')  # noqa
+        help_text=_(
+            'More information in <a href="https://docs.readthedocs.io/page/guides/autobuild-docs-for-pull-requests.html">our docs</a>.'  # noqa
+        ),
     )
     external_builds_privacy_level = models.CharField(
         _('Privacy level of Pull Requests'),
diff --git a/readthedocs/projects/tests/test_views.py b/readthedocs/projects/tests/test_views.py
@@ -0,0 +1,100 @@
+from django.contrib.auth.models import User
+from django.test import TestCase
+from django.urls import reverse
+from django_dynamic_fixture import get
+
+from readthedocs.integrations.models import Integration
+from readthedocs.projects.models import Project
+
+
+class TestExternalBuildOption(TestCase):
+    def setUp(self):
+        self.user = get(User)
+        self.project = get(Project, users=[self.user])
+        self.integration = get(
+            Integration,
+            integration_type=Integration.GITHUB_WEBHOOK,
+            project=self.project,
+        )
+        self.url = reverse("projects_advanced", args=[self.project.slug])
+        self.client.force_login(self.user)
+
+    def test_unsuported_integration(self):
+        self.integration.delete()
+        resp = self.client.get(self.url)
+        field = resp.context["form"].fields["external_builds_enabled"]
+        self.assertTrue(field.disabled)
+        self.assertTrue(
+            field.help_text.startswith(
+                "To build from pull requests you need a GitHub or GitLab"
+            )
+        )
+
+        get(
+            Integration,
+            project=self.project,
+            integration_type=Integration.BITBUCKET_WEBHOOK,
+        )
+        resp = self.client.get(self.url)
+        field = resp.context["form"].fields["external_builds_enabled"]
+        self.assertTrue(field.disabled)
+        self.assertTrue(
+            field.help_text.startswith(
+                "To build from pull requests you need a GitHub or GitLab"
+            )
+        )
+
+    def test_github_integration(self):
+        self.integration.provider_data = {}
+        self.integration.save()
+
+        resp = self.client.get(self.url)
+        field = resp.context["form"].fields["external_builds_enabled"]
+        self.assertFalse(field.disabled)
+        self.assertTrue(field.help_text.startswith("More information in"))
+
+        self.integration.provider_data = {"events": ["pull_request"]}
+        self.integration.save()
+        resp = self.client.get(self.url)
+        field = resp.context["form"].fields["external_builds_enabled"]
+        self.assertFalse(field.disabled)
+        self.assertTrue(field.help_text.startswith("More information in"))
+
+        self.integration.provider_data = {"events": []}
+        self.integration.save()
+        resp = self.client.get(self.url)
+        field = resp.context["form"].fields["external_builds_enabled"]
+        self.assertTrue(field.disabled)
+        self.assertTrue(
+            field.help_text.startswith(
+                "To build from pull requests your repository's webhook needs to send pull request events."
+            )
+        )
+
+    def test_gitlab_integration(self):
+        self.integration.integration_type = Integration.GITLAB_WEBHOOK
+        self.integration.provider_data = {}
+        self.integration.save()
+
+        resp = self.client.get(self.url)
+        field = resp.context["form"].fields["external_builds_enabled"]
+        self.assertFalse(field.disabled)
+        self.assertTrue(field.help_text.startswith("More information in"))
+
+        self.integration.provider_data = {"merge_requests_events": True}
+        self.integration.save()
+        resp = self.client.get(self.url)
+        field = resp.context["form"].fields["external_builds_enabled"]
+        self.assertFalse(field.disabled)
+        self.assertTrue(field.help_text.startswith("More information in"))
+
+        self.integration.provider_data = {"merge_requests_events": False}
+        self.integration.save()
+        resp = self.client.get(self.url)
+        field = resp.context["form"].fields["external_builds_enabled"]
+        self.assertTrue(field.disabled)
+        self.assertTrue(
+            field.help_text.startswith(
+                "To build from pull requests your repository's webhook needs to send pull request events."
+            )
+        )
