stackchain/stackchain-dashboard
1
0
Fork 0
Code Issues 9 Pull Requests 1 Actions Packages Projects Releases 713 Wiki Activity

Request teammate review from mobile pull detail #1331

Merged
rockachopa merged 1 commits from timmy/1330-request-teammate-review into main 2026-08-24 02:21:20 +00:00
Conversation 0 Commits 1 Files Changed 8 +373 -3
8 changed files with 373 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
frontend/dashboard.css
View File

@ -958,11 +958,12 @@ textarea { resize: vertical; min-height: 120px; }
.issue-sheet-actions #watch-issue-detail, .pull-sheet-actions #watch-pull-detail { min-width:0; min-height:44px; }
.detail-watch-status:not(:empty) { margin-top:8px; overflow-wrap:anywhere; }
.issue-sheet-actions a { border:1px solid #60a5fa; border-radius:10px; font-weight:700; }
.issue-handoff, .pull-ownership { margin-top:14px; padding:12px; border:1px solid #2a496e; border-radius:12px; }
.issue-handoff > div, .pull-ownership > div { display:grid; gap:8px; margin-top:10px; }
.issue-handoff select, .pull-ownership select { width:100%; max-width:100%; padding:8px; border:1px solid #1f3a5f; border-radius:8px; background:#0b1526; color:var(--text); }
.issue-handoff, .pull-ownership, .pull-review-request { margin-top:14px; padding:12px; border:1px solid #2a496e; border-radius:12px; }
.issue-handoff > div, .pull-ownership > div, .pull-review-request > div { display:grid; gap:8px; margin-top:10px; }
.issue-handoff select, .pull-ownership select, .pull-review-request select { width:100%; max-width:100%; padding:8px; border:1px solid #1f3a5f; border-radius:8px; background:#0b1526; color:var(--text); }
.issue-handoff select, .issue-handoff button { min-height:44px; }
.pull-ownership select, .pull-ownership button { min-height:44px; }
.pull-review-request select, .pull-review-request button { min-height:44px; }
.issue-retry { min-height:44px; width:100%; margin-top:10px; }
.new-issue { min-height:44px; }
.find-work-action { min-height:44px; }

1
frontend/dashboard.js
View File

@ -4659,6 +4659,7 @@
const detail = offlineDetail || await pullController.load(item);
if (selectedPull !== item) return;
selectedPullDetail = detail;
pullController.setReviewDetail(detail);
pullConversation = pullController.conversation(item, detail.conversation);
qs('#pull-sheet-title').textContent = detail.title || 'Assigned pull request';
qs('#pull-sheet-body').innerHTML = renderMarkdown(detail.body || 'No description provided.');

10
frontend/index.html
View File

@ -1720,6 +1720,16 @@
<div id="pull-files"></div>
<button id="merge-pull" type="button" disabled>Merge</button>
</details>
<details class="pull-review-request" id="pull-review-request">
<summary>Request review</summary>
<div>
<button id="load-pull-reviewers" type="button">Choose reviewer</button>
<label for="pull-review-recipient" class="small">Eligible repository reviewer</label>
<select id="pull-review-recipient" disabled><option value="">Select a teammate</option></select>
<button id="confirm-pull-review" type="button" disabled>Request review</button>
<div id="pull-review-request-status" class="small" aria-live="assertive">Load teammates to request a review.</div>
</div>
</details>
<details class="pull-ownership" id="pull-ownership">
<summary>Ownership</summary>
<div>

83
frontend/pull-sheet.js
View File

@ -101,7 +101,70 @@ function resetOwnershipControls(doc, item, checkpointed) {
qs('#pull-handoff-status').textContent = 'Load teammates to transfer ownership.';
}
function resetReviewRequestControls(doc, detail) {
const qs = selector => doc.querySelector(selector);
qs('#pull-review-request').open = false;
qs('#pull-review-request').dataset.headSha = detail?.head_sha || '';
qs('#pull-review-recipient').innerHTML = '<option value="">Select a teammate</option>';
qs('#pull-review-recipient').disabled = true;
qs('#confirm-pull-review').disabled = true;
qs('#load-pull-reviewers').disabled = !detail?.head_sha;
qs('#pull-review-request-status').textContent = detail?.head_sha ?
'Load teammates to request a review.' : 'Load the current pull request before requesting review.';
}
function bindReviewRequestControls(doc, controller, getSelected) {
const qs = selector => doc.querySelector(selector);
const load = qs('#load-pull-reviewers');
if (load.dataset.reviewRequestBound === 'true') return;
load.dataset.reviewRequestBound = 'true';
controller.setReviewDetail = detail => resetReviewRequestControls(doc, detail);
resetReviewRequestControls(doc, null);
load.addEventListener('click', async () => {
const selected = getSelected();
if (!selected) return;
const select = qs('#pull-review-recipient');
load.disabled = true;
qs('#pull-review-request-status').textContent = 'Loading eligible reviewers…';
try {
const count = renderHandoffCandidates(select, await controller.loadReviewCandidates(selected), doc);
select.disabled = !count;
qs('#confirm-pull-review').disabled = true;
qs('#pull-review-request-status').textContent = count ?
'Choose a teammate to review the current head.' : 'No eligible reviewers were found.';
if (count) select.focus();
else load.disabled = false;
} catch (error) {
qs('#pull-review-request-status').textContent = error.message + ' Retry loading reviewers.';
load.disabled = false;
load.focus();
}
});
qs('#pull-review-recipient').addEventListener('change', event => {
qs('#confirm-pull-review').disabled = !event.target.value;
});
qs('#confirm-pull-review').addEventListener('click', async () => {
const selected = getSelected();
const detail = { head_sha: qs('#pull-review-request').dataset.headSha };
const reviewer = qs('#pull-review-recipient').value;
if (!selected || !detail?.head_sha || !reviewer ||
!globalThis.confirm('Request review of ' + selected.key + ' at ' + detail.head_sha.slice(0, 8) + ' from @' + reviewer + '?')) return;
const button = qs('#confirm-pull-review');
button.disabled = true;
qs('#pull-review-request-status').textContent = 'Requesting review…';
try {
await controller.requestReview(selected, reviewer, detail.head_sha);
qs('#pull-review-request-status').textContent = 'Review requested from @' + reviewer + '.';
} catch (error) {
qs('#pull-review-request-status').textContent = error.message + ' Selection kept; retry the request.';
button.disabled = false;
button.focus();
}
});
}
function bindOwnershipControls(doc, controller, getSelected, finish) {
bindReviewRequestControls(doc, controller, getSelected);
const qs = selector => doc.querySelector(selector);
const load = qs('#load-pull-handoff');
if (load.dataset.ownershipBound === 'true') return;
@ -172,6 +235,8 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT
let checkRequest = null;
let ownershipRequest = null;
let candidateRequest = null;
let reviewCandidateRequest = null;
let reviewRequestMutation = null;
const reviewRequests = new Map();
const reviewCache = new Map();
const pathFor = item => 'api/v1/repos/' + String(item.repository || '').split('/')
@ -224,6 +289,22 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT
}).finally(() => { candidateRequest = null; });
return candidateRequest;
},
loadReviewCandidates(item) {
if (reviewCandidateRequest) return reviewCandidateRequest;
reviewCandidateRequest = fetchJson(pathFor(item) + '/review-candidates', {
headers: { Accept: 'application/json' },
}).finally(() => { reviewCandidateRequest = null; });
return reviewCandidateRequest;
},
requestReview(item, reviewer, expectedHeadSha) {
if (reviewRequestMutation) return reviewRequestMutation;
reviewRequestMutation = fetchJson(pathFor(item) + '/request-review', {
method: 'POST',
headers: { Accept: 'application/json', 'Content-Type': 'application/json' },
body: JSON.stringify({ reviewer, expected_head_sha: expectedHeadSha }),
}).finally(() => { reviewRequestMutation = null; });
return reviewRequestMutation;
},
handoff(item, recipient) {
if (ownershipRequest) return ownershipRequest;
ownershipRequest = fetchJson(pathFor(item) + '/handoff', {
@ -323,4 +404,6 @@ createPullSheet.renderHandoffCandidates = renderHandoffCandidates;
createPullSheet.ownershipExitMessage = ownershipExitMessage;
createPullSheet.resetOwnershipControls = resetOwnershipControls;
createPullSheet.bindOwnershipControls = bindOwnershipControls;
createPullSheet.resetReviewRequestControls = resetReviewRequestControls;
createPullSheet.bindReviewRequestControls = bindReviewRequestControls;
if (typeof module !== 'undefined' && module.exports) module.exports = createPullSheet;

66
src/gitea_proxy.py
View File

@ -1962,6 +1962,72 @@ async def pull_handoff_candidates(repository: str) -> list[dict]:
][:25]
async def pull_review_candidates(repository: str, number: int) -> list[dict]:
pull, candidates = await asyncio.gather(
fetch(f"repos/{repository}/pulls/{number}"),
pull_handoff_candidates(repository),
)
if not isinstance(pull, dict):
raise ValueError("Gitea pull request response was not an object")
excluded = {
item["login"].casefold()
for item in (pull.get("requested_reviewers") or [])
if isinstance(item, dict) and isinstance(item.get("login"), str)
}
author = pull.get("user") if isinstance(pull.get("user"), dict) else {}
if isinstance(author.get("login"), str):
excluded.add(author["login"].casefold())
return [
item for item in candidates
if item["login"].casefold() not in excluded
][:25]
async def request_assigned_pull_review(
repository: str, number: int, reviewer: str, expected_head_sha: str
) -> dict:
login, pull = await _current_login_and_target(
f"repos/{repository}/pulls/{number}"
)
head = pull.get("head") if isinstance(pull.get("head"), dict) else {}
if (
pull.get("state") != "open"
or pull.get("merged") is True
or not _login_in_users(login, pull.get("assignees"))
or head.get("sha") != expected_head_sha
):
raise IssueNotAvailableError("Pull request is no longer eligible for review request")
eligible = {
item["login"] for item in await pull_review_candidates(repository, number)
}
if reviewer not in eligible:
raise IssueNotAvailableError("Reviewer is not eligible")
response = await _get_client().post(
f"/api/v1/repos/{repository}/pulls/{number}/requested_reviewers",
headers=_auth(),
json={"reviewers": [reviewer]},
)
response.raise_for_status()
confirmed_response = await _get_client().get(
f"/api/v1/repos/{repository}/pulls/{number}", headers=_auth()
)
confirmed_response.raise_for_status()
confirmed = confirmed_response.json()
requested = [
item["login"] for item in (confirmed.get("requested_reviewers") or [])
if isinstance(item, dict) and isinstance(item.get("login"), str)
] if isinstance(confirmed, dict) else []
if reviewer not in requested:
raise ValueError("Gitea did not confirm the requested reviewer")
return {
"repository": repository,
"number": number,
"head_sha": expected_head_sha,
"requested_reviewers": requested,
"reviewer": reviewer,
}
async def _change_assigned_pull_owners(
repository: str, number: int, recipient: str | None
) -> dict:

60
src/main.py
View File

@ -1108,6 +1108,15 @@ class IssueHandoff(BaseModel):
)
class PullReviewRequest(BaseModel):
reviewer: str = Field(
min_length=1, max_length=255, pattern=r"^[A-Za-z0-9_.-]+$"
)
expected_head_sha: str = Field(
min_length=7, max_length=64, pattern=r"^[A-Fa-f0-9]+$"
)
class IssueReassignment(IssueHandoff):
expected_assignees: list[str] = Field(min_length=1, max_length=10)
@ -6662,6 +6671,57 @@ async def handoff_assigned_pull(
return JSONResponse(result)
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/review-candidates")
async def pull_review_candidates(
owner: str, repo: str, number: int = PathParam(gt=0)
) -> JSONResponse:
repository = f"{owner}/{repo}"
try:
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
result = await asyncio.wait_for(
gitea_proxy.pull_review_candidates(repository, number),
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
)
except HTTPException:
raise
except Exception:
return JSONResponse(
{"error": "Reviewers could not be loaded. Please retry."},
status_code=503,
headers={"Retry-After": "1"},
)
return JSONResponse(result, headers={"Cache-Control": "no-store"})
@app.post("/api/v1/repos/{owner}/{repo}/pulls/{number}/request-review")
async def request_assigned_pull_review(
request: PullReviewRequest,
owner: str,
repo: str,
number: int = PathParam(gt=0),
) -> JSONResponse:
try:
result = await asyncio.wait_for(
gitea_proxy.request_assigned_pull_review(
f"{owner}/{repo}", number, request.reviewer, request.expected_head_sha
),
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
)
except gitea_proxy.IssueNotAvailableError:
return JSONResponse(
{"error": "The pull request or reviewer changed. Reload before requesting review."},
status_code=409,
)
except Exception:
return JSONResponse(
{"error": "The review request could not be confirmed. Please retry."},
status_code=503,
headers={"Retry-After": "1"},
)
return JSONResponse(result)
@app.patch("/api/v1/repos/{owner}/{repo}/pulls/{number}/release")
async def release_assigned_pull(
owner: str, repo: str, number: int = PathParam(gt=0)

59
tests/test_my_work.py
View File

@ -7539,6 +7539,65 @@ const item = {{repository:'stackchain/api',number:7}};
assert output["released"] == {"assignees": []}
def test_pull_sheet_loads_reviewers_and_single_flights_review_request():
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 || 'GET', body:options.body ? JSON.parse(options.body) : null}});
if (url.endsWith('/review-candidates')) return Promise.resolve([{{login:'casey',name:'Casey'}}]);
return new Promise(resolve => {{ finish = resolve; }});
}}
}});
const item = {{repository:'stackchain/api',number:7}};
(async () => {{
const candidates = await controller.loadReviewCandidates(item);
const first = controller.requestReview(item, 'casey', 'abc1234');
const duplicate = controller.requestReview(item, 'casey', 'abc1234');
finish({{reviewer:'casey',requested_reviewers:['sam','casey']}});
const results = await Promise.all([first, duplicate]);
process.stdout.write(JSON.stringify({{calls,candidates,results,same:first===duplicate}}));
}})();
"""
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/review-candidates", "method": "GET", "body": None},
{"url": "api/v1/repos/stackchain/api/pulls/7/request-review", "method": "POST", "body": {
"reviewer": "casey", "expected_head_sha": "abc1234"
}},
],
"candidates": [{"login": "casey", "name": "Casey"}],
"results": [
{"reviewer": "casey", "requested_reviewers": ["sam", "casey"]},
{"reviewer": "casey", "requested_reviewers": ["sam", "casey"]},
],
"same": True,
}
def test_pull_sheet_exposes_touch_safe_review_request_flow():
root = PULL_SHEET.parents[1]
html = (root / "frontend" / "index.html").read_text()
css = (root / "frontend" / "dashboard.css").read_text()
dashboard = (root / "frontend" / "dashboard.js").read_text()
assert 'id="pull-review-request"' in html
assert 'id="load-pull-reviewers"' in html
assert 'id="pull-review-recipient"' in html
assert 'id="confirm-pull-review"' in html
assert 'id="pull-review-request-status" class="small" aria-live="assertive"' in html
assert '.pull-review-request select, .pull-review-request button { min-height:44px;' in css
assert "createPullSheet.bindOwnershipControls(document, pullController" in dashboard
assert "pullController.setReviewDetail(detail)" in dashboard
def test_pull_sheet_surfaces_pending_merge_confirmation_guidance():
script = f"""
const createPullSheet = require({json.dumps(str(PULL_SHEET))});

90
tests/test_pull_api.py
View File

@ -640,6 +640,45 @@ async def test_assigned_pull_ownership_endpoints_list_handoff_and_release(monkey
]
@pytest.mark.anyio
async def test_assigned_pull_review_request_endpoints_list_and_submit(monkeypatch):
calls = []
async def assigned(repository, number):
calls.append(("assigned", repository, number))
return True
async def candidates(repository, number):
calls.append(("candidates", repository, number))
return [{"login": "casey", "name": "Casey"}]
async def request_review(repository, number, reviewer, expected_head_sha):
calls.append(("request", repository, number, reviewer, expected_head_sha))
return {"number": number, "reviewer": reviewer, "requested_reviewers": [reviewer]}
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
monkeypatch.setattr(main.gitea_proxy, "pull_review_candidates", candidates, raising=False)
monkeypatch.setattr(main.gitea_proxy, "request_assigned_pull_review", request_review, raising=False)
transport = httpx.ASGITransport(app=main.app)
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
listed = await client.get("/api/v1/repos/stackchain/api/pulls/7/review-candidates")
requested = await client.post(
"/api/v1/repos/stackchain/api/pulls/7/request-review",
json={"reviewer": "casey", "expected_head_sha": "abc1234"},
)
assert listed.status_code == 200
assert listed.headers["cache-control"] == "no-store"
assert listed.json() == [{"login": "casey", "name": "Casey"}]
assert requested.status_code == 200
assert requested.json()["requested_reviewers"] == ["casey"]
assert calls == [
("assigned", "stackchain/api", 7),
("candidates", "stackchain/api", 7),
("request", "stackchain/api", 7, "casey", "abc1234"),
]
@pytest.mark.anyio
async def test_gitea_pull_handoff_preserves_coassignees_and_confirms_ownership_exit():
requests = []
@ -709,6 +748,57 @@ async def test_pull_handoff_candidates_are_bounded_and_exclude_invalid_or_curren
]
@pytest.mark.anyio
async def test_gitea_pull_review_request_filters_candidates_and_confirms_requested_reviewer():
requests = []
review_requested = False
async def handler(request):
nonlocal review_requested
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("/assignees"):
return httpx.Response(200, json=[
{"login": "timmy", "full_name": "Timmy"},
{"login": "alex", "full_name": "Alexander"},
{"login": "sam", "full_name": "Sam"},
{"login": "casey", "full_name": "Casey"},
])
if request.url.path.endswith("/pulls/7"):
requested = [{"login": "sam"}]
if review_requested:
requested.append({"login": "casey"})
return httpx.Response(200, json={
"number": 7, "state": "open", "merged": False,
"head": {"sha": "abc123"}, "user": {"login": "alex"},
"assignees": [{"login": "timmy"}],
"requested_reviewers": requested,
})
if request.method == "POST" and request.url.path.endswith("/requested_reviewers"):
review_requested = True
return httpx.Response(201, json={})
raise AssertionError(f"unexpected request: {request.method} {request.url.path}")
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
try:
candidates = await gitea_proxy.pull_review_candidates("stackchain/api", 7)
result = await gitea_proxy.request_assigned_pull_review(
"stackchain/api", 7, "casey", "abc123"
)
finally:
await gitea_proxy.stop_client()
assert candidates == [{"login": "casey", "name": "Casey"}]
assert result == {
"repository": "stackchain/api", "number": 7, "head_sha": "abc123",
"requested_reviewers": ["sam", "casey"], "reviewer": "casey",
}
mutation = next(item for item in requests if item[0] == "POST")
assert mutation[1] == "/api/v1/repos/stackchain/api/pulls/7/requested_reviewers"
assert mutation[2] == b'{"reviewers":["casey"]}'
@pytest.mark.anyio
async def test_gitea_pull_release_removes_current_login_case_insensitively():
async def handler(request):