From 091a01ba67414a2bb87c4e0f5ccb10db8183ce7a Mon Sep 17 00:00:00 2001 From: timmy Date: Mon, 24 Aug 2026 06:42:45 +0000 Subject: [PATCH] feat: address requested review feedback (Closes #1338) --- frontend/dashboard.css | 11 ++ frontend/index.html | 24 +++ frontend/pull-sheet.js | 175 +++++++++++++++++- src/gitea_proxy.py | 4 + ..._mobile_address_review_feedback_release.py | 133 +++++++++++++ tests/test_pull_api.py | 44 ++++- 6 files changed, 381 insertions(+), 10 deletions(-) create mode 100644 tests/e2e/test_mobile_address_review_feedback_release.py diff --git a/frontend/dashboard.css b/frontend/dashboard.css index 43226ed..f563690 100644 --- a/frontend/dashboard.css +++ b/frontend/dashboard.css @@ -1274,6 +1274,17 @@ textarea { resize: vertical; min-height: 120px; } .pull-review-feedback-file li { margin:6px 0; } .pull-review-feedback-file small { display:block; color:#93c5fd; } .pull-review-feedback-file button { min-height:44px; width:100%; margin-top:4px; } +.address-review-feedback { min-height:44px; width:100%; margin-top:8px; } +.pull-feedback-pass { margin-top:10px; padding:10px; border:1px solid #3b82f6; border-radius:10px; overflow-wrap:anywhere; } +.pull-feedback-pass-heading { display:flex; align-items:start; justify-content:space-between; gap:8px; } +.pull-feedback-pass-heading h4 { margin:2px 0 8px; } +.pull-feedback-pass p { white-space:pre-wrap; } +.pull-feedback-pass textarea { width:100%; min-height:88px; box-sizing:border-box; } +.pull-feedback-dispositions { display:grid; grid-template-columns:1fr; gap:6px; margin:10px 0; padding:8px; } +.pull-feedback-dispositions button[aria-pressed="true"] { border-color:#60a5fa; background:#173b63; } +.pull-feedback-navigation { display:grid; grid-template-columns:1fr 1fr; gap:8px; margin-top:8px; } +.pull-feedback-pass button { min-height:44px; } +#finish-pull-feedback, #request-feedback-review { width:100%; margin-top:8px; } #request-updated-pull-review { min-height:44px; width:100%; margin-top:8px; } .pull-review { margin-top:14px; overflow:hidden; border:1px solid #2a496e; border-radius:10px; padding:0 10px 10px; } .pull-review summary { min-height:44px; display:flex; align-items:center; cursor:pointer; } diff --git a/frontend/index.html b/frontend/index.html index c502015..9fc798c 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -1726,6 +1726,30 @@
Reviewer status not loaded.
+
ChecksNot loaded diff --git a/frontend/pull-sheet.js b/frontend/pull-sheet.js index db22224..6eae4ac 100644 --- a/frontend/pull-sheet.js +++ b/frontend/pull-sheet.js @@ -47,8 +47,13 @@ function renderReviewerFeedback(reviewer, escapeHtml) { if (!summary && !files) return ''; const reviewedHead = reviewer?.status === 'outdated' && reviewer?.head_sha ? '

Reviewed head ' + escapeHtml(reviewer.head_sha.slice(0, 8)) + '

' : ''; + const addressable = Number.isInteger(reviewer?.review_id) && reviewer.review_id > 0 && + ['changes_requested', 'outdated'].includes(reviewer?.status) && + (reviewer.comments || []).some(comment => Number.isInteger(comment?.id) && comment.id > 0); + const address = addressable ? '' : ''; return '
Review feedback' + - reviewedHead + summary + files + '
'; + reviewedHead + summary + files + address + '
'; } function renderReviewerStatuses(reviewers, escapeHtml) { @@ -190,6 +195,8 @@ function resetReviewRequestControls(doc, detail) { qs('#pull-reviewer-status').textContent = 'Reviewer status not loaded.'; qs('#pull-reviewer-statuses').textContent = ''; qs('#request-updated-pull-review').hidden = true; + qs('#pull-feedback-pass').hidden = true; + qs('#pull-feedback-status').textContent = ''; } function bindReviewRequestControls(doc, controller, getSelected, getDetail) { @@ -259,6 +266,123 @@ function bindReviewRequestControls(doc, controller, getSelected, getDetail) { }); } +function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin) { + const qs = selector => doc.querySelector(selector); + const panel = qs('#pull-feedback-pass'); + if (panel.dataset.feedbackBound === 'true') return; + panel.dataset.feedbackBound = 'true'; + let active = null; + const comments = () => (active?.reviewer?.comments || []) + .filter(comment => Number.isInteger(comment?.id) && comment.id > 0); + const current = () => comments()[active?.state?.index || 0]; + const save = () => { + if (active) controller.saveFeedbackPass(active.item, active.detail, active.reviewer, active.login, active.state); + }; + const render = () => { + if (!active) return; + const items = comments(); + active.state.index = Math.max(0, Math.min(Number(active.state.index) || 0, items.length - 1)); + const comment = current(); + const response = active.state.responses[String(comment.id)] || {}; + qs('#pull-feedback-progress').textContent = 'Comment ' + (active.state.index + 1) + ' of ' + items.length; + qs('#pull-feedback-heading').textContent = 'Address @' + active.reviewer.login + '’s feedback'; + qs('#pull-feedback-location').textContent = comment.path + (comment.line ? ' · line ' + Number(comment.line) : ''); + qs('#pull-feedback-body').textContent = comment.body; + qs('#pull-feedback-note').value = response.note || ''; + panel.querySelectorAll('[data-feedback-disposition]').forEach(button => { + button.setAttribute('aria-pressed', String(button.dataset.feedbackDisposition === response.disposition)); + }); + qs('#previous-pull-feedback').disabled = active.state.index === 0; + qs('#next-pull-feedback').disabled = active.state.index >= items.length - 1; + qs('#finish-pull-feedback').disabled = active.state.posted === true || + items.some(item => !active.state.responses[String(item.id)]?.disposition); + qs('#request-feedback-review').hidden = active.state.posted !== true; + const found = focusPullFile(doc, comment.path); + qs('#pull-feedback-context').textContent = found ? + 'Matching change opened below.' : 'Matching change is unavailable in this preview.'; + panel.scrollIntoView({ block:'center', behavior:'smooth' }); + }; + qs('#pull-reviewer-statuses').addEventListener('click', event => { + const button = event.target.closest?.('[data-address-review-feedback]'); + if (!button) return; + const detail = getDetail?.(); + const item = getSelected?.(); + const reviewId = Number(button.dataset.addressReviewFeedback); + const reviewer = (detail?.reviewers || []).find(candidate => candidate.review_id === reviewId); + if (!detail || !item || !reviewer) return; + const login = getLogin?.() || ''; + active = { item, detail, reviewer, login, + state:controller.loadFeedbackPass(item, detail, reviewer, login) }; + panel.hidden = false; + qs('#pull-feedback-status').textContent = active.state.posted ? + 'Response already posted. You can request an updated review.' : 'Progress saves on this device.'; + render(); + }); + panel.querySelector('.pull-feedback-dispositions').addEventListener('click', event => { + const disposition = event.target.dataset?.feedbackDisposition; + const comment = current(); + if (!active || !comment || !['addressed', 'discussion', 'skipped'].includes(disposition)) return; + const key = String(comment.id); + active.state.responses[key] = { ...(active.state.responses[key] || {}), disposition }; + save(); + render(); + }); + qs('#pull-feedback-note').addEventListener('input', event => { + const comment = current(); + if (!active || !comment) return; + const key = String(comment.id); + active.state.responses[key] = { ...(active.state.responses[key] || {}), note:event.target.value.slice(0, 1000) }; + save(); + }); + qs('#previous-pull-feedback').addEventListener('click', () => { + if (!active) return; + active.state.index -= 1; save(); render(); + }); + qs('#next-pull-feedback').addEventListener('click', () => { + if (!active) return; + active.state.index += 1; save(); render(); + }); + qs('#close-pull-feedback').addEventListener('click', () => { + panel.hidden = true; + doc.querySelector('[data-address-review-feedback="' + active?.reviewer?.review_id + '"]')?.focus(); + }); + qs('#finish-pull-feedback').addEventListener('click', async () => { + if (!active || qs('#finish-pull-feedback').disabled) return; + const latest = getDetail?.(); + if (!latest || latest.head_sha !== active.state.headSha) { + qs('#pull-feedback-status').textContent = 'The pull request changed. Refresh review data before posting.'; + return; + } + if (!globalThis.confirm('Post one feedback response to ' + active.item.key + '?')) return; + qs('#finish-pull-feedback').disabled = true; + qs('#pull-feedback-status').textContent = 'Posting feedback response…'; + try { + await controller.postFeedbackResponse(active.item, active.detail, active.reviewer, active.login, active.state); + active.state.posted = true; + save(); + qs('#pull-feedback-status').textContent = 'Response posted. Request an updated review when ready.'; + render(); + } catch (error) { + qs('#pull-feedback-status').textContent = error.message + ' Progress kept; retry without duplicating the response.'; + qs('#finish-pull-feedback').disabled = false; + } + }); + qs('#request-feedback-review').addEventListener('click', async () => { + if (!active?.state?.posted) return; + const button = qs('#request-feedback-review'); + button.disabled = true; + qs('#pull-feedback-status').textContent = 'Requesting updated review…'; + try { + await controller.requestReview(active.item, active.reviewer.login, active.detail.head_sha); + qs('#pull-feedback-status').textContent = 'Response posted; updated review requested from @' + active.reviewer.login + '.'; + button.hidden = true; + } catch (error) { + qs('#pull-feedback-status').textContent = 'Response posted; review not requested. ' + error.message; + button.disabled = false; + } + }); +} + function bindContextEditor(doc, controller, getSelected, getDetail, getLogin) { const qs = selector => doc.querySelector(selector); const form = qs('#pull-edit-form'); @@ -333,6 +457,7 @@ function bindContextEditor(doc, controller, getSelected, getDetail, getLogin) { function bindOwnershipControls(doc, controller, getSelected, finish, getDetail, getLogin) { bindReviewRequestControls(doc, controller, getSelected, getDetail); + bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin); controller.edit = bindContextEditor(doc, controller, getSelected, getDetail, getLogin); const qs = selector => doc.querySelector(selector); const load = qs('#load-pull-handoff'); @@ -407,6 +532,7 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT let reviewCandidateRequest = null; let reviewRequestMutation = null; let editRequest = null; + let feedbackRequest = null; const reviewRequests = new Map(); const reviewCache = new Map(); const pathFor = item => 'api/v1/repos/' + String(item.repository || '').split('/') @@ -415,6 +541,8 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT const operationKey = item => draftKey(item) + ':operation'; const editDraftKey = item => 'stackchain.pull-edit.v1:' + item.repository + '#' + item.number; const reviewKey = (item, detail) => 'stackchain.pull-review.v1:' + item.repository + '#' + item.number + ':' + detail.head_sha; + const feedbackKey = (item, detail, reviewer, login) => 'stackchain.feedback-pass.v1:' + + item.repository + '#' + item.number + ':' + reviewer.review_id + ':' + reviewer.head_sha + ':' + detail.head_sha + ':' + login; const fileNames = detail => (detail?.files || []).map(file => file.filename).filter(Boolean); function reviewState(item, detail) { @@ -490,6 +618,50 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT try { storage?.setItem(editDraftKey(item), JSON.stringify(draft)); } catch (_error) { /* The edit fields remain the fallback. */ } }, + loadFeedbackPass(item, detail, reviewer, login) { + const fresh = { headSha:detail.head_sha, index:0, responses:{}, posted:false }; + try { + const saved = JSON.parse(storage?.getItem(feedbackKey(item, detail, reviewer, login)) || 'null'); + return saved && saved.headSha === detail.head_sha && saved.responses && typeof saved.responses === 'object' + ? { ...fresh, ...saved } : fresh; + } catch (_error) { return fresh; } + }, + saveFeedbackPass(item, detail, reviewer, login, state) { + try { storage?.setItem(feedbackKey(item, detail, reviewer, login), JSON.stringify(state)); } + catch (_error) { /* The visible pass remains usable without storage. */ } + }, + feedbackSummary(reviewer, state) { + const labels = { addressed:'Addressed', discussion:'Needs discussion', skipped:'Skip for now' }; + const lines = (reviewer.comments || []).filter(comment => Number.isInteger(comment?.id) && comment.id > 0) + .map(comment => { + const response = state.responses[String(comment.id)] || {}; + const path = String(comment.path || '').replaceAll('`', "'").replaceAll('\n', ' ').slice(0, 300); + const body = String(comment.body || '').replaceAll('\n', ' ').slice(0, 180); + const note = String(response.note || '').replaceAll('\n', ' ').slice(0, 1000); + return '- `' + path + '` · comment #' + comment.id + ' · **' + (labels[response.disposition] || 'Unresolved') + + '** — ' + body + (note ? '\n - Response: ' + note : ''); + }); + return ('Addressed review feedback from @' + reviewer.login + ' (review #' + reviewer.review_id + ').\n\n' + + lines.join('\n')).slice(0, 10000); + }, + postFeedbackResponse(item, detail, reviewer, login, state) { + if (feedbackRequest) return feedbackRequest; + const key = feedbackKey(item, detail, reviewer, login); + let operationId; + try { + operationId = storage?.getItem(key + ':operation') || String(createOperationId()).slice(0, 128); + storage?.setItem(key + ':operation', operationId); + } catch (_error) { operationId = String(createOperationId()).slice(0, 128); } + feedbackRequest = fetchJson(pathFor(item) + '/comments', { + method:'POST', + headers:{ Accept:'application/json', 'Content-Type':'application/json', 'Idempotency-Key':operationId }, + body:JSON.stringify({ body:this.feedbackSummary(reviewer, state) }), + }).then(result => { + try { storage?.removeItem(key + ':operation'); } catch (_error) { /* Confirmed upstream. */ } + return result; + }).finally(() => { feedbackRequest = null; }); + return feedbackRequest; + }, updateContent(item, draft) { if (editRequest) return editRequest; this.saveEditDraft(item, draft); @@ -616,4 +788,5 @@ createPullSheet.bindOwnershipControls = bindOwnershipControls; createPullSheet.bindContextEditor = bindContextEditor; createPullSheet.resetReviewRequestControls = resetReviewRequestControls; createPullSheet.bindReviewRequestControls = bindReviewRequestControls; +createPullSheet.bindFeedbackControls = bindFeedbackControls; if (typeof module !== 'undefined' && module.exports) module.exports = createPullSheet; diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 69da4bc..b4d24a3 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -2781,6 +2781,9 @@ def _normalize_review_comments(comments: object) -> list[dict]: if not isinstance(path, str) or not path.strip() or not isinstance(body, str) or not body.strip(): continue item = {"path": path.strip()[:300], "body": body.strip()[:500]} + comment_id = comment.get("id") + if isinstance(comment_id, int) and comment_id > 0: + item["id"] = comment_id position = comment.get("new_position") or comment.get("old_position") if isinstance(position, int) and position > 0: item["line"] = position @@ -2849,6 +2852,7 @@ def _normalize_reviewer_statuses(pull: dict, reviews: object, head_sha: str) -> current = review["commit_id"] == head_sha status = state_names[review["state"]] if current else "outdated" statuses[key] = { + "review_id": review["id"], "login": review["login"], "status": status, "head_sha": review["commit_id"], diff --git a/tests/e2e/test_mobile_address_review_feedback_release.py b/tests/e2e/test_mobile_address_review_feedback_release.py new file mode 100644 index 0000000..e109ed5 --- /dev/null +++ b/tests/e2e/test_mobile_address_review_feedback_release.py @@ -0,0 +1,133 @@ +from pathlib import Path + +import pytest + + +ROOT = Path(__file__).parents[2] + + +@pytest.mark.parametrize("viewport", [(320, 568), (390, 844)]) +def test_mobile_author_addresses_every_review_comment_and_requests_updated_review(viewport): + playwright = pytest.importorskip("playwright.sync_api") + html = (ROOT / "frontend" / "index.html").read_text() + detail = { + "state": "open", + "draft": False, + "mergeable": True, + "merged": False, + "ci_state": "success", + "head_sha": "current-head", + "reviewers": [{ + "review_id": 42, + "login": "sam", + "status": "changes_requested", + "head_sha": "current-head", + "blocking": True, + "comments": [ + {"id": 99, "path": "src/api.py", "body": "Handle the empty state.", "line": 12}, + {"id": 100, "path": "frontend/app.js", "body": "Explain the retry.", "line": 8}, + ], + }], + "files": [ + {"filename": "src/api.py", "status": "modified", "diff_available": True, + "diff_lines": ["@@ -11 +11 @@", "+return empty"]}, + {"filename": "frontend/app.js", "status": "modified", "diff_available": True, + "diff_lines": ["@@ -7 +7 @@", "+showRetry()"]}, + ], + } + + with playwright.sync_playwright() as runtime: + try: + browser = runtime.chromium.launch(headless=True) + except Exception as error: + pytest.skip(f"Chromium is not installed: {error}") + page = browser.new_page(viewport={"width": viewport[0], "height": viewport[1]}) + page.set_content(html, wait_until="domcontentloaded") + page.add_style_tag(path=ROOT / "frontend" / "dashboard.css") + page.add_script_tag(path=ROOT / "frontend" / "pull-sheet.js") + page.evaluate( + """detail => { + globalThis.confirm = () => true; + globalThis.feedbackCalls = []; + globalThis.feedbackStored = new Map(); + globalThis.feedbackStorage = { + getItem:key => feedbackStored.has(key) ? feedbackStored.get(key) : null, + setItem:(key, value) => feedbackStored.set(key, String(value)), + removeItem:key => feedbackStored.delete(key), + }; + globalThis.feedbackItem = {repository:'stackchain/api', number:7, key:'stackchain/api#7'}; + globalThis.feedbackDetail = detail; + globalThis.feedbackController = createPullSheet({ + storage: feedbackStorage, + createOperationId: () => 'feedback-operation', + fetchJson: async (path, options = {}) => { + feedbackCalls.push({path, options}); + if (path.endsWith('/comments')) return {id: 501, body:JSON.parse(options.body).body}; + if (path.endsWith('/request-review')) return { + number:7, reviewer:'sam', head_sha:'current-head', requested_reviewers:['sam'] + }; + throw new Error('unexpected request'); + }, + }); + document.querySelector('#pull-sheet').classList.add('open'); + document.querySelector('#pull-review').open = true; + document.querySelector('#pull-files').innerHTML = detail.files.map((file, index) => + createPullSheet.renderFile(file, index, false, value => String(value) + .replaceAll('&', '&').replaceAll('<', '<').replaceAll('>', '>')) + ).join(''); + createPullSheet.bindFeedbackControls(document, feedbackController, + () => feedbackItem, () => feedbackDetail, () => 'timmy'); + createPullSheet.review(detail, null, document); + }""", + detail, + ) + + page.locator(".pull-review-feedback summary").click() + page.locator('[data-address-review-feedback="42"]').click() + assert page.locator("#pull-feedback-progress").text_content() == "Comment 1 of 2" + assert page.locator("#finish-pull-feedback").is_disabled() + page.locator('[data-feedback-disposition="addressed"]').click() + page.locator("#next-pull-feedback").click() + page.locator("#pull-feedback-note").fill("Added a visible retry message.") + page.locator('[data-feedback-disposition="discussion"]').click() + assert not page.locator("#finish-pull-feedback").is_disabled() + page.locator("#finish-pull-feedback").click() + page.locator("#request-feedback-review").click() + + page.wait_for_function("feedbackCalls.length === 2") + result = page.evaluate( + """() => ({ + calls:feedbackCalls, + scrollWidth:document.documentElement.scrollWidth, + clientWidth:document.documentElement.clientWidth, + controlHeights:Array.from(document.querySelectorAll('#pull-feedback-pass button')) + .filter(button => !button.hidden).map(button => button.getBoundingClientRect().height), + status:document.querySelector('#pull-feedback-status').textContent, + secondExpanded:document.querySelector('[data-pull-filename="frontend/app.js"] .pull-file-toggle') + .getAttribute('aria-expanded'), + persisted:Array.from(feedbackStored.keys()).some(key => key.includes('feedback-pass')), + })""" + ) + result["resetHidden"] = page.evaluate( + """() => { + createPullSheet.resetReviewRequestControls(document, feedbackDetail); + return document.querySelector('#pull-feedback-pass').hidden; + }""" + ) + browser.close() + + assert result["scrollWidth"] <= result["clientWidth"] + assert min(result["controlHeights"]) >= 44 + assert result["secondExpanded"] == "true" + assert result["persisted"] is True + assert result["resetHidden"] is True + assert result["calls"][0]["path"].endswith("/comments") + assert result["calls"][0]["options"]["headers"]["Idempotency-Key"] == "feedback-operation" + summary = result["calls"][0]["options"]["body"] + assert "Handle the empty state" in summary + assert "Addressed" in summary + assert "Needs discussion" in summary + assert "Added a visible retry message" in summary + assert result["calls"][1]["path"].endswith("/request-review") + assert '"reviewer":"sam"' in result["calls"][1]["options"]["body"] + assert result["status"] == "Response posted; updated review requested from @sam." diff --git a/tests/test_pull_api.py b/tests/test_pull_api.py index f71ef84..86d4667 100644 --- a/tests/test_pull_api.py +++ b/tests/test_pull_api.py @@ -225,14 +225,14 @@ def test_reviewer_statuses_distinguish_waiting_current_and_outdated_decisions(): assert gitea_proxy._normalize_reviewer_statuses(pull, reviews, "new-head") == [ {"login": "casey", "status": "waiting", "head_sha": "new-head", "blocking": True}, - {"login": "lee", "status": "commented", "head_sha": "new-head", "blocking": False}, - {"login": "pat", "status": "approved", "head_sha": "new-head", "blocking": False}, - {"login": "sam", "status": "changes_requested", "head_sha": "new-head", "blocking": True}, + {"review_id": 3, "login": "lee", "status": "commented", "head_sha": "new-head", "blocking": False}, + {"review_id": 4, "login": "pat", "status": "approved", "head_sha": "new-head", "blocking": False}, + {"review_id": 2, "login": "sam", "status": "changes_requested", "head_sha": "new-head", "blocking": True}, ] pull["requested_reviewers"] = [] assert gitea_proxy._normalize_reviewer_statuses(pull, reviews[:1], "new-head") == [ - {"login": "casey", "status": "outdated", "head_sha": "old-head", "blocking": True}, + {"review_id": 1, "login": "casey", "status": "outdated", "head_sha": "old-head", "blocking": True}, ] @@ -280,13 +280,39 @@ def test_review_comments_are_bounded_and_require_a_file_and_body(): assert len(normalized) == 20 assert normalized[0] == { - "path": "src/api.py", "body": "Handle the empty state", "line": 4, + "id": 1, "path": "src/api.py", "body": "Handle the empty state", "line": 4, } assert normalized[1]["line"] == 7 assert len(normalized[1]["body"]) == 500 assert all(item["path"] and item["body"] for item in normalized) +def test_review_feedback_preserves_stable_review_and_comment_identities(): + pull = {"requested_reviewers": []} + reviews = [{ + "id": 42, + "user": {"login": "sam"}, + "state": "REQUEST_CHANGES", + "commit_id": "abc123", + }] + + statuses = gitea_proxy._normalize_reviewer_statuses(pull, reviews, "abc123") + comments = gitea_proxy._normalize_review_comments([ + {"id": 99, "path": "src/api.py", "body": "Handle the empty state", "new_position": 4}, + {"id": -1, "path": "src/other.py", "body": "Invalid identity is omitted"}, + ]) + + assert statuses == [{ + "review_id": 42, + "login": "sam", + "status": "changes_requested", + "head_sha": "abc123", + "blocking": True, + }] + assert comments[0]["id"] == 99 + assert "id" not in comments[1] + + def test_inline_feedback_is_loaded_only_for_each_reviewers_latest_decision(): reviews = [ {"id": 7, "user": {"login": "sam"}, "state": "REQUEST_CHANGES"}, @@ -416,7 +442,7 @@ async def test_gitea_assigned_pull_review_includes_bounded_diff_previews(): assert detail["files"][1]["diff_available"] is False assert detail["reviewers"] == [ {"login": "casey", "status": "waiting", "head_sha": "abc123", "blocking": True}, - {"login": "sam", "status": "approved", "head_sha": "abc123", "blocking": False}, + {"review_id": 8, "login": "sam", "status": "approved", "head_sha": "abc123", "blocking": False}, ] assert "conversation" not in detail @@ -441,7 +467,7 @@ async def test_assigned_pull_review_includes_latest_inline_feedback(): ]) if path.endswith("/pulls/7/reviews/8/comments"): return httpx.Response(200, json=[ - {"path": "src/api.py", "body": "Return before parsing.", "new_position": 12}, + {"id": 99, "path": "src/api.py", "body": "Return before parsing.", "new_position": 12}, ]) if path.endswith("/pulls/7/files"): return httpx.Response(200, json=[{"filename": "src/api.py", "status": "modified"}]) @@ -459,9 +485,9 @@ async def test_assigned_pull_review_includes_latest_inline_feedback(): assert "/api/v1/repos/stackchain/api/pulls/7/reviews/7/comments" not in requests assert detail["reviewers"] == [{ - "login": "sam", "status": "changes_requested", "head_sha": "abc123", "blocking": True, + "review_id": 8, "login": "sam", "status": "changes_requested", "head_sha": "abc123", "blocking": True, "summary": "Please handle the empty state.", - "comments": [{"path": "src/api.py", "body": "Return before parsing.", "line": 12}], + "comments": [{"id": 99, "path": "src/api.py", "body": "Return before parsing.", "line": 12}], }]