Skip to content

Commit f033e0a

Browse files
wgordon17wgordon17
committed
fix: adds createRoot wrapper, avatar CDN check, PR tab tests
1 parent 1c3baa8 commit f033e0a

File tree

3 files changed

+206
-9
lines changed

3 files changed

+206
-9
lines changed

‎src/app/services/poll.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createSignal, createEffect, untrack, onCleanup } from "solid-js";
1+
import { createSignal, createEffect, createRoot, untrack, onCleanup } from "solid-js";
22
import { getClient } from "./github";
33
import { config } from "../stores/config";
44
import { user, onAuthCleared } from "../stores/auth";
@@ -91,14 +91,18 @@ onAuthCleared(resetPollState);
9191
// silently seeds all items (including the new tracked user's) without flooding
9292
// the user with "new item" notifications for pre-existing content.
9393
// Use a flag to skip the initial run (module-level mount).
94+
// Wrapped in createRoot to provide a reactive owner at module scope (per SolidJS gotcha).
95+
// Subscribes to array length only — fires on add/remove, not property mutations.
9496
let _trackedUsersMounted = false;
95-
createEffect(() => {
96-
void (config.trackedUsers?.length ?? 0); // reactive subscription
97-
if (!_trackedUsersMounted) {
98-
_trackedUsersMounted = true;
99-
return;
100-
}
101-
untrack(() => _resetNotificationState());
97+
createRoot(() => {
98+
createEffect(() => {
99+
void (config.trackedUsers?.length ?? 0);
100+
if (!_trackedUsersMounted) {
101+
_trackedUsersMounted = true;
102+
return;
103+
}
104+
untrack(() => _resetNotificationState());
105+
});
102106
});
103107

104108
/**

‎src/app/stores/config.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ export const RepoRefSchema = z.object({
2525

2626
export const TrackedUserSchema = z.object({
2727
login: z.string(),
28-
avatarUrl: z.string().url(),
28+
avatarUrl: z.string().url().refine(
29+
(u) => u.startsWith("https://avatars.githubusercontent.com/"),
30+
"Avatar URL must be from GitHub CDN"
31+
),
2932
name: z.string().nullable(),
3033
});
3134

‎tests/components/dashboard/PullRequestsTab.test.tsx‎

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest";
2+
import { render, screen } from "@solidjs/testing-library";
3+
import { makePullRequest } from "../../helpers/index";
4+
5+
// ── localStorage mock ─────────────────────────────────────────────────────────
6+
7+
const localStorageMock = (() => {
8+
let store: Record<string, string> = {};
9+
return {
10+
getItem: (key: string) => store[key] ?? null,
11+
setItem: (key: string, val: string) => { store[key] = val; },
12+
removeItem: (key: string) => { delete store[key]; },
13+
clear: () => { store = {}; },
14+
};
15+
})();
16+
17+
Object.defineProperty(globalThis, "localStorage", {
18+
value: localStorageMock,
19+
writable: true,
20+
configurable: true,
21+
});
22+
23+
// ── Mocks ─────────────────────────────────────────────────────────────────────
24+
25+
vi.mock("../../../src/app/lib/url", () => ({
26+
isSafeGitHubUrl: () => true,
27+
}));
28+
29+
// ── Imports ───────────────────────────────────────────────────────────────────
30+
31+
import PullRequestsTab from "../../../src/app/components/dashboard/PullRequestsTab";
32+
import { setTabFilter, setAllExpanded, resetViewState } from "../../../src/app/stores/view";
33+
import type { TrackedUser } from "../../../src/app/stores/config";
34+
35+
// ── Setup ─────────────────────────────────────────────────────────────────────
36+
37+
beforeEach(() => {
38+
vi.clearAllMocks();
39+
localStorageMock.clear();
40+
resetViewState();
41+
});
42+
43+
// ── Tests ─────────────────────────────────────────────────────────────────────
44+
45+
describe("PullRequestsTab — user filter chip", () => {
46+
it("does not show User filter chip when allUsers has only 1 entry", () => {
47+
render(() => (
48+
<PullRequestsTab
49+
pullRequests={[makePullRequest()]}
50+
userLogin="me"
51+
allUsers={[{ login: "me", label: "Me" }]}
52+
/>
53+
));
54+
expect(screen.queryByText("User:")).toBeNull();
55+
});
56+
57+
it("shows User filter chip when allUsers has > 1 entry", () => {
58+
render(() => (
59+
<PullRequestsTab
60+
pullRequests={[makePullRequest()]}
61+
userLogin="me"
62+
allUsers={[
63+
{ login: "me", label: "Me" },
64+
{ login: "tracked1", label: "tracked1" },
65+
]}
66+
/>
67+
));
68+
screen.getByText("User:");
69+
});
70+
});
71+
72+
describe("PullRequestsTab — user filter logic", () => {
73+
it("shows all PRs when user filter is 'all'", () => {
74+
const prs = [
75+
makePullRequest({ id: 1, title: "My PR", repoFullName: "owner/repo-a", surfacedBy: ["me"] }),
76+
makePullRequest({ id: 2, title: "Tracked PR", repoFullName: "owner/repo-b", surfacedBy: ["tracked1"] }),
77+
];
78+
setAllExpanded("pullRequests", ["owner/repo-a", "owner/repo-b"], true);
79+
80+
render(() => (
81+
<PullRequestsTab
82+
pullRequests={prs}
83+
userLogin="me"
84+
allUsers={[
85+
{ login: "me", label: "Me" },
86+
{ login: "tracked1", label: "tracked1" },
87+
]}
88+
/>
89+
));
90+
91+
screen.getByText("My PR");
92+
screen.getByText("Tracked PR");
93+
});
94+
95+
it("filters PRs to only the tracked user's items when user filter is set", () => {
96+
const prs = [
97+
makePullRequest({ id: 1, title: "My PR", repoFullName: "owner/repo-a", surfacedBy: ["me"] }),
98+
makePullRequest({ id: 2, title: "Tracked PR", repoFullName: "owner/repo-b", surfacedBy: ["tracked1"] }),
99+
];
100+
101+
setTabFilter("pullRequests", "user", "tracked1");
102+
setAllExpanded("pullRequests", ["owner/repo-a", "owner/repo-b"], true);
103+
104+
render(() => (
105+
<PullRequestsTab
106+
pullRequests={prs}
107+
userLogin="me"
108+
allUsers={[
109+
{ login: "me", label: "Me" },
110+
{ login: "tracked1", label: "tracked1" },
111+
]}
112+
/>
113+
));
114+
115+
expect(screen.queryByText("My PR")).toBeNull();
116+
screen.getByText("Tracked PR");
117+
});
118+
119+
it("uses userLogin as fallback surfacedBy for items with undefined surfacedBy", () => {
120+
const prs = [
121+
makePullRequest({ id: 1, title: "Legacy PR", repoFullName: "owner/repo", surfacedBy: undefined }),
122+
];
123+
124+
setTabFilter("pullRequests", "user", "me");
125+
setAllExpanded("pullRequests", ["owner/repo"], true);
126+
127+
render(() => (
128+
<PullRequestsTab
129+
pullRequests={prs}
130+
userLogin="me"
131+
allUsers={[
132+
{ login: "me", label: "Me" },
133+
{ login: "tracked1", label: "tracked1" },
134+
]}
135+
/>
136+
));
137+
138+
screen.getByText("Legacy PR");
139+
});
140+
141+
it("hides legacy PRs (no surfacedBy) when filtered to tracked user", () => {
142+
const prs = [
143+
makePullRequest({ id: 1, title: "Legacy PR", repoFullName: "owner/repo", surfacedBy: undefined }),
144+
];
145+
146+
setTabFilter("pullRequests", "user", "tracked1");
147+
setAllExpanded("pullRequests", ["owner/repo"], true);
148+
149+
render(() => (
150+
<PullRequestsTab
151+
pullRequests={prs}
152+
userLogin="me"
153+
allUsers={[
154+
{ login: "me", label: "Me" },
155+
{ login: "tracked1", label: "tracked1" },
156+
]}
157+
/>
158+
));
159+
160+
expect(screen.queryByText("Legacy PR")).toBeNull();
161+
});
162+
});
163+
164+
describe("PullRequestsTab — avatar badge", () => {
165+
it("renders avatar img for PRs surfaced by tracked users", () => {
166+
const trackedUsers: TrackedUser[] = [
167+
{ login: "tracked1", avatarUrl: "https://avatars.githubusercontent.com/u/1", name: "Tracked One" },
168+
];
169+
const prs = [
170+
makePullRequest({ id: 1, title: "Tracked PR", repoFullName: "owner/repo", surfacedBy: ["tracked1"] }),
171+
];
172+
173+
setAllExpanded("pullRequests", ["owner/repo"], true);
174+
175+
render(() => (
176+
<PullRequestsTab
177+
pullRequests={prs}
178+
userLogin="me"
179+
trackedUsers={trackedUsers}
180+
allUsers={[
181+
{ login: "me", label: "Me" },
182+
{ login: "tracked1", label: "tracked1" },
183+
]}
184+
/>
185+
));
186+
187+
const img = screen.getByAltText("tracked1");
188+
expect(img.getAttribute("src")).toBe("https://avatars.githubusercontent.com/u/1");
189+
});
190+
});

0 commit comments

Comments
 (0)