refactor: drop unnecessary channel query when jumping to a reply in channel msg list

Author: MartinCupela

src/components/Message/hooks/__tests__/useMessageAlsoSentInChannelNavigation.test.tsx

@@ -0,0 +1,117 @@
+import { renderHook } from '@testing-library/react';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+// Shared spies + mutable per-test state. `vi.hoisted` runs before the `vi.mock` factories below, so
+// they can close over these.
+const mocks = vi.hoisted(() => ({
+  ingestChannel: vi.fn(),
+  jumpToMessage: vi.fn(() => Promise.resolve(true)),
+  open: vi.fn(),
+  // `query` should never be called by the hook (the jump bootstraps the channel) — asserted below.
+  query: vi.fn(() => Promise.resolve()),
+  state: {
+    channelSlot: undefined as string | undefined,
+    message: { id: 'reply-1', parent_id: 'parent-1', show_in_channel: true } as
+      | { id: string; parent_id?: string; show_in_channel?: boolean }
+      | undefined,
+    thread: undefined as unknown,
+  },
+}));
+
+vi.mock('../../../../context', () => ({
+  useChannel: () => ({
+    cid: 'messaging:general',
+    initialized: false,
+    messagePaginator: { jumpToMessage: mocks.jumpToMessage },
+    query: mocks.query,
+  }),
+  useChatContext: () => ({
+    channelPaginatorsOrchestrator: { ingestChannel: mocks.ingestChannel },
+    client: {
+      getThread: vi.fn(),
+      notifications: { addError: vi.fn() },
+      threads: { threadsById: {} },
+    },
+  }),
+  useMessageContext: () => ({ message: mocks.state.message }),
+  useTranslationContext: () => ({ t: (key: string) => key }),
+}));
+
+vi.mock('../../../ChatView', () => ({
+  useChatViewNavigation: () => ({ open: mocks.open }),
+  useSlotForKey: () => mocks.state.channelSlot,
+}));
+
+vi.mock('../../../Threads', () => ({ useThreadContext: () => mocks.state.thread }));
+
+import { useMessageAlsoSentInChannelNavigation } from '../useMessageAlsoSentInChannelNavigation';
+
+const renderNavigation = () =>
+  renderHook(() => useMessageAlsoSentInChannelNavigation()).result;
+
+describe('useMessageAlsoSentInChannelNavigation', () => {
+  beforeEach(() => {
+    mocks.state.channelSlot = undefined;
+    mocks.state.thread = undefined;
+    mocks.state.message = { id: 'reply-1', parent_id: 'parent-1', show_in_channel: true };
+    vi.clearAllMocks();
+  });
+
+  it('derives isInThread / isShownInChannel from context', () => {
+    mocks.state.thread = { id: 'parent-1' };
+    const result = renderNavigation();
+    expect(result.current.isInThread).toBe(true);
+    expect(result.current.isShownInChannel).toBe(true);
+
+    mocks.state.thread = undefined;
+    mocks.state.message = { id: 'reply-1', show_in_channel: false };
+    const next = renderNavigation();
+    expect(next.current.isInThread).toBe(false);
+    expect(next.current.isShownInChannel).toBe(false);
+  });
+
+  describe('viewReplyInChannel', () => {
+    it('when the channel is not shown: opens it, jumps, then ingests — with no separate init query', async () => {
+      mocks.state.channelSlot = undefined; // channel absent from the active view → navigate
+
+      const result = renderNavigation();
+      await result.current.viewReplyInChannel('reply-1');
+
+      expect(mocks.open).toHaveBeenCalledTimes(1);
+      expect(mocks.open).toHaveBeenCalledWith(
+        expect.objectContaining({ key: 'messaging:general', kind: 'channel' }),
+      );
+      expect(mocks.jumpToMessage).toHaveBeenCalledWith('reply-1');
+      expect(mocks.ingestChannel).toHaveBeenCalledTimes(1);
+      // The jump's own `channel.query({ messages: { id_around } })` bootstraps the channel, so we
+      // must NOT fire a separate init query.
+      expect(mocks.query).not.toHaveBeenCalled();
+
+      // Order: open → jumpToMessage → ingestChannel (ingest after the jump so channel.data is loaded).
+      const openOrder = mocks.open.mock.invocationCallOrder[0];
+      const jumpOrder = mocks.jumpToMessage.mock.invocationCallOrder[0];
+      const ingestOrder = mocks.ingestChannel.mock.invocationCallOrder[0];
+      expect(openOrder).toBeLessThan(jumpOrder);
+      expect(ingestOrder).toBeGreaterThan(jumpOrder);
+    });
+
+    it('when the channel is already shown: only jumps (no navigate/ingest/query)', async () => {
+      mocks.state.channelSlot = 'main-channel'; // channel already bound in the active view
+
+      const result = renderNavigation();
+      await result.current.viewReplyInChannel('reply-1');
+
+      expect(mocks.jumpToMessage).toHaveBeenCalledWith('reply-1');
+      expect(mocks.open).not.toHaveBeenCalled();
+      expect(mocks.ingestChannel).not.toHaveBeenCalled();
+      expect(mocks.query).not.toHaveBeenCalled();
+    });
+
+    it('defaults the target to the current message id', async () => {
+      mocks.state.channelSlot = 'main-channel';
+      const result = renderNavigation();
+      await result.current.viewReplyInChannel();
+      expect(mocks.jumpToMessage).toHaveBeenCalledWith('reply-1');
+    });
+  });
+});

src/components/Message/hooks/useMessageAlsoSentInChannelNavigation.ts

@@ -58,29 +58,17 @@ export const useMessageAlsoSentInChannelNavigation =
 
     const viewReplyInChannel = async (messageId = message?.id) => {
       if (!messageId) return;
-      if (!channelSlot) {
-        // The channel isn't shown in the active view — navigate to it. `open` resolves the
-        // channel kind's view from the slot registry and switches there (no hard-coded view name);
-        // `ingestChannel` surfaces it in the channel list(s) incrementally instead of forcing a
-        // full list re-query.
+      // The channel isn't shown in the active view when it has no slot — we need to navigate to it.
+      const needsNavigation = !channelSlot;
+      if (needsNavigation) {
         open({ key: channel.cid ?? undefined, kind: 'channel', source: channel });
-        // Load the channel's state (members, config, read state; registers it in
-        // `client.activeChannels`) so `ingestChannel` below can match it against paginator filters
-        // and the panel has data to render. `messages: { limit: 0 }` fetches no messages — the
-        // `jumpToMessage` call at the end loads the message window around the target, so pulling a
-        // default page here would be wasted, immediately-superseded work. `channel.initialized` is
-        // only set by `channel.watch()` (the channel list watches its channels), so this guard
-        // effectively means "not already loaded via the list"; a plain `query()` neither watches
-        // nor flips `initialized`.
-        if (!channel.initialized) {
-          await channel.query({ messages: { limit: 0 } });
-        }
-        // Surface the (now initialized) channel in the list(s) so its data is available for filter
-        // matching; `ingestChannel` dedupes by cid and inserts in sort order — safe to call always.
-        channelPaginatorsOrchestrator.ingestChannel(channel);
       }
 
       await channel.messagePaginator.jumpToMessage(messageId);
+
+      if (needsNavigation) {
+        channelPaginatorsOrchestrator.ingestChannel(channel);
+      }
     };
 
     const viewReplyInThread = async (