feat: hand off assigned pull requests (Closes #567)
This commit is contained in:
1 parent
50d51ed159
commit
0304f5ab2c
9 files changed
+627
-30
No files matched your search
@@ -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():
|
||||
|
||||
+134
-1
@@ -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
|
||||
|
||||
@@ -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"]
|
||||
Reference in new issue
Block a user