Allow to build an external version based on its source and base branch.

- Use a namedtuple to pass more data about the external version around
- Automation rules can now receive extra kwargs to be passed down to the match and action functions
- Automation rules now return a tuple, the first element indicate if rule matched and the second is the return value from the action,
this was done, so we can know if the build was triggered or not for external versions.
- Moved the actions to the base class, since it doesn't look like we are going to have a different set of actions
- If the user doesn't have automation rules for external versions we just build everything as before,
  but if the user has at least one, we use the rules to trigger the build.

Ref #7653

Author: stsewd

readthedocs/api/v2/views/integrations.py

@@ -3,6 +3,7 @@
 import hashlib
 import hmac
 import json
+from functools import namedtuple
 import logging
 import re
 
@@ -56,6 +57,12 @@
 BITBUCKET_PUSH = 'repo:push'
 
 
+ExternalVersionData = namedtuple(
+    'ExternalVersionData',
+    ['id', 'source_branch', 'base_branch', 'commit'],
+)
+
+
 class WebhookMixin:
 
     """Base class for Webhook mixins."""
@@ -227,20 +234,20 @@ def get_external_version_response(self, project):
         :param project: Project instance
         :type project: readthedocs.projects.models.Project
         """
-        identifier, verbose_name = self.get_external_version_data()
+        version_data = self.get_external_version_data()
         # create or get external version object using `verbose_name`.
-        external_version = get_or_create_external_version(
-            project, identifier, verbose_name
-        )
+        external_version = get_or_create_external_version(project, version_data)
         # returns external version verbose_name (pull/merge request number)
         to_build = build_external_version(
-            project=project, version=external_version, commit=identifier
+            project=project,
+            version=external_version,
+            version_data=version_data,
         )
 
         return {
-            'build_triggered': True,
+            'build_triggered': bool(to_build),
             'project': project.slug,
-            'versions': [to_build],
+            'versions': [to_build] if to_build else [],
         }
 
     def get_delete_external_version_response(self, project):
@@ -258,11 +265,9 @@ def get_delete_external_version_response(self, project):
         :param project: Project instance
         :type project: Project
         """
-        identifier, verbose_name = self.get_external_version_data()
+        version_data = self.get_external_version_data()
         # Delete external version
-        deleted_version = delete_external_version(
-            project, identifier, verbose_name
-        )
+        deleted_version = delete_external_version(project, version_data)
         return {
             'version_deleted': deleted_version is not None,
             'project': project.slug,
@@ -320,13 +325,16 @@ def get_data(self):
     def get_external_version_data(self):
         """Get Commit Sha and pull request number from payload."""
         try:
-            identifier = self.data['pull_request']['head']['sha']
-            verbose_name = str(self.data['number'])
-
-            return identifier, verbose_name
+            data = ExternalVersionData(
+                id=str(self.data['number']),
+                commit=self.data['pull_request']['head']['sha'],
+                source_branch=self.data['pull_request']['head']['ref'],
+                base_branch=self.data['pull_request']['base']['ref'],
+            )
+            return data
 
         except KeyError:
-            raise ParseError('Parameters "sha" and "number" are required')
+            raise ParseError('Invalid payload')
 
     def is_payload_valid(self):
         """
@@ -530,13 +538,16 @@ def is_payload_valid(self):
     def get_external_version_data(self):
         """Get commit SHA and merge request number from payload."""
         try:
-            identifier = self.data['object_attributes']['last_commit']['id']
-            verbose_name = str(self.data['object_attributes']['iid'])
-
-            return identifier, verbose_name
+            data = ExternalVersionData(
+                id=str(self.data['object_attributes']['iid']),
+                commit=self.data['object_attributes']['last_commit']['id'],
+                source_branch=self.data['object_attributes']['source_branch'],
+                base_branch=self.data['object_attributes']['target_branch'],
+            )
+            return data
 
         except KeyError:
-            raise ParseError('Parameters "id" and "iid" are required')
+            raise ParseError('Invalid payload')
 
     def handle_webhook(self):
         """

readthedocs/builds/automation_actions.py

@@ -10,6 +10,7 @@
 
 import logging
 
+from readthedocs.builds.utils import match_regex
 from readthedocs.core.utils import trigger_build
 from readthedocs.projects.constants import PRIVATE, PUBLIC
 
@@ -31,6 +32,29 @@ def activate_version(version, match_result, action_arg, *args, **kwargs):
         )
 
 
+def build_external_version(version, match_result, action_arg, version_data, **kwargs):
+    """
+    Build an external version if matches the given base branch.
+
+    :param action_arg: A pattern to match the base branch.
+    :param version_data: `ExternalVersionData` instance.
+    :returns: A boolean indicating if the build was triggered.
+    """
+    base_branch_regex = action_arg
+    result = match_regex(
+        base_branch_regex,
+        version_data.base_branch,
+    )
+    if result:
+        trigger_build(
+            project=version.project,
+            version=version,
+            commit=version.identifier,
+        )
+        return True
+    return False
+
+
 def set_default_version(version, match_result, action_arg, *args, **kwargs):
     """
     Sets version as the project's default version.

readthedocs/builds/forms.py

@@ -1,5 +1,6 @@
 """Django forms for the builds app."""
 
+import itertools
 import re
 import textwrap
 
@@ -14,6 +15,8 @@
     ALL_VERSIONS,
     BRANCH,
     BRANCH_TEXT,
+    EXTERNAL,
+    EXTERNAL_TEXT,
     TAG,
     TAG_TEXT,
 )
@@ -99,8 +102,9 @@ class RegexAutomationRuleForm(forms.ModelForm):
             """
             A regular expression to match the version.
             <a href="https://docs.readthedocs.io/page/automation-rules.html#user-defined-matches">
-              Check the documentation for valid patterns.
+              Check the documentation
             </a>
+            for valid patterns.
             """
         )),
         required=False,
@@ -113,6 +117,7 @@ class Meta:
             'predefined_match_arg',
             'match_arg',
             'version_type',
+            'action_arg',
             'action',
         ]
         # Don't pollute the UI with help texts
@@ -133,6 +138,7 @@ def __init__(self, *args, **kwargs):
             (None, '-' * 9),
             (BRANCH, BRANCH_TEXT),
             (TAG, TAG_TEXT),
+            (EXTERNAL, EXTERNAL_TEXT),
         ]
 
         # Remove privacy actions not available in community
@@ -155,6 +161,48 @@ def __init__(self, *args, **kwargs):
         if self.instance.pk and self.instance.predefined_match_arg:
             self.initial['match_arg'] = self.instance.get_match_arg()
 
+    def clean_action(self):
+        """Check the action is allowed for the type of version."""
+        action = self.cleaned_data['action']
+        version_type = self.cleaned_data['version_type']
+        internal_allowed_actions = set(
+            itertools.chain(
+                VersionAutomationRule.allowed_actions_on_create.keys(),
+                VersionAutomationRule.allowed_actions_on_delete.keys(),
+            )
+        )
+        allowed_actions = {
+            BRANCH: internal_allowed_actions,
+            TAG: internal_allowed_actions,
+            EXTERNAL: set(VersionAutomationRule.allowed_actions_on_external_versions.keys()),
+        }
+        if action not in allowed_actions.get(version_type, []):
+            raise forms.ValidationError(
+                _('Invalid action for this version type.'),
+            )
+        return action
+
+    def clean_action_arg(self):
+        """
+        Validate the action argument.
+
+        Currently only external versions accept this argument,
+        and it's the pattern to match the base_branch.
+        """
+        action_arg = self.cleaned_data['action_arg']
+        version_type = self.cleaned_data['version_type']
+        if version_type == EXTERNAL:
+            if not action_arg:
+                action_arg = '.*'
+            try:
+                re.compile(action_arg)
+                return action_arg
+            except Exception:
+                raise forms.ValidationError(
+                    _('Invalid Python regular expression.'),
+                )
+        return ''
+
     def clean_match_arg(self):
         """Check that a custom match was given if a predefined match wasn't used."""
         match_arg = self.cleaned_data['match_arg']
@@ -185,6 +233,7 @@ def save(self, commit=True):
                 predefined_match_arg=self.cleaned_data['predefined_match_arg'],
                 version_type=self.cleaned_data['version_type'],
                 action=self.cleaned_data['action'],
+                action_arg=self.cleaned_data['action_arg'],
             )
         if not rule.description:
             rule.description = rule.get_description()

readthedocs/builds/managers.py

@@ -194,8 +194,7 @@ class VersionAutomationRuleManager(PolymorphicManager):
     """
 
     def add_rule(
-        self, *, project, description, match_arg, version_type,
-        action, action_arg=None, predefined_match_arg=None,
+        self, *, project, description, match_arg, version_type, action, **kwargs,
     ):
         """
         Append an automation rule to `project`.
@@ -219,9 +218,8 @@ def add_rule(
             priority=priority,
             description=description,
             match_arg=match_arg,
-            predefined_match_arg=predefined_match_arg,
             version_type=version_type,
             action=action,
-            action_arg=action_arg,
+            **kwargs,
         )
         return rule

readthedocs/builds/migrations/0029_add_build_external_version_action.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.2.16 on 2020-11-16 17:37
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('builds', '0028_add_delete_version_action'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='versionautomationrule',
+            name='action',
+            field=models.CharField(choices=[('activate-version', 'Activate version'), ('hide-version', 'Hide version'), ('make-version-public', 'Make version public'), ('make-version-private', 'Make version private'), ('set-default-version', 'Set version as default'), ('delete-version', 'Delete version (on branch/tag deletion)'), ('build-external-version', 'Build version')], help_text='Action to apply to matching versions', max_length=32, verbose_name='Action'),
+        ),
+    ]