test(usersettings): add integration tests for notification settings route

[dougrathbone]

Description

Adds integration tests for the POST /user/:id/settings/notifications route to provide regression coverage for the unawaited userRepository.save() fixed in #2760.

Prior to #2760, the handler returned 200 before the database write completed. There were no tests asserting the settings were actually persisted to the database, so the race condition went undetected. This suite locks in the expected behavior: changes must be readable from the database after the response returns.

AI Disclosure: I used Claude Code to help write the tests. I reviewed and understood all generated code before submitting.

How Has This Been Tested?

New test suite at server/routes/user/usersettings.test.ts using the real SQLite test database via setupTestDb(), following the same pattern as server/routes/auth.test.ts:

• persists notification settings to the database -- the core regression guard: re-fetches the user from the DB with relations: { settings: true } after the POST and asserts the saved values match what was sent. This test would have caught the unawaited save.
• returns the updated values in the response -- asserts the response body reflects the values that were sent.
• returns 404 for a non-existent user -- asserts the correct error response for a missing user ID.
• returns 403 for a non-owner non-admin trying to update another user settings -- asserts the authorization guard is enforced.

All 4 tests pass.

Screenshots / Logs (if applicable)

N/A -- test-only change with no runtime behaviour change.

Checklist:

• I have read and followed the contribution guidelines.
• Disclosed any use of AI (see our policy)
• I have updated the documentation accordingly. (no documentation changes required for a test-only PR)
• All new and existing tests passed.
• Successful build pnpm build
• Translation keys pnpm i18n:extract (no new UI strings added)
• Database migration (if required) (no schema changes)

Summary by CodeRabbit

• Tests
  • Added an integration test suite for the user notification settings endpoint. Verifies updates persist, response payload reflects changes, returns 404 for missing users, and enforces authorization (403) for non-owners. Includes test setup for authenticated agents and coverage for error-handling behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dffa4c75-3986-46c4-97e5-2c8fceaf57c2

📥 Commits

Reviewing files that changed from the base of the PR and between 0b68aae and 17a3a12.

📒 Files selected for processing (1)

• server/routes/user/usersettings.test.ts

---

📝 Walkthrough

Walkthrough

Adds a new integration test suite for POST /user/:id/settings/notifications, including Express app wiring, session/auth middleware, test DB setup, an authenticated supertest agent helper, and four tests validating persistence, response, 404, and 403 cases.

Changes

Cohort / File(s)	Summary
User Settings Notification Tests server/routes/user/usersettings.test.ts	Adds an integration test file that builds an Express test app (JSON body parsing, express-session, checkUser auth), mounts auth and user routes, registers error-to-JSON middleware, initializes test DB, provides authenticatedAgent() helper, and defines four tests covering persistence, response echo, 404 for missing user, and 403 for unauthorized updates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped in with tests and a nibble of cheer,
Posting notifications so settings appear.
I log in, I assert, I fetch what I wrote,
404s and 403s? I note every vote.
Hooray for green runs and a passing report!

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely summarizes the main change: adding integration tests for a notification settings route, with appropriate test-scoping prefix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/routes/user/usersettings.test.ts`:
- Around line 56-64: The helper authenticatedAgent mutates the singleton
returned by getSettings() by setting settings.main.localLogin = true and never
restores it, leaking state across tests; modify authenticatedAgent to save the
prior value (e.g., const prev = settings.main.localLogin), set it to true for
the login attempt, and then restore settings.main.localLogin = prev in a finally
block (or equivalent) after the POST to '/auth/local' so the global setting is
always returned to its original value even if the request/assert fails.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f86ffac1-e6b4-4441-ba22-9d375050bb1c

📥 Commits

Reviewing files that changed from the base of the PR and between 868430b and 0b68aae.

📒 Files selected for processing (1)

• server/routes/user/usersettings.test.ts

[dougrathbone]

Fixed in the latest commit:

• Settings leak: authenticatedAgent now saves settings.main.localLogin before setting it to true and restores it in a finally block.
• Clear-text cookie (GitHub Advanced Security): added cookie: { secure: false } explicitly to the test session config to make the intent clear -- supertest uses HTTP, not HTTPS, so this is intentional. The flag was always implicitly false; making it explicit also suppresses the finding.