mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix(settings): merge per-account maps on server load (compose identity switch)
With multiple accounts and settings sync on, the composer's default sender would switch depending on login order and get disturbed by logging a third account off. Cause: the per-account maps preferredIdentityIds (#507) and allMailFolderIds live in EVERY account's synced settings blob, and loadFromServer(username, serverUrl) runs once per connected account calling importSettings(), which REPLACED the whole map with that account's server copy. So the last account to load clobbered the map (last-writer-wins by login order), and a stale blob from a since-removed account could win too. Fix: importSettings takes an optional { serverAccountId }; loadFromServer passes generateAccountId(username, serverUrl). For the per-account maps, a server load now merges only that account's OWN entry into the existing map instead of replacing it - each account's server is authoritative only for itself, so other accounts' entries survive. File imports (no serverAccountId) still replace wholesale. + regression tests: a per-account load preserves other accounts' entries, only trusts the loading account, file import still replaces, same for allMailFolderIds. tsc + eslint clean.
This commit is contained in:
@@ -56,6 +56,54 @@ describe('settings-store per-account preferredIdentityIds (issue #507)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// Regression: multi-account logins used to clobber the per-account map. Each
|
||||
// account's server load must merge only its OWN entry, preserving the others,
|
||||
// so the composer's default sender no longer depends on login order.
|
||||
describe('importSettings per-account merge (multi-account login order)', () => {
|
||||
beforeEach(() => useSettingsStore.setState({ preferredIdentityIds: {}, allMailFolderIds: {} }));
|
||||
|
||||
it('a per-account server load updates only its own entry, preserving others', () => {
|
||||
useSettingsStore.setState({ preferredIdentityIds: { 'a@h': 'idA', 'b@h': 'idB' } });
|
||||
// Account B's server blob carries a STALE entry for A plus B's own value.
|
||||
useSettingsStore.getState().importSettings(
|
||||
JSON.stringify({ preferredIdentityIds: { 'a@h': 'STALE', 'b@h': 'idB2' } }),
|
||||
{ serverAccountId: 'b@h' },
|
||||
);
|
||||
const map = useSettingsStore.getState().preferredIdentityIds;
|
||||
expect(map['a@h']).toBe('idA'); // preserved, not clobbered by B's stale copy
|
||||
expect(map['b@h']).toBe('idB2'); // B is authoritative for itself
|
||||
});
|
||||
|
||||
it('fills the loading account entry when the map had none, without importing others', () => {
|
||||
useSettingsStore.setState({ preferredIdentityIds: { 'a@h': 'idA' } });
|
||||
useSettingsStore.getState().importSettings(
|
||||
JSON.stringify({ preferredIdentityIds: { 'a@h': 'x', 'b@h': 'idB' } }),
|
||||
{ serverAccountId: 'b@h' },
|
||||
);
|
||||
// Only b@h (the loading account) is trusted; a@h keeps its local value.
|
||||
expect(useSettingsStore.getState().preferredIdentityIds).toEqual({ 'a@h': 'idA', 'b@h': 'idB' });
|
||||
});
|
||||
|
||||
it('a file import (no serverAccountId) still replaces the whole map', () => {
|
||||
useSettingsStore.setState({ preferredIdentityIds: { 'a@h': 'idA' } });
|
||||
useSettingsStore.getState().importSettings(
|
||||
JSON.stringify({ preferredIdentityIds: { 'b@h': 'idB' } }),
|
||||
);
|
||||
expect(useSettingsStore.getState().preferredIdentityIds).toEqual({ 'b@h': 'idB' });
|
||||
});
|
||||
|
||||
it('applies the same per-account merge to allMailFolderIds', () => {
|
||||
useSettingsStore.setState({ allMailFolderIds: { 'a@h': ['x'], 'b@h': ['y'] } });
|
||||
useSettingsStore.getState().importSettings(
|
||||
JSON.stringify({ allMailFolderIds: { 'a@h': ['STALE'], 'b@h': ['y2'] } }),
|
||||
{ serverAccountId: 'b@h' },
|
||||
);
|
||||
const map = useSettingsStore.getState().allMailFolderIds;
|
||||
expect(map['a@h']).toEqual(['x']);
|
||||
expect(map['b@h']).toEqual(['y2']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('migrateSettings identity map', () => {
|
||||
it('adds an empty preferredIdentityIds map for pre-v6 users', () => {
|
||||
const out = migrateSettings({ allMailFolderIds: {} }, 6) as unknown as Record<string, unknown>;
|
||||
|
||||
@@ -4,6 +4,7 @@ import { useThemeStore } from './theme-store';
|
||||
import { useLocaleStore } from './locale-store';
|
||||
import type { NotificationSoundChoice } from '@/lib/notification-sound';
|
||||
import { apiFetch } from '@/lib/browser-navigation';
|
||||
import { generateAccountId } from '@/lib/account-utils';
|
||||
import {
|
||||
DEFAULT_SUB_ADDRESS_DELIMITER,
|
||||
isValidSubAddressDelimiter,
|
||||
@@ -418,7 +419,7 @@ interface SettingsState {
|
||||
) => void;
|
||||
resetToDefaults: () => void;
|
||||
exportSettings: () => string;
|
||||
importSettings: (json: string) => boolean;
|
||||
importSettings: (json: string, opts?: { serverAccountId?: string }) => boolean;
|
||||
|
||||
// Folder icons
|
||||
setFolderIcon: (mailboxId: string, icon: string) => void;
|
||||
@@ -772,7 +773,7 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
return JSON.stringify(settings, null, 2);
|
||||
},
|
||||
|
||||
importSettings: (json: string) => {
|
||||
importSettings: (json: string, opts?: { serverAccountId?: string }) => {
|
||||
try {
|
||||
const settings = JSON.parse(json);
|
||||
|
||||
@@ -808,6 +809,24 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
if (DEVICE_LOCAL_SETTING_KEYS.has(key)) {
|
||||
return;
|
||||
}
|
||||
// Per-account maps (accountId -> value) live in every account's
|
||||
// synced settings blob, so a per-account server load must NOT
|
||||
// replace the whole map - each account's server is authoritative
|
||||
// only for its OWN entry. Merging preserves the other accounts'
|
||||
// local values; without this the last account to load clobbers
|
||||
// the map and the composer picks another account's default sender
|
||||
// (login-order dependent). File imports (no serverAccountId)
|
||||
// still replace wholesale.
|
||||
if (opts?.serverAccountId && (key === 'preferredIdentityIds' || key === 'allMailFolderIds')) {
|
||||
const existing = (get() as unknown as Record<string, unknown>)[key];
|
||||
const merged: Record<string, unknown> = isPlainRecord(existing) ? { ...existing } : {};
|
||||
const incoming = settings[key] as Record<string, unknown>;
|
||||
if (Object.prototype.hasOwnProperty.call(incoming, opts.serverAccountId)) {
|
||||
merged[opts.serverAccountId] = incoming[opts.serverAccountId];
|
||||
}
|
||||
set({ [key]: merged });
|
||||
return;
|
||||
}
|
||||
set({ [key]: settings[key] });
|
||||
}
|
||||
});
|
||||
@@ -980,7 +999,11 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
}
|
||||
if (settings && typeof settings === 'object') {
|
||||
isLoadingFromServer = true;
|
||||
get().importSettings(JSON.stringify(settings));
|
||||
// Merge (not replace) per-account maps for the account being loaded,
|
||||
// so multi-account logins don't clobber each other by login order.
|
||||
get().importSettings(JSON.stringify(settings), {
|
||||
serverAccountId: generateAccountId(username, serverUrl),
|
||||
});
|
||||
isLoadingFromServer = false;
|
||||
syncLog('Settings loaded from server successfully');
|
||||
// The per-account preferred sender identity (#507) is re-applied by
|
||||
|
||||
Reference in New Issue
Block a user