Skip to content
9 changes: 6 additions & 3 deletions readthedocs/oauth/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,9 @@ def _handle_push_event(self):
webhook_rules = project.automation_rules.filter(
enabled=True,
action__in=AutomationRule.BUILD_ACTIONS,
version_types__contains=version_type,
# NOTE: we cannot use __contains on JSON field on SQLite because it's not supported.
# We are using `rule.match_version` below to check this instead.
# version_types__contains=version_type,
)
if webhook_rules.exists():
changed_files = self._get_changed_files_from_push_event()
Expand Down Expand Up @@ -660,7 +662,9 @@ def _handle_pull_request_event(self):
webhook_rules = project.automation_rules.filter(
enabled=True,
action__in=AutomationRule.BUILD_ACTIONS,
version_types__contains=EXTERNAL,
# NOTE: we cannot use __contains on JSON field on SQLite because it's not supported.
# We are using `rule.match_version` below to check this instead.
# version_types__contains=EXTERNAL,
)
if webhook_rules.exists():
rule_triggered = False
Expand Down Expand Up @@ -932,7 +936,6 @@ def _get_commit_message_from_pull_request_event(self, project):

gh_commit = gh_repository.get_commit(
self.data["pull_request"]["head"]["sha"],
commit_files_per_page=0,
)
return gh_commit.commit.message

Expand Down
81 changes: 56 additions & 25 deletions readthedocs/oauth/tests/test_githubapp_webhook.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import json
from unittest import mock

from readthedocs.builds.models import WebhookAutomationRule, VersionAutomationRule
from readthedocs.builds.constants import ALL_VERSIONS
from readthedocs.projects.models import AutomationRule
import requests_mock
from allauth.socialaccount.models import SocialAccount
from django.conf import settings
Expand Down Expand Up @@ -1051,7 +1052,7 @@ def test_github_app_authorization(self):


class TestGitHubAppWebhookWithAutomationRules(TestCase):
"""Tests for webhook filtering with WebhookAutomationRule."""
"""Tests for webhook filtering with AutomationRule."""

def setUp(self):
self.user = get(User)
Expand Down Expand Up @@ -1092,16 +1093,17 @@ def post_webhook(self, event, payload):

@mock.patch("readthedocs.builds.automation_actions.trigger_build")
def test_push_branch_with_matching_webhook_rule(self, trigger_build):
"""Test that push triggers build when WebhookAutomationRule matches."""
"""Test that push triggers build when AutomationRule matches."""

# Create a webhook automation rule that matches docs files
rule = get(
WebhookAutomationRule,
AutomationRule,
project=self.project,
priority=0,
match_arg="docs/*.rst",
action=VersionAutomationRule.TRIGGER_BUILD_ACTION,
version_type=BRANCH,
version_predefined_match_pattern=ALL_VERSIONS,
webhook_files_match_pattern=["docs/*.rst"],
action=AutomationRule.TRIGGER_BUILD_ACTION,
version_types=[BRANCH],
)

payload = {
Expand All @@ -1117,6 +1119,9 @@ def test_push_branch_with_matching_webhook_rule(self, trigger_build):
"id": self.remote_repository.remote_id,
"full_name": self.remote_repository.full_name,
},
"head_commit": {
"message": "Update docs",
},
"commits": [
{
"added": ["docs/index.rst"],
Expand All @@ -1133,16 +1138,17 @@ def test_push_branch_with_matching_webhook_rule(self, trigger_build):

@mock.patch("readthedocs.builds.automation_actions.trigger_build")
def test_push_branch_with_non_matching_webhook_rule(self, trigger_build):
"""Test that push does not trigger build when WebhookAutomationRule doesn't match."""
"""Test that push does not trigger build when AutomationRule doesn't match."""

# Create a webhook automation rule that matches docs files
rule = get(
WebhookAutomationRule,
AutomationRule,
project=self.project,
priority=0,
match_arg="docs/*.rst",
action=VersionAutomationRule.TRIGGER_BUILD_ACTION,
version_type=BRANCH,
version_predefined_match_pattern=ALL_VERSIONS,
webhook_files_match_pattern=["docs/*.rst"],
action=AutomationRule.TRIGGER_BUILD_ACTION,
version_types=[BRANCH],
)

payload = {
Expand Down Expand Up @@ -1174,7 +1180,7 @@ def test_push_branch_with_non_matching_webhook_rule(self, trigger_build):

@mock.patch("readthedocs.core.views.hooks.trigger_build")
def test_push_branch_without_webhook_rules(self, trigger_build):
"""Test that push triggers build normally when no WebhookAutomationRules exist."""
"""Test that push triggers build normally when no AutomationRules exist."""
# No automation rules - should trigger build as usual
payload = {
"installation": {
Expand Down Expand Up @@ -1211,19 +1217,20 @@ def test_push_branch_without_webhook_rules(self, trigger_build):
@mock.patch("readthedocs.builds.automation_actions.trigger_build")
@requests_mock.Mocker(kw="request")
def test_pull_request_with_matching_webhook_rule(self, trigger_build, request):
"""Test that PR triggers build when WebhookAutomationRule matches."""
"""Test that PR triggers build when AutomationRule matches."""

self.project.external_builds_enabled = True
self.project.save()

# Create a webhook automation rule for external versions (PRs)
rule = get(
WebhookAutomationRule,
AutomationRule,
project=self.project,
priority=0,
match_arg="docs/**",
action=VersionAutomationRule.TRIGGER_BUILD_ACTION,
version_type=EXTERNAL,
version_predefined_match_pattern=ALL_VERSIONS,
webhook_files_match_pattern=["docs/**"],
action=AutomationRule.TRIGGER_BUILD_ACTION,
version_types=[EXTERNAL],
)

# Mock the GitHub API call to get PR files
Expand All @@ -1246,8 +1253,18 @@ def test_pull_request_with_matching_webhook_rule(self, trigger_build, request):
f"{api_url}/repositories/{self.remote_repository.remote_id}/pulls/1",
json={
"url": f"https://api.github.com/repos/{self.remote_repository.full_name}/pulls/1",
"issue_url": f"https://api.github.com/repos/{self.remote_repository.full_name}/issues/1",
},
)
request.get(
f"{api_url}/repositories/{self.remote_repository.remote_id}/commits/1234abcd",
json={
"commit": {
"message": "Update docs",
},
}
)

request.get(
f"{api_url}/repos/{self.remote_repository.full_name}/pulls/1",
json={
Expand All @@ -1256,7 +1273,20 @@ def test_pull_request_with_matching_webhook_rule(self, trigger_build, request):
)
request.get(
f"{api_url}/repos/{self.remote_repository.full_name}/pulls/1/files",
json=[{"filename": "docs/index.rst"}],
json=[
{
"filename": "docs/index.rst",
}
],
)
request.get(
f"{api_url}/repos/{self.remote_repository.full_name}/issues/1/labels",
json=[
{
"name": "bug",
"url": f"https://api.github.com/repos/{self.remote_repository.full_name}/labels/bug",
}
],
)

payload = {
Expand Down Expand Up @@ -1295,19 +1325,20 @@ def test_pull_request_with_matching_webhook_rule(self, trigger_build, request):
@mock.patch("readthedocs.builds.automation_actions.trigger_build")
@requests_mock.Mocker(kw="request")
def test_pull_request_with_non_matching_webhook_rule(self, trigger_build, request):
"""Test that PR does not trigger build when WebhookAutomationRule doesn't match."""
"""Test that PR does not trigger build when AutomationRule doesn't match."""

self.project.external_builds_enabled = True
self.project.save()

# Create a webhook automation rule for external versions (PRs)
rule = get(
WebhookAutomationRule,
AutomationRule,
project=self.project,
priority=0,
match_arg="docs/**",
action=VersionAutomationRule.TRIGGER_BUILD_ACTION,
version_type=EXTERNAL,
version_predefined_match_pattern=ALL_VERSIONS,
webhook_files_match_pattern=["docs/**"],
action=AutomationRule.TRIGGER_BUILD_ACTION,
version_types=[EXTERNAL],
)

# Mock the GitHub API call to get PR files
Expand Down Expand Up @@ -1367,7 +1398,7 @@ def test_pull_request_with_non_matching_webhook_rule(self, trigger_build, reques

@mock.patch("readthedocs.oauth.tasks.trigger_build")
def test_pull_request_without_webhook_rules(self, trigger_build):
"""Test that PR triggers build normally when no WebhookAutomationRules exist."""
"""Test that PR triggers build normally when no AutomationRules exist."""
self.project.external_builds_enabled = True
self.project.save()

Expand Down
Loading