Submit pull request reviews from mobile My Work #156
|
|
@ -15,8 +15,10 @@ python3 -m pip install -r requirements.txt
|
||||||
|
|
||||||
Point the dashboard at the Gitea server root (without `/api/v1`) and provide a
|
Point the dashboard at the Gitea server root (without `/api/v1`) and provide a
|
||||||
token that can read dashboard data, update the authenticated user's notification
|
token that can read dashboard data, update the authenticated user's notification
|
||||||
threads, and create issue comments. Pull-request replies use Gitea's issue-comment
|
threads, create issue comments, and submit pull-request reviews. Pull-request
|
||||||
API. Serve the dashboard only to trusted users on its own origin; cross-origin API
|
replies use Gitea's issue-comment API; native Comment, Approve, and Request changes
|
||||||
|
reviews require repository write permission. Serve the dashboard only to trusted
|
||||||
|
users on its own origin; cross-origin API
|
||||||
access is intentionally disabled. Then start the API and bundled frontend:
|
access is intentionally disabled. Then start the API and bundled frontend:
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
|
|
|
||||||
|
|
@ -88,7 +88,7 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
.review-feedback { display:grid; gap:8px; }
|
.review-feedback { display:grid; gap:8px; }
|
||||||
.review-feedback label { display:grid; gap:6px; }
|
.review-feedback label { display:grid; gap:6px; }
|
||||||
.review-feedback select { min-height:44px; padding:8px; border-radius:8px; border:1px solid #1f3a5f; background:#0b1526; color:#e5e7eb; }
|
.review-feedback select { min-height:44px; padding:8px; border-radius:8px; border:1px solid #1f3a5f; background:#0b1526; color:#e5e7eb; }
|
||||||
.review-handoff { position:sticky; bottom:0; z-index:3; display:grid; gap:8px; padding:10px 4px; background:rgba(11,21,38,.98); border-top:1px solid #2a496e; }
|
.review-handoff { position:sticky; bottom:0; z-index:3; display:grid; gap:8px; padding:10px 4px; padding-bottom:calc(10px + env(safe-area-inset-bottom)); background:rgba(11,21,38,.98); border-top:1px solid #2a496e; }
|
||||||
.review-handoff button { min-height:44px; }
|
.review-handoff button { min-height:44px; }
|
||||||
.review-handoff-link { min-height:44px; display:flex; align-items:center; justify-content:center; border:1px solid #60a5fa; border-radius:8px; color:#bfdbfe; font-weight:700; text-decoration:none; }
|
.review-handoff-link { min-height:44px; display:flex; align-items:center; justify-content:center; border:1px solid #60a5fa; border-radius:8px; color:#bfdbfe; font-weight:700; text-decoration:none; }
|
||||||
.review-handoff-link[hidden] { display:none; }
|
.review-handoff-link[hidden] { display:none; }
|
||||||
|
|
@ -318,6 +318,8 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
<textarea id="review-summary" placeholder="Summarize your review"></textarea>
|
<textarea id="review-summary" placeholder="Summarize your review"></textarea>
|
||||||
</label>
|
</label>
|
||||||
<div class="review-handoff">
|
<div class="review-handoff">
|
||||||
|
<button id="submit-review">Submit review</button>
|
||||||
|
<span class="small" id="review-submit-status" aria-live="assertive"></span>
|
||||||
<button id="copy-review-feedback">Copy feedback & continue in Gitea</button>
|
<button id="copy-review-feedback">Copy feedback & continue in Gitea</button>
|
||||||
<span class="small" id="review-handoff-status" aria-live="polite"></span>
|
<span class="small" id="review-handoff-status" aria-live="polite"></span>
|
||||||
<textarea class="review-copy-fallback" id="review-copy-fallback" readonly hidden aria-label="Feedback to copy manually"></textarea>
|
<textarea class="review-copy-fallback" id="review-copy-fallback" readonly hidden aria-label="Feedback to copy manually"></textarea>
|
||||||
|
|
@ -383,6 +385,7 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
let progress = null;
|
let progress = null;
|
||||||
let draft = null;
|
let draft = null;
|
||||||
let reviewFiles = [];
|
let reviewFiles = [];
|
||||||
|
let selectedReviewHead = '';
|
||||||
let bulkConfirmationPending = false;
|
let bulkConfirmationPending = false;
|
||||||
let bulkMarkPending = false;
|
let bulkMarkPending = false;
|
||||||
let reviewHandoffPending = false;
|
let reviewHandoffPending = false;
|
||||||
|
|
@ -717,8 +720,11 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
qs('#review-handoff-link').hidden = true;
|
qs('#review-handoff-link').hidden = true;
|
||||||
qs('#review-handoff-link').href = item.url;
|
qs('#review-handoff-link').href = item.url;
|
||||||
qs('#review-handoff-status').textContent = '';
|
qs('#review-handoff-status').textContent = '';
|
||||||
|
qs('#review-submit-status').textContent = '';
|
||||||
|
qs('#submit-review').disabled = true;
|
||||||
draft = null;
|
draft = null;
|
||||||
reviewFiles = [];
|
reviewFiles = [];
|
||||||
|
selectedReviewHead = '';
|
||||||
qs('#close-review-sheet').focus();
|
qs('#close-review-sheet').focus();
|
||||||
try {
|
try {
|
||||||
const detail = await reviewController.load(selectedReview);
|
const detail = await reviewController.load(selectedReview);
|
||||||
|
|
@ -742,6 +748,7 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
files: detail.files || [],
|
files: detail.files || [],
|
||||||
});
|
});
|
||||||
reviewFiles = detail.files || [];
|
reviewFiles = detail.files || [];
|
||||||
|
selectedReviewHead = detail.head_sha || '';
|
||||||
draft = createReviewController.createDraft({
|
draft = createReviewController.createDraft({
|
||||||
storage: localStorage,
|
storage: localStorage,
|
||||||
repository: item.repository,
|
repository: item.repository,
|
||||||
|
|
@ -771,6 +778,7 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
escapeHtml(review.state || 'commented') + (review.body ? '<div class="small">' + escapeHtml(review.body) + '</div>' : '') + '</div>'
|
escapeHtml(review.state || 'commented') + (review.body ? '<div class="small">' + escapeHtml(review.body) + '</div>' : '') + '</div>'
|
||||||
).join('') : '<div>No prior reviews.</div>';
|
).join('') : '<div>No prior reviews.</div>';
|
||||||
qs('#review-sheet-status').textContent = 'Ready to review · by ' + (detail.author || 'unknown author');
|
qs('#review-sheet-status').textContent = 'Ready to review · by ' + (detail.author || 'unknown author');
|
||||||
|
qs('#submit-review').disabled = false;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (selectedReview !== item) return;
|
if (selectedReview !== item) return;
|
||||||
qs('#review-sheet-status').textContent = error.message + ' Retry here or use Open in Gitea.';
|
qs('#review-sheet-status').textContent = error.message + ' Retry here or use Open in Gitea.';
|
||||||
|
|
@ -785,6 +793,7 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
progress = null;
|
progress = null;
|
||||||
draft = null;
|
draft = null;
|
||||||
reviewFiles = [];
|
reviewFiles = [];
|
||||||
|
selectedReviewHead = '';
|
||||||
if (reviewTrigger?.isConnected) reviewTrigger.focus();
|
if (reviewTrigger?.isConnected) reviewTrigger.focus();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -978,6 +987,37 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
});
|
});
|
||||||
qs('#review-summary').addEventListener('input', event => draft?.setSummary(event.target.value));
|
qs('#review-summary').addEventListener('input', event => draft?.setSummary(event.target.value));
|
||||||
qs('#review-decision').addEventListener('change', event => draft?.setDecision(event.target.value));
|
qs('#review-decision').addEventListener('change', event => draft?.setDecision(event.target.value));
|
||||||
|
qs('#submit-review').addEventListener('click', async () => {
|
||||||
|
if (!draft || !selectedReview || !selectedReviewHead) return;
|
||||||
|
const snapshot = draft.snapshot();
|
||||||
|
const labels = { approve: 'Approve', request_changes: 'Request changes' };
|
||||||
|
if (labels[snapshot.decision] && !window.confirm(
|
||||||
|
labels[snapshot.decision] + ' ' + selectedReview.repository + '#' + selectedReview.number + '?'
|
||||||
|
)) return;
|
||||||
|
const button = qs('#submit-review');
|
||||||
|
button.disabled = true;
|
||||||
|
qs('#review-submit-status').textContent = 'Submitting review…';
|
||||||
|
try {
|
||||||
|
const result = await reviewController.submit(selectedReview, {
|
||||||
|
decision: snapshot.decision,
|
||||||
|
body: createReviewController.formatFeedback(snapshot, reviewFiles),
|
||||||
|
expected_head_sha: selectedReviewHead,
|
||||||
|
});
|
||||||
|
draft.clear();
|
||||||
|
progress?.clear();
|
||||||
|
qs('#review-decision').value = 'comment';
|
||||||
|
qs('#review-summary').value = '';
|
||||||
|
document.querySelectorAll('.review-note').forEach(note => { note.value = ''; });
|
||||||
|
if (progress) showReviewProgress(progress.snapshot());
|
||||||
|
qs('#review-submit-status').textContent = 'Review submitted · ' + (result.state || 'complete') + '.';
|
||||||
|
await load();
|
||||||
|
button.focus();
|
||||||
|
} catch (error) {
|
||||||
|
qs('#review-submit-status').textContent = error.message + ' Your draft is safe; retry or open in Gitea.';
|
||||||
|
button.disabled = false;
|
||||||
|
button.focus();
|
||||||
|
}
|
||||||
|
});
|
||||||
qs('#copy-review-feedback').addEventListener('click', async () => {
|
qs('#copy-review-feedback').addEventListener('click', async () => {
|
||||||
if (!draft || !selectedReview || reviewHandoffPending) return;
|
if (!draft || !selectedReview || reviewHandoffPending) return;
|
||||||
reviewHandoffPending = true;
|
reviewHandoffPending = true;
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,6 @@
|
||||||
function createReviewController({ fetchJson }) {
|
function createReviewController({ fetchJson }) {
|
||||||
|
let pendingSubmission = null;
|
||||||
|
|
||||||
function endpoint(item) {
|
function endpoint(item) {
|
||||||
const [owner, repo] = String(item.repository || '').split('/');
|
const [owner, repo] = String(item.repository || '').split('/');
|
||||||
if (!owner || !repo || !Number.isInteger(Number(item.number))) {
|
if (!owner || !repo || !Number.isInteger(Number(item.number))) {
|
||||||
|
|
@ -12,7 +14,17 @@ function createReviewController({ fetchJson }) {
|
||||||
return fetchJson(endpoint(item), { headers: { Accept: 'application/json' } });
|
return fetchJson(endpoint(item), { headers: { Accept: 'application/json' } });
|
||||||
}
|
}
|
||||||
|
|
||||||
return { load };
|
function submit(item, payload) {
|
||||||
|
if (pendingSubmission) return pendingSubmission;
|
||||||
|
pendingSubmission = fetchJson(endpoint(item), {
|
||||||
|
method: 'POST',
|
||||||
|
headers: { Accept: 'application/json', 'Content-Type': 'application/json' },
|
||||||
|
body: JSON.stringify(payload),
|
||||||
|
}).finally(() => { pendingSubmission = null; });
|
||||||
|
return pendingSubmission;
|
||||||
|
}
|
||||||
|
|
||||||
|
return { load, submit };
|
||||||
}
|
}
|
||||||
|
|
||||||
function diffLineClass(line) {
|
function diffLineClass(line) {
|
||||||
|
|
@ -90,7 +102,14 @@ function createProgress({ storage, repository, number, headSha, files }) {
|
||||||
return snapshot();
|
return snapshot();
|
||||||
}
|
}
|
||||||
|
|
||||||
return { snapshot, markReviewed };
|
function clear() {
|
||||||
|
reviewed = [];
|
||||||
|
newHead = false;
|
||||||
|
try { storage.removeItem(key); } catch (error) { /* cleared in memory */ }
|
||||||
|
return snapshot();
|
||||||
|
}
|
||||||
|
|
||||||
|
return { snapshot, markReviewed, clear };
|
||||||
}
|
}
|
||||||
|
|
||||||
function createDraft({ storage, repository, number, headSha, files }) {
|
function createDraft({ storage, repository, number, headSha, files }) {
|
||||||
|
|
@ -140,7 +159,13 @@ function createDraft({ storage, repository, number, headSha, files }) {
|
||||||
return snapshot();
|
return snapshot();
|
||||||
}
|
}
|
||||||
|
|
||||||
return { snapshot, setNote, setSummary, setDecision };
|
function clear() {
|
||||||
|
draft = { notes: {}, summary: '', decision: 'comment' };
|
||||||
|
try { storage.removeItem(key); } catch (error) { /* cleared in memory */ }
|
||||||
|
return snapshot();
|
||||||
|
}
|
||||||
|
|
||||||
|
return { snapshot, setNote, setSummary, setDecision, clear };
|
||||||
}
|
}
|
||||||
|
|
||||||
function formatFeedback(draft, files) {
|
function formatFeedback(draft, files) {
|
||||||
|
|
|
||||||
|
|
@ -14,6 +14,10 @@ REVIEW_DIFF_MAX_LINES = 400
|
||||||
_client: httpx.AsyncClient | None = None
|
_client: httpx.AsyncClient | None = None
|
||||||
|
|
||||||
|
|
||||||
|
class StaleReviewError(ValueError):
|
||||||
|
"""Raised before mutation when a pull request head changed during review."""
|
||||||
|
|
||||||
|
|
||||||
def _auth() -> dict[str, str]:
|
def _auth() -> dict[str, str]:
|
||||||
headers: dict[str, str] = {"Accept": "application/json"}
|
headers: dict[str, str] = {"Accept": "application/json"}
|
||||||
if GITEA_TOKEN:
|
if GITEA_TOKEN:
|
||||||
|
|
@ -416,6 +420,43 @@ async def pull_review_detail(repository: str, number: int) -> dict:
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
async def submit_pull_review(
|
||||||
|
repository: str,
|
||||||
|
number: int,
|
||||||
|
expected_head_sha: str,
|
||||||
|
decision: str,
|
||||||
|
body: str,
|
||||||
|
) -> dict:
|
||||||
|
base = f"repos/{repository}/pulls/{number}"
|
||||||
|
pull = await fetch(base)
|
||||||
|
head = pull.get("head") if isinstance(pull, dict) else None
|
||||||
|
current_sha = head.get("sha") if isinstance(head, dict) else None
|
||||||
|
if current_sha != expected_head_sha:
|
||||||
|
raise StaleReviewError("Pull request changed while it was being reviewed")
|
||||||
|
response = await _get_client().post(
|
||||||
|
f"/api/v1/{base}/reviews",
|
||||||
|
headers=_auth(),
|
||||||
|
json={
|
||||||
|
"body": body,
|
||||||
|
"event": {
|
||||||
|
"comment": "COMMENT",
|
||||||
|
"approve": "APPROVE",
|
||||||
|
"request_changes": "REQUEST_CHANGES",
|
||||||
|
}[decision],
|
||||||
|
"commit_id": current_sha,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
response.raise_for_status()
|
||||||
|
review = response.json()
|
||||||
|
if not isinstance(review, dict):
|
||||||
|
raise ValueError("Gitea review response was not an object")
|
||||||
|
return {
|
||||||
|
"id": review.get("id"),
|
||||||
|
"state": review.get("state") or "COMMENT",
|
||||||
|
"url": _safe_web_url(review.get("html_url")),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
async def activity_events(user: dict | None = None) -> list[dict]:
|
async def activity_events(user: dict | None = None) -> list[dict]:
|
||||||
if user is None:
|
if user is None:
|
||||||
user = await current_user()
|
user = await current_user()
|
||||||
|
|
|
||||||
50
src/main.py
50
src/main.py
|
|
@ -93,6 +93,19 @@ class NotificationReply(BaseModel):
|
||||||
return value
|
return value
|
||||||
|
|
||||||
|
|
||||||
|
class PullReviewSubmission(BaseModel):
|
||||||
|
decision: str
|
||||||
|
body: str = Field(max_length=10_000)
|
||||||
|
expected_head_sha: str
|
||||||
|
|
||||||
|
@field_validator("decision")
|
||||||
|
@classmethod
|
||||||
|
def validate_decision(cls, value: str) -> str:
|
||||||
|
if value not in {"comment", "approve", "request_changes"}:
|
||||||
|
raise ValueError("unsupported review decision")
|
||||||
|
return value
|
||||||
|
|
||||||
|
|
||||||
def _context_payload(user_data, repo_data, issues_data, prs_data) -> dict:
|
def _context_payload(user_data, repo_data, issues_data, prs_data) -> dict:
|
||||||
user_model = User(
|
user_model = User(
|
||||||
id=user_data["id"],
|
id=user_data["id"],
|
||||||
|
|
@ -680,3 +693,40 @@ async def review_detail(owner: str, repo: str, number: int):
|
||||||
{"error": "Pull request review details are temporarily unavailable"},
|
{"error": "Pull request review details are temporarily unavailable"},
|
||||||
status_code=503,
|
status_code=503,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@app.post("/api/v1/repos/{owner}/{repo}/pulls/{number}/review", status_code=201)
|
||||||
|
async def submit_review(
|
||||||
|
submission: PullReviewSubmission, owner: str, repo: str, number: int
|
||||||
|
):
|
||||||
|
repository = f"{owner}/{repo}"
|
||||||
|
|
||||||
|
async def submit_requested_review():
|
||||||
|
if not await is_requested_review(repository, number):
|
||||||
|
raise HTTPException(status_code=404, detail="Review request not found")
|
||||||
|
return await gitea_proxy.submit_pull_review(
|
||||||
|
repository,
|
||||||
|
number,
|
||||||
|
submission.expected_head_sha,
|
||||||
|
submission.decision,
|
||||||
|
submission.body,
|
||||||
|
)
|
||||||
|
|
||||||
|
try:
|
||||||
|
result = await asyncio.wait_for(
|
||||||
|
submit_requested_review(), timeout=REVIEW_DETAIL_TIMEOUT_SECONDS
|
||||||
|
)
|
||||||
|
except HTTPException:
|
||||||
|
raise
|
||||||
|
except gitea_proxy.StaleReviewError:
|
||||||
|
return JSONResponse(
|
||||||
|
{"error": "New commits were pushed. Refresh the review before submitting."},
|
||||||
|
status_code=409,
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
return JSONResponse(
|
||||||
|
{"error": "The review could not be submitted. Your draft is safe; please retry."},
|
||||||
|
status_code=503,
|
||||||
|
headers={"Retry-After": "1"},
|
||||||
|
)
|
||||||
|
return JSONResponse(result, status_code=201)
|
||||||
|
|
|
||||||
|
|
@ -974,7 +974,12 @@ async def test_review_sheet_loads_details_and_preserves_safe_gitea_handoff():
|
||||||
assert "review-files" in html
|
assert "review-files" in html
|
||||||
assert "review-history" in html
|
assert "review-history" in html
|
||||||
assert "open-review-gitea" in html
|
assert "open-review-gitea" in html
|
||||||
assert "reviewController.submit" not in html
|
assert 'id="submit-review"' in html
|
||||||
|
assert 'id="review-submit-status"' in html and 'aria-live="assertive"' in html
|
||||||
|
assert "reviewController.submit(selectedReview" in html
|
||||||
|
assert "window.confirm" in html
|
||||||
|
assert "expected_head_sha: selectedReviewHead" in html
|
||||||
|
assert "await load()" in html
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
|
|
@ -1003,6 +1008,86 @@ async def test_mobile_review_sheet_captures_and_safely_hands_off_feedback():
|
||||||
assert "handoffWindow.opener = null" in html
|
assert "handoffWindow.opener = null" in html
|
||||||
|
|
||||||
|
|
||||||
|
def test_review_controller_submits_once_while_request_is_in_flight():
|
||||||
|
script = f"""
|
||||||
|
const createReviewController = require({json.dumps(str(REVIEW_SHEET))});
|
||||||
|
let resolveRequest;
|
||||||
|
const calls = [];
|
||||||
|
const controller = createReviewController({{ fetchJson: (url, options) => {{
|
||||||
|
calls.push({{url, options}});
|
||||||
|
return new Promise(resolve => {{ resolveRequest = resolve; }});
|
||||||
|
}} }});
|
||||||
|
const item = {{repository:'stackchain/api', number:7}};
|
||||||
|
const payload = {{decision:'approve', body:'Looks good.', expected_head_sha:'abc123'}};
|
||||||
|
const first = controller.submit(item, payload);
|
||||||
|
const second = controller.submit(item, payload);
|
||||||
|
resolveRequest({{id:91, state:'APPROVED'}});
|
||||||
|
Promise.all([first, second]).then(results => process.stdout.write(JSON.stringify({{
|
||||||
|
calls, results
|
||||||
|
}})));
|
||||||
|
"""
|
||||||
|
result = subprocess.run(
|
||||||
|
["node", "-e", script], check=True, capture_output=True, text=True
|
||||||
|
)
|
||||||
|
|
||||||
|
payload = json.loads(result.stdout)
|
||||||
|
assert len(payload["calls"]) == 1
|
||||||
|
assert payload["calls"][0] == {
|
||||||
|
"url": "api/v1/repos/stackchain/api/pulls/7/review",
|
||||||
|
"options": {
|
||||||
|
"method": "POST",
|
||||||
|
"headers": {
|
||||||
|
"Accept": "application/json",
|
||||||
|
"Content-Type": "application/json",
|
||||||
|
},
|
||||||
|
"body": json.dumps(
|
||||||
|
{
|
||||||
|
"decision": "approve",
|
||||||
|
"body": "Looks good.",
|
||||||
|
"expected_head_sha": "abc123",
|
||||||
|
},
|
||||||
|
separators=(",", ":"),
|
||||||
|
),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
assert payload["results"] == [
|
||||||
|
{"id": 91, "state": "APPROVED"},
|
||||||
|
{"id": 91, "state": "APPROVED"},
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_successful_review_can_clear_head_scoped_draft_and_progress():
|
||||||
|
script = f"""
|
||||||
|
const reviewSheet = require({json.dumps(str(REVIEW_SHEET))});
|
||||||
|
const values = new Map();
|
||||||
|
const removed = [];
|
||||||
|
const storage = {{
|
||||||
|
getItem: key => values.get(key) || null,
|
||||||
|
setItem: (key, value) => values.set(key, value),
|
||||||
|
removeItem: key => {{ removed.push(key); values.delete(key); }},
|
||||||
|
}};
|
||||||
|
const options = {{storage, repository:'stackchain/api', number:7, headSha:'abc123', files:[{{filename:'a.py'}}]}};
|
||||||
|
const draft = reviewSheet.createDraft(options);
|
||||||
|
const progress = reviewSheet.createProgress(options);
|
||||||
|
draft.setSummary('Looks good.');
|
||||||
|
progress.markReviewed('a.py');
|
||||||
|
draft.clear();
|
||||||
|
progress.clear();
|
||||||
|
process.stdout.write(JSON.stringify({{removed, draft:draft.snapshot(), progress:progress.snapshot()}}));
|
||||||
|
"""
|
||||||
|
result = subprocess.run(
|
||||||
|
["node", "-e", script], check=True, capture_output=True, text=True
|
||||||
|
)
|
||||||
|
|
||||||
|
payload = json.loads(result.stdout)
|
||||||
|
assert payload["removed"] == [
|
||||||
|
"stackchain.review-draft.v1:stackchain/api#7@abc123",
|
||||||
|
"stackchain.review-progress.v1:stackchain/api#7@abc123",
|
||||||
|
]
|
||||||
|
assert payload["draft"] == {"notes": {}, "summary": "", "decision": "comment"}
|
||||||
|
assert payload["progress"]["reviewedCount"] == 0
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_mobile_review_failure_offers_an_in_place_retry_for_the_same_item():
|
async def test_mobile_review_failure_offers_an_in_place_retry_for_the_same_item():
|
||||||
html = await dashboard()
|
html = await dashboard()
|
||||||
|
|
|
||||||
|
|
@ -3,7 +3,7 @@ import asyncio
|
||||||
import httpx
|
import httpx
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from src import main
|
from src import gitea_proxy, main
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
|
|
@ -76,13 +76,202 @@ async def test_review_detail_has_one_retryable_deadline_and_cancels_pending_work
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_public_review_endpoint_does_not_expose_service_token_mutations():
|
async def test_review_submission_posts_validated_decision_for_requested_current_head(monkeypatch):
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
async def requested(repository, number):
|
||||||
|
return (repository, number) == ("stackchain/api", 7)
|
||||||
|
|
||||||
|
async def submit(repository, number, expected_head_sha, decision, body):
|
||||||
|
calls.append((repository, number, expected_head_sha, decision, body))
|
||||||
|
return {"id": 91, "state": "APPROVED", "url": "https://forge.example/reviews/91"}
|
||||||
|
|
||||||
|
monkeypatch.setattr(main, "is_requested_review", requested)
|
||||||
|
monkeypatch.setattr(main.gitea_proxy, "submit_pull_review", submit, raising=False)
|
||||||
transport = httpx.ASGITransport(app=main.app)
|
transport = httpx.ASGITransport(app=main.app)
|
||||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||||
response = await client.post(
|
response = await client.post(
|
||||||
"/api/v1/repos/stackchain/api/pulls/7/review",
|
"/api/v1/repos/stackchain/api/pulls/7/review",
|
||||||
json={"action": "approve"},
|
json={
|
||||||
|
"decision": "approve",
|
||||||
|
"body": "Looks good on mobile.",
|
||||||
|
"expected_head_sha": "abc123",
|
||||||
|
},
|
||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 405
|
assert response.status_code == 201
|
||||||
|
assert response.headers["cache-control"] == "no-store"
|
||||||
|
assert response.json() == {
|
||||||
|
"id": 91,
|
||||||
|
"state": "APPROVED",
|
||||||
|
"url": "https://forge.example/reviews/91",
|
||||||
|
}
|
||||||
|
assert calls == [
|
||||||
|
("stackchain/api", 7, "abc123", "approve", "Looks good on mobile.")
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_gitea_review_submission_checks_head_then_maps_decision_upstream():
|
||||||
|
requests = []
|
||||||
|
|
||||||
|
async def handler(request):
|
||||||
|
requests.append(request)
|
||||||
|
if request.method == "GET":
|
||||||
|
return httpx.Response(200, json={"head": {"sha": "abc123"}})
|
||||||
|
return httpx.Response(
|
||||||
|
200,
|
||||||
|
json={
|
||||||
|
"id": 91,
|
||||||
|
"state": "APPROVED",
|
||||||
|
"html_url": "https://forge.example/reviews/91",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||||
|
try:
|
||||||
|
result = await gitea_proxy.submit_pull_review(
|
||||||
|
"stackchain/api", 7, "abc123", "approve", "Looks good."
|
||||||
|
)
|
||||||
|
finally:
|
||||||
|
await gitea_proxy.stop_client()
|
||||||
|
|
||||||
|
assert [(request.method, request.url.path) for request in requests] == [
|
||||||
|
("GET", "/api/v1/repos/stackchain/api/pulls/7"),
|
||||||
|
("POST", "/api/v1/repos/stackchain/api/pulls/7/reviews"),
|
||||||
|
]
|
||||||
|
assert requests[1].content == b'{"body":"Looks good.","event":"APPROVE","commit_id":"abc123"}'
|
||||||
|
assert result == {
|
||||||
|
"id": 91,
|
||||||
|
"state": "APPROVED",
|
||||||
|
"url": "https://forge.example/reviews/91",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_gitea_review_submission_rejects_changed_head_without_posting():
|
||||||
|
methods = []
|
||||||
|
|
||||||
|
async def handler(request):
|
||||||
|
methods.append(request.method)
|
||||||
|
return httpx.Response(200, json={"head": {"sha": "new456"}})
|
||||||
|
|
||||||
|
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||||
|
try:
|
||||||
|
with pytest.raises(gitea_proxy.StaleReviewError):
|
||||||
|
await gitea_proxy.submit_pull_review(
|
||||||
|
"stackchain/api", 7, "abc123", "request_changes", "Please revise."
|
||||||
|
)
|
||||||
|
finally:
|
||||||
|
await gitea_proxy.stop_client()
|
||||||
|
|
||||||
|
assert methods == ["GET"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_review_submission_returns_conflict_when_pull_head_changed(monkeypatch):
|
||||||
|
async def requested(repository, number):
|
||||||
|
return True
|
||||||
|
|
||||||
|
async def submit(*args):
|
||||||
|
raise gitea_proxy.StaleReviewError("changed")
|
||||||
|
|
||||||
|
monkeypatch.setattr(main, "is_requested_review", requested)
|
||||||
|
monkeypatch.setattr(main.gitea_proxy, "submit_pull_review", submit)
|
||||||
|
transport = httpx.ASGITransport(app=main.app)
|
||||||
|
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||||
|
response = await client.post(
|
||||||
|
"/api/v1/repos/stackchain/api/pulls/7/review",
|
||||||
|
json={
|
||||||
|
"decision": "request_changes",
|
||||||
|
"body": "Please revise.",
|
||||||
|
"expected_head_sha": "abc123",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 409
|
||||||
|
assert response.json() == {
|
||||||
|
"error": "New commits were pushed. Refresh the review before submitting."
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_review_submission_rejects_unsupported_decision_before_upstream(monkeypatch):
|
||||||
|
called = False
|
||||||
|
|
||||||
|
async def requested(repository, number):
|
||||||
|
nonlocal called
|
||||||
|
called = True
|
||||||
|
return True
|
||||||
|
|
||||||
|
monkeypatch.setattr(main, "is_requested_review", requested)
|
||||||
|
transport = httpx.ASGITransport(app=main.app)
|
||||||
|
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||||
|
response = await client.post(
|
||||||
|
"/api/v1/repos/stackchain/api/pulls/7/review",
|
||||||
|
json={
|
||||||
|
"decision": "merge",
|
||||||
|
"body": "Ship it.",
|
||||||
|
"expected_head_sha": "abc123",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 422
|
||||||
|
assert called is False
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_review_submission_rejects_oversized_feedback_before_upstream(monkeypatch):
|
||||||
|
called = False
|
||||||
|
|
||||||
|
async def requested(repository, number):
|
||||||
|
nonlocal called
|
||||||
|
called = True
|
||||||
|
return True
|
||||||
|
|
||||||
|
monkeypatch.setattr(main, "is_requested_review", requested)
|
||||||
|
transport = httpx.ASGITransport(app=main.app)
|
||||||
|
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||||
|
response = await client.post(
|
||||||
|
"/api/v1/repos/stackchain/api/pulls/7/review",
|
||||||
|
json={
|
||||||
|
"decision": "comment",
|
||||||
|
"body": "x" * 10_001,
|
||||||
|
"expected_head_sha": "abc123",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 422
|
||||||
|
assert called is False
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_review_submission_deadline_cancels_request_check(monkeypatch):
|
||||||
|
cancelled = asyncio.Event()
|
||||||
|
|
||||||
|
async def requested(repository, number):
|
||||||
|
try:
|
||||||
|
await asyncio.Event().wait()
|
||||||
|
finally:
|
||||||
|
cancelled.set()
|
||||||
|
|
||||||
|
monkeypatch.setattr(main, "is_requested_review", requested)
|
||||||
|
monkeypatch.setattr(main, "REVIEW_DETAIL_TIMEOUT_SECONDS", 0.01)
|
||||||
|
transport = httpx.ASGITransport(app=main.app)
|
||||||
|
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||||
|
response = await asyncio.wait_for(
|
||||||
|
client.post(
|
||||||
|
"/api/v1/repos/stackchain/api/pulls/7/review",
|
||||||
|
json={
|
||||||
|
"decision": "comment",
|
||||||
|
"body": "Review note.",
|
||||||
|
"expected_head_sha": "abc123",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
timeout=0.2,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 503
|
||||||
|
assert response.headers["retry-after"] == "1"
|
||||||
|
assert cancelled.is_set()
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user