Skip to content

Commit 52e3df0

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 52e3df0

10 files changed

Lines changed: 687 additions & 38 deletions

File tree

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

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

.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)