fix(identity): sync default sender identity per account (#507)
The default sender identity (`preferredPrimaryId`) lived only in the browser-local `identity-storage` store and was never written to the synced settings, so the choice was lost on clearing site data / switching browsers and never appeared in exported settings. Persist it 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. This supersedes the earlier username-keyed fix that had landed on main: the username-keyed map, `loadIdentities()` fallback write, and the `applyPreferredIdentityOrdering` store action (plus its settings-store hook) are removed so a single account-keyed mechanism remains. - settings-store: `preferredIdentityIds` (accountId -> identityId) in state, defaults, export, import (non-record guard), rehydrate coercion, v6 migration. - auth-store: `applyPreferredIdentity(accountId?)` reorders the active account's identities once synced settings load, and performs the one-time migration of the pre-#507 browser-local default into the synced map (keyed by accountId). Invoked from every `loadFromServer().finally()` (login / OAuth / SSO / switch / restore). `loadIdentities()` now only applies the local fallback ordering. - identity-manager-modal: the star action writes the choice by `activeAccountId`. - identity-store: `preferredPrimaryId` kept as a local (sync-off) fallback. - tests: per-account independence, export/import round-trip, import guard, and applyPreferredIdentity reorder / active-account gating / local-default migration.
This commit is contained in:
committed by
Linus Rath
parent
20d02214df
commit
01e5cd69cf
@@ -0,0 +1,90 @@
|
||||
import { describe, it, expect, beforeEach } from 'vitest';
|
||||
import { applyPreferredIdentity, useAuthStore } from '../auth-store';
|
||||
import { useIdentityStore } from '../identity-store';
|
||||
import { useAccountStore } from '../account-store';
|
||||
import { useSettingsStore } from '../settings-store';
|
||||
import type { Identity } from '@/lib/jmap/types';
|
||||
|
||||
const makeIdentity = (overrides: Partial<Identity> = {}): Identity => ({
|
||||
id: 'id-1',
|
||||
name: 'Test User',
|
||||
email: 'test@example.com',
|
||||
mayDelete: true,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const IDS = [
|
||||
makeIdentity({ id: 'id-1', name: 'Alice', email: 'alice@example.com' }),
|
||||
makeIdentity({ id: 'id-2', name: 'Bob', email: 'bob@example.com' }),
|
||||
makeIdentity({ id: 'id-3', name: 'Carol', email: 'carol@example.com' }),
|
||||
];
|
||||
|
||||
/**
|
||||
* applyPreferredIdentity() is the single mechanism that honours the synced,
|
||||
* per-account default sender identity (#507). These tests drive the real
|
||||
* zustand stores directly (as the other auth-store tests do).
|
||||
*/
|
||||
describe('applyPreferredIdentity (issue #507)', () => {
|
||||
beforeEach(() => {
|
||||
useIdentityStore.setState({ identities: [...IDS], preferredPrimaryId: null });
|
||||
useAuthStore.setState({ identities: [...IDS], primaryIdentity: IDS[0] });
|
||||
useAccountStore.setState({ activeAccountId: 'acc-1' });
|
||||
useSettingsStore.setState({ preferredIdentityIds: {} });
|
||||
});
|
||||
|
||||
it('reorders the active account so the synced preferred identity is primary', () => {
|
||||
useSettingsStore.setState({ preferredIdentityIds: { 'acc-1': 'id-3' } });
|
||||
|
||||
applyPreferredIdentity('acc-1');
|
||||
|
||||
expect(useAuthStore.getState().identities.map((i) => i.id)).toEqual(['id-3', 'id-1', 'id-2']);
|
||||
expect(useAuthStore.getState().primaryIdentity?.id).toBe('id-3');
|
||||
expect(useIdentityStore.getState().preferredPrimaryId).toBe('id-3');
|
||||
});
|
||||
|
||||
it('defaults to the active account when no accountId is passed', () => {
|
||||
useSettingsStore.setState({ preferredIdentityIds: { 'acc-1': 'id-2' } });
|
||||
|
||||
applyPreferredIdentity();
|
||||
|
||||
expect(useAuthStore.getState().identities[0].id).toBe('id-2');
|
||||
});
|
||||
|
||||
it('is a no-op when the target is not the active account', () => {
|
||||
useSettingsStore.setState({ preferredIdentityIds: { 'acc-2': 'id-3' } });
|
||||
|
||||
applyPreferredIdentity('acc-2');
|
||||
|
||||
// active account's live ordering must be untouched
|
||||
expect(useAuthStore.getState().identities.map((i) => i.id)).toEqual(['id-1', 'id-2', 'id-3']);
|
||||
});
|
||||
|
||||
it('is a no-op when the account has no synced default and no local fallback', () => {
|
||||
applyPreferredIdentity('acc-1');
|
||||
|
||||
expect(useAuthStore.getState().identities.map((i) => i.id)).toEqual(['id-1', 'id-2', 'id-3']);
|
||||
expect(useSettingsStore.getState().preferredIdentityIds).toEqual({});
|
||||
});
|
||||
|
||||
it('migrates the pre-#507 browser-local default into the synced map, keyed by accountId', () => {
|
||||
// No synced entry, but a local (identity-storage) preferred primary exists.
|
||||
useIdentityStore.setState({ preferredPrimaryId: 'id-2' });
|
||||
|
||||
applyPreferredIdentity('acc-1');
|
||||
|
||||
// adopted, persisted per account, and applied to the live ordering
|
||||
expect(useSettingsStore.getState().preferredIdentityIds).toEqual({ 'acc-1': 'id-2' });
|
||||
expect(useAuthStore.getState().identities[0].id).toBe('id-2');
|
||||
});
|
||||
|
||||
it('prefers the synced value over the local fallback', () => {
|
||||
useIdentityStore.setState({ preferredPrimaryId: 'id-2' });
|
||||
useSettingsStore.setState({ preferredIdentityIds: { 'acc-1': 'id-3' } });
|
||||
|
||||
applyPreferredIdentity('acc-1');
|
||||
|
||||
expect(useAuthStore.getState().identities[0].id).toBe('id-3');
|
||||
// the synced value is not overwritten by the migration
|
||||
expect(useSettingsStore.getState().preferredIdentityIds).toEqual({ 'acc-1': 'id-3' });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,58 @@
|
||||
import { describe, it, expect, beforeEach } from 'vitest';
|
||||
import { useSettingsStore } 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' });
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user