diff --git a/.changeset/dm-routing.md b/.changeset/dm-routing.md new file mode 100644 index 000000000..019d56853 --- /dev/null +++ b/.changeset/dm-routing.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Route DM rooms to /direct before checking space parents to fix incorrect navigation. diff --git a/src/app/hooks/useRoomNavigate.test.ts b/src/app/hooks/useRoomNavigate.test.ts new file mode 100644 index 000000000..353285a1e --- /dev/null +++ b/src/app/hooks/useRoomNavigate.test.ts @@ -0,0 +1,111 @@ +import { renderHook } from '@testing-library/react'; +import { createElement, type ReactNode } from 'react'; +import { describe, expect, it, vi, beforeEach } from 'vitest'; +import { createStore, Provider } from 'jotai'; +import { mDirectAtom } from '$state/mDirectList'; +import { roomToParentsAtom } from '$state/room/roomToParents'; +import { useRoomNavigate } from './useRoomNavigate'; + +const mockNavigate = vi.fn<(path: string) => void>(); + +// Preserve the real generatePath (used by $pages/pathUtils) while stubbing useNavigate. +vi.mock('react-router-dom', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + useNavigate: () => mockNavigate, + }; +}); + +vi.mock('$hooks/useMatrixClient', () => ({ + useMatrixClient: () => ({ getRoom: vi.fn<() => null>() }), +})); + +// Return the roomId as-is so path assertions are straightforward. +vi.mock('$utils/matrix', () => ({ + getCanonicalAliasOrRoomId: (_mx: unknown, roomId: string) => roomId, +})); + +vi.mock('$hooks/router/useSelectedSpace', () => ({ + useSelectedSpace: () => undefined, +})); + +vi.mock('$state/hooks/settings', () => ({ + useSetting: () => [false, vi.fn()], +})); + +function makeWrapper(store: ReturnType) { + return function Wrapper({ children }: { children: ReactNode }) { + return createElement(Provider, { store }, children); + }; +} + +describe('useRoomNavigate', () => { + beforeEach(() => { + mockNavigate.mockReset(); + }); + + describe('navigateRoom', () => { + it('routes a DM room to /direct even when it has space parents (regression)', () => { + // Regression guard: before the fix, a DM room that also appeared in + // roomToParents would be routed to the space path instead of /direct. + const store = createStore(); + const dmRoomId = '!dm:example.org'; + const spaceId = '!space:example.org'; + + store.set(mDirectAtom, { type: 'INITIALIZE', rooms: new Set([dmRoomId]) }); + const roomToParents = new Map>(); + roomToParents.set(dmRoomId, new Set([spaceId])); + store.set(roomToParentsAtom, { type: 'INITIALIZE', roomToParents }); + + const { result } = renderHook(() => useRoomNavigate(), { + wrapper: makeWrapper(store), + }); + + result.current.navigateRoom(dmRoomId); + + expect(mockNavigate).toHaveBeenCalledOnce(); + expect(mockNavigate.mock.calls[0]![0]).toMatch(/^\/direct\//); + }); + + it('routes a non-DM room with an orphan space parent through the space path', () => { + const store = createStore(); + const roomId = '!room:example.org'; + const spaceId = '!space:example.org'; + + store.set(mDirectAtom, { type: 'INITIALIZE', rooms: new Set() }); + // spaceId is a parent of roomId and is itself an orphan (top-level space) + const roomToParents = new Map>(); + roomToParents.set(roomId, new Set([spaceId])); + store.set(roomToParentsAtom, { type: 'INITIALIZE', roomToParents }); + + const { result } = renderHook(() => useRoomNavigate(), { + wrapper: makeWrapper(store), + }); + + result.current.navigateRoom(roomId); + + expect(mockNavigate).toHaveBeenCalledOnce(); + const navigatedPath = mockNavigate.mock.calls[0]![0] as string; + expect(navigatedPath).not.toMatch(/^\/direct\//); + expect(navigatedPath).not.toMatch(/^\/home\//); + }); + + it('routes an orphan room with no parents to /home', () => { + const store = createStore(); + const roomId = '!room:example.org'; + + store.set(mDirectAtom, { type: 'INITIALIZE', rooms: new Set() }); + store.set(roomToParentsAtom, { type: 'INITIALIZE', roomToParents: new Map() }); + + const { result } = renderHook(() => useRoomNavigate(), { + wrapper: makeWrapper(store), + }); + + result.current.navigateRoom(roomId); + + expect(mockNavigate).toHaveBeenCalledOnce(); + expect(mockNavigate.mock.calls[0]![0]).toMatch(/^\/home\//); + }); + }); +}); diff --git a/src/app/hooks/useRoomNavigate.ts b/src/app/hooks/useRoomNavigate.ts index 8e4abb172..791efa6c5 100644 --- a/src/app/hooks/useRoomNavigate.ts +++ b/src/app/hooks/useRoomNavigate.ts @@ -36,6 +36,13 @@ export const useRoomNavigate = () => { const navigateRoom = useCallback( (roomId: string, eventId?: string, opts?: NavigateOptions) => { const roomIdOrAlias = getCanonicalAliasOrRoomId(mx, roomId); + + // DM rooms always navigate to /direct, regardless of space membership. + if (mDirects.has(roomId)) { + navigate(getDirectRoomPath(roomIdOrAlias, eventId), opts); + return; + } + const openSpaceTimeline = developerTools && spaceSelectedId === roomId; const orphanParents = openSpaceTimeline ? [roomId] : getOrphanParents(roomToParents, roomId); @@ -56,11 +63,6 @@ export const useRoomNavigate = () => { return; } - if (mDirects.has(roomId)) { - navigate(getDirectRoomPath(roomIdOrAlias, eventId), opts); - return; - } - navigate(getHomeRoomPath(roomIdOrAlias, eventId), opts); }, [mx, navigate, spaceSelectedId, roomToParents, mDirects, developerTools]