From 8e357a097b4fb69660a8fda5e65597bce1f22ed0 Mon Sep 17 00:00:00 2001 From: timmy Date: Tue, 11 Aug 2026 15:18:30 +0000 Subject: [PATCH] feat: edit and delete owned comments (Closes #575) --- frontend/comment-actions.js | 103 ++++++++++++++++++++++ frontend/conversation.js | 15 ++++ frontend/dashboard.css | 4 + frontend/dashboard.js | 46 +++++++++- frontend/index.html | 1 + frontend/my-work.js | 3 + frontend/service-worker.js | 1 + src/frontend_bundle.py | 1 + src/gitea_proxy.py | 68 +++++++++++++++ src/main.py | 125 +++++++++++++++++++++++++++ tests/test_frontend_bundle.py | 4 +- tests/test_issue_api.py | 157 ++++++++++++++++++++++++++++++++++ tests/test_my_work.py | 107 +++++++++++++++++++++++ tests/test_service_worker.py | 1 + 14 files changed, 634 insertions(+), 2 deletions(-) create mode 100644 frontend/comment-actions.js diff --git a/frontend/comment-actions.js b/frontend/comment-actions.js new file mode 100644 index 0000000..15e42b9 --- /dev/null +++ b/frontend/comment-actions.js @@ -0,0 +1,103 @@ +function createCommentActions({ fetchJson, getLogin, confirmDelete = () => false }) { + const encodedRepository = repository => String(repository || '').split('/') + .map(encodeURIComponent).join('/'); + + function pathFor(context, commentId) { + const item = context?.item || {}; + if (context?.kind === 'update') { + return 'api/v1/notifications/' + encodeURIComponent(item.notification_id) + + '/comments/' + encodeURIComponent(commentId); + } + if (!['issue', 'pull'].includes(context?.kind)) throw new Error('Comment conversation is unavailable.'); + return 'api/v1/repos/' + encodedRepository(item.repository) + '/' + + (context.kind === 'pull' ? 'pulls/' : 'issues/') + encodeURIComponent(item.number) + + '/comments/' + encodeURIComponent(commentId); + } + + const controller = { + isOwned(comment) { + const login = String(getLogin() || '').trim(); + return Boolean(login && comment && comment.author === login); + }, + async edit(context, pager, commentId, body) { + const draft = String(body || '').trim(); + if (!draft) throw new Error('Comment must not be blank.'); + const comment = await fetchJson(pathFor(context, commentId), { + method: 'PATCH', + headers: { Accept: 'application/json', 'Content-Type': 'application/json' }, + body: JSON.stringify({ body: draft }), + }); + pager.replace(comment); + return pager.snapshot(); + }, + async remove(context, pager, commentId) { + if (!confirmDelete('Delete this comment permanently?')) return null; + const result = await fetchJson(pathFor(context, commentId), { + method: 'DELETE', headers: { Accept: 'application/json' }, + }); + if (!result?.deleted || Number(result.id) !== Number(commentId)) { + throw new Error('Comment deletion was not confirmed.'); + } + pager.remove(commentId); + return pager.snapshot(); + }, + actionHtml(comment) { + return controller.isOwned(comment) ? + '
' + + '' + + '
' : ''; + }, + wire({ root, getSurface, isOffline, escapeHtml }) { + root.addEventListener('click', async event => { + const button = event.target.closest('[data-comment-action]'); + if (!button) return; + const card = button.closest('.issue-comment'); + const commentId = Number(card?.dataset.commentId); + const surface = getSurface(); + const comment = surface.pager?.snapshot().comments.find(item => item.id === commentId); + if (!comment || !controller.isOwned(comment)) return; + if (isOffline()) { + surface.status.textContent = 'Reconnect to edit or delete this comment.'; + return; + } + if (button.dataset.commentAction === 'delete') { + try { + const state = await controller.remove(surface.context, surface.pager, commentId); + if (state) { + surface.render(state); + surface.status.textContent = 'Comment deleted.'; + } + } catch (error) { + surface.status.textContent = error.message || 'Comment deletion failed. Retry or open it in Gitea.'; + } + return; + } + card.innerHTML = '
Editing your comment
' + + '' + + '
' + + '
'; + const textarea = card.querySelector('.comment-edit-textarea'); + textarea.focus(); + card.querySelector('[data-comment-edit-cancel]').addEventListener('click', () => surface.render(surface.pager.snapshot())); + card.querySelector('[data-comment-edit-save]').addEventListener('click', async saveEvent => { + const save = saveEvent.currentTarget; + save.disabled = true; + surface.status.textContent = 'Saving comment…'; + try { + const state = await controller.edit(surface.context, surface.pager, commentId, textarea.value); + surface.render(state); + surface.status.textContent = 'Comment updated.'; + } catch (error) { + save.disabled = false; + surface.status.textContent = error.message || 'Comment update failed. Your edit is safe; retry or open it in Gitea.'; + textarea.focus(); + } + }); + }); + }, + }; + return controller; +} + +if (typeof module !== 'undefined' && module.exports) module.exports = createCommentActions; \ No newline at end of file diff --git a/frontend/conversation.js b/frontend/conversation.js index f64af9f..56ad7e5 100644 --- a/frontend/conversation.js +++ b/frontend/conversation.js @@ -63,6 +63,21 @@ function createConversationPager({ loadPage }) { } return snapshot(); }, + replace(comment) { + if (!comment || !Number.isInteger(comment.id)) return snapshot(); + state = { + ...state, + comments: state.comments.map(existing => existing.id === comment.id ? { ...comment } : existing), + }; + return snapshot(); + }, + remove(commentId) { + const comments = state.comments.filter(comment => comment.id !== commentId); + if (comments.length !== state.comments.length) { + state = { ...state, comments, total: Math.max(0, state.total - 1) }; + } + return snapshot(); + }, }; } diff --git a/frontend/dashboard.css b/frontend/dashboard.css index e11f8ae..4fd36f3 100644 --- a/frontend/dashboard.css +++ b/frontend/dashboard.css @@ -50,6 +50,10 @@ button { background: linear-gradient(180deg,#1f3a5f,#15324d); border:1px solid # .security-event strong, .security-event span { display:block; overflow-wrap:anywhere; } #load-more-security-activity { width:100%; min-height:44px; } button:hover { filter: brightness(1.15); } +.issue-comment { min-width:0; overflow-wrap:anywhere; } +.comment-owned-actions { display:flex; gap:8px; flex-wrap:wrap; margin:8px 0; } +.comment-owned-actions button { min-height:44px; min-width:72px; } +.comment-edit-textarea { box-sizing:border-box; display:block; width:100%; max-width:100%; min-height:132px; resize:vertical; overflow-wrap:anywhere; } .panel { border: 1px solid #1b2d45; border-radius: 14px; padding: 12px; background: rgba(11,21,38,.92); } .panel > summary { cursor: pointer; list-style-position: inside; } .panel > summary h2 { display: inline-block; margin-left: 4px; } diff --git a/frontend/dashboard.js b/frontend/dashboard.js index caa6520..8f3ade9 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -236,6 +236,8 @@ return payload; } + let commentActions = { isOwned: () => false, wire: () => {} }; + function loadMentionCandidates(repository, query) { return fetchReviewJson( 'api/v1/repos/' + repository + '/mention-candidates?q=' + encodeURIComponent(query), @@ -413,6 +415,21 @@ text: shareParams.get('text') || '', url: shareParams.get('url') || '', }; + const commentActionFeatures = createFeatureLoader({ + document, + urls: { + 'comment-actions': document.querySelector('meta[name="stackchain-feature-comment-actions"]')?.content || '', + }, + }); + await commentActionFeatures.run('comment-actions', { + status: qs('#my-work-action-status'), retryLabel:'Reload to retry comment actions.', + }, () => { + commentActions = createCommentActions({ + fetchJson: fetchReviewJson, + getLogin: () => confirmedOwnerLogin, + confirmDelete: message => window.confirm(message), + }); + }); const issueCaptureFeatures = createFeatureLoader({ document, urls: { @@ -2346,10 +2363,11 @@ } function renderIssueComment(comment) { + const actions = commentActions.actionHtml?.(comment) || ''; return '
' + escapeHtml(comment.author || 'Unknown author') + (comment.created_at ? ' · ' + escapeHtml(fmt(comment.created_at)) : '') + - '
' + + '
' + actions + '
' + renderMarkdown(comment.body || 'No comment body provided.') + '
'; } @@ -2381,6 +2399,32 @@ comments.length + ' of ' + Math.max(state.total || 0, comments.length) + ' messages loaded.' : 'No comments yet.'; } + function commentSurface(selector) { + if (selector === '#issue-comments') return { + context:{kind:'issue',item:selectedIssue}, pager:issueConversation, + render:renderIssueConversation, status:qs('#issue-sheet-status'), + }; + if (selector === '#pull-comments') return { + context:{kind:'pull',item:selectedPull}, pager:pullConversation, + render:renderPullConversation, status:qs('#pull-sheet-status'), + }; + return { + context:{kind:'update',item:selectedUpdate}, pager:notificationReader.commentPager(), + render:renderUpdateConversation, status:qs('#update-sheet-status'), + }; + } + + function wireCommentActions(selector) { + commentActions.wire({ + root:qs(selector), getSurface:()=>commentSurface(selector), + isOffline:()=>offlineWorkMode || navigator.onLine === false, escapeHtml, + }); + } + + wireCommentActions('#issue-comments'); + wireCommentActions('#pull-comments'); + wireCommentActions('#update-comments'); + function renderIssueLabelEditor(item, confirmedNames, labels) { const list = qs('#issue-label-list'); const status = qs('#issue-label-status'); diff --git a/frontend/index.html b/frontend/index.html index b1ab69e..fd8af5d 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -811,6 +811,7 @@ + diff --git a/frontend/my-work.js b/frontend/my-work.js index 85d5dc0..8bf8832 100644 --- a/frontend/my-work.js +++ b/frontend/my-work.js @@ -395,6 +395,9 @@ function createNotificationReader({ return { open, + commentPager() { + return conversationPager; + }, appendReply(comment) { if (!conversationPager) return false; onConversation(conversationPager.append(comment)); diff --git a/frontend/service-worker.js b/frontend/service-worker.js index 6c0b935..7bd732e 100644 --- a/frontend/service-worker.js +++ b/frontend/service-worker.js @@ -46,6 +46,7 @@ const SHELL = [ BASE + 'static/later-picker.js', BASE + 'static/pick-work.js', BASE + 'static/conversation.js', + BASE + 'static/comment-actions.js', BASE + 'static/issue-attachment.js', BASE + 'static/issue-sheet.js', BASE + 'static/create-issue-sheet.js', diff --git a/src/frontend_bundle.py b/src/frontend_bundle.py index adebbc6..14c0e5f 100644 --- a/src/frontend_bundle.py +++ b/src/frontend_bundle.py @@ -17,6 +17,7 @@ COMMONJS_EXPORT_LINE = re.compile( ) WORKER_RUNTIME_SOURCE = "static/background-issue-sync.js" FEATURE_SOURCES = { + "comment-actions": ("static/conversation.js", "static/comment-actions.js"), "issue-capture": ("static/create-issue-sheet.js",), "pull-workflow": ("static/pull-sheet.js", "static/review-sheet.js"), "push-notifications": ("static/push-notifications.js",), diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 6594a47..179bddb 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -20,6 +20,10 @@ class GiteaOverloadedError(RuntimeError): """Raised when bounded transport admission expires before capacity is available.""" +class CommentMutationForbiddenError(RuntimeError): + """Raised when a comment is not owned by the operator or enclosing thread.""" + + class GiteaTransport: """Application-lifetime HTTP transport with single-flight concurrent reads.""" @@ -963,6 +967,25 @@ async def notification_conversation_page( ) +async def notification_conversation_target(thread_id: int) -> tuple[str, int]: + thread = await fetch(f"notifications/threads/{thread_id}") + if not isinstance(thread, dict): + raise ValueError("Gitea notification thread response was not an object") + repository = thread.get("repository") + subject_value = thread.get("subject") + subject = subject_value if isinstance(subject_value, dict) else {} + repository_name = repository.get("full_name") if isinstance(repository, dict) else None + subject_path = _gitea_api_path(subject.get("url")) + match = re.fullmatch(r"repos/([^/]+/[^/]+)/(issues|pulls)/(\d+)", subject_path) + if ( + not match + or match.group(1) != repository_name + or subject.get("type") not in {"Issue", "Pull"} + ): + raise ValueError("Notification subject is not a supported conversation") + return match.group(1), int(match.group(3)) + + async def reply_to_notification(thread_id: int, body: str) -> dict: thread = await fetch(f"notifications/threads/{thread_id}") if not isinstance(thread, dict): @@ -1077,6 +1100,51 @@ async def comment_on_issue(repository: str, number: int, body: str) -> dict: return _normalize_issue_comment(comment) +async def _owned_comment(repository: str, number: int, comment_id: int) -> dict: + user, comment = await asyncio.gather( + current_user(), + fetch(f"repos/{repository}/issues/comments/{comment_id}"), + ) + login = user.get("login") if isinstance(user, dict) else None + author = comment.get("user") if isinstance(comment, dict) else None + expected_issue_path = f"repos/{repository}/issues/{number}" + if ( + not isinstance(login, str) + or not login + or not isinstance(comment, dict) + or not isinstance(author, dict) + or author.get("login") != login + or _gitea_api_path(comment.get("issue_url")) != expected_issue_path + ): + raise CommentMutationForbiddenError("Comment is not editable in this conversation") + return comment + + +async def edit_owned_comment( + repository: str, number: int, comment_id: int, body: str +) -> dict: + await _owned_comment(repository, number, comment_id) + response = await _get_client().patch( + f"/api/v1/repos/{repository}/issues/comments/{comment_id}", + headers=_auth(), + json={"body": body}, + ) + response.raise_for_status() + comment = response.json() + if not isinstance(comment, dict): + raise ValueError("Gitea comment response was not an object") + return _normalize_issue_comment(comment) + + +async def delete_owned_comment(repository: str, number: int, comment_id: int) -> dict: + await _owned_comment(repository, number, comment_id) + response = await _get_client().delete( + f"/api/v1/repos/{repository}/issues/comments/{comment_id}", headers=_auth() + ) + response.raise_for_status() + return {"id": comment_id, "deleted": True} + + async def upload_assigned_issue_attachment( repository: str, number: int, diff --git a/src/main.py b/src/main.py index c0f5ba9..c550a30 100644 --- a/src/main.py +++ b/src/main.py @@ -3918,6 +3918,131 @@ async def comment_on_assigned_issue( return JSONResponse(result, status_code=201) +async def _edit_conversation_comment( + repository: str, number: int, comment_id: int, body: str +) -> JSONResponse: + try: + result = await asyncio.wait_for( + gitea_proxy.edit_owned_comment(repository, number, comment_id, body), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + except gitea_proxy.CommentMutationForbiddenError as exc: + raise HTTPException( + status_code=403, detail="You can only change your own comments" + ) from exc + except HTTPException: + raise + except Exception: + return JSONResponse( + {"error": "The comment could not be updated. Your edit is safe; please retry."}, + status_code=503, + headers={"Retry-After": "1"}, + ) + return JSONResponse(result) + + +async def _delete_conversation_comment( + repository: str, number: int, comment_id: int +) -> JSONResponse: + try: + result = await asyncio.wait_for( + gitea_proxy.delete_owned_comment(repository, number, comment_id), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + except gitea_proxy.CommentMutationForbiddenError as exc: + raise HTTPException( + status_code=403, detail="You can only change your own comments" + ) from exc + except HTTPException: + raise + except Exception: + return JSONResponse( + {"error": "The comment deletion could not be confirmed. Please retry."}, + status_code=503, + headers={"Retry-After": "1"}, + ) + return JSONResponse(result) + + +@app.patch("/api/v1/repos/{owner}/{repo}/issues/{number}/comments/{comment_id}") +async def edit_assigned_issue_comment( + comment: IssueComment, + owner: str, + repo: str, + number: int = PathParam(gt=0), + comment_id: int = PathParam(gt=0), +): + repository = f"{owner}/{repo}" + if not await gitea_proxy.is_assigned_issue(repository, number): + raise HTTPException(status_code=404, detail="Assigned issue not found") + return await _edit_conversation_comment(repository, number, comment_id, comment.body) + + +@app.delete("/api/v1/repos/{owner}/{repo}/issues/{number}/comments/{comment_id}") +async def delete_assigned_issue_comment( + owner: str, + repo: str, + number: int = PathParam(gt=0), + comment_id: int = PathParam(gt=0), +): + repository = f"{owner}/{repo}" + if not await gitea_proxy.is_assigned_issue(repository, number): + raise HTTPException(status_code=404, detail="Assigned issue not found") + return await _delete_conversation_comment(repository, number, comment_id) + + +@app.patch("/api/v1/repos/{owner}/{repo}/pulls/{number}/comments/{comment_id}") +async def edit_assigned_pull_comment( + comment: IssueComment, + owner: str, + repo: str, + number: int = PathParam(gt=0), + comment_id: int = PathParam(gt=0), +): + repository = f"{owner}/{repo}" + if not await gitea_proxy.is_assigned_pull(repository, number): + raise HTTPException(status_code=404, detail="Assigned pull request not found") + return await _edit_conversation_comment(repository, number, comment_id, comment.body) + + +@app.delete("/api/v1/repos/{owner}/{repo}/pulls/{number}/comments/{comment_id}") +async def delete_assigned_pull_comment( + owner: str, + repo: str, + number: int = PathParam(gt=0), + comment_id: int = PathParam(gt=0), +): + repository = f"{owner}/{repo}" + if not await gitea_proxy.is_assigned_pull(repository, number): + raise HTTPException(status_code=404, detail="Assigned pull request not found") + return await _delete_conversation_comment(repository, number, comment_id) + + +@app.patch("/api/v1/notifications/{thread_id}/comments/{comment_id}") +async def edit_notification_comment( + comment: IssueComment, + thread_id: int = PathParam(gt=0), + comment_id: int = PathParam(gt=0), +): + try: + repository, number = await gitea_proxy.notification_conversation_target(thread_id) + except Exception as exc: + raise HTTPException(status_code=404, detail="Notification conversation not found") from exc + return await _edit_conversation_comment(repository, number, comment_id, comment.body) + + +@app.delete("/api/v1/notifications/{thread_id}/comments/{comment_id}") +async def delete_notification_comment( + thread_id: int = PathParam(gt=0), + comment_id: int = PathParam(gt=0), +): + try: + repository, number = await gitea_proxy.notification_conversation_target(thread_id) + except Exception as exc: + raise HTTPException(status_code=404, detail="Notification conversation not found") from exc + return await _delete_conversation_comment(repository, number, comment_id) + + @app.post("/api/v1/repos/{owner}/{repo}/issues/{number}/attachments", status_code=201) async def attach_to_assigned_issue( request: Request, diff --git a/tests/test_frontend_bundle.py b/tests/test_frontend_bundle.py index e20628c..fba4944 100644 --- a/tests/test_frontend_bundle.py +++ b/tests/test_frontend_bundle.py @@ -45,7 +45,9 @@ def test_page_runtime_is_one_deterministic_content_addressed_bundle(tmp_path): def test_product_workflows_are_stable_lazy_feature_chunks(tmp_path): first = build_frontend(FRONTEND) - assert set(first.feature_bundles) == {"issue-capture", "pull-workflow", "push-notifications", "device-setup"} + assert set(first.feature_bundles) == { + "comment-actions", "issue-capture", "pull-workflow", "push-notifications", "device-setup" + } capture = first.feature_bundles["issue-capture"] pull_workflow = first.feature_bundles["pull-workflow"] assert b"function createIssueCapture" not in first.runtime_bytes diff --git a/tests/test_issue_api.py b/tests/test_issue_api.py index 6a848ec..5472a5d 100644 --- a/tests/test_issue_api.py +++ b/tests/test_issue_api.py @@ -8,6 +8,163 @@ from src import gitea_proxy, main from src.idempotency import IdempotencyLedger +@pytest.mark.anyio +async def test_owned_comment_mutations_verify_thread_and_author_before_writing(): + requests = [] + + async def handler(request): + requests.append(request) + if request.url.path == "/api/v1/user": + return httpx.Response(200, json={"login": "timmy"}) + if request.method == "GET" and request.url.path.endswith("/issues/comments/42"): + return httpx.Response(200, json={ + "id": 42, + "body": "Original", + "user": {"login": "timmy"}, + "issue_url": f"{gitea_proxy.GITEA_URL}/api/v1/repos/stackchain/api/issues/17", + "html_url": f"{gitea_proxy.GITEA_URL}/stackchain/api/issues/17#issuecomment-42", + }) + if request.method == "PATCH": + return httpx.Response(200, json={ + "id": 42, "body": "Corrected", "user": {"login": "timmy"}, + "html_url": "https://gitea.example/stackchain/api/issues/17#issuecomment-42", + }) + return httpx.Response(204) + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + edited = await gitea_proxy.edit_owned_comment("stackchain/api", 17, 42, "Corrected") + deleted = await gitea_proxy.delete_owned_comment("stackchain/api", 17, 42) + finally: + await gitea_proxy.stop_client() + + assert edited["body"] == "Corrected" + assert deleted == {"id": 42, "deleted": True} + assert [(request.method, request.url.path) for request in requests] == [ + ("GET", "/api/v1/user"), + ("GET", "/api/v1/repos/stackchain/api/issues/comments/42"), + ("PATCH", "/api/v1/repos/stackchain/api/issues/comments/42"), + ("GET", "/api/v1/user"), + ("GET", "/api/v1/repos/stackchain/api/issues/comments/42"), + ("DELETE", "/api/v1/repos/stackchain/api/issues/comments/42"), + ] + assert json.loads(requests[2].content) == {"body": "Corrected"} + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "comment", + [ + {"id": 42, "user": {"login": "alex"}, "issue_url": "https://gitea.example/api/v1/repos/stackchain/api/issues/17"}, + {"id": 42, "user": {"login": "timmy"}, "issue_url": f"{gitea_proxy.GITEA_URL}/api/v1/repos/stackchain/api/issues/99"}, + {"id": 42, "user": {"login": "timmy"}, "issue_url": "https://evil.example/api/v1/repos/stackchain/api/issues/17"}, + ], +) +async def test_owned_comment_mutations_reject_other_author_or_thread_without_writing(comment): + requests = [] + + async def handler(request): + requests.append(request) + if request.url.path == "/api/v1/user": + return httpx.Response(200, json={"login": "timmy"}) + return httpx.Response(200, json=comment) + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + with pytest.raises(gitea_proxy.CommentMutationForbiddenError): + await gitea_proxy.edit_owned_comment("stackchain/api", 17, 42, "Nope") + finally: + await gitea_proxy.stop_client() + + assert [request.method for request in requests] == ["GET", "GET"] + + +@pytest.mark.anyio +@pytest.mark.parametrize( + ("base_path", "availability_call"), + [ + ("/api/v1/repos/stackchain/api/issues/17", "issue"), + ("/api/v1/repos/stackchain/api/pulls/17", "pull"), + ("/api/v1/notifications/91", "notification"), + ], +) +async def test_comment_mutation_routes_cover_issue_pull_and_unread_update( + monkeypatch, base_path, availability_call +): + calls = [] + + async def available_issue(repository, number): + calls.append(("issue", repository, number)) + return True + + async def available_pull(repository, number): + calls.append(("pull", repository, number)) + return True + + async def notification_target(thread_id): + calls.append(("notification", thread_id)) + return "stackchain/api", 17 + + async def edit(repository, number, comment_id, body): + calls.append(("edit", repository, number, comment_id, body)) + return {"id": comment_id, "author": "timmy", "body": body} + + async def delete(repository, number, comment_id): + calls.append(("delete", repository, number, comment_id)) + return {"id": comment_id, "deleted": True} + + monkeypatch.setattr(main.gitea_proxy, "is_assigned_issue", available_issue) + monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", available_pull) + monkeypatch.setattr( + main.gitea_proxy, "notification_conversation_target", notification_target, + raising=False, + ) + monkeypatch.setattr(main.gitea_proxy, "edit_owned_comment", edit) + monkeypatch.setattr(main.gitea_proxy, "delete_owned_comment", delete) + + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + edited = await client.patch(base_path + "/comments/42", json={"body": " Corrected "}) + deleted = await client.delete(base_path + "/comments/42") + + assert edited.status_code == 200 + assert edited.json()["body"] == "Corrected" + assert deleted.status_code == 200 + assert deleted.json() == {"id": 42, "deleted": True} + if availability_call == "notification": + assert calls == [ + ("notification", 91), ("edit", "stackchain/api", 17, 42, "Corrected"), + ("notification", 91), ("delete", "stackchain/api", 17, 42), + ] + else: + assert calls == [ + (availability_call, "stackchain/api", 17), + ("edit", "stackchain/api", 17, 42, "Corrected"), + (availability_call, "stackchain/api", 17), + ("delete", "stackchain/api", 17, 42), + ] + + +@pytest.mark.anyio +async def test_comment_mutation_route_returns_forbidden_without_hiding_saved_edit(monkeypatch): + async def available(repository, number): + return True + + async def forbidden(*args): + raise gitea_proxy.CommentMutationForbiddenError("not owned") + + monkeypatch.setattr(main.gitea_proxy, "is_assigned_issue", available) + monkeypatch.setattr(main.gitea_proxy, "edit_owned_comment", forbidden) + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.patch( + "/api/v1/repos/stackchain/api/issues/17/comments/42", + json={"body": "Keep this draft"}, + ) + assert response.status_code == 403 + assert response.json()["detail"] == "You can only change your own comments" + + async def repository_access_from(loader, repository): repositories = await loader() return next( diff --git a/tests/test_my_work.py b/tests/test_my_work.py index 7fbf262..0a5056e 100644 --- a/tests/test_my_work.py +++ b/tests/test_my_work.py @@ -18,6 +18,7 @@ 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" +COMMENT_ACTIONS = Path(__file__).parents[1] / "frontend" / "comment-actions.js" PICK_WORK = Path(__file__).parents[1] / "frontend" / "pick-work.js" WORK_ROUTE = Path(__file__).parents[1] / "frontend" / "work-route.js" UPDATE_OWNERSHIP = Path(__file__).parents[1] / "frontend" / "update-ownership.js" @@ -3862,6 +3863,112 @@ Promise.all([first, duplicate]).then(() => {{ assert output["state"]["total"] == 4 +def test_owned_comment_actions_edit_delete_all_conversation_routes_and_preserve_failed_edit(): + script = f""" +const createConversationPager = require({json.dumps(str(CONVERSATION))}); +const createCommentActions = require({json.dumps(str(COMMENT_ACTIONS))}); +const requests = []; +let shouldFail = false; +const controller = createCommentActions({{ + getLogin: () => 'timmy', + confirmDelete: () => true, + fetchJson: async (url, options) => {{ + requests.push([url, options.method, options.body || null]); + if (shouldFail) throw new Error('offline'); + if (options.method === 'DELETE') return {{id:42,deleted:true}}; + return {{id:42,author:'timmy',body:JSON.parse(options.body).body,created_at:'2026-08-11T10:00:00Z'}}; + }}, +}}); +const cancelController = createCommentActions({{ + getLogin: () => 'timmy', confirmDelete: () => false, + fetchJson: async () => {{ throw new Error('delete should not run'); }}, +}}); +const contexts = [ + {{kind:'issue',item:{{repository:'stackchain/api',number:17}}}}, + {{kind:'pull',item:{{repository:'stackchain/api',number:17}}}}, + {{kind:'update',item:{{notification_id:91}}}}, +]; +(async () => {{ + const results = []; + for (const context of contexts) {{ + const pager = createConversationPager({{loadPage:async()=>({{}})}}); + pager.reset({{comments:[{{id:42,author:'timmy',body:'Original'}},{{id:43,author:'alex',body:'Other'}}],total:2}}); + results.push(controller.isOwned(pager.snapshot().comments[0])); + results.push(controller.isOwned(pager.snapshot().comments[1])); + await controller.edit(context, pager, 42, 'Corrected'); + results.push(pager.snapshot().comments[0].body); + await controller.remove(context, pager, 42); + results.push([pager.snapshot().comments.map(item=>item.id), pager.snapshot().total]); + }} + const pager = createConversationPager({{loadPage:async()=>({{}})}}); + pager.reset({{comments:[{{id:42,author:'timmy',body:'Original'}}],total:1}}); + shouldFail = true; + let failure = ''; + try {{ await controller.edit(contexts[0], pager, 42, 'Unsaved correction'); }} catch (error) {{ failure=error.message; }} + const cancelled = await cancelController.remove(contexts[0], pager, 42); + process.stdout.write(JSON.stringify({{requests,results,failure,cancelled,state:pager.snapshot()}})); +}})(); +""" + result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + output = json.loads(result.stdout) + assert [request[:2] for request in output["requests"][:6]] == [ + ["api/v1/repos/stackchain/api/issues/17/comments/42", "PATCH"], + ["api/v1/repos/stackchain/api/issues/17/comments/42", "DELETE"], + ["api/v1/repos/stackchain/api/pulls/17/comments/42", "PATCH"], + ["api/v1/repos/stackchain/api/pulls/17/comments/42", "DELETE"], + ["api/v1/notifications/91/comments/42", "PATCH"], + ["api/v1/notifications/91/comments/42", "DELETE"], + ] + assert output["results"] == [True, False, "Corrected", [[43], 1]] * 3 + assert output["failure"] == "offline" + assert output["cancelled"] is None + assert output["state"]["comments"][0]["body"] == "Original" + + +@pytest.mark.anyio +async def test_owned_comment_actions_render_inline_for_mobile_in_all_conversations(): + html = await dashboard() + dashboard_javascript = (Path(__file__).parents[1] / "frontend" / "dashboard.js").read_text() + action_javascript = COMMENT_ACTIONS.read_text() + javascript = dashboard_javascript + action_javascript + css = (Path(__file__).parents[1] / "frontend" / "dashboard.css").read_text() + service_worker = (Path(__file__).parents[1] / "frontend" / "service-worker.js").read_text() + + assert '' in html + assert html.index('static/comment-actions.js') < html.index('static/dashboard.js') + assert "BASE + 'static/comment-actions.js'" in service_worker + assert 'data-comment-action="edit"' in javascript + assert 'data-comment-action="delete"' in javascript + assert 'class="comment-edit-textarea"' in javascript + assert "wireCommentActions('#issue-comments')" in javascript + assert "wireCommentActions('#pull-comments')" in javascript + assert "wireCommentActions('#update-comments')" in javascript + assert ".comment-owned-actions button" in css and "min-height:44px" in css + assert ".comment-edit-textarea" in css and "max-width:100%" in css + assert ".issue-comment" in css and "overflow-wrap:anywhere" in css + + +def test_notification_reader_exposes_current_comment_pager_only_while_open(): + script = f""" +const {{createNotificationReader}} = require({json.dumps(str(MY_WORK))}); +const createConversationPager = require({json.dumps(str(CONVERSATION))}); +const reader = createNotificationReader({{ + load: async () => ({{conversation:{{comments:[{{id:42,body:'Before'}}],total:1}}}}), + markRead: async()=>{{}}, onOpen:()=>{{}}, onDetail:()=>{{}}, onItems:()=>{{}}, onStatus:()=>{{}}, onClose:()=>{{}}, + loadConversation: async()=>({{}}), createPager:createConversationPager, +}}); +(async()=>{{ + const before = reader.commentPager(); + await reader.open({{notification_id:91}}); + const pager = reader.commentPager(); + pager.replace({{id:42,body:'After'}}); + process.stdout.write(JSON.stringify({{before:before===null,body:pager.snapshot().comments[0].body}})); +}})(); +""" + result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + assert json.loads(result.stdout) == {"before": True, "body": "After"} + + def test_conversation_pager_keeps_loaded_messages_when_older_page_fails(): script = f""" const createConversationPager = require({json.dumps(str(CONVERSATION))}); diff --git a/tests/test_service_worker.py b/tests/test_service_worker.py index 57dd1be..d8904ee 100644 --- a/tests/test_service_worker.py +++ b/tests/test_service_worker.py @@ -693,6 +693,7 @@ def test_install_precaches_complete_subpath_scoped_app_shell(): "/dashboard/static/later-picker.js", "/dashboard/static/pick-work.js", "/dashboard/static/conversation.js", + "/dashboard/static/comment-actions.js", "/dashboard/static/issue-attachment.js", "/dashboard/static/issue-sheet.js", "/dashboard/static/create-issue-sheet.js", -- 2.43.0