feat: make assigned pull sheets read-first (#217)
This commit is contained in:
1 parent
ea1f4ad837
commit
174dac05b9
6 files changed
+251
-26
No files matched your search
@@ -2023,6 +2023,63 @@ process.stdout.write(JSON.stringify({{
|
||||
assert output["next"] == "frontend/app.js"
|
||||
|
||||
|
||||
def test_pull_sheet_lazy_review_is_single_flight_cached_by_head_and_retryable():
|
||||
script = f"""
|
||||
const createPullSheet = require({json.dumps(str(PULL_SHEET))});
|
||||
const calls = [];
|
||||
let rejectFirst;
|
||||
const fetchJson = url => {{
|
||||
calls.push(url);
|
||||
if (calls.length === 1) return new Promise((_resolve, reject) => {{ rejectFirst = reject; }});
|
||||
return Promise.resolve({{head_sha:'abc123', files:[]}});
|
||||
}};
|
||||
const sheet = createPullSheet({{fetchJson, storage:null}});
|
||||
const item = {{repository:'stackchain/api', number:7}};
|
||||
const first = sheet.loadReview(item, 'abc123');
|
||||
const concurrent = sheet.loadReview(item, 'abc123');
|
||||
rejectFirst(new Error('diff unavailable'));
|
||||
Promise.allSettled([first, concurrent]).then(async failed => {{
|
||||
const retried = await sheet.loadReview(item, 'abc123');
|
||||
const cached = await sheet.loadReview(item, 'abc123');
|
||||
process.stdout.write(JSON.stringify({{
|
||||
same:first === concurrent,
|
||||
failed:failed.map(result => result.status),
|
||||
retried,
|
||||
cached,
|
||||
calls,
|
||||
}}));
|
||||
}});
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
output = json.loads(result.stdout)
|
||||
|
||||
assert output["same"] is True
|
||||
assert output["failed"] == ["rejected", "rejected"]
|
||||
assert output["retried"]["head_sha"] == "abc123"
|
||||
assert output["cached"] == output["retried"]
|
||||
assert output["calls"] == [
|
||||
"api/v1/repos/stackchain/api/pulls/7/review-data",
|
||||
"api/v1/repos/stackchain/api/pulls/7/review-data",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_mobile_pull_sheet_puts_reading_before_collapsed_review_controls():
|
||||
html = await dashboard()
|
||||
|
||||
body = html.index('id="pull-sheet-body"')
|
||||
conversation = html.index('<h2>Full conversation</h2>', body)
|
||||
composer = html.index('id="pull-comment-title"', conversation)
|
||||
review = html.index('id="pull-review"', composer)
|
||||
files = html.index('id="pull-files"', review)
|
||||
|
||||
assert '<summary><h2>Review & merge</h2></summary>' in html
|
||||
assert '<details class="pull-review" id="pull-review">' in html
|
||||
assert 'id="pull-review-retry"' in html
|
||||
assert 'id="pull-review-status"' in html
|
||||
assert body < conversation < composer < review < files
|
||||
|
||||
|
||||
def test_pull_sheet_renders_mobile_diff_fallbacks_and_review_controls():
|
||||
script = f"""
|
||||
const createPullSheet = require({json.dumps(str(PULL_SHEET))});
|
||||
|
||||
+73
-5
@@ -54,7 +54,77 @@ async def test_assigned_pull_detail_rejects_unassigned_pull(monkeypatch):
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_gitea_assigned_pull_detail_includes_bounded_diff_previews():
|
||||
async def test_gitea_assigned_pull_detail_loads_reading_without_review_resources():
|
||||
requests = []
|
||||
|
||||
async def handler(request):
|
||||
requests.append((request.method, request.url.path))
|
||||
if request.url.path.endswith("/pulls/7"):
|
||||
return httpx.Response(200, json={
|
||||
"number": 7,
|
||||
"title": "Read this first",
|
||||
"body": "The discussion should not wait for the diff.",
|
||||
"state": "open",
|
||||
"mergeable": True,
|
||||
"head": {"sha": "abc123"},
|
||||
"user": {"login": "alex"},
|
||||
})
|
||||
if request.url.path.endswith("/issues/7/comments"):
|
||||
return httpx.Response(200, json=[{
|
||||
"id": 9, "body": "Question", "user": {"login": "sam"},
|
||||
"created_at": "2026-08-07T18:00:00Z",
|
||||
}], headers={"X-Total-Count": "1"})
|
||||
raise AssertionError(f"review resource requested during read load: {request.url.path}")
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
detail = await gitea_proxy.pull_completion_detail("stackchain/api", 7)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert requests == [
|
||||
("GET", "/api/v1/repos/stackchain/api/pulls/7"),
|
||||
("GET", "/api/v1/repos/stackchain/api/issues/7/comments"),
|
||||
]
|
||||
assert detail["head_sha"] == "abc123"
|
||||
assert detail["conversation"]["comments"][0]["body"] == "Question"
|
||||
assert "files" not in detail
|
||||
assert "ci_state" not in detail
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_assigned_pull_review_endpoint_loads_review_payload_on_demand(monkeypatch):
|
||||
calls = []
|
||||
|
||||
async def assigned(repository, number):
|
||||
calls.append(("assigned", repository, number))
|
||||
return True
|
||||
|
||||
async def review(repository, number):
|
||||
calls.append(("review", repository, number))
|
||||
return {
|
||||
"repository": repository, "number": number, "head_sha": "abc123",
|
||||
"state": "open", "draft": False, "mergeable": True, "merged": False,
|
||||
"ci_state": "success", "files": [{"filename": "src/api.py"}],
|
||||
}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "pull_completion_review", review, raising=False)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.get("/api/v1/repos/stackchain/api/pulls/7/review-data")
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.headers["cache-control"] == "no-store"
|
||||
assert response.json()["files"] == [{"filename": "src/api.py"}]
|
||||
assert calls == [
|
||||
("assigned", "stackchain/api", 7),
|
||||
("review", "stackchain/api", 7),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_gitea_assigned_pull_review_includes_bounded_diff_previews():
|
||||
async def handler(request):
|
||||
path = request.url.path
|
||||
if path.endswith("/pulls/7"):
|
||||
@@ -86,7 +156,7 @@ async def test_gitea_assigned_pull_detail_includes_bounded_diff_previews():
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
detail = await gitea_proxy.pull_completion_detail("stackchain/api", 7)
|
||||
detail = await gitea_proxy.pull_completion_review("stackchain/api", 7)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
@@ -94,9 +164,7 @@ async def test_gitea_assigned_pull_detail_includes_bounded_diff_previews():
|
||||
assert "+new" in detail["files"][0]["diff_lines"]
|
||||
assert detail["files"][1]["diff_binary"] is True
|
||||
assert detail["files"][1]["diff_available"] is False
|
||||
assert detail["conversation"] == {
|
||||
"comments": [], "page": 1, "older_page": None, "total": 0
|
||||
}
|
||||
assert "conversation" not in detail
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
|
||||
Reference in new issue
Block a user