feat: cancel pending pull review requests (Closes #1340)
This commit is contained in:
1 parent
7517d81046
commit
d776a2b003
6 files changed
+284
-1
No files matched your search
@@ -7662,6 +7662,45 @@ const item = {{repository:'stackchain/api',number:7}};
|
||||
}
|
||||
|
||||
|
||||
def test_pull_sheet_single_flights_pending_review_cancellation():
|
||||
script = f"""
|
||||
const createPullSheet = require({json.dumps(str(PULL_SHEET))});
|
||||
const calls = [];
|
||||
let finish;
|
||||
const controller = createPullSheet({{
|
||||
storage:null,
|
||||
fetchJson:(url, options={{}}) => {{
|
||||
calls.push({{url, method:options.method, body:JSON.parse(options.body)}});
|
||||
return new Promise(resolve => {{ finish = resolve; }});
|
||||
}}
|
||||
}});
|
||||
const notified = [];
|
||||
controller.onReviewCancelled = result => notified.push(result);
|
||||
const item = {{repository:'stackchain/api',number:7}};
|
||||
(async () => {{
|
||||
const first = controller.cancelReview(item, 'sam', 'abc1234');
|
||||
const duplicate = controller.cancelReview(item, 'sam', 'abc1234');
|
||||
finish({{reviewer:'sam',head_sha:'abc1234',requested_reviewers:['casey']}});
|
||||
const results = await Promise.all([first, duplicate]);
|
||||
process.stdout.write(JSON.stringify({{calls,results,same:first===duplicate,notified}}));
|
||||
}})();
|
||||
"""
|
||||
output = json.loads(subprocess.run(
|
||||
["node", "-e", script], check=True, capture_output=True, text=True
|
||||
).stdout)
|
||||
|
||||
assert output == {
|
||||
"calls": [{
|
||||
"url": "api/v1/repos/stackchain/api/pulls/7/request-review",
|
||||
"method": "DELETE",
|
||||
"body": {"reviewer": "sam", "expected_head_sha": "abc1234"},
|
||||
}],
|
||||
"results": [{"reviewer": "sam", "head_sha": "abc1234", "requested_reviewers": ["casey"]}] * 2,
|
||||
"same": True,
|
||||
"notified": [{"reviewer": "sam", "head_sha": "abc1234", "requested_reviewers": ["casey"]}],
|
||||
}
|
||||
|
||||
|
||||
def test_pull_sheet_exposes_touch_safe_review_request_flow():
|
||||
root = PULL_SHEET.parents[1]
|
||||
html = (root / "frontend" / "index.html").read_text()
|
||||
@@ -7772,6 +7811,36 @@ process.stdout.write(JSON.stringify({{
|
||||
assert "Approved current head" in output["html"]
|
||||
|
||||
|
||||
def test_pull_sheet_offers_cancel_only_for_pending_review_requests():
|
||||
script = f"""
|
||||
const createPullSheet = require({json.dumps(str(PULL_SHEET))});
|
||||
const html = createPullSheet.renderReviewerStatuses([
|
||||
{{login:'sam',status:'waiting'}},
|
||||
{{login:'casey',status:'approved'}},
|
||||
{{login:'lee',status:'commented'}},
|
||||
], value => String(value));
|
||||
process.stdout.write(html);
|
||||
"""
|
||||
html = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True).stdout
|
||||
|
||||
assert html.count('data-cancel-review-request=') == 1
|
||||
assert 'data-cancel-review-request="sam"' in html
|
||||
assert "Cancel request" in html
|
||||
assert 'data-cancel-review-request="casey"' not in html
|
||||
assert 'data-cancel-review-request="lee"' not in html
|
||||
|
||||
|
||||
def test_pull_sheet_pending_review_cancellation_is_touch_safe_and_opens_replacement_picker():
|
||||
root = PULL_SHEET.parents[1]
|
||||
css = (root / "frontend" / "dashboard.css").read_text()
|
||||
script = PULL_SHEET.read_text()
|
||||
|
||||
assert ".cancel-review-request { min-height:44px;" in css
|
||||
assert "controller.cancelReview(selected, reviewer, detail.head_sha)" in script
|
||||
assert "Choose a replacement reviewer." in script
|
||||
assert "qs('#pull-review-request').open = true" in script
|
||||
|
||||
|
||||
def test_pull_sheet_renders_bounded_feedback_grouped_by_file():
|
||||
script = f"""
|
||||
const createPullSheet = require({json.dumps(str(PULL_SHEET))});
|
||||
|
||||
@@ -970,6 +970,30 @@ async def test_assigned_pull_review_request_endpoints_list_and_submit(monkeypatc
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_assigned_pull_cancel_review_endpoint_forwards_head_scoped_request(monkeypatch):
|
||||
calls = []
|
||||
|
||||
async def cancel(repository, number, reviewer, expected_head_sha):
|
||||
calls.append((repository, number, reviewer, expected_head_sha))
|
||||
return {
|
||||
"repository": repository, "number": number, "reviewer": reviewer,
|
||||
"head_sha": expected_head_sha, "requested_reviewers": ["casey"],
|
||||
}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "cancel_assigned_pull_review", cancel, raising=False)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.request(
|
||||
"DELETE", "/api/v1/repos/stackchain/api/pulls/7/request-review",
|
||||
json={"reviewer": "sam", "expected_head_sha": "abc1234"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["requested_reviewers"] == ["casey"]
|
||||
assert calls == [("stackchain/api", 7, "sam", "abc1234")]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_gitea_pull_handoff_preserves_coassignees_and_confirms_ownership_exit():
|
||||
requests = []
|
||||
@@ -1090,6 +1114,73 @@ async def test_gitea_pull_review_request_filters_candidates_and_confirms_request
|
||||
assert mutation[2] == b'{"reviewers":["casey"]}'
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_gitea_cancel_pending_pull_review_confirms_authoritative_removal():
|
||||
requests = []
|
||||
cancelled = False
|
||||
|
||||
async def handler(request):
|
||||
nonlocal cancelled
|
||||
requests.append((request.method, request.url.path, request.content))
|
||||
if request.url.path.endswith("/user"):
|
||||
return httpx.Response(200, json={"login": "timmy"})
|
||||
if request.url.path.endswith("/pulls/7"):
|
||||
return httpx.Response(200, json={
|
||||
"number": 7, "state": "open", "merged": False,
|
||||
"head": {"sha": "abc123"}, "assignees": [{"login": "timmy"}],
|
||||
"requested_reviewers": [{"login": "casey"}] if cancelled else [{"login": "sam"}, {"login": "casey"}],
|
||||
})
|
||||
if request.method == "DELETE" and request.url.path.endswith("/requested_reviewers"):
|
||||
assert request.content == b'{"reviewers":["sam"]}'
|
||||
cancelled = True
|
||||
return httpx.Response(204)
|
||||
raise AssertionError(f"unexpected request: {request.method} {request.url.path}")
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
result = await gitea_proxy.cancel_assigned_pull_review(
|
||||
"stackchain/api", 7, "sam", "abc123"
|
||||
)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert result == {
|
||||
"repository": "stackchain/api", "number": 7, "head_sha": "abc123",
|
||||
"requested_reviewers": ["casey"], "reviewer": "sam",
|
||||
}
|
||||
assert [method for method, _path, _body in requests] == ["GET", "GET", "DELETE", "GET"]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_gitea_cancel_pending_pull_review_rejects_head_drift_before_mutation():
|
||||
requests = []
|
||||
|
||||
async def handler(request):
|
||||
requests.append(request.method)
|
||||
if request.url.path.endswith("/user"):
|
||||
return httpx.Response(200, json={"login": "timmy"})
|
||||
if request.url.path.endswith("/pulls/7"):
|
||||
return httpx.Response(200, json={
|
||||
"number": 7, "state": "open", "merged": False,
|
||||
"head": {"sha": "new-head"}, "assignees": [{"login": "timmy"}],
|
||||
"requested_reviewers": [{"login": "sam"}],
|
||||
})
|
||||
if request.method == "DELETE":
|
||||
return httpx.Response(204)
|
||||
raise AssertionError(request.url.path)
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
with pytest.raises(gitea_proxy.IssueNotAvailableError):
|
||||
await gitea_proxy.cancel_assigned_pull_review(
|
||||
"stackchain/api", 7, "sam", "old-head"
|
||||
)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert requests == ["GET", "GET"]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_gitea_pull_release_removes_current_login_case_insensitively():
|
||||
async def handler(request):
|
||||
|
||||
Reference in new issue
Block a user