Skip to content

Commit c320c05

Browse files
authored
gh-556: Fix over-reaching PR-edited event handler (#557)
gh-555 introduced the PR edited handler to handle draft PRs. The implementation was over-reaching, and triggered on any edited events (including renaming PR). This PR fixes that by removing the PR-edited handler and replacing it with two more specific handlers that more narrowly match the original intent, namely, respond to changes in the draft state of the PR (publish / unpublish). Fixes gh-556
1 parent 0f350ce commit c320c05

File tree

2 files changed

+71
-17
lines changed

2 files changed

+71
-17
lines changed

bedevere/stage.py

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,16 @@ async def stage(gh, issue, blocked_on):
100100
await gh.post(issue["labels_url"], data=[label_name])
101101

102102

103+
async def stage_for_review(gh, pull_request):
104+
"""Apply "awaiting review" label."""
105+
issue = await util.issue_for_PR(gh, pull_request)
106+
username = util.user_login(pull_request)
107+
if await util.is_core_dev(gh, username):
108+
await stage(gh, issue, Blocker.core_review)
109+
else:
110+
await stage(gh, issue, Blocker.review)
111+
112+
103113
@router.register("pull_request", action="opened")
104114
async def opened_pr(event, gh, *arg, **kwargs):
105115
"""Decide if a new pull request requires a review.
@@ -111,24 +121,20 @@ async def opened_pr(event, gh, *arg, **kwargs):
111121
pull_request = event.data["pull_request"]
112122
if pull_request.get("draft"):
113123
return
114-
issue = await util.issue_for_PR(gh, pull_request)
115-
username = util.user_login(pull_request)
116-
if await util.is_core_dev(gh, username):
117-
await stage(gh, issue, Blocker.core_review)
118-
else:
119-
await stage(gh, issue, Blocker.review)
124+
await stage_for_review(gh, pull_request)
120125

121126

122-
@router.register("pull_request", action="edited")
123-
async def edited_pr(event, gh, *arg, **kwargs):
127+
@router.register("pull_request", action="converted_to_draft")
128+
async def pr_converted_to_draft(event, gh, *arg, **kwargs):
124129
pull_request = event.data["pull_request"]
125130
issue = await util.issue_for_PR(gh, pull_request)
126-
username = util.user_login(pull_request)
127-
if pull_request.get("draft"):
128-
await _remove_stage_labels(gh, issue)
129-
else:
130-
blocked_on = Blocker.core_review if await util.is_core_dev(gh, username) else Blocker.review
131-
await stage(gh, issue, blocked_on)
131+
await _remove_stage_labels(gh, issue)
132+
133+
134+
@router.register("pull_request", action="ready_for_review")
135+
async def draft_pr_published(event, gh, *arg, **kwargs):
136+
pull_request = event.data["pull_request"]
137+
await stage_for_review(gh, pull_request)
132138

133139

134140
@router.register("push")

tests/test_stage.py

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ async def test_opened_draft_pr():
9999
assert len(gh.post_) == 0
100100

101101
# Draft PR is published
102-
data["action"] = "edited"
102+
data["action"] = "ready_for_review"
103103
data["pull_request"]["draft"] = False
104104
event = sansio.Event(data, event="pull_request", delivery_id="12345")
105105
gh = FakeGH(
@@ -111,8 +111,8 @@ async def test_opened_draft_pr():
111111
assert post_[0] == "https://api.github.com/labels"
112112
assert post_[1] == [awaiting.Blocker.core_review.value]
113113

114-
# Published PR is unpublished (set back to Draft)
115-
data["action"] = "edited"
114+
# Published PR is unpublished (converted to Draft)
115+
data["action"] = "converted_to_draft"
116116
data["pull_request"]["draft"] = True
117117
encoded_label = "awaiting%20core%20review"
118118
items[issue_url] = {
@@ -140,6 +140,54 @@ async def test_opened_draft_pr():
140140
)
141141

142142

143+
async def test_edited_pr_title():
144+
# regression test for https://github.com/python/bedevere/issues/556
145+
# test that editing the PR title doesn't change the Blocker labels
146+
username = "itamaro"
147+
issue_url = "https://api.github.com/issue/42"
148+
data = {
149+
"action": "edited",
150+
"pull_request": {
151+
"user": {
152+
"login": username,
153+
},
154+
"issue_url": issue_url,
155+
"draft": False,
156+
"title": "So long and thanks for all the fish",
157+
},
158+
"changes": {
159+
"title": "So long and thanks for all the phish",
160+
}
161+
}
162+
event = sansio.Event(data, event="pull_request", delivery_id="12345")
163+
teams = [{"name": "python core", "id": 6}]
164+
encoded_label = "awaiting%20review"
165+
items = {
166+
f"https://api.github.com/teams/6/memberships/{username}": gidgethub.BadRequest(
167+
status_code=http.HTTPStatus(404)
168+
),
169+
issue_url: {
170+
"labels": [
171+
{
172+
"url": f"https://api.github.com/repos/python/cpython/labels/{encoded_label}",
173+
"name": "awaiting review",
174+
},
175+
{
176+
"url": "https://api.github.com/repos/python/cpython/labels/CLA%20signed",
177+
"name": "CLA signed",
178+
},
179+
],
180+
"labels_url": "https://api.github.com/repos/python/cpython/issues/12345/labels{/name}",
181+
},
182+
}
183+
gh = FakeGH(
184+
getiter={"https://api.github.com/orgs/python/teams": teams}, getitem=items
185+
)
186+
await awaiting.router.dispatch(event, gh)
187+
assert len(gh.post_) == 0
188+
assert gh.delete_url is None
189+
190+
143191
async def test_opened_pr():
144192
# New PR from a core dev.
145193
username = "brettcannon"

0 commit comments

Comments
 (0)