fix: keep loading older mail when the server sends no total bulwarkmail/webmail#1364

src/stores/__tests__/email-store.test.ts

@@ -154,7 +154,7 @@ import { generateAccountId } from '../../lib/account-utils';
 const TEST_ACCOUNT_ID = generateAccountId('test@example.com', 'https://mail.example.com');
 
 import * as emailApi from '../../api/email';
-import { useEmailStore } from '../email-store';
+import { useEmailStore, pagingTotal } from '../email-store';
 import { useSettingsStore } from '../settings-store';
 
 const mockGetMailboxesWithState = emailApi.getMailboxesWithState as ReturnType<typeof vi.fn>;
@@ -402,6 +402,40 @@ describe('email-store', () => {
       expect(useEmailStore.getState().emails).toHaveLength(2);
     });
 
+    // #1364: a server that sends no `total` must not stop the list after
+    // its first page; full pages keep it going, a short one ends it.
+    it('keeps paging while full pages come back without a total', async () => {
+      useSettingsStore.getState().updateSetting('emailsPerPage', 2);
+      useEmailStore.setState({
+        currentMailboxId: 'mb-1',
+        emails: [{ id: 'e1' } as any, { id: 'e2' } as any],
+        totalEmails: pagingTotal(0, 2, 2, 2),
+        loading: false,
+      });
+      expect(useEmailStore.getState().totalEmails).toBe(3);
+
+      try {
+        mockQueryEmailPage.mockResolvedValueOnce({
+          ids: ['e3', 'e4'], total: 0, list: [{ id: 'e3' }, { id: 'e4' }], threads: [],
+        });
+        await useEmailStore.getState().loadMoreEmails();
+        mockQueryEmailPage.mockResolvedValueOnce({
+          ids: ['e5'], total: 0, list: [{ id: 'e5' }], threads: [],
+        });
+        await useEmailStore.getState().loadMoreEmails();
+      } finally {
+        useSettingsStore.getState().updateSetting('emailsPerPage', 25);
+      }
+
+      expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['e1', 'e2', 'e3', 'e4', 'e5']);
+      expect(mockQueryEmailPage).toHaveBeenLastCalledWith('mb-1', expect.objectContaining({ position: 4 }));
+      // The short page was the last one.
+      expect(useEmailStore.getState().totalEmails).toBe(5);
+      mockQueryEmailPage.mockClear();
+      await useEmailStore.getState().loadMoreEmails();
+      expect(mockQueryEmailPage).not.toHaveBeenCalled();
+    });
+
     it('should not load if already at total', async () => {
       useEmailStore.setState({
         currentMailboxId: 'mb-1',
@@ -1589,3 +1623,17 @@ describe('email-store', () => {
     });
   });
 });
+
+describe('pagingTotal', () => {
+  it('trusts a total the server gave', () => {
+    expect(pagingTotal(120, 25, 25, 25)).toBe(120);
+    expect(pagingTotal(25, 25, 25, 25)).toBe(25);
+    expect(pagingTotal(0, 0, 0, 25)).toBe(0);
+  });
+
+  it('reads a missing or too-small total from whether the page came back full', () => {
+    expect(pagingTotal(0, 25, 25, 25)).toBe(26);
+    expect(pagingTotal(0, 30, 5, 25)).toBe(30);
+    expect(pagingTotal(10, 50, 25, 25)).toBe(51);
+  });
+});

src/stores/email-store.ts

@@ -635,6 +635,22 @@ function listComparator(state: EmailState): (a: Email, b: Email) => number {
   return compareEmails(levels, { pinnedFirst: true });
 }
 
+/**
+ * The total that drives load-more. RFC 8620 makes Email/query's `total`
+ * optional and some servers leave it out (#1364); the API layer reads a
+ * missing one as 0, which stopped the list after its first page. A total
+ * below the rows already loaded is one the server didn't give: then a full
+ * page says there may be more (one more than loaded keeps load-more asking)
+ * and a short page that the end is reached. `rowsLoaded` counts this page.
+ */
+export function pagingTotal(serverTotal: number, rowsLoaded: number, pageLength: number, limit: number): number {
+  if (serverTotal > 0 && serverTotal >= rowsLoaded) return serverTotal;
+  // One Email/get caps a page below `emailsPerPage` on some servers.
+  const maxGet = typeof jmapClient.getMaxObjectsInGet === 'function' ? jmapClient.getMaxObjectsInGet() : limit;
+  const fullPage = pageLength > 0 && pageLength >= Math.min(limit, maxGet);
+  return fullPage ? rowsLoaded + 1 : rowsLoaded;
+}
+
 interface SpanningPage {
   /** Stamped rows of every account that answered, merged under the list order. */
   list: Email[];
@@ -684,7 +700,7 @@ async function fetchSpanningPage(
       if (!firstError) firstError = p.error;
       continue;
     }
-    page.total += p.total;
+    page.total += pagingTotal(p.total, (loaded[stamp] ?? 0) + p.list.length, p.list.length, limit);
     for (const e of p.list) page.list.push({ ...e, jmapAccountId: stamp });
     for (const th of p.threads) page.threads.push({ ...th, id: `${stamp}:${th.id}` });
   }
@@ -1143,7 +1159,7 @@ export const useEmailStore = create<EmailState>()(
         });
         return;
       }
-      const { list, total, threads } = await queryEmailPage(scope.mailboxId, {
+      const { list, total: serverTotal, ids, threads } = await queryEmailPage(scope.mailboxId, {
         position,
         limit,
         sort: await resolveSort(state, scope.accountId),
@@ -1152,6 +1168,7 @@ export const useEmailStore = create<EmailState>()(
         threads: !useSettingsStore.getState().disableThreading,
       });
       if (get().activeAccountId !== activeAccountId || get().currentMailboxId !== currentMailboxId) return;
+      const total = pagingTotal(serverTotal, position + ids.length, ids.length, limit);
       // A message that arrived between pages shifts positions and would come
       // back a second time — drop ids we already show (duplicate keys).
       const existingIds = new Set(get().emails.map((e) => e.id));
@@ -2832,7 +2849,9 @@ async function refreshEmailsImpl(): Promise<void> {
           const trimmed = out.slice(0, Math.max(limit, baseEmails.length));
 
           const nextQueryState = queryChanges.newQueryState;
-          const nextTotal = queryChanges.total;
+          // Without a total from the server, a window of at least a page
+          // may go on past it.
+          const nextTotal = pagingTotal(queryChanges.total, trimmed.length, trimmed.length, limit);
 
           if (viewChanged()) return;
 
@@ -2896,11 +2915,12 @@ async function refreshEmailsImpl(): Promise<void> {
 
       if (viewChanged()) return;
 
+      const total = pagingTotal(queryRes.total, queryRes.ids.length, queryRes.ids.length, limit);
       const updates: Partial<EmailState> = {
         // Rows the user just read/unstarred in this filtered view stay put
         // until the view is re-opened, instead of vanishing under them.
         emails: baseView ? queryRes.list : mergeRetainedRows(get().emails, queryRes.list, get().retainedIds),
-        totalEmails: queryRes.total,
+        totalEmails: total,
         threadCounts: withThreadCounts(get().threadCounts, queryRes.threads),
         loading: false,
       };
@@ -2916,7 +2936,7 @@ async function refreshEmailsImpl(): Promise<void> {
           ...get().mailboxSnapshots,
           [currentMailboxId]: {
             emails: queryRes.list,
-            total: queryRes.total,
+            total,
             queryState: queryRes.queryState,
           },
         };