From 60d145657cbc9e465815eb477e0ea93574c263c3 Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Fri, 29 May 2026 11:00:35 -0400 Subject: [PATCH] Sort PR backlog by age, fix copy Assisted-by: Cursor Signed-off-by: David Butenhof --- .../src/github_pm/status_report_service.py | 7 ++- backend/tests/test_status_report_api.py | 52 +++++++++++++++++++ frontend/src/utils/clipboard.js | 36 ++++++++----- frontend/src/utils/clipboard.test.js | 25 +++++++++ 4 files changed, 106 insertions(+), 14 deletions(-) diff --git a/backend/src/github_pm/status_report_service.py b/backend/src/github_pm/status_report_service.py index 0256322..405cdb8 100644 --- a/backend/src/github_pm/status_report_service.py +++ b/backend/src/github_pm/status_report_service.py @@ -221,7 +221,12 @@ def post_gql(payload: dict[str, Any]) -> dict[str, Any]: and not n.get("isDraft") and _updated_strictly_before_start_date(n, start_date) ] - backlog_filtered.sort(key=lambda n: int(n["number"])) + backlog_filtered.sort( + key=lambda n: ( + -_calendar_days_since_update_to_end(n, end_date), + int(n["number"]), + ) + ) backlog_items = [_backlog_item_from_gql_node(n, end_date) for n in backlog_filtered] return ProjectStatusReportResponse( diff --git a/backend/tests/test_status_report_api.py b/backend/tests/test_status_report_api.py index b0ed3a2..fc3f86d 100644 --- a/backend/tests/test_status_report_api.py +++ b/backend/tests/test_status_report_api.py @@ -538,6 +538,58 @@ async def override_conn(): } ] + def test_pr_backlog_sorted_by_age_descending(self, client, mock_connector_graphql): + """PR backlog lists stalest PRs first (days_since_update), not by PR number.""" + mock_connector_graphql.status_backlog_nodes = [ + { + "number": 50, + "title": "Recently stale", + "url": "https://github.com/test/repo/pull/50", + "createdAt": "2025-01-01T10:00:00Z", + "updatedAt": "2025-04-03T12:00:00Z", + "state": "OPEN", + "isDraft": False, + "mergedAt": None, + "additions": 1, + "deletions": 0, + "labels": {"nodes": []}, + "milestone": None, + "author": {"__typename": "User", "login": "u"}, + }, + { + "number": 100, + "title": "Very stale", + "url": "https://github.com/test/repo/pull/100", + "createdAt": "2025-01-01T10:00:00Z", + "updatedAt": "2025-04-01T10:00:00Z", + "state": "OPEN", + "isDraft": False, + "mergedAt": None, + "additions": 1, + "deletions": 0, + "labels": {"nodes": []}, + "milestone": None, + "author": {"__typename": "User", "login": "u"}, + }, + ] + + async def override_conn(): + yield mock_connector_graphql + + app.dependency_overrides[connection] = override_conn + try: + r = client.get( + "/api/v1/project-status", + params={"start_date": "2025-04-04", "end_date": "2025-04-10"}, + ) + finally: + app.dependency_overrides.clear() + + assert r.status_code == 200 + backlog = r.json()["pr_backlog"] + assert [row["number"] for row in backlog] == [100, 50] + assert [row["days_since_update"] for row in backlog] == [9, 7] + def test_opened_prs_exclude_closed_without_merge(self, client): """PRs with GitHub state CLOSED (not merged) must not appear in opened_pull_requests.""" gitctx = MagicMock() diff --git a/frontend/src/utils/clipboard.js b/frontend/src/utils/clipboard.js index 41a50e0..f789fcc 100644 --- a/frontend/src/utils/clipboard.js +++ b/frontend/src/utils/clipboard.js @@ -19,6 +19,14 @@ function escapeHtmlAttr(s) { return String(s).replace(/&/g, '&').replace(/"/g, '"'); } +/** + * @param {string} s + * @returns {string} + */ +function withoutTrailingNewlines(s) { + return s.replace(/[\r\n]+$/, ''); +} + /** * @param {Record} row * @returns {string} @@ -39,17 +47,19 @@ function statusItemAgeSuffix(row) { * @returns {string} */ export function formatStatusSectionClipboardMarkdown(items) { - return (items || []) - .map((row) => { - const title = (row.title != null ? String(row.title) : '').trim(); - const age = statusItemAgeSuffix(row); - const url = (row.html_url != null ? String(row.html_url) : '').trim(); - if (!url) { - return `#${row.number} ${title}${age}`.trim(); - } - return `[#${row.number}](${url}) ${title}${age}`.trim(); - }) - .join('\n'); + return withoutTrailingNewlines( + (items || []) + .map((row) => { + const title = (row.title != null ? String(row.title) : '').trim(); + const age = statusItemAgeSuffix(row); + const url = (row.html_url != null ? String(row.html_url) : '').trim(); + if (!url) { + return `#${row.number} ${title}${age}`.trim(); + } + return `[#${row.number}](${url}) ${title}${age}`.trim(); + }) + .join('\n') + ); } /** @@ -72,7 +82,7 @@ export function formatStatusSectionClipboardHtml(items) { const href = escapeHtmlAttr(url); return `#${row.number} ${title}${age}`; }) - .join('
\n'); + .join('
'); } /** @@ -98,7 +108,7 @@ export async function copyStatusSectionToClipboard(items) { return; } const innerHtml = formatStatusSectionClipboardHtml(items); - const htmlDoc = `
${innerHtml}
`; + const htmlDoc = `${innerHtml}`; if ( typeof navigator !== 'undefined' && diff --git a/frontend/src/utils/clipboard.test.js b/frontend/src/utils/clipboard.test.js index b55482d..615e4ca 100644 --- a/frontend/src/utils/clipboard.test.js +++ b/frontend/src/utils/clipboard.test.js @@ -37,6 +37,23 @@ describe('formatStatusSectionClipboardMarkdown', () => { expect(formatStatusSectionClipboardMarkdown(null)).toBe(''); }); + it('does not end with a trailing newline', () => { + const text = formatStatusSectionClipboardMarkdown([ + { + number: 42, + title: 'Hello world', + html_url: 'https://github.com/o/r/pull/42', + }, + { + number: 7, + title: 'Second', + html_url: 'https://github.com/o/r/issues/7', + }, + ]); + expect(text.endsWith('\n')).toBe(false); + expect(text.endsWith('\r')).toBe(false); + }); + it('appends age suffix for PR backlog rows with days_since_update', () => { expect( formatStatusSectionClipboardMarkdown([ @@ -79,6 +96,14 @@ describe('formatStatusSectionClipboardHtml', () => { ]); expect(html).toContain('T (3 days)'); }); + + it('does not end with a trailing newline', () => { + const html = formatStatusSectionClipboardHtml([ + { number: 1, title: 'A', html_url: 'https://github.com/o/r/pull/1' }, + { number: 2, title: 'B', html_url: 'https://github.com/o/r/pull/2' }, + ]); + expect(html.endsWith('\n')).toBe(false); + }); }); describe('copyTextToClipboard', () => {