mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 16:06:32 +00:00
fix: stop asking for the identities on every message open
The viewer and the read-receipt banner each sent `Identity/get` whenever the identity list was empty. Identities are not persisted, so the first open of every session asked twice, and an account the server lists no identities for asked on every open. The settings store now remembers which signed-in account (server, login, JMAP account) its identities were read for, and the viewer and the banner only ask when they are not held for the current account. Concurrent reads share one request. Switching accounts drops the old account's identities instead of letting the quick reply send through one the new account does not have.
This commit is contained in:
@@ -42,14 +42,15 @@ export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMa
|
||||
const styles = React.useMemo(() => makeStyles(c), [c]);
|
||||
const t = useLocaleStore((s) => s.t);
|
||||
const identities = useSettingsStore((s) => s.identities);
|
||||
const fetchIdentities = useSettingsStore((s) => s.fetchIdentities);
|
||||
const ensureIdentities = useSettingsStore((s) => s.ensureIdentities);
|
||||
const readReceiptResponse = useSettingsStore((s) => s.readReceiptResponse);
|
||||
const mailboxes = useEmailStore((s) => s.mailboxes);
|
||||
const [busy, setBusy] = React.useState(false);
|
||||
const [handledLocally, setHandledLocally] = React.useState(false);
|
||||
|
||||
React.useEffect(() => { setHandledLocally(false); }, [email.id]);
|
||||
React.useEffect(() => { if (identities.length === 0) void fetchIdentities(); }, [identities.length, fetchIdentities]);
|
||||
// Held for the account after the first read, even when it has none.
|
||||
React.useEffect(() => { void ensureIdentities(); }, [ensureIdentities]);
|
||||
|
||||
const eligibleFolder = !['sent', 'drafts', 'trash', 'junk', 'spam'].includes(currentMailboxRole || '');
|
||||
const identity = React.useMemo(() => findReceivingIdentity(identities, email), [identities, email]);
|
||||
|
||||
@@ -83,7 +83,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) {
|
||||
const currentMailboxId = useEmailStore((s) => s.currentMailboxId);
|
||||
const disableThreading = useSettingsStore((s) => s.disableThreading);
|
||||
const identities = useSettingsStore((s) => s.identities);
|
||||
const fetchIdentities = useSettingsStore((s) => s.fetchIdentities);
|
||||
const ensureIdentities = useSettingsStore((s) => s.ensureIdentities);
|
||||
const deleteAction = useSettingsStore((s) => s.deleteAction);
|
||||
const permanentlyDeleteJunk = useSettingsStore((s) => s.permanentlyDeleteJunk);
|
||||
const postExportAction = useSettingsStore((s) => s.postExportAction);
|
||||
@@ -92,7 +92,8 @@ export default function EmailThreadScreen({ route, navigation }: Props) {
|
||||
const exportLowercase = useSettingsStore((s) => s.exportLowercase);
|
||||
const exportStripDiacritics = useSettingsStore((s) => s.exportStripDiacritics);
|
||||
const resolvedTheme = useResolvedTheme();
|
||||
React.useEffect(() => { if (identities.length === 0) void fetchIdentities(); }, [identities.length, fetchIdentities]);
|
||||
// Held for the account after the first read, even when it has none.
|
||||
React.useEffect(() => { void ensureIdentities(); }, [ensureIdentities]);
|
||||
|
||||
// The account whose mail the store's list holds (the open folder's). Its
|
||||
// rows only stand for this message when that is the message's account.
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const client = vi.hoisted(() => ({
|
||||
accountId: 'acc-1',
|
||||
serverUrl: 'https://mail.example',
|
||||
username: 'me',
|
||||
}));
|
||||
|
||||
vi.mock('../../api/jmap-client', () => ({ jmapClient: client }));
|
||||
vi.mock('../../api/identity', () => ({ getIdentities: vi.fn() }));
|
||||
|
||||
import { getIdentities } from '../../api/identity';
|
||||
import { useSettingsStore } from '../settings-store';
|
||||
|
||||
const mockGetIdentities = getIdentities as ReturnType<typeof vi.fn>;
|
||||
const me = { id: 'i1', name: 'Me', email: 'me@example.com' };
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
client.accountId = 'acc-1';
|
||||
client.serverUrl = 'https://mail.example';
|
||||
useSettingsStore.getState().reset();
|
||||
});
|
||||
|
||||
describe('identities', () => {
|
||||
it('reads them once per account, even when the account has none', async () => {
|
||||
mockGetIdentities.mockResolvedValue([]);
|
||||
|
||||
await useSettingsStore.getState().ensureIdentities();
|
||||
await useSettingsStore.getState().ensureIdentities();
|
||||
|
||||
expect(mockGetIdentities).toHaveBeenCalledTimes(1);
|
||||
expect(useSettingsStore.getState().identities).toEqual([]);
|
||||
});
|
||||
|
||||
it('shares one request between concurrent callers', async () => {
|
||||
mockGetIdentities.mockResolvedValue([me]);
|
||||
|
||||
await Promise.all([
|
||||
useSettingsStore.getState().ensureIdentities(),
|
||||
useSettingsStore.getState().ensureIdentities(),
|
||||
useSettingsStore.getState().fetchIdentities(),
|
||||
]);
|
||||
|
||||
expect(mockGetIdentities).toHaveBeenCalledTimes(1);
|
||||
expect(useSettingsStore.getState().identities).toEqual([me]);
|
||||
});
|
||||
|
||||
it('drops another account’s identities and reads this one’s', async () => {
|
||||
mockGetIdentities.mockResolvedValueOnce([me]);
|
||||
await useSettingsStore.getState().ensureIdentities();
|
||||
|
||||
client.serverUrl = 'https://other.example';
|
||||
let release!: (list: unknown[]) => void;
|
||||
mockGetIdentities.mockReturnValueOnce(new Promise((r) => { release = r; }));
|
||||
const pending = useSettingsStore.getState().ensureIdentities();
|
||||
|
||||
expect(useSettingsStore.getState().identities).toEqual([]);
|
||||
release([{ id: 'i2', name: 'Other', email: 'other@example.com' }]);
|
||||
await pending;
|
||||
expect(useSettingsStore.getState().identities.map((i) => i.id)).toEqual(['i2']);
|
||||
});
|
||||
|
||||
it('asks again after a failed read', async () => {
|
||||
mockGetIdentities.mockRejectedValueOnce(new Error('offline')).mockResolvedValueOnce([me]);
|
||||
|
||||
await useSettingsStore.getState().ensureIdentities();
|
||||
await useSettingsStore.getState().ensureIdentities();
|
||||
|
||||
expect(mockGetIdentities).toHaveBeenCalledTimes(2);
|
||||
expect(useSettingsStore.getState().identities).toEqual([me]);
|
||||
});
|
||||
|
||||
it('keeps a read that lands after switching accounts out of the new one', async () => {
|
||||
let release!: (list: unknown[]) => void;
|
||||
mockGetIdentities.mockReturnValueOnce(new Promise((r) => { release = r; }));
|
||||
const pending = useSettingsStore.getState().ensureIdentities();
|
||||
|
||||
client.accountId = 'acc-2';
|
||||
release([me]);
|
||||
await pending;
|
||||
|
||||
expect(useSettingsStore.getState().identities).toEqual([]);
|
||||
expect(useSettingsStore.getState().identitiesFor).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,7 @@ import { create } from 'zustand';
|
||||
import AsyncStorage from '@react-native-async-storage/async-storage';
|
||||
import type { Identity } from '../api/types';
|
||||
import { getIdentities as fetchIdentities } from '../api/identity';
|
||||
import { jmapClient } from '../api/jmap-client';
|
||||
import type { SortLevel, MessageListOrderScope } from '../lib/message-list-order';
|
||||
|
||||
export type ExternalContentPolicy = 'allow' | 'block' | 'ask';
|
||||
@@ -442,11 +443,19 @@ const DEFAULT_PERSISTED: PersistedSettings = {
|
||||
|
||||
export interface SettingsState extends PersistedSettings {
|
||||
identities: Identity[];
|
||||
/** The signed-in account (server, login, JMAP account) `identities` were read for. */
|
||||
identitiesFor: string | null;
|
||||
loading: boolean;
|
||||
error: string | null;
|
||||
hydrated: boolean;
|
||||
|
||||
/** Read the identities; concurrent calls share one request. */
|
||||
fetchIdentities: () => Promise<void>;
|
||||
/**
|
||||
* Read the identities only when they are not held for the signed-in account
|
||||
* yet. An account without identities is not asked again on every call.
|
||||
*/
|
||||
ensureIdentities: () => Promise<void>;
|
||||
hydrate: () => Promise<void>;
|
||||
|
||||
// Generic setter — preferred for new code.
|
||||
@@ -670,21 +679,58 @@ function stripDisplayName(email: string): string {
|
||||
return (angleMatch ? angleMatch[2] : trimmed).toLowerCase().trim();
|
||||
}
|
||||
|
||||
// Which signed-in account the identities belong to. Two servers can hand out
|
||||
// the same JMAP account ids, so the server and login are part of it.
|
||||
function identityScope(): string | null {
|
||||
try {
|
||||
return `${jmapClient.serverUrl ?? ''}|${jmapClient.username ?? ''}|${jmapClient.accountId}`;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
let identitiesInFlight: { scope: string | null; promise: Promise<void> } | null = null;
|
||||
|
||||
export const useSettingsStore = create<SettingsState>((set, get) => ({
|
||||
...DEFAULT_PERSISTED,
|
||||
identities: [],
|
||||
identitiesFor: null,
|
||||
loading: false,
|
||||
error: null,
|
||||
hydrated: false,
|
||||
|
||||
fetchIdentities: async () => {
|
||||
set({ loading: true, error: null });
|
||||
try {
|
||||
const identities = await fetchIdentities();
|
||||
set({ identities, loading: false });
|
||||
} catch (err) {
|
||||
set({ loading: false, error: err instanceof Error ? err.message : 'Failed to load identities' });
|
||||
}
|
||||
fetchIdentities: () => {
|
||||
const scope = identityScope();
|
||||
if (identitiesInFlight && identitiesInFlight.scope === scope) return identitiesInFlight.promise;
|
||||
let promise: Promise<void> | undefined;
|
||||
promise = (async () => {
|
||||
set({ loading: true, error: null });
|
||||
try {
|
||||
const identities = await fetchIdentities();
|
||||
// Signed in to another account meanwhile: these are not its identities.
|
||||
if (identityScope() !== scope) {
|
||||
set({ loading: false });
|
||||
return;
|
||||
}
|
||||
set({ identities, identitiesFor: scope, loading: false });
|
||||
} catch (err) {
|
||||
set({ loading: false, error: err instanceof Error ? err.message : 'Failed to load identities' });
|
||||
} finally {
|
||||
if (identitiesInFlight?.promise === promise) identitiesInFlight = null;
|
||||
}
|
||||
})();
|
||||
identitiesInFlight = { scope, promise };
|
||||
return promise;
|
||||
},
|
||||
|
||||
ensureIdentities: () => {
|
||||
const scope = identityScope();
|
||||
const held = get().identitiesFor;
|
||||
if (scope !== null && held === scope) return Promise.resolve();
|
||||
// Another account's identities must not stand in (the quick reply would
|
||||
// send through one this account does not have).
|
||||
if (held !== null && held !== scope) set({ identities: [], identitiesFor: null });
|
||||
return get().fetchIdentities();
|
||||
},
|
||||
|
||||
hydrate: async () => {
|
||||
@@ -804,6 +850,7 @@ export const useSettingsStore = create<SettingsState>((set, get) => ({
|
||||
|
||||
reset: () => set({
|
||||
identities: [],
|
||||
identitiesFor: null,
|
||||
loading: false,
|
||||
error: null,
|
||||
}),
|
||||
|
||||
Reference in New Issue
Block a user