Skip to content

Commit e81b37c

Browse files
authored
fix(onboarding): freezes org display order after initial sort (#59)
* fix(onboarding): freezes org display order after initial sort * fix(onboarding): addresses quality gate review findings * test(onboarding): implements S5 accordion retry stability test * test(onboarding): adds BDD step definitions for org order stability * fix(test): includes BDD steps in vitest config, fixes cleanup * refactor(test): removes misleading comment and redundant vars in S4 * chore: pins vitest-cucumber to exact version * refactor(test): cleans up BDD step definitions per review * test(onboarding): adds S5 BDD scenario, fixes review gaps * fix(test): strengthens S7 with discriminating org name * refactor(test): deduplicates helpers, adds staggered-load test Hoists getOrgHeaderOrder, getAccordionOrder, and 5 entry fixtures to describe scope. Adds regex metacharacter escaping to both helpers. Adds S8 test for invalidation-trickle-in guard with deferred promises. Documents .steps.tsx BDD convention in CONTRIBUTING.md. Trims verbose comment in RepoSelector.tsx. Fixes stale comment in BDD step file. * chore: regenerates lockfile after vite 8.0.5 rebase
1 parent d98acad commit e81b37c

8 files changed

Lines changed: 1129 additions & 9 deletions

File tree

‎CONTRIBUTING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ pnpm test -- tests/path/to/test.ts
6565

6666
## Testing
6767

68-
Tests live in `tests/` and mirror the `src/` directory structure. Test files end in `.test.ts` or `.test.tsx`.
68+
Tests live in `tests/` and mirror the `src/` directory structure. Test files end in `.test.ts` or `.test.tsx`. BDD step definitions end in `.steps.tsx` and are also picked up by Vitest automatically.
6969

7070
Factory helpers in `tests/helpers/index.tsx` (`makeIssue`, `makePullRequest`, `makeWorkflowRun`) give you typed test fixtures — use them instead of hand-rolling objects.
7171

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
"zod": "4.3.6"
3131
},
3232
"devDependencies": {
33+
"@amiceli/vitest-cucumber": "6.3.0",
3334
"@cloudflare/vite-plugin": "1.30.1",
3435
"@cloudflare/vitest-pool-workers": "0.13.4",
3536
"@playwright/test": "1.58.2",

‎pnpm-lock.yaml‎

Lines changed: 82 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -342,21 +342,53 @@ export default function RepoSelector(props: RepoSelectorProps) {
342342
new Set((props.monitoredRepos ?? []).map((r) => r.fullName))
343343
);
344344

345+
// Plain let — not signals; mutating a signal inside createMemo causes infinite re-evaluation.
346+
let frozenOrder: string[] | null = null;
347+
let frozenOrgsKey = "";
348+
345349
const sortedOrgStates = createMemo(() => {
346350
const states = orgStates();
347-
// Defer sorting until all orgs have loaded: prevents layout shift during
348-
// trickle-in, and ensures each org's type ("user" vs "org") is resolved
349-
// from fetchOrgs before we sort on it. loadedCount is not reset by retryOrg,
350-
// so sorting stays active during retries.
351+
352+
// Invalidate frozen order when org membership changes (key is order-independent).
353+
const currentKey = [...props.selectedOrgs].sort().join(",");
354+
if (currentKey !== frozenOrgsKey) {
355+
frozenOrder = null;
356+
frozenOrgsKey = currentKey;
357+
}
358+
359+
// Replay frozen order if available, appending any orgs not yet in the list.
360+
if (frozenOrder !== null) {
361+
const stateMap = new Map(states.map((s) => [s.org, s]));
362+
const result: OrgRepoState[] = [];
363+
for (const org of frozenOrder) {
364+
const s = stateMap.get(org);
365+
if (s) {
366+
result.push(s);
367+
stateMap.delete(org);
368+
}
369+
}
370+
for (const s of stateMap.values()) result.push(s);
371+
return result;
372+
}
373+
374+
// Defer sorting until all orgs have loaded (prevents layout shift during trickle-in).
351375
if (loadedCount() < props.selectedOrgs.length) return states;
352-
// Order: personal org first, then remaining orgs alphabetically.
353-
// Repos within each org retain their existing recency order from fetchRepos.
354-
return [...states].sort((a, b) => {
376+
377+
// Guard against stale orgStates: the memo runs synchronously when selectedOrgs changes,
378+
// but createEffect resets orgStates asynchronously, so stale entries may still be present.
379+
// Return the stale list (not []) to avoid a flash-to-empty before the effect resets state.
380+
const selectedOrgSet = new Set(props.selectedOrgs);
381+
if (states.some((s) => !selectedOrgSet.has(s.org))) return states;
382+
383+
// Sort: personal org first, then alphabetically. Capture order to freeze it.
384+
const sorted = [...states].sort((a, b) => {
355385
const aIsUser = a.type === "user" ? 0 : 1;
356386
const bIsUser = b.type === "user" ? 0 : 1;
357387
if (aIsUser !== bIsUser) return aIsUser - bIsUser;
358388
return a.org.localeCompare(b.org, "en");
359389
});
390+
frozenOrder = sorted.map((s) => s.org);
391+
return sorted;
360392
});
361393

362394
function toRepoRef(entry: RepoEntry): RepoRef {
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
Feature: Freeze org display order in RepoSelector after initial sort
2+
3+
The RepoSelector component sorts organizations (personal first, then
4+
alphabetical) when all orgs finish loading. After the initial sort, the
5+
order is frozen to prevent visual re-ordering on reactive updates like
6+
repo retries or checkbox toggles. The frozen order is invalidated when
7+
the set of selected organizations changes (e.g., granting access to a
8+
new org or revoking access), triggering a fresh sort. Invalidation uses
9+
a serialized sorted Set comparison, not length, to detect membership
10+
changes even when the org count stays the same.
11+
12+
Background:
13+
Given the user is authenticated with a GitHub account
14+
15+
Scenario: S1 - Org order remains stable after repo retry
16+
Given the RepoSelector displays 3 orgs sorted as "alice", "acme-corp", "beta-org" with beta-org showing a Retry button
17+
When the user clicks the Retry button on "beta-org" and the repos load successfully
18+
Then the org header order remains "alice", "acme-corp", "beta-org"
19+
20+
Scenario: S2 - Org order remains stable when toggling a repo checkbox
21+
Given the RepoSelector displays 3 orgs sorted as "alice", "acme-corp", "beta-org" with all repos loaded
22+
When the user toggles a repo checkbox under "acme-corp"
23+
Then the org header order remains "alice", "acme-corp", "beta-org"
24+
25+
Scenario: S3 - Frozen order invalidated when a new org is granted
26+
Given the RepoSelector displays 2 orgs sorted as "alice", "delta-inc" with order frozen
27+
When the user grants access to a new org "acme-corp" and it finishes loading
28+
Then the org header order becomes "alice", "acme-corp", "delta-inc"
29+
30+
Scenario: S4 - Initial sort applies personal org first
31+
Given the RepoSelector is displayed with 4 orgs "charlie", "acme-corp", "beta-org", "delta-inc" where "charlie" is the personal org
32+
When all orgs finish loading
33+
Then the org header order is "charlie", "acme-corp", "beta-org", "delta-inc"
34+
35+
Scenario: S5 - Org order stable in accordion layout after retry
36+
Given the RepoSelector displays 7 orgs in accordion layout with "alice" first and "echo-labs" showing a Retry button
37+
When the user clicks Retry on "echo-labs" and its repos load successfully
38+
Then the org header order remains "alice", "acme-corp", "beta-org", "charlie-co", "delta-inc", "echo-labs", "foxtrot-io"
39+
And the currently expanded accordion panel remains expanded
40+
41+
Scenario: S6 - New org appears in correct sorted position with 6+ orgs
42+
Given the RepoSelector displays 6 orgs all loaded and sorted with "alice" as the personal org
43+
When the user grants access to a new org "beta-org" and it finishes loading
44+
Then the org header order becomes "alice", "acme-corp", "beta-org", "charlie-co", "delta-inc", "echo-labs", "foxtrot-io"
45+
46+
Scenario: S7 - Frozen order invalidated on simultaneous add and remove
47+
Given the RepoSelector displays 3 orgs sorted as "alice", "acme-corp", "delta-inc" with order frozen
48+
When the user's org access changes so that "delta-inc" is removed and "aaa-org" is added and aaa-org finishes loading
49+
Then the org header order becomes "alice", "aaa-org", "acme-corp"
50+
And "delta-inc" no longer appears in the list

0 commit comments

Comments
 (0)