From 97e88cd8b334cbc76f630ca5972d401e433d2530 Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Thu, 10 Sep 2026 08:00:59 -0400 Subject: [PATCH] Fix formatting --- backend/src/github_pm/api.py | 19 +- backend/tests/test_api.py | 40 ++- frontend/src/components/IssueCard.jsx | 323 ++++++++++++++++-- frontend/src/components/IssueCard.test.jsx | 87 ++++- .../src/components/MarkdownInputModal.jsx | 35 +- .../components/MarkdownInputModal.test.jsx | 19 +- .../src/components/MilestoneCard.test.jsx | 32 ++ frontend/src/services/api.js | 8 +- frontend/src/services/api.test.js | 8 +- 9 files changed, 474 insertions(+), 97 deletions(-) diff --git a/backend/src/github_pm/api.py b/backend/src/github_pm/api.py index 17d1068..daf8132 100644 --- a/backend/src/github_pm/api.py +++ b/backend/src/github_pm/api.py @@ -3,7 +3,7 @@ from datetime import datetime import re import time -from typing import Annotated, Any, AsyncGenerator, NoReturn +from typing import Annotated, Any, AsyncGenerator, Literal, NoReturn from urllib.parse import quote_plus from fastapi import APIRouter, Body, Depends, HTTPException, Path, Query @@ -475,33 +475,38 @@ async def update_issue_body( return updated -class CloseWithComment(BaseModel): - """Body for closing an issue with an optional comment. +class CloseIssueRequest(BaseModel): + """Body for closing an issue with a reason and optional comment. Generated-by: Cursor """ - body: str = Field(title="Comment Body", min_length=1) + reason: Literal["done", "obsolete"] = Field(title="Close Reason", default="done") + body: str | None = Field(title="Comment Body", default=None) @api_router.post("/issues/{issue_number}/close-with-comment") async def close_issue_with_comment( gitctx: Annotated[Connector, Depends(connection)], issue_number: Annotated[int, Path(title="Issue")], - comment: Annotated[CloseWithComment, Body(title="Comment")], + comment: Annotated[CloseIssueRequest, Body(title="Comment")], ): """Add a comment to an issue and mark it closed. Generated-by: Cursor """ + resolved_body = (comment.body or "").strip() or ( + "Obsolete" if comment.reason == "obsolete" else "Done" + ) + state_reason = "not_planned" if comment.reason == "obsolete" else "completed" created_comment = gitctx.post( f"/repos/{context.github_repo}/issues/{issue_number}/comments", - data={"body": comment.body}, + data={"body": resolved_body}, headers=_GITHUB_BODY_ACCEPT, ) closed_issue = gitctx.patch( f"/repos/{context.github_repo}/issues/{issue_number}", - data={"state": "closed"}, + data={"state": "closed", "state_reason": state_reason}, headers=_GITHUB_BODY_ACCEPT, ) logger.info("Closed issue #%s with comment", issue_number) diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 84cb7cf..adc8331 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -20,7 +20,7 @@ api_router, clear_issue_parent, close_issue_with_comment, - CloseWithComment, + CloseIssueRequest, connection, Connector, create_comment, @@ -1995,14 +1995,46 @@ async def test_close_with_comment_success(self): with patch("github_pm.api.context") as mock_context: mock_context.github_repo = "test/repo" result = await close_issue_with_comment( - mock_gitctx, 42, CloseWithComment(body="Done") + mock_gitctx, 42, CloseIssueRequest(reason="done", body="Done") ) assert result == {"comment": mock_comment, "issue": mock_issue} - mock_gitctx.post.assert_called_once() + mock_gitctx.post.assert_called_once_with( + "/repos/test/repo/issues/42/comments", + data={"body": "Done"}, + headers={"Accept": "application/vnd.github.full+json"}, + ) + mock_gitctx.patch.assert_called_once_with( + "/repos/test/repo/issues/42", + data={"state": "closed", "state_reason": "completed"}, + headers={"Accept": "application/vnd.github.full+json"}, + ) + + @pytest.mark.asyncio + async def test_close_with_obsolete_default_comment(self): + mock_comment = {"id": 1, "body": "Obsolete"} + mock_issue = {"number": 42, "state": "closed"} + mock_gitctx = Mock(spec=Connector) + mock_gitctx.post = Mock(return_value=mock_comment) + mock_gitctx.patch = Mock(return_value=mock_issue) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await close_issue_with_comment( + mock_gitctx, + 42, + CloseIssueRequest(reason="obsolete", body=" "), + ) + + assert result == {"comment": mock_comment, "issue": mock_issue} + mock_gitctx.post.assert_called_once_with( + "/repos/test/repo/issues/42/comments", + data={"body": "Obsolete"}, + headers={"Accept": "application/vnd.github.full+json"}, + ) mock_gitctx.patch.assert_called_once_with( "/repos/test/repo/issues/42", - data={"state": "closed"}, + data={"state": "closed", "state_reason": "not_planned"}, headers={"Accept": "application/vnd.github.full+json"}, ) diff --git a/frontend/src/components/IssueCard.jsx b/frontend/src/components/IssueCard.jsx index a16f80e..7eb663c 100644 --- a/frontend/src/components/IssueCard.jsx +++ b/frontend/src/components/IssueCard.jsx @@ -9,10 +9,14 @@ import { Button, Checkbox, Modal, + Tabs, + Tab, + TabTitleText, TextInput, TextArea, Form, FormGroup, + Radio, } from '@patternfly/react-core'; import { CodeBranchIcon, @@ -42,8 +46,9 @@ import { removeIssueAssignees, adoptParentMilestone, createComment, - closeIssueWithComment, + closeIssue, createIssue, + renderMarkdown, updateIssueBody, addBlockedBy, removeBlockedBy, @@ -176,6 +181,18 @@ const IssueCard = ({ const [removingLinkKey, setRemovingLinkKey] = useState(null); const linkMenuRef = useRef(null); const linkToggleRef = useRef(null); + const [isCloseMenuOpen, setIsCloseMenuOpen] = useState(false); + const [closeReason, setCloseReason] = useState('done'); + const [closeComment, setCloseComment] = useState(''); + const [closeError, setCloseError] = useState(null); + const [closeBusy, setCloseBusy] = useState(false); + const [closeActiveTab, setCloseActiveTab] = useState(0); + const [closePreviewHtml, setClosePreviewHtml] = useState(''); + const [closePreviewLoading, setClosePreviewLoading] = useState(false); + const [closePreviewError, setClosePreviewError] = useState(null); + const closeMenuRef = useRef(null); + const closeToggleRef = useRef(null); + const closePreviewRequestId = useRef(0); useEffect(() => { setDescriptionBody(issue.body || ''); @@ -245,6 +262,26 @@ const IssueCard = ({ setCurrentBlocking(issue.blocking || []); }, [issue.blocking]); + useEffect(() => { + if (!isCloseMenuOpen || closeActiveTab !== 1) return; + + const requestId = ++closePreviewRequestId.current; + setClosePreviewLoading(true); + setClosePreviewError(null); + + renderMarkdown(closeComment || '') + .then((data) => { + if (closePreviewRequestId.current !== requestId) return; + setClosePreviewHtml(data.html || ''); + setClosePreviewLoading(false); + }) + .catch((err) => { + if (closePreviewRequestId.current !== requestId) return; + setClosePreviewError(err.message); + setClosePreviewLoading(false); + }); + }, [isCloseMenuOpen, closeActiveTab, closeComment]); + // Fetch reactions if total_count > 0 useEffect(() => { // Reset reactions when issue changes @@ -599,13 +636,23 @@ const IssueCard = ({ setLinkError(null); setLinkIssueNumber(''); } + if ( + isCloseMenuOpen && + closeToggleRef.current && + !closeToggleRef.current.contains(event.target) && + closeMenuRef.current && + !closeMenuRef.current.contains(event.target) + ) { + resetCloseMenu(); + } }; if ( isLabelMenuOpen || isMilestoneMenuOpen || isAssigneesMenuOpen || - isLinkMenuOpen + isLinkMenuOpen || + isCloseMenuOpen ) { document.addEventListener('mousedown', handleClickOutside); return () => { @@ -617,7 +664,9 @@ const IssueCard = ({ isMilestoneMenuOpen, isAssigneesMenuOpen, isLinkMenuOpen, + isCloseMenuOpen, handleApplyAssignees, + resetCloseMenu, ]); const notifyLabelsChanged = () => { @@ -857,9 +906,37 @@ const IssueCard = ({ ); }; - const handleCloseWithComment = async (body) => { - await closeIssueWithComment(issue.number, body); - onIssueClosed?.({ issueNumber: issue.number }); + function resetCloseMenu() { + setIsCloseMenuOpen(false); + setCloseReason('done'); + setCloseComment(''); + setCloseError(null); + setCloseBusy(false); + setCloseActiveTab(0); + setClosePreviewHtml(''); + setClosePreviewLoading(false); + setClosePreviewError(null); + } + + const handleCloseIssue = async () => { + setCloseBusy(true); + setCloseError(null); + try { + const result = await closeIssue(issue.number, { + reason: closeReason, + body: closeComment, + }); + resetCloseMenu(); + onIssueUpdate?.({ + ...issue, + ...result?.issue, + }); + onIssueClosed?.({ issueNumber: issue.number }); + } catch (err) { + console.error('Failed to close issue:', err); + setCloseError(err.message); + setCloseBusy(false); + } }; const handleCreateSubIssue = async (payload) => { @@ -1260,6 +1337,8 @@ const IssueCard = ({ const depth = issue.hierarchy_depth || 0; const childCount = issue.child_count ?? (issue.children || []).length; const indentPx = enableHierarchy ? depth * 16 : 0; + const issueState = String(issue.state || 'open').toLowerCase(); + const isClosableIssue = !issue.pull_request && issueState !== 'closed'; const handleAdoptParentMilestone = async () => { try { @@ -2077,27 +2156,223 @@ const IssueCard = ({ overflowWrap: 'break-word', }} > - {issue.type && ( - - +
+ {issue.type && ( + + + {issue.type.name} + + + )} + {issue.title} +
+ {isClosableIssue && ( +
- {issue.type.name} - - - )} - {issue.title} + + {isCloseMenuOpen && ( +
e.stopPropagation()} + > +
+ +
+ setCloseReason('done')} + /> + setCloseReason('obsolete')} + /> +
+
+ + + setCloseActiveTab(Number(key)) + } + aria-label={`Close comment editor for issue #${issue.number}`} + > + Edit} + > +
+