From c63b23865c9b685141129ab86f51298716a5ef05 Mon Sep 17 00:00:00 2001 From: timmy Date: Fri, 7 Aug 2026 16:22:30 +0000 Subject: [PATCH] feat: load complete mobile conversations (#209) --- frontend/conversation.js | 69 +++++++++++++++++++++++++ frontend/index.html | 89 +++++++++++++++++++++++++++----- frontend/issue-sheet.js | 11 +++- frontend/pull-sheet.js | 11 +++- src/gitea_proxy.py | 71 +++++++++++++++++++------ src/main.py | 58 +++++++++++++++++++++ tests/test_issue_api.py | 108 +++++++++++++++++++++++++++++++++++++++ tests/test_my_work.py | 107 ++++++++++++++++++++++++++++++++++++++ tests/test_pull_api.py | 34 ++++++++++++ 9 files changed, 528 insertions(+), 30 deletions(-) create mode 100644 frontend/conversation.js diff --git a/frontend/conversation.js b/frontend/conversation.js new file mode 100644 index 0000000..f64af9f --- /dev/null +++ b/frontend/conversation.js @@ -0,0 +1,69 @@ +function createConversationPager({ loadPage }) { + let state = { comments: [], page: 1, older_page: null, total: 0 }; + let olderRequest = null; + + const validComments = comments => Array.isArray(comments) + ? comments.filter(comment => comment && Number.isInteger(comment.id)) + : []; + + const unique = comments => { + const seen = new Set(); + return comments.filter(comment => { + if (seen.has(comment.id)) return false; + seen.add(comment.id); + return true; + }).sort((left, right) => { + const leftTime = Date.parse(left.created_at || ''); + const rightTime = Date.parse(right.created_at || ''); + if (Number.isFinite(leftTime) && Number.isFinite(rightTime) && leftTime !== rightTime) { + return leftTime - rightTime; + } + return left.id - right.id; + }); + }; + + const snapshot = () => ({ ...state, comments: state.comments.map(comment => ({ ...comment })) }); + + return { + reset(page) { + const comments = unique(validComments(page?.comments)); + state = { + comments, + page: Number.isInteger(page?.page) ? page.page : 1, + older_page: Number.isInteger(page?.older_page) ? page.older_page : null, + total: Number.isInteger(page?.total) ? Math.max(page.total, comments.length) : comments.length, + }; + olderRequest = null; + return snapshot(); + }, + snapshot, + loadOlder() { + if (olderRequest) return olderRequest; + if (!Number.isInteger(state.older_page)) return Promise.resolve(snapshot()); + const requestedPage = state.older_page; + olderRequest = Promise.resolve(loadPage(requestedPage)).then(page => { + state = { + comments: unique(validComments(page?.comments).concat(state.comments)), + page: Number.isInteger(page?.page) ? page.page : requestedPage, + older_page: Number.isInteger(page?.older_page) ? page.older_page : null, + total: Number.isInteger(page?.total) ? Math.max(page.total, state.comments.length) : state.total, + }; + return snapshot(); + }).finally(() => { olderRequest = null; }); + return olderRequest; + }, + append(comment) { + if (!comment || !Number.isInteger(comment.id)) return snapshot(); + if (!state.comments.some(existing => existing.id === comment.id)) { + state = { + ...state, + comments: state.comments.concat(comment), + total: Math.max(state.total + 1, state.comments.length + 1), + }; + } + return snapshot(); + }, + }; +} + +if (typeof module !== 'undefined' && module.exports) module.exports = createConversationPager; diff --git a/frontend/index.html b/frontend/index.html index 65967cf..57b10e3 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -212,6 +212,7 @@ textarea { resize: vertical; min-height: 120px; } .pull-sheet-header button, .pull-sheet-actions button, .pull-sheet-actions a, .pull-comment-composer button { min-height:44px; } .pull-sheet-content { overflow-wrap:anywhere; white-space:pre-wrap; } .pull-file, .pull-comment-card { margin:8px 0; padding:10px; border:1px solid #203a5c; border-radius:10px; } +.conversation-more { min-height:44px; width:100%; margin:8px 0; } .pull-file-toggle, .pull-review-file { min-height:44px; width:100%; } .pull-file-toggle { display:flex; justify-content:space-between; align-items:center; gap:8px; text-align:left; } .pull-review-file { margin-top:8px; } @@ -449,8 +450,10 @@ textarea { resize: vertical; min-height: 120px; }
-

Recent discussion

+

Full conversation

+ +

Add comment

@@ -569,7 +572,9 @@ textarea { resize: vertical; min-height: 120px; }

Changed files

Review progress unavailable.
-

Recent discussion

+

Full conversation

+ +

Add comment

@@ -657,6 +662,7 @@ textarea { resize: vertical; min-height: 120px; } + @@ -713,10 +719,12 @@ textarea { resize: vertical; min-height: 120px; } let updateTrigger = null; let selectedIssue = null; let selectedIssueDetail = null; + let issueConversation = null; let issueTrigger = null; let selectedPull = null; let pullTrigger = null; let selectedPullDetail = null; + let pullConversation = null; let pullReviewState = null; let creatingIssue = false; let findingWork = false; @@ -1271,13 +1279,32 @@ textarea { resize: vertical; min-height: 120px; } } function renderIssueComment(comment) { - return '
' + + return '
' + escapeHtml(comment.author || 'Unknown author') + (comment.created_at ? ' · ' + escapeHtml(fmt(comment.created_at)) : '') + '
' + escapeHtml(comment.body || 'No comment body provided.') + '
'; } + function renderIssueConversation(state) { + const comments = state?.comments || []; + qs('#issue-comments').innerHTML = comments.length ? + comments.map(renderIssueComment).join('') : '
No comments yet.
'; + qs('#load-older-issue-comments').hidden = !Number.isInteger(state?.older_page); + qs('#issue-conversation-status').textContent = comments.length ? + comments.length + ' of ' + Math.max(state.total || 0, comments.length) + ' messages loaded.' : 'No comments yet.'; + } + + function renderPullConversation(state) { + const comments = state?.comments || []; + qs('#pull-comments').innerHTML = comments.length ? comments.map(comment => + '
' + renderIssueComment(comment) + '
' + ).join('') : '
No comments yet.
'; + qs('#load-older-pull-comments').hidden = !Number.isInteger(state?.older_page); + qs('#pull-conversation-status').textContent = comments.length ? + comments.length + ' of ' + Math.max(state.total || 0, comments.length) + ' messages loaded.' : 'No comments yet.'; + } + async function loadIssueLabelEditor(item, confirmedNames) { const list = qs('#issue-label-list'); const status = qs('#issue-label-status'); @@ -1336,6 +1363,7 @@ textarea { resize: vertical; min-height: 120px; } if (!item) return; selectedIssue = item; selectedIssueDetail = null; + issueConversation = null; issueTrigger = trigger; qs('#issue-sheet').classList.add('open'); qs('#issue-sheet-key').textContent = item.key || ''; @@ -1345,6 +1373,8 @@ textarea { resize: vertical; min-height: 120px; } qs('#issue-labels').textContent = ''; qs('#issue-assignees').textContent = ''; qs('#issue-comments').textContent = ''; + qs('#load-older-issue-comments').hidden = true; + qs('#issue-conversation-status').textContent = 'Loading newest messages…'; qs('#issue-comment').value = issueController.loadDraft(item); qs('#issue-comment-status').textContent = ''; qs('#issue-label-list').textContent = ''; @@ -1371,6 +1401,7 @@ textarea { resize: vertical; min-height: 120px; } const detail = await issueController.load(item); if (selectedIssue !== item) return; selectedIssueDetail = detail; + issueConversation = issueController.conversation(item, detail.conversation); qs('#issue-sheet-title').textContent = detail.title || 'Assigned issue'; qs('#issue-sheet-body').textContent = detail.body || 'No description provided.'; qs('#issue-labels').innerHTML = (detail.labels || []).map(label => @@ -1380,8 +1411,7 @@ textarea { resize: vertical; min-height: 120px; } loadIssueMilestoneEditor(item, detail.milestone); qs('#issue-assignees').textContent = (detail.assignees || []).length ? 'Assigned to ' + detail.assignees.join(', ') : 'No assignee reported'; - qs('#issue-comments').innerHTML = (detail.comments || []).length ? - detail.comments.map(renderIssueComment).join('') : '
No comments yet.
'; + renderIssueConversation(issueConversation.snapshot()); qs('#open-issue-gitea').href = detail.url || item.url || '#'; qs('#issue-sheet-status').textContent = 'Issue ready · ' + (detail.state || 'open'); qs('#edit-issue-content').disabled = false; @@ -1408,6 +1438,7 @@ textarea { resize: vertical; min-height: 120px; } qs('#issue-sheet').classList.remove('open'); selectedIssue = null; selectedIssueDetail = null; + issueConversation = null; if (issueTrigger?.isConnected) issueTrigger.focus(); } @@ -1463,6 +1494,7 @@ textarea { resize: vertical; min-height: 120px; } selectedPull = item; pullTrigger = trigger; selectedPullDetail = null; + pullConversation = null; pullReviewState = null; qs('#pull-sheet').classList.add('open'); qs('#pull-sheet-key').textContent = item.key || ''; @@ -1473,6 +1505,8 @@ textarea { resize: vertical; min-height: 120px; } qs('#pull-review-progress').textContent = 'Loading review progress…'; qs('#next-unreviewed-pull-file').disabled = true; qs('#pull-comments').textContent = ''; + qs('#load-older-pull-comments').hidden = true; + qs('#pull-conversation-status').textContent = 'Loading newest messages…'; qs('#pull-comment').value = pullController.loadDraft(item); qs('#pull-comment-status').textContent = ''; qs('#pull-ci-state').textContent = 'CI unknown'; @@ -1485,13 +1519,12 @@ textarea { resize: vertical; min-height: 120px; } const detail = await pullController.load(item); if (selectedPull !== item) return; selectedPullDetail = detail; + pullConversation = pullController.conversation(item, detail.conversation); qs('#pull-sheet-title').textContent = detail.title || 'Assigned pull request'; qs('#pull-sheet-body').textContent = detail.body || 'No description provided.'; qs('#pull-ci-state').textContent = 'CI ' + (detail.ci_state || 'unknown'); renderPullReview(detail); - qs('#pull-comments').innerHTML = (detail.comments || []).length ? detail.comments.map(comment => - '
' + renderIssueComment(comment) + '
' - ).join('') : '
No comments yet.
'; + renderPullConversation(pullConversation.snapshot()); qs('#open-pull-gitea').href = detail.url || item.url || '#'; qs('#pull-sheet-status').textContent = 'Pull request ready · by ' + (detail.author || 'unknown author'); } catch (error) { @@ -1510,6 +1543,7 @@ textarea { resize: vertical; min-height: 120px; } qs('#pull-sheet').classList.remove('open'); selectedPull = null; selectedPullDetail = null; + pullConversation = null; pullReviewState = null; if (pullTrigger?.isConnected) pullTrigger.focus(); } @@ -2326,6 +2360,22 @@ textarea { resize: vertical; min-height: 120px; } qs('#issue-comment').addEventListener('input', event => { if (selectedIssue) issueController.saveDraft(selectedIssue, event.target.value); }); + qs('#load-older-issue-comments').addEventListener('click', async () => { + if (!issueConversation) return; + const button = qs('#load-older-issue-comments'); + const panel = qs('#issue-sheet .issue-sheet-panel'); + const previousHeight = panel.scrollHeight; + button.disabled = true; + qs('#issue-conversation-status').textContent = 'Loading older messages…'; + try { + renderIssueConversation(await issueConversation.loadOlder()); + panel.scrollTop += panel.scrollHeight - previousHeight; + } catch (error) { + qs('#issue-conversation-status').textContent = error.message + ' Loaded messages and your draft are safe; retry.'; + } finally { + button.disabled = false; + } + }); qs('#save-issue-labels').addEventListener('click', async () => { if (!selectedIssue || !lastContextSnapshot) return; const editing = selectedIssue; @@ -2428,9 +2478,7 @@ textarea { resize: vertical; min-height: 120px; } qs('#issue-comment-status').textContent = 'Posting comment…'; try { const comment = await issueController.comment(selectedIssue, body); - const empty = qs('#issue-comments .muted'); - if (empty) empty.remove(); - qs('#issue-comments').insertAdjacentHTML('beforeend', renderIssueComment(comment)); + if (issueConversation) renderIssueConversation(issueConversation.append(comment)); qs('#issue-comment').value = ''; qs('#issue-comment-status').textContent = 'Comment posted.'; } catch (error) { @@ -2486,6 +2534,22 @@ textarea { resize: vertical; min-height: 120px; } if (selectedPull) openPullSheet(selectedPull, pullTrigger); }); qs('#next-unreviewed-pull-file').addEventListener('click', focusNextUnreviewedPullFile); + qs('#load-older-pull-comments').addEventListener('click', async () => { + if (!pullConversation) return; + const button = qs('#load-older-pull-comments'); + const panel = qs('#pull-sheet .pull-sheet-panel'); + const previousHeight = panel.scrollHeight; + button.disabled = true; + qs('#pull-conversation-status').textContent = 'Loading older messages…'; + try { + renderPullConversation(await pullConversation.loadOlder()); + panel.scrollTop += panel.scrollHeight - previousHeight; + } catch (error) { + qs('#pull-conversation-status').textContent = error.message + ' Loaded messages and your draft are safe; retry.'; + } finally { + button.disabled = false; + } + }); qs('#pull-comment').addEventListener('input', event => { if (selectedPull) pullController.saveDraft(selectedPull, event.target.value); }); @@ -2502,8 +2566,7 @@ textarea { resize: vertical; min-height: 120px; } qs('#pull-comment-status').textContent = 'Posting comment…'; try { const comment = await pullController.comment(selectedPull, body); - qs('#pull-comments .muted')?.remove(); - qs('#pull-comments').insertAdjacentHTML('beforeend', '
' + renderIssueComment(comment) + '
'); + if (pullConversation) renderPullConversation(pullConversation.append(comment)); qs('#pull-comment').value = ''; qs('#pull-comment-status').textContent = 'Comment posted.'; } catch (error) { diff --git a/frontend/issue-sheet.js b/frontend/issue-sheet.js index 556a648..d17069d 100644 --- a/frontend/issue-sheet.js +++ b/frontend/issue-sheet.js @@ -1,4 +1,4 @@ -function createIssueSheet({ fetchJson, storage, createOperationId = () => globalThis.crypto?.randomUUID?.() || String(Date.now()) + '-' + Math.random() }) { +function createIssueSheet({ fetchJson, storage, createConversationPager = globalThis.createConversationPager, createOperationId = () => globalThis.crypto?.randomUUID?.() || String(Date.now()) + '-' + Math.random() }) { let commentRequest = null; let closeRequest = null; let releaseRequest = null; @@ -21,6 +21,15 @@ function createIssueSheet({ fetchJson, storage, createOperationId = () => global headers: { Accept: 'application/json' }, }); }, + conversation(item, initialPage) { + const pager = createConversationPager({ + loadPage: page => fetchJson(issuePath(item) + '/comments?page=' + encodeURIComponent(page) + '&limit=20', { + headers: { Accept: 'application/json' }, + }), + }); + pager.reset(initialPage || { comments: [], page: 1, older_page: null, total: 0 }); + return pager; + }, loadLabels(item) { return fetchJson(issuePath(item) + '/labels', { headers: { Accept: 'application/json' }, diff --git a/frontend/pull-sheet.js b/frontend/pull-sheet.js index 78baf33..762dbb4 100644 --- a/frontend/pull-sheet.js +++ b/frontend/pull-sheet.js @@ -37,7 +37,7 @@ function renderFile(file, index, reviewed, escapeHtml) { '" aria-pressed="' + String(reviewed) + '">' + (reviewed ? 'Reviewed' : 'Mark reviewed') + ''; } -function createPullSheet({ fetchJson, storage, createOperationId = () => globalThis.crypto?.randomUUID?.() || String(Date.now()) + '-' + Math.random() }) { +function createPullSheet({ fetchJson, storage, createConversationPager = globalThis.createConversationPager, createOperationId = () => globalThis.crypto?.randomUUID?.() || String(Date.now()) + '-' + Math.random() }) { let commentRequest = null; let mergeRequest = null; const pathFor = item => 'api/v1/repos/' + String(item.repository || '').split('/') @@ -62,6 +62,15 @@ function createPullSheet({ fetchJson, storage, createOperationId = () => globalT load(item) { return fetchJson(pathFor(item) + '/detail', { headers: { Accept: 'application/json' } }); }, + conversation(item, initialPage) { + const pager = createConversationPager({ + loadPage: page => fetchJson(pathFor(item) + '/comments?page=' + encodeURIComponent(page) + '&limit=20', { + headers: { Accept: 'application/json' }, + }), + }); + pager.reset(initialPage || { comments: [], page: 1, older_page: null, total: 0 }); + return pager; + }, reviewState, toggleReviewed(item, detail, filename) { const state = reviewState(item, detail); diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 86d8ac9..97ab58d 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -942,10 +942,58 @@ async def update_assigned_issue_milestone( } +async def issue_conversation_page( + repository: str, + number: int, + page: int | None = None, + limit: int = 20, +) -> dict: + """Return one bounded comment page, opening on the newest page by default.""" + bounded_limit = min(50, max(1, limit)) + requested_page = max(1, page or 1) + path = f"/api/v1/repos/{repository}/issues/{number}/comments" + + async def load(selected_page: int) -> tuple[list, int]: + response = await _get_client().get( + path, + headers=_auth(), + params={"limit": bounded_limit, "page": selected_page}, + ) + response.raise_for_status() + payload = response.json() + if not isinstance(payload, list): + raise ValueError("Gitea issue comments response was not a list") + comments = [item for item in payload if isinstance(item, dict)] + try: + total = max(len(comments), int(response.headers.get("X-Total-Count", len(comments)))) + except (TypeError, ValueError): + total = len(comments) + return comments, total + + comments, total = await load(requested_page) + newest_page = max(1, (total + bounded_limit - 1) // bounded_limit) + if len(comments) >= total: + selected_page = newest_page if page is None else min(requested_page, newest_page) + start = (selected_page - 1) * bounded_limit + comments = comments[start:start + bounded_limit] + else: + selected_page = requested_page + if page is None and newest_page != requested_page: + selected_page = newest_page + comments, confirmed_total = await load(selected_page) + total = max(total, confirmed_total) + return { + "comments": [_normalize_issue_comment(item) for item in comments], + "page": selected_page, + "older_page": selected_page - 1 if selected_page > 1 else None, + "total": total, + } + + async def issue_detail(repository: str, number: int) -> dict: base = f"repos/{repository}/issues/{number}" - issue, comments = await asyncio.gather( - fetch(base), fetch(f"{base}/comments?limit=20&page=1") + issue, conversation = await asyncio.gather( + fetch(base), issue_conversation_page(repository, number) ) if not isinstance(issue, dict): raise ValueError("Gitea issue response was not an object") @@ -953,12 +1001,7 @@ async def issue_detail(repository: str, number: int) -> dict: labels: list = labels_value if isinstance(labels_value, list) else [] assignees_value = issue.get("assignees") assignees: list = assignees_value if isinstance(assignees_value, list) else [] - comments_value: list = comments if isinstance(comments, list) else [] - normalized_comments = [ - _normalize_issue_comment(comment) - for comment in comments_value - if isinstance(comment, dict) - ] + normalized_comments = conversation["comments"] return { "repository": repository, "number": number, @@ -984,6 +1027,7 @@ async def issue_detail(repository: str, number: int) -> dict: if isinstance(assignee, dict) and isinstance(assignee.get("login"), str) ], "comments": normalized_comments, + "conversation": conversation, } @@ -1159,10 +1203,10 @@ async def pull_completion_detail(repository: str, number: int) -> dict: raise ValueError("Gitea pull request response was not an object") head = pull.get("head") if isinstance(pull.get("head"), dict) else {} sha = head.get("sha") if isinstance(head.get("sha"), str) else "" - files, status, comments, diff_result = await asyncio.gather( + files, status, conversation, diff_result = await asyncio.gather( fetch(f"{base}/files"), fetch(f"repos/{repository}/commits/{sha}/status"), - fetch(f"repos/{repository}/issues/{number}/comments?limit=20&page=1"), + issue_conversation_page(repository, number), fetch_text( f"repos/{repository}/pulls/{number}.diff", REVIEW_DIFF_MAX_BYTES ), @@ -1202,11 +1246,8 @@ async def pull_completion_detail(repository: str, number: int) -> dict: for item in (files if isinstance(files, list) else [])[:100] if isinstance(item, dict) and isinstance(item.get("filename"), str) ], - "comments": [ - _normalize_issue_comment(item) - for item in (comments if isinstance(comments, list) else [])[:20] - if isinstance(item, dict) - ], + "comments": conversation["comments"], + "conversation": conversation, } diff --git a/src/main.py b/src/main.py index 2abcb5d..0f9b051 100644 --- a/src/main.py +++ b/src/main.py @@ -1415,6 +1415,35 @@ async def create_assigned_issue( return JSONResponse(result, status_code=201) +@app.get("/api/v1/repos/{owner}/{repo}/issues/{number}/comments") +async def assigned_issue_conversation( + owner: str, + repo: str, + number: int = PathParam(gt=0), + page: int | None = Query(default=None, ge=1, le=100), + limit: int = Query(default=20, ge=1, le=50), +): + repository = f"{owner}/{repo}" + + async def load_conversation(): + if not await gitea_proxy.is_assigned_issue(repository, number): + raise HTTPException(status_code=404, detail="Assigned issue not found") + return await gitea_proxy.issue_conversation_page(repository, number, page, limit) + + try: + return await asyncio.wait_for( + load_conversation(), timeout=ISSUE_ACTION_TIMEOUT_SECONDS + ) + except HTTPException: + raise + except Exception: + return JSONResponse( + {"error": "The conversation could not be loaded. Your draft is safe; please retry."}, + status_code=503, + headers={"Retry-After": "1"}, + ) + + @app.post("/api/v1/repos/{owner}/{repo}/issues/{number}/comments", status_code=201) async def comment_on_assigned_issue( comment: IssueComment, @@ -1587,6 +1616,35 @@ async def assigned_pull_detail(owner: str, repo: str, number: int = PathParam(gt ) +@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/comments") +async def assigned_pull_conversation( + owner: str, + repo: str, + number: int = PathParam(gt=0), + page: int | None = Query(default=None, ge=1, le=100), + limit: int = Query(default=20, ge=1, le=50), +): + repository = f"{owner}/{repo}" + + async def load_conversation(): + if not await gitea_proxy.is_assigned_pull(repository, number): + raise HTTPException(status_code=404, detail="Assigned pull request not found") + return await gitea_proxy.issue_conversation_page(repository, number, page, limit) + + try: + return await asyncio.wait_for( + load_conversation(), timeout=ISSUE_ACTION_TIMEOUT_SECONDS + ) + except HTTPException: + raise + except Exception: + return JSONResponse( + {"error": "The conversation could not be loaded. Your draft is safe; please retry."}, + status_code=503, + headers={"Retry-After": "1"}, + ) + + @app.post("/api/v1/repos/{owner}/{repo}/pulls/{number}/comments", status_code=201) async def comment_on_assigned_pull( comment: IssueComment, diff --git a/tests/test_issue_api.py b/tests/test_issue_api.py index 84d81e3..165e828 100644 --- a/tests/test_issue_api.py +++ b/tests/test_issue_api.py @@ -912,6 +912,44 @@ async def test_issue_detail_endpoint_returns_assigned_issue_with_no_store(monkey } +@pytest.mark.anyio +async def test_assigned_issue_conversation_endpoint_is_authorized_bounded_and_no_store(monkeypatch): + calls = [] + + async def assigned(repository, number): + calls.append(("assigned", repository, number)) + return repository == "stackchain/api" + + async def conversation(repository, number, page, limit): + calls.append(("conversation", repository, number, page, limit)) + return {"comments": [{"id": 21}], "page": 2, "older_page": 1, "total": 21} + + monkeypatch.setattr(main.gitea_proxy, "is_assigned_issue", assigned) + monkeypatch.setattr(main.gitea_proxy, "issue_conversation_page", conversation) + 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/issues/7/comments?page=2&limit=20" + ) + missing = await client.get( + "/api/v1/repos/private/secret/issues/7/comments?page=2&limit=20" + ) + invalid = await client.get( + "/api/v1/repos/stackchain/api/issues/7/comments?page=999&limit=999" + ) + + assert response.status_code == 200 + assert response.headers["cache-control"] == "no-store" + assert response.json()["older_page"] == 1 + assert missing.status_code == 404 + assert invalid.status_code == 422 + assert calls == [ + ("assigned", "stackchain/api", 7), + ("conversation", "stackchain/api", 7, 2, 20), + ("assigned", "private/secret", 7), + ] + + @pytest.mark.anyio async def test_issue_detail_deadline_is_retryable_sanitized_and_cancels_work(monkeypatch): cancelled = asyncio.Event() @@ -1157,9 +1195,79 @@ async def test_gitea_issue_detail_returns_normalized_context_and_recent_comments "url": "https://forge.example/stackchain/api/issues/7#issuecomment-81", } ], + "conversation": { + "comments": [ + { + "id": 81, + "author": "sam", + "body": "Latest update", + "created_at": "2026-08-07T10:00:00Z", + "url": "https://forge.example/stackchain/api/issues/7#issuecomment-81", + } + ], + "page": 1, + "older_page": None, + "total": 1, + }, } +@pytest.mark.anyio +async def test_gitea_conversation_page_opens_newest_page_and_reports_older_history(): + requests = [] + + async def handler(request): + requests.append((request.url.path, request.url.query.decode())) + page = request.url.params.get("page") + comments = { + "1": [{"id": value, "body": f"Comment {value}", "user": {"login": "sam"}} + for value in range(1, 21)], + "3": [{"id": value, "body": f"Comment {value}", "user": {"login": "sam"}} + for value in range(41, 48)], + }[page] + return httpx.Response(200, json=comments, headers={"X-Total-Count": "47"}) + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + result = await gitea_proxy.issue_conversation_page("stackchain/api", 7) + finally: + await gitea_proxy.stop_client() + + assert requests == [ + ("/api/v1/repos/stackchain/api/issues/7/comments", "limit=20&page=1"), + ("/api/v1/repos/stackchain/api/issues/7/comments", "limit=20&page=3"), + ] + assert [comment["id"] for comment in result["comments"]] == list(range(41, 48)) + assert result["page"] == 3 + assert result["older_page"] == 2 + assert result["total"] == 47 + + +@pytest.mark.anyio +async def test_gitea_conversation_page_locally_bounds_unpaginated_gitea_comments(): + requests = [] + all_comments = [ + {"id": value, "body": f"Comment {value}", "user": {"login": "sam"}} + for value in range(1, 48) + ] + + async def handler(request): + requests.append(request.url.query.decode()) + return httpx.Response(200, json=all_comments, headers={"X-Total-Count": "47"}) + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + result = await gitea_proxy.issue_conversation_page("stackchain/api", 7) + finally: + await gitea_proxy.stop_client() + + assert requests == ["limit=20&page=1"] + assert [comment["id"] for comment in result["comments"]] == list(range(41, 48)) + assert result["page"] == 3 + assert result["older_page"] == 2 + assert result["total"] == 47 + + @pytest.mark.anyio async def test_update_assigned_issue_labels_validates_and_returns_confirmed_labels(monkeypatch): calls = [] diff --git a/tests/test_my_work.py b/tests/test_my_work.py index 47747f6..317a087 100644 --- a/tests/test_my_work.py +++ b/tests/test_my_work.py @@ -12,6 +12,7 @@ REVIEW_SHEET = Path(__file__).parents[1] / "frontend" / "review-sheet.js" ISSUE_SHEET = Path(__file__).parents[1] / "frontend" / "issue-sheet.js" CREATE_ISSUE_SHEET = Path(__file__).parents[1] / "frontend" / "create-issue-sheet.js" PULL_SHEET = Path(__file__).parents[1] / "frontend" / "pull-sheet.js" +CONVERSATION = Path(__file__).parents[1] / "frontend" / "conversation.js" PICK_WORK = Path(__file__).parents[1] / "frontend" / "pick-work.js" WORK_ROUTE = Path(__file__).parents[1] / "frontend" / "work-route.js" @@ -1569,6 +1570,100 @@ reader.open(item, [item]).then(() => {{ ] +def test_conversation_pager_prepends_older_pages_deduplicates_and_appends_once(): + script = f""" +const createConversationPager = require({json.dumps(str(CONVERSATION))}); +let calls = 0; +const pager = createConversationPager({{ + loadPage: async page => {{ calls += 1; return {{ + comments:[{{id:20,body:'duplicate'}},{{id:1,body:'oldest'}}], + page, older_page:null, total:3 + }}; }} +}}); +pager.reset({{comments:[{{id:20,body:'middle'}},{{id:21,body:'newest'}}],page:2,older_page:1,total:3}}); +const first = pager.loadOlder(); +const duplicate = pager.loadOlder(); +Promise.all([first, duplicate]).then(() => {{ + pager.append({{id:22,body:'posted'}}); + pager.append({{id:22,body:'posted'}}); + process.stdout.write(JSON.stringify({{calls,same:first===duplicate,state:pager.snapshot()}})); +}}); +""" + result = subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ) + output = json.loads(result.stdout) + assert output["calls"] == 1 + assert output["same"] is True + assert [comment["id"] for comment in output["state"]["comments"]] == [1, 20, 21, 22] + assert output["state"]["older_page"] is None + assert output["state"]["total"] == 4 + + +def test_conversation_pager_keeps_loaded_messages_when_older_page_fails(): + script = f""" +const createConversationPager = require({json.dumps(str(CONVERSATION))}); +const pager = createConversationPager({{loadPage: async () => {{ throw new Error('offline'); }}}}); +pager.reset({{comments:[{{id:21,body:'newest'}}],page:2,older_page:1,total:21}}); +pager.loadOlder().catch(error => process.stdout.write(JSON.stringify({{ + error:error.message, state:pager.snapshot() +}}))); +""" + result = subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ) + output = json.loads(result.stdout) + assert output["error"] == "offline" + assert output["state"]["comments"] == [{"id": 21, "body": "newest"}] + assert output["state"]["older_page"] == 1 + + +def test_issue_sheet_conversation_loads_older_history_through_assigned_boundary(): + script = f""" +const createIssueSheet = require({json.dumps(str(ISSUE_SHEET))}); +const createConversationPager = require({json.dumps(str(CONVERSATION))}); +const calls = []; +const controller = createIssueSheet({{ + createConversationPager, + fetchJson: async url => {{ calls.push(url); return {{comments:[{{id:1}}],page:1,older_page:null,total:21}}; }} +}}); +const pager = controller.conversation( + {{repository:'stackchain/api',number:7}}, + {{comments:[{{id:21}}],page:2,older_page:1,total:21}} +); +pager.loadOlder().then(state => process.stdout.write(JSON.stringify({{calls,state}}))); +""" + result = subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ) + output = json.loads(result.stdout) + assert output["calls"] == ["api/v1/repos/stackchain/api/issues/7/comments?page=1&limit=20"] + assert [comment["id"] for comment in output["state"]["comments"]] == [1, 21] + + +def test_pull_sheet_conversation_loads_older_history_through_assigned_boundary(): + script = f""" +const createPullSheet = require({json.dumps(str(PULL_SHEET))}); +const createConversationPager = require({json.dumps(str(CONVERSATION))}); +const calls = []; +const controller = createPullSheet({{ + createConversationPager, + fetchJson: async url => {{ calls.push(url); return {{comments:[{{id:1}}],page:1,older_page:null,total:21}}; }} +}}); +const pager = controller.conversation( + {{repository:'stackchain/api',number:7}}, + {{comments:[{{id:21}}],page:2,older_page:1,total:21}} +); +pager.loadOlder().then(state => process.stdout.write(JSON.stringify({{calls,state}}))); +""" + result = subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ) + output = json.loads(result.stdout) + assert output["calls"] == ["api/v1/repos/stackchain/api/pulls/7/comments?page=1&limit=20"] + assert [comment["id"] for comment in output["state"]["comments"]] == [1, 21] + + def test_issue_sheet_loads_encoded_assigned_issue_detail_path(): script = f""" const createIssueSheet = require({json.dumps(str(ISSUE_SHEET))}); @@ -1860,6 +1955,12 @@ async def test_assigned_pulls_open_accessible_mobile_completion_sheet(): assert 'id="pull-files"' in html and 'id="pull-comments"' in html assert 'id="pull-review-progress"' in html and 'aria-live="polite"' in html assert 'id="next-unreviewed-pull-file"' in html + assert 'id="load-older-pull-comments"' in html + assert 'id="pull-conversation-status"' in html and 'aria-live="assertive"' in html + assert '' in html + assert "pullController.conversation(item, detail.conversation)" in html + assert "pullConversation.loadOlder()" in html + assert "pullConversation.append(comment)" in html assert 'id="pull-comment"' in html and 'maxlength="10000"' in html assert 'id="merge-pull"' in html and 'id="open-pull-gitea"' in html assert '' in html @@ -1885,6 +1986,12 @@ async def test_assigned_issues_open_accessible_mobile_action_sheet_with_safe_mut assert 'id="issue-sheet-body"' in html assert 'id="issue-labels"' in html and 'id="issue-assignees"' in html assert 'id="issue-comments"' in html + assert 'id="load-older-issue-comments"' in html + assert 'id="issue-conversation-status"' in html and 'aria-live="assertive"' in html + assert "issueController.conversation(item, detail.conversation)" in html + assert "issueConversation.loadOlder()" in html + assert "issueConversation.append(comment)" in html + assert '.conversation-more { min-height:44px;' in html assert 'id="issue-comment"' in html and 'maxlength="10000"' in html assert 'id="send-issue-comment"' in html assert 'id="close-issue"' in html diff --git a/tests/test_pull_api.py b/tests/test_pull_api.py index 4a6a3cd..b05789a 100644 --- a/tests/test_pull_api.py +++ b/tests/test_pull_api.py @@ -94,6 +94,40 @@ 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 + } + + +@pytest.mark.anyio +async def test_assigned_pull_conversation_endpoint_reuses_issue_thread_with_pull_authorization(monkeypatch): + calls = [] + + async def assigned(repository, number): + calls.append(("assigned", repository, number)) + return True + + async def conversation(repository, number, page, limit): + calls.append(("conversation", repository, number, page, limit)) + return {"comments": [{"id": 41}], "page": 3, "older_page": 2, "total": 47} + + monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned) + monkeypatch.setattr(main.gitea_proxy, "issue_conversation_page", conversation) + 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/comments?page=3&limit=20" + ) + + assert response.status_code == 200 + assert response.headers["cache-control"] == "no-store" + assert response.json() == { + "comments": [{"id": 41}], "page": 3, "older_page": 2, "total": 47 + } + assert calls == [ + ("assigned", "stackchain/api", 7), + ("conversation", "stackchain/api", 7, 3, 20), + ] @pytest.mark.anyio