fix: draw an opened message with its conversation once, and open a message link with its conversation instead of alone first

The conversation effect threw away the conversation the open had just put on screen and fetched it again, which drew the whole mail view once more per opened message. A message link showed the message alone, then a placeholder for the conversation, then the message again, moving the body down. The routing test now expects the link's fetch to leave opening to the mail view, which opens the message with its conversation.

components/mail/__tests__/mail-app-conversation-pane.test.tsx

@@ -21,13 +21,19 @@ vi.mock('next/dynamic', () => ({ default: (load: () => Promise<React.ComponentTy
     return <Suspense fallback={null}><Component {...props} /></Suspense>;
   };
 } }));
-type ViewerProps = { mvConversation?: { id: string }[]; conversationPane?: boolean };
+type ViewerProps = { email?: { id: string } | null; mvConversation?: { id: string }[]; mvConversationPending?: boolean; conversationPane?: boolean };
+// What the reader was drawn with, render by render: message, conversation, waiting.
+const readerDraws = vi.hoisted(() => [] as string[]);
 vi.mock('@/components/email/email-viewer', () => ({
-  EmailViewer: ({ mvConversation, conversationPane }: ViewerProps) => (
-    <div data-testid="reader" data-pane={conversationPane ? 'conversation' : 'message'}>
-      {mvConversation ? mvConversation.map((e) => e.id).join(',') : 'no conversation'}
-    </div>
-  ),
+  EmailViewer: ({ email, mvConversation, mvConversationPending, conversationPane }: ViewerProps) => {
+    const conversation = mvConversation ? mvConversation.map((e) => e.id).join(',') : 'no conversation';
+    readerDraws.push(`${email?.id ?? '-'} ${conversation}${mvConversationPending ? ' waiting' : ''}`);
+    return (
+      <div data-testid="reader" data-pane={conversationPane ? 'conversation' : 'message'}>
+        {conversation}
+      </div>
+    );
+  },
 }));
 vi.mock('@/components/email/email-composer', () => ({ EmailComposer: () => null }));
 vi.mock('@/components/layout/sidebar', () => ({ Sidebar: () => null }));
@@ -124,6 +130,7 @@ beforeEach(() => {
     fetchEmailContent,
   } as never);
   opened = [];
+  readerDraws.length = 0;
   unsubscribe = useEmailStore.subscribe((state, prev) => {
     if (state.selectedEmail !== prev.selectedEmail) opened.push(state.selectedEmail?.id ?? null);
   });
@@ -155,6 +162,35 @@ describe('The reading pane in the default layout', () => {
     await waitFor(() => expect(reader).toHaveTextContent('m0,m1'));
   });
 
+  it('opens a message with its conversation at once, in one draw', async () => {
+    useEmailStore.setState({ selectedEmail: inbox()[1], listRows: null } as never);
+    render(<MailApp />);
+    const reader = await screen.findByTestId('reader', {}, { timeout: 5000 });
+    await waitFor(() => expect(reader).toHaveTextContent('no conversation'));
+    readerDraws.length = 0;
+    await press('k');
+    await waitFor(() => expect(selectedId()).toBe('m1'));
+    await waitFor(() => expect(reader).toHaveTextContent('m0,m1'));
+    await act(async () => { await new Promise((resolve) => setTimeout(resolve, 50)); });
+    // The open brought the conversation with the message; fetching it again
+    // after the open drew the whole mail view once more.
+    expect(readerDraws.filter((draw) => draw.startsWith('m1 '))).toEqual(['m1 m0,m1']);
+  });
+
+  it('shows a linked message with its conversation, not alone first', async () => {
+    useEmailStore.setState({ selectedEmail: null } as never);
+    fetchEmailContent.mockImplementation(async (_client: unknown, id: string, _owner?: string, options?: { select?: boolean }) => {
+      const email = { ...inbox().find((e) => e.id === id)!, bodyValues: {} };
+      if (options?.select !== false) useEmailStore.setState({ selectedEmail: email } as never);
+      return email;
+    });
+    render(<MailApp linkSegments={['message', 'm1']} />);
+    const reader = await screen.findByTestId('reader', {}, { timeout: 5000 });
+    await waitFor(() => expect(reader).toHaveTextContent('m0,m1'), { timeout: 5000 });
+    await act(async () => { await new Promise((resolve) => setTimeout(resolve, 50)); });
+    expect(readerDraws.filter((draw) => draw.startsWith('m1 ') && draw !== 'm1 m0,m1')).toEqual([]);
+  });
+
   it('shows the message alone with "Show emails as individual messages"', async () => {
     useSettingsStore.setState({ disableThreading: true } as never);
     render(<MailApp />);

components/mail/__tests__/mail-app-source-routing.test.tsx

@@ -272,7 +272,8 @@ describe('shared message deep link from the personal Inbox (#1491)', () => {
     });
     const mounted = render(<MailApp linkSegments={['message', 'eaaaaab']} />);
     try {
-      await waitFor(() => expect(fetchContent).toHaveBeenCalledWith(client, 'eaaaaab', 'ce'));
+      // Fetched without opening: it opens with its conversation, in the reading pane.
+      await waitFor(() => expect(fetchContent).toHaveBeenCalledWith(client, 'eaaaaab', 'ce', { select: false }));
       await waitFor(() => expect(useEmailStore.getState().selectedMailbox).toBe('ce:a'));
       expect(fetchEmails).toHaveBeenCalledWith(client, 'ce:a', { background: true });
       expect(useEmailStore.getState().selectedEmail?.subject).toBe('GRP one');

components/mail/mail-app.tsx

@@ -396,6 +396,13 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
     threadId: string; emailId: string; startedAt: number; promise: Promise<Email[] | null>;
   } | null>(null);
   const mvConversationOpenSeqRef = useRef(0);
+  // The open that put a message on screen together with its conversation (or
+  // with none, a message alone in its thread). The effect below then has
+  // nothing to fetch, and resetting and setting it again would draw the whole
+  // mail view twice more for each message opened.
+  const mvConversationSettledRef = useRef<{
+    emailId: string; threadId: string; client: EmailBlobContext['client']; accountId?: string;
+  } | null>(null);
   // compose-p2-6: a reply to the open message saved as a draft. The reader
   // shows it under the message, and Reply opens it again. The conversation
   // fetch notes the thread's drafts; a scan reads them fresh after a composer
@@ -2299,11 +2306,30 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
       }
       setLoadingEmail(true);
       try {
-        const email = await fetchEmailContent(activeClient, link.id, link.ownerAccountId);
+        // With the conversation in the reader, the message waits for it, as on
+        // a click: shown alone first, it would move down once it came.
+        const withConversation = readerConversationView && !cursorOnly;
+        const email = withConversation
+          ? await fetchEmailContent(activeClient, link.id, link.ownerAccountId, { select: false })
+          : await fetchEmailContent(activeClient, link.id, link.ownerAccountId);
         if (!email) {
           toast.error(t('deep_link.message_not_found'));
           return;
         }
+        if (withConversation) {
+          const source = resolveEmailBlobContext(email, activeClient);
+          const conversation = await Promise.race([
+            loadMvConversation(email, activeClient),
+            new Promise<undefined>((resolve) => setTimeout(() => resolve(undefined), MV_CONVERSATION_WAIT_MS)),
+          ]);
+          if (conversation !== undefined) {
+            setMvConversationResult(conversation ? { emails: conversation, source, threadId: email.threadId } : null);
+            mvConversationSettledRef.current = {
+              emailId: email.id, threadId: email.threadId, client: source.client, accountId: source.accountId,
+            };
+          }
+          useEmailStore.setState({ selectedEmail: email });
+        }
         await showInOwningFolder(activeClient, email);
         if (link.fullscreen && !isMobile) {
           // The owning-folder branch above may have skipped selecting (the
@@ -4990,11 +5016,19 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
 
   useEffect(() => {
     ++mvConversationOpenSeqRef.current;
+    const settled = mvConversationSettledRef.current;
+    mvConversationSettledRef.current = null;
     if (!mvConversationThreadId || !client || !mvConversationRef) {
       setMvConversationResult(null);
       setMvConversationLoading(null);
       return;
     }
+    if (settled && settled.emailId === mvConversationRef.id && settled.threadId === mvConversationThreadId
+      && settled.client === mvBlobSource.client && settled.accountId === mvBlobSource.accountId) {
+      // The open already has the conversation on screen with the message.
+      setMvConversationLoading(null);
+      return () => { ++mvConversationOpenSeqRef.current; };
+    }
     let cancelled = false;
     setMvConversationResult(null);
     setMvConversationLoading(mvConversationThreadId);
@@ -5442,9 +5476,14 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
             new Promise<undefined>((resolve) => setTimeout(() => resolve(undefined), MV_CONVERSATION_WAIT_MS)),
           ]);
           if (openSeq !== emailOpenSeqRef.current) return 'opened';
-          if (conversation !== undefined) setMvConversationResult(conversation ? {
-            emails: conversation, source, threadId: fullEmail.threadId,
-          } : null);
+          if (conversation !== undefined) {
+            setMvConversationResult(conversation ? {
+              emails: conversation, source, threadId: fullEmail.threadId,
+            } : null);
+            mvConversationSettledRef.current = {
+              emailId: fullEmail.id, threadId: fullEmail.threadId, client: source.client, accountId: source.accountId,
+            };
+          }
         }
         // One write: the message, and the end of its loading state.
         finishOpenEmail(fullEmail);

stores/email-store.ts

@@ -467,8 +467,11 @@ interface EmailStore {
   // times; later calls are no-ops while a prior one is in flight.
   prefetchInitialData: (client: IJMAPClient) => Promise<void>;
   loadMoreEmails: (client: IJMAPClient) => Promise<void>;
-  /** `ownerAccountId`: the JMAP account a link names for the message (#1351). */
-  fetchEmailContent: (client: IJMAPClient, emailId: string, ownerAccountId?: string) => Promise<Email | null>;
+  /**
+   * `ownerAccountId`: the JMAP account a link names for the message (#1351).
+   * `select: false`: hand the message back without opening it.
+   */
+  fetchEmailContent: (client: IJMAPClient, emailId: string, ownerAccountId?: string, options?: { select?: boolean }) => Promise<Email | null>;
   fetchQuota: (client: IJMAPClient) => Promise<void>;
   /**
    * `options.outbox`: a send from the composer or quick reply, which offline
@@ -4349,7 +4352,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
     }
   },
 
-  fetchEmailContent: async (client, emailId, ownerAccountId) => {
+  fetchEmailContent: async (client, emailId, ownerAccountId, options) => {
     try {
       // Route to the owning account. In aggregate views (All Mail, unified,
       // cross-account) the selected mailbox is virtual, so derive the client +
@@ -4382,7 +4385,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
         annotatedEmail.accountId = listEmail?.accountId;
         annotatedEmail.accountLabel = listEmail?.accountLabel;
         annotatedEmail.sourceFolder = listEmail?.sourceFolder;
-        set({ selectedEmail: annotatedEmail });
+        if (options?.select !== false) set({ selectedEmail: annotatedEmail });
         return annotatedEmail;
       }
       return email;