Skip to content

Commit 6bc382f

Browse files
committed
refactor: extracts isUserInvolved, documents approximations
1 parent 1d40198 commit 6bc382f

File tree

5 files changed

+82
-26
lines changed

5 files changed

+82
-26
lines changed

src/app/components/dashboard/IssuesTab.tsx

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import SkeletonRows from "../shared/SkeletonRows";
1414
import ChevronIcon from "../shared/ChevronIcon";
1515
import ExpandCollapseButtons from "../shared/ExpandCollapseButtons";
1616
import { deriveInvolvementRoles, formatStarCount } from "../../lib/format";
17-
import { groupByRepo, computePageLayout, slicePageGroups, orderRepoGroups } from "../../lib/grouping";
17+
import { groupByRepo, computePageLayout, slicePageGroups, orderRepoGroups, isUserInvolved } from "../../lib/grouping";
1818
import { createReorderHighlight } from "../../lib/reorderHighlight";
1919
import RepoLockControls from "../shared/RepoLockControls";
2020
import RepoGitHubLink from "../shared/RepoGitHubLink";
@@ -103,16 +103,8 @@ export default function IssuesTab(props: IssuesTabProps) {
103103
}
104104
});
105105

106-
function isInvolvedItem(item: Issue): boolean {
107-
const login = userLoginLower();
108-
const surfacedBy = item.surfacedBy ?? [];
109-
if (surfacedBy.length > 0) return surfacedBy.includes(login);
110-
if (monitoredRepoNameSet().has(item.repoFullName)) {
111-
return item.userLogin.toLowerCase() === login ||
112-
item.assigneeLogins.some((a) => a.toLowerCase() === login);
113-
}
114-
return true;
115-
}
106+
const isInvolvedItem = (item: Issue) =>
107+
isUserInvolved(item, userLoginLower(), monitoredRepoNameSet());
116108

117109
const sortPref = createMemo(() => {
118110
const pref = viewState.sortPreferences["issues"];

src/app/components/dashboard/PersonalSummaryStrip.tsx

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,18 @@ export default function PersonalSummaryStrip(props: PersonalSummaryStripProps) {
8787
const { prsAwaitingReview, prsReadyToMerge, prsBlocked } = prCounts();
8888
const running = runningActions();
8989
const items: SummaryCount[] = [];
90-
// Summary counts from unfiltered data — set scope=all so the filtered view
91-
// matches. The specific filters (role, checkStatus) already ensure relevance.
90+
// ── Count-to-filter contract ──
91+
// Counts are computed from unfiltered data (ignoring scope, globalFilter, showPrRuns).
92+
// Click filters set scope=all so tabs don't hide items the count included.
93+
// Known approximations (single-value filter system cannot express these):
94+
// - "ready to merge": count requires reviewDecision=APPROVED||null, but filter
95+
// can't express OR — PRs with CHANGES_REQUESTED + passing CI may appear
96+
// - "awaiting review": count excludes self-authored PRs (!isAuthor), but
97+
// role=reviewer filter includes them if user is both author+reviewer (rare)
98+
// - globalFilter (org/repo) is NOT applied here — counts are persistent
99+
// awareness across all repos, matching the tab badge behavior
100+
// - "running": count includes all in_progress runs; click enables showPrRuns
101+
// so PR-triggered runs are visible in the tab
92102
if (assignedIssues > 0) items.push({
93103
label: assignedIssues === 1 ? "issue assigned" : "issues assigned",
94104
count: assignedIssues,

src/app/components/dashboard/PullRequestsTab.tsx

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import SizeBadge from "../shared/SizeBadge";
1818
import RoleBadge from "../shared/RoleBadge";
1919
import SkeletonRows from "../shared/SkeletonRows";
2020
import ChevronIcon from "../shared/ChevronIcon";
21-
import { groupByRepo, computePageLayout, slicePageGroups, orderRepoGroups } from "../../lib/grouping";
21+
import { groupByRepo, computePageLayout, slicePageGroups, orderRepoGroups, isUserInvolved } from "../../lib/grouping";
2222
import { createReorderHighlight } from "../../lib/reorderHighlight";
2323
import { createFlashDetection } from "../../lib/flashDetection";
2424
import RepoLockControls from "../shared/RepoLockControls";
@@ -172,17 +172,9 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
172172
}
173173
});
174174

175-
function isInvolvedItem(item: PullRequest): boolean {
176-
const login = userLoginLower();
177-
const surfacedBy = item.surfacedBy ?? [];
178-
if (surfacedBy.length > 0) return surfacedBy.includes(login);
179-
if (monitoredRepoNameSet().has(item.repoFullName)) {
180-
return item.userLogin.toLowerCase() === login ||
181-
item.assigneeLogins.some(a => a.toLowerCase() === login) ||
182-
(item.enriched !== false && item.reviewerLogins.some(r => r.toLowerCase() === login));
183-
}
184-
return true;
185-
}
175+
const isInvolvedItem = (item: PullRequest) =>
176+
isUserInvolved(item, userLoginLower(), monitoredRepoNameSet(),
177+
item.enriched !== false ? item.reviewerLogins : undefined);
186178

187179
const sortPref = createMemo(() => {
188180
const pref = viewState.sortPreferences["pullRequests"];

src/app/lib/grouping.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,31 @@ export function orderRepoGroups<G extends { repoFullName: string }>(
7373
return [...locked, ...unlocked];
7474
}
7575

76+
/**
77+
* Three-tier involvement check for scope filtering.
78+
* Shared by IssuesTab and PullRequestsTab — keep both call sites in sync.
79+
*
80+
* Tier 1: surfacedBy annotation present → check if user is included
81+
* Tier 2: monitored repo (no surfacedBy) → field-based fallback (author/assignee)
82+
* Pass reviewerLogins for PRs (only when enriched — unenriched PRs have [])
83+
* Tier 3: non-monitored, no surfacedBy → pass (fetched via involves:{user})
84+
*/
85+
export function isUserInvolved(
86+
item: { repoFullName: string; userLogin: string; assigneeLogins: string[]; surfacedBy?: string[] },
87+
login: string,
88+
monitoredRepos: ReadonlySet<string>,
89+
reviewerLogins?: string[],
90+
): boolean {
91+
const surfacedBy = item.surfacedBy ?? [];
92+
if (surfacedBy.length > 0) return surfacedBy.includes(login);
93+
if (monitoredRepos.has(item.repoFullName)) {
94+
return item.userLogin.toLowerCase() === login ||
95+
item.assigneeLogins.some(a => a.toLowerCase() === login) ||
96+
(reviewerLogins != null && reviewerLogins.some(r => r.toLowerCase() === login));
97+
}
98+
return true;
99+
}
100+
76101
export function detectReorderedRepos(
77102
previousOrder: string[],
78103
currentOrder: string[]

tests/lib/grouping.test.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect } from "vitest";
2-
import { groupByRepo, computePageLayout, slicePageGroups, type RepoGroup } from "../../src/app/lib/grouping";
2+
import { groupByRepo, computePageLayout, slicePageGroups, isUserInvolved, type RepoGroup } from "../../src/app/lib/grouping";
33

44
interface Item {
55
repoFullName: string;
@@ -59,6 +59,43 @@ describe("groupByRepo", () => {
5959
});
6060
});
6161

62+
describe("isUserInvolved", () => {
63+
const base = { repoFullName: "org/repo", userLogin: "author", assigneeLogins: [] as string[] };
64+
const monitored = new Set(["org/monitored"]);
65+
66+
it("returns true when surfacedBy includes user", () => {
67+
expect(isUserInvolved({ ...base, surfacedBy: ["me"] }, "me", monitored)).toBe(true);
68+
});
69+
70+
it("returns false when surfacedBy excludes user", () => {
71+
expect(isUserInvolved({ ...base, surfacedBy: ["other"] }, "me", monitored)).toBe(false);
72+
});
73+
74+
it("returns true for non-monitored item with no surfacedBy (fetched via involves:{user})", () => {
75+
expect(isUserInvolved(base, "me", monitored)).toBe(true);
76+
});
77+
78+
it("returns true for monitored repo item when user is author", () => {
79+
expect(isUserInvolved({ ...base, repoFullName: "org/monitored", userLogin: "me" }, "me", monitored)).toBe(true);
80+
});
81+
82+
it("returns true for monitored repo item when user is assignee", () => {
83+
expect(isUserInvolved({ ...base, repoFullName: "org/monitored", assigneeLogins: ["me"] }, "me", monitored)).toBe(true);
84+
});
85+
86+
it("returns false for monitored repo item when user is not author/assignee", () => {
87+
expect(isUserInvolved({ ...base, repoFullName: "org/monitored" }, "me", monitored)).toBe(false);
88+
});
89+
90+
it("returns true for monitored repo item when user is in reviewerLogins", () => {
91+
expect(isUserInvolved({ ...base, repoFullName: "org/monitored" }, "me", monitored, ["me"])).toBe(true);
92+
});
93+
94+
it("does not check reviewerLogins when not provided", () => {
95+
expect(isUserInvolved({ ...base, repoFullName: "org/monitored" }, "me", monitored)).toBe(false);
96+
});
97+
});
98+
6299
describe("computePageLayout", () => {
63100
it("returns single page for empty groups", () => {
64101
const result = computePageLayout([], 10);

0 commit comments

Comments
 (0)