diff --git a/backend/src/github_pm/api.py b/backend/src/github_pm/api.py index c3dcbb6..17d1068 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 +from typing import Annotated, Any, AsyncGenerator, NoReturn from urllib.parse import quote_plus from fastapi import APIRouter, Body, Depends, HTTPException, Path, Query @@ -13,6 +13,7 @@ from github_pm.context import context from github_pm.issue_hierarchy import ( apply_graphql_hierarchy, + apply_graphql_links, build_issue_forest, collect_descendant_numbers, is_ancestor, @@ -153,7 +154,7 @@ def post( f"{self.base_url}{path}", json=data, headers=headers ) ) - return response.json() + return response.json() if response.content else {} def post_text( self, path: str, text: str, headers: dict[str, str] | None = None @@ -188,6 +189,41 @@ def delete( return response.json() if response.content else {} +def _github_http_error_detail(exc: requests.HTTPError) -> str: + """Best-effort message from a GitHub REST error response. + + Generated-by: Cursor + """ + response = exc.response + if response is None: + return str(exc) + try: + payload = response.json() + except ValueError: + return (response.text or str(exc)).strip() or str(exc) + if isinstance(payload, dict): + message = payload.get("message") + errors = payload.get("errors") + if message and errors: + return f"{message}: {errors}" + if message: + return str(message) + return (response.text or str(exc)).strip() or str(exc) + + +def _raise_github_http_error(exc: requests.HTTPError) -> NoReturn: + """Convert ``requests.HTTPError`` into an ``HTTPException`` with GitHub's status. + + Generated-by: Cursor + """ + status = 502 + if exc.response is not None: + status = exc.response.status_code or status + raise HTTPException( + status_code=status, detail=_github_http_error_detail(exc) + ) from exc + + async def connection() -> AsyncGenerator[Connector]: """FastAPI Dependency to open & close Github connections""" connector = None @@ -202,6 +238,11 @@ async def connection() -> AsyncGenerator[Connector]: start = time.time() yield connector logger.debug(f"Elapsed time: {time.time() - start:.3f} seconds") + except HTTPException: + raise + except requests.HTTPError as e: + logger.exception(f"GitHub HTTP error: {str(e)!r}") + _raise_github_http_error(e) except Exception as e: logger.exception(f"GitHub error: {str(e)!r}") raise HTTPException( @@ -282,17 +323,7 @@ async def get_issues( ) data = response["data"] issue_node = data["repository"]["issue"] - closed_refs = issue_node.get("closedByPullRequestsReferences") or {} - closed = closed_refs.get("nodes") or [] - if len(closed) > 0: - i["closed_by"] = [ - { - "number": linked["number"], - "title": linked["title"], - "url": linked["url"], - } - for linked in closed - ] + apply_graphql_links(i, issue_node) apply_graphql_hierarchy(i, issue_node) except Exception as e: logger.exception( @@ -322,25 +353,11 @@ async def get_issue( headers=_GITHUB_BODY_ACCEPT, ) if "pull_request" not in issue: - query = """query($owner: String!, $repo: String!, $issue: Int!) { - repository(owner: $owner, name: $repo, followRenames: true) { - issue(number: $issue) { - closedByPullRequestsReferences(first: 100, includeClosedPrs: true) { - nodes { - number - title - url - } - } - } - } - } - """ try: response = gitctx.post( "/graphql", data={ - "query": query, + "query": ISSUE_HIERARCHY_GRAPHQL, "variables": { "owner": gitctx.owner, "repo": gitctx.repo, @@ -350,19 +367,10 @@ async def get_issue( ) data = response["data"] issue_node = data["repository"]["issue"] - closed = issue_node["closedByPullRequestsReferences"]["nodes"] - if len(closed) > 0: - issue["closed_by"] = [ - { - "number": linked["number"], - "title": linked["title"], - "url": linked["url"], - } - for linked in closed - ] + apply_graphql_links(issue, issue_node) except Exception as e: logger.exception( - f"Error finding linked PRs for issue {issue['number']}: {e!r}" + f"Error finding linked PRs/dependencies for issue {issue['number']}: {e!r}" ) return issue @@ -881,6 +889,184 @@ async def adopt_parent_milestone( } +# """Issue dependencies (blocked by / blocking)""" + + +class AddDependency(BaseModel): + issue_number: int = Field(title="Related Issue Number") + + +def _dependency_link_payload(issue: dict) -> dict: + """Normalize a GitHub REST issue into the Planning dependency shape. + + Generated-by: Cursor + """ + state = issue.get("state") or "" + if isinstance(state, str): + state = state.upper() + return { + "id": issue["id"], + "number": issue["number"], + "title": issue.get("title"), + "url": issue.get("html_url") or issue.get("url"), + "state": state, + } + + +def _get_issue_for_dependency(gitctx: Connector, issue_number: int) -> dict: + """Fetch an issue and reject pull requests / missing targets. + + Generated-by: Cursor + """ + try: + issue = gitctx.get(f"/repos/{context.github_repo}/issues/{issue_number}") + except requests.HTTPError as exc: + status = getattr(exc.response, "status_code", None) + if status == 404: + raise HTTPException( + status_code=404, detail=f"Issue #{issue_number} not found" + ) from exc + raise + if "pull_request" in issue: + raise HTTPException( + status_code=422, + detail=f"#{issue_number} is a pull request; dependencies require issues", + ) + return issue + + +def _dependency_already_exists(exc: requests.HTTPError) -> bool: + """True when GitHub rejects a duplicate blocked-by edge. + + Generated-by: Cursor + """ + if exc.response is None or exc.response.status_code != 422: + return False + detail = _github_http_error_detail(exc).lower() + return "already been taken" in detail or "already exists" in detail + + +def _post_blocked_by_dependency( + gitctx: Connector, blocked_issue_number: int, blocking_issue_id: int +) -> dict: + """POST a blocked-by edge; return GitHub's body (possibly empty). + + Generated-by: Cursor + """ + try: + return gitctx.post( + f"/repos/{context.github_repo}/issues/{blocked_issue_number}/" + "dependencies/blocked_by", + data={"issue_id": blocking_issue_id}, + ) + except requests.HTTPError as exc: + if _dependency_already_exists(exc): + return {} + _raise_github_http_error(exc) + + +@api_router.post("/issues/{issue_number}/dependencies/blocked_by") +async def add_blocked_by( + gitctx: Annotated[Connector, Depends(connection)], + issue_number: Annotated[int, Path(title="Issue")], + body: Annotated[AddDependency, Body(title="Dependency")], +): + """Mark this issue as blocked by another issue. + + Generated-by: Cursor + """ + if body.issue_number == issue_number: + raise HTTPException( + status_code=422, detail="An issue cannot be blocked by itself" + ) + _get_issue_for_dependency(gitctx, issue_number) + blocker = _get_issue_for_dependency(gitctx, body.issue_number) + linked = _post_blocked_by_dependency(gitctx, issue_number, blocker["id"]) + return { + "issue_number": issue_number, + "relationship": "blocked_by", + "linked_issue": _dependency_link_payload(linked or blocker), + } + + +@api_router.delete( + "/issues/{issue_number}/dependencies/blocked_by/{blocking_issue_number}" +) +async def remove_blocked_by( + gitctx: Annotated[Connector, Depends(connection)], + issue_number: Annotated[int, Path(title="Issue")], + blocking_issue_number: Annotated[int, Path(title="Blocking Issue Number")], +): + """Remove a blocked-by dependency from this issue. + + Generated-by: Cursor + """ + blocker = _get_issue_for_dependency(gitctx, blocking_issue_number) + try: + gitctx.delete( + f"/repos/{context.github_repo}/issues/{issue_number}/dependencies/" + f"blocked_by/{blocker['id']}" + ) + except requests.HTTPError as exc: + _raise_github_http_error(exc) + return { + "issue_number": issue_number, + "relationship": "blocked_by", + "blocking_issue_number": blocking_issue_number, + "message": "blocked_by removed", + } + + +@api_router.post("/issues/{issue_number}/dependencies/blocking") +async def add_blocking( + gitctx: Annotated[Connector, Depends(connection)], + issue_number: Annotated[int, Path(title="Issue")], + body: Annotated[AddDependency, Body(title="Dependency")], +): + """Mark this issue as blocking another issue. + + Generated-by: Cursor + """ + if body.issue_number == issue_number: + raise HTTPException(status_code=422, detail="An issue cannot block itself") + current = _get_issue_for_dependency(gitctx, issue_number) + blocked = _get_issue_for_dependency(gitctx, body.issue_number) + _post_blocked_by_dependency(gitctx, body.issue_number, current["id"]) + return { + "issue_number": issue_number, + "relationship": "blocking", + "linked_issue": _dependency_link_payload(blocked), + } + + +@api_router.delete( + "/issues/{issue_number}/dependencies/blocking/{blocked_issue_number}" +) +async def remove_blocking( + gitctx: Annotated[Connector, Depends(connection)], + issue_number: Annotated[int, Path(title="Issue")], + blocked_issue_number: Annotated[int, Path(title="Blocked Issue Number")], +): + """Stop this issue from blocking another issue. + + Generated-by: Cursor + """ + current = _get_issue_for_dependency(gitctx, issue_number) + try: + gitctx.delete( + f"/repos/{context.github_repo}/issues/{blocked_issue_number}/dependencies/" + f"blocked_by/{current['id']}" + ) + except requests.HTTPError as exc: + _raise_github_http_error(exc) + return { + "issue_number": issue_number, + "relationship": "blocking", + "blocked_issue_number": blocked_issue_number, + "message": "blocking removed", + } + + # """Label Management""" diff --git a/backend/src/github_pm/issue_hierarchy.py b/backend/src/github_pm/issue_hierarchy.py index 14096f6..72721af 100644 --- a/backend/src/github_pm/issue_hierarchy.py +++ b/backend/src/github_pm/issue_hierarchy.py @@ -148,6 +148,51 @@ def apply_graphql_hierarchy(issue: dict, issue_node: dict | None) -> None: } +def _dependency_nodes(connection: dict | None) -> list[dict]: + """Map GraphQL blockedBy/blocking connection nodes to API payloads. + + Generated-by: Cursor + """ + nodes = (connection or {}).get("nodes") or [] + return [ + { + "id": linked.get("databaseId"), + "number": linked["number"], + "title": linked.get("title"), + "url": linked.get("url"), + "state": linked.get("state"), + } + for linked in nodes + if linked.get("number") is not None + ] + + +def apply_graphql_links(issue: dict, issue_node: dict | None) -> None: + """Attach closed-by PRs and blocked-by / blocking issue links from GraphQL. + + Generated-by: Cursor + """ + if not issue_node: + return + closed_refs = issue_node.get("closedByPullRequestsReferences") or {} + closed = closed_refs.get("nodes") or [] + if closed: + issue["closed_by"] = [ + { + "number": linked["number"], + "title": linked.get("title"), + "url": linked.get("url"), + } + for linked in closed + ] + blocked_by = _dependency_nodes(issue_node.get("blockedBy")) + if blocked_by: + issue["blocked_by"] = blocked_by + blocking = _dependency_nodes(issue_node.get("blocking")) + if blocking: + issue["blocking"] = blocking + + ISSUE_HIERARCHY_GRAPHQL = """ query($owner: String!, $repo: String!, $issue: Int!) { repository(owner: $owner, name: $repo, followRenames: true) { @@ -159,6 +204,24 @@ def apply_graphql_hierarchy(issue: dict, issue_node: dict | None) -> None: url } } + blockedBy(first: 50) { + nodes { + databaseId + number + title + url + state + } + } + blocking(first: 50) { + nodes { + databaseId + number + title + url + state + } + } parent { number title diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 596463a..84cb7cf 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -11,8 +11,11 @@ import requests from github_pm.api import ( + add_blocked_by, + add_blocking, add_label_to_issue, add_milestone_to_issue, + AddDependency, adopt_parent_milestone, api_router, clear_issue_parent, @@ -37,6 +40,8 @@ get_labels, get_milestones, get_project, + remove_blocked_by, + remove_blocking, remove_label_from_issue, remove_milestone_from_issue, render_markdown, @@ -349,6 +354,67 @@ async def test_get_issues_with_linked_prs(self): assert result["issues"][0]["closed_by"][1]["number"] == 456 mock_gitctx.post.assert_called_once() + @pytest.mark.asyncio + async def test_get_issues_with_dependencies(self): + """Test getting issues with blocked_by / blocking from GraphQL.""" + mock_issues = [ + { + "id": 1, + "number": 1, + "title": "Issue 1", + "labels": [], + } + ] + + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get_paged = Mock(return_value=mock_issues) + mock_gitctx.post = Mock( + return_value={ + "data": { + "repository": { + "issue": { + "closedByPullRequestsReferences": {"nodes": []}, + "blockedBy": { + "nodes": [ + { + "databaseId": 17, + "number": 17, + "title": "Blocker", + "url": "https://github.com/test/repo/issues/17", + "state": "OPEN", + } + ] + }, + "blocking": { + "nodes": [ + { + "databaseId": 88, + "number": 88, + "title": "Waiting", + "url": "https://github.com/test/repo/issues/88", + "state": "OPEN", + } + ] + }, + } + } + } + } + ) + mock_gitctx.owner = "test" + mock_gitctx.repo = "repo" + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await get_issues(mock_gitctx, milestone_number=1) + + assert len(result["issues"]) == 1 + issue = result["issues"][0] + assert issue["blocked_by"][0]["number"] == 17 + assert issue["blocked_by"][0]["id"] == 17 + assert issue["blocking"][0]["number"] == 88 + assert "closed_by" not in issue + @pytest.mark.asyncio async def test_get_issues_with_no_milestone(self): """Test getting issues with milestone_number=0 (no milestone).""" @@ -1118,6 +1184,175 @@ async def test_clear_parent_missing(self): assert exc.value.status_code == 404 +class TestIssueDependencies: + """Test blocked_by / blocking dependency endpoints.""" + + @pytest.mark.asyncio + async def test_add_blocked_by(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock( + side_effect=[ + {"id": 1, "number": 1, "title": "Blocked"}, + { + "id": 17, + "number": 17, + "title": "Blocker", + "html_url": "https://github.com/test/repo/issues/17", + "state": "open", + }, + ] + ) + mock_gitctx.post = Mock( + return_value={ + "id": 17, + "number": 17, + "title": "Blocker", + "html_url": "https://github.com/test/repo/issues/17", + "state": "open", + } + ) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await add_blocked_by( + mock_gitctx, 1, AddDependency(issue_number=17) + ) + + assert result["relationship"] == "blocked_by" + assert result["linked_issue"]["number"] == 17 + assert result["linked_issue"]["state"] == "OPEN" + mock_gitctx.post.assert_called_once_with( + "/repos/test/repo/issues/1/dependencies/blocked_by", + data={"issue_id": 17}, + ) + + @pytest.mark.asyncio + async def test_add_blocked_by_idempotent_when_already_linked(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock( + side_effect=[ + {"id": 1, "number": 1, "title": "Blocked"}, + { + "id": 17, + "number": 17, + "title": "Blocker", + "html_url": "https://github.com/test/repo/issues/17", + "state": "open", + }, + ] + ) + response = Mock() + response.status_code = 422 + response.json.return_value = { + "message": ( + "An error occurred while adding the blocking issue to the issue. " + "Validation failed: Target issue has already been taken" + ) + } + response.text = "Validation failed" + mock_gitctx.post = Mock(side_effect=requests.HTTPError(response=response)) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await add_blocked_by( + mock_gitctx, 1, AddDependency(issue_number=17) + ) + + assert result["linked_issue"]["number"] == 17 + assert result["relationship"] == "blocked_by" + + @pytest.mark.asyncio + async def test_add_blocked_by_rejects_self(self): + mock_gitctx = Mock(spec=Connector) + with pytest.raises(HTTPException) as exc: + await add_blocked_by(mock_gitctx, 5, AddDependency(issue_number=5)) + assert exc.value.status_code == 422 + + @pytest.mark.asyncio + async def test_add_blocked_by_rejects_pull_request(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock( + side_effect=[ + {"id": 1, "number": 1, "title": "Issue"}, + {"id": 9, "number": 9, "title": "PR", "pull_request": {}}, + ] + ) + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + with pytest.raises(HTTPException) as exc: + await add_blocked_by(mock_gitctx, 1, AddDependency(issue_number=9)) + assert exc.value.status_code == 422 + + @pytest.mark.asyncio + async def test_remove_blocked_by(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock( + return_value={"id": 17, "number": 17, "title": "Blocker"} + ) + mock_gitctx.delete = Mock(return_value={}) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await remove_blocked_by(mock_gitctx, 1, 17) + + assert result["blocking_issue_number"] == 17 + mock_gitctx.delete.assert_called_once_with( + "/repos/test/repo/issues/1/dependencies/blocked_by/17" + ) + + @pytest.mark.asyncio + async def test_add_blocking(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock( + side_effect=[ + {"id": 1, "number": 1, "title": "Blocker"}, + { + "id": 88, + "number": 88, + "title": "Waiting", + "html_url": "https://github.com/test/repo/issues/88", + "state": "open", + }, + ] + ) + mock_gitctx.post = Mock( + return_value={ + "id": 1, + "number": 1, + "title": "Blocker", + "html_url": "https://github.com/test/repo/issues/1", + "state": "open", + } + ) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await add_blocking(mock_gitctx, 1, AddDependency(issue_number=88)) + + assert result["relationship"] == "blocking" + assert result["linked_issue"]["number"] == 88 + assert result["linked_issue"]["title"] == "Waiting" + mock_gitctx.post.assert_called_once_with( + "/repos/test/repo/issues/88/dependencies/blocked_by", + data={"issue_id": 1}, + ) + + @pytest.mark.asyncio + async def test_remove_blocking(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock(return_value={"id": 1, "number": 1, "title": "Blocker"}) + mock_gitctx.delete = Mock(return_value={}) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await remove_blocking(mock_gitctx, 1, 88) + + assert result["blocked_issue_number"] == 88 + mock_gitctx.delete.assert_called_once_with( + "/repos/test/repo/issues/88/dependencies/blocked_by/1" + ) + + class TestAdoptParentMilestone: """Test POST adopt-parent-milestone.""" diff --git a/backend/tests/test_issue_hierarchy.py b/backend/tests/test_issue_hierarchy.py index 1de35b6..ea68831 100644 --- a/backend/tests/test_issue_hierarchy.py +++ b/backend/tests/test_issue_hierarchy.py @@ -6,6 +6,7 @@ from github_pm.api import _sort_items_by_labels from github_pm.issue_hierarchy import ( apply_graphql_hierarchy, + apply_graphql_links, build_issue_forest, collect_descendant_numbers, is_ancestor, @@ -147,6 +148,86 @@ def test_applies_parent_and_summary(self): assert issue["sub_issues_summary"]["percent_completed"] == 25 +class TestApplyGraphqlLinks: + def test_applies_closed_by_and_dependencies(self): + issue = {"number": 1} + apply_graphql_links( + issue, + { + "closedByPullRequestsReferences": { + "nodes": [ + { + "number": 9, + "title": "PR", + "url": "https://example.com/pull/9", + } + ] + }, + "blockedBy": { + "nodes": [ + { + "databaseId": 100, + "number": 2, + "title": "Blocker", + "url": "https://example.com/issues/2", + "state": "OPEN", + } + ] + }, + "blocking": { + "nodes": [ + { + "databaseId": 200, + "number": 3, + "title": "Blocked", + "url": "https://example.com/issues/3", + "state": "CLOSED", + } + ] + }, + }, + ) + assert issue["closed_by"] == [ + { + "number": 9, + "title": "PR", + "url": "https://example.com/pull/9", + } + ] + assert issue["blocked_by"] == [ + { + "id": 100, + "number": 2, + "title": "Blocker", + "url": "https://example.com/issues/2", + "state": "OPEN", + } + ] + assert issue["blocking"] == [ + { + "id": 200, + "number": 3, + "title": "Blocked", + "url": "https://example.com/issues/3", + "state": "CLOSED", + } + ] + + def test_omits_empty_link_collections(self): + issue = {"number": 1} + apply_graphql_links( + issue, + { + "closedByPullRequestsReferences": {"nodes": []}, + "blockedBy": {"nodes": []}, + "blocking": None, + }, + ) + assert "closed_by" not in issue + assert "blocked_by" not in issue + assert "blocking" not in issue + + class TestCollectDescendants: def test_bfs_descendants(self): tree = {1: [2, 3], 2: [4], 3: [], 4: []} diff --git a/frontend/src/components/IssueCard.jsx b/frontend/src/components/IssueCard.jsx index 7bd5228..a16f80e 100644 --- a/frontend/src/components/IssueCard.jsx +++ b/frontend/src/components/IssueCard.jsx @@ -24,6 +24,8 @@ import { ExclamationTriangleIcon, PlusIcon, PencilAltIcon, + LockIcon, + ArrowRightIcon, } from '@patternfly/react-icons'; import { getDaysSince, formatDate } from '../utils/dateUtils'; import { @@ -43,6 +45,10 @@ import { closeIssueWithComment, createIssue, updateIssueBody, + addBlockedBy, + removeBlockedBy, + addBlocking, + removeBlocking, } from '../services/api'; import CommentCard from './CommentCard'; import Reactions from './Reactions'; @@ -158,6 +164,18 @@ const IssueCard = ({ const [commentCount, setCommentCount] = useState(issue.comments || 0); const [descriptionBody, setDescriptionBody] = useState(issue.body || ''); const [descriptionHtml, setDescriptionHtml] = useState(issue.body_html || ''); + const [currentBlockedBy, setCurrentBlockedBy] = useState( + issue.blocked_by || [] + ); + const [currentBlocking, setCurrentBlocking] = useState(issue.blocking || []); + const [isLinkMenuOpen, setIsLinkMenuOpen] = useState(false); + const [linkRelation, setLinkRelation] = useState('blocked_by'); + const [linkIssueNumber, setLinkIssueNumber] = useState(''); + const [linkError, setLinkError] = useState(null); + const [linkBusy, setLinkBusy] = useState(false); + const [removingLinkKey, setRemovingLinkKey] = useState(null); + const linkMenuRef = useRef(null); + const linkToggleRef = useRef(null); useEffect(() => { setDescriptionBody(issue.body || ''); @@ -219,6 +237,14 @@ const IssueCard = ({ setCurrentAssignees(Array.isArray(issue.assignees) ? issue.assignees : []); }, [issue.assignees]); + useEffect(() => { + setCurrentBlockedBy(issue.blocked_by || []); + }, [issue.blocked_by]); + + useEffect(() => { + setCurrentBlocking(issue.blocking || []); + }, [issue.blocking]); + // Fetch reactions if total_count > 0 useEffect(() => { // Reset reactions when issue changes @@ -562,9 +588,25 @@ const IssueCard = ({ // Clicking outside - apply changes before closing handleApplyAssignees(); } + if ( + isLinkMenuOpen && + linkToggleRef.current && + !linkToggleRef.current.contains(event.target) && + linkMenuRef.current && + !linkMenuRef.current.contains(event.target) + ) { + setIsLinkMenuOpen(false); + setLinkError(null); + setLinkIssueNumber(''); + } }; - if (isLabelMenuOpen || isMilestoneMenuOpen || isAssigneesMenuOpen) { + if ( + isLabelMenuOpen || + isMilestoneMenuOpen || + isAssigneesMenuOpen || + isLinkMenuOpen + ) { document.addEventListener('mousedown', handleClickOutside); return () => { document.removeEventListener('mousedown', handleClickOutside); @@ -574,6 +616,7 @@ const IssueCard = ({ isLabelMenuOpen, isMilestoneMenuOpen, isAssigneesMenuOpen, + isLinkMenuOpen, handleApplyAssignees, ]); @@ -840,8 +883,191 @@ const IssueCard = ({ }); }; - // Column 4: PR icon, "closed by #", or blank - const renderPrColumn = () => { + // Column 4: Links — closed-by PRs, blocked-by / blocking issues, or PR branch icon + const linkChicletStyle = (opts = {}) => ({ + display: 'inline-flex', + alignItems: 'center', + gap: '0.25rem', + padding: '0.125rem 0.25rem 0.125rem 0.375rem', + fontSize: '0.75rem', + fontWeight: '500', + borderRadius: '0.25rem', + whiteSpace: 'nowrap', + maxWidth: '100%', + backgroundColor: opts.backgroundColor || '#f0f0f0', + color: opts.color || '#151515', + opacity: opts.dimmed ? 0.65 : 1, + textDecoration: opts.dimmed ? 'line-through' : 'none', + }); + + const notifyLinksChanged = (blockedBy, blocking) => { + if (onIssueUpdate) { + onIssueUpdate({ + ...issue, + blocked_by: blockedBy, + blocking, + }); + } + }; + + const handleRemoveBlockedBy = async (depNumber) => { + const key = `blocked-by-${depNumber}`; + setRemovingLinkKey(key); + setLinkError(null); + try { + await removeBlockedBy(issue.number, depNumber); + const next = currentBlockedBy.filter((d) => d.number !== depNumber); + setCurrentBlockedBy(next); + notifyLinksChanged(next, currentBlocking); + } catch (err) { + console.error('Failed to remove blocked-by link:', err); + setLinkError(err.message); + } finally { + setRemovingLinkKey(null); + } + }; + + const handleRemoveBlocking = async (depNumber) => { + const key = `blocking-${depNumber}`; + setRemovingLinkKey(key); + setLinkError(null); + try { + await removeBlocking(issue.number, depNumber); + const next = currentBlocking.filter((d) => d.number !== depNumber); + setCurrentBlocking(next); + notifyLinksChanged(currentBlockedBy, next); + } catch (err) { + console.error('Failed to remove blocking link:', err); + setLinkError(err.message); + } finally { + setRemovingLinkKey(null); + } + }; + + const handleAddLink = async () => { + const parsed = parseInt(String(linkIssueNumber).trim(), 10); + if (!Number.isFinite(parsed) || parsed <= 0) { + setLinkError('Enter a valid issue number'); + return; + } + if (parsed === issue.number) { + setLinkError('An issue cannot link to itself'); + return; + } + const existing = + linkRelation === 'blocked_by' ? currentBlockedBy : currentBlocking; + if (existing.some((d) => d.number === parsed)) { + setLinkError(`#${parsed} is already linked`); + return; + } + + setLinkBusy(true); + setLinkError(null); + try { + const result = + linkRelation === 'blocked_by' + ? await addBlockedBy(issue.number, parsed) + : await addBlocking(issue.number, parsed); + const linked = result.linked_issue; + if (linkRelation === 'blocked_by') { + const next = [...currentBlockedBy, linked]; + setCurrentBlockedBy(next); + notifyLinksChanged(next, currentBlocking); + } else { + const next = [...currentBlocking, linked]; + setCurrentBlocking(next); + notifyLinksChanged(currentBlockedBy, next); + } + setLinkIssueNumber(''); + setIsLinkMenuOpen(false); + } catch (err) { + console.error('Failed to add link:', err); + setLinkError(err.message); + } finally { + setLinkBusy(false); + } + }; + + const renderLinkChiclet = ({ + key, + href, + number, + title, + relationshipLabel, + Icon, + iconColor, + backgroundColor, + dimmed, + onRemove, + removeLabel, + removing, + }) => ( + + + e.stopPropagation()} + > + + #{number} + + {onRemove && ( + + )} + + + ); + + const renderLinksColumn = () => { if (issue.pull_request) { return ( @@ -855,32 +1081,174 @@ const IssueCard = ({ ); } - if (issue.closed_by && issue.closed_by.length > 0) { - return ( - - closed by{' '} - {issue.closed_by.map((pr, index) => ( - - {index > 0 && ', '} - - + {closedBy.map((pr) => + renderLinkChiclet({ + key: `closed-${pr.number}`, + href: pr.url, + number: pr.number, + title: pr.title, + relationshipLabel: 'Closed by', + Icon: CodeBranchIcon, + iconColor: '#0066cc', + backgroundColor: '#e7f1fa', + }) + )} + {currentBlockedBy.map((dep) => + renderLinkChiclet({ + key: `blocked-by-${dep.number}`, + href: dep.url, + number: dep.number, + title: dep.title, + relationshipLabel: 'Blocked by', + Icon: LockIcon, + iconColor: '#f0ab00', + backgroundColor: '#fdf7e7', + dimmed: dep.state === 'CLOSED', + onRemove: () => handleRemoveBlockedBy(dep.number), + removeLabel: `Remove blocked-by #${dep.number}`, + removing: removingLinkKey === `blocked-by-${dep.number}`, + }) + )} + {currentBlocking.map((dep) => + renderLinkChiclet({ + key: `blocking-${dep.number}`, + href: dep.url, + number: dep.number, + title: dep.title, + relationshipLabel: 'Blocking', + Icon: ArrowRightIcon, + iconColor: '#0066cc', + backgroundColor: '#e7f1fa', + dimmed: dep.state === 'CLOSED', + onRemove: () => handleRemoveBlocking(dep.number), + removeLabel: `Remove blocking #${dep.number}`, + removing: removingLinkKey === `blocking-${dep.number}`, + }) + )} +
+ + + + {isLinkMenuOpen && ( +
e.stopPropagation()} + > +
+ + +
+ { + const stringValue = + typeof value === 'string' + ? value + : value?.target?.value || ''; + setLinkIssueNumber(stringValue); + }} + placeholder="Issue number" + aria-label="Related issue number" + onKeyDown={(e) => { + if (e.key === 'Enter') { + e.preventDefault(); + handleAddLink(); + } + }} + /> + {linkError && ( + + )} + +
+ )} +
+ {linkError && !isLinkMenuOpen && ( + + )} + + ); }; const cellStyle = { @@ -1331,8 +1699,8 @@ const IssueCard = ({ - {/* Column 4: PR icon or closed by #pr or blank */} - {renderPrColumn()} + {/* Column 4: Links (closed-by PRs, blocked-by / blocking) */} + {renderLinksColumn()} {/* Column 5: Milestone control */} diff --git a/frontend/src/components/IssueCard.test.jsx b/frontend/src/components/IssueCard.test.jsx index 839b354..8fdc284 100644 --- a/frontend/src/components/IssueCard.test.jsx +++ b/frontend/src/components/IssueCard.test.jsx @@ -160,6 +160,176 @@ describe('IssueCard', () => { }); }); + it('renders Links column closed-by and dependency chiclets', async () => { + const issueWithLinks = { + ...mockIssue, + closed_by: [ + { + number: 123, + title: 'Fix it', + url: 'https://github.com/test/repo/pull/123', + }, + ], + blocked_by: [ + { + id: 17, + number: 17, + title: 'Blocker', + url: 'https://github.com/test/repo/issues/17', + state: 'OPEN', + }, + ], + blocking: [ + { + id: 88, + number: 88, + title: 'Waiting', + url: 'https://github.com/test/repo/issues/88', + state: 'CLOSED', + }, + ], + }; + await act(async () => { + render( + + + + +
+ ); + }); + await waitFor(() => { + const closedBy = screen.getByRole('link', { name: '#123' }); + expect(closedBy).toHaveAttribute( + 'href', + 'https://github.com/test/repo/pull/123' + ); + const blockedBy = screen.getByRole('link', { name: '#17' }); + expect(blockedBy).toHaveAttribute( + 'href', + 'https://github.com/test/repo/issues/17' + ); + const blocking = screen.getByRole('link', { name: '#88' }); + expect(blocking).toHaveAttribute( + 'href', + 'https://github.com/test/repo/issues/88' + ); + }); + expect( + screen.getByRole('button', { name: 'Remove blocked-by #17' }) + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Remove blocking #88' }) + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Add link' }) + ).toBeInTheDocument(); + }); + + it('removes a blocked-by link when × is clicked', async () => { + const user = userEvent.setup(); + const onIssueUpdate = vi.fn(); + api.removeBlockedBy.mockResolvedValue({ + issue_number: 459, + relationship: 'blocked_by', + blocking_issue_number: 17, + }); + const issueWithLinks = { + ...mockIssue, + blocked_by: [ + { + id: 17, + number: 17, + title: 'Blocker', + url: 'https://github.com/test/repo/issues/17', + state: 'OPEN', + }, + ], + }; + await act(async () => { + render( + + + + +
+ ); + }); + await user.click( + screen.getByRole('button', { name: 'Remove blocked-by #17' }) + ); + await waitFor(() => { + expect(api.removeBlockedBy).toHaveBeenCalledWith(459, 17); + }); + expect(screen.queryByRole('link', { name: '#17' })).not.toBeInTheDocument(); + expect(onIssueUpdate).toHaveBeenCalledWith( + expect.objectContaining({ blocked_by: [] }) + ); + }); + + it('adds a blocking link from the Links popover', async () => { + const user = userEvent.setup(); + const onIssueUpdate = vi.fn(); + api.addBlocking.mockResolvedValue({ + issue_number: 459, + relationship: 'blocking', + linked_issue: { + id: 88, + number: 88, + title: 'Waiting', + url: 'https://github.com/test/repo/issues/88', + state: 'OPEN', + }, + }); + await act(async () => { + render( + + + + +
+ ); + }); + await user.click(screen.getByRole('button', { name: 'Add link' })); + await user.click(screen.getByRole('button', { name: 'Blocking' })); + await user.type( + screen.getByRole('textbox', { name: 'Related issue number' }), + '88' + ); + await user.click(screen.getByRole('button', { name: 'Add' })); + await waitFor(() => { + expect(api.addBlocking).toHaveBeenCalledWith(459, 88); + }); + expect(screen.getByRole('link', { name: '#88' })).toBeInTheDocument(); + expect(onIssueUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + blocking: [expect.objectContaining({ number: 88, title: 'Waiting' })], + }) + ); + }); + + it('renders branch icon in Links column for pull request rows', async () => { + const prIssue = { + ...mockIssue, + pull_request: {}, + title: 'My PR', + }; + await act(async () => { + render( + + + + +
+ ); + }); + await waitFor(() => { + expect(screen.getByText('#459')).toBeInTheDocument(); + }); + // PatternFly CodeBranchIcon renders an svg; tooltip content is "Pull Request" + expect(document.querySelector('svg')).toBeTruthy(); + }); + it('renders labels when present', async () => { await act(async () => { render(); diff --git a/frontend/src/components/MilestoneCard.jsx b/frontend/src/components/MilestoneCard.jsx index 2a10358..1adb902 100644 --- a/frontend/src/components/MilestoneCard.jsx +++ b/frontend/src/components/MilestoneCard.jsx @@ -40,7 +40,7 @@ const itemTableHeader = (includeType) => ( Number {includeType && Type} Author - PR + Links Milestone Labels Title diff --git a/frontend/src/components/MilestoneCard.test.jsx b/frontend/src/components/MilestoneCard.test.jsx index 5e0160e..26c7dd6 100644 --- a/frontend/src/components/MilestoneCard.test.jsx +++ b/frontend/src/components/MilestoneCard.test.jsx @@ -209,6 +209,9 @@ describe('MilestoneCard', () => { const prToggle = screen.getByRole('button', { name: /show 1 pr/i }); expect(prToggle).toBeInTheDocument(); expect(prToggle).toHaveAttribute('aria-expanded', 'false'); + expect( + screen.getByRole('columnheader', { name: 'Links' }) + ).toBeInTheDocument(); await user.click(prToggle); diff --git a/frontend/src/services/api.js b/frontend/src/services/api.js index 60f8e16..e9b3367 100644 --- a/frontend/src/services/api.js +++ b/frontend/src/services/api.js @@ -1,6 +1,32 @@ // Generated-by: Cursor const API_BASE = '/api/v1'; +const readErrorDetail = async (response) => { + try { + const body = await response.json(); + if (typeof body?.detail === 'string' && body.detail.trim()) { + return body.detail; + } + if (body?.detail != null) { + return JSON.stringify(body.detail); + } + if (typeof body?.message === 'string' && body.message.trim()) { + return body.message; + } + } catch { + // Ignore non-JSON error bodies. + } + return response.statusText || `HTTP ${response.status}`; +}; + +const raiseForResponse = async (response, fallback) => { + if (response.ok) { + return; + } + const detail = await readErrorDetail(response); + throw new Error(`${fallback}: ${detail}`); +}; + export const fetchMilestones = async () => { const response = await fetch(`${API_BASE}/milestones`); if (!response.ok) { @@ -272,6 +298,58 @@ export const adoptParentMilestone = async (issueNumber) => { return response.json(); }; +export const addBlockedBy = async (issueNumber, relatedIssueNumber) => { + const response = await fetch( + `${API_BASE}/issues/${issueNumber}/dependencies/blocked_by`, + { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ issue_number: relatedIssueNumber }), + } + ); + await raiseForResponse(response, 'Failed to add blocked-by link'); + return response.json(); +}; + +export const removeBlockedBy = async (issueNumber, blockingIssueNumber) => { + const response = await fetch( + `${API_BASE}/issues/${issueNumber}/dependencies/blocked_by/${blockingIssueNumber}`, + { + method: 'DELETE', + } + ); + await raiseForResponse(response, 'Failed to remove blocked-by link'); + return response.json(); +}; + +export const addBlocking = async (issueNumber, relatedIssueNumber) => { + const response = await fetch( + `${API_BASE}/issues/${issueNumber}/dependencies/blocking`, + { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ issue_number: relatedIssueNumber }), + } + ); + await raiseForResponse(response, 'Failed to add blocking link'); + return response.json(); +}; + +export const removeBlocking = async (issueNumber, blockedIssueNumber) => { + const response = await fetch( + `${API_BASE}/issues/${issueNumber}/dependencies/blocking/${blockedIssueNumber}`, + { + method: 'DELETE', + } + ); + await raiseForResponse(response, 'Failed to remove blocking link'); + return response.json(); +}; + export const fetchIssueReactions = async (issueNumber) => { const response = await fetch(`${API_BASE}/issues/${issueNumber}/reactions`); if (!response.ok) { diff --git a/frontend/src/services/api.test.js b/frontend/src/services/api.test.js index a184d6e..1abb249 100644 --- a/frontend/src/services/api.test.js +++ b/frontend/src/services/api.test.js @@ -12,6 +12,10 @@ import { setIssueParent, clearIssueParent, adoptParentMilestone, + addBlockedBy, + removeBlockedBy, + addBlocking, + removeBlocking, createComment, closeIssueWithComment, renderMarkdown, @@ -269,6 +273,64 @@ describe('api', () => { }); }); + describe('dependency APIs', () => { + it('addBlockedBy POSTs issue_number', async () => { + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ relationship: 'blocked_by' }), + }); + await addBlockedBy(1, 17); + expect(global.fetch).toHaveBeenCalledWith( + '/api/v1/issues/1/dependencies/blocked_by', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ issue_number: 17 }), + } + ); + }); + + it('removeBlockedBy DELETEs by blocking issue number', async () => { + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ message: 'blocked_by removed' }), + }); + await removeBlockedBy(1, 17); + expect(global.fetch).toHaveBeenCalledWith( + '/api/v1/issues/1/dependencies/blocked_by/17', + { method: 'DELETE' } + ); + }); + + it('addBlocking POSTs issue_number', async () => { + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ relationship: 'blocking' }), + }); + await addBlocking(1, 88); + expect(global.fetch).toHaveBeenCalledWith( + '/api/v1/issues/1/dependencies/blocking', + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ issue_number: 88 }), + } + ); + }); + + it('removeBlocking DELETEs by blocked issue number', async () => { + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ message: 'blocking removed' }), + }); + await removeBlocking(1, 88); + expect(global.fetch).toHaveBeenCalledWith( + '/api/v1/issues/1/dependencies/blocking/88', + { method: 'DELETE' } + ); + }); + }); + describe('comment and issue write APIs', () => { it('createComment POSTs body', async () => { global.fetch.mockResolvedValue({