Skip to content

Commit 976b572

Browse files
[feature] Flag invalid pull requests that do not reference validated issues #709
- Add pull_request_target edited/ready_for_review triggers - Implement regex issue references extraction helper - Add GraphQL project v2 board assignment check - Validate that PRs from external contributors link to open, labeled, and validated organization issues - Apply 'invalid' label and post warning comment if invalid - Automatically close invalid PRs after 24 hours in the stale PR workflow - Update documentation and add unit tests Closes #709
1 parent 9f3639a commit 976b572

8 files changed

Lines changed: 644 additions & 36 deletions

File tree

.github/actions/bot-autoassign/base.py

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ def __init__(self):
99
self.repository_name = os.environ.get("REPOSITORY")
1010
self.event_name = os.environ.get("GITHUB_EVENT_NAME")
1111
self.event_payload = None
12+
bot_username = os.environ.get("BOT_USERNAME", "openwisp-companion")
13+
self.bot_username = bot_username
14+
self.bot_login = (
15+
bot_username if bot_username.endswith("[bot]") else f"{bot_username}[bot]"
16+
)
1217

1318
if self.github_token and self.repository_name:
1419
try:
@@ -25,3 +30,223 @@ def __init__(self):
2530

2631
def load_event_payload(self, event_payload):
2732
self.event_payload = event_payload
33+
34+
def get_issue_projects(self, owner, repo_name, issue_number):
35+
query = """
36+
query($owner: String!, $repo: String!, $issueNumber: Int!) {
37+
repository(owner: $owner, name: $repo) {
38+
issue(number: $issueNumber) {
39+
projectItems(first: 10) {
40+
nodes {
41+
project {
42+
title
43+
}
44+
}
45+
}
46+
}
47+
}
48+
}
49+
"""
50+
variables = {"owner": owner, "repo": repo_name, "issueNumber": issue_number}
51+
headers, result = self.github.requester.graphql_query(query, variables)
52+
repo_data = result.get("data", {}).get("repository", {}) or {}
53+
issue_data = repo_data.get("issue", {}) or {}
54+
project_items = issue_data.get("projectItems", {}) or {}
55+
nodes = project_items.get("nodes", []) or []
56+
57+
return [
58+
node["project"]["title"]
59+
for node in nodes
60+
if node and node.get("project") and node["project"].get("title")
61+
]
62+
63+
def validate_pr_issues(self, pr):
64+
"""Validate if a pull request is from an exempt user or references a validated issue."""
65+
if not self.github or not self.repository_name:
66+
print("GitHub client or repository name not initialized")
67+
return False
68+
69+
pr_author = (
70+
pr.user.login
71+
if pr.user and isinstance(getattr(pr.user, "login", None), str)
72+
else ""
73+
)
74+
75+
# Check if the author is in the exclude list
76+
exclude_authors_env = os.environ.get("EXCLUDE_PR_AUTHORS", "dependabot[bot]")
77+
excluded_authors = [
78+
auth.strip() for auth in exclude_authors_env.split(",") if auth.strip()
79+
]
80+
if pr_author in excluded_authors:
81+
print(f"Author {pr_author} is in the exclude list. Proceeding.")
82+
return True
83+
84+
# Check if the author is a maintainer/collaborator
85+
author_association = getattr(pr, "author_association", None)
86+
if isinstance(author_association, str) and author_association in [
87+
"OWNER",
88+
"MEMBER",
89+
"COLLABORATOR",
90+
]:
91+
print(
92+
f"Author {pr_author} is exempt due to association: {author_association}. Proceeding."
93+
)
94+
return True
95+
96+
# For external contributors, extract linked issues
97+
from utils import extract_all_linked_issues
98+
99+
pr_body = pr.body if isinstance(pr.body, str) else ""
100+
linked_issues = extract_all_linked_issues(pr_body, self.repository_name)
101+
if not linked_issues:
102+
print("No linked issues found in PR body for external contributor.")
103+
return False
104+
105+
current_org = self.repository_name.split("/")[0].lower()
106+
required_projects = [
107+
"OpenWISP Contributor's Board",
108+
"OpenWISP Priorities for next releases",
109+
]
110+
111+
for owner, repo_name, issue_number in linked_issues:
112+
# 1. Check if the issue belongs to the same organization
113+
if owner.lower() != current_org:
114+
print(
115+
f"Issue {owner}/{repo_name}#{issue_number} does not belong "
116+
f"to organization {current_org}, skipping validation."
117+
)
118+
continue
119+
120+
try:
121+
from github import GithubException
122+
123+
target_repo = self.github.get_repo(f"{owner}/{repo_name}")
124+
issue = target_repo.get_issue(issue_number)
125+
except Exception as e:
126+
if isinstance(e, GithubException) and e.status == 404:
127+
print(
128+
f"Issue {owner}/{repo_name}#{issue_number} not found, skipping validation."
129+
)
130+
continue
131+
else:
132+
print(
133+
f"Error fetching issue {owner}/{repo_name}#{issue_number}: {e}"
134+
)
135+
raise
136+
137+
# 2. Check if the issue is a PR
138+
if issue.pull_request:
139+
print(
140+
f"Reference {owner}/{repo_name}#{issue_number} is a pull request, skipping validation."
141+
)
142+
continue
143+
144+
# 3. Check if the issue is open
145+
if issue.state != "open":
146+
print(
147+
f"Issue {owner}/{repo_name}#{issue_number} is not open, skipping validation."
148+
)
149+
continue
150+
151+
# 4. Check if the issue has at least one label, and none are invalid/wontfix
152+
issue_labels = [label.name.lower() for label in issue.labels]
153+
valid_labels = [
154+
lbl for lbl in issue_labels if lbl not in ["invalid", "wontfix"]
155+
]
156+
if not valid_labels:
157+
print(
158+
f"Issue {owner}/{repo_name}#{issue_number} has no valid labels, skipping validation."
159+
)
160+
continue
161+
if any(lbl in ["invalid", "wontfix"] for lbl in issue_labels):
162+
print(
163+
f"Issue {owner}/{repo_name}#{issue_number} contains "
164+
"invalid/wontfix label, skipping validation."
165+
)
166+
continue
167+
168+
# 5. Check if the issue is assigned to one of the required projects
169+
try:
170+
projects = self.get_issue_projects(owner, repo_name, issue_number)
171+
except Exception as e:
172+
print(
173+
f"Error fetching projects for issue {owner}/{repo_name}#{issue_number}: {e}"
174+
)
175+
raise
176+
177+
has_valid_project = any(
178+
project in required_projects for project in projects
179+
)
180+
if has_valid_project:
181+
print(
182+
f"Issue {owner}/{repo_name}#{issue_number} is validated. PR is valid."
183+
)
184+
return True
185+
else:
186+
print(
187+
f"Issue {owner}/{repo_name}#{issue_number} is not assigned "
188+
"to any required project, skipping validation."
189+
)
190+
191+
return False
192+
193+
def get_bot_comment(self, pr, comment_type, after_date=None, issue_comments=None):
194+
"""Get the comment of this bot with the given marker if it exists.
195+
196+
If ``after_date`` is provided, only considers comments posted after that date.
197+
"""
198+
try:
199+
if issue_comments is None:
200+
issue_comments = list(pr.get_issue_comments())
201+
marker = f"<!-- bot:{comment_type} -->"
202+
for comment in issue_comments:
203+
if (
204+
comment.user
205+
and comment.user.login == self.bot_login
206+
and marker in comment.body
207+
):
208+
if after_date and comment.created_at <= after_date:
209+
continue
210+
return comment
211+
return None
212+
except Exception as e:
213+
print(f"Error getting bot comment for PR #{pr.number}: {e}")
214+
return None
215+
216+
def has_bot_comment(self, pr, comment_type, after_date=None, issue_comments=None):
217+
"""Check if PR already has a specific type of bot comment.
218+
219+
Uses HTML markers. If ``after_date`` is provided,
220+
only considers comments posted after that date.
221+
"""
222+
return bool(self.get_bot_comment(pr, comment_type, after_date, issue_comments))
223+
224+
def get_invalid_unvalidated_issue_comment(self, pr_author):
225+
"""Returns the comment body warning that the PR is invalid/unvalidated."""
226+
greeting = f"Hi @{pr_author},\n\n" if pr_author else "Hi,\n\n"
227+
return (
228+
"<!-- bot:invalid_unvalidated_issue -->\n\n"
229+
f"{greeting}"
230+
"Thank you for your interest in contributing to OpenWISP.\n\n"
231+
"This pull request has been flagged because external contributors "
232+
"must target an issue validated by maintainers before requesting "
233+
"review.\n\n"
234+
"Please link this pull request to a validated issue by adding "
235+
"`Fixes #ISSUE_NUMBER`, `Closes #ISSUE_NUMBER`, or "
236+
"`Related to #ISSUE_NUMBER` to the pull request description. "
237+
"The issue may be in this repository or another OpenWISP "
238+
"repository.\n\n"
239+
"If there is no validated issue yet, please open one first and wait "
240+
"for maintainer validation before continuing with this pull "
241+
"request.\n\n"
242+
"An issue is considered validated when it is open, has an appropriate "
243+
"label other than `invalid` or `wontfix`, and is assigned to one of "
244+
"the OpenWISP contributor project boards mentioned in the "
245+
"contributing guidelines.\n\n"
246+
"Please see the OpenWISP policy on unsolicited and AI-assisted "
247+
"contributions:\n"
248+
"https://openwisp.io/docs/dev/general/code-of-conduct.html\n\n"
249+
"If this is not resolved within 24 hours, this pull request "
250+
"will be closed automatically. "
251+
"Thank you for your understanding."
252+
)

.github/actions/bot-autoassign/issue_assignment_bot.py

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import os
21
import re
32

43
from base import GitHubBot
@@ -14,9 +13,6 @@
1413

1514

1615
class IssueAssignmentBot(GitHubBot):
17-
def __init__(self):
18-
super().__init__()
19-
self.bot_username = os.environ.get("BOT_USERNAME", "openwisp-companion")
2016

2117
def is_bot_assign_command(self, comment_body):
2218
if not comment_body:
@@ -442,14 +438,40 @@ def handle_pull_request(self):
442438
return False
443439
if action in ["opened", "reopened"]:
444440
self.auto_assign_issues_from_pr(pr_number, pr_author, pr_body)
445-
# We consider the event handled even if no issues were linked
446-
return True
447441
elif action == "closed":
448442
if pr.get("merged", False):
449443
print(f"PR #{pr_number} was merged, keeping issue assignments")
450444
else:
451445
self.unassign_issues_from_pr(pr_body, pr_author)
452446
return True
447+
448+
if action in ["opened", "reopened", "edited", "ready_for_review"]:
449+
pr_obj = self.repo.get_pull(pr_number)
450+
is_valid = self.validate_pr_issues(pr_obj)
451+
labels = []
452+
try:
453+
labels = [label.name for label in pr_obj.labels]
454+
except (TypeError, AttributeError):
455+
pass
456+
457+
if is_valid:
458+
if "invalid" in labels:
459+
pr_obj.remove_from_labels("invalid")
460+
print(f"Removed 'invalid' label from PR #{pr_number}")
461+
else:
462+
if "invalid" not in labels:
463+
pr_obj.add_to_labels("invalid")
464+
print(f"Added 'invalid' label to PR #{pr_number}")
465+
466+
if not self.has_bot_comment(pr_obj, "invalid_unvalidated_issue"):
467+
comment_body = self.get_invalid_unvalidated_issue_comment(
468+
pr_author
469+
)
470+
pr_obj.create_issue_comment(comment_body)
471+
print(
472+
f"Posted unvalidated issue warning comment on PR #{pr_number}"
473+
)
474+
return True
453475
print(f"PR action '{action}' not handled")
454476
return True
455477
except Exception as e:

0 commit comments

Comments
 (0)