Drop the DELETE FROM rooms crutch from the onboarding fixture (LC-603) #566

Merged

longjacksonle merged 1 commit from test/lc603-onboarding-reachability into main

2026-07-20 19:21:22 +02:00

longjacksonle commented

2026-07-20 19:14:57 +02:00

Owner

Closes LC-603.

What this is

A test-only change. LC-603 turned out to be already fixed, by LC-604 rather than by any work on this ticket, so the deliverable here is the regression coverage that locks the fix in plus a fixture that stops describing a world that no longer exists.

The original problem

show_dashboard in routes/home.rs is !room_counts.is_empty() || !dm_counts.is_empty(). db::chat::list_room_unread_counts used to count a room as accessible when r.room_type = 'public' OR rm.user_id IS NOT NULL, and migrations/chat/0001_create_tables.sql seeds the public rooms general and random. So on any seeded instance every user had at least one accessible room, always took the dashboard branch, and the LC-372 onboarding page was dead code in practice. A brand-new user landed on five cards reading "No unread channels", "No unread mentions", and so on: five negations and no next step, in place of the quick actions written for exactly that moment.

Why it no longer reproduces

Two changes since. LC-604 replaced the ad-hoc predicate with accessible_rooms_sql, which requires enclave membership before a channel counts as visible, and LC-516 removed the auto-join-to-General behaviour (backfill_general_membership has no callers in server/src any more; it is a test fixture). A fresh non-admin signup is therefore a member of no enclave, the seeded public rooms are not visible to them, and they reach the onboarding page as designed.

What changed here

app_with_user(false) in server/tests/last_visited.rs ran DELETE FROM rooms to force the empty state. That crutch is gone. The fixture now only skips the backfill and leaves the user a plain member, which is precisely the state a real new signup is in, with the seeded rooms left in place.

This matters because with the delete in place the two onboarding tests were asserting against an instance state that never occurs in production. They passed for a reason unrelated to the behaviour they claim to cover.

Verification

Both onboarding tests pass on current main with the seeded rooms present. To confirm they are not vacuous I temporarily restored the pre-LC-604 predicate in accessible_rooms_sql (with accessible_rooms_binds dropped back to 1 to match), and both fail:

test home_renders_welcome_when_no_cookie ... FAILED   should render welcome page
test home_welcome_renders_quick_actions ... FAILED    renders the empty-state subtitle

Restored afterwards; the diff here touches no production code. Full suite green, 181 test binaries, 0 failures. just check clean.

The product question

The ticket asked for a deliberate call on whether a user who can see busy public rooms should still be shown onboarding. The LC-604 scoping answers it without a separate decision: "can see" now means "is a member of the enclave that owns the room". The one remaining case is an enclave member who has joined no individual rooms, who still gets the dashboard. That reads correct to me: they were admitted to a space, and its public channels are legitimately theirs. Flagging it rather than changing it silently.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FhDVMViNHqUbwmfe2aXTv5

Closes LC-603. ## What this is A test-only change. LC-603 turned out to be already fixed, by LC-604 rather than by any work on this ticket, so the deliverable here is the regression coverage that locks the fix in plus a fixture that stops describing a world that no longer exists. ## The original problem `show_dashboard` in `routes/home.rs` is `!room_counts.is_empty() || !dm_counts.is_empty()`. `db::chat::list_room_unread_counts` used to count a room as accessible when `r.room_type = 'public' OR rm.user_id IS NOT NULL`, and `migrations/chat/0001_create_tables.sql` seeds the public rooms `general` and `random`. So on any seeded instance every user had at least one accessible room, always took the dashboard branch, and the LC-372 onboarding page was dead code in practice. A brand-new user landed on five cards reading "No unread channels", "No unread mentions", and so on: five negations and no next step, in place of the quick actions written for exactly that moment. ## Why it no longer reproduces Two changes since. LC-604 replaced the ad-hoc predicate with `accessible_rooms_sql`, which requires enclave membership before a channel counts as visible, and LC-516 removed the auto-join-to-General behaviour (`backfill_general_membership` has no callers in `server/src` any more; it is a test fixture). A fresh non-admin signup is therefore a member of no enclave, the seeded public rooms are not visible to them, and they reach the onboarding page as designed. ## What changed here `app_with_user(false)` in `server/tests/last_visited.rs` ran `DELETE FROM rooms` to force the empty state. That crutch is gone. The fixture now only skips the backfill and leaves the user a plain member, which is precisely the state a real new signup is in, with the seeded rooms left in place. This matters because with the delete in place the two onboarding tests were asserting against an instance state that never occurs in production. They passed for a reason unrelated to the behaviour they claim to cover. ## Verification Both onboarding tests pass on current `main` with the seeded rooms present. To confirm they are not vacuous I temporarily restored the pre-LC-604 predicate in `accessible_rooms_sql` (with `accessible_rooms_binds` dropped back to 1 to match), and both fail: ``` test home_renders_welcome_when_no_cookie ... FAILED should render welcome page test home_welcome_renders_quick_actions ... FAILED renders the empty-state subtitle ``` Restored afterwards; the diff here touches no production code. Full suite green, 181 test binaries, 0 failures. `just check` clean. ## The product question The ticket asked for a deliberate call on whether a user who can see busy public rooms should still be shown onboarding. The LC-604 scoping answers it without a separate decision: "can see" now means "is a member of the enclave that owns the room". The one remaining case is an enclave member who has joined no individual rooms, who still gets the dashboard. That reads correct to me: they were admitted to a space, and its public channels are legitimately theirs. Flagging it rather than changing it silently. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FhDVMViNHqUbwmfe2aXTv5

longjacksonle added 1 commit

2026-07-20 19:14:57 +02:00

test(home): drop the DELETE FROM rooms crutch from the onboarding fixture (LC-603)

check-secrets / Nosey parker (push) Successful in 9s

Details

check-secrets / Kingfisher (push) Successful in 13s

Details

check-secrets / Kingfisher (pull_request) Successful in 7s

Details

check-secrets / TruffleHog (pull_request) Successful in 7s

Details

check-secrets / Nosey parker (pull_request) Successful in 4s

Details

Check / clippy + fmt + tests (pull_request) Successful in 5m11s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

check-secrets / TruffleHog (push) Successful in 11s

Details

ddec0a54ab

LC-603 reported that the LC-372 onboarding empty state was unreachable in production: `show_dashboard` is `!room_counts.is_empty() || !dm_counts.is_empty()`, `list_room_unread_counts` read `room_type = 'public'` as public to the whole instance, and the migration seeds the public rooms `general` and `random`. Every user on a seeded instance therefore had an accessible room and always took the dashboard branch.

That is no longer true, and it was fixed by another ticket rather than this one. LC-604 replaced the ad-hoc predicate with `accessible_rooms_sql`, which requires enclave membership before a channel counts as visible, and LC-516 had already removed the auto-join-to-General behaviour. A fresh non-admin signup is a member of no enclave, so the seeded public rooms are no longer visible to them and they reach the onboarding page as designed.

The fixture still encoded the old world. `app_with_user(false)` ran `DELETE FROM rooms` to force the empty state, which meant the two onboarding tests were asserting against an instance state that never occurs. Removing the delete leaves the fixture describing a real new signup: plain member, no enclave, seeded rooms present.

Verified the tests are not vacuous by temporarily restoring the pre-LC-604 predicate in `accessible_rooms_sql`: `home_renders_welcome_when_no_cookie` and `home_welcome_renders_quick_actions` both fail, and both pass on current `main`. They are now regression coverage for the reachability LC-603 asked about, rather than tests of a synthetic empty database.

No production change. The product question the ticket raised (should a user who can merely see busy public rooms be shown onboarding) is answered by the LC-604 scoping: "can see" now means "is a member of the enclave", which is a reasonable engagement signal on its own.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FhDVMViNHqUbwmfe2aXTv5