feat: continue assigned approvals into merge (#240)
This commit is contained in:
parent
9289434ad2
commit
2bd8426db8
|
|
@ -113,6 +113,9 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
.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; }
|
||||||
|
.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-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 { 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%; }
|
.review-progress-actions button { min-height:44px; max-width:100%; }
|
||||||
|
|
@ -707,6 +710,9 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
<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>
|
||||||
<a class="review-handoff-link" id="review-handoff-link" target="_blank" rel="noopener noreferrer" hidden>Continue to Gitea</a>
|
<a class="review-handoff-link" id="review-handoff-link" target="_blank" rel="noopener noreferrer" hidden>Continue to Gitea</a>
|
||||||
</div>
|
</div>
|
||||||
|
<div class="review-merge-continuation">
|
||||||
|
<button id="continue-review-to-merge" type="button" hidden>Continue to merge</button>
|
||||||
|
</div>
|
||||||
</section>
|
</section>
|
||||||
<h2>Review history</h2>
|
<h2>Review history</h2>
|
||||||
<div id="review-history" class="muted"></div>
|
<div id="review-history" class="muted"></div>
|
||||||
|
|
@ -2119,6 +2125,7 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
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('#review-submit-status').textContent = '';
|
||||||
|
qs('#continue-review-to-merge').hidden = true;
|
||||||
qs('#submit-review').disabled = true;
|
qs('#submit-review').disabled = true;
|
||||||
closeInlineComposer();
|
closeInlineComposer();
|
||||||
draft = null;
|
draft = null;
|
||||||
|
|
@ -3133,12 +3140,21 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
button.disabled = true;
|
button.disabled = true;
|
||||||
qs('#review-submit-status').textContent = 'Submitting review…';
|
qs('#review-submit-status').textContent = 'Submitting review…';
|
||||||
try {
|
try {
|
||||||
|
const item = selectedReview;
|
||||||
|
const progressSnapshot = progress?.snapshot() || { reviewed: [] };
|
||||||
const result = await reviewController.submit(selectedReview, {
|
const result = await reviewController.submit(selectedReview, {
|
||||||
decision: snapshot.decision,
|
decision: snapshot.decision,
|
||||||
body: createReviewController.formatFeedback(snapshot, reviewFiles),
|
body: createReviewController.formatFeedback(snapshot, reviewFiles),
|
||||||
expected_head_sha: selectedReviewHead,
|
expected_head_sha: selectedReviewHead,
|
||||||
comments: snapshot.comments,
|
comments: snapshot.comments,
|
||||||
});
|
});
|
||||||
|
const canContinueToMerge = createReviewController.prepareMergeContinuation({
|
||||||
|
storage: localStorage,
|
||||||
|
item,
|
||||||
|
headSha: selectedReviewHead,
|
||||||
|
reviewed: progressSnapshot.reviewed,
|
||||||
|
decision: snapshot.decision,
|
||||||
|
});
|
||||||
draft.clear();
|
draft.clear();
|
||||||
progress?.clear();
|
progress?.clear();
|
||||||
qs('#review-decision').value = 'comment';
|
qs('#review-decision').value = 'comment';
|
||||||
|
|
@ -3146,15 +3162,25 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
document.querySelectorAll('.review-note').forEach(note => { note.value = ''; });
|
document.querySelectorAll('.review-note').forEach(note => { note.value = ''; });
|
||||||
if (progress) showReviewProgress(progress.snapshot());
|
if (progress) showReviewProgress(progress.snapshot());
|
||||||
qs('#review-submit-status').textContent = 'Review submitted · ' + (result.state || 'complete') + '.';
|
qs('#review-submit-status').textContent = 'Review submitted · ' + (result.state || 'complete') + '.';
|
||||||
|
if (canContinueToMerge) {
|
||||||
|
qs('#continue-review-to-merge').hidden = false;
|
||||||
|
qs('#continue-review-to-merge').focus();
|
||||||
|
} else {
|
||||||
await load();
|
await load();
|
||||||
if (workSession.active()) workSession.complete();
|
if (workSession.active()) workSession.complete();
|
||||||
else button.focus();
|
else button.focus();
|
||||||
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
qs('#review-submit-status').textContent = error.message + ' Your draft is safe; retry or open in Gitea.';
|
qs('#review-submit-status').textContent = error.message + ' Your draft is safe; retry or open in Gitea.';
|
||||||
button.disabled = false;
|
button.disabled = false;
|
||||||
button.focus();
|
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 () => {
|
qs('#copy-review-feedback').addEventListener('click', async () => {
|
||||||
if (!draft || !selectedReview || reviewHandoffPending) return;
|
if (!draft || !selectedReview || reviewHandoffPending) return;
|
||||||
reviewHandoffPending = true;
|
reviewHandoffPending = true;
|
||||||
|
|
|
||||||
|
|
@ -309,6 +309,18 @@ async function copyAndContinue({ text, url, copy, open, fallback }) {
|
||||||
return { copied: true, opened: true };
|
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.renderDiffFile = renderDiffFile;
|
||||||
createReviewController.parseDiffLines = parseDiffLines;
|
createReviewController.parseDiffLines = parseDiffLines;
|
||||||
createReviewController.toggleDiff = toggleDiff;
|
createReviewController.toggleDiff = toggleDiff;
|
||||||
|
|
@ -316,6 +328,7 @@ createReviewController.createProgress = createProgress;
|
||||||
createReviewController.createDraft = createDraft;
|
createReviewController.createDraft = createDraft;
|
||||||
createReviewController.formatFeedback = formatFeedback;
|
createReviewController.formatFeedback = formatFeedback;
|
||||||
createReviewController.copyAndContinue = copyAndContinue;
|
createReviewController.copyAndContinue = copyAndContinue;
|
||||||
|
createReviewController.prepareMergeContinuation = prepareMergeContinuation;
|
||||||
|
|
||||||
if (typeof module !== 'undefined' && module.exports) {
|
if (typeof module !== 'undefined' && module.exports) {
|
||||||
module.exports = createReviewController;
|
module.exports = createReviewController;
|
||||||
|
|
|
||||||
|
|
@ -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 OUTAGE_STATUSES = new Set([500, 502, 503, 504]);
|
||||||
const BASE = new URL('./', self.location.href).pathname;
|
const BASE = new URL('./', self.location.href).pathname;
|
||||||
const SHELL = [
|
const SHELL = [
|
||||||
|
|
|
||||||
|
|
@ -3106,6 +3106,63 @@ process.stdout.write(JSON.stringify({{removed, draft:draft.snapshot(), progress:
|
||||||
assert payload["progress"]["reviewedCount"] == 0
|
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
|
@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()
|
||||||
|
|
|
||||||
|
|
@ -63,10 +63,10 @@ async function dispatch(name, request) {{
|
||||||
return json.loads(completed.stdout)
|
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()
|
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():
|
def test_install_precaches_complete_subpath_scoped_app_shell():
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user