Wire the Settings email notification toggles to the backend - #1324
Merged
Conversation
The Settings checkboxes were uncontrolled defaultChecked stubs -- toggling persisted nothing and reset on reload. They now load and mutate /v1/notifications/preferences with optimistic rollback. Two categories remain: onboarding updates fan out to the three verification_* types and transfer status maps to ramp_completed -- the stored type strings the dispatch worker consults, pinned by a test because the PUT endpoint accepts any keys without validation. The recipient-approvals toggle is dropped (no such notification type exists yet) and the transfer caption no longer mentions failures.
✅ Deploy Preview for vortexfi ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vrtx-dashboard ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vortex-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
Pull request overview
Wires dashboard email-notification settings to persisted backend preferences.
Changes:
- Adds category-to-notification-type mappings and API helpers.
- Implements optimistic preference updates with rollback.
- Removes the unsupported recipient toggle and updates product documentation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
docs/product-dashboard.md |
Documents the implemented notification settings. |
apps/dashboard/src/services/api/notification-preferences.service.ts |
Adds preference mapping and API methods. |
apps/dashboard/src/services/api/notification-preferences.service.test.ts |
Tests category preference semantics. |
apps/dashboard/src/routes/_app/settings.tsx |
Connects checkboxes to persisted preferences. |
apps/dashboard/src/hooks/useNotificationPreferences.ts |
Manages querying and optimistic updates. |
…contract The dashboard's preference toggles write prefs keyed by these strings and the API's dispatch worker mutes on them, but each side carried its own literals -- the preferences endpoint accepts arbitrary keys, so drift would mute nothing, silently. The enum now lives in shared and the API model re-exports it under its historical NotificationType name.
Review follow-ups on the Settings toggles: a checkbox now reflects emailEnabled AND the per-type keys, and enabling a category under a global mute lifts the switch while pinning the other category to its effective (muted) state. Toggling is blocked until the GET resolves -- a PUT built from a fallback would replace the saved document -- and while a PUT is in flight, since overlapping full-document snapshots can complete out of order. The optimistic handlers are extracted and tested against a real QueryClient (apply, rollback, no-op without loaded data), and the stale operations spec that still declared email dispatch unimplemented now documents the delivery-time preference gating.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (4)
apps/dashboard/src/routes/_app/settings.tsx:16
- The onboarding category now also controls
verification_expired, but its Settings description still says it only covers approval or rejection. A user may leave this enabled without realizing expiry emails are included; update the copy to describe all three outcomes.
const NOTIFICATION_PREFS: Array<{ id: EmailNotificationCategory; label: string; description: string }> = [
{
description: "When a corridor's KYB/KYC is approved or rejected.",
apps/dashboard/src/routes/_app/settings.tsx:79
- The new tests cover the mapping helpers and cache callbacks, but none exercises this actual Settings wiring or the service request. A regression such as removing
onCheckedChange, using the wrong category, or sending the wrong endpoint/body would still pass. Add an interaction/contract test that loads preferences, toggles each checkbox, and verifies the PUT payload plus rollback on failure.
<Checkbox
checked={categoryEnabled(pref.id)}
disabled={controlsDisabled}
id={pref.id}
onCheckedChange={checked => setCategoryEnabled(pref.id, checked === true)}
docs/security-spec/07-operations/notifications.md:61
- This says clients cannot write either table, but the same spec documents client-triggered
POST .../readandread-allmutations of thenotificationstable. Narrow the invariant to row creation so the audit-facing spec does not contradict its endpoint contract.
server-side triggered only; the `email_notifications` queue is unrelated to the
in-app `notifications` table this spec covers, and no client can write either.
apps/dashboard/src/hooks/useNotificationPreferences.ts:62
- When the GET fails,
query.dataremains undefined, so both controls are rendered as checked and disabled regardless of the saved opt-outs, with no indication that loading failed. Avoid presenting an invented enabled state and expose an error state so Settings can distinguish a failed load from real preferences.
const categoryEnabled = (category: EmailNotificationCategory): boolean =>
query.data ? isCategoryEnabled(query.data, category) : true;
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 free
to 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.
The three notification checkboxes on the dashboard Settings page were uncontrolled
defaultCheckedstubs: toggling persisted nothing and reset on reload (an acknowledged gap indocs/product-dashboard.md).They are now wired to
GET/PUT /v1/notifications/preferences, which the email dispatch worker consults at delivery time — so an opt-out applies even to rows already sitting in the queue.Category mapping. The prefs keys the dispatcher checks are the stored notification type strings, and the PUT endpoint accepts arbitrary keys without validation, so a drifted key would mute nothing, silently. The mapping is therefore pinned by a unit test:
verification_approved,verification_rejected,verification_expiredramp_completedA category displays as off when any of its types is muted; re-enabling rewrites all of them, so partial states written by anything else self-heal. Unrelated prefs keys are preserved on write.
Product changes
Testing: new unit tests cover the type-key contract, the opt-out semantics (only explicit
falsemutes), partial-mute display/healing, and foreign-key preservation. 84/84 dashboard unit tests,tsc, and Biome pass. The product spec's acknowledged-gaps and next-steps sections are updated in the same change (the in-app feed remains client-mocked and stays listed).