From 4c472299e65d6e279829c2e7c9b6447fa12ecdb8 Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Sat, 25 Jul 2026 08:40:14 -0400 Subject: [PATCH 01/11] Separate issues and PRs in API & display Generated-by: Cursor Signed-off-by: David Butenhof --- backend/src/github_pm/api.py | 131 ++++---- backend/tests/test_api.py | 139 ++++++-- frontend/README.md | 2 +- frontend/src/components/MilestoneCard.jsx | 301 ++++++++++-------- .../src/components/MilestoneCard.test.jsx | 127 ++++++-- frontend/src/services/api.test.js | 9 +- 6 files changed, 458 insertions(+), 251 deletions(-) diff --git a/backend/src/github_pm/api.py b/backend/src/github_pm/api.py index 4693daf..ae58536 100644 --- a/backend/src/github_pm/api.py +++ b/backend/src/github_pm/api.py @@ -185,6 +185,26 @@ async def get_project(): } +def _sort_items_by_labels(items: list[dict], sort_by: list[str]) -> list[dict]: + """Sort open items by label priority, then by number within each bucket. + + Assisted-by: Cursor + """ + buckets: dict[str, list] = defaultdict(list) + for item in items: + labels = {label["name"].lower() for label in item["labels"]} + for label in sort_by: + if label in labels: + buckets[label].append(item) + break + else: + buckets["other"].append(item) + ordered: list[dict] = [] + for label in sort_by + ["other"]: + ordered.extend(sorted(buckets[label], key=lambda x: x["number"])) + return ordered + + @api_router.get("/issues/{milestone_number}") async def get_issues( gitctx: Annotated[Connector, Depends(connection)], @@ -193,76 +213,79 @@ async def get_issues( str | None, Query(title="Sort", description="List of labels to sort by") ] = None, ): + """Return open issues and pull requests for a milestone as separate lists. + + Each list is sorted independently by the optional label sort criteria. + Assisted-by: Cursor + """ if sort: sort_by = [s.strip() for s in sort.split(",")] else: sort_by = [] - sorted_issues = defaultdict(list) start = time.time() milestone = "none" if milestone_number == 0 else milestone_number - issues = gitctx.get_paged( + raw_items = gitctx.get_paged( f"/repos/{context.github_repo}/issues?milestone={milestone}&state=open", headers={"Accept": "application/vnd.github.html+json"}, ) - for i in issues: - labels = set([label["name"].lower() for label in i["labels"]]) - if "pull_request" not in i: - 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 - } + issues: list[dict] = [] + pull_requests: list[dict] = [] + for i in raw_items: + if "pull_request" in i: + pull_requests.append(i) + continue + 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, - "variables": { - "owner": gitctx.owner, - "repo": gitctx.repo, - "issue": i["number"], - }, + } + """ + try: + response = gitctx.post( + "/graphql", + data={ + "query": query, + "variables": { + "owner": gitctx.owner, + "repo": gitctx.repo, + "issue": i["number"], }, - ) - data = response["data"] - issue_node = data["repository"]["issue"] - closed = issue_node["closedByPullRequestsReferences"]["nodes"] - if len(closed) > 0: - i["closed_by"] = [ - { - "number": linked["number"], - "title": linked["title"], - "url": linked["url"], - } - for linked in closed - ] - except Exception as e: - logger.exception( - f"Error finding linked PRs for issue {i['number']}: {e!r}" - ) - continue - for label in sort_by: - if label in labels: - sorted_issues[label].append(i) - break - else: - sorted_issues["other"].append(i) - all_issues = [] - for label in sort_by + ["other"]: - all_issues.extend(sorted(sorted_issues[label], key=lambda x: x["number"])) + }, + ) + data = response["data"] + issue_node = data["repository"]["issue"] + closed = issue_node["closedByPullRequestsReferences"]["nodes"] + if len(closed) > 0: + i["closed_by"] = [ + { + "number": linked["number"], + "title": linked["title"], + "url": linked["url"], + } + for linked in closed + ] + except Exception as e: + logger.exception(f"Error finding linked PRs for issue {i['number']}: {e!r}") + continue + issues.append(i) + sorted_issues = _sort_items_by_labels(issues, sort_by) + sorted_prs = _sort_items_by_labels(pull_requests, sort_by) logger.debug( - f"{len(issues)}({len(all_issues)}) issues: {time.time() - start:.3f} seconds" + "%s(%s issues, %s PRs) items: %.3f seconds", + len(raw_items), + len(sorted_issues), + len(sorted_prs), + time.time() - start, ) - return all_issues + return {"issues": sorted_issues, "pull_requests": sorted_prs} @api_router.get("/issue/{issue_number}") diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 75f64e9..00772fd 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -206,9 +206,10 @@ async def test_get_issues_with_milestone(self): result = await get_issues(mock_gitctx, milestone_number=1) # Assert - assert len(result) == 2 - assert result[0]["id"] == 1 - assert result[1]["id"] == 2 + assert len(result["issues"]) == 1 + assert len(result["pull_requests"]) == 1 + assert result["pull_requests"][0]["id"] == 1 + assert result["issues"][0]["id"] == 2 mock_gitctx.get_paged.assert_called_once() # Issue 1 has pull_request, so no GraphQL call # Issue 2 doesn't have pull_request, so GraphQL should be called @@ -263,17 +264,18 @@ async def test_get_issues_with_linked_prs(self): result = await get_issues(mock_gitctx, milestone_number=1) # Assert - assert len(result) == 1 - assert result[0]["id"] == 1 - assert "closed_by" in result[0] - assert len(result[0]["closed_by"]) == 2 - assert result[0]["closed_by"][0]["number"] == 123 - assert result[0]["closed_by"][0]["title"] == "Fix Issue 1" + assert len(result["issues"]) == 1 + assert result["pull_requests"] == [] + assert result["issues"][0]["id"] == 1 + assert "closed_by" in result["issues"][0] + assert len(result["issues"][0]["closed_by"]) == 2 + assert result["issues"][0]["closed_by"][0]["number"] == 123 + assert result["issues"][0]["closed_by"][0]["title"] == "Fix Issue 1" assert ( - result[0]["closed_by"][0]["url"] + result["issues"][0]["closed_by"][0]["url"] == "https://github.com/test/repo/pull/123" ) - assert result[0]["closed_by"][1]["number"] == 456 + assert result["issues"][0]["closed_by"][1]["number"] == 456 mock_gitctx.post.assert_called_once() @pytest.mark.asyncio @@ -310,8 +312,9 @@ async def test_get_issues_with_no_milestone(self): result = await get_issues(mock_gitctx, milestone_number=0) # Assert - assert len(result) == 1 - assert result[0]["id"] == 1 + assert len(result["issues"]) == 1 + assert result["pull_requests"] == [] + assert result["issues"][0]["id"] == 1 mock_gitctx.get_paged.assert_called_once() # GraphQL should be called for issue without pull_request assert mock_gitctx.post.call_count == 1 @@ -362,13 +365,14 @@ async def test_get_issues_with_sort_single_label(self): result = await get_issues(mock_gitctx, milestone_number=0, sort="bug") # Assert - assert len(result) == 3 + assert len(result["issues"]) == 3 + assert result["pull_requests"] == [] # First should be bug issue - assert result[0]["id"] == 1 - assert result[0]["title"] == "Bug Issue" + assert result["issues"][0]["id"] == 1 + assert result["issues"][0]["title"] == "Bug Issue" # Then other issues - assert result[1]["id"] == 2 - assert result[2]["id"] == 3 + assert result["issues"][1]["id"] == 2 + assert result["issues"][2]["id"] == 3 @pytest.mark.asyncio async def test_get_issues_with_sort_multiple_labels(self): @@ -424,12 +428,13 @@ async def test_get_issues_with_sort_multiple_labels(self): ) # Assert - assert len(result) == 4 + assert len(result["issues"]) == 4 + assert result["pull_requests"] == [] # Order should be: bug, feature, enhancement, other - assert result[0]["id"] == 1 # bug - assert result[1]["id"] == 2 # feature - assert result[2]["id"] == 3 # enhancement - assert result[3]["id"] == 4 # other (unlabeled) + assert result["issues"][0]["id"] == 1 # bug + assert result["issues"][1]["id"] == 2 # feature + assert result["issues"][2]["id"] == 3 # enhancement + assert result["issues"][3]["id"] == 4 # other (unlabeled) @pytest.mark.asyncio async def test_get_issues_with_sort_case_insensitive(self): @@ -473,10 +478,11 @@ async def test_get_issues_with_sort_case_insensitive(self): ) # Assert - assert len(result) == 2 + assert len(result["issues"]) == 2 + assert result["pull_requests"] == [] # Should match despite case difference - assert result[0]["id"] == 1 # bug (matched "BUG") - assert result[1]["id"] == 2 # feature (matched "Feature") + assert result["issues"][0]["id"] == 1 # bug (matched "BUG") + assert result["issues"][1]["id"] == 2 # feature (matched "Feature") @pytest.mark.asyncio async def test_get_issues_with_sort_whitespace_stripped(self): @@ -520,10 +526,11 @@ async def test_get_issues_with_sort_whitespace_stripped(self): ) # Assert - assert len(result) == 2 + assert len(result["issues"]) == 2 + assert result["pull_requests"] == [] # Should match despite whitespace - assert result[0]["id"] == 1 # bug - assert result[1]["id"] == 2 # feature + assert result["issues"][0]["id"] == 1 # bug + assert result["issues"][1]["id"] == 2 # feature @pytest.mark.asyncio async def test_get_issues_with_sort_first_match_wins(self): @@ -570,10 +577,11 @@ async def test_get_issues_with_sort_first_match_wins(self): ) # Assert - assert len(result) == 2 + assert len(result["issues"]) == 2 + assert result["pull_requests"] == [] # Issue 1 should be in bug category (first match), not feature - assert result[0]["id"] == 1 # bug (first match wins) - assert result[1]["id"] == 2 # feature + assert result["issues"][0]["id"] == 1 # bug (first match wins) + assert result["issues"][1]["id"] == 2 # feature @pytest.mark.asyncio async def test_get_issues_with_sort_all_other(self): @@ -615,10 +623,71 @@ async def test_get_issues_with_sort_all_other(self): result = await get_issues(mock_gitctx, milestone_number=0, sort=None) # Assert - assert len(result) == 2 + assert len(result["issues"]) == 2 + assert result["pull_requests"] == [] # Both should be in "other" since no sort labels specified - assert result[0]["id"] == 1 - assert result[1]["id"] == 2 + assert result["issues"][0]["id"] == 1 + assert result["issues"][1]["id"] == 2 + + @pytest.mark.asyncio + async def test_get_issues_sorts_issues_and_prs_separately(self): + """Issues and PRs are sorted independently by the same label criteria.""" + # Arrange + mock_items = [ + { + "id": 1, + "number": 1, + "title": "Bug Issue", + "labels": [{"name": "bug"}], + }, + { + "id": 2, + "number": 2, + "title": "Feature Issue", + "labels": [{"name": "feature"}], + }, + { + "id": 10, + "number": 10, + "title": "Feature PR", + "pull_request": {}, + "labels": [{"name": "feature"}], + }, + { + "id": 11, + "number": 11, + "title": "Bug PR", + "pull_request": {}, + "labels": [{"name": "bug"}], + }, + ] + + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get_paged = Mock(return_value=mock_items) + mock_gitctx.post = Mock( + return_value={ + "data": { + "repository": { + "issue": {"closedByPullRequestsReferences": {"nodes": []}} + } + } + } + ) + mock_gitctx.owner = "test" + mock_gitctx.repo = "repo" + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + + # Act + result = await get_issues( + mock_gitctx, milestone_number=1, sort="bug,feature" + ) + + # Assert: each list sorted independently (bug then feature) + assert [i["id"] for i in result["issues"]] == [1, 2] + assert [p["id"] for p in result["pull_requests"]] == [11, 10] + assert mock_gitctx.post.call_count == 2 class TestGetComments: diff --git a/frontend/README.md b/frontend/README.md index feb7d2f..9aebc45 100644 --- a/frontend/README.md +++ b/frontend/README.md @@ -87,7 +87,7 @@ Or add `BACKEND_PORT=8080` to a `.env.development` file in this directory. ### API Endpoints - `GET /api/milestones` - Returns list of milestones -- `GET /api/issues/{milestone_number}` - Returns issues for a milestone +- `GET /api/issues/{milestone_number}` - Returns `{ issues, pull_requests }` for a milestone ## Project Structure diff --git a/frontend/src/components/MilestoneCard.jsx b/frontend/src/components/MilestoneCard.jsx index ef39d99..4678512 100644 --- a/frontend/src/components/MilestoneCard.jsx +++ b/frontend/src/components/MilestoneCard.jsx @@ -1,4 +1,5 @@ // Generated-by: Cursor +// Assisted-by: Cursor import React, { useState, useEffect, useRef, useCallback } from 'react'; import { Card, @@ -12,32 +13,129 @@ import { import { fetchIssues } from '../services/api'; import IssueCard from './IssueCard'; +const itemTableHeader = ( + + + + + Number + + + Author + + + PR + + + Milestone + + + Labels + + + Title + + + +); + const MilestoneCard = ({ milestone, sortOrder = [], issueMilestoneRefresh = { key: 0, milestoneNumbers: [] }, onIssueMilestoneMoved, }) => { - const [isExpanded, setIsExpanded] = useState(false); + const [isIssuesExpanded, setIsIssuesExpanded] = useState(false); + const [isPrsExpanded, setIsPrsExpanded] = useState(false); const [issues, setIssues] = useState([]); + const [pullRequests, setPullRequests] = useState([]); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); const [hasLoadedOnce, setHasLoadedOnce] = useState(false); const prevMilestoneNumberRef = useRef(milestone.number); + const applyFetchedData = useCallback((data) => { + setIssues(data.issues || []); + setPullRequests(data.pull_requests || []); + }, []); + const refetchIssues = useCallback(() => { setLoading(true); setError(null); return fetchIssues(milestone.number, sortOrder) .then((data) => { - setIssues(data); + applyFetchedData(data); setLoading(false); }) .catch((err) => { setError(err.message); setLoading(false); }); - }, [milestone.number, sortOrder]); + }, [milestone.number, sortOrder, applyFetchedData]); // Reset loaded state when milestone changes useEffect(() => { @@ -45,18 +143,20 @@ const MilestoneCard = ({ prevMilestoneNumberRef.current = milestone.number; setHasLoadedOnce(false); setIssues([]); + setPullRequests([]); setError(null); setLoading(false); + setIsPrsExpanded(false); } }, [milestone.number]); useEffect(() => { - if (isExpanded && !hasLoadedOnce && !loading) { + if (isIssuesExpanded && !hasLoadedOnce && !loading) { setLoading(true); setError(null); fetchIssues(milestone.number, sortOrder) .then((data) => { - setIssues(data); + applyFetchedData(data); setLoading(false); setHasLoadedOnce(true); }) @@ -66,7 +166,14 @@ const MilestoneCard = ({ setHasLoadedOnce(true); }); } - }, [isExpanded, milestone.number, hasLoadedOnce, loading, sortOrder]); + }, [ + isIssuesExpanded, + milestone.number, + hasLoadedOnce, + loading, + sortOrder, + applyFetchedData, + ]); // Re-fetch issues when sort order changes (if already loaded) const prevSortOrderRef = useRef(sortOrder); @@ -74,7 +181,7 @@ const MilestoneCard = ({ // Only refetch if sort order actually changed and issues are already loaded const sortOrderChanged = JSON.stringify(prevSortOrderRef.current) !== JSON.stringify(sortOrder); - if (isExpanded && hasLoadedOnce && !loading && sortOrderChanged) { + if (isIssuesExpanded && hasLoadedOnce && !loading && sortOrderChanged) { prevSortOrderRef.current = sortOrder; refetchIssues(); } else { @@ -87,7 +194,7 @@ const MilestoneCard = ({ const { key, milestoneNumbers } = issueMilestoneRefresh; if (key === 0) return; if (!milestoneNumbers.includes(milestone.number)) return; - if (!isExpanded || !hasLoadedOnce) return; + if (!isIssuesExpanded || !hasLoadedOnce) return; refetchIssues(); // Bump `key` and `milestoneNumbers` update together; refetch only when key changes. // eslint-disable-next-line react-hooks/exhaustive-deps @@ -103,8 +210,8 @@ const MilestoneCard = ({ }); }; - const getToggleText = () => { - const baseText = isExpanded ? 'Hide' : 'Show'; + const getIssuesToggleText = () => { + const baseText = isIssuesExpanded ? 'Hide' : 'Show'; if (hasLoadedOnce) { const count = issues.length; @@ -115,6 +222,44 @@ const MilestoneCard = ({ return `${baseText} Issues`; }; + const getPrsToggleText = () => { + const baseText = isPrsExpanded ? 'Hide' : 'Show'; + const count = pullRequests.length; + const prText = count === 1 ? 'PR' : 'PRs'; + return `${baseText} ${count} ${prText}`; + }; + + const updateItemInList = (setter) => (updatedIssue) => { + setter((prev) => + prev.map((item) => (item.id === updatedIssue.id ? updatedIssue : item)) + ); + }; + + const renderItemTable = (items, onItemUpdate) => ( + + {itemTableHeader} + + {items.map((issue) => ( + { + onIssueMilestoneMoved?.(detail); + }} + onIssueUpdate={onItemUpdate} + /> + ))} + +
+ ); + return ( @@ -131,9 +276,9 @@ const MilestoneCard = ({ setIsExpanded(!isExpanded)} - isExpanded={isExpanded} + toggleText={getIssuesToggleText()} + onToggle={() => setIsIssuesExpanded(!isIssuesExpanded)} + isExpanded={isIssuesExpanded} > {loading && (
@@ -148,133 +293,31 @@ const MilestoneCard = ({ )} {!loading && !error && issues.length > 0 && ( - - - - - - - - - - - - - {issues.map((issue) => ( - { - onIssueMilestoneMoved?.(detail); - }} - onIssueUpdate={(updatedIssue) => { - // Update the issue in the issues array - setIssues((prevIssues) => - prevIssues.map((i) => - i.id === updatedIssue.id ? updatedIssue : i - ) - ); - }} - /> - ))} - -
- - Number - - Author - - PR - - Milestone - - Labels - - Title -
+ <>{renderItemTable(issues, updateItemInList(setIssues))} )} {!loading && !error && issues.length === 0 && - isExpanded && + isIssuesExpanded && hasLoadedOnce && (

No issues found for this milestone.

)} + + {hasLoadedOnce && !error && pullRequests.length > 0 && ( +
+ setIsPrsExpanded(!isPrsExpanded)} + isExpanded={isPrsExpanded} + > + {renderItemTable(pullRequests, updateItemInList(setPullRequests))} + +
+ )} ); diff --git a/frontend/src/components/MilestoneCard.test.jsx b/frontend/src/components/MilestoneCard.test.jsx index 7d49c48..9b6c86f 100644 --- a/frontend/src/components/MilestoneCard.test.jsx +++ b/frontend/src/components/MilestoneCard.test.jsx @@ -1,4 +1,5 @@ // Generated-by: Cursor +// Assisted-by: Cursor import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, waitFor, act } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; @@ -16,6 +17,31 @@ describe('MilestoneCard', () => { due_on: '2025-12-31T00:00:00Z', }; + const mockIssue = { + id: 1, + number: 459, + title: 'Test Issue', + body: 'Issue body', + html_url: 'https://github.com/test/issue/459', + user: { login: 'testuser', avatar_url: 'https://avatar.url' }, + created_at: '2025-01-01T00:00:00Z', + labels: [], + comments: 0, + }; + + const mockPr = { + id: 2, + number: 460, + title: 'Test PR', + body: 'PR body', + html_url: 'https://github.com/test/pull/460', + user: { login: 'testuser', avatar_url: 'https://avatar.url' }, + created_at: '2025-01-01T00:00:00Z', + labels: [], + comments: 0, + pull_request: {}, + }; + beforeEach(() => { vi.clearAllMocks(); api.fetchLabels.mockResolvedValue([]); @@ -64,20 +90,10 @@ describe('MilestoneCard', () => { it('expands and fetches issues when clicked', async () => { const user = userEvent.setup(); - const mockIssues = [ - { - id: 1, - number: 459, - title: 'Test Issue', - body: 'Issue body', - html_url: 'https://github.com/test/issue/459', - user: { login: 'testuser', avatar_url: 'https://avatar.url' }, - created_at: '2025-01-01T00:00:00Z', - labels: [], - comments: 0, - }, - ]; - api.fetchIssues.mockResolvedValue(mockIssues); + api.fetchIssues.mockResolvedValue({ + issues: [mockIssue], + pull_requests: [], + }); await act(async () => { render(); @@ -130,7 +146,7 @@ describe('MilestoneCard', () => { it('shows empty message when no issues', async () => { const user = userEvent.setup(); - api.fetchIssues.mockResolvedValue([]); + api.fetchIssues.mockResolvedValue({ issues: [], pull_requests: [] }); await act(async () => { render(); @@ -143,22 +159,75 @@ describe('MilestoneCard', () => { }); }); + it('does not show PR expander before issues are loaded', async () => { + await act(async () => { + render(); + }); + expect( + screen.queryByRole('button', { name: /show \d+ prs?/i }) + ).not.toBeInTheDocument(); + }); + + it('shows closed PR expander when pull requests are discovered', async () => { + const user = userEvent.setup(); + api.fetchIssues.mockResolvedValue({ + issues: [mockIssue], + pull_requests: [mockPr], + }); + + await act(async () => { + render(); + }); + + await user.click(screen.getByRole('button', { name: /show issues/i })); + + await waitFor(() => { + expect(screen.getByText(/Test Issue/)).toBeInTheDocument(); + }); + + const prToggle = screen.getByRole('button', { name: /show 1 pr/i }); + expect(prToggle).toBeInTheDocument(); + expect(prToggle).toHaveAttribute('aria-expanded', 'false'); + + await user.click(prToggle); + + await waitFor(() => { + expect(prToggle).toHaveAttribute('aria-expanded', 'true'); + expect(screen.getByText(/Test PR/)).toBeInTheDocument(); + }); + expect( + screen.getByRole('button', { name: /hide 1 pr/i }) + ).toBeInTheDocument(); + }); + + it('does not show PR expander when there are no pull requests', async () => { + const user = userEvent.setup(); + api.fetchIssues.mockResolvedValue({ + issues: [mockIssue], + pull_requests: [], + }); + + await act(async () => { + render(); + }); + + await user.click(screen.getByRole('button', { name: /show issues/i })); + + await waitFor(() => { + expect(screen.getByText(/Test Issue/)).toBeInTheDocument(); + }); + + expect( + screen.queryByRole('button', { name: /show \d+ prs?/i }) + ).not.toBeInTheDocument(); + }); + it('refetches issues when issueMilestoneRefresh targets this milestone', async () => { const user = userEvent.setup(); - const mockIssues = [ - { - id: 1, - number: 459, - title: 'Test Issue', - body: 'Issue body', - html_url: 'https://github.com/test/issue/459', - user: { login: 'testuser', avatar_url: 'https://avatar.url' }, - created_at: '2025-01-01T00:00:00Z', - labels: [], - comments: 0, - }, - ]; - api.fetchIssues.mockResolvedValue(mockIssues); + api.fetchIssues.mockResolvedValue({ + issues: [mockIssue], + pull_requests: [], + }); const onIssueMilestoneMoved = vi.fn(); const initialRefresh = { key: 0, milestoneNumbers: [] }; diff --git a/frontend/src/services/api.test.js b/frontend/src/services/api.test.js index d384dc1..613cc87 100644 --- a/frontend/src/services/api.test.js +++ b/frontend/src/services/api.test.js @@ -45,14 +45,17 @@ describe('api', () => { describe('fetchIssues', () => { it('fetches issues successfully', async () => { - const mockIssues = [{ id: 1, number: 459, title: 'Test Issue' }]; + const mockPayload = { + issues: [{ id: 1, number: 459, title: 'Test Issue' }], + pull_requests: [{ id: 2, number: 460, title: 'Test PR' }], + }; global.fetch.mockResolvedValue({ ok: true, - json: async () => mockIssues, + json: async () => mockPayload, }); const result = await fetchIssues(6); - expect(result).toEqual(mockIssues); + expect(result).toEqual(mockPayload); expect(global.fetch).toHaveBeenCalledWith('/api/v1/issues/6'); }); From 9bbd1a61dfcab3ce77c625d7901f7011224d55d1 Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Sat, 25 Jul 2026 09:00:46 -0400 Subject: [PATCH 02/11] Support targeted REFRESH Signed-off-by: David Butenhof --- frontend/src/App.jsx | 36 +++++++- frontend/src/App.test.jsx | 86 +++++++++++++++++++ frontend/src/components/IssueCard.jsx | 17 +++- frontend/src/components/IssueCard.test.jsx | 63 ++++++++++++++ frontend/src/components/MilestoneCard.jsx | 7 +- .../src/components/MilestoneCard.test.jsx | 38 ++++++++ 6 files changed, 241 insertions(+), 6 deletions(-) diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index d493296..159f354 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -1,4 +1,5 @@ // Generated-by: Cursor +// Assisted-by: Cursor import React, { useState, useEffect } from 'react'; import { Page, @@ -70,6 +71,16 @@ const App = () => { key: 0, milestoneNumbers: [], }); + // Milestone numbers whose issue/PR lists may be stale after local edits. + const [dirtyMilestoneNumbers, setDirtyMilestoneNumbers] = useState([]); + + const markMilestonesDirty = (numbers) => { + const normalized = numbers + .map((n) => (n == null ? 0 : n)) + .filter((n) => typeof n === 'number'); + if (normalized.length === 0) return; + setDirtyMilestoneNumbers((prev) => [...new Set([...prev, ...normalized])]); + }; useEffect(() => { fetchProject() @@ -227,13 +238,20 @@ const App = () => { fromMilestoneNumber, toMilestoneNumber, }) => { - const nums = [fromMilestoneNumber, toMilestoneNumber].filter( - (n) => n != null - ); + markMilestonesDirty([fromMilestoneNumber, toMilestoneNumber]); + }; + + const handleIssueLabelsChanged = ({ milestoneNumber }) => { + markMilestonesDirty([milestoneNumber]); + }; + + const handleRefreshDirtyMilestones = () => { + if (dirtyMilestoneNumbers.length === 0) return; setIssueMilestoneRefresh((s) => ({ key: s.key + 1, - milestoneNumbers: [...new Set(nums)], + milestoneNumbers: [...dirtyMilestoneNumbers], })); + setDirtyMilestoneNumbers([]); }; const handleLabelChange = () => { @@ -286,6 +304,15 @@ const App = () => {
+
diff --git a/frontend/src/App.test.jsx b/frontend/src/App.test.jsx index af0100e..7c009b8 100644 --- a/frontend/src/App.test.jsx +++ b/frontend/src/App.test.jsx @@ -343,4 +343,90 @@ describe('App', () => { ); }); }); + + it('disables Refresh until milestones are marked dirty, then refreshes on click', async () => { + const user = userEvent.setup(); + const mockMilestones = [ + { + number: 6, + title: 'v0.6.0', + description: 'Version 0.6.0', + due_on: null, + }, + { + number: 10, + title: 'Next release', + description: '', + due_on: null, + }, + ]; + const mockIssue = { + id: 1, + number: 459, + title: 'Test Issue', + body: 'Issue body', + html_url: 'https://github.com/test/issue/459', + user: { login: 'testuser', avatar_url: 'https://avatar.url' }, + created_at: '2025-01-01T00:00:00Z', + labels: [], + comments: 0, + milestone: { number: 6, title: 'v0.6.0' }, + assignees: [], + }; + + api.fetchMilestones.mockResolvedValue(mockMilestones); + api.fetchIssues.mockResolvedValue({ + issues: [mockIssue], + pull_requests: [], + }); + api.setIssueMilestone.mockResolvedValue({}); + + await act(async () => { + render(); + }); + + await waitFor(() => { + expect(screen.getAllByText('v0.6.0').length).toBeGreaterThan(0); + }); + + const refreshButton = screen.getByRole('button', { name: /^Refresh$/i }); + expect(refreshButton).toBeDisabled(); + + const showIssuesButtons = screen.getAllByRole('button', { + name: /show issues/i, + }); + await user.click(showIssuesButtons[0]); + await waitFor(() => { + expect(screen.getByText(/Test Issue/)).toBeInTheDocument(); + }); + expect(api.fetchIssues).toHaveBeenCalledTimes(1); + + await user.click(screen.getByRole('button', { name: 'v0.6.0' })); + const nextReleaseChoices = screen.getAllByText('Next release'); + // Menu item appears before the destination milestone card title in interaction order + await user.click(nextReleaseChoices[0]); + + await waitFor(() => { + expect(api.setIssueMilestone).toHaveBeenCalledWith(459, 10); + }); + + // Milestone moves mark dirty but do not auto-refetch + expect(api.fetchIssues).toHaveBeenCalledTimes(1); + + const dirtyRefresh = screen.getByRole('button', { + name: /Refresh \(2\)/i, + }); + expect(dirtyRefresh).toBeEnabled(); + + await user.click(dirtyRefresh); + + await waitFor(() => { + expect(api.fetchIssues).toHaveBeenCalledTimes(2); + }); + expect(api.fetchIssues).toHaveBeenLastCalledWith(6, []); + + await waitFor(() => { + expect(screen.getByRole('button', { name: /^Refresh$/i })).toBeDisabled(); + }); + }); }); diff --git a/frontend/src/components/IssueCard.jsx b/frontend/src/components/IssueCard.jsx index 07b2513..f973d66 100644 --- a/frontend/src/components/IssueCard.jsx +++ b/frontend/src/components/IssueCard.jsx @@ -1,4 +1,5 @@ // Generated-by: Cursor +// Assisted-by: Cursor import React, { useState, useEffect, useRef, useCallback } from 'react'; import { ExpandableSection, @@ -77,7 +78,12 @@ const getTypeContrastColor = (colorName) => { return darkColors.includes(colorName.toLowerCase()) ? '#ffffff' : '#000000'; }; -const IssueCard = ({ issue, onMilestoneChange, onIssueUpdate }) => { +const IssueCard = ({ + issue, + onMilestoneChange, + onLabelsChange, + onIssueUpdate, +}) => { const daysSince = getDaysSince(issue.created_at); const [isDescriptionExpanded, setIsDescriptionExpanded] = useState(false); const [isCommentsExpanded, setIsCommentsExpanded] = useState(false); @@ -525,12 +531,20 @@ const IssueCard = ({ issue, onMilestoneChange, onIssueUpdate }) => { handleApplyAssignees, ]); + const notifyLabelsChanged = () => { + if (!onLabelsChange) return; + const milestoneNumber = + currentMilestone?.number ?? issue.milestone?.number ?? 0; + onLabelsChange({ milestoneNumber }); + }; + const handleRemoveLabel = async (labelName) => { try { await removeLabel(issue.number, labelName); setCurrentLabels( currentLabels.filter((label) => label.name !== labelName) ); + notifyLabelsChanged(); } catch (err) { console.error('Failed to remove label:', err); setLabelsError(err.message); @@ -551,6 +565,7 @@ const IssueCard = ({ issue, onMilestoneChange, onIssueUpdate }) => { currentLabels.filter((label) => label.name !== labelName) ); } + notifyLabelsChanged(); } catch (err) { console.error('Failed to toggle label:', err); setLabelsError(err.message); diff --git a/frontend/src/components/IssueCard.test.jsx b/frontend/src/components/IssueCard.test.jsx index d9743dc..791c5cc 100644 --- a/frontend/src/components/IssueCard.test.jsx +++ b/frontend/src/components/IssueCard.test.jsx @@ -417,4 +417,67 @@ describe('IssueCard', () => { }); }); }); + + it('calls onLabelsChange when a label is removed', async () => { + const user = userEvent.setup(); + const onLabelsChange = vi.fn(); + api.removeLabel.mockResolvedValue({}); + const issueWithMilestone = { + ...mockIssue, + milestone: { number: 6, title: 'Current' }, + }; + + await act(async () => { + render( + + + + +
+ ); + }); + + await user.click( + screen.getByRole('button', { name: 'Remove enhancement label' }) + ); + + await waitFor(() => { + expect(api.removeLabel).toHaveBeenCalledWith(459, 'enhancement'); + expect(onLabelsChange).toHaveBeenCalledWith({ milestoneNumber: 6 }); + }); + }); + + it('calls onLabelsChange with milestone 0 when issue has no milestone', async () => { + const user = userEvent.setup(); + const onLabelsChange = vi.fn(); + api.removeLabel.mockResolvedValue({}); + const issueWithoutMilestone = { + ...mockIssue, + milestone: null, + }; + + await act(async () => { + render( + + + + +
+ ); + }); + + await user.click( + screen.getByRole('button', { name: 'Remove enhancement label' }) + ); + + await waitFor(() => { + expect(onLabelsChange).toHaveBeenCalledWith({ milestoneNumber: 0 }); + }); + }); }); diff --git a/frontend/src/components/MilestoneCard.jsx b/frontend/src/components/MilestoneCard.jsx index 4678512..52ff5f0 100644 --- a/frontend/src/components/MilestoneCard.jsx +++ b/frontend/src/components/MilestoneCard.jsx @@ -108,6 +108,7 @@ const MilestoneCard = ({ sortOrder = [], issueMilestoneRefresh = { key: 0, milestoneNumbers: [] }, onIssueMilestoneMoved, + onIssueLabelsChanged, }) => { const [isIssuesExpanded, setIsIssuesExpanded] = useState(false); const [isPrsExpanded, setIsPrsExpanded] = useState(false); @@ -194,7 +195,8 @@ const MilestoneCard = ({ const { key, milestoneNumbers } = issueMilestoneRefresh; if (key === 0) return; if (!milestoneNumbers.includes(milestone.number)) return; - if (!isIssuesExpanded || !hasLoadedOnce) return; + // Refresh even when collapsed so expanding later shows fresh data. + if (!hasLoadedOnce) return; refetchIssues(); // Bump `key` and `milestoneNumbers` update together; refetch only when key changes. // eslint-disable-next-line react-hooks/exhaustive-deps @@ -253,6 +255,9 @@ const MilestoneCard = ({ onMilestoneChange={(detail) => { onIssueMilestoneMoved?.(detail); }} + onLabelsChange={(detail) => { + onIssueLabelsChanged?.(detail); + }} onIssueUpdate={onItemUpdate} /> ))} diff --git a/frontend/src/components/MilestoneCard.test.jsx b/frontend/src/components/MilestoneCard.test.jsx index 9b6c86f..a5c11f2 100644 --- a/frontend/src/components/MilestoneCard.test.jsx +++ b/frontend/src/components/MilestoneCard.test.jsx @@ -264,4 +264,42 @@ describe('MilestoneCard', () => { }); expect(api.fetchIssues).toHaveBeenLastCalledWith(6, []); }); + + it('refetches loaded milestones even when issues section is collapsed', async () => { + const user = userEvent.setup(); + api.fetchIssues.mockResolvedValue({ + issues: [mockIssue], + pull_requests: [], + }); + + const { rerender } = await act(async () => + render( + + ) + ); + + await user.click(screen.getByRole('button', { name: /show issues/i })); + await waitFor(() => { + expect(api.fetchIssues).toHaveBeenCalledTimes(1); + }); + + // Collapse issues section + await user.click(screen.getByRole('button', { name: /hide 1 issue/i })); + + await act(async () => { + rerender( + + ); + }); + + await waitFor(() => { + expect(api.fetchIssues).toHaveBeenCalledTimes(2); + }); + }); }); From 99b76319794cd0367c517af5ebd47bfeea3f5624 Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Sat, 25 Jul 2026 09:06:22 -0400 Subject: [PATCH 03/11] Don't scroll UI header Generated-by: Cursor Signed-off-by: David Butenhof --- frontend/src/App.jsx | 115 ++++++++++++++++++++------------------ frontend/src/App.test.jsx | 25 +++++++++ frontend/src/icon.css | 14 +++++ 3 files changed, 99 insertions(+), 55 deletions(-) diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 159f354..4e366a6 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -259,19 +259,64 @@ const App = () => { // but we can add a callback if needed in the future }; + const renderPlanningContent = () => ( + <> + {loading && ( + + + + )} + + {error && ( + + {error} + + )} + + {!loading && !error && ( +
+ {milestones.length === 0 && ( + + There are no milestones available. + + )} + {milestones.length > 0 && + milestones.map((milestone) => ( + + ))} +
+ )} + + ); + return ( - +
{ display: 'flex', alignItems: 'center', gap: '1rem', - background: 'transparent', }} > { style={{ height: '100px', width: 'auto', - background: 'transparent', - backgroundColor: 'transparent', display: 'block', }} /> @@ -337,63 +379,26 @@ const App = () => { activeKey={activeViewTab} onSelect={(_event, key) => setActiveViewTab(key)} aria-label="Main views" - mountOnEnter - style={{ marginTop: '1rem' }} > Planning} - > - {loading && ( - - - - )} - - {error && ( - - {error} - - )} - - {!loading && !error && ( -
- {milestones.length === 0 && ( - - There are no milestones available. - - )} - {milestones.length > 0 && - milestones.map((milestone) => ( - - ))} -
- )} - - SDLC}> - - + /> + SDLC} /> Project status} - > - - + /> + + {activeViewTab === 'planning' && renderPlanningContent()} + {activeViewTab === 'sdlc' && } + {activeViewTab === 'project-status' && } + setIsManageMilestonesOpen(false)} diff --git a/frontend/src/App.test.jsx b/frontend/src/App.test.jsx index 7c009b8..cf8e460 100644 --- a/frontend/src/App.test.jsx +++ b/frontend/src/App.test.jsx @@ -429,4 +429,29 @@ describe('App', () => { expect(screen.getByRole('button', { name: /^Refresh$/i })).toBeDisabled(); }); }); + + it('keeps the header chrome sticky while view content scrolls independently', async () => { + api.fetchMilestones.mockResolvedValue([]); + + await act(async () => { + render(); + }); + + await waitFor(() => { + expect( + screen.getByRole('button', { name: /^Refresh$/i }) + ).toBeInTheDocument(); + }); + + const chrome = document.querySelector('.app-page-chrome'); + expect(chrome).toBeTruthy(); + expect(chrome.className).toMatch(/pf-m-sticky-top/); + expect(chrome).toContainElement( + screen.getByRole('button', { name: /^Refresh$/i }) + ); + expect(chrome).toContainElement( + screen.getByRole('tab', { name: /^Planning$/i }) + ); + expect(document.querySelector('.app-page-content')).toBeTruthy(); + }); }); diff --git a/frontend/src/icon.css b/frontend/src/icon.css index d1c8857..42d4443 100644 --- a/frontend/src/icon.css +++ b/frontend/src/icon.css @@ -1,4 +1,18 @@ /* Generated-by: Cursor */ +/* Assisted-by: Cursor */ + +/* Keep app chrome (title + actions + tabs) visible while content scrolls */ +.app-page-chrome.pf-m-sticky-top { + z-index: 300; + background-color: var(--pf-v5-global--BackgroundColor--100) !important; +} + +.app-header-icon-container, +.app-icon { + background: transparent !important; + background-color: transparent !important; +} + /* Override PatternFly styles to ensure transparent background for icon */ html, body, From 76f3cf7c68888b863bb36add0a97aed043750f33 Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Sat, 25 Jul 2026 11:18:13 -0400 Subject: [PATCH 04/11] Support EPIC -> STORY hierarchy Signed-off-by: David Butenhof --- backend/src/github_pm/api.py | 216 +++++++++++-- backend/src/github_pm/issue_hierarchy.py | 241 ++++++++++++++ backend/tests/test_api.py | 245 ++++++++++++++ backend/tests/test_issue_hierarchy.py | 160 +++++++++ frontend/src/App.jsx | 40 ++- frontend/src/components/IssueCard.jsx | 146 ++++++++- frontend/src/components/IssueCard.test.jsx | 98 ++++++ frontend/src/components/MilestoneCard.jsx | 303 +++++++++++------- .../src/components/MilestoneCard.test.jsx | 44 +++ frontend/src/components/PlanningBoard.jsx | 55 ++++ .../src/components/PlanningDnDContext.jsx | 238 ++++++++++++++ .../components/PlanningDnDContext.test.jsx | 149 +++++++++ frontend/src/services/api.js | 37 +++ frontend/src/services/api.test.js | 41 +++ frontend/src/utils/issueHierarchy.js | 109 +++++++ frontend/src/utils/issueHierarchy.test.js | 63 ++++ 16 files changed, 2033 insertions(+), 152 deletions(-) create mode 100644 backend/src/github_pm/issue_hierarchy.py create mode 100644 backend/tests/test_issue_hierarchy.py create mode 100644 frontend/src/components/PlanningBoard.jsx create mode 100644 frontend/src/components/PlanningDnDContext.jsx create mode 100644 frontend/src/components/PlanningDnDContext.test.jsx create mode 100644 frontend/src/utils/issueHierarchy.js create mode 100644 frontend/src/utils/issueHierarchy.test.js diff --git a/backend/src/github_pm/api.py b/backend/src/github_pm/api.py index ae58536..90e3a70 100644 --- a/backend/src/github_pm/api.py +++ b/backend/src/github_pm/api.py @@ -11,6 +11,14 @@ import requests from github_pm.context import context +from github_pm.issue_hierarchy import ( + apply_graphql_hierarchy, + build_issue_forest, + collect_descendant_numbers, + is_ancestor, + ISSUE_HIERARCHY_GRAPHQL, + PARENT_ONLY_GRAPHQL, +) from github_pm.logger import logger api_router = APIRouter() @@ -213,9 +221,11 @@ async def get_issues( str | None, Query(title="Sort", description="List of labels to sort by") ] = None, ): - """Return open issues and pull requests for a milestone as separate lists. + """Return open issues (as a sub-issue forest) and PRs for a milestone. - Each list is sorted independently by the optional label sort criteria. + Issues are nested via GitHub parent/sub-issue links discovered over GraphQL. + Pull requests remain a flat list. Each sibling list is sorted by optional + label criteria. Assisted-by: Cursor """ if sort: @@ -234,25 +244,11 @@ async def get_issues( if "pull_request" in i: pull_requests.append(i) continue - 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, @@ -262,7 +258,8 @@ async def get_issues( ) data = response["data"] issue_node = data["repository"]["issue"] - closed = issue_node["closedByPullRequestsReferences"]["nodes"] + closed_refs = issue_node.get("closedByPullRequestsReferences") or {} + closed = closed_refs.get("nodes") or [] if len(closed) > 0: i["closed_by"] = [ { @@ -272,20 +269,23 @@ async def get_issues( } for linked in closed ] + apply_graphql_hierarchy(i, issue_node) except Exception as e: - logger.exception(f"Error finding linked PRs for issue {i['number']}: {e!r}") + logger.exception( + f"Error enriching hierarchy/PRs for issue {i['number']}: {e!r}" + ) continue issues.append(i) - sorted_issues = _sort_items_by_labels(issues, sort_by) + forest = build_issue_forest(issues, sort_by, _sort_items_by_labels) sorted_prs = _sort_items_by_labels(pull_requests, sort_by) logger.debug( "%s(%s issues, %s PRs) items: %.3f seconds", len(raw_items), - len(sorted_issues), + len(forest), len(sorted_prs), time.time() - start, ) - return {"issues": sorted_issues, "pull_requests": sorted_prs} + return {"issues": forest, "pull_requests": sorted_prs} @api_router.get("/issue/{issue_number}") @@ -477,6 +477,178 @@ async def remove_milestone_from_issue( return issue +def _list_sub_issue_numbers(gitctx: Connector, issue_number: int) -> list[int]: + """List child issue numbers via REST; fall back to empty on failure. + + Generated-by: Cursor + """ + try: + items = gitctx.get_paged( + f"/repos/{context.github_repo}/issues/{issue_number}/sub_issues" + ) + return [item["number"] for item in items if "number" in item] + except Exception as e: + logger.exception("Failed listing sub-issues for #%s: %r", issue_number, e) + return [] + + +def _cascade_milestone( + gitctx: Connector, + root_number: int, + milestone_number: int | None, +) -> list[int]: + """Set milestone on root and all GitHub descendants. Returns updated numbers. + + Generated-by: Cursor + """ + + def list_children(n: int) -> list[int]: + return _list_sub_issue_numbers(gitctx, n) + + numbers = [root_number] + collect_descendant_numbers(list_children, root_number) + updated: list[int] = [] + for number in numbers: + gitctx.patch( + f"/repos/{context.github_repo}/issues/{number}", + data={"milestone": milestone_number}, + ) + updated.append(number) + return updated + + +def _graphql_issue_node(gitctx: Connector, issue_number: int) -> dict: + response = gitctx.post( + "/graphql", + data={ + "query": PARENT_ONLY_GRAPHQL, + "variables": { + "owner": gitctx.owner, + "repo": gitctx.repo, + "issue": issue_number, + }, + }, + ) + data = response.get("data") or {} + repo = data.get("repository") or {} + issue_node = repo.get("issue") + if not issue_node: + raise HTTPException(status_code=404, detail=f"Issue #{issue_number} not found") + return issue_node + + +class SetIssueParent(BaseModel): + parent_number: int = Field(title="Parent Issue Number") + + +@api_router.put("/issues/{issue_number}/parent") +async def set_issue_parent( + gitctx: Annotated[Connector, Depends(connection)], + issue_number: Annotated[int, Path(title="Issue")], + body: Annotated[SetIssueParent, Body(title="Parent")], +): + """Link issue as a sub-issue of parent, cascading milestone when needed. + + Generated-by: Cursor + """ + parent_number = body.parent_number + if parent_number == issue_number: + raise HTTPException(status_code=422, detail="An issue cannot be its own parent") + + def list_children(n: int) -> list[int]: + return _list_sub_issue_numbers(gitctx, n) + + if is_ancestor(list_children, issue_number, parent_number): + raise HTTPException( + status_code=422, + detail="Cannot link an issue under one of its descendants", + ) + + child = gitctx.get(f"/repos/{context.github_repo}/issues/{issue_number}") + parent = gitctx.get(f"/repos/{context.github_repo}/issues/{parent_number}") + + gitctx.post( + f"/repos/{context.github_repo}/issues/{parent_number}/sub_issues", + data={"sub_issue_id": child["id"], "replace_parent": True}, + ) + + child_ms = (child.get("milestone") or {}).get("number") + parent_ms = (parent.get("milestone") or {}).get("number") + updated_issue_numbers: list[int] = [] + if child_ms != parent_ms: + # parent_ms may be None (no milestone); still cascade to match parent. + updated_issue_numbers = _cascade_milestone(gitctx, issue_number, parent_ms) + + return { + "issue_number": issue_number, + "parent_number": parent_number, + "from_milestone": child_ms, + "to_milestone": parent_ms, + "updated_issue_numbers": updated_issue_numbers, + } + + +@api_router.delete("/issues/{issue_number}/parent") +async def clear_issue_parent( + gitctx: Annotated[Connector, Depends(connection)], + issue_number: Annotated[int, Path(title="Issue")], +): + """Unlink issue from its current parent (becomes an epic). + + Generated-by: Cursor + """ + child = gitctx.get(f"/repos/{context.github_repo}/issues/{issue_number}") + issue_node = _graphql_issue_node(gitctx, issue_number) + parent = issue_node.get("parent") + if not parent or parent.get("number") is None: + raise HTTPException( + status_code=404, detail=f"Issue #{issue_number} has no parent" + ) + parent_number = parent["number"] + gitctx.delete( + f"/repos/{context.github_repo}/issues/{parent_number}/sub_issue", + data={"sub_issue_id": child["id"]}, + ) + return { + "issue_number": issue_number, + "parent_number": parent_number, + "message": "parent cleared", + } + + +@api_router.post("/issues/{issue_number}/adopt-parent-milestone") +async def adopt_parent_milestone( + gitctx: Annotated[Connector, Depends(connection)], + issue_number: Annotated[int, Path(title="Issue")], +): + """Move this issue and descendants to its GitHub parent's milestone. + + Generated-by: Cursor + """ + issue = gitctx.get(f"/repos/{context.github_repo}/issues/{issue_number}") + issue_node = _graphql_issue_node(gitctx, issue_number) + parent = issue_node.get("parent") + if not parent or parent.get("number") is None: + raise HTTPException( + status_code=404, detail=f"Issue #{issue_number} has no parent" + ) + parent_ms_info = parent.get("milestone") + if not parent_ms_info or parent_ms_info.get("number") is None: + raise HTTPException( + status_code=422, + detail=f"Parent #{parent['number']} has no milestone to adopt", + ) + to_milestone = parent_ms_info["number"] + from_milestone = (issue.get("milestone") or {}).get("number") + updated = _cascade_milestone(gitctx, issue_number, to_milestone) + return { + "issue_number": issue_number, + "parent_number": parent["number"], + "from_milestone": from_milestone, + "to_milestone": to_milestone, + "updated_issue_numbers": updated, + } + + # """Label Management""" diff --git a/backend/src/github_pm/issue_hierarchy.py b/backend/src/github_pm/issue_hierarchy.py new file mode 100644 index 0000000..14096f6 --- /dev/null +++ b/backend/src/github_pm/issue_hierarchy.py @@ -0,0 +1,241 @@ +"""Build and mutate GitHub sub-issue hierarchies for the planner. + +Generated-by: Cursor +""" + +from __future__ import annotations + +from typing import Any, Callable + +from github_pm.logger import logger + +SortFn = Callable[[list[dict], list[str]], list[dict]] + + +def _external_parent_payload(parent: dict[str, Any] | None) -> dict[str, Any] | None: + """Normalize a GraphQL parent node into the API's external_parent shape.""" + if not parent or parent.get("number") is None: + return None + milestone = parent.get("milestone") + return { + "number": parent["number"], + "title": parent.get("title"), + "milestone": ( + { + "number": milestone.get("number"), + "title": milestone.get("title"), + } + if isinstance(milestone, dict) + else None + ), + } + + +def _would_create_cycle( + child_number: int, + parent_number: int, + parent_of: dict[int, int], +) -> bool: + """Return True if linking child under parent would create a cycle.""" + if child_number == parent_number: + return True + current: int | None = parent_number + seen: set[int] = set() + while current is not None: + if current == child_number: + return True + if current in seen: + break + seen.add(current) + current = parent_of.get(current) + return False + + +def build_issue_forest( + issues: list[dict], + sort_by: list[str], + sort_fn: SortFn, +) -> list[dict]: + """Nest open milestone issues into a forest using GraphQL parent links. + + Issues whose parent is outside this open set become display roots with + ``external_parent`` set. Cyclic edges are broken by treating them as + external/orphan roots. + + Assisted-by: Cursor + """ + by_number: dict[int, dict] = {} + for issue in issues: + node = dict(issue) + node["children"] = [] + node["parent_number"] = None + node["external_parent"] = None + node["hierarchy_depth"] = 0 + node["child_count"] = 0 + by_number[node["number"]] = node + + # Proposed in-tree edges: child -> parent when parent is in this set. + parent_of: dict[int, int] = {} + external: dict[int, dict[str, Any]] = {} + + for number, node in by_number.items(): + parent_info = node.get("_parent_info") + if not parent_info or parent_info.get("number") is None: + continue + parent_number = parent_info["number"] + if parent_number in by_number: + parent_of[number] = parent_number + else: + external[number] = _external_parent_payload(parent_info) + + # Drop cyclic edges. + for child, parent in list(parent_of.items()): + if _would_create_cycle( + child, parent, {k: v for k, v in parent_of.items() if k != child} + ): + logger.warning("Breaking cyclic sub-issue edge #%s -> #%s", child, parent) + parent_info = by_number[child].get("_parent_info") + external[child] = _external_parent_payload(parent_info) or { + "number": parent, + "title": None, + "milestone": None, + } + del parent_of[child] + + children_map: dict[int, list[int]] = {n: [] for n in by_number} + for child, parent in parent_of.items(): + children_map[parent].append(child) + by_number[child]["parent_number"] = parent + + for number, payload in external.items(): + by_number[number]["external_parent"] = payload + + def attach(number: int, depth: int) -> dict: + node = by_number[number] + node["hierarchy_depth"] = depth + child_nodes = [attach(c, depth + 1) for c in children_map[number]] + node["children"] = sort_fn(child_nodes, sort_by) + node["child_count"] = len(node["children"]) + # Strip internal enrichment key before returning. + node.pop("_parent_info", None) + return node + + roots = [number for number in by_number if number not in parent_of] + forest = [attach(n, 0) for n in roots] + return sort_fn(forest, sort_by) + + +def apply_graphql_hierarchy(issue: dict, issue_node: dict | None) -> None: + """Attach parent / sub-issue summary fields from a GraphQL issue node. + + Generated-by: Cursor + """ + if not issue_node: + return + parent = issue_node.get("parent") + if parent: + issue["_parent_info"] = { + "number": parent.get("number"), + "title": parent.get("title"), + "milestone": parent.get("milestone"), + } + summary = issue_node.get("subIssuesSummary") + if summary: + issue["sub_issues_summary"] = { + "total": summary.get("total", 0), + "completed": summary.get("completed", 0), + "percent_completed": summary.get("percentCompleted", 0), + } + + +ISSUE_HIERARCHY_GRAPHQL = """ +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 + } + } + parent { + number + title + milestone { + number + title + } + } + subIssues(first: 100) { + nodes { + number + } + } + subIssuesSummary { + total + completed + percentCompleted + } + } + } +} +""" + +PARENT_ONLY_GRAPHQL = """ +query($owner: String!, $repo: String!, $issue: Int!) { + repository(owner: $owner, name: $repo, followRenames: true) { + issue(number: $issue) { + id + number + parent { + number + title + milestone { + number + title + } + } + subIssues(first: 100) { + nodes { + number + } + } + } + } +} +""" + + +def collect_descendant_numbers( + list_sub_issues: Callable[[int], list[int]], + root_number: int, +) -> list[int]: + """BFS over GitHub sub-issues starting at root (excluding root itself). + + Generated-by: Cursor + """ + descendants: list[int] = [] + queue = list(list_sub_issues(root_number)) + seen: set[int] = {root_number} + while queue: + number = queue.pop(0) + if number in seen: + continue + seen.add(number) + descendants.append(number) + queue.extend(list_sub_issues(number)) + return descendants + + +def is_ancestor( + list_sub_issues: Callable[[int], list[int]], + ancestor_number: int, + descendant_number: int, +) -> bool: + """True if ancestor_number is an ancestor of descendant_number via sub-issues.""" + if ancestor_number == descendant_number: + return True + return descendant_number in collect_descendant_numbers( + list_sub_issues, ancestor_number + ) diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 00772fd..af7629a 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -13,7 +13,9 @@ from github_pm.api import ( add_label_to_issue, add_milestone_to_issue, + adopt_parent_milestone, api_router, + clear_issue_parent, connection, Connector, create_label, @@ -31,6 +33,8 @@ get_project, remove_label_from_issue, remove_milestone_from_issue, + set_issue_parent, + SetIssueParent, ) from github_pm.app import app @@ -210,11 +214,66 @@ async def test_get_issues_with_milestone(self): assert len(result["pull_requests"]) == 1 assert result["pull_requests"][0]["id"] == 1 assert result["issues"][0]["id"] == 2 + assert result["issues"][0]["hierarchy_depth"] == 0 + assert result["issues"][0]["children"] == [] + assert result["issues"][0]["child_count"] == 0 mock_gitctx.get_paged.assert_called_once() # Issue 1 has pull_request, so no GraphQL call # Issue 2 doesn't have pull_request, so GraphQL should be called assert mock_gitctx.post.call_count == 1 + @pytest.mark.asyncio + async def test_get_issues_builds_hierarchy(self): + """Test nesting when GraphQL reports an in-milestone parent.""" + mock_issues = [ + {"id": 10, "number": 10, "title": "Epic", "labels": []}, + {"id": 20, "number": 20, "title": "Story", "labels": []}, + ] + + def graphql_side_effect(*_args, **kwargs): + issue_num = kwargs["data"]["variables"]["issue"] + parent = None + if issue_num == 20: + parent = { + "number": 10, + "title": "Epic", + "milestone": {"number": 1, "title": "M1"}, + } + return { + "data": { + "repository": { + "issue": { + "closedByPullRequestsReferences": {"nodes": []}, + "parent": parent, + "subIssues": {"nodes": []}, + "subIssuesSummary": { + "total": 0, + "completed": 0, + "percentCompleted": 0, + }, + } + } + } + } + + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get_paged = Mock(return_value=mock_issues) + mock_gitctx.post = Mock(side_effect=graphql_side_effect) + 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 + epic = result["issues"][0] + assert epic["number"] == 10 + assert epic["child_count"] == 1 + assert epic["children"][0]["number"] == 20 + assert epic["children"][0]["hierarchy_depth"] == 1 + assert epic["children"][0]["parent_number"] == 10 + @pytest.mark.asyncio async def test_get_issues_with_linked_prs(self): """Test getting issues with linked PRs from GraphQL.""" @@ -907,6 +966,192 @@ async def test_remove_milestone_from_issue(self): ) +class TestSetIssueParent: + """Test PUT /issues/{n}/parent.""" + + @pytest.mark.asyncio + async def test_set_parent_same_milestone(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock( + side_effect=[ + {"id": 200, "number": 20, "milestone": {"number": 1}}, + {"id": 100, "number": 10, "milestone": {"number": 1}}, + ] + ) + mock_gitctx.get_paged = Mock(return_value=[]) + mock_gitctx.post = Mock(return_value={}) + mock_gitctx.patch = Mock() + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await set_issue_parent( + mock_gitctx, 20, SetIssueParent(parent_number=10) + ) + + assert result["parent_number"] == 10 + assert result["updated_issue_numbers"] == [] + mock_gitctx.post.assert_called_once_with( + "/repos/test/repo/issues/10/sub_issues", + data={"sub_issue_id": 200, "replace_parent": True}, + ) + mock_gitctx.patch.assert_not_called() + + @pytest.mark.asyncio + async def test_set_parent_cascades_milestone(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock( + side_effect=[ + {"id": 200, "number": 20, "milestone": {"number": 1}}, + {"id": 100, "number": 10, "milestone": {"number": 2}}, + ] + ) + mock_gitctx.get_paged = Mock( + side_effect=[ + [], # cycle check: children of 20 + [{"number": 30}], # cascade: children of 20 + [], # cascade: children of 30 + ] + ) + mock_gitctx.post = Mock(return_value={}) + mock_gitctx.patch = Mock(return_value={}) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await set_issue_parent( + mock_gitctx, 20, SetIssueParent(parent_number=10) + ) + + assert result["from_milestone"] == 1 + assert result["to_milestone"] == 2 + assert result["updated_issue_numbers"] == [20, 30] + assert mock_gitctx.patch.call_count == 2 + + @pytest.mark.asyncio + async def test_set_parent_rejects_self(self): + mock_gitctx = Mock(spec=Connector) + with pytest.raises(HTTPException) as exc: + await set_issue_parent(mock_gitctx, 5, SetIssueParent(parent_number=5)) + assert exc.value.status_code == 422 + + @pytest.mark.asyncio + async def test_set_parent_rejects_descendant(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get_paged = Mock(return_value=[{"number": 10}]) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + with pytest.raises(HTTPException) as exc: + await set_issue_parent( + mock_gitctx, 20, SetIssueParent(parent_number=10) + ) + assert exc.value.status_code == 422 + + +class TestClearIssueParent: + """Test DELETE /issues/{n}/parent.""" + + @pytest.mark.asyncio + async def test_clear_parent(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock(return_value={"id": 200, "number": 20}) + mock_gitctx.post = Mock( + return_value={ + "data": { + "repository": { + "issue": { + "number": 20, + "parent": {"number": 10, "title": "Epic"}, + "subIssues": {"nodes": []}, + } + } + } + } + ) + mock_gitctx.delete = Mock(return_value={}) + 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 clear_issue_parent(mock_gitctx, 20) + + assert result["parent_number"] == 10 + mock_gitctx.delete.assert_called_once_with( + "/repos/test/repo/issues/10/sub_issue", + data={"sub_issue_id": 200}, + ) + + @pytest.mark.asyncio + async def test_clear_parent_missing(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock(return_value={"id": 200, "number": 20}) + mock_gitctx.post = Mock( + return_value={ + "data": { + "repository": { + "issue": { + "number": 20, + "parent": None, + "subIssues": {"nodes": []}, + } + } + } + } + ) + mock_gitctx.owner = "test" + mock_gitctx.repo = "repo" + + with pytest.raises(HTTPException) as exc: + await clear_issue_parent(mock_gitctx, 20) + assert exc.value.status_code == 404 + + +class TestAdoptParentMilestone: + """Test POST adopt-parent-milestone.""" + + @pytest.mark.asyncio + async def test_adopt_cascades(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.get = Mock( + return_value={"id": 200, "number": 20, "milestone": {"number": 1}} + ) + mock_gitctx.post = Mock( + return_value={ + "data": { + "repository": { + "issue": { + "number": 20, + "parent": { + "number": 10, + "title": "Epic", + "milestone": {"number": 2, "title": "M2"}, + }, + "subIssues": {"nodes": [{"number": 30}]}, + } + } + } + } + ) + mock_gitctx.get_paged = Mock( + side_effect=[ + [{"number": 30}], + [], + ] + ) + mock_gitctx.patch = Mock(return_value={}) + 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 adopt_parent_milestone(mock_gitctx, 20) + + assert result["from_milestone"] == 1 + assert result["to_milestone"] == 2 + assert result["updated_issue_numbers"] == [20, 30] + assert mock_gitctx.patch.call_count == 2 + + class TestGetLabels: """Test the get_labels endpoint.""" diff --git a/backend/tests/test_issue_hierarchy.py b/backend/tests/test_issue_hierarchy.py new file mode 100644 index 0000000..1de35b6 --- /dev/null +++ b/backend/tests/test_issue_hierarchy.py @@ -0,0 +1,160 @@ +"""Tests for issue hierarchy forest building. + +Generated-by: Cursor +""" + +from github_pm.api import _sort_items_by_labels +from github_pm.issue_hierarchy import ( + apply_graphql_hierarchy, + build_issue_forest, + collect_descendant_numbers, + is_ancestor, +) + + +def _issue(number: int, labels=None, parent=None, **extra): + item = { + "id": number * 10, + "number": number, + "title": f"Issue {number}", + "labels": labels or [], + **extra, + } + if parent is not None: + item["_parent_info"] = parent + return item + + +class TestBuildIssueForest: + def test_flat_peers_are_epics(self): + issues = [_issue(1), _issue(2), _issue(3)] + forest = build_issue_forest(issues, [], _sort_items_by_labels) + assert [n["number"] for n in forest] == [1, 2, 3] + for node in forest: + assert node["hierarchy_depth"] == 0 + assert node["parent_number"] is None + assert node["external_parent"] is None + assert node["children"] == [] + assert node["child_count"] == 0 + + def test_nests_when_parent_in_set(self): + issues = [ + _issue(1), + _issue(2, parent={"number": 1, "title": "Epic", "milestone": None}), + _issue(3, parent={"number": 2, "title": "Story", "milestone": None}), + ] + forest = build_issue_forest(issues, [], _sort_items_by_labels) + assert len(forest) == 1 + epic = forest[0] + assert epic["number"] == 1 + assert epic["hierarchy_depth"] == 0 + assert epic["child_count"] == 1 + story = epic["children"][0] + assert story["number"] == 2 + assert story["hierarchy_depth"] == 1 + assert story["parent_number"] == 1 + substory = story["children"][0] + assert substory["number"] == 3 + assert substory["hierarchy_depth"] == 2 + assert substory["parent_number"] == 2 + + def test_external_parent_becomes_root(self): + issues = [ + _issue( + 5, + parent={ + "number": 99, + "title": "Other Epic", + "milestone": {"number": 7, "title": "v1"}, + }, + ) + ] + forest = build_issue_forest(issues, [], _sort_items_by_labels) + assert len(forest) == 1 + node = forest[0] + assert node["number"] == 5 + assert node["hierarchy_depth"] == 0 + assert node["parent_number"] is None + assert node["external_parent"] == { + "number": 99, + "title": "Other Epic", + "milestone": {"number": 7, "title": "v1"}, + } + + def test_breaks_cycles(self): + issues = [ + _issue(1, parent={"number": 2, "title": "B", "milestone": None}), + _issue(2, parent={"number": 1, "title": "A", "milestone": None}), + ] + forest = build_issue_forest(issues, [], _sort_items_by_labels) + # One edge breaks; both should appear as roots (or one nested). + numbers = {n["number"] for n in forest} + assert numbers == {1, 2} or (len(forest) == 1 and forest[0]["child_count"] == 1) + # No infinite nesting: depths finite and children don't loop. + for root in forest: + assert root["hierarchy_depth"] == 0 + + def test_label_sort_within_siblings(self): + issues = [ + _issue(1), + _issue( + 3, + labels=[{"name": "bug"}], + parent={"number": 1, "title": "E", "milestone": None}, + ), + _issue( + 2, + labels=[{"name": "feature"}], + parent={"number": 1, "title": "E", "milestone": None}, + ), + _issue(4, labels=[{"name": "bug"}]), + _issue(5, labels=[{"name": "feature"}]), + ] + forest = build_issue_forest(issues, ["bug", "feature"], _sort_items_by_labels) + # Roots: 4 (bug), 5 (feature), 1 (other, with children) + assert [n["number"] for n in forest] == [4, 5, 1] + children = forest[2]["children"] + assert [c["number"] for c in children] == [3, 2] + + def test_strips_internal_parent_info(self): + issues = [ + _issue(1, parent={"number": 99, "title": "X", "milestone": None}), + ] + forest = build_issue_forest(issues, [], _sort_items_by_labels) + assert "_parent_info" not in forest[0] + + +class TestApplyGraphqlHierarchy: + def test_applies_parent_and_summary(self): + issue = {"number": 1} + apply_graphql_hierarchy( + issue, + { + "parent": { + "number": 2, + "title": "P", + "milestone": {"number": 3, "title": "M"}, + }, + "subIssuesSummary": { + "total": 4, + "completed": 1, + "percentCompleted": 25, + }, + }, + ) + assert issue["_parent_info"]["number"] == 2 + assert issue["sub_issues_summary"]["total"] == 4 + assert issue["sub_issues_summary"]["percent_completed"] == 25 + + +class TestCollectDescendants: + def test_bfs_descendants(self): + tree = {1: [2, 3], 2: [4], 3: [], 4: []} + + def list_children(n): + return tree.get(n, []) + + assert collect_descendant_numbers(list_children, 1) == [2, 3, 4] + assert is_ancestor(list_children, 1, 4) is True + assert is_ancestor(list_children, 2, 3) is False + assert is_ancestor(list_children, 1, 1) is True diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 4e366a6..3a03d4b 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -20,6 +20,8 @@ import { fetchAssignees, } from './services/api'; import MilestoneCard from './components/MilestoneCard'; +import PlanningBoard from './components/PlanningBoard'; +import { PlanningDnDProvider } from './components/PlanningDnDContext'; import SdlcKpisPanel from './components/SdlcKpisPanel'; import ProjectStatusPanel from './components/ProjectStatusPanel'; import ManageMilestones from './components/ManageMilestones'; @@ -73,6 +75,7 @@ const App = () => { }); // Milestone numbers whose issue/PR lists may be stale after local edits. const [dirtyMilestoneNumbers, setDirtyMilestoneNumbers] = useState([]); + const [hierarchyAction, setHierarchyAction] = useState(null); const markMilestonesDirty = (numbers) => { const normalized = numbers @@ -82,6 +85,25 @@ const App = () => { setDirtyMilestoneNumbers((prev) => [...new Set([...prev, ...normalized])]); }; + const handleHierarchyChanged = (action) => { + setHierarchyAction({ ...action, key: Date.now() }); + if (action.type === 'relink') { + markMilestonesDirty([ + action.sourceMilestoneNumber, + action.targetMilestoneNumber, + action.fromMilestone, + action.toMilestone, + ]); + } else if (action.type === 'unlink') { + markMilestonesDirty([action.sourceMilestoneNumber]); + } else if (action.type === 'error') { + markMilestonesDirty([ + action.sourceMilestoneNumber, + action.targetMilestoneNumber, + ]); + } + }; + useEffect(() => { fetchProject() .then((data) => { @@ -260,7 +282,10 @@ const App = () => { }; const renderPlanningContent = () => ( - <> + {loading && ( @@ -274,13 +299,7 @@ const App = () => { )} {!loading && !error && ( -
+ {milestones.length === 0 && ( There are no milestones available. @@ -293,13 +312,14 @@ const App = () => { milestone={milestone} sortOrder={sortOrder} issueMilestoneRefresh={issueMilestoneRefresh} + hierarchyAction={hierarchyAction} onIssueMilestoneMoved={handleIssueMilestoneMoved} onIssueLabelsChanged={handleIssueLabelsChanged} /> ))} -
+ )} - + ); return ( diff --git a/frontend/src/components/IssueCard.jsx b/frontend/src/components/IssueCard.jsx index f973d66..fdf51bc 100644 --- a/frontend/src/components/IssueCard.jsx +++ b/frontend/src/components/IssueCard.jsx @@ -18,6 +18,10 @@ import { CodeBranchIcon, CaretDownIcon, CaretRightIcon, + LayerGroupIcon, + CatalogIcon, + TaskIcon, + ExclamationTriangleIcon, } from '@patternfly/react-icons'; import { getDaysSince, formatDate } from '../utils/dateUtils'; import { @@ -32,6 +36,7 @@ import { fetchAssignees, setIssueAssignees, removeIssueAssignees, + adoptParentMilestone, } from '../services/api'; import CommentCard from './CommentCard'; import Reactions from './Reactions'; @@ -83,6 +88,16 @@ const IssueCard = ({ onMilestoneChange, onLabelsChange, onIssueUpdate, + enableHierarchy = false, + columnCount = 7, + isHierarchyExpanded = false, + onToggleHierarchy, + isDropTarget = false, + isDragging = false, + onDragStartIssue, + onDragOverIssue, + onDragEndIssue, + onAdoptParentMilestone, }) => { const daysSince = getDaysSince(issue.created_at); const [isDescriptionExpanded, setIsDescriptionExpanded] = useState(false); @@ -783,9 +798,130 @@ const IssueCard = ({ borderBottom: '1px solid #d2d2d2', }; + const depth = issue.hierarchy_depth || 0; + const childCount = issue.child_count ?? (issue.children || []).length; + const indentPx = enableHierarchy ? depth * 16 : 0; + + const handleAdoptParentMilestone = async () => { + try { + const result = await adoptParentMilestone(issue.number); + onAdoptParentMilestone?.({ + fromMilestoneNumber: result.from_milestone ?? currentMilestone?.number, + toMilestoneNumber: result.to_milestone, + }); + } catch (err) { + console.error('Failed to adopt parent milestone:', err); + } + }; + + const renderHierarchyType = () => { + if (!enableHierarchy) return null; + let Icon = LayerGroupIcon; + let label = 'Epic'; + if (depth === 1) { + Icon = CatalogIcon; + label = 'Story'; + } else if (depth >= 2) { + Icon = TaskIcon; + label = 'Sub-story'; + } + return ( + +
+ {childCount > 0 && ( + + )} + + + + {depth === 0 && childCount > 0 && ( + + {childCount} + + )} + + + {issue.external_parent && ( + + + + + + + )} +
+ + ); + }; + + const rowStyle = { + backgroundColor: isDropTarget + ? '#e7f1fa' + : isDragging + ? '#f0f0f0' + : undefined, + opacity: isDragging ? 0.6 : 1, + }; + return ( <> - + {/* Column 1: Expansion icon */} {issue.body_html ? ( @@ -808,7 +944,7 @@ const IssueCard = ({ {/* Column 2: Issue number link */} - + - {/* Column 3: Owner avatar, creation date, assigned chiclet stacked */} + {renderHierarchyType()} + + {/* Column: Owner avatar, creation date, assigned chiclet stacked */}
{ expect(onLabelsChange).toHaveBeenCalledWith({ milestoneNumber: 0 }); }); }); + + it('shows Epic type icon and child count when hierarchy enabled', async () => { + const epic = { + ...mockIssue, + hierarchy_depth: 0, + child_count: 2, + children: [{ number: 2 }, { number: 3 }], + }; + await act(async () => { + render( + + + + +
+ ); + }); + expect(screen.getByLabelText('Epic')).toBeInTheDocument(); + expect(screen.getByText('2')).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Show sub-issues' }) + ).toBeInTheDocument(); + }); + + it('shows Story and Sub-story icons by depth', async () => { + await act(async () => { + render( + + + + + +
+ ); + }); + expect(screen.getByLabelText('Story')).toBeInTheDocument(); + expect(screen.getByLabelText('Sub-story')).toBeInTheDocument(); + }); + + it('calls adoptParentMilestone for external parent', async () => { + const user = userEvent.setup(); + const onAdopt = vi.fn(); + api.adoptParentMilestone.mockResolvedValue({ + from_milestone: 1, + to_milestone: 2, + }); + const issue = { + ...mockIssue, + hierarchy_depth: 0, + child_count: 0, + external_parent: { + number: 99, + title: 'Other', + milestone: { number: 2, title: 'M2' }, + }, + }; + await act(async () => { + render( + + + + +
+ ); + }); + await user.click(screen.getByText('Match parent milestone')); + await waitFor(() => { + expect(api.adoptParentMilestone).toHaveBeenCalledWith(459); + expect(onAdopt).toHaveBeenCalledWith({ + fromMilestoneNumber: 1, + toMilestoneNumber: 2, + }); + }); + }); }); diff --git a/frontend/src/components/MilestoneCard.jsx b/frontend/src/components/MilestoneCard.jsx index 52ff5f0..db49b52 100644 --- a/frontend/src/components/MilestoneCard.jsx +++ b/frontend/src/components/MilestoneCard.jsx @@ -12,93 +12,35 @@ import { } from '@patternfly/react-core'; import { fetchIssues } from '../services/api'; import IssueCard from './IssueCard'; +import { usePlanningDnD } from './PlanningDnDContext'; +import { + flattenVisibleIssues, + collectSubtreeNumbers, + removeIssueFromForest, + insertIssueInForest, + reannotateDepths, +} from '../utils/issueHierarchy'; + +const thStyle = { + padding: '0.5rem', + textAlign: 'left', + fontSize: '0.75rem', + fontWeight: '600', + color: '#6a6e73', + textTransform: 'uppercase', +}; -const itemTableHeader = ( +const itemTableHeader = (includeType) => ( - - - Number - - - Author - - - PR - - - Milestone - - - Labels - - - Title - + + Number + {includeType && Type} + Author + PR + Milestone + Labels + Title ); @@ -109,6 +51,7 @@ const MilestoneCard = ({ issueMilestoneRefresh = { key: 0, milestoneNumbers: [] }, onIssueMilestoneMoved, onIssueLabelsChanged, + hierarchyAction, }) => { const [isIssuesExpanded, setIsIssuesExpanded] = useState(false); const [isPrsExpanded, setIsPrsExpanded] = useState(false); @@ -117,7 +60,9 @@ const MilestoneCard = ({ const [loading, setLoading] = useState(false); const [error, setError] = useState(null); const [hasLoadedOnce, setHasLoadedOnce] = useState(false); + const [expandedHierarchy, setExpandedHierarchy] = useState(() => new Set()); const prevMilestoneNumberRef = useRef(milestone.number); + const dnd = usePlanningDnD(); const applyFetchedData = useCallback((data) => { setIssues(data.issues || []); @@ -148,6 +93,7 @@ const MilestoneCard = ({ setError(null); setLoading(false); setIsPrsExpanded(false); + setExpandedHierarchy(new Set()); } }, [milestone.number]); @@ -179,7 +125,6 @@ const MilestoneCard = ({ // Re-fetch issues when sort order changes (if already loaded) const prevSortOrderRef = useRef(sortOrder); useEffect(() => { - // Only refetch if sort order actually changed and issues are already loaded const sortOrderChanged = JSON.stringify(prevSortOrderRef.current) !== JSON.stringify(sortOrder); if (isIssuesExpanded && hasLoadedOnce && !loading && sortOrderChanged) { @@ -195,13 +140,62 @@ const MilestoneCard = ({ const { key, milestoneNumbers } = issueMilestoneRefresh; if (key === 0) return; if (!milestoneNumbers.includes(milestone.number)) return; - // Refresh even when collapsed so expanding later shows fresh data. if (!hasLoadedOnce) return; refetchIssues(); - // Bump `key` and `milestoneNumbers` update together; refetch only when key changes. // eslint-disable-next-line react-hooks/exhaustive-deps }, [issueMilestoneRefresh.key]); + // Apply optimistic hierarchy mutations from Planning DnD / adopt. + useEffect(() => { + if (!hierarchyAction || !hasLoadedOnce) return; + const { + type, + issueNumber, + parentNumber, + sourceMilestoneNumber, + targetMilestoneNumber, + issueSnapshot, + } = hierarchyAction; + + if (type === 'unlink') { + if (sourceMilestoneNumber !== milestone.number) return; + setIssues((prev) => { + const { forest, removed } = removeIssueFromForest(prev, issueNumber); + if (!removed) return prev; + const asEpic = { + ...removed, + parent_number: null, + external_parent: null, + hierarchy_depth: 0, + }; + return reannotateDepths([...forest, asEpic]); + }); + return; + } + + if (type === 'relink') { + const touchesSource = sourceMilestoneNumber === milestone.number; + const touchesTarget = targetMilestoneNumber === milestone.number; + if (!touchesSource && !touchesTarget) return; + + setIssues((prev) => { + let forest = prev; + let removed = issueSnapshot || null; + if (touchesSource) { + const result = removeIssueFromForest(forest, issueNumber); + forest = result.forest; + removed = result.removed || removed; + } + if (touchesTarget && removed) { + forest = insertIssueInForest(forest, removed, parentNumber); + forest = reannotateDepths(forest); + setExpandedHierarchy((exp) => new Set(exp).add(parentNumber)); + } + return forest; + }); + } + }, [hierarchyAction, hasLoadedOnce, milestone.number]); + const formatDueDate = (dueOn) => { if (!dueOn) return 'No due date'; const date = new Date(dueOn); @@ -216,7 +210,9 @@ const MilestoneCard = ({ const baseText = isIssuesExpanded ? 'Hide' : 'Show'; if (hasLoadedOnce) { - const count = issues.length; + const countVisible = (nodes) => + (nodes || []).reduce((sum, n) => sum + 1 + countVisible(n.children), 0); + const count = countVisible(issues); const issueText = count === 1 ? 'issue' : 'issues'; return `${baseText} ${count} ${issueText}`; } @@ -231,13 +227,93 @@ const MilestoneCard = ({ return `${baseText} ${count} ${prText}`; }; - const updateItemInList = (setter) => (updatedIssue) => { - setter((prev) => - prev.map((item) => (item.id === updatedIssue.id ? updatedIssue : item)) + const updateItemInForest = (setter) => (updatedIssue) => { + setter((prev) => { + const mapNodes = (nodes) => + (nodes || []).map((item) => { + if (item.id === updatedIssue.id) { + return { + ...item, + ...updatedIssue, + children: updatedIssue.children ?? item.children, + }; + } + return { + ...item, + children: mapNodes(item.children), + }; + }); + return mapNodes(prev); + }); + }; + + const toggleHierarchy = (issueNumber) => { + setExpandedHierarchy((prev) => { + const next = new Set(prev); + if (next.has(issueNumber)) next.delete(issueNumber); + else next.add(issueNumber); + return next; + }); + }; + + const renderIssueTable = () => { + const visible = flattenVisibleIssues(issues, expandedHierarchy); + return ( + + {itemTableHeader(true)} + + {visible.map((issue) => ( + toggleHierarchy(issue.number)} + isDropTarget={ + dnd?.hoverParent?.issue?.number === issue.number && + dnd?.hoverParent?.milestoneNumber === milestone.number + } + isDragging={dnd?.dragged?.issue?.number === issue.number} + onDragStartIssue={(e) => { + dnd?.beginDrag(e, { + issue, + sourceMilestoneNumber: milestone.number, + descendantNumbers: collectSubtreeNumbers(issue), + }); + }} + onDragOverIssue={(e) => { + e.preventDefault(); + dnd?.pointerOverParent(issue, milestone.number); + }} + onDragEndIssue={() => { + dnd?.finishDrag(); + }} + onMilestoneChange={(detail) => { + onIssueMilestoneMoved?.(detail); + }} + onLabelsChange={(detail) => { + onIssueLabelsChanged?.(detail); + }} + onIssueUpdate={updateItemInForest(setIssues)} + onAdoptParentMilestone={(detail) => { + onIssueMilestoneMoved?.(detail); + }} + /> + ))} + +
); }; - const renderItemTable = (items, onItemUpdate) => ( + const renderPrTable = () => ( - {itemTableHeader} + {itemTableHeader(false)} - {items.map((issue) => ( + {pullRequests.map((issue) => ( { onIssueMilestoneMoved?.(detail); }} onLabelsChange={(detail) => { onIssueLabelsChanged?.(detail); }} - onIssueUpdate={onItemUpdate} + onIssueUpdate={updateItemInForest(setPullRequests)} /> ))} @@ -297,31 +375,24 @@ const MilestoneCard = ({ )} - {!loading && !error && issues.length > 0 && ( - <>{renderItemTable(issues, updateItemInList(setIssues))} + {!loading && !error && issues.length === 0 && hasLoadedOnce && ( +

+ No issues found for this milestone. +

)} - {!loading && - !error && - issues.length === 0 && - isIssuesExpanded && - hasLoadedOnce && ( -

- No issues found for this milestone. -

- )} + {!loading && !error && issues.length > 0 && renderIssueTable()} - {hasLoadedOnce && !error && pullRequests.length > 0 && ( -
- setIsPrsExpanded(!isPrsExpanded)} - isExpanded={isPrsExpanded} - > - {renderItemTable(pullRequests, updateItemInList(setPullRequests))} - -
+ {hasLoadedOnce && pullRequests.length > 0 && ( + setIsPrsExpanded(!isPrsExpanded)} + isExpanded={isPrsExpanded} + style={{ marginTop: '0.75rem' }} + > + {renderPrTable()} + )} diff --git a/frontend/src/components/MilestoneCard.test.jsx b/frontend/src/components/MilestoneCard.test.jsx index a5c11f2..037fb24 100644 --- a/frontend/src/components/MilestoneCard.test.jsx +++ b/frontend/src/components/MilestoneCard.test.jsx @@ -302,4 +302,48 @@ describe('MilestoneCard', () => { expect(api.fetchIssues).toHaveBeenCalledTimes(2); }); }); + + it('renders nested children when epic hierarchy is expanded', async () => { + const user = userEvent.setup(); + const nested = { + ...mockIssue, + hierarchy_depth: 0, + child_count: 1, + children: [ + { + id: 99, + number: 500, + title: 'Child Story', + html_url: 'https://github.com/test/issue/500', + user: { login: 'testuser', avatar_url: 'https://avatar.url' }, + created_at: '2025-01-01T00:00:00Z', + labels: [], + comments: 0, + hierarchy_depth: 1, + child_count: 0, + children: [], + }, + ], + }; + api.fetchIssues.mockResolvedValue({ + issues: [nested], + pull_requests: [], + }); + + await act(async () => { + render(); + }); + + await user.click(screen.getByRole('button', { name: /show issues/i })); + await waitFor(() => { + expect(screen.getByText(/Test Issue/)).toBeInTheDocument(); + }); + expect(screen.queryByText(/Child Story/)).not.toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: 'Show sub-issues' })); + await waitFor(() => { + expect(screen.getByText(/Child Story/)).toBeInTheDocument(); + }); + expect(screen.getByText('Type')).toBeInTheDocument(); + }); }); diff --git a/frontend/src/components/PlanningBoard.jsx b/frontend/src/components/PlanningBoard.jsx new file mode 100644 index 0000000..b64e25d --- /dev/null +++ b/frontend/src/components/PlanningBoard.jsx @@ -0,0 +1,55 @@ +// Generated-by: Cursor +import React, { useEffect } from 'react'; +import { Alert } from '@patternfly/react-core'; +import { usePlanningDnD } from './PlanningDnDContext'; + +/** + * Scrollable planning bounds used for unlink-zone detection during issue DnD. + * + * Generated-by: Cursor + */ +const PlanningBoard = ({ children }) => { + const dnd = usePlanningDnD(); + + useEffect(() => { + if (!dnd?.dragged) return undefined; + + const onDragOver = (e) => { + e.preventDefault(); + const bounds = dnd.planningBoundsRef.current; + if (!bounds) return; + const rect = bounds.getBoundingClientRect(); + const inside = + e.clientX >= rect.left && + e.clientX <= rect.right && + e.clientY >= rect.top && + e.clientY <= rect.bottom; + dnd.pointerOverPlanning(inside); + }; + + window.addEventListener('dragover', onDragOver); + return () => window.removeEventListener('dragover', onDragOver); + }, [dnd]); + + return ( +
+ {dnd?.error && ( + + {dnd.error} + + )} + {children} +
+ ); +}; + +export default PlanningBoard; diff --git a/frontend/src/components/PlanningDnDContext.jsx b/frontend/src/components/PlanningDnDContext.jsx new file mode 100644 index 0000000..f483d74 --- /dev/null +++ b/frontend/src/components/PlanningDnDContext.jsx @@ -0,0 +1,238 @@ +// Generated-by: Cursor +import React, { + createContext, + useCallback, + useContext, + useMemo, + useRef, + useState, +} from 'react'; +import { clearIssueParent, setIssueParent } from '../services/api'; + +const PlanningDnDContext = createContext(null); + +export const usePlanningDnD = () => useContext(PlanningDnDContext); + +/** + * Coordinates cross-milestone issue parent drag/relink/unlink for Planning. + * + * Generated-by: Cursor + */ +export const PlanningDnDProvider = ({ + children, + onHierarchyChanged, + milestones = [], +}) => { + const planningBoundsRef = useRef(null); + const dragImageRef = useRef(null); + const draggedRef = useRef(null); + const hoverParentRef = useRef(null); + const isUnlinkZoneRef = useRef(false); + const onHierarchyChangedRef = useRef(onHierarchyChanged); + onHierarchyChangedRef.current = onHierarchyChanged; + + const [dragged, setDragged] = useState(null); + const [hoverParent, setHoverParentState] = useState(null); + const [isUnlinkZone, setIsUnlinkZoneState] = useState(false); + const [error, setError] = useState(null); + + const setHoverParent = useCallback((value) => { + hoverParentRef.current = value; + setHoverParentState(value); + }, []); + + const setIsUnlinkZone = useCallback((value) => { + isUnlinkZoneRef.current = value; + setIsUnlinkZoneState(value); + }, []); + + const milestoneTitle = useCallback( + (number) => { + const m = milestones.find((x) => x.number === number); + return m?.title || (number === 0 ? 'none' : `#${number}`); + }, + [milestones] + ); + + const updateDragImage = useCallback((issueNumber, opts = {}) => { + const el = dragImageRef.current; + if (!el) return; + const { parentLabel, milestoneLabel, unlink } = opts; + if (unlink) { + el.textContent = `#${issueNumber} → Unlink (become Epic)`; + el.style.backgroundColor = '#f0ab00'; + el.style.color = '#1b1d21'; + return; + } + const parentPart = parentLabel + ? ` under ${parentLabel}` + : ' (pick a parent)'; + const msPart = milestoneLabel ? ` · milestone: ${milestoneLabel}` : ''; + el.textContent = `#${issueNumber}${parentPart}${msPart}`; + el.style.backgroundColor = '#0066cc'; + el.style.color = '#fff'; + }, []); + + const beginDrag = useCallback( + (e, payload) => { + setError(null); + draggedRef.current = payload; + setDragged(payload); + setHoverParent(null); + setIsUnlinkZone(false); + e.dataTransfer.effectAllowed = 'move'; + e.dataTransfer.setData( + 'application/x-guidellm-issue', + String(payload.issue.number) + ); + e.dataTransfer.setData('text/plain', String(payload.issue.number)); + + if (!dragImageRef.current) { + const el = document.createElement('div'); + el.style.position = 'absolute'; + el.style.top = '-1000px'; + el.style.left = '-1000px'; + el.style.padding = '0.35rem 0.75rem'; + el.style.borderRadius = '0.25rem'; + el.style.fontSize = '0.875rem'; + el.style.fontWeight = '600'; + el.style.pointerEvents = 'none'; + el.style.whiteSpace = 'nowrap'; + el.style.zIndex = '10000'; + document.body.appendChild(el); + dragImageRef.current = el; + } + updateDragImage(payload.issue.number, {}); + e.dataTransfer.setDragImage(dragImageRef.current, 12, 12); + }, + [setHoverParent, setIsUnlinkZone, updateDragImage] + ); + + const pointerOverParent = useCallback( + (parentIssue, parentMilestoneNumber) => { + const current = draggedRef.current; + if (!current) return; + if (parentIssue.number === current.issue.number) return; + if (current.descendantNumbers?.has(parentIssue.number)) return; + setIsUnlinkZone(false); + setHoverParent({ + issue: parentIssue, + milestoneNumber: parentMilestoneNumber, + }); + updateDragImage(current.issue.number, { + parentLabel: `#${parentIssue.number}`, + milestoneLabel: milestoneTitle(parentMilestoneNumber), + }); + }, + [milestoneTitle, setHoverParent, setIsUnlinkZone, updateDragImage] + ); + + const pointerOverPlanning = useCallback( + (inside) => { + const current = draggedRef.current; + if (!current) return; + if (!inside) { + setIsUnlinkZone(true); + setHoverParent(null); + updateDragImage(current.issue.number, { unlink: true }); + } else if (isUnlinkZoneRef.current && !hoverParentRef.current) { + setIsUnlinkZone(false); + updateDragImage(current.issue.number, {}); + } + }, + [setHoverParent, setIsUnlinkZone, updateDragImage] + ); + + const finishDrag = useCallback(async () => { + const current = draggedRef.current; + const parent = hoverParentRef.current; + const unlink = isUnlinkZoneRef.current; + + draggedRef.current = null; + hoverParentRef.current = null; + isUnlinkZoneRef.current = false; + setDragged(null); + setHoverParentState(null); + setIsUnlinkZoneState(false); + + if (!current) return; + + try { + if (unlink) { + if ( + current.issue.parent_number == null && + !current.issue.external_parent + ) { + return; + } + await clearIssueParent(current.issue.number); + onHierarchyChangedRef.current?.({ + type: 'unlink', + issueNumber: current.issue.number, + issueSnapshot: current.issue, + sourceMilestoneNumber: current.sourceMilestoneNumber, + }); + return; + } + + if (parent) { + const result = await setIssueParent( + current.issue.number, + parent.issue.number + ); + onHierarchyChangedRef.current?.({ + type: 'relink', + issueNumber: current.issue.number, + issueSnapshot: current.issue, + parentNumber: parent.issue.number, + sourceMilestoneNumber: current.sourceMilestoneNumber, + targetMilestoneNumber: parent.milestoneNumber, + fromMilestone: result.from_milestone, + toMilestone: result.to_milestone, + updatedIssueNumbers: result.updated_issue_numbers || [], + }); + } + } catch (err) { + setError(err.message || String(err)); + onHierarchyChangedRef.current?.({ + type: 'error', + message: err.message || String(err), + sourceMilestoneNumber: current.sourceMilestoneNumber, + targetMilestoneNumber: parent?.milestoneNumber, + }); + } + }, []); + + const value = useMemo( + () => ({ + planningBoundsRef, + dragged, + hoverParent, + isUnlinkZone, + error, + clearError: () => setError(null), + beginDrag, + pointerOverParent, + pointerOverPlanning, + finishDrag, + setHoverParent, + }), + [ + dragged, + hoverParent, + isUnlinkZone, + error, + beginDrag, + pointerOverParent, + pointerOverPlanning, + finishDrag, + setHoverParent, + ] + ); + + return ( + + {children} + + ); +}; diff --git a/frontend/src/components/PlanningDnDContext.test.jsx b/frontend/src/components/PlanningDnDContext.test.jsx new file mode 100644 index 0000000..3ec2418 --- /dev/null +++ b/frontend/src/components/PlanningDnDContext.test.jsx @@ -0,0 +1,149 @@ +// Generated-by: Cursor +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, act, waitFor } from '@testing-library/react'; +import { PlanningDnDProvider, usePlanningDnD } from './PlanningDnDContext'; +import * as api from '../services/api'; + +vi.mock('../services/api'); + +const Probe = ({ issue }) => { + const dnd = usePlanningDnD(); + return ( +
+ + + {dnd.isUnlinkZone ? 'unlink' : 'link'} + +
+ ); +}; + +const makeDataTransfer = () => ({ + effectAllowed: 'move', + setData: vi.fn(), + setDragImage: vi.fn(), +}); + +describe('PlanningDnDContext', () => { + beforeEach(() => { + vi.clearAllMocks(); + api.setIssueParent.mockResolvedValue({ + from_milestone: 1, + to_milestone: 2, + updated_issue_numbers: [5], + }); + api.clearIssueParent.mockResolvedValue({ parent_number: 9 }); + }); + + it('relinks on drop over a parent', async () => { + const onHierarchyChanged = vi.fn(); + const issue = { number: 5, parent_number: null }; + + await act(async () => { + render( + + + + ); + }); + + const dragBtn = screen.getByText('drag-me'); + + await act(async () => { + const start = new Event('dragstart', { bubbles: true }); + Object.defineProperty(start, 'dataTransfer', { + value: makeDataTransfer(), + }); + dragBtn.dispatchEvent(start); + }); + + await act(async () => { + screen.getByText('drop-target').click(); + }); + + await act(async () => { + dragBtn.dispatchEvent(new Event('dragend', { bubbles: true })); + }); + + await waitFor(() => { + expect(api.setIssueParent).toHaveBeenCalledWith(5, 10); + expect(onHierarchyChanged).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'relink', + parentNumber: 10, + targetMilestoneNumber: 2, + }) + ); + }); + }); + + it('unlinks when released in unlink zone', async () => { + const onHierarchyChanged = vi.fn(); + const issue = { number: 5, parent_number: 9 }; + + await act(async () => { + render( + + + + ); + }); + + const dragBtn = screen.getByText('drag-me'); + + await act(async () => { + const start = new Event('dragstart', { bubbles: true }); + Object.defineProperty(start, 'dataTransfer', { + value: makeDataTransfer(), + }); + dragBtn.dispatchEvent(start); + }); + + await act(async () => { + screen.getByText('leave-bounds').click(); + }); + expect(screen.getByTestId('unlink').textContent).toBe('unlink'); + + await act(async () => { + dragBtn.dispatchEvent(new Event('dragend', { bubbles: true })); + }); + + await waitFor(() => { + expect(api.clearIssueParent).toHaveBeenCalledWith(5); + expect(onHierarchyChanged).toHaveBeenCalledWith( + expect.objectContaining({ type: 'unlink', issueNumber: 5 }) + ); + }); + }); +}); diff --git a/frontend/src/services/api.js b/frontend/src/services/api.js index f778066..8a92549 100644 --- a/frontend/src/services/api.js +++ b/frontend/src/services/api.js @@ -146,6 +146,43 @@ export const removeIssueMilestone = async (issueNumber, milestoneNumber) => { return response.json(); }; +export const setIssueParent = async (issueNumber, parentNumber) => { + const response = await fetch(`${API_BASE}/issues/${issueNumber}/parent`, { + method: 'PUT', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ parent_number: parentNumber }), + }); + if (!response.ok) { + throw new Error(`Failed to set parent: ${response.statusText}`); + } + return response.json(); +}; + +export const clearIssueParent = async (issueNumber) => { + const response = await fetch(`${API_BASE}/issues/${issueNumber}/parent`, { + method: 'DELETE', + }); + if (!response.ok) { + throw new Error(`Failed to clear parent: ${response.statusText}`); + } + return response.json(); +}; + +export const adoptParentMilestone = async (issueNumber) => { + const response = await fetch( + `${API_BASE}/issues/${issueNumber}/adopt-parent-milestone`, + { + method: 'POST', + } + ); + if (!response.ok) { + throw new Error(`Failed to adopt parent milestone: ${response.statusText}`); + } + 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 613cc87..cb37d6b 100644 --- a/frontend/src/services/api.test.js +++ b/frontend/src/services/api.test.js @@ -9,6 +9,9 @@ import { fetchEscapedDefectRate, fetchBugBacklogDelta, fetchProjectStatusReport, + setIssueParent, + clearIssueParent, + adoptParentMilestone, } from './api'; describe('api', () => { @@ -221,4 +224,42 @@ describe('api', () => { ).rejects.toThrow('Failed to fetch project status report'); }); }); + + describe('hierarchy APIs', () => { + it('setIssueParent PUTs parent_number', async () => { + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ parent_number: 10 }), + }); + await setIssueParent(20, 10); + expect(global.fetch).toHaveBeenCalledWith('/api/v1/issues/20/parent', { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ parent_number: 10 }), + }); + }); + + it('clearIssueParent DELETEs parent', async () => { + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ message: 'parent cleared' }), + }); + await clearIssueParent(20); + expect(global.fetch).toHaveBeenCalledWith('/api/v1/issues/20/parent', { + method: 'DELETE', + }); + }); + + it('adoptParentMilestone POSTs', async () => { + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ to_milestone: 2 }), + }); + await adoptParentMilestone(20); + expect(global.fetch).toHaveBeenCalledWith( + '/api/v1/issues/20/adopt-parent-milestone', + { method: 'POST' } + ); + }); + }); }); diff --git a/frontend/src/utils/issueHierarchy.js b/frontend/src/utils/issueHierarchy.js new file mode 100644 index 0000000..97b59e7 --- /dev/null +++ b/frontend/src/utils/issueHierarchy.js @@ -0,0 +1,109 @@ +// Generated-by: Cursor +/** + * Flatten a nested issue forest into visible rows based on hierarchy expand state. + */ +export const flattenVisibleIssues = (forest, expandedNumbers) => { + const rows = []; + const walk = (nodes) => { + for (const node of nodes || []) { + rows.push(node); + const children = node.children || []; + if (children.length > 0 && expandedNumbers.has(node.number)) { + walk(children); + } + } + }; + walk(forest); + return rows; +}; + +/** + * Collect issue numbers in a subtree (including root). + */ +export const collectSubtreeNumbers = (issue) => { + const numbers = new Set([issue.number]); + const walk = (node) => { + for (const child of node.children || []) { + numbers.add(child.number); + walk(child); + } + }; + walk(issue); + return numbers; +}; + +/** + * Remove an issue (by number) from a forest; returns { forest, removed }. + */ +export const removeIssueFromForest = (forest, issueNumber) => { + let removed = null; + const filterNodes = (nodes) => { + const next = []; + for (const node of nodes || []) { + if (node.number === issueNumber) { + removed = node; + continue; + } + const children = filterNodes(node.children || []); + next.push({ + ...node, + children, + child_count: children.length, + }); + } + return next; + }; + return { forest: filterNodes(forest), removed }; +}; + +/** + * Insert issue as child of parentNumber, or as root if parentNumber is null. + */ +export const insertIssueInForest = (forest, issue, parentNumber) => { + const node = { + ...issue, + parent_number: parentNumber, + external_parent: null, + children: issue.children || [], + child_count: (issue.children || []).length, + }; + + if (parentNumber == null) { + return [...forest, { ...node, hierarchy_depth: 0 }]; + } + + const mapNodes = (nodes, depth) => + (nodes || []).map((n) => { + if (n.number === parentNumber) { + const children = [ + ...(n.children || []), + { ...node, hierarchy_depth: depth + 1 }, + ]; + return { + ...n, + children, + child_count: children.length, + }; + } + return { + ...n, + children: mapNodes(n.children || [], depth + 1), + }; + }); + + return mapNodes(forest, 0); +}; + +/** + * Recompute hierarchy_depth for every node from roots. + */ +export const reannotateDepths = (forest, depth = 0) => + (forest || []).map((node) => { + const children = reannotateDepths(node.children || [], depth + 1); + return { + ...node, + hierarchy_depth: depth, + children, + child_count: children.length, + }; + }); diff --git a/frontend/src/utils/issueHierarchy.test.js b/frontend/src/utils/issueHierarchy.test.js new file mode 100644 index 0000000..f12ad85 --- /dev/null +++ b/frontend/src/utils/issueHierarchy.test.js @@ -0,0 +1,63 @@ +// Generated-by: Cursor +import { describe, it, expect } from 'vitest'; +import { + flattenVisibleIssues, + collectSubtreeNumbers, + removeIssueFromForest, + insertIssueInForest, + reannotateDepths, +} from './issueHierarchy'; + +describe('issueHierarchy utils', () => { + const forest = [ + { + number: 1, + id: 1, + children: [ + { + number: 2, + id: 2, + children: [{ number: 3, id: 3, children: [] }], + child_count: 1, + }, + ], + child_count: 1, + }, + { number: 4, id: 4, children: [], child_count: 0 }, + ]; + + it('flattens only expanded branches', () => { + expect( + flattenVisibleIssues(forest, new Set()).map((i) => i.number) + ).toEqual([1, 4]); + expect( + flattenVisibleIssues(forest, new Set([1])).map((i) => i.number) + ).toEqual([1, 2, 4]); + expect( + flattenVisibleIssues(forest, new Set([1, 2])).map((i) => i.number) + ).toEqual([1, 2, 3, 4]); + }); + + it('collects subtree numbers', () => { + expect([...collectSubtreeNumbers(forest[0])].sort()).toEqual([1, 2, 3]); + }); + + it('removes an issue from the forest', () => { + const { forest: next, removed } = removeIssueFromForest(forest, 2); + expect(removed.number).toBe(2); + expect(next[0].children).toEqual([]); + expect(next[0].child_count).toBe(0); + }); + + it('inserts under a parent and reannotates depths', () => { + const inserted = insertIssueInForest( + [{ number: 1, id: 1, children: [], child_count: 0 }], + { number: 9, id: 9, children: [] }, + 1 + ); + const annotated = reannotateDepths(inserted); + expect(annotated[0].hierarchy_depth).toBe(0); + expect(annotated[0].children[0].number).toBe(9); + expect(annotated[0].children[0].hierarchy_depth).toBe(1); + }); +}); From 855bd64c3db2c7527c839917d5c232f8c46dc6df Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Sat, 25 Jul 2026 14:37:48 -0400 Subject: [PATCH 05/11] Allow adding comments / issues Generated-by: Cursor Signed-off-by: David Butenhof --- backend/src/github_pm/api.py | 171 +++++++ backend/tests/test_api.py | 211 ++++++++ frontend/src/components/IssueCard.jsx | 225 ++++++--- frontend/src/components/IssueCard.test.jsx | 36 +- .../src/components/MarkdownInputModal.jsx | 461 ++++++++++++++++++ .../components/MarkdownInputModal.test.jsx | 151 ++++++ frontend/src/components/MilestoneCard.jsx | 63 ++- .../src/components/MilestoneCard.test.jsx | 21 + frontend/src/services/api.js | 61 +++ frontend/src/services/api.test.js | 83 ++++ 10 files changed, 1417 insertions(+), 66 deletions(-) create mode 100644 frontend/src/components/MarkdownInputModal.jsx create mode 100644 frontend/src/components/MarkdownInputModal.test.jsx diff --git a/backend/src/github_pm/api.py b/backend/src/github_pm/api.py index 90e3a70..fd43675 100644 --- a/backend/src/github_pm/api.py +++ b/backend/src/github_pm/api.py @@ -150,6 +150,25 @@ def post( ) return response.json() + def post_text( + self, path: str, text: str, headers: dict[str, str] | None = None + ) -> str: + """POST plain text (e.g. ``/markdown/raw``); returns response body as text. + + Generated-by: Cursor + """ + request_headers = {"Content-Type": "text/plain; charset=utf-8"} + if headers: + request_headers.update(headers) + response = self._with_504_retry( + lambda: self.github.post( + f"{self.base_url}{path}", + data=text.encode("utf-8"), + headers=request_headers, + ) + ) + return response.text + def delete( self, path: str, @@ -359,6 +378,158 @@ async def get_comments( return comments +class CreateComment(BaseModel): + """Body for creating an issue comment. + + Generated-by: Cursor + """ + + body: str = Field(title="Comment Body", min_length=1) + + +@api_router.post("/comments/{issue_number}") +async def create_comment( + gitctx: Annotated[Connector, Depends(connection)], + issue_number: Annotated[int, Path(title="Issue")], + comment: Annotated[CreateComment, Body(title="Comment")], +): + """Create a comment on an issue. + + Generated-by: Cursor + """ + created = gitctx.post( + f"/repos/{context.github_repo}/issues/{issue_number}/comments", + data={"body": comment.body}, + headers={"Accept": "application/vnd.github.html+json"}, + ) + logger.info("Created comment on issue #%s", issue_number) + return created + + +class CloseWithComment(BaseModel): + """Body for closing an issue with an optional comment. + + Generated-by: Cursor + """ + + body: str = Field(title="Comment Body", min_length=1) + + +@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")], +): + """Add a comment to an issue and mark it closed. + + Generated-by: Cursor + """ + created_comment = gitctx.post( + f"/repos/{context.github_repo}/issues/{issue_number}/comments", + data={"body": comment.body}, + headers={"Accept": "application/vnd.github.html+json"}, + ) + closed_issue = gitctx.patch( + f"/repos/{context.github_repo}/issues/{issue_number}", + data={"state": "closed"}, + headers={"Accept": "application/vnd.github.html+json"}, + ) + logger.info("Closed issue #%s with comment", issue_number) + return {"comment": created_comment, "issue": closed_issue} + + +class RenderMarkdown(BaseModel): + """Body for rendering markdown via GitHub. + + Generated-by: Cursor + """ + + text: str = Field(title="Markdown Text", default="") + + +@api_router.post("/markdown") +async def render_markdown( + gitctx: Annotated[Connector, Depends(connection)], + payload: Annotated[RenderMarkdown, Body(title="Markdown")], +): + """Render markdown to HTML using GitHub ``POST /markdown/raw``. + + Generated-by: Cursor + """ + html = gitctx.post_text("/markdown/raw", payload.text) + return {"html": html} + + +class CreateIssue(BaseModel): + """Body for creating a repository issue (optionally as a sub-issue). + + Generated-by: Cursor + """ + + title: str = Field(title="Issue Title", min_length=1) + body: str | None = Field(default=None, title="Issue Body") + labels: list[str] | None = Field(default=None, title="Labels") + assignees: list[str] | None = Field(default=None, title="Assignees") + type: str | None = Field( + default=None, + title="Issue Type", + description="GitHub issue type name, e.g. Bug or Feature", + ) + milestone: int | None = Field(default=None, title="Milestone Number") + parent_number: int | None = Field( + default=None, + title="Parent Issue Number", + description="When set, link the new issue as a sub-issue of this parent", + ) + + +@api_router.post("/issues") +async def create_issue( + gitctx: Annotated[Connector, Depends(connection)], + issue: Annotated[CreateIssue, Body(title="Issue")], +): + """Create an issue; optionally assign milestone and/or parent. + + Generated-by: Cursor + """ + data: dict[str, Any] = {"title": issue.title} + if issue.body is not None: + data["body"] = issue.body + if issue.labels is not None: + data["labels"] = issue.labels + if issue.assignees is not None: + data["assignees"] = issue.assignees + if issue.type is not None: + data["type"] = issue.type + if issue.milestone is not None and issue.milestone != 0: + data["milestone"] = issue.milestone + + created = gitctx.post( + f"/repos/{context.github_repo}/issues", + data=data, + headers={"Accept": "application/vnd.github.html+json"}, + ) + logger.info("Created issue #%s: %s", created.get("number"), issue.title) + + parent_number = issue.parent_number + if parent_number is not None: + if parent_number == created["number"]: + raise HTTPException( + status_code=422, detail="An issue cannot be its own parent" + ) + gitctx.post( + f"/repos/{context.github_repo}/issues/{parent_number}/sub_issues", + data={"sub_issue_id": created["id"], "replace_parent": True}, + ) + logger.info( + "Linked issue #%s as sub-issue of #%s", created["number"], parent_number + ) + created["parent_number"] = parent_number + + return created + + @api_router.get("/issues/{issue_number}/reactions") async def get_issue_reactions( gitctx: Annotated[Connector, Depends(connection)], diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index af7629a..943bbcb 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -16,10 +16,16 @@ adopt_parent_milestone, api_router, clear_issue_parent, + close_issue_with_comment, + CloseWithComment, connection, Connector, + create_comment, + create_issue, create_label, create_milestone, + CreateComment, + CreateIssue, CreateLabel, CreateMilestone, delete_label, @@ -33,6 +39,8 @@ get_project, remove_label_from_issue, remove_milestone_from_issue, + render_markdown, + RenderMarkdown, set_issue_parent, SetIssueParent, ) @@ -1624,3 +1632,206 @@ def test_follows_next_link(self): assert items == [{"number": 1}, {"number": 2}] assert mock_session.get.call_count == 2 + + +class TestCreateComment: + """Test the create_comment endpoint. + + Generated-by: Cursor + """ + + @pytest.mark.asyncio + async def test_create_comment_success(self): + mock_comment = { + "id": 99, + "body": "Hello", + "body_html": "

Hello

", + } + mock_gitctx = Mock(spec=Connector) + mock_gitctx.post = Mock(return_value=mock_comment) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await create_comment(mock_gitctx, 42, CreateComment(body="Hello")) + + assert result == mock_comment + mock_gitctx.post.assert_called_once_with( + "/repos/test/repo/issues/42/comments", + data={"body": "Hello"}, + headers={"Accept": "application/vnd.github.html+json"}, + ) + + +class TestCloseIssueWithComment: + """Test the close_issue_with_comment endpoint. + + Generated-by: Cursor + """ + + @pytest.mark.asyncio + async def test_close_with_comment_success(self): + mock_comment = {"id": 1, "body": "Done"} + 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, CloseWithComment(body="Done") + ) + + assert result == {"comment": mock_comment, "issue": mock_issue} + mock_gitctx.post.assert_called_once() + mock_gitctx.patch.assert_called_once_with( + "/repos/test/repo/issues/42", + data={"state": "closed"}, + headers={"Accept": "application/vnd.github.html+json"}, + ) + + +class TestRenderMarkdown: + """Test the render_markdown endpoint. + + Generated-by: Cursor + """ + + @pytest.mark.asyncio + async def test_render_markdown_success(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.post_text = Mock(return_value="

hi

") + + result = await render_markdown(mock_gitctx, RenderMarkdown(text="**hi**")) + + assert result == {"html": "

hi

"} + mock_gitctx.post_text.assert_called_once_with("/markdown/raw", "**hi**") + + @pytest.mark.asyncio + async def test_render_markdown_empty(self): + mock_gitctx = Mock(spec=Connector) + mock_gitctx.post_text = Mock(return_value="") + + result = await render_markdown(mock_gitctx, RenderMarkdown(text="")) + + assert result == {"html": ""} + mock_gitctx.post_text.assert_called_once_with("/markdown/raw", "") + + +class TestCreateIssue: + """Test the create_issue endpoint. + + Generated-by: Cursor + """ + + @pytest.mark.asyncio + async def test_create_issue_basic(self): + mock_created = {"id": 100, "number": 50, "title": "New"} + mock_gitctx = Mock(spec=Connector) + mock_gitctx.post = Mock(return_value=mock_created) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await create_issue( + mock_gitctx, + CreateIssue(title="New", body="Body text", type="Bug"), + ) + + assert result == mock_created + call_args = mock_gitctx.post.call_args + assert call_args[0][0] == "/repos/test/repo/issues" + assert call_args[1]["data"]["title"] == "New" + assert call_args[1]["data"]["body"] == "Body text" + assert call_args[1]["data"]["type"] == "Bug" + assert mock_gitctx.post.call_count == 1 + + @pytest.mark.asyncio + async def test_create_issue_with_milestone_labels_assignees(self): + mock_created = {"id": 100, "number": 50, "title": "Epic"} + mock_gitctx = Mock(spec=Connector) + mock_gitctx.post = Mock(return_value=mock_created) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await create_issue( + mock_gitctx, + CreateIssue( + title="Epic", + labels=["enhancement"], + assignees=["alice"], + milestone=6, + type="Feature", + ), + ) + + assert result["number"] == 50 + data = mock_gitctx.post.call_args[1]["data"] + assert data["labels"] == ["enhancement"] + assert data["assignees"] == ["alice"] + assert data["milestone"] == 6 + assert data["type"] == "Feature" + + @pytest.mark.asyncio + async def test_create_issue_skips_milestone_zero(self): + mock_created = {"id": 100, "number": 50, "title": "No MS"} + mock_gitctx = Mock(spec=Connector) + mock_gitctx.post = Mock(return_value=mock_created) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + await create_issue(mock_gitctx, CreateIssue(title="No MS", milestone=0)) + + assert "milestone" not in mock_gitctx.post.call_args[1]["data"] + + @pytest.mark.asyncio + async def test_create_issue_with_parent(self): + mock_created = {"id": 100, "number": 50, "title": "Child"} + mock_gitctx = Mock(spec=Connector) + mock_gitctx.post = Mock(return_value=mock_created) + + with patch("github_pm.api.context") as mock_context: + mock_context.github_repo = "test/repo" + result = await create_issue( + mock_gitctx, + CreateIssue(title="Child", parent_number=10, milestone=6), + ) + + assert result["parent_number"] == 10 + assert mock_gitctx.post.call_count == 2 + parent_call = mock_gitctx.post.call_args_list[1] + assert parent_call[0][0] == "/repos/test/repo/issues/10/sub_issues" + assert parent_call[1]["data"] == { + "sub_issue_id": 100, + "replace_parent": True, + } + + +class TestConnectorPostText: + """Test Connector.post_text for markdown/raw. + + Generated-by: Cursor + """ + + def test_post_text_returns_response_text(self): + mock_session = Mock() + mock_response = Mock() + mock_response.status_code = 200 + mock_response.raise_for_status = Mock() + mock_response.text = "

ok

" + mock_session.post.return_value = mock_response + + with ( + patch("github_pm.api.requests.session", return_value=mock_session), + patch("github_pm.api.context") as mock_context, + ): + mock_context.github_repo = "o/r" + mock_context.github_token = "tok" + conn = Connector("tok", github_repo="o/r") + html = conn.post_text("/markdown/raw", "**ok**") + + assert html == "

ok

" + mock_session.post.assert_called_once() + call_kwargs = mock_session.post.call_args + assert call_kwargs[0][0] == "https://api.github.com/markdown/raw" + assert call_kwargs[1]["data"] == b"**ok**" + assert call_kwargs[1]["headers"]["Content-Type"] == "text/plain; charset=utf-8" diff --git a/frontend/src/components/IssueCard.jsx b/frontend/src/components/IssueCard.jsx index fdf51bc..d870fae 100644 --- a/frontend/src/components/IssueCard.jsx +++ b/frontend/src/components/IssueCard.jsx @@ -22,6 +22,7 @@ import { CatalogIcon, TaskIcon, ExclamationTriangleIcon, + PlusIcon, } from '@patternfly/react-icons'; import { getDaysSince, formatDate } from '../utils/dateUtils'; import { @@ -37,10 +38,14 @@ import { setIssueAssignees, removeIssueAssignees, adoptParentMilestone, + createComment, + closeIssueWithComment, + createIssue, } from '../services/api'; import CommentCard from './CommentCard'; import Reactions from './Reactions'; import UserAvatar from './UserAvatar'; +import MarkdownInputModal from './MarkdownInputModal'; import labelsCache, { clearLabelsCache } from '../utils/labelsCache'; import milestonesCache from '../utils/milestonesCache'; import assigneesCache from '../utils/assigneesCache'; @@ -98,6 +103,9 @@ const IssueCard = ({ onDragOverIssue, onDragEndIssue, onAdoptParentMilestone, + onIssueClosed, + onIssueCreated, + milestoneNumber = null, }) => { const daysSince = getDaysSince(issue.created_at); const [isDescriptionExpanded, setIsDescriptionExpanded] = useState(false); @@ -138,13 +146,16 @@ const IssueCard = ({ const [pendingAssignees, setPendingAssignees] = useState([]); // Temporary selection while dropdown is open const assigneesMenuRef = useRef(null); const assigneesToggleRef = useRef(null); + const [isCommentModalOpen, setIsCommentModalOpen] = useState(false); + const [isCreateSubIssueOpen, setIsCreateSubIssueOpen] = useState(false); + const [commentCount, setCommentCount] = useState(issue.comments || 0); useEffect(() => { if ( isCommentsExpanded && comments.length === 0 && !commentsLoading && - issue.comments > 0 + commentCount > 0 ) { setCommentsLoading(true); setCommentsError(null); @@ -161,11 +172,15 @@ const IssueCard = ({ }, [ isCommentsExpanded, issue.number, - issue.comments, + commentCount, comments.length, commentsLoading, ]); + useEffect(() => { + setCommentCount(issue.comments || 0); + }, [issue.comments]); + // Collapse comments when description is collapsed useEffect(() => { if (!isDescriptionExpanded) { @@ -745,8 +760,44 @@ const IssueCard = ({ const getCommentsToggleText = () => { return isCommentsExpanded - ? `Hide Comments (${issue.comments})` - : `Show Comments (${issue.comments})`; + ? `Hide Comments (${commentCount})` + : `Show Comments (${commentCount})`; + }; + + const handleAddComment = async (body) => { + const created = await createComment(issue.number, body); + setComments((prev) => [...prev, created]); + setCommentCount((prev) => prev + 1); + setIsCommentsExpanded(true); + if (onIssueUpdate) { + onIssueUpdate({ ...issue, comments: (commentCount || 0) + 1 }); + } + }; + + const handleCloseWithComment = async (body) => { + await closeIssueWithComment(issue.number, body); + onIssueClosed?.({ issueNumber: issue.number }); + }; + + const handleCreateSubIssue = async (payload) => { + const created = await createIssue({ + ...payload, + milestone: + milestoneNumber ?? + currentMilestone?.number ?? + issue.milestone?.number ?? + undefined, + parent_number: issue.number, + }); + onIssueCreated?.({ + issue: created, + parentNumber: issue.number, + milestoneNumber: + milestoneNumber ?? + currentMilestone?.number ?? + issue.milestone?.number ?? + 0, + }); }; // Column 4: PR icon, "closed by #", or blank @@ -869,6 +920,19 @@ const IssueCard = ({ )} + + + {issue.external_parent && ( {/* Column 1: Expansion icon */} {/* Column 2: Issue number link */} @@ -1632,7 +1694,7 @@ const IssueCard = ({ {/* Expanded description and comments - only shown when expanded */} - {issue.body_html && isDescriptionExpanded && ( + {isDescriptionExpanded && ( )} + setIsCommentModalOpen(false)} + mode="comment" + title={`Comment on #${issue.number}`} + submitLabel="Submit" + showCloseWithComment + onSubmit={handleAddComment} + onCloseWithComment={handleCloseWithComment} + /> + setIsCreateSubIssueOpen(false)} + mode="issue" + title={`Add sub-issue under #${issue.number}`} + submitLabel="Create Issue" + onSubmit={handleCreateSubIssue} + /> { }); }); - it('does not show expansion icon when body is absent', async () => { + it('shows expansion icon when body is absent (for comments)', async () => { const issueWithoutBody = { ...mockIssue, body: null, body_html: null }; await act(async () => { render(); }); await waitFor(() => { expect( - screen.queryByRole('button', { name: /show description/i }) - ).not.toBeInTheDocument(); + screen.getByRole('button', { name: /show description/i }) + ).toBeInTheDocument(); }); }); @@ -507,6 +507,36 @@ describe('IssueCard', () => { expect( screen.getByRole('button', { name: 'Show sub-issues' }) ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Add sub-issue' }) + ).toBeInTheDocument(); + }); + + it('shows Add Comment under expanded description', async () => { + const user = userEvent.setup(); + await act(async () => { + render(); + }); + await user.click(screen.getByRole('button', { name: /show description/i })); + expect( + screen.getByRole('button', { name: /show comments/i }) + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Add Comment' }) + ).toBeInTheDocument(); + }); + + it('opens comment modal from Add Comment', async () => { + const user = userEvent.setup(); + await act(async () => { + render(); + }); + await user.click(screen.getByRole('button', { name: /show description/i })); + await user.click(screen.getByRole('button', { name: 'Add Comment' })); + expect(screen.getByText('Comment on #459')).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Close with Comment' }) + ).toBeInTheDocument(); }); it('shows Story and Sub-story icons by depth', async () => { diff --git a/frontend/src/components/MarkdownInputModal.jsx b/frontend/src/components/MarkdownInputModal.jsx new file mode 100644 index 0000000..c6b0dc0 --- /dev/null +++ b/frontend/src/components/MarkdownInputModal.jsx @@ -0,0 +1,461 @@ +// Generated-by: Cursor +import React, { useState, useEffect, useRef } from 'react'; +import { + Modal, + Button, + Tabs, + Tab, + TabTitleText, + TextInput, + TextArea, + Form, + FormGroup, + Checkbox, + Spinner, + Alert, + Tooltip, +} from '@patternfly/react-core'; +import { renderMarkdown, fetchLabels, fetchAssignees } from '../services/api'; +import labelsCache from '../utils/labelsCache'; +import assigneesCache from '../utils/assigneesCache'; +import UserAvatar from './UserAvatar'; + +const ISSUE_TYPES = ['Bug', 'Feature']; + +/** + * Popup input window with Edit / Preview markdown tabs. + * + * Modes: + * - comment: body only; actions Submit + optional Close with Comment + Cancel + * - issue: title, type, labels, assignees + body; action Submit + Cancel + * + * Generated-by: Cursor + */ +const MarkdownInputModal = ({ + isOpen, + onClose, + mode = 'comment', + title = 'Add Comment', + submitLabel = 'Submit', + onSubmit, + showCloseWithComment = false, + onCloseWithComment, +}) => { + const [activeTab, setActiveTab] = useState(0); + const [body, setBody] = useState(''); + const [issueTitle, setIssueTitle] = useState(''); + const [issueType, setIssueType] = useState('Feature'); + const [selectedLabels, setSelectedLabels] = useState([]); + const [selectedAssignees, setSelectedAssignees] = useState([]); + const [availableLabels, setAvailableLabels] = useState([]); + const [availableAssignees, setAvailableAssignees] = useState([]); + const [labelsLoading, setLabelsLoading] = useState(false); + const [assigneesLoading, setAssigneesLoading] = useState(false); + const [previewHtml, setPreviewHtml] = useState(''); + const [previewLoading, setPreviewLoading] = useState(false); + const [previewError, setPreviewError] = useState(null); + const [submitError, setSubmitError] = useState(null); + const [isSubmitting, setIsSubmitting] = useState(false); + const textareaRef = useRef(null); + const previewRequestId = useRef(0); + + const resetState = () => { + setActiveTab(0); + setBody(''); + setIssueTitle(''); + setIssueType('Feature'); + setSelectedLabels([]); + setSelectedAssignees([]); + setPreviewHtml(''); + setPreviewError(null); + setSubmitError(null); + setIsSubmitting(false); + }; + + useEffect(() => { + if (!isOpen) { + resetState(); + return; + } + if (mode !== 'issue') return; + + // Load labels + if (labelsCache.data.length > 0) { + setAvailableLabels(labelsCache.data); + } else { + setLabelsLoading(true); + fetchLabels() + .then((data) => { + labelsCache.data = data; + setAvailableLabels(data); + setLabelsLoading(false); + }) + .catch(() => setLabelsLoading(false)); + } + + // Load assignees + if (assigneesCache.data.length > 0) { + setAvailableAssignees(assigneesCache.data); + } else { + setAssigneesLoading(true); + fetchAssignees() + .then((data) => { + assigneesCache.data = data; + setAvailableAssignees(data); + setAssigneesLoading(false); + }) + .catch(() => setAssigneesLoading(false)); + } + }, [isOpen, mode]); + + useEffect(() => { + if (!isOpen || activeTab !== 1) return; + + const requestId = ++previewRequestId.current; + setPreviewLoading(true); + setPreviewError(null); + + renderMarkdown(body || '') + .then((data) => { + if (previewRequestId.current !== requestId) return; + setPreviewHtml(data.html || ''); + setPreviewLoading(false); + }) + .catch((err) => { + if (previewRequestId.current !== requestId) return; + setPreviewError(err.message); + setPreviewLoading(false); + }); + }, [isOpen, activeTab, body]); + + const handleClose = () => { + if (isSubmitting) return; + onClose?.(); + }; + + const buildIssuePayload = () => ({ + title: issueTitle.trim(), + body: body || undefined, + type: issueType || undefined, + labels: selectedLabels.length > 0 ? selectedLabels : undefined, + assignees: selectedAssignees.length > 0 ? selectedAssignees : undefined, + }); + + const handleSubmit = async () => { + if (mode === 'issue' && !issueTitle.trim()) { + setSubmitError('Title is required'); + return; + } + if (mode === 'comment' && !body.trim()) { + setSubmitError('Comment body is required'); + return; + } + setIsSubmitting(true); + setSubmitError(null); + try { + if (mode === 'issue') { + await onSubmit?.(buildIssuePayload()); + } else { + await onSubmit?.(body); + } + onClose?.(); + } catch (err) { + setSubmitError(err.message || 'Submit failed'); + } finally { + setIsSubmitting(false); + } + }; + + const handleCloseWithComment = async () => { + if (!body.trim()) { + setSubmitError('Comment body is required'); + return; + } + setIsSubmitting(true); + setSubmitError(null); + try { + await onCloseWithComment?.(body); + onClose?.(); + } catch (err) { + setSubmitError(err.message || 'Close with comment failed'); + } finally { + setIsSubmitting(false); + } + }; + + const toggleLabel = (name, checked) => { + setSelectedLabels((prev) => + checked ? [...prev, name] : prev.filter((n) => n !== name) + ); + }; + + const toggleAssignee = (login, checked) => { + setSelectedAssignees((prev) => + checked ? [...prev, login] : prev.filter((n) => n !== login) + ); + }; + + const actions = [ + , + ]; + + if (showCloseWithComment && mode === 'comment') { + actions.push( + + ); + } + + actions.push( + + ); + + return ( + +
+ {mode === 'issue' && ( + <> + + { + const stringValue = + typeof value === 'string' + ? value + : value?.target?.value || ''; + setIssueTitle(stringValue); + setSubmitError(null); + }} + placeholder="Issue title" + isRequired + /> + + +
+ {ISSUE_TYPES.map((typeName) => ( + + ))} +
+ + + {labelsLoading ? ( + + ) : ( +
+ {availableLabels.map((label) => { + const checked = selectedLabels.includes(label.name); + return ( + +
+ + toggleLabel(label.name, isChecked) + } + label={ + + + {label.name} + + } + /> +
+ + ); + })} + {availableLabels.length === 0 && ( + + No labels available + + )} +
+ )} + + + {assigneesLoading ? ( + + ) : ( +
+ {availableAssignees.map((assignee) => { + const checked = selectedAssignees.includes(assignee.login); + return ( + + toggleAssignee(assignee.login, isChecked) + } + label={ + + + {assignee.login} + + } + /> + ); + })} + {availableAssignees.length === 0 && ( + + No assignees available + + )} +
+ )} + + + )} + + + setActiveTab(key)} + style={{ marginBottom: '0.5rem' }} + > + Edit}> +
- {issue.body_html ? ( - - ) : null} +
-
- {issue.comments > 0 && ( -
- setIsCommentsExpanded(!isCommentsExpanded)} - isExpanded={isCommentsExpanded} - > -
- {commentsLoading && ( -
- + {issue.body_html ? ( +
+ ) : ( +

+ No description. +

+ )} +
+ setIsCommentsExpanded(!isCommentsExpanded)} + isExpanded={isCommentsExpanded} + > +
+ {commentsLoading && ( +
+ +
+ )} + {commentsError && ( + + {commentsError} + + )} + {!commentsLoading && + !commentsError && + comments.length > 0 && ( +
+ {comments.map((comment) => ( + + ))}
)} - {commentsError && ( - - {commentsError} - + {!commentsLoading && + !commentsError && + comments.length === 0 && + isCommentsExpanded && ( +

+ No comments found. +

)} - {!commentsLoading && - !commentsError && - comments.length > 0 && ( -
- {comments.map((comment) => ( - - ))} -
- )} - {!commentsLoading && - !commentsError && - comments.length === 0 && - isCommentsExpanded && ( -

- No comments found. -

- )} -
- +
+ +
+
- )} +
{issue.reactions?.total_count > 0 && (
{reactionsLoading && ( @@ -1709,6 +1794,24 @@ const IssueCard = ({