Cover both branches of / after the LC-575 dashboard (LC-600) #558

Merged

longjacksonle merged 1 commit from fix/lc600-home-branch-tests into main

2026-07-19 03:00:37 +02:00

longjacksonle commented

2026-07-19 02:51:23 +02:00

Owner

Closes LC-600.

tests/last_visited.rs::home_renders_welcome_when_no_cookie and ::home_welcome_renders_quick_actions had been failing on main. Both asserted the pre-LC-575 onboarding page ("Welcome", "start something new") while building their fixture with a user in #general - and since LC-575 any user with an accessible room gets the dashboard instead. The tests were asserting behaviour that commit deliberately replaced. The production code is correct; the fixtures were stale.

Reaching the empty state is harder than it looks

Skipping the General backfill is not enough. db::chat::list_room_unread_counts treats a room as accessible when r.room_type = 'public' OR rm.user_id IS NOT NULL, and the is_admin branch returns every non-DM room outright - while the migration seeds the public rooms general and random. So on a seeded instance every user has an accessible room, admin or not, member or not.

The in_general: false fixture therefore also drops the user to a plain member and clears the seeded rooms. Without both, no fixture can render the empty state at all. I found this by probing the rendered body rather than assuming, after two wrong guesses about the cause.

Adds the missing branch

home_renders_dashboard_when_user_has_rooms covers the path that actually runs for most users and was previously untested - which is precisely how the stale assertions survived the LC-575 merge. It also asserts the empty state is not rendered, so the two branches cannot silently collapse into one again.

The repeated request/assert/read-body block is extracted into get_home.

Verification

just check clean. cargo test --test last_visited: 7 passed, 0 failed (was 5 passed, 2 failed).

Findings from this work, not fixed here

The LC-372 empty state is nearly unreachable in production, for the same reason it was unreachable in the fixture: a fresh instance always seeds two public rooms, and any public room flips a user to the dashboard. Either the empty state is dead code worth deleting, or show_dashboard should not count rooms the user has never joined. Worth a product decision.
tests/routes_drafts.rs::sidebar_shows_draft_pencil_for_room_with_draft fails on main, and predates this branch - I bisected it to 4018da4, the parent of the LC-598 merge, so it is not from the recent menu work either. Not addressed here.
The lc77_* render fixtures are stale on main. They are golden files that auto-rewrite on mismatch (lc77_webhook_render_fixture.rs:107), so a local full-suite run silently regenerates four files and leaves the tree dirty; the drift includes the LC-553 reaction button, so it has been stale a while. The tests pass either way, which is why nobody noticed. I reverted those regenerated files here to keep this PR scoped.

All three share a root cause worth naming: CI runs only the lib tests and skips the integration binaries, so this class of rot is invisible until someone runs just test locally.

🤖 Generated with Claude Code

https://claude.ai/code/session_01P2Lh6DKB15inZTb5z8miu7

Closes LC-600. `tests/last_visited.rs::home_renders_welcome_when_no_cookie` and `::home_welcome_renders_quick_actions` had been failing on `main`. Both asserted the pre-LC-575 onboarding page (`"Welcome"`, `"start something new"`) while building their fixture with a user in `#general` - and since LC-575 any user with an accessible room gets the dashboard instead. The tests were asserting behaviour that commit deliberately replaced. The production code is correct; the fixtures were stale. ## Reaching the empty state is harder than it looks Skipping the General backfill is not enough. `db::chat::list_room_unread_counts` treats a room as accessible when `r.room_type = 'public' OR rm.user_id IS NOT NULL`, and the `is_admin` branch returns every non-DM room outright - while the migration seeds the *public* rooms `general` and `random`. So on a seeded instance every user has an accessible room, admin or not, member or not. The `in_general: false` fixture therefore also drops the user to a plain member **and** clears the seeded rooms. Without both, no fixture can render the empty state at all. I found this by probing the rendered body rather than assuming, after two wrong guesses about the cause. ## Adds the missing branch `home_renders_dashboard_when_user_has_rooms` covers the path that actually runs for most users and was previously untested - which is precisely how the stale assertions survived the LC-575 merge. It also asserts the empty state is *not* rendered, so the two branches cannot silently collapse into one again. The repeated request/assert/read-body block is extracted into `get_home`. ## Verification `just check` clean. `cargo test --test last_visited`: 7 passed, 0 failed (was 5 passed, 2 failed). ## Findings from this work, not fixed here - **The LC-372 empty state is nearly unreachable in production**, for the same reason it was unreachable in the fixture: a fresh instance always seeds two public rooms, and any public room flips a user to the dashboard. Either the empty state is dead code worth deleting, or `show_dashboard` should not count rooms the user has never joined. Worth a product decision. - **`tests/routes_drafts.rs::sidebar_shows_draft_pencil_for_room_with_draft` fails on `main`**, and predates this branch - I bisected it to `4018da4`, the parent of the LC-598 merge, so it is not from the recent menu work either. Not addressed here. - **The `lc77_*` render fixtures are stale on `main`.** They are golden files that auto-rewrite on mismatch (`lc77_webhook_render_fixture.rs:107`), so a local full-suite run silently regenerates four files and leaves the tree dirty; the drift includes the LC-553 reaction button, so it has been stale a while. The tests pass either way, which is why nobody noticed. I reverted those regenerated files here to keep this PR scoped. All three share a root cause worth naming: CI runs only the lib tests and skips the integration binaries, so this class of rot is invisible until someone runs `just test` locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01P2Lh6DKB15inZTb5z8miu7

longjacksonle added 1 commit

2026-07-19 02:51:24 +02:00

test(home): cover both branches of / after the LC-575 dashboard

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

dfae090354

`tests/last_visited.rs::home_renders_welcome_when_no_cookie` and `::home_welcome_renders_quick_actions` had been failing on main. Both asserted the pre-LC-575 onboarding page ("Welcome", "start something new") while building their fixture with a user in #general - and since LC-575 any user with an accessible room gets the dashboard instead. The tests were asserting behaviour that commit deliberately replaced; the production code was correct.

Reaching the empty state takes more than skipping the General backfill. `db::chat::list_room_unread_counts` treats a room as accessible when `r.room_type = 'public' OR rm.user_id IS NOT NULL`, and admins see every non-DM room outright, while the migration seeds the public rooms `general` and `random`. So the fixture now also drops the user to a plain member and clears the seeded rooms; without both, no fixture can render the empty state at all.

Adds `home_renders_dashboard_when_user_has_rooms` for the branch that actually runs for most users, which was untested - that gap is why the stale assertions survived the LC-575 merge. It also asserts the empty state is *not* rendered, so the two branches cannot silently collapse into one again.

Extracts the repeated request/assert/read-body block into `get_home`.

Worth noting for follow-up: the LC-372 empty state is hard to reach in production too, since a fresh instance always seeds those two public rooms.

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

longjacksonle scheduled this pull request to auto merge when all checks succeed

2026-07-19 02:53:30 +02:00