fix(dashboard): persist notification preferences after save (#824)
* fix(dashboard): persist notification preferences after save Personal notification toggles were reset when a stale GET response arrived after save. Org toggles also triggered a spurious org save because change detection compared against stale currentOrg settings, and the UI reset because currentOrg was not updated after org save. Co-authored-by: Rotimi Best <[email protected]> * refactor(dashboard): send only changed settings key and tidy load guard The org update endpoint deep-merges settings server-side, so spreading local settings into the payload risked writing stale values over unrelated keys. Also drop a redundant load-version bump and let only the latest in-flight preferences load clear the loading flag. Co-authored-by: Rotimi Best <[email protected]> * refactor(dashboard): hydrate org notification defaults on org id transition The sync effect read orgHasChanges and savedOrgToggles while writing them through applyOrgToggleState, causing an extra effect pass after hydration. /account returns settings together with the org id and role, so gating hydration on org id transitions is sufficient and also discards unsaved edits when switching orgs instead of saving them against the wrong org. Addresses CodeRabbit review on PR #824. Co-authored-by: Rotimi Best <[email protected]> ---------
This commit is contained in:
co-authored by
Rotimi Best
parent
8af0675895
commit
dc55f936a1
@@ -16,8 +16,6 @@
|
|||||||
import { Switch } from '@cio/ui/base/switch';
|
import { Switch } from '@cio/ui/base/switch';
|
||||||
import BellIcon from '@lucide/svelte/icons/bell';
|
import BellIcon from '@lucide/svelte/icons/bell';
|
||||||
import Building2Icon from '@lucide/svelte/icons/building-2';
|
import Building2Icon from '@lucide/svelte/icons/building-2';
|
||||||
import { onMount } from 'svelte';
|
|
||||||
import { get } from 'svelte/store';
|
|
||||||
|
|
||||||
function buildToggleState<T extends string>(keys: readonly T[], source?: Partial<Record<T, boolean>> | null) {
|
function buildToggleState<T extends string>(keys: readonly T[], source?: Partial<Record<T, boolean>> | null) {
|
||||||
return Object.fromEntries(keys.map((key) => [key, source?.[key] !== false])) as Record<T, boolean>;
|
return Object.fromEntries(keys.map((key) => [key, source?.[key] !== false])) as Record<T, boolean>;
|
||||||
@@ -26,10 +24,12 @@
|
|||||||
let personalToggles = $state(buildToggleState(PERSONAL_EMAIL_NOTIFICATION_TOGGLE_KEYS));
|
let personalToggles = $state(buildToggleState(PERSONAL_EMAIL_NOTIFICATION_TOGGLE_KEYS));
|
||||||
let savedPersonalToggles = $state(buildToggleState(PERSONAL_EMAIL_NOTIFICATION_TOGGLE_KEYS));
|
let savedPersonalToggles = $state(buildToggleState(PERSONAL_EMAIL_NOTIFICATION_TOGGLE_KEYS));
|
||||||
let orgToggles = $state(buildToggleState(EMAIL_NOTIFICATION_TOGGLE_KEYS));
|
let orgToggles = $state(buildToggleState(EMAIL_NOTIFICATION_TOGGLE_KEYS));
|
||||||
|
let savedOrgToggles = $state(buildToggleState(EMAIL_NOTIFICATION_TOGGLE_KEYS));
|
||||||
let isSavingPersonal = $state(false);
|
let isSavingPersonal = $state(false);
|
||||||
let isSavingOrg = $state(false);
|
let isSavingOrg = $state(false);
|
||||||
let isLoadingPersonal = $state(false);
|
let isLoadingPersonal = $state(false);
|
||||||
let hasHydratedOrgToggles = $state(false);
|
let personalLoadVersion = 0;
|
||||||
|
let lastHydratedOrgId: string | null = null;
|
||||||
|
|
||||||
interface Props {
|
interface Props {
|
||||||
hasUnsavedChanges?: boolean;
|
hasUnsavedChanges?: boolean;
|
||||||
@@ -63,25 +63,25 @@
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function syncOrgTogglesFromCurrentOrg() {
|
function applyOrgToggleState(source?: Partial<Record<string, boolean>> | null) {
|
||||||
const org = get(currentOrg);
|
const next = buildToggleState(EMAIL_NOTIFICATION_TOGGLE_KEYS, source);
|
||||||
|
|
||||||
if (!org) return;
|
|
||||||
|
|
||||||
const next = buildToggleState(EMAIL_NOTIFICATION_TOGGLE_KEYS, org.settings?.emailNotifications);
|
|
||||||
|
|
||||||
for (const key of EMAIL_NOTIFICATION_TOGGLE_KEYS) {
|
for (const key of EMAIL_NOTIFICATION_TOGGLE_KEYS) {
|
||||||
orgToggles[key] = next[key];
|
orgToggles[key] = next[key];
|
||||||
|
savedOrgToggles[key] = next[key];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async function loadPersonalPreferences() {
|
async function loadPersonalPreferences() {
|
||||||
if (!$currentOrg?.id) return;
|
const loadVersion = ++personalLoadVersion;
|
||||||
|
|
||||||
isLoadingPersonal = true;
|
isLoadingPersonal = true;
|
||||||
|
|
||||||
const result = await memberEmailNotificationsApi.fetch();
|
const result = await memberEmailNotificationsApi.fetch();
|
||||||
|
|
||||||
|
// A newer load or a save started while this request was in flight — it owns the state now.
|
||||||
|
if (loadVersion !== personalLoadVersion) return;
|
||||||
|
|
||||||
if (result?.data) {
|
if (result?.data) {
|
||||||
applyPersonalToggleState(result.data);
|
applyPersonalToggleState(result.data);
|
||||||
}
|
}
|
||||||
@@ -89,19 +89,6 @@
|
|||||||
isLoadingPersonal = false;
|
isLoadingPersonal = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
function tryHydrateOrgToggles() {
|
|
||||||
if (hasHydratedOrgToggles || !get(isOrgAdmin)) return;
|
|
||||||
|
|
||||||
if (!get(currentOrg)?.id) return;
|
|
||||||
|
|
||||||
syncOrgTogglesFromCurrentOrg();
|
|
||||||
hasHydratedOrgToggles = true;
|
|
||||||
}
|
|
||||||
|
|
||||||
onMount(() => {
|
|
||||||
tryHydrateOrgToggles();
|
|
||||||
});
|
|
||||||
|
|
||||||
$effect(() => {
|
$effect(() => {
|
||||||
const orgId = $currentOrg?.id;
|
const orgId = $currentOrg?.id;
|
||||||
|
|
||||||
@@ -110,14 +97,17 @@
|
|||||||
void loadPersonalPreferences();
|
void loadPersonalPreferences();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Org defaults hydrate once per organization. `/account` returns `settings`
|
||||||
|
// together with the org id and role, so an id transition never misses
|
||||||
|
// late-arriving settings — and unsaved edits from another org are discarded
|
||||||
|
// instead of being saved against the wrong organization.
|
||||||
$effect(() => {
|
$effect(() => {
|
||||||
if (hasHydratedOrgToggles) return;
|
|
||||||
|
|
||||||
const orgId = $isOrgAdmin ? $currentOrg?.id : null;
|
const orgId = $isOrgAdmin ? $currentOrg?.id : null;
|
||||||
|
|
||||||
if (!orgId) return;
|
if (!orgId || orgId === lastHydratedOrgId) return;
|
||||||
|
|
||||||
tryHydrateOrgToggles();
|
lastHydratedOrgId = orgId;
|
||||||
|
applyOrgToggleState($currentOrg?.settings?.emailNotifications);
|
||||||
});
|
});
|
||||||
|
|
||||||
const personalHasChanges = $derived(
|
const personalHasChanges = $derived(
|
||||||
@@ -125,9 +115,7 @@
|
|||||||
);
|
);
|
||||||
|
|
||||||
const orgHasChanges = $derived(
|
const orgHasChanges = $derived(
|
||||||
EMAIL_NOTIFICATION_TOGGLE_KEYS.some(
|
EMAIL_NOTIFICATION_TOGGLE_KEYS.some((key) => orgToggles[key] !== savedOrgToggles[key])
|
||||||
(key) => orgToggles[key] !== ($currentOrg?.settings?.emailNotifications?.[key] !== false)
|
|
||||||
)
|
|
||||||
);
|
);
|
||||||
|
|
||||||
$effect(() => {
|
$effect(() => {
|
||||||
@@ -146,7 +134,11 @@
|
|||||||
|
|
||||||
export function handleDiscard() {
|
export function handleDiscard() {
|
||||||
applyPersonalToggleState(savedPersonalToggles);
|
applyPersonalToggleState(savedPersonalToggles);
|
||||||
syncOrgTogglesFromCurrentOrg();
|
|
||||||
|
for (const key of EMAIL_NOTIFICATION_TOGGLE_KEYS) {
|
||||||
|
orgToggles[key] = savedOrgToggles[key];
|
||||||
|
}
|
||||||
|
|
||||||
hasUnsavedChanges = false;
|
hasUnsavedChanges = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -164,6 +156,7 @@
|
|||||||
if (!$currentOrg?.id) return;
|
if (!$currentOrg?.id) return;
|
||||||
|
|
||||||
isSavingPersonal = true;
|
isSavingPersonal = true;
|
||||||
|
personalLoadVersion++;
|
||||||
|
|
||||||
const result = await memberEmailNotificationsApi.update(
|
const result = await memberEmailNotificationsApi.update(
|
||||||
toStoredPreferences(PERSONAL_EMAIL_NOTIFICATION_TOGGLE_KEYS, personalToggles)
|
toStoredPreferences(PERSONAL_EMAIL_NOTIFICATION_TOGGLE_KEYS, personalToggles)
|
||||||
@@ -181,19 +174,25 @@
|
|||||||
if (!$currentOrg) return;
|
if (!$currentOrg) return;
|
||||||
|
|
||||||
isSavingOrg = true;
|
isSavingOrg = true;
|
||||||
const existingSettings = ($currentOrg.settings as Record<string, unknown>) || {};
|
const emailNotifications = toStoredPreferences(EMAIL_NOTIFICATION_TOGGLE_KEYS, orgToggles);
|
||||||
|
|
||||||
|
// The API deep-merges `settings`, so only send the key being changed —
|
||||||
|
// spreading local settings here could write stale values over other keys.
|
||||||
await orgApi.update(
|
await orgApi.update(
|
||||||
$currentOrg.id,
|
$currentOrg.id,
|
||||||
{
|
{
|
||||||
settings: {
|
settings: { emailNotifications }
|
||||||
...existingSettings,
|
|
||||||
emailNotifications: toStoredPreferences(EMAIL_NOTIFICATION_TOGGLE_KEYS, orgToggles)
|
|
||||||
}
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
syncOrgTogglesFromCurrentOrg();
|
currentOrg.update((org) => ({
|
||||||
|
...org,
|
||||||
|
settings: {
|
||||||
|
...(org.settings as Record<string, unknown> | undefined),
|
||||||
|
emailNotifications
|
||||||
|
}
|
||||||
|
}));
|
||||||
|
applyOrgToggleState(emailNotifications);
|
||||||
snackbar.success(t.get('settings.notifications.org.save_success'));
|
snackbar.success(t.get('settings.notifications.org.save_success'));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user