fix(identity): sync default sender identity per account (#507)
The default sender identity (`preferredPrimaryId`) lived only in the browser-local `identity-storage` Zustand store and was never written to the server-side synced settings. As a result the choice was lost when clearing site data or switching browsers, and never appeared in the exported settings JSON. Persist the default identity in the synced settings store, keyed per account (`preferredIdentityIds: Record<accountId, identityId>`), mirroring the existing per-account `allMailFolderIds`. Per-account keying is required because JMAP identity ids are account-scoped and would otherwise collide across accounts / the unified mailbox. - settings-store: add `preferredIdentityIds` to state, defaults, export (so it shows in exported JSON), import (with a non-record guard), rehydrate coercion, and a v6->v7 migration. - auth-store: add `applyPreferredIdentity()`, invoked in every `loadFromServer().finally()` (login / OAuth / SSO / switch / restore) so the synced default reorders the active account's identities once server settings load (the composer defaults From to identities[0]). - identity-manager-modal: the star action also writes the choice to the synced per-account map, triggering server sync + export inclusion. - identity-store: keep `preferredPrimaryId` in local persist as a sync-off fallback; synced settings are the durable cross-device source of truth. - tests: per-account independence, export/import round-trip, non-record import guard, and v6->v7 migration.
This commit is contained in:
@@ -9,6 +9,7 @@ import { ConfirmDialog } from '@/components/ui/confirm-dialog';
|
||||
import { IdentityForm } from './identity-form';
|
||||
import { useIdentityStore } from '@/stores/identity-store';
|
||||
import { useAuthStore } from '@/stores/auth-store';
|
||||
import { useAccountStore } from '@/stores/account-store';
|
||||
import { useSettingsStore } from '@/stores/settings-store';
|
||||
|
||||
function useSyncIdentities() {
|
||||
@@ -207,15 +208,16 @@ export function IdentityManagerModal({ isOpen, onClose }: IdentityManagerModalPr
|
||||
|
||||
const handleSetPrimary = useCallback((identity: Identity) => {
|
||||
setPreferredPrimary(identity.id);
|
||||
// Persist to the synced settings (keyed by username, matching how
|
||||
// loadIdentities reads it back) so the choice survives a new browser /
|
||||
// cleared site data and reaches other devices (#507).
|
||||
const username = useAuthStore.getState().username || '';
|
||||
if (username) {
|
||||
// Persist the choice per account in the synced settings store so it
|
||||
// survives clearing site data, follows the user across devices, and shows
|
||||
// up in exported settings (issue #507). JMAP identity ids are account-
|
||||
// scoped, so the default is keyed by the active account.
|
||||
const activeAccountId = useAccountStore.getState().activeAccountId;
|
||||
if (activeAccountId) {
|
||||
const current = useSettingsStore.getState().preferredIdentityIds;
|
||||
useSettingsStore.getState().updateSetting('preferredIdentityIds', {
|
||||
...current,
|
||||
[username]: identity.id,
|
||||
[activeAccountId]: identity.id,
|
||||
});
|
||||
}
|
||||
// Re-sort: move the preferred identity to the front
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
import { describe, it, expect, beforeEach } from 'vitest';
|
||||
import { useSettingsStore, migrateSettings } from '../settings-store';
|
||||
|
||||
describe('settings-store per-account preferredIdentityIds (issue #507)', () => {
|
||||
beforeEach(() => {
|
||||
useSettingsStore.setState({ preferredIdentityIds: {} });
|
||||
});
|
||||
|
||||
it('defaults to an empty record (no account has a synced default)', () => {
|
||||
expect(useSettingsStore.getState().preferredIdentityIds).toEqual({});
|
||||
});
|
||||
|
||||
it('keeps each account default independent', () => {
|
||||
useSettingsStore.setState({
|
||||
preferredIdentityIds: { 'acct-1': 'b', 'acct-2': 'c' },
|
||||
});
|
||||
const map = useSettingsStore.getState().preferredIdentityIds;
|
||||
expect(map['acct-1']).toBe('b');
|
||||
expect(map['acct-2']).toBe('c');
|
||||
expect(map['acct-3']).toBeUndefined();
|
||||
});
|
||||
|
||||
it('round-trips through export -> import so the choice survives clearing site data', () => {
|
||||
useSettingsStore.setState({ preferredIdentityIds: { 'acct-1': 'b' } });
|
||||
const json = useSettingsStore.getState().exportSettings();
|
||||
// Appears in exported JSON (issue #507 acceptance criterion).
|
||||
expect(JSON.parse(json).preferredIdentityIds).toEqual({ 'acct-1': 'b' });
|
||||
|
||||
// Simulate a fresh browser: clear, then import the exported settings.
|
||||
useSettingsStore.setState({ preferredIdentityIds: {} });
|
||||
expect(useSettingsStore.getState().importSettings(json)).toBe(true);
|
||||
expect(useSettingsStore.getState().preferredIdentityIds).toEqual({ 'acct-1': 'b' });
|
||||
});
|
||||
|
||||
describe('importSettings non-record guard', () => {
|
||||
it('ignores a legacy array shape', () => {
|
||||
useSettingsStore.setState({ preferredIdentityIds: { 'acct-1': 'b' } });
|
||||
const ok = useSettingsStore.getState().importSettings(
|
||||
JSON.stringify({ preferredIdentityIds: ['b'] }),
|
||||
);
|
||||
expect(ok).toBe(true);
|
||||
expect(useSettingsStore.getState().preferredIdentityIds).toEqual({ 'acct-1': 'b' });
|
||||
});
|
||||
|
||||
it('ignores a null value', () => {
|
||||
useSettingsStore.setState({ preferredIdentityIds: { 'acct-1': 'b' } });
|
||||
useSettingsStore.getState().importSettings(JSON.stringify({ preferredIdentityIds: null }));
|
||||
expect(useSettingsStore.getState().preferredIdentityIds).toEqual({ 'acct-1': 'b' });
|
||||
});
|
||||
|
||||
it('accepts a proper per-account record', () => {
|
||||
useSettingsStore.getState().importSettings(
|
||||
JSON.stringify({ preferredIdentityIds: { 'acct-9': 'a' } }),
|
||||
);
|
||||
expect(useSettingsStore.getState().preferredIdentityIds).toEqual({ 'acct-9': 'a' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('migrateSettings v6 -> v7', () => {
|
||||
it('adds an empty preferredIdentityIds map for pre-v7 users', () => {
|
||||
const out = migrateSettings({ allMailFolderIds: {} }, 6) as unknown as Record<string, unknown>;
|
||||
expect(out.preferredIdentityIds).toEqual({});
|
||||
});
|
||||
|
||||
it('coerces a non-record preferredIdentityIds to an empty map', () => {
|
||||
const out = migrateSettings(
|
||||
{ allMailFolderIds: {}, preferredIdentityIds: ['b'] },
|
||||
7,
|
||||
) as unknown as Record<string, unknown>;
|
||||
expect(out.preferredIdentityIds).toEqual({});
|
||||
});
|
||||
|
||||
it('preserves a valid per-account map across migration', () => {
|
||||
const out = migrateSettings(
|
||||
{ allMailFolderIds: {}, preferredIdentityIds: { 'acct-1': 'b' } },
|
||||
7,
|
||||
) as unknown as Record<string, unknown>;
|
||||
expect(out.preferredIdentityIds).toEqual({ 'acct-1': 'b' });
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -233,6 +233,40 @@ function loadIdentities(rawIdentities: Identity[], username: string): { identiti
|
||||
return { identities, primaryIdentity };
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-apply the per-account default sender identity once synced settings are
|
||||
* available (issue #507). The choice is stored server-side in the settings
|
||||
* store (`preferredIdentityIds`, keyed by AccountEntry.id), so it can only be
|
||||
* applied after `loadFromServer` resolves. It reorders the account's identities
|
||||
* so the preferred one is primary - the composer defaults its `From` to
|
||||
* identities[0]. No-op when nothing is configured for the account.
|
||||
*
|
||||
* @param accountId The account to apply for; defaults to the active account.
|
||||
*/
|
||||
export function applyPreferredIdentity(accountId?: string | null): void {
|
||||
const targetId = accountId ?? useAccountStore.getState().activeAccountId;
|
||||
if (!targetId) return;
|
||||
|
||||
const preferred = useSettingsStore.getState().preferredIdentityIds[targetId];
|
||||
if (!preferred) return;
|
||||
|
||||
const idStore = useIdentityStore.getState();
|
||||
// Only touch the live identity store when it currently holds this account's
|
||||
// identities (true for the active account). Switching snapshots/restores the
|
||||
// ordering per account, so a background account's order is restored later.
|
||||
if (useAccountStore.getState().activeAccountId !== targetId) return;
|
||||
|
||||
idStore.setPreferredPrimary(preferred);
|
||||
const ids = [...idStore.identities];
|
||||
const idx = ids.findIndex((i) => i.id === preferred);
|
||||
if (idx > 0) {
|
||||
const [p] = ids.splice(idx, 1);
|
||||
ids.unshift(p);
|
||||
idStore.setIdentities(ids);
|
||||
}
|
||||
useAuthStore.setState({ identities: ids, primaryIdentity: ids[0] ?? null });
|
||||
}
|
||||
|
||||
function getLocaleLoginPath(): string {
|
||||
if (typeof window === 'undefined') return '/en/login';
|
||||
|
||||
@@ -638,6 +672,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
if (!config.settingsSyncEnabled) return;
|
||||
useSettingsStore.getState().loadFromServer(username, serverUrl).finally(() => {
|
||||
useSettingsStore.getState().enableSync(username, serverUrl);
|
||||
applyPreferredIdentity(accountId);
|
||||
});
|
||||
}).catch(() => {});
|
||||
|
||||
@@ -840,6 +875,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
if (!config.settingsSyncEnabled) return;
|
||||
useSettingsStore.getState().loadFromServer(username, serverUrl).finally(() => {
|
||||
useSettingsStore.getState().enableSync(username, serverUrl);
|
||||
applyPreferredIdentity(accountId);
|
||||
});
|
||||
}).catch(() => {});
|
||||
|
||||
@@ -977,6 +1013,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
if (!cfg.settingsSyncEnabled) return;
|
||||
useSettingsStore.getState().loadFromServer(username, ssoServerUrl).finally(() => {
|
||||
useSettingsStore.getState().enableSync(username, ssoServerUrl);
|
||||
applyPreferredIdentity(accountId);
|
||||
});
|
||||
}).catch(() => {});
|
||||
|
||||
@@ -1363,6 +1400,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
if (!config.settingsSyncEnabled) return;
|
||||
useSettingsStore.getState().loadFromServer(targetAccount.username, targetAccount.serverUrl).finally(() => {
|
||||
useSettingsStore.getState().enableSync(targetAccount.username, targetAccount.serverUrl);
|
||||
applyPreferredIdentity(targetAccount.id);
|
||||
});
|
||||
}).catch(() => {});
|
||||
},
|
||||
@@ -1534,6 +1572,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
if (!config.settingsSyncEnabled) return;
|
||||
useSettingsStore.getState().loadFromServer(targetAccount.username, targetAccount.serverUrl).finally(() => {
|
||||
useSettingsStore.getState().enableSync(targetAccount.username, targetAccount.serverUrl);
|
||||
applyPreferredIdentity(targetAccount.id);
|
||||
});
|
||||
}).catch(() => {});
|
||||
return;
|
||||
@@ -1647,6 +1686,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
if (!config.settingsSyncEnabled) return;
|
||||
useSettingsStore.getState().loadFromServer(state.username || '', state.serverUrl!).finally(() => {
|
||||
useSettingsStore.getState().enableSync(state.username || '', state.serverUrl!);
|
||||
applyPreferredIdentity(accountId);
|
||||
});
|
||||
}).catch(() => {});
|
||||
return;
|
||||
@@ -1718,6 +1758,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
if (!config.settingsSyncEnabled) return;
|
||||
useSettingsStore.getState().loadFromServer(username, serverUrl).finally(() => {
|
||||
useSettingsStore.getState().enableSync(username, serverUrl);
|
||||
applyPreferredIdentity(accountId);
|
||||
});
|
||||
}).catch(() => {});
|
||||
return;
|
||||
|
||||
@@ -123,7 +123,14 @@ export const useIdentityStore = create<IdentityStore>()(
|
||||
}),
|
||||
{
|
||||
name: 'identity-storage',
|
||||
// Only persist sub-addressing data, not identities (they're server-side)
|
||||
// Only persist sub-addressing data, not identities (they're server-side).
|
||||
// The default sender identity (`preferredPrimaryId`) is the per-account
|
||||
// value for the *active* account; it is kept here purely as a local
|
||||
// fallback so the choice survives a reload when settings sync is off.
|
||||
// The durable, cross-device, exportable source of truth is the synced
|
||||
// settings store, keyed per account (`preferredIdentityIds`), which is
|
||||
// re-applied via applyPreferredIdentity() once server settings load and
|
||||
// overrides this value per account (issue #507).
|
||||
partialize: (state) => ({
|
||||
subAddress: state.subAddress,
|
||||
preferredPrimaryId: state.preferredPrimaryId,
|
||||
|
||||
@@ -262,6 +262,12 @@ interface SettingsState {
|
||||
// -> defaults to inbox + custom folders; an explicit [] = "no own folders".
|
||||
allMailFolderIds: Record<string, string[]>;
|
||||
|
||||
// Per-account default sender identity, keyed by AccountEntry.id -> JMAP
|
||||
// Identity id. Synced (and exported) so the chosen default survives clearing
|
||||
// site data and follows the user across browsers/devices (issue #507). Kept
|
||||
// per account because JMAP identity ids are account-scoped and would collide.
|
||||
preferredIdentityIds: Record<string, string>;
|
||||
|
||||
// Email Display
|
||||
disableThreading: boolean; // Show emails as individual messages instead of grouped by conversation
|
||||
|
||||
@@ -455,6 +461,7 @@ const DEFAULT_SETTINGS = {
|
||||
unifiedCrossAccount: false,
|
||||
|
||||
allMailFolderIds: {} as Record<string, string[]>,
|
||||
preferredIdentityIds: {} as Record<string, string>,
|
||||
|
||||
enableCrossUnreadView: false,
|
||||
enableCrossStarredView: false,
|
||||
@@ -640,6 +647,7 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
includeGroupInUnified: state.includeGroupInUnified,
|
||||
unifiedCrossAccount: state.unifiedCrossAccount,
|
||||
allMailFolderIds: state.allMailFolderIds,
|
||||
preferredIdentityIds: state.preferredIdentityIds,
|
||||
enableCrossUnreadView: state.enableCrossUnreadView,
|
||||
enableCrossStarredView: state.enableCrossStarredView,
|
||||
enableCrossAllView: state.enableCrossAllView,
|
||||
@@ -697,8 +705,8 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
if (key === 'allMailFolderIds' && !isPlainRecord(settings[key])) {
|
||||
return;
|
||||
}
|
||||
// Defensive: a non-record (e.g. a legacy scalar) would break the
|
||||
// per-account map lookups - ignore it.
|
||||
// Per-account map (accountId -> identityId); ignore any legacy
|
||||
// global/non-record value rather than corrupting the map.
|
||||
if (key === 'preferredIdentityIds' && !isPlainRecord(settings[key])) {
|
||||
return;
|
||||
}
|
||||
@@ -900,7 +908,7 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
}),
|
||||
{
|
||||
name: 'settings-storage',
|
||||
version: 6,
|
||||
version: 7,
|
||||
migrate: migrateSettings,
|
||||
onRehydrateStorage: () => {
|
||||
return (state) => {
|
||||
@@ -979,6 +987,11 @@ export function migrateSettings(persisted: unknown, version: number): SettingsSt
|
||||
// configuration (matches the new-install default).
|
||||
state.includeGroupInUnified = true;
|
||||
}
|
||||
// v7: introduced the per-account default-identity map (issue #507).
|
||||
// Coerce any missing/legacy value to an empty record.
|
||||
if (version < 7 || !isPlainRecord(state.preferredIdentityIds)) {
|
||||
state.preferredIdentityIds = {};
|
||||
}
|
||||
return state as unknown as SettingsState;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user