Cover NotificationSettings component wiring, incl. status-slot cross-wiring - #810
Merged
Merged
Conversation
…wiring PR #805 covered the useNotificationSettings hook; the component that wires it into the browser/Slack sections had no direct unit coverage. Add a component test that renders NotificationSettings with only its api + web-notifications seams mocked, asserting the wiring rather than re-testing hook logic: - Save gates on hasChanges and reflects the transient saving state. - A browser-notification toggle drives the browser-settings POST and its saved message renders in the Browser section. - webMessage/webError render in the Browser section and message/error in the Slack section — neither status slot cross-wires into the other (the silent bug a mis-wired prop would produce). - The enable toggle, event toggles, and test button stay behind the granted permission gate, including when a saved enabled flag outlives its permission. Verified non-vacuous against 6 source mutations (two cross-wire swaps, Save-ignores-hasChanges, checkbox/test-button permission gates) — all caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Recurring test-enforcer run. The local suite was green on the first pass (check clean; server 2292 pass / 8 skip, web 268, browser-ext 51; E2E 169 pass / 12 skip), the flakes list was empty, and the CI scan surfaced no real flakes. So this run spends its effort on the highest-value missing coverage per the carried-forward
next_focus.PR #805 covered the
useNotificationSettingshook, but theNotificationSettingscomponent that wires it into the browser/Slack sections had no direct unit coverage — it was exercised only through E2E. This adds a focused component test that renders the real hook with only itsapi+web-notificationsseams mocked, and asserts the wiring rather than re-testing hook logic.Tests added (
apps/web/src/components/app/notification-settings.test.tsx, 7 tests)Saving.../disabled state, and re-baselines back to disabled after the server response (asserting the real POST payload, not the mock's echo).webErroronly in the Browser section; a Slack-save failure surfaceserroronly in the Slack section. A mis-wired prop (webError/errorswapped between sections) is the exact silent bug these guard against.Verified non-vacuous against 6 source mutations (two cross-wire swaps, Save-ignores-hasChanges, checkbox permission gate, test-button permission gate) — all caught.
CI-scan note
The only recent CI failures were 4 pre-merge iterations of PR #808 (
agent-crud.spec.ts:210pins deep-equality), fixed by the author before merge — deterministic (pins are written in a single synchronous INSERT), not a flake. No new flake entries were recorded.Validation
pnpm run check,pnpm run finalize:web, and the full unit suite (web 268 → 275) all green. No product code changed, so E2E is unaffected (169 pass / 12 skip at the start of the run).🤖 Generated with Claude Code