Skip to content

Commit 62a46a3

Browse files
committed
fix: addresses PR review findings across 22 issues
- refactor(api): extracts executeLightCombinedQuery shared helper, eliminating ~80 lines of duplication between graphqlLightCombinedSearch and graphqlGlobalUserSearch - fix(api): removes superfluous !userLogin from early exit guard - fix(api): guards onLightData to only fire when results exist - perf(api): swaps PR_SEARCH_QUERY to LIGHT_PR_SEARCH_QUERY in discoverUpstreamRepos for lower GraphQL point cost - fix(api): unifies VALID_LOGIN with VALID_TRACKED_LOGIN regex - refactor(dashboard): extracts buildSurfacedByUsers helper, removes duplicated surfacedByBadge logic from IssuesTab and PullRequestsTab - fix(dashboard): guards allUsers memo against empty login - fix(dashboard): adds stale user-filter guard for removed tracked users - fix(settings): renumbers section comments after Tracked Users insertion - refactor(tabs): imports TrackedUser type instead of inline declaration - perf(onboarding): uses Set for manualUpstreamRepos dedup - test: adds ConfigSchema, TrackedUsersSection, RepoSelector, and api-users coverage for identified gaps
1 parent d685817 commit 62a46a3

File tree

12 files changed

+316
-194
lines changed

12 files changed

+316
-194
lines changed

src/app/components/dashboard/DashboardPage.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -307,10 +307,14 @@ export default function DashboardPage() {
307307
}));
308308

309309
const userLogin = createMemo(() => user()?.login ?? "");
310-
const allUsers = createMemo(() => [
311-
{ login: userLogin().toLowerCase(), label: "Me" },
312-
...config.trackedUsers.map((u: TrackedUser) => ({ login: u.login, label: u.login })),
313-
]);
310+
const allUsers = createMemo(() => {
311+
const login = userLogin().toLowerCase();
312+
if (!login) return [];
313+
return [
314+
{ login, label: "Me" },
315+
...config.trackedUsers.map((u: TrackedUser) => ({ login: u.login, label: u.login })),
316+
];
317+
});
314318

315319
return (
316320
<div class="min-h-screen bg-base-200">

src/app/components/dashboard/IssuesTab.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { createEffect, createMemo, createSignal, For, Show } from "solid-js";
2-
import { config } from "../../stores/config";
2+
import { config, type TrackedUser } from "../../stores/config";
33
import { viewState, setSortPreference, setTabFilter, resetTabFilter, resetAllTabFilters, ignoreItem, unignoreItem, toggleExpandedRepo, setAllExpanded, pruneExpandedRepos, type IssueFilterField } from "../../stores/view";
44
import type { Issue } from "../../services/api";
55
import ItemRow from "./ItemRow";
6-
import UserAvatarBadge from "../shared/UserAvatarBadge";
6+
import UserAvatarBadge, { buildSurfacedByUsers } from "../shared/UserAvatarBadge";
77
import IgnoreBadge from "./IgnoreBadge";
88
import SortDropdown from "../shared/SortDropdown";
99
import type { SortOption } from "../shared/SortDropdown";
@@ -22,7 +22,7 @@ export interface IssuesTabProps {
2222
loading?: boolean;
2323
userLogin: string;
2424
allUsers?: { login: string; label: string }[];
25-
trackedUsers?: { login: string; avatarUrl: string; name: string | null }[];
25+
trackedUsers?: TrackedUser[];
2626
}
2727

2828
type SortField = "repo" | "title" | "author" | "createdAt" | "updatedAt" | "comments";
@@ -108,8 +108,11 @@ export default function IssuesTab(props: IssuesTabProps) {
108108
}
109109

110110
if (tabFilter.user !== "all") {
111-
const surfacedBy = issue.surfacedBy ?? [props.userLogin.toLowerCase()];
112-
if (!surfacedBy.includes(tabFilter.user)) return false;
111+
const validUser = !props.allUsers || props.allUsers.some(u => u.login === tabFilter.user);
112+
if (validUser) {
113+
const surfacedBy = issue.surfacedBy ?? [props.userLogin.toLowerCase()];
114+
if (!surfacedBy.includes(tabFilter.user)) return false;
115+
}
113116
}
114117

115118
meta.set(issue.id, { roles });
@@ -319,10 +322,7 @@ export default function IssuesTab(props: IssuesTabProps) {
319322
surfacedByBadge={
320323
props.trackedUsers && props.trackedUsers.length > 0
321324
? <UserAvatarBadge
322-
users={(issue.surfacedBy ?? []).flatMap((login) => {
323-
const u = trackedUserMap().get(login);
324-
return u ? [{ login: u.login, avatarUrl: u.avatarUrl }] : [];
325-
})}
325+
users={buildSurfacedByUsers(issue.surfacedBy, trackedUserMap())}
326326
currentUserLogin={props.userLogin}
327327
/>
328328
: undefined

src/app/components/dashboard/PullRequestsTab.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { createEffect, createMemo, createSignal, For, Show } from "solid-js";
2-
import { config } from "../../stores/config";
2+
import { config, type TrackedUser } from "../../stores/config";
33
import { viewState, setSortPreference, ignoreItem, unignoreItem, setTabFilter, resetTabFilter, resetAllTabFilters, toggleExpandedRepo, setAllExpanded, pruneExpandedRepos, type PullRequestFilterField } from "../../stores/view";
44
import type { PullRequest } from "../../services/api";
55
import { deriveInvolvementRoles, prSizeCategory } from "../../lib/format";
66
import ExpandCollapseButtons from "../shared/ExpandCollapseButtons";
77
import ItemRow from "./ItemRow";
8-
import UserAvatarBadge from "../shared/UserAvatarBadge";
8+
import UserAvatarBadge, { buildSurfacedByUsers } from "../shared/UserAvatarBadge";
99
import StatusDot from "../shared/StatusDot";
1010
import IgnoreBadge from "./IgnoreBadge";
1111
import SortDropdown from "../shared/SortDropdown";
@@ -25,7 +25,7 @@ export interface PullRequestsTabProps {
2525
loading?: boolean;
2626
userLogin: string;
2727
allUsers?: { login: string; label: string }[];
28-
trackedUsers?: { login: string; avatarUrl: string; name: string | null }[];
28+
trackedUsers?: TrackedUser[];
2929
}
3030

3131
type SortField = "repo" | "title" | "author" | "createdAt" | "updatedAt" | "checkStatus" | "reviewDecision" | "size";
@@ -191,8 +191,11 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
191191
}
192192

193193
if (tabFilters.user !== "all") {
194-
const surfacedBy = pr.surfacedBy ?? [props.userLogin.toLowerCase()];
195-
if (!surfacedBy.includes(tabFilters.user)) return false;
194+
const validUser = !props.allUsers || props.allUsers.some(u => u.login === tabFilters.user);
195+
if (validUser) {
196+
const surfacedBy = pr.surfacedBy ?? [props.userLogin.toLowerCase()];
197+
if (!surfacedBy.includes(tabFilters.user)) return false;
198+
}
196199
}
197200

198201
meta.set(pr.id, { roles, sizeCategory });
@@ -463,10 +466,7 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
463466
surfacedByBadge={
464467
props.trackedUsers && props.trackedUsers.length > 0
465468
? <UserAvatarBadge
466-
users={(pr.surfacedBy ?? []).flatMap((login) => {
467-
const u = trackedUserMap().get(login);
468-
return u ? [{ login: u.login, avatarUrl: u.avatarUrl }] : [];
469-
})}
469+
users={buildSurfacedByUsers(pr.surfacedBy, trackedUserMap())}
470470
currentUserLogin={props.userLogin}
471471
/>
472472
: undefined

src/app/components/onboarding/RepoSelector.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -355,9 +355,10 @@ export default function RepoSelector(props: RepoSelectorProps) {
355355
}
356356

357357
// Manually-added upstream repos not in the discovered list
358-
const manualUpstreamRepos = createMemo(() =>
359-
(props.upstreamRepos ?? []).filter(r => !discoveredRepos().some(d => d.fullName === r.fullName))
360-
);
358+
const manualUpstreamRepos = createMemo(() => {
359+
const discoveredSet = new Set(discoveredRepos().map(r => r.fullName));
360+
return (props.upstreamRepos ?? []).filter(r => !discoveredSet.has(r.fullName));
361+
});
361362

362363
// Upstream repos visible in the discovery list (discovered + manually added that aren't org repos)
363364
const filteredDiscovered = createMemo(() => {

src/app/components/settings/SettingsPage.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ export default function SettingsPage() {
378378
</SettingRow>
379379
</Section>
380380

381-
{/* Section 3: GitHub Actions */}
381+
{/* Section 4: GitHub Actions */}
382382
<Section title="GitHub Actions">
383383
<SettingRow
384384
label="Max workflows per repo"
@@ -418,7 +418,7 @@ export default function SettingsPage() {
418418
</SettingRow>
419419
</Section>
420420

421-
{/* Section 4: Notifications */}
421+
{/* Section 5: Notifications */}
422422
<Section title="Notifications">
423423
<SettingRow
424424
label="Enable notifications"
@@ -510,7 +510,7 @@ export default function SettingsPage() {
510510
</SettingRow>
511511
</Section>
512512

513-
{/* Section 5: Appearance */}
513+
{/* Section 6: Appearance */}
514514
<Section title="Appearance">
515515
<div class="px-4 py-2 border-b border-base-300">
516516
<p class="text-sm font-medium text-base-content mb-2">Theme</p>
@@ -550,7 +550,7 @@ export default function SettingsPage() {
550550
</SettingRow>
551551
</Section>
552552

553-
{/* Section 6: Tabs */}
553+
{/* Section 7: Tabs */}
554554
<Section title="Tabs">
555555
<SettingRow
556556
label="Default tab"
@@ -584,7 +584,7 @@ export default function SettingsPage() {
584584
</SettingRow>
585585
</Section>
586586

587-
{/* Section 7: Data */}
587+
{/* Section 8: Data */}
588588
<Section title="Data">
589589
{/* Clear cache */}
590590
<SettingRow

src/app/components/shared/UserAvatarBadge.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
import { createMemo, For, Show } from "solid-js";
22

3+
export function buildSurfacedByUsers(
4+
surfacedBy: string[] | undefined,
5+
trackedUserMap: Map<string, { login: string; avatarUrl: string }>,
6+
): { login: string; avatarUrl: string }[] {
7+
return (surfacedBy ?? []).flatMap((login) => {
8+
const u = trackedUserMap.get(login);
9+
return u ? [{ login: u.login, avatarUrl: u.avatarUrl }] : [];
10+
});
11+
}
12+
313
interface UserAvatarBadgeProps {
414
users: { login: string; avatarUrl: string }[];
515
currentUserLogin: string;

0 commit comments

Comments
 (0)