Skip to content

Commit d685817

Browse files
committed
feat(onboarding): sorts org groups by personal-first then alphabetical
Replaces activity-based org sorting in RepoSelector with personal org first (type user), then remaining orgs alphabetically. Upstream section remains structurally last. Repos within orgs retain recency order from fetchRepos.
1 parent 49864f2 commit d685817

File tree

2 files changed

+41
-39
lines changed

2 files changed

+41
-39
lines changed

src/app/components/onboarding/RepoSelector.tsx

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -223,20 +223,18 @@ export default function RepoSelector(props: RepoSelectorProps) {
223223

224224
const sortedOrgStates = createMemo(() => {
225225
const states = orgStates();
226-
// Defer sorting during initial load to prevent layout shift as orgs trickle in.
227-
// After initial load (all orgs resolved), sorting stays active during retries
228-
// because loadedCount is not reset by retryOrg.
226+
// Defer sorting until all orgs have loaded: prevents layout shift during
227+
// trickle-in, and ensures each org's type ("user" vs "org") is resolved
228+
// from fetchOrgs before we sort on it. loadedCount is not reset by retryOrg,
229+
// so sorting stays active during retries.
229230
if (loadedCount() < props.selectedOrgs.length) return states;
230-
const maxPushedAt = new Map(
231-
states.map((s) => [
232-
s.org,
233-
s.repos.reduce((max, r) => r.pushedAt && r.pushedAt > max ? r.pushedAt : max, ""),
234-
])
235-
);
231+
// Order: personal org first, then remaining orgs alphabetically.
232+
// Repos within each org retain their existing recency order from fetchRepos.
236233
return [...states].sort((a, b) => {
237-
const aMax = maxPushedAt.get(a.org) ?? "";
238-
const bMax = maxPushedAt.get(b.org) ?? "";
239-
return aMax > bMax ? -1 : aMax < bMax ? 1 : 0;
234+
const aIsUser = a.type === "user" ? 0 : 1;
235+
const bIsUser = b.type === "user" ? 0 : 1;
236+
if (aIsUser !== bIsUser) return aIsUser - bIsUser;
237+
return a.org.localeCompare(b.org, "en");
240238
});
241239
});
242240

tests/components/onboarding/RepoSelector.test.tsx

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -233,27 +233,27 @@ describe("RepoSelector", () => {
233233
});
234234
});
235235

236-
it("sorts org groups by most recent activity", async () => {
237-
const staleRepos: RepoEntry[] = [
238-
{ owner: "stale-org", name: "old-repo", fullName: "stale-org/old-repo", pushedAt: "2025-01-01T00:00:00Z" },
239-
];
240-
const activeRepos: RepoEntry[] = [
241-
{ owner: "active-org", name: "new-repo", fullName: "active-org/new-repo", pushedAt: "2026-03-23T00:00:00Z" },
242-
];
236+
it("shows personal org first, then remaining orgs alphabetically", async () => {
237+
vi.mocked(api.fetchOrgs).mockResolvedValue([
238+
{ login: "zebra-org", avatarUrl: "", type: "org" },
239+
{ login: "testuser", avatarUrl: "", type: "user" },
240+
{ login: "alpha-org", avatarUrl: "", type: "org" },
241+
]);
243242
vi.mocked(api.fetchRepos).mockImplementation((_client, org) => {
244-
if (org === "stale-org") return Promise.resolve(staleRepos);
245-
return Promise.resolve(activeRepos);
243+
return Promise.resolve([
244+
{ owner: org as string, name: "repo", fullName: `${org}/repo`, pushedAt: "2026-03-20T00:00:00Z" },
245+
]);
246246
});
247247
render(() => (
248-
<RepoSelector selectedOrgs={["stale-org", "active-org"]} selected={[]} onChange={vi.fn()} />
248+
<RepoSelector selectedOrgs={["zebra-org", "testuser", "alpha-org"]} selected={[]} onChange={vi.fn()} />
249249
));
250250
await waitFor(() => {
251-
screen.getByText("old-repo");
252-
screen.getByText("new-repo");
251+
expect(screen.getAllByText("repo").length).toBe(3);
253252
});
254-
const orgHeaders = screen.getAllByText(/^(active-org|stale-org)$/);
255-
expect(orgHeaders[0].textContent).toBe("active-org");
256-
expect(orgHeaders[1].textContent).toBe("stale-org");
253+
const orgHeaders = screen.getAllByText(/^(testuser|alpha-org|zebra-org)$/);
254+
expect(orgHeaders[0].textContent).toBe("testuser");
255+
expect(orgHeaders[1].textContent).toBe("alpha-org");
256+
expect(orgHeaders[2].textContent).toBe("zebra-org");
257257
});
258258

259259
it("does not show timestamp for repos with null pushedAt", async () => {
@@ -295,28 +295,32 @@ describe("RepoSelector", () => {
295295
}
296296
});
297297

298-
it("preserves org order when all repos have null pushedAt", async () => {
299-
const nullOrg1: RepoEntry[] = [
300-
{ owner: "stale-org", name: "null-repo-1", fullName: "stale-org/null-repo-1", pushedAt: null },
298+
it("sorts orgs alphabetically regardless of repo activity", async () => {
299+
vi.mocked(api.fetchOrgs).mockResolvedValue([
300+
{ login: "stale-org", avatarUrl: "", type: "org" },
301+
{ login: "active-org", avatarUrl: "", type: "org" },
302+
]);
303+
const staleRepos: RepoEntry[] = [
304+
{ owner: "stale-org", name: "old-repo", fullName: "stale-org/old-repo", pushedAt: "2025-01-01T00:00:00Z" },
301305
];
302-
const nullOrg2: RepoEntry[] = [
303-
{ owner: "active-org", name: "null-repo-2", fullName: "active-org/null-repo-2", pushedAt: null },
306+
const activeRepos: RepoEntry[] = [
307+
{ owner: "active-org", name: "new-repo", fullName: "active-org/new-repo", pushedAt: "2026-03-23T00:00:00Z" },
304308
];
305309
vi.mocked(api.fetchRepos).mockImplementation((_client, org) => {
306-
if (org === "stale-org") return Promise.resolve(nullOrg1);
307-
return Promise.resolve(nullOrg2);
310+
if (org === "stale-org") return Promise.resolve(staleRepos);
311+
return Promise.resolve(activeRepos);
308312
});
309313
render(() => (
310314
<RepoSelector selectedOrgs={["stale-org", "active-org"]} selected={[]} onChange={vi.fn()} />
311315
));
312316
await waitFor(() => {
313-
screen.getByText("null-repo-1");
314-
screen.getByText("null-repo-2");
317+
screen.getByText("old-repo");
318+
screen.getByText("new-repo");
315319
});
316320
const orgHeaders = screen.getAllByText(/^(active-org|stale-org)$/);
317-
// Both have null pushedAt → comparator returns 0 → original order preserved
318-
expect(orgHeaders[0].textContent).toBe("stale-org");
319-
expect(orgHeaders[1].textContent).toBe("active-org");
321+
// Alphabetical: active-org before stale-org, regardless of pushedAt
322+
expect(orgHeaders[0].textContent).toBe("active-org");
323+
expect(orgHeaders[1].textContent).toBe("stale-org");
320324
});
321325

322326
it("each org group has a scrollable region with aria-label", async () => {

0 commit comments

Comments
 (0)