Skip to content

Commit c7da56b

Browse files
wgordon17wgordon17
committed
fix: addresses review findings from Phase 4 swarm
1 parent 85317ce commit c7da56b

File tree

11 files changed

+65
-28
lines changed

11 files changed

+65
-28
lines changed

‎src/app/components/dashboard/ActionsTab.tsx‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,13 +231,6 @@ export default function ActionsTab(props: ActionsTabProps) {
231231
</div>
232232
</Show>
233233

234-
{/* Upstream repos exclusion note */}
235-
<Show when={props.hasUpstreamRepos}>
236-
<p class="text-xs text-base-content/40 text-center py-2">
237-
Workflow runs are not tracked for upstream repositories.
238-
</p>
239-
</Show>
240-
241234
{/* Repo groups */}
242235
<Show when={repoGroups().length > 0}>
243236
<For each={repoGroups()}>
@@ -327,6 +320,13 @@ export default function ActionsTab(props: ActionsTabProps) {
327320
}}
328321
</For>
329322
</Show>
323+
324+
{/* Upstream repos exclusion note */}
325+
<Show when={props.hasUpstreamRepos}>
326+
<p class="text-xs text-base-content/40 text-center py-2">
327+
Workflow runs are not tracked for upstream repositories.
328+
</p>
329+
</Show>
330330
</div>
331331
);
332332
}

‎src/app/components/dashboard/DashboardPage.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ export default function DashboardPage() {
308308

309309
const userLogin = createMemo(() => user()?.login ?? "");
310310
const allUsers = createMemo(() => [
311-
{ login: userLogin(), label: "Me" },
311+
{ login: userLogin().toLowerCase(), label: "Me" },
312312
...config.trackedUsers.map((u: TrackedUser) => ({ login: u.login, label: u.login })),
313313
]);
314314

‎src/app/components/dashboard/IssuesTab.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ const sortOptions: SortOption[] = [
5858
export default function IssuesTab(props: IssuesTabProps) {
5959
const [page, setPage] = createSignal(0);
6060

61+
const trackedUserMap = createMemo(() =>
62+
new Map(props.trackedUsers?.map(u => [u.login, u]) ?? [])
63+
);
64+
6165
const filterGroups = createMemo<FilterChipGroupDef[]>(() => {
6266
const users = props.allUsers;
6367
if (!users || users.length <= 1) return issueFilterGroups;
@@ -316,7 +320,7 @@ export default function IssuesTab(props: IssuesTabProps) {
316320
props.trackedUsers && props.trackedUsers.length > 0
317321
? <UserAvatarBadge
318322
users={(issue.surfacedBy ?? []).flatMap((login) => {
319-
const u = props.trackedUsers!.find((t) => t.login === login);
323+
const u = trackedUserMap().get(login);
320324
return u ? [{ login: u.login, avatarUrl: u.avatarUrl }] : [];
321325
})}
322326
currentUserLogin={props.userLogin}

‎src/app/components/dashboard/PullRequestsTab.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ const sortOptions: SortOption[] = [
123123
export default function PullRequestsTab(props: PullRequestsTabProps) {
124124
const [page, setPage] = createSignal(0);
125125

126+
const trackedUserMap = createMemo(() =>
127+
new Map(props.trackedUsers?.map(u => [u.login, u]) ?? [])
128+
);
129+
126130
const filterGroups = createMemo<FilterChipGroupDef[]>(() => {
127131
const users = props.allUsers;
128132
if (!users || users.length <= 1) return prFilterGroups;
@@ -460,7 +464,7 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
460464
props.trackedUsers && props.trackedUsers.length > 0
461465
? <UserAvatarBadge
462466
users={(pr.surfacedBy ?? []).flatMap((login) => {
463-
const u = props.trackedUsers!.find((t) => t.login === login);
467+
const u = trackedUserMap().get(login);
464468
return u ? [{ login: u.login, avatarUrl: u.avatarUrl }] : [];
465469
})}
466470
currentUserLogin={props.userLogin}

‎src/app/components/onboarding/RepoSelector.tsx‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ export default function RepoSelector(props: RepoSelectorProps) {
149149
setDiscoveringUpstream(true);
150150
setDiscoveredRepos([]);
151151
setDiscoveryCapped(false);
152-
// Build exclude set from all org repos + already-selected repos
152+
// Build exclude set from all org repos + already-selected repos + current upstream repos
153153
const allOrgFullNames = new Set<string>();
154154
for (const state of orgStates()) {
155155
for (const repo of state.repos) {
@@ -159,6 +159,9 @@ export default function RepoSelector(props: RepoSelectorProps) {
159159
for (const repo of props.selected) {
160160
allOrgFullNames.add(repo.fullName);
161161
}
162+
for (const repo of props.upstreamRepos ?? []) {
163+
allOrgFullNames.add(repo.fullName);
164+
}
162165
void discoverUpstreamRepos(discoveryClient, currentUser.login, allOrgFullNames)
163166
.then((repos) => {
164167
if (version !== effectVersion) return;
@@ -353,6 +356,11 @@ export default function RepoSelector(props: RepoSelectorProps) {
353356
if (e.key === "Enter") handleManualAdd();
354357
}
355358

359+
// Manually-added upstream repos not in the discovered list
360+
const manualUpstreamRepos = createMemo(() =>
361+
(props.upstreamRepos ?? []).filter(r => !discoveredRepos().some(d => d.fullName === r.fullName))
362+
);
363+
356364
// Upstream repos visible in the discovery list (discovered + manually added that aren't org repos)
357365
const filteredDiscovered = createMemo(() => {
358366
const query = q();
@@ -607,9 +615,9 @@ export default function RepoSelector(props: RepoSelectorProps) {
607615
</Show>
608616

609617
{/* Manually-added upstream repos not in discovered list */}
610-
<Show when={(props.upstreamRepos ?? []).filter(r => !discoveredRepos().some(d => d.fullName === r.fullName)).length > 0}>
618+
<Show when={manualUpstreamRepos().length > 0}>
611619
<div class="flex flex-col gap-1">
612-
<For each={(props.upstreamRepos ?? []).filter(r => !discoveredRepos().some(d => d.fullName === r.fullName))}>
620+
<For each={manualUpstreamRepos()}>
613621
{(repo) => (
614622
<div class="flex items-center gap-2 px-1">
615623
<span class="text-sm flex-1">{repo.fullName}</span>

‎src/app/components/settings/TrackedUsersSection.tsx‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,13 @@ export default function TrackedUsersSection(props: TrackedUsersSectionProps) {
2828
// Check self-tracking
2929
const currentLogin = user()?.login?.toLowerCase();
3030
if (currentLogin && raw === currentLogin) {
31-
setValidationError("Already tracking this user");
31+
setValidationError("Your activity is already included in your dashboard");
32+
return;
33+
}
34+
35+
// Soft cap
36+
if (props.users.length >= 10) {
37+
setValidationError("Maximum of 10 tracked users");
3238
return;
3339
}
3440

@@ -85,7 +91,7 @@ export default function TrackedUsersSection(props: TrackedUsersSectionProps) {
8591
<button
8692
type="button"
8793
onClick={() => void handleAdd()}
88-
disabled={validating()}
94+
disabled={validating() || props.users.length >= 10}
8995
class="btn btn-sm btn-primary"
9096
>
9197
{validating() ? "Adding..." : "Add"}
@@ -148,6 +154,13 @@ export default function TrackedUsersSection(props: TrackedUsersSectionProps) {
148154
cause GitHub rate limiting.
149155
</div>
150156
</Show>
157+
158+
{/* Cap reached message */}
159+
<Show when={props.users.length >= 10}>
160+
<div role="alert" class="alert alert-info text-xs py-2">
161+
Maximum of 10 tracked users
162+
</div>
163+
</Show>
151164
</div>
152165
);
153166
}

‎src/app/components/shared/UserAvatarBadge.tsx‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
1-
import { For, Show } from "solid-js";
1+
import { createMemo, For, Show } from "solid-js";
22

33
interface UserAvatarBadgeProps {
44
users: { login: string; avatarUrl: string }[];
55
currentUserLogin: string;
66
}
77

88
export default function UserAvatarBadge(props: UserAvatarBadgeProps) {
9-
const trackedUsers = () =>
9+
const trackedUsers = createMemo(() =>
1010
props.users.filter(
1111
(u) => u.login.toLowerCase() !== props.currentUserLogin.toLowerCase()
12-
);
12+
)
13+
);
1314

1415
return (
1516
<Show when={trackedUsers().length > 0}>
16-
<div class="flex items-center">
17+
<div
18+
class="flex items-center"
19+
aria-label={`Surfaced by: ${trackedUsers().map(u => u.login).join(", ")}`}
20+
>
1721
<For each={trackedUsers()}>
1822
{(u, i) => (
1923
<div

‎src/app/services/api.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,7 +1318,9 @@ function mergeTrackedUserResults(
13181318
for (const issue of trackedResult.issues) {
13191319
const existing = issueMap.get(issue.id);
13201320
if (existing) {
1321-
existing.surfacedBy = [...(existing.surfacedBy ?? []), login];
1321+
if (!existing.surfacedBy?.includes(login)) {
1322+
existing.surfacedBy = [...(existing.surfacedBy ?? []), login];
1323+
}
13221324
} else {
13231325
issueMap.set(issue.id, { ...issue, surfacedBy: [login] });
13241326
}
@@ -1327,7 +1329,9 @@ function mergeTrackedUserResults(
13271329
for (const pr of trackedResult.pullRequests) {
13281330
const existing = prMap.get(pr.id);
13291331
if (existing) {
1330-
existing.surfacedBy = [...(existing.surfacedBy ?? []), login];
1332+
if (!existing.surfacedBy?.includes(login)) {
1333+
existing.surfacedBy = [...(existing.surfacedBy ?? []), login];
1334+
}
13311335
} else {
13321336
prMap.set(pr.id, { ...pr, surfacedBy: [login] });
13331337
// Register node ID for backfill if this PR is new
@@ -2096,7 +2100,7 @@ export async function validateGitHubUser(
20962100
: AVATAR_FALLBACK;
20972101

20982102
return {
2099-
login: raw.login,
2103+
login: raw.login.toLowerCase(),
21002104
avatarUrl,
21012105
name: raw.name ?? null,
21022106
};
@@ -2142,9 +2146,9 @@ export async function discoverUpstreamRepos(
21422146
),
21432147
]);
21442148

2145-
for (const err of errors) {
2149+
if (errors.length > 0) {
21462150
pushNotification(
2147-
`upstream-discovery:${err.repo}`,
2151+
"upstream-discovery",
21482152
`Upstream repo discovery partial failure — some repositories may be missing`,
21492153
"warning"
21502154
);

‎src/app/stores/config.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export const ConfigSchema = z.object({
3535
selectedOrgs: z.array(z.string()).default([]),
3636
selectedRepos: z.array(RepoRefSchema).default([]),
3737
upstreamRepos: z.array(RepoRefSchema).default([]),
38-
trackedUsers: z.array(TrackedUserSchema).default([]),
38+
trackedUsers: z.array(TrackedUserSchema).max(10).default([]),
3939
refreshInterval: z.number().min(0).max(3600).default(300),
4040
hotPollInterval: z.number().min(10).max(120).default(30),
4141
maxWorkflowsPerRepo: z.number().min(1).max(20).default(5),

‎tests/components/settings/TrackedUsersSection.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ describe("TrackedUsersSection — adding a user", () => {
215215
await user.type(screen.getByRole("textbox", { name: /github username/i }), "CurrentUser");
216216
await user.click(screen.getByRole("button", { name: /add/i }));
217217

218-
screen.getByText("Already tracking this user");
218+
screen.getByText("Your activity is already included in your dashboard");
219219
expect(onSave).not.toHaveBeenCalled();
220220
expect(apiModule.validateGitHubUser).not.toHaveBeenCalled();
221221
});

0 commit comments

Comments
 (0)