diff --git a/frontend/dashboard.css b/frontend/dashboard.css index b056965..e11f8ae 100644 --- a/frontend/dashboard.css +++ b/frontend/dashboard.css @@ -356,10 +356,11 @@ textarea { resize: vertical; min-height: 120px; } .issue-sheet-actions { position:sticky; bottom:0; z-index:3; display:grid; gap:8px; margin-top:14px; padding:10px 4px; padding-bottom:calc(10px + env(safe-area-inset-bottom)); background:rgba(11,21,38,.98); border-top:1px solid #2a496e; } .issue-sheet-actions button, .issue-sheet-actions a { min-height:44px; display:flex; align-items:center; justify-content:center; } .issue-sheet-actions a { border:1px solid #60a5fa; border-radius:10px; font-weight:700; } -.issue-handoff { margin-top:14px; padding:12px; border:1px solid #2a496e; border-radius:12px; } -.issue-handoff > div { display:grid; gap:8px; margin-top:10px; } -.issue-handoff select { width:100%; max-width:100%; padding:8px; border:1px solid #1f3a5f; border-radius:8px; background:#0b1526; color:var(--text); } +.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 select, .issue-handoff button { min-height:44px; } +.pull-ownership select, .pull-ownership 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; } diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 6ebedb3..caa6520 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -448,7 +448,17 @@ return await pullWorkflowFeatures.run('pull-workflow', { trigger, status: qs('#my-work-action-status'), retryLabel:'Tap the work card to retry.', }, () => { - if (!pullController) pullController = createPullSheet({ fetchJson: fetchReviewJson, storage: localStorage }); + if (!pullController) { + pullController = createPullSheet({ fetchJson: fetchReviewJson, storage: localStorage }); + createPullSheet.bindOwnershipControls(document, pullController, () => selectedPull, async item => { + const continuing = workSession.checkpointed(item); + lastContextSnapshot = createPullSheet.removeFromSnapshot(lastContextSnapshot, item); + closePullSheet(); + if (continuing) return await completeOwnershipExitToday(item); + paintMyWork(lastContextSnapshot); + return false; + }); + } if (!reviewController) reviewController = createReviewController({ fetchJson: fetchReviewJson, storage: localStorage }); if (!wrapPreference) { wrapPreference = createReviewController.createWrapPreference({ @@ -1700,9 +1710,9 @@ '#issue-handoff-recipient', '#confirm-issue-handoff', '#issue-due-date', '#save-issue-labels', '#save-issue-due-date', '#clear-issue-due-date', '#issue-milestone', '#save-issue-milestone', '#load-older-issue-comments', - ] : [ + ] : createPullSheet.ownershipSelectors().concat([ '#merge-pull', '#pull-review-retry', '#next-unreviewed-pull-file', '#load-older-pull-comments', - ]; + ]); selectors.forEach(selector => { const control = qs(selector); if (control) control.disabled = true; @@ -1712,6 +1722,7 @@ qs('#issue-handoff').inert = true; } else { qs('#pull-review').inert = true; + qs('#pull-ownership').inert = true; } } @@ -2594,21 +2605,6 @@ qs('#pull-review-status').textContent = 'Checks refreshed for the current head.'; } - function focusNextUnreviewedPullFile() { - if (!selectedPullDetail || !pullReviewState) return; - const filename = pullController.nextUnreviewed(selectedPullDetail, pullReviewState); - const article = Array.from(qs('#pull-files').querySelectorAll('.pull-file')) - .find(file => file.dataset.pullFilename === filename); - const toggle = article?.querySelector('.pull-file-toggle'); - const panel = toggle && document.getElementById(toggle.getAttribute('aria-controls')); - if (toggle && panel) { - toggle.setAttribute('aria-expanded', 'true'); - panel.hidden = false; - toggle.scrollIntoView({ block: 'center', behavior: 'smooth' }); - toggle.focus(); - } - } - async function loadPullReview({ refresh = false } = {}) { if (!selectedPull || !selectedPullDetail?.head_sha) return; const item = selectedPull; @@ -2633,16 +2629,12 @@ } } - function sameWorkTarget(left, right) { - return Boolean(left && right && left.repository === right.repository && - Number(left.number) === Number(right.number)); - } - async function openPullSheet(item, trigger, offlineDetail = null) { if (!item) return; if (!await ensurePullWorkflow(trigger)) return; - if (!sameWorkTarget(selectedPull, item)) pullAttachmentController.clear(); + if (!createPullSheet.sameTarget(selectedPull, item)) pullAttachmentController.clear(); qs('#pull-review').inert = false; + qs('#pull-ownership').inert = false; selectedPull = item; pullMentions.dismiss(); pullTrigger = trigger; @@ -2672,6 +2664,7 @@ qs('#pull-merge-state').textContent = 'Review data not loaded'; qs('#merge-pull').disabled = true; qs('#merge-pull').textContent = workSession.active() ? 'Merge & next' : 'Merge'; + createPullSheet.resetOwnershipControls(document, item, candidate => workSession.checkpointed(candidate)); qs('#retry-pull-load').hidden = true; qs('#open-pull-gitea').href = item.url || '#'; setCommentNextVisibility('pull'); @@ -4571,7 +4564,9 @@ if (selectedPull === item) qs('#pull-checks-summary').textContent = 'Refresh failed · retry'; } finally { button.disabled = false; } }); - qs('#next-unreviewed-pull-file').addEventListener('click', focusNextUnreviewedPullFile); + qs('#next-unreviewed-pull-file').addEventListener('click', () => { + createPullSheet.focusNextUnreviewed(document, selectedPullDetail, pullReviewState, pullController); + }); qs('#load-older-pull-comments').addEventListener('click', async () => { if (!pullConversation) return; const button = qs('#load-older-pull-comments'); diff --git a/frontend/index.html b/frontend/index.html index e5ebb5f..b1ab69e 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -655,8 +655,19 @@
+
+ Ownership +
+ + + + +
Load teammates to transfer ownership.
+
+
+ Open in Gitea
Defer
diff --git a/frontend/pull-sheet.js b/frontend/pull-sheet.js index fc0394c..d42f75d 100644 --- a/frontend/pull-sheet.js +++ b/frontend/pull-sheet.js @@ -41,10 +41,137 @@ function renderFile(file, index, reviewed, escapeHtml) { '" aria-pressed="' + String(reviewed) + '">' + (reviewed ? 'Reviewed' : 'Mark reviewed') + ''; } +function focusNextUnreviewed(doc, detail, state, controller) { + if (!detail || !state) return; + const filename = controller.nextUnreviewed(detail, state); + const article = Array.from(doc.querySelectorAll('#pull-files .pull-file')) + .find(file => file.dataset.pullFilename === filename); + const toggle = article?.querySelector('.pull-file-toggle'); + const panel = toggle && doc.getElementById(toggle.getAttribute('aria-controls')); + if (!toggle || !panel) return; + toggle.setAttribute('aria-expanded', 'true'); + panel.hidden = false; + toggle.scrollIntoView({ block: 'center', behavior: 'smooth' }); + toggle.focus(); +} + +function removeFromSnapshot(data, item) { + return { ...data, pulls:(data.pulls || []).filter(candidate => + candidate.repository !== item.repository || candidate.number !== item.number) }; +} + +function sameTarget(left, right) { + return Boolean(left && right && left.repository === right.repository && Number(left.number) === Number(right.number)); +} + +function ownershipSelectors() { + return ['#release-pull', '#load-pull-handoff', '#pull-handoff-recipient', '#confirm-pull-handoff']; +} + +function renderHandoffCandidates(select, candidates, doc) { + select.textContent = ''; + const placeholder = doc.createElement('option'); + placeholder.value = ''; + placeholder.textContent = candidates.length ? 'Select a teammate' : 'No eligible teammates'; + select.appendChild(placeholder); + candidates.forEach(candidate => { + const option = doc.createElement('option'); + option.value = candidate.login; + option.textContent = candidate.name + (candidate.name === candidate.login ? '' : ' (@' + candidate.login + ')'); + select.appendChild(option); + }); + return candidates.length; +} + +function ownershipExitMessage(item, action, transitionResult) { + return item.key + ' ' + action + (transitionResult === 'opened' ? + '. Next work item opened.' : '. Choose the next ready Today item.'); +} + +function resetOwnershipControls(doc, item, checkpointed) { + const qs = selector => doc.querySelector(selector); + qs('#pull-ownership').open = false; + qs('#pull-handoff-recipient').innerHTML = ''; + qs('#pull-handoff-recipient').disabled = true; + qs('#confirm-pull-handoff').disabled = true; + qs('#confirm-pull-handoff').textContent = checkpointed(item) ? 'Hand off & next' : 'Confirm handoff'; + qs('#load-pull-handoff').disabled = false; + qs('#release-pull').disabled = false; + qs('#release-pull').textContent = checkpointed(item) ? 'Release & next' : 'Release assignment'; + qs('#pull-handoff-status').textContent = 'Load teammates to transfer ownership.'; +} + +function bindOwnershipControls(doc, controller, getSelected, finish) { + const qs = selector => doc.querySelector(selector); + const load = qs('#load-pull-handoff'); + if (load.dataset.ownershipBound === 'true') return; + load.dataset.ownershipBound = 'true'; + load.addEventListener('click', async () => { + const selected = getSelected(); + if (!selected) return; + const select = qs('#pull-handoff-recipient'); + load.disabled = true; + qs('#pull-handoff-status').textContent = 'Loading eligible teammates…'; + try { + const count = renderHandoffCandidates(select, await controller.loadHandoffCandidates(selected), doc); + select.disabled = !count; + qs('#confirm-pull-handoff').disabled = true; + qs('#pull-handoff-status').textContent = count ? + 'Choose who should own this pull request next.' : 'No other eligible assignees were found.'; + if (count) select.focus(); + } catch (error) { + qs('#pull-handoff-status').textContent = error.message + ' Retry loading teammates.'; + load.disabled = false; + load.focus(); + } + }); + qs('#pull-handoff-recipient').addEventListener('change', event => { + qs('#confirm-pull-handoff').disabled = !event.target.value; + }); + qs('#confirm-pull-handoff').addEventListener('click', async () => { + const selected = getSelected(); + const recipient = qs('#pull-handoff-recipient').value; + if (!selected || !recipient || !globalThis.confirm('Hand off ' + selected.key + ' to @' + recipient + '?')) return; + const button = qs('#confirm-pull-handoff'); + button.disabled = true; + qs('#pull-handoff-status').textContent = 'Confirming handoff…'; + try { + await controller.handoff(selected, recipient); + const transition = await finish(selected); + qs('#my-work-action-status').textContent = transition === false ? + selected.key + ' handed off to @' + recipient + '.' : + ownershipExitMessage(selected, 'handed off to @' + recipient, transition); + } catch (error) { + qs('#pull-handoff-status').textContent = error.message + ' The pull request remains in My Work; retry.'; + button.disabled = false; + button.focus(); + } + }); + qs('#release-pull').addEventListener('click', async () => { + const selected = getSelected(); + if (!selected || !globalThis.confirm('Release ' + selected.key + ' from your My Work?')) return; + const button = qs('#release-pull'); + button.disabled = true; + qs('#pull-sheet-status').textContent = 'Releasing assignment…'; + try { + await controller.release(selected); + const transition = await finish(selected); + qs('#my-work-action-status').textContent = transition === false ? + selected.key + ' released.' : ownershipExitMessage(selected, 'released', transition); + } catch (error) { + qs('#pull-sheet-status').textContent = error.message + ' The pull request remains in My Work; retry.'; + button.disabled = false; + button.focus(); + } + }); +} + function createPullSheet({ fetchJson, storage, createConversationPager = globalThis.createConversationPager, createOperationId = () => globalThis.crypto?.randomUUID?.() || String(Date.now()) + '-' + Math.random() }) { let commentRequest = null; let mergeRequest = null; let checkRequest = null; + let ownershipRequest = null; + let candidateRequest = null; const reviewRequests = new Map(); const reviewCache = new Map(); const pathFor = item => 'api/v1/repos/' + String(item.repository || '').split('/') @@ -90,6 +217,29 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT }).finally(() => { checkRequest = null; }); return checkRequest; }, + loadHandoffCandidates(item) { + if (candidateRequest) return candidateRequest; + candidateRequest = fetchJson(pathFor(item) + '/handoff-candidates', { + headers: { Accept: 'application/json' }, + }).finally(() => { candidateRequest = null; }); + return candidateRequest; + }, + handoff(item, recipient) { + if (ownershipRequest) return ownershipRequest; + ownershipRequest = fetchJson(pathFor(item) + '/handoff', { + method: 'PATCH', + headers: { Accept: 'application/json', 'Content-Type': 'application/json' }, + body: JSON.stringify({ recipient }), + }).finally(() => { ownershipRequest = null; }); + return ownershipRequest; + }, + release(item) { + if (ownershipRequest) return ownershipRequest; + ownershipRequest = fetchJson(pathFor(item) + '/release', { + method: 'PATCH', headers: { Accept: 'application/json' }, + }).finally(() => { ownershipRequest = null; }); + return ownershipRequest; + }, conversation(item, initialPage) { const pager = createConversationPager({ loadPage: page => fetchJson(pathFor(item) + '/comments?page=' + encodeURIComponent(page) + '&limit=20', { @@ -165,4 +315,12 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT createPullSheet.mergeEligibility = mergeEligibility; createPullSheet.renderFile = renderFile; +createPullSheet.focusNextUnreviewed = focusNextUnreviewed; +createPullSheet.removeFromSnapshot = removeFromSnapshot; +createPullSheet.sameTarget = sameTarget; +createPullSheet.ownershipSelectors = ownershipSelectors; +createPullSheet.renderHandoffCandidates = renderHandoffCandidates; +createPullSheet.ownershipExitMessage = ownershipExitMessage; +createPullSheet.resetOwnershipControls = resetOwnershipControls; +createPullSheet.bindOwnershipControls = bindOwnershipControls; if (typeof module !== 'undefined' && module.exports) module.exports = createPullSheet; diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 8c3bab3..57b2e45 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -1498,6 +1498,92 @@ async def handoff_assigned_issue( } +async def pull_handoff_candidates(repository: str) -> list[dict]: + candidates = await issue_handoff_candidates(repository) + return [ + item for item in candidates + if re.fullmatch(r"[A-Za-z0-9_.-]+", item["login"]) + ][:25] + + +async def _change_assigned_pull_owners( + repository: str, number: int, recipient: str | None +) -> dict: + login, pull = await _current_login_and_target( + f"repos/{repository}/pulls/{number}" + ) + assignees_value = pull.get("assignees") + assignees = assignees_value if isinstance(assignees_value, list) else [] + login_folded = login.casefold() + assigned_to_login = any( + isinstance(item, dict) + and isinstance(item.get("login"), str) + and item["login"].casefold() == login_folded + for item in assignees + ) + if ( + pull.get("state") != "open" + or pull.get("merged") is True + or not assigned_to_login + ): + raise IssueNotAvailableError("Pull request is not assigned to the current user") + + if recipient is not None: + eligible = { + item["login"] for item in await pull_handoff_candidates(repository) + } + if recipient not in eligible: + raise IssueNotAvailableError("Handoff recipient is not eligible") + + desired = [ + item["login"] for item in assignees + if isinstance(item, dict) + and isinstance(item.get("login"), str) + and item["login"].casefold() != login_folded + ] + if recipient is not None and recipient not in desired: + desired.append(recipient) + response = await _get_client().patch( + f"/api/v1/repos/{repository}/issues/{number}", + headers=_auth(), + json={"assignees": desired}, + ) + response.raise_for_status() + confirmed = response.json() + confirmed_value = confirmed.get("assignees") if isinstance(confirmed, dict) else None + confirmed_assignees = [ + item["login"] for item in confirmed_value + if isinstance(item, dict) and isinstance(item.get("login"), str) + ] if isinstance(confirmed_value, list) else [] + if ( + not isinstance(confirmed, dict) + or confirmed.get("number") != number + or login_folded in {item.casefold() for item in confirmed_assignees} + or set(confirmed_assignees) != set(desired) + or (recipient is not None and recipient not in confirmed_assignees) + ): + raise ValueError("Gitea did not confirm pull request ownership change") + result = { + "repository": repository, + "number": number, + "state": confirmed.get("state", "open"), + "assignees": confirmed_assignees, + } + if recipient is not None: + result["recipient"] = recipient + return result + + +async def handoff_assigned_pull( + repository: str, number: int, recipient: str +) -> dict: + return await _change_assigned_pull_owners(repository, number, recipient) + + +async def release_assigned_pull(repository: str, number: int) -> dict: + return await _change_assigned_pull_owners(repository, number, None) + + async def update_issue_labels(repository: str, number: int, label_ids: list[int]) -> dict: response = await _get_client().patch( f"/api/v1/repos/{repository}/issues/{number}", diff --git a/src/main.py b/src/main.py index be73b1f..73794db 100644 --- a/src/main.py +++ b/src/main.py @@ -4317,6 +4317,77 @@ async def assigned_pull_checks( ) +@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/handoff-candidates") +async def pull_handoff_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_handoff_candidates(repository), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + except HTTPException: + raise + except Exception: + return JSONResponse( + {"error": "Teammates could not be loaded. Please retry."}, + status_code=503, + headers={"Retry-After": "1"}, + ) + return JSONResponse(result, headers={"Cache-Control": "no-store"}) + + +@app.patch("/api/v1/repos/{owner}/{repo}/pulls/{number}/handoff") +async def handoff_assigned_pull( + handoff: IssueHandoff, + owner: str, + repo: str, + number: int = PathParam(gt=0), +) -> JSONResponse: + try: + result = await asyncio.wait_for( + gitea_proxy.handoff_assigned_pull( + f"{owner}/{repo}", number, handoff.recipient + ), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + except gitea_proxy.IssueNotAvailableError: + return JSONResponse( + {"error": "The pull request or recipient changed. Reload before handing off."}, + status_code=409, + ) + except Exception: + return JSONResponse( + {"error": "The handoff could not be confirmed. It remains in My Work; 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) +) -> JSONResponse: + try: + result = await asyncio.wait_for( + gitea_proxy.release_assigned_pull(f"{owner}/{repo}", number), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + except gitea_proxy.IssueNotAvailableError: + raise HTTPException(status_code=404, detail="Assigned pull request not found") + except Exception: + return JSONResponse( + {"error": "The assignment could not be released. It remains in My Work; please retry."}, + status_code=503, + headers={"Retry-After": "1"}, + ) + return JSONResponse(result) + + @app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/comments") async def assigned_pull_conversation( owner: str, diff --git a/tests/test_issue_attachment_ui.py b/tests/test_issue_attachment_ui.py index b5cd144..bb5f694 100644 --- a/tests/test_issue_attachment_ui.py +++ b/tests/test_issue_attachment_ui.py @@ -494,7 +494,7 @@ def test_retrying_same_pull_load_preserves_screenshot_but_target_change_clears_i "\n function closePullSheet", 1 )[0] - assert "if (!sameWorkTarget(selectedPull, item)) pullAttachmentController.clear();" in open_body + assert "if (!createPullSheet.sameTarget(selectedPull, item)) pullAttachmentController.clear();" in open_body def test_pull_screenshot_comment_uploads_or_durably_admits_before_clearing_draft(): diff --git a/tests/test_my_work.py b/tests/test_my_work.py index 1c14394..7fbf262 100644 --- a/tests/test_my_work.py +++ b/tests/test_my_work.py @@ -4129,6 +4129,121 @@ Promise.all([first, duplicate]).then(async comments => {{ assert len(output["comments"]) == 2 and len(output["merges"]) == 2 +def test_pull_sheet_removes_one_pull_from_snapshot_without_touching_other_work(): + script = f""" +const createPullSheet = require({json.dumps(str(PULL_SHEET))}); +const data = {{ + issues:[{{repository:'stackchain/api',number:7}}], + pulls:[ + {{repository:'stackchain/api',number:7,title:'Transfer me'}}, + {{repository:'stackchain/web',number:7,title:'Keep me'}} + ] +}}; +const item = {{repository:'stackchain/api',number:7}}; +process.stdout.write(JSON.stringify({{ + snapshot:createPullSheet.removeFromSnapshot(data, item), + same:createPullSheet.sameTarget(item, {{repository:'stackchain/api',number:7}}), + selectors:createPullSheet.ownershipSelectors() +}})); +""" + output = json.loads(subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ).stdout) + assert output["snapshot"]["issues"] == [{"repository": "stackchain/api", "number": 7}] + assert output["snapshot"]["pulls"] == [{"repository": "stackchain/web", "number": 7, "title": "Keep me"}] + assert output["same"] is True + assert output["selectors"] == [ + "#release-pull", "#load-pull-handoff", "#pull-handoff-recipient", "#confirm-pull-handoff" + ] + + +def test_pull_sheet_ownership_presenters_render_candidates_and_exit_messages(): + script = f""" +const createPullSheet = require({json.dumps(str(PULL_SHEET))}); +const children = []; +const elements = new Map([ + ['#pull-ownership', {{open:true}}], + ['#pull-handoff-recipient', {{innerHTML:'stale',disabled:false}}], + ['#confirm-pull-handoff', {{disabled:false,textContent:''}}], + ['#load-pull-handoff', {{disabled:true}}], + ['#release-pull', {{disabled:true,textContent:''}}], + ['#pull-handoff-status', {{textContent:''}}], +]); +const select = {{textContent:'stale', appendChild:node => children.push(node)}}; +const doc = {{ + createElement:() => ({{value:'',textContent:''}}), + querySelector:selector => elements.get(selector) +}}; +const count = createPullSheet.renderHandoffCandidates(select, [ + {{login:'alex',name:'Alexander'}}, {{login:'sam',name:'sam'}} +], doc); +createPullSheet.resetOwnershipControls(doc, {{key:'stackchain/api#7'}}, () => true); +const item = {{key:'stackchain/api#7'}}; +process.stdout.write(JSON.stringify({{ + count, children, + reset:Object.fromEntries(elements), + handed:createPullSheet.ownershipExitMessage(item, 'handed off to @alex', 'opened'), + released:createPullSheet.ownershipExitMessage(item, 'released', 'gated') +}})); +""" + output = json.loads(subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ).stdout) + assert output["count"] == 2 + assert output["children"] == [ + {"value": "", "textContent": "Select a teammate"}, + {"value": "alex", "textContent": "Alexander (@alex)"}, + {"value": "sam", "textContent": "sam"}, + ] + assert output["reset"]["#pull-ownership"]["open"] is False + assert output["reset"]["#confirm-pull-handoff"]["textContent"] == "Hand off & next" + assert output["reset"]["#release-pull"]["textContent"] == "Release & next" + assert output["reset"]["#pull-handoff-status"]["textContent"] == "Load teammates to transfer ownership." + assert output["handed"] == "stackchain/api#7 handed off to @alex. Next work item opened." + assert output["released"] == "stackchain/api#7 released. Choose the next ready Today item." + + +def test_pull_sheet_single_flights_handoff_and_release_ownership_mutations(): + 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('/handoff')) return new Promise(resolve => {{ finish = resolve; }}); + return Promise.resolve(url.endsWith('/handoff-candidates') ? [{{login:'alex',name:'Alexander'}}] : {{assignees:[]}}); + }} +}}); +const item = {{repository:'stackchain/api',number:7}}; +(async () => {{ + const candidates = await controller.loadHandoffCandidates(item); + const first = controller.handoff(item, 'alex'); + const second = controller.handoff(item, 'alex'); + finish({{assignees:['alex'],recipient:'alex'}}); + const handoffs = await Promise.all([first, second]); + const released = await controller.release(item); + process.stdout.write(JSON.stringify({{calls,candidates,handoffs,released}})); +}})(); +""" + 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/handoff-candidates", "method": "GET", "body": None}, + {"url": "api/v1/repos/stackchain/api/pulls/7/handoff", "method": "PATCH", "body": {"recipient": "alex"}}, + {"url": "api/v1/repos/stackchain/api/pulls/7/release", "method": "PATCH", "body": None}, + ] + assert output["candidates"] == [{"login": "alex", "name": "Alexander"}] + assert output["handoffs"] == [ + {"assignees": ["alex"], "recipient": "alex"}, + {"assignees": ["alex"], "recipient": "alex"}, + ] + assert output["released"] == {"assignees": []} + + def test_pull_sheet_surfaces_pending_merge_confirmation_guidance(): script = f""" const createPullSheet = require({json.dumps(str(PULL_SHEET))}); @@ -4282,6 +4397,24 @@ sheet.loadReview(item, 'abc123').then(first => {{ assert output["progress"] == {"reviewed": ["src/api.py"], "total": 1, "complete": True} +@pytest.mark.anyio +async def test_assigned_pull_sheet_exposes_mobile_ownership_exit_and_today_continuation(): + html = await dashboard() + + assert 'id="pull-ownership"' in html + assert 'id="pull-handoff-recipient"' in html + assert 'id="load-pull-handoff"' in html + assert 'id="confirm-pull-handoff"' in html + assert 'id="release-pull"' in html + assert 'id="pull-handoff-status" class="small" aria-live="assertive"' in html + assert '.pull-ownership select, .pull-ownership button { min-height:44px;' in html + assert "createPullSheet.bindOwnershipControls(" in html + assert "document, pullController, () => selectedPull" in html + assert "createPullSheet.removeFromSnapshot(lastContextSnapshot" in html + assert "if (continuing) return await completeOwnershipExitToday(item)" in html + assert "createPullSheet.resetOwnershipControls(document, item" in html + + @pytest.mark.anyio async def test_mobile_pull_sheet_puts_reading_before_collapsed_review_controls(): html = await dashboard() @@ -4348,7 +4481,7 @@ async def test_assigned_pulls_open_accessible_mobile_completion_sheet(): assert "pullController.load(item)" in html assert "createPullSheet.renderFile" in html assert "pullController.toggleReviewed" in html - assert "pullController.nextUnreviewed" in html + assert "createPullSheet.focusNextUnreviewed(document" in html assert ".pull-diff { overflow-x:auto;" in html assert ".pull-file-toggle, .pull-review-file { min-height:44px;" in html assert "window.confirm('Merge ' + selectedPull.key" in html diff --git a/tests/test_pull_api.py b/tests/test_pull_api.py index d7a8bc0..cec47d0 100644 --- a/tests/test_pull_api.py +++ b/tests/test_pull_api.py @@ -459,3 +459,145 @@ async def test_gitea_merge_rejects_failed_ci_without_upstream_mutation(): ("GET", "/api/v1/repos/stackchain/api/pulls/7"), ("GET", "/api/v1/repos/stackchain/api/commits/abc123/status"), ] + + +@pytest.mark.anyio +async def test_assigned_pull_ownership_endpoints_list_handoff_and_release(monkeypatch): + calls = [] + + async def assigned(repository, number): + calls.append(("assigned", repository, number)) + return True + + async def candidates(repository): + calls.append(("candidates", repository)) + return [{"login": "alex", "name": "Alexander"}] + + async def handoff(repository, number, recipient): + calls.append(("handoff", repository, number, recipient)) + return {"repository": repository, "number": number, "assignees": [recipient], "recipient": recipient} + + async def release(repository, number): + calls.append(("release", repository, number)) + return {"repository": repository, "number": number, "assignees": []} + + monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned) + monkeypatch.setattr(main.gitea_proxy, "pull_handoff_candidates", candidates, raising=False) + monkeypatch.setattr(main.gitea_proxy, "handoff_assigned_pull", handoff, raising=False) + monkeypatch.setattr(main.gitea_proxy, "release_assigned_pull", release, 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/handoff-candidates") + transferred = await client.patch( + "/api/v1/repos/stackchain/api/pulls/7/handoff", json={"recipient": "alex"} + ) + released = await client.patch("/api/v1/repos/stackchain/api/pulls/7/release") + + assert listed.status_code == 200 + assert listed.headers["cache-control"] == "no-store" + assert listed.json() == [{"login": "alex", "name": "Alexander"}] + assert transferred.status_code == 200 + assert transferred.json()["recipient"] == "alex" + assert released.status_code == 200 + assert released.json()["assignees"] == [] + assert calls == [ + ("assigned", "stackchain/api", 7), + ("candidates", "stackchain/api"), + ("handoff", "stackchain/api", 7, "alex"), + ("release", "stackchain/api", 7), + ] + + +@pytest.mark.anyio +async def test_gitea_pull_handoff_preserves_coassignees_and_confirms_ownership_exit(): + requests = [] + + async def handler(request): + 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("/pulls/7"): + return httpx.Response(200, json={ + "number": 7, "state": "open", "merged": False, + "assignees": [{"login": "timmy"}, {"login": "sam"}], + }) + if request.url.path.endswith("/assignees"): + return httpx.Response(200, json=[ + {"login": "timmy"}, {"login": "sam"}, + {"login": "alex", "full_name": "Alexander"}, + ]) + if request.method == "PATCH" and request.url.path.endswith("/issues/7"): + assert request.read() + return httpx.Response(200, json={ + "number": 7, "state": "open", + "assignees": [{"login": "sam"}, {"login": "alex"}], + }) + raise AssertionError(f"unexpected request: {request.method} {request.url.path}") + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + result = await gitea_proxy.handoff_assigned_pull("stackchain/api", 7, "alex") + finally: + await gitea_proxy.stop_client() + + assert result == { + "repository": "stackchain/api", "number": 7, "state": "open", + "assignees": ["sam", "alex"], "recipient": "alex", + } + assert [(method, path) for method, path, _body in requests] == [ + ("GET", "/api/v1/user"), + ("GET", "/api/v1/repos/stackchain/api/pulls/7"), + ("GET", "/api/v1/user"), + ("GET", "/api/v1/repos/stackchain/api/assignees"), + ("PATCH", "/api/v1/repos/stackchain/api/issues/7"), + ] + + +@pytest.mark.anyio +async def test_pull_handoff_candidates_are_bounded_and_exclude_invalid_or_current_logins(): + async def handler(request): + 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"}, {"login": "bad login"}, {"login": ""}] + + [{"login": f"user-{index:02d}"} for index in range(30)] + )) + raise AssertionError(f"unexpected request: {request.method} {request.url.path}") + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + candidates = await gitea_proxy.pull_handoff_candidates("stackchain/api") + finally: + await gitea_proxy.stop_client() + + assert len(candidates) == 25 + assert [candidate["login"] for candidate in candidates] == [ + f"user-{index:02d}" for index in range(25) + ] + + +@pytest.mark.anyio +async def test_gitea_pull_release_removes_current_login_case_insensitively(): + async def handler(request): + if request.url.path.endswith("/user"): + return httpx.Response(200, json={"login": "timmy"}) + if request.url.path.endswith("/pulls/7"): + return httpx.Response(200, json={ + "number": 7, "state": "open", "merged": False, + "assignees": [{"login": "Timmy"}, {"login": "sam"}], + }) + if request.method == "PATCH" and request.url.path.endswith("/issues/7"): + assert request.content == b'{"assignees":["sam"]}' + return httpx.Response(200, json={ + "number": 7, "state": "open", "assignees": [{"login": "sam"}], + }) + raise AssertionError(f"unexpected request: {request.method} {request.url.path}") + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + released = await gitea_proxy.release_assigned_pull("stackchain/api", 7) + finally: + await gitea_proxy.stop_client() + + assert released["assignees"] == ["sam"]