Skip to content

Commit f8a1eac

Browse files
committed
fix(ui): addresses PR review findings for badges
- adds globalFilter (org/repo) to tabCounts memo so badges match displayed items - passes per-type ignored count to createReorderHighlight instead of total - makes getIgnoredCount parameter required, removes optional chaining - uses consistent block-form filter style across all tabCounts branches - adds comprehensive reorderHighlight tests covering suppression and resumption - adds tests for PR/run ignore badge decrement and showPrRuns toggle - uses precise digit-extraction assertions for badge count verification
1 parent 1b758a4 commit f8a1eac

File tree

7 files changed

+223
-23
lines changed

7 files changed

+223
-23
lines changed

src/app/components/dashboard/ActionsTab.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ export default function ActionsTab(props: ActionsTabProps) {
211211
const highlightedReposActions = createReorderHighlight(
212212
() => repoGroups().map(g => g.repoFullName),
213213
() => viewState.lockedRepos.actions,
214-
() => viewState.ignoredItems.length,
214+
() => viewState.ignoredItems.filter(i => i.type === "workflowRun").length,
215215
);
216216

217217
return (

src/app/components/dashboard/DashboardPage.tsx

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ export default function DashboardPage() {
324324
const refreshTick = createMemo(() => (dashboardData.lastRefreshedAt?.getTime() ?? 0) + clockTick());
325325

326326
const tabCounts = createMemo(() => {
327+
const { org, repo } = viewState.globalFilter;
327328
const ignoredByType = (type: string) =>
328329
new Set(viewState.ignoredItems.filter((i) => i.type === type).map((i) => i.id));
329330

@@ -335,14 +336,21 @@ export default function DashboardPage() {
335336
issues: dashboardData.issues.filter((i) => {
336337
if (ignoredIssues.has(String(i.id))) return false;
337338
if (viewState.hideDepDashboard && i.title === "Dependency Dashboard") return false;
339+
if (repo && i.repoFullName !== repo) return false;
340+
if (org && !i.repoFullName.startsWith(org + "/")) return false;
341+
return true;
342+
}).length,
343+
pullRequests: dashboardData.pullRequests.filter((p) => {
344+
if (ignoredPRs.has(String(p.id))) return false;
345+
if (repo && p.repoFullName !== repo) return false;
346+
if (org && !p.repoFullName.startsWith(org + "/")) return false;
338347
return true;
339348
}).length,
340-
pullRequests: dashboardData.pullRequests.filter(
341-
(p) => !ignoredPRs.has(String(p.id))
342-
).length,
343349
actions: dashboardData.workflowRuns.filter((w) => {
344350
if (ignoredRuns.has(String(w.id))) return false;
345351
if (!viewState.showPrRuns && w.isPrRun) return false;
352+
if (repo && w.repoFullName !== repo) return false;
353+
if (org && !w.repoFullName.startsWith(org + "/")) return false;
346354
return true;
347355
}).length,
348356
};

src/app/components/dashboard/IssuesTab.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ export default function IssuesTab(props: IssuesTabProps) {
203203
const highlightedReposIssues = createReorderHighlight(
204204
() => repoGroups().map(g => g.repoFullName),
205205
() => viewState.lockedRepos.issues,
206-
() => viewState.ignoredItems.length,
206+
() => viewState.ignoredItems.filter(i => i.type === "issue").length,
207207
);
208208

209209
function handleSort(field: string, direction: "asc" | "desc") {

src/app/components/dashboard/PullRequestsTab.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
301301
const highlightedReposPRs = createReorderHighlight(
302302
() => repoGroups().map(g => g.repoFullName),
303303
() => viewState.lockedRepos.pullRequests,
304-
() => viewState.ignoredItems.length,
304+
() => viewState.ignoredItems.filter(i => i.type === "pullRequest").length,
305305
);
306306

307307
function handleSort(field: string, direction: "asc" | "desc") {

src/app/lib/reorderHighlight.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,18 @@ import { detectReorderedRepos } from "./grouping";
44
export function createReorderHighlight(
55
getRepoOrder: Accessor<string[]>,
66
getLockedOrder: Accessor<string[]>,
7-
getIgnoredCount?: Accessor<number>,
7+
getIgnoredCount: Accessor<number>,
88
): Accessor<ReadonlySet<string>> {
99
let prevOrder: string[] = [];
1010
let prevLocked: string[] = [];
11-
let prevIgnoredCount = getIgnoredCount?.() ?? 0;
11+
let prevIgnoredCount = getIgnoredCount();
1212
let timeout: ReturnType<typeof setTimeout> | undefined;
1313
const [highlighted, setHighlighted] = createSignal<ReadonlySet<string>>(new Set());
1414

1515
createEffect(() => {
1616
const currentOrder = getRepoOrder();
1717
const currentLocked = getLockedOrder();
18-
const currentIgnoredCount = getIgnoredCount?.() ?? 0;
18+
const currentIgnoredCount = getIgnoredCount();
1919

2020
const lockedChanged = currentLocked.length !== prevLocked.length
2121
|| currentLocked.some((r, i) => r !== prevLocked[i]);

tests/components/DashboardPage.test.tsx

Lines changed: 78 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,7 @@ describe("DashboardPage — tab badge counts", () => {
219219
render(() => <DashboardPage />);
220220
await waitFor(() => {
221221
const issuesTab = screen.getByRole("tab", { name: /Issues/ });
222-
expect(issuesTab.textContent).toContain("1");
223-
expect(issuesTab.textContent).not.toContain("3");
222+
expect(issuesTab.textContent?.replace(/\D+/g, "")).toBe("1");
224223
});
225224
});
226225

@@ -238,17 +237,17 @@ describe("DashboardPage — tab badge counts", () => {
238237
render(() => <DashboardPage />);
239238
// hideDepDashboard defaults to true — badge shows 1
240239
await waitFor(() => {
241-
expect(screen.getByRole("tab", { name: /Issues/ }).textContent).toContain("1");
240+
expect(screen.getByRole("tab", { name: /Issues/ }).textContent?.replace(/\D+/g, "")).toBe("1");
242241
});
243242

244243
// Toggle off — badge should update to 2
245244
viewStore.updateViewState({ hideDepDashboard: false });
246245
await waitFor(() => {
247-
expect(screen.getByRole("tab", { name: /Issues/ }).textContent).toContain("2");
246+
expect(screen.getByRole("tab", { name: /Issues/ }).textContent?.replace(/\D+/g, "")).toBe("2");
248247
});
249248
});
250249

251-
it("decrements badge on ignore and increments on un-ignore", async () => {
250+
it("decrements issue badge on ignore and increments on un-ignore", async () => {
252251
vi.mocked(pollService.fetchAllData).mockResolvedValue({
253252
issues: [
254253
makeIssue({ id: 1, title: "Issue A" }),
@@ -262,19 +261,64 @@ describe("DashboardPage — tab badge counts", () => {
262261

263262
render(() => <DashboardPage />);
264263
await waitFor(() => {
265-
expect(screen.getByRole("tab", { name: /Issues/ }).textContent).toContain("2");
264+
expect(screen.getByRole("tab", { name: /Issues/ }).textContent?.replace(/\D+/g, "")).toBe("2");
266265
});
267266

268267
// Ignore one item — badge should decrement to 1
269268
viewStore.ignoreItem({ id: "1", type: "issue", repo: "owner/repo", title: "Issue A", ignoredAt: Date.now() });
270269
await waitFor(() => {
271-
expect(screen.getByRole("tab", { name: /Issues/ }).textContent).toContain("1");
270+
expect(screen.getByRole("tab", { name: /Issues/ }).textContent?.replace(/\D+/g, "")).toBe("1");
272271
});
273272

274273
// Un-ignore — badge should increment back to 2
275274
viewStore.unignoreItem("1");
276275
await waitFor(() => {
277-
expect(screen.getByRole("tab", { name: /Issues/ }).textContent).toContain("2");
276+
expect(screen.getByRole("tab", { name: /Issues/ }).textContent?.replace(/\D+/g, "")).toBe("2");
277+
});
278+
});
279+
280+
it("decrements PR badge on ignore", async () => {
281+
vi.mocked(pollService.fetchAllData).mockResolvedValue({
282+
issues: [],
283+
pullRequests: [
284+
makePullRequest({ id: 10, title: "PR A" }),
285+
makePullRequest({ id: 11, title: "PR B" }),
286+
makePullRequest({ id: 12, title: "PR C" }),
287+
],
288+
workflowRuns: [],
289+
errors: [],
290+
});
291+
292+
render(() => <DashboardPage />);
293+
await waitFor(() => {
294+
expect(screen.getByRole("tab", { name: /Pull Requests/ }).textContent?.replace(/\D+/g, "")).toBe("3");
295+
});
296+
297+
viewStore.ignoreItem({ id: "10", type: "pullRequest", repo: "owner/repo", title: "PR A", ignoredAt: Date.now() });
298+
await waitFor(() => {
299+
expect(screen.getByRole("tab", { name: /Pull Requests/ }).textContent?.replace(/\D+/g, "")).toBe("2");
300+
});
301+
});
302+
303+
it("decrements Actions badge on ignore", async () => {
304+
vi.mocked(pollService.fetchAllData).mockResolvedValue({
305+
issues: [],
306+
pullRequests: [],
307+
workflowRuns: [
308+
makeWorkflowRun({ id: 20, isPrRun: false }),
309+
makeWorkflowRun({ id: 21, isPrRun: false }),
310+
],
311+
errors: [],
312+
});
313+
314+
render(() => <DashboardPage />);
315+
await waitFor(() => {
316+
expect(screen.getByRole("tab", { name: /Actions/ }).textContent?.replace(/\D+/g, "")).toBe("2");
317+
});
318+
319+
viewStore.ignoreItem({ id: "20", type: "workflowRun", repo: "owner/repo", title: "CI", ignoredAt: Date.now() });
320+
await waitFor(() => {
321+
expect(screen.getByRole("tab", { name: /Actions/ }).textContent?.replace(/\D+/g, "")).toBe("1");
278322
});
279323
});
280324

@@ -292,9 +336,32 @@ describe("DashboardPage — tab badge counts", () => {
292336

293337
render(() => <DashboardPage />);
294338
await waitFor(() => {
295-
const actionsTab = screen.getByRole("tab", { name: /Actions/ });
296-
expect(actionsTab.textContent).toContain("1");
297-
expect(actionsTab.textContent).not.toContain("3");
339+
expect(screen.getByRole("tab", { name: /Actions/ }).textContent?.replace(/\D+/g, "")).toBe("1");
340+
});
341+
});
342+
343+
it("includes PR-triggered runs in badge count when showPrRuns is enabled", async () => {
344+
vi.mocked(pollService.fetchAllData).mockResolvedValue({
345+
issues: [],
346+
pullRequests: [],
347+
workflowRuns: [
348+
makeWorkflowRun({ id: 20, isPrRun: false }),
349+
makeWorkflowRun({ id: 21, isPrRun: true }),
350+
makeWorkflowRun({ id: 22, isPrRun: true }),
351+
],
352+
errors: [],
353+
});
354+
355+
render(() => <DashboardPage />);
356+
// Default: showPrRuns=false — badge shows 1
357+
await waitFor(() => {
358+
expect(screen.getByRole("tab", { name: /Actions/ }).textContent?.replace(/\D+/g, "")).toBe("1");
359+
});
360+
361+
// Toggle on — badge should update to 3
362+
viewStore.updateViewState({ showPrRuns: true });
363+
await waitFor(() => {
364+
expect(screen.getByRole("tab", { name: /Actions/ }).textContent?.replace(/\D+/g, "")).toBe("3");
298365
});
299366
});
300367
});

tests/lib/reorderHighlight.test.ts

Lines changed: 128 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import { describe, it, expect } from "vitest";
2-
import { createRoot, createSignal } from "solid-js";
2+
import { createRoot, createSignal, batch, type Accessor } from "solid-js";
33
import { createReorderHighlight } from "../../src/app/lib/reorderHighlight";
44

55
describe("createReorderHighlight", () => {
66
it("returns an accessor that starts as empty set", () => {
77
createRoot((dispose) => {
88
const [order] = createSignal<string[]>(["a", "b"]);
99
const [locked] = createSignal<string[]>([]);
10-
const highlighted = createReorderHighlight(order, locked);
10+
const [ignored] = createSignal(0);
11+
const highlighted = createReorderHighlight(order, locked, ignored);
1112

1213
expect(highlighted()).toBeInstanceOf(Set);
1314
expect(highlighted().size).toBe(0);
@@ -20,12 +21,136 @@ describe("createReorderHighlight", () => {
2021
createRoot((dispose) => {
2122
const [order] = createSignal<string[]>(["a", "b", "c"]);
2223
const [locked] = createSignal<string[]>([]);
23-
const highlighted = createReorderHighlight(order, locked);
24+
const [ignored] = createSignal(0);
25+
const highlighted = createReorderHighlight(order, locked, ignored);
2426

2527
// First render seeds prevOrder — no detection
2628
expect(highlighted().size).toBe(0);
2729
dispose();
2830
});
2931
});
3032

33+
it("highlights reordered repos when ignored count is unchanged", () => {
34+
let highlighted!: Accessor<ReadonlySet<string>>;
35+
let setOrder!: (v: string[]) => void;
36+
let disposeRoot!: () => void;
37+
38+
createRoot((dispose) => {
39+
const [order, _setOrder] = createSignal<string[]>(["a", "b", "c"]);
40+
const [locked] = createSignal<string[]>([]);
41+
const [ignored] = createSignal(0);
42+
setOrder = _setOrder;
43+
highlighted = createReorderHighlight(order, locked, ignored);
44+
disposeRoot = dispose;
45+
});
46+
47+
// Seed complete — reorder outside batch to trigger effect synchronously
48+
setOrder(["c", "a", "b"]);
49+
expect(highlighted().size).toBeGreaterThan(0);
50+
51+
disposeRoot();
52+
});
53+
54+
it("suppresses highlight when ignored count changes simultaneously with reorder", () => {
55+
let highlighted!: Accessor<ReadonlySet<string>>;
56+
let setOrder!: (v: string[]) => void;
57+
let setIgnored!: (v: number) => void;
58+
let disposeRoot!: () => void;
59+
60+
createRoot((dispose) => {
61+
const [order, _setOrder] = createSignal<string[]>(["a", "b", "c"]);
62+
const [locked] = createSignal<string[]>([]);
63+
const [ignored, _setIgnored] = createSignal(0);
64+
setOrder = _setOrder;
65+
setIgnored = _setIgnored;
66+
highlighted = createReorderHighlight(order, locked, ignored);
67+
disposeRoot = dispose;
68+
});
69+
70+
// Reorder AND increment ignored count in single batch — should suppress
71+
batch(() => {
72+
setIgnored(1);
73+
setOrder(["c", "a", "b"]);
74+
});
75+
expect(highlighted().size).toBe(0);
76+
77+
disposeRoot();
78+
});
79+
80+
it("resumes highlighting after an ignored-count-change cycle", () => {
81+
let highlighted!: Accessor<ReadonlySet<string>>;
82+
let setOrder!: (v: string[]) => void;
83+
let setIgnored!: (v: number) => void;
84+
let disposeRoot!: () => void;
85+
86+
createRoot((dispose) => {
87+
const [order, _setOrder] = createSignal<string[]>(["a", "b", "c"]);
88+
const [locked] = createSignal<string[]>([]);
89+
const [ignored, _setIgnored] = createSignal(0);
90+
setOrder = _setOrder;
91+
setIgnored = _setIgnored;
92+
highlighted = createReorderHighlight(order, locked, ignored);
93+
disposeRoot = dispose;
94+
});
95+
96+
// Suppress via batched ignored count change + reorder
97+
batch(() => {
98+
setIgnored(1);
99+
setOrder(["c", "a", "b"]);
100+
});
101+
expect(highlighted().size).toBe(0);
102+
103+
// Next reorder without ignore change — should highlight again
104+
setOrder(["b", "c", "a"]);
105+
expect(highlighted().size).toBeGreaterThan(0);
106+
107+
disposeRoot();
108+
});
109+
110+
it("seeds prevIgnoredCount correctly with non-zero initial value", () => {
111+
let highlighted!: Accessor<ReadonlySet<string>>;
112+
let setOrder!: (v: string[]) => void;
113+
let disposeRoot!: () => void;
114+
115+
createRoot((dispose) => {
116+
const [order, _setOrder] = createSignal<string[]>(["a", "b", "c"]);
117+
const [locked] = createSignal<string[]>([]);
118+
const [ignored] = createSignal(3);
119+
setOrder = _setOrder;
120+
highlighted = createReorderHighlight(order, locked, ignored);
121+
disposeRoot = dispose;
122+
});
123+
124+
// Reorder without changing ignored count (still 3) — should highlight
125+
setOrder(["c", "a", "b"]);
126+
expect(highlighted().size).toBeGreaterThan(0);
127+
128+
disposeRoot();
129+
});
130+
131+
it("suppresses highlight when ignored count decrements (un-ignore)", () => {
132+
let highlighted!: Accessor<ReadonlySet<string>>;
133+
let setOrder!: (v: string[]) => void;
134+
let setIgnored!: (v: number) => void;
135+
let disposeRoot!: () => void;
136+
137+
createRoot((dispose) => {
138+
const [order, _setOrder] = createSignal<string[]>(["a", "b", "c"]);
139+
const [locked] = createSignal<string[]>([]);
140+
const [ignored, _setIgnored] = createSignal(2);
141+
setOrder = _setOrder;
142+
setIgnored = _setIgnored;
143+
highlighted = createReorderHighlight(order, locked, ignored);
144+
disposeRoot = dispose;
145+
});
146+
147+
// Reorder AND decrement ignored count in single batch — should suppress
148+
batch(() => {
149+
setIgnored(1);
150+
setOrder(["c", "a", "b"]);
151+
});
152+
expect(highlighted().size).toBe(0);
153+
154+
disposeRoot();
155+
});
31156
});

0 commit comments

Comments
 (0)