From 668e01023c258255d5d8d5bf0540af1d87f81863 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:59:56 +0200 Subject: [PATCH] fix: stop the contacts tab downloading every card on each open Opening Contacts re-ran the full `ContactCard/query` + `ContactCard/get` for every account (58 KB on the audit server) although the cards are loaded at startup and every live `ContactCard` state change already refetches them. The tab now only fetches when no complete fetch happened this session, after a reset or account switch, after a filtered load, or when the last full fetch is more than 5 minutes old, which covers changes missed while the app was in the background or live updates were down. Pull to refresh and the Refresh buttons still force a reload (PF6). --- src/screens/ContactsScreen.tsx | 7 ++- src/stores/__tests__/contacts-store.test.ts | 48 ++++++++++++++++++++- src/stores/contacts-store.ts | 24 ++++++++++- 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/screens/ContactsScreen.tsx b/src/screens/ContactsScreen.tsx index 52bd518d..b98b5d52 100644 --- a/src/screens/ContactsScreen.tsx +++ b/src/screens/ContactsScreen.tsx @@ -106,6 +106,7 @@ export default function ContactsScreen() { const hydrated = useContactsStore((s) => s.hydrated); const fetchAddressBooks = useContactsStore((s) => s.fetchAddressBooks); const fetchContacts = useContactsStore((s) => s.fetchContacts); + const fetchContactsIfStale = useContactsStore((s) => s.fetchContactsIfStale); const hydrate = useContactsStore((s) => s.hydrate); const bulkDelete = useContactsStore((s) => s.bulkDelete); const moveContactsToAddressBook = useContactsStore((s) => s.moveContactsToAddressBook); @@ -130,10 +131,12 @@ export default function ContactsScreen() { void hydrate(); }, [hydrate]); + // The cards loaded at startup and follow live changes; only download them + // again when that copy is stale. Pull to refresh still forces a reload. React.useEffect(() => { void fetchAddressBooks(); - void fetchContacts(); - }, [fetchAddressBooks, fetchContacts]); + void fetchContactsIfStale(); + }, [fetchAddressBooks, fetchContactsIfStale]); const groups = React.useMemo(() => contacts.filter(isGroup), [contacts]); const individuals = React.useMemo(() => contacts.filter((c) => !isGroup(c)), [contacts]); diff --git a/src/stores/__tests__/contacts-store.test.ts b/src/stores/__tests__/contacts-store.test.ts index 90db6073..9a2ebb96 100644 --- a/src/stores/__tests__/contacts-store.test.ts +++ b/src/stores/__tests__/contacts-store.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; vi.mock('../../api/contacts', () => ({ getAddressBooks: vi.fn(), @@ -53,6 +53,7 @@ import { selectGroupMembers, sortContactsByName, selectCreateTargetBookId, + CONTACTS_STALE_MS, } from '../contacts-store'; import type { AddressBook, ContactCard } from '../../api/types'; @@ -144,6 +145,51 @@ describe('contacts-store', () => { }); }); + describe('fetchContactsIfStale (PF6)', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it('does not download the cards again while the last full fetch is fresh', async () => { + vi.useFakeTimers(); + mockGetAllContacts.mockResolvedValue([card('c1')]); + await useContactsStore.getState().fetchContacts(); + expect(mockGetAllContacts).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(CONTACTS_STALE_MS - 1000); + await useContactsStore.getState().fetchContactsIfStale(); + expect(mockGetAllContacts).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(2000); + await useContactsStore.getState().fetchContactsIfStale(); + expect(mockGetAllContacts).toHaveBeenCalledTimes(2); + }); + + it('fetches when nothing was loaded this session, after a reset or after a filtered load', async () => { + mockGetAllContacts.mockResolvedValue([card('c1')]); + await useContactsStore.getState().fetchContactsIfStale(); + expect(mockGetAllContacts).toHaveBeenCalledTimes(1); + + useContactsStore.getState().reset(); + await useContactsStore.getState().fetchContactsIfStale(); + expect(mockGetAllContacts).toHaveBeenCalledTimes(2); + + mockQueryContacts.mockResolvedValue([]); + await useContactsStore.getState().fetchContacts({ text: 'x' }); + await useContactsStore.getState().fetchContactsIfStale(); + expect(mockGetAllContacts).toHaveBeenCalledTimes(3); + }); + + it('retries on the next open when the last fetch failed', async () => { + mockGetAllContacts.mockRejectedValueOnce(new Error('offline')).mockResolvedValue([card('c1')]); + await useContactsStore.getState().fetchContactsIfStale(); + expect(useContactsStore.getState().contactsFetchedAt).toBe(0); + await useContactsStore.getState().fetchContactsIfStale(); + expect(mockGetAllContacts).toHaveBeenCalledTimes(2); + expect(useContactsStore.getState().contacts).toHaveLength(1); + }); + }); + describe('createContact', () => { it('should create and append to list', async () => { useContactsStore.setState({ contacts: [card('c1')], addressBooks: [{ id: 'ab-1', name: 'P' }] }); diff --git a/src/stores/contacts-store.ts b/src/stores/contacts-store.ts index 056ecb66..b4a588fb 100644 --- a/src/stores/contacts-store.ts +++ b/src/stores/contacts-store.ts @@ -61,6 +61,8 @@ export interface ContactsState { loading: boolean; error: string | null; hydrated: boolean; + /** When `contacts` last got every card from the server (0 = not this session). */ + contactsFetchedAt: number; // Trusted senders are stored as contacts in a dedicated JMAP address book so // the allow-list syncs across devices (matches the webmail behavior). @@ -78,6 +80,8 @@ export interface ContactsState { hydrate: () => Promise; fetchAddressBooks: () => Promise; fetchContacts: (filter?: { text?: string; inAddressBook?: string }) => Promise; + /** `fetchContacts()` unless every card was fetched within `CONTACTS_STALE_MS`. */ + fetchContactsIfStale: () => Promise; refresh: () => Promise; handleStateChange: (change: StateChange) => Promise; @@ -225,6 +229,12 @@ let addressBooksPromise: Promise | null = null; let contactsPromise: Promise | null = null; let trustedSendersPromise: Promise | null = null; +// Startup fetches every card and live `ContactCard` state changes refetch +// them, so opening Contacts only downloads the cards again once the last full +// fetch is this old - it covers changes missed while the app was in the +// background or live updates were down (PF6). +export const CONTACTS_STALE_MS = 5 * 60_000; + export const useContactsStore = create()( persist( (set, get) => { @@ -292,6 +302,7 @@ export const useContactsStore = create()( loading: false, error: null, hydrated: false, + contactsFetchedAt: 0, trustedSendersBookId: null, trustedSenderEmails: [], @@ -340,13 +351,15 @@ export const useContactsStore = create()( set({ loading: true, error: null }); try { let contacts: ContactCard[]; - if (filter && Object.keys(filter).length > 0) { + const filtered = !!filter && Object.keys(filter).length > 0; + if (filtered) { const ids = (await queryContacts(filter)) ?? []; contacts = ids.length > 0 ? ((await fetchContactsByIds(ids)) ?? []) : []; } else { contacts = (await fetchAllContacts()) ?? []; } - set({ contacts, loading: false }); + // A filtered load leaves only some of the cards in state. + set({ contacts, loading: false, contactsFetchedAt: filtered ? 0 : Date.now() }); } catch (err) { set({ loading: false, error: err instanceof Error ? err.message : 'Failed to load contacts' }); } @@ -356,6 +369,12 @@ export const useContactsStore = create()( return contactsPromise; }, + fetchContactsIfStale: async () => { + const fetchedAt = get().contactsFetchedAt; + if (fetchedAt > 0 && Date.now() - fetchedAt < CONTACTS_STALE_MS) return; + await get().fetchContacts(); + }, + refresh: async () => { await get().fetchContacts(); }, @@ -807,6 +826,7 @@ export const useContactsStore = create()( selectedCategory: { type: 'all' }, loading: false, error: null, + contactsFetchedAt: 0, trustedSendersBookId: null, trustedSenderEmails: [], trustedSendersLoaded: false,