From 2bd8426db8ca98e439d4d27e36ad4f9ad0986ebb Mon Sep 17 00:00:00 2001 From: timmy Date: Fri, 7 Aug 2026 23:12:55 +0000 Subject: [PATCH] feat: continue assigned approvals into merge (#240) --- frontend/index.html | 32 ++++++++++++++++++-- frontend/review-sheet.js | 13 ++++++++ frontend/service-worker.js | 2 +- tests/test_my_work.py | 57 ++++++++++++++++++++++++++++++++++++ tests/test_service_worker.py | 4 +-- 5 files changed, 102 insertions(+), 6 deletions(-) diff --git a/frontend/index.html b/frontend/index.html index 336d278..1c25f49 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -113,6 +113,9 @@ textarea { resize: vertical; min-height: 120px; } .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[hidden] { display:none; } +.review-merge-continuation { position:sticky; bottom:0; z-index:5; padding:10px 4px; padding-bottom:calc(10px + env(safe-area-inset-bottom)); background:rgba(11,21,38,.98); } +#continue-review-to-merge { min-height:44px; width:100%; } +#continue-review-to-merge[hidden] { display:none; } .review-copy-fallback { min-height:140px; } .review-progress-actions { position:sticky; bottom:0; z-index:2; display:flex; align-items:center; justify-content:space-between; gap:10px; margin:8px -4px 0; padding:10px 4px; background:rgba(11,21,38,.96); border-top:1px solid #2a496e; } .review-progress-actions button { min-height:44px; max-width:100%; } @@ -707,6 +710,9 @@ textarea { resize: vertical; min-height: 120px; } +
+ +

Review history

@@ -2119,6 +2125,7 @@ textarea { resize: vertical; min-height: 120px; } qs('#review-handoff-link').href = item.url; qs('#review-handoff-status').textContent = ''; qs('#review-submit-status').textContent = ''; + qs('#continue-review-to-merge').hidden = true; qs('#submit-review').disabled = true; closeInlineComposer(); draft = null; @@ -3133,12 +3140,21 @@ textarea { resize: vertical; min-height: 120px; } button.disabled = true; qs('#review-submit-status').textContent = 'Submitting review…'; try { + const item = selectedReview; + const progressSnapshot = progress?.snapshot() || { reviewed: [] }; const result = await reviewController.submit(selectedReview, { decision: snapshot.decision, body: createReviewController.formatFeedback(snapshot, reviewFiles), expected_head_sha: selectedReviewHead, comments: snapshot.comments, }); + const canContinueToMerge = createReviewController.prepareMergeContinuation({ + storage: localStorage, + item, + headSha: selectedReviewHead, + reviewed: progressSnapshot.reviewed, + decision: snapshot.decision, + }); draft.clear(); progress?.clear(); qs('#review-decision').value = 'comment'; @@ -3146,15 +3162,25 @@ textarea { resize: vertical; min-height: 120px; } 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(); - if (workSession.active()) workSession.complete(); - else button.focus(); + if (canContinueToMerge) { + qs('#continue-review-to-merge').hidden = false; + qs('#continue-review-to-merge').focus(); + } else { + await load(); + if (workSession.active()) workSession.complete(); + else 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('#continue-review-to-merge').addEventListener('click', () => { + const item = selectedReview; + if (!item) return; + workRoute.open({ ...item, kind:'pull', is_review:false }, { replace:true }); + }); qs('#copy-review-feedback').addEventListener('click', async () => { if (!draft || !selectedReview || reviewHandoffPending) return; reviewHandoffPending = true; diff --git a/frontend/review-sheet.js b/frontend/review-sheet.js index b4c6110..bdd6158 100644 --- a/frontend/review-sheet.js +++ b/frontend/review-sheet.js @@ -309,6 +309,18 @@ async function copyAndContinue({ text, url, copy, open, fallback }) { return { copied: true, opened: true }; } +function prepareMergeContinuation({ storage, item, headSha, reviewed, decision }) { + const reasons = Array.isArray(item?.work_reasons) ? item.work_reasons : []; + if (decision !== 'approve' || !reasons.includes('assigned_to_me') || !headSha) return false; + const filenames = Array.isArray(reviewed) + ? reviewed.filter(filename => typeof filename === 'string' && filename) + : []; + const key = 'stackchain.pull-review.v1:' + item.repository + '#' + item.number + ':' + headSha; + try { storage?.setItem(key, JSON.stringify(filenames)); } + catch (_error) { return false; } + return true; +} + createReviewController.renderDiffFile = renderDiffFile; createReviewController.parseDiffLines = parseDiffLines; createReviewController.toggleDiff = toggleDiff; @@ -316,6 +328,7 @@ createReviewController.createProgress = createProgress; createReviewController.createDraft = createDraft; createReviewController.formatFeedback = formatFeedback; createReviewController.copyAndContinue = copyAndContinue; +createReviewController.prepareMergeContinuation = prepareMergeContinuation; if (typeof module !== 'undefined' && module.exports) { module.exports = createReviewController; diff --git a/frontend/service-worker.js b/frontend/service-worker.js index 523facc..32bf00d 100644 --- a/frontend/service-worker.js +++ b/frontend/service-worker.js @@ -1,4 +1,4 @@ -const CACHE = 'stackchain-dashboard-shell-v8'; +const CACHE = 'stackchain-dashboard-shell-v9'; const OUTAGE_STATUSES = new Set([500, 502, 503, 504]); const BASE = new URL('./', self.location.href).pathname; const SHELL = [ diff --git a/tests/test_my_work.py b/tests/test_my_work.py index 3972a08..d288420 100644 --- a/tests/test_my_work.py +++ b/tests/test_my_work.py @@ -3106,6 +3106,63 @@ process.stdout.write(JSON.stringify({{removed, draft:draft.snapshot(), progress: assert payload["progress"]["reviewedCount"] == 0 +def test_approved_assigned_review_carries_exact_head_progress_into_merge(): + script = f""" +const reviewSheet = require({json.dumps(str(REVIEW_SHEET))}); +const values = new Map(); +const storage = {{ + getItem: key => values.get(key) || null, + setItem: (key, value) => values.set(key, value), +}}; +const assigned = {{ + repository:'stackchain/api', number:7, + work_reasons:['assigned_to_me', 'review_requested'] +}}; +const carried = reviewSheet.prepareMergeContinuation({{ + storage, item:assigned, headSha:'abc123', + reviewed:['a.py', 'b.py'], decision:'approve' +}}); +const unassigned = reviewSheet.prepareMergeContinuation({{ + storage, item:{{...assigned, number:8, work_reasons:['review_requested']}}, + headSha:'def456', reviewed:['c.py'], decision:'approve' +}}); +const changesRequested = reviewSheet.prepareMergeContinuation({{ + storage, item:{{...assigned, number:9}}, headSha:'ghi789', + reviewed:['d.py'], decision:'request_changes' +}}); +process.stdout.write(JSON.stringify({{carried, unassigned, changesRequested, entries:[...values.entries()]}})); +""" + result = subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ) + + assert json.loads(result.stdout) == { + "carried": True, + "unassigned": False, + "changesRequested": False, + "entries": [[ + "stackchain.pull-review.v1:stackchain/api#7:abc123", + '["a.py","b.py"]', + ]], + } + + +@pytest.mark.anyio +async def test_successful_assigned_approval_continues_to_merge_in_place(): + html = await dashboard() + + assert 'id="continue-review-to-merge"' in html + assert '#continue-review-to-merge[hidden]' in html and 'display:none' in html + assert '.review-merge-continuation' in html and 'position:sticky' in html + assert '#continue-review-to-merge { min-height:44px;' in html + assert "createReviewController.prepareMergeContinuation" in html + assert "reviewed: progressSnapshot.reviewed" in html + assert "decision: snapshot.decision" in html + assert "workRoute.open({ ...item, kind:'pull', is_review:false }, { replace:true })" in html + assert "qs('#continue-review-to-merge').hidden = false" in html + assert "qs('#continue-review-to-merge').focus()" in html + + @pytest.mark.anyio async def test_mobile_review_failure_offers_an_in_place_retry_for_the_same_item(): html = await dashboard() diff --git a/tests/test_service_worker.py b/tests/test_service_worker.py index 39b69ad..c29b6f4 100644 --- a/tests/test_service_worker.py +++ b/tests/test_service_worker.py @@ -63,10 +63,10 @@ async function dispatch(name, request) {{ return json.loads(completed.stdout) -def test_identity_bound_outbox_scripts_ship_in_a_new_shell_cache(): +def test_review_to_merge_flow_ships_in_a_new_shell_cache(): source = WORKER.read_text() - assert "stackchain-dashboard-shell-v8" in source + assert "stackchain-dashboard-shell-v9" in source def test_install_precaches_complete_subpath_scoped_app_shell():