diff --git a/app/views/RoomView/List/definitions.ts b/app/views/RoomView/List/definitions.ts index 7ed5c4d929e..163fd74f936 100644 --- a/app/views/RoomView/List/definitions.ts +++ b/app/views/RoomView/List/definitions.ts @@ -2,7 +2,7 @@ import { type RefObject } from 'react'; import { type FlatListProps } from 'react-native'; import { type FlatList } from 'react-native-gesture-handler'; -import { type TAnyMessageModel } from '../../../definitions'; +import { type RoomType, type TAnyMessageModel } from '../../../definitions'; export type TListRef = RefObject | null>; @@ -21,6 +21,7 @@ export interface IListContainerRef { export interface IListContainerProps { renderRow: Function; rid: string; + t: RoomType; tmid?: string; listRef: TListRef; hideSystemMessages: string[]; diff --git a/app/views/RoomView/List/hooks/buildVisibleSystemTypesClause.ts b/app/views/RoomView/List/hooks/buildVisibleSystemTypesClause.ts new file mode 100644 index 00000000000..4d20c4235bc --- /dev/null +++ b/app/views/RoomView/List/hooks/buildVisibleSystemTypesClause.ts @@ -0,0 +1,18 @@ +import { Q } from '@nozbe/watermelondb'; + +import { MESSAGE_TYPE_ANY_LOAD } from '../../../../lib/constants/messageTypeLoad'; + +/** + * When the user hides system message types, applying `take(N)` before filtering would return + * mostly hidden rows. This clause matches the JS filter `!m.t || !hideSystemMessages.includes(m.t)` + * so `take` applies to visible rows only. + */ +export function buildVisibleSystemTypesClause(hideSystemMessages: string[]): Q.Or | null { + if (!hideSystemMessages.length) { + return null; + } + + const notHidden = Q.and(...hideSystemMessages.map(h => Q.where('t', Q.notEq(h)))); + + return Q.or(Q.where('t', null), Q.where('t', Q.oneOf([...MESSAGE_TYPE_ANY_LOAD])), notHidden); +} diff --git a/app/views/RoomView/List/hooks/useMessages.test.tsx b/app/views/RoomView/List/hooks/useMessages.test.tsx new file mode 100644 index 00000000000..1e583491fc7 --- /dev/null +++ b/app/views/RoomView/List/hooks/useMessages.test.tsx @@ -0,0 +1,215 @@ +import React from 'react'; +import { act, renderHook, waitFor } from '@testing-library/react-native'; +import { Provider } from 'react-redux'; + +import { ROOM } from '../../../../actions/actionsTypes'; +import { type RoomType, type TAnyMessageModel } from '../../../../definitions'; +import database from '../../../../lib/database'; +import { getMessageById } from '../../../../lib/database/services/Message'; +import { getThreadById } from '../../../../lib/database/services/Thread'; +import { MessageTypeLoad } from '../../../../lib/constants/messageTypeLoad'; +import { mockedStore } from '../../../../reducers/mockedStore'; +import { useMessages } from './useMessages'; + +jest.mock('../../../../lib/database', () => ({ + __esModule: true, + default: { + active: { + get: jest.fn() + } + } +})); + +jest.mock('../../../../lib/database/services/Message', () => ({ + getMessageById: jest.fn(() => Promise.resolve(null)) +})); + +jest.mock('../../../../lib/database/services/Thread', () => ({ + getThreadById: jest.fn(() => Promise.resolve(null)) +})); + +jest.mock('../../../../lib/services/restApi', () => ({ + readThreads: jest.fn(() => Promise.resolve()) +})); + +jest.mock('../../../../lib/methods/helpers', () => { + const actual = jest.requireActual('../../../../lib/methods/helpers'); + return { + ...actual, + useDebounce: (fn: (...args: unknown[]) => unknown) => fn + }; +}); + +const mockDbGet = database.active.get as unknown as jest.Mock; +const mockGetThreadById = jest.mocked(getThreadById); +const mockGetMessageById = jest.mocked(getMessageById); + +const baseArgs = { + rid: 'ROOM_ID', + showMessageInMainThread: true, + hideSystemMessages: [] as string[], + serverVersion: '3.0.0' as string | null, + t: 'c' as RoomType +}; + +const msg = (overrides: Partial & { id: string }): TAnyMessageModel => + ({ + ts: new Date('2024-01-01'), + ...overrides + } as TAnyMessageModel); + +describe('useMessages', () => { + let emittedRows: TAnyMessageModel[]; + + const wrapper = ({ children }: { children: React.ReactNode }) => {children}; + + beforeEach(() => { + emittedRows = []; + jest.clearAllMocks(); + mockDbGet.mockImplementation(() => ({ + query: jest.fn().mockReturnValue({ + observe: () => ({ + subscribe: (onNext: (rows: TAnyMessageModel[]) => void) => { + onNext(emittedRows); + return { unsubscribe: jest.fn() }; + } + }) + }) + })); + }); + + const renderUseMessages = (overrides: Partial[0]> = {}) => + renderHook(() => useMessages({ ...baseArgs, ...overrides }), { wrapper }); + + it('returns fetchMessages as a function', async () => { + emittedRows = [msg({ id: 'm1' })]; + const { result } = renderUseMessages(); + await waitFor(() => { + expect(typeof result.current[2]).toBe('function'); + }); + }); + + it('loads main room messages from the messages collection', async () => { + emittedRows = [msg({ id: 'a' }), msg({ id: 'b' })]; + const { result } = renderUseMessages({ rid: 'R1' }); + await waitFor(() => { + expect(result.current[0].map(m => m.id)).toEqual(['a', 'b']); + }); + expect(mockDbGet).toHaveBeenCalledWith('messages'); + }); + + it('does not query the database when rid is empty', async () => { + emittedRows = [msg({ id: 'x' })]; + const { result } = renderUseMessages({ rid: '' }); + await act(async () => { + await result.current[2](); + }); + expect(mockDbGet).not.toHaveBeenCalled(); + }); + + it('filters out system message types listed in hideSystemMessages', async () => { + emittedRows = [msg({ id: '1', t: 'uj' }), msg({ id: '2', t: undefined }), msg({ id: '3', t: 'room_changed_topic' })]; + const { result } = renderUseMessages({ hideSystemMessages: ['uj'] }); + await waitFor(() => { + expect(result.current[0].map(m => m.id)).toEqual(['2', '3']); + }); + }); + + it('returns visibleMessagesIds aligned with visible messages', async () => { + emittedRows = [msg({ id: 'p' }), msg({ id: 'q' })]; + const { result } = renderUseMessages(); + await waitFor(() => { + expect(result.current[1].current).toEqual(['p', 'q']); + }); + }); + + it('dispatches room history request when server is 3.16+, user hides system messages, and a load row exists', async () => { + const dispatchSpy = jest.spyOn(mockedStore, 'dispatch'); + emittedRows = [msg({ id: 'm1', t: undefined }), msg({ id: 'load-more-x', t: MessageTypeLoad.MORE })]; + renderUseMessages({ + serverVersion: '6.0.0', + hideSystemMessages: ['uj'] + }); + await waitFor(() => { + expect(dispatchSpy).toHaveBeenCalledWith( + expect.objectContaining({ + type: ROOM.HISTORY_REQUEST, + rid: 'ROOM_ID', + t: 'c', + loaderId: 'load-more-x' + }) + ); + }); + dispatchSpy.mockRestore(); + }); + + it('does not dispatch roomHistoryRequest again when loaderId has not changed', async () => { + const dispatchSpy = jest.spyOn(mockedStore, 'dispatch'); + emittedRows = [msg({ id: 'load-more-x', t: MessageTypeLoad.MORE })]; + + const { rerender } = renderUseMessages({ + serverVersion: '6.0.0', + hideSystemMessages: ['uj'] + }); + + await waitFor(() => { + const historyDispatches = dispatchSpy.mock.calls.filter(([a]: any) => a?.type === ROOM.HISTORY_REQUEST); + expect(historyDispatches.length).toBe(1); + }); + + // Simulate a new message arriving — visibleMessages changes but loaderId stays the same + emittedRows = [msg({ id: 'new-msg' }), msg({ id: 'load-more-x', t: MessageTypeLoad.MORE })]; + rerender({}); + + await waitFor(() => { + const historyDispatches = dispatchSpy.mock.calls.filter(([a]: any) => a?.type === ROOM.HISTORY_REQUEST); + expect(historyDispatches.length).toBe(1); // still only once + }); + + dispatchSpy.mockRestore(); + }); + + it('does not dispatch room history request when server is below 3.16', async () => { + const dispatchSpy = jest.spyOn(mockedStore, 'dispatch'); + emittedRows = [msg({ id: 'load-more-x', t: MessageTypeLoad.MORE })]; + const { result } = renderUseMessages({ + serverVersion: '3.15.0', + hideSystemMessages: ['uj'] + }); + await waitFor(() => { + expect(result.current[0].length).toBeGreaterThan(0); + }); + const historyDispatches = dispatchSpy.mock.calls.filter(([a]: any) => a?.type === ROOM.HISTORY_REQUEST); + expect(historyDispatches.length).toBe(0); + dispatchSpy.mockRestore(); + }); + + it('queries thread_messages and appends thread parent when tmid is set', async () => { + const parent = { + ...msg({ id: 'parent-thread', t: 'discussion-created' }), + collection: { table: 'threads' } + } as TAnyMessageModel; + mockGetThreadById.mockResolvedValueOnce(parent); + emittedRows = [msg({ id: 'tm1', tmid: 'THREAD_ID' })]; + const { result } = renderUseMessages({ tmid: 'THREAD_ID' }); + await waitFor(() => { + expect(mockDbGet).toHaveBeenCalledWith('thread_messages'); + }); + await waitFor(() => { + const ids = result.current[0].map(m => m.id); + expect(ids).toContain('parent-thread'); + expect(ids).toContain('tm1'); + }); + }); + + it('falls back to getMessageById when thread record is missing', async () => { + const parent = msg({ id: 'fallback-parent', t: undefined }); + mockGetThreadById.mockResolvedValueOnce(null); + mockGetMessageById.mockResolvedValueOnce(parent as TAnyMessageModel); + emittedRows = [msg({ id: 'only-child', tmid: 'TM' })]; + const { result } = renderUseMessages({ tmid: 'TM' }); + await waitFor(() => { + expect(result.current[0].map(m => m.id)).toContain('fallback-parent'); + }); + }); +}); diff --git a/app/views/RoomView/List/hooks/useMessages.ts b/app/views/RoomView/List/hooks/useMessages.ts index da21e3193be..58a9dae7f2f 100644 --- a/app/views/RoomView/List/hooks/useMessages.ts +++ b/app/views/RoomView/List/hooks/useMessages.ts @@ -1,33 +1,41 @@ -import { useCallback, useLayoutEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; import { Q } from '@nozbe/watermelondb'; import { type Subscription } from 'rxjs'; +import { useDispatch } from 'react-redux'; -import { type TAnyMessageModel } from '../../../../definitions'; +import { type RoomType, type TAnyMessageModel } from '../../../../definitions'; import database from '../../../../lib/database'; import { getMessageById } from '../../../../lib/database/services/Message'; import { getThreadById } from '../../../../lib/database/services/Thread'; import { compareServerVersion, useDebounce } from '../../../../lib/methods/helpers'; import { readThreads } from '../../../../lib/services/restApi'; +import { MESSAGE_TYPE_ANY_LOAD, type MessageTypeLoad } from '../../../../lib/constants/messageTypeLoad'; import { QUERY_SIZE } from '../constants'; +import { buildVisibleSystemTypesClause } from './buildVisibleSystemTypesClause'; +import { roomHistoryRequest } from '../../../../actions/room'; export const useMessages = ({ rid, tmid, showMessageInMainThread, serverVersion, - hideSystemMessages + hideSystemMessages, + t }: { rid: string; tmid?: string; showMessageInMainThread: boolean; serverVersion: string | null; hideSystemMessages: string[]; + t: RoomType; }) => { - const [messages, setMessages] = useState([]); + const [rawMessages, setRawMessages] = useState([]); const thread = useRef(null); const count = useRef(0); const subscription = useRef(null); const messagesIds = useRef([]); + const lastDispatchedLoaderId = useRef(null); + const dispatch = useDispatch(); const fetchMessages = useCallback(async () => { unsubscribe(); @@ -38,6 +46,11 @@ export const useMessages = ({ } const db = database.active; + + // Apply the filter to the DB query. This guarantees Q.take() grabs + // exactly enough messages to keep pagination from breaking. + const visibleSystemClause = hideSystemMessages.length ? buildVisibleSystemTypesClause(hideSystemMessages) : null; + let observable; if (tmid) { // If the thread doesn't exist yet, we fetch it from messages, but trying to get it from threads when possible. @@ -50,13 +63,22 @@ export const useMessages = ({ } observable = db .get('thread_messages') - .query(Q.where('rid', tmid), Q.sortBy('ts', Q.desc), Q.skip(0), Q.take(count.current)) + .query( + Q.where('rid', tmid), + ...(visibleSystemClause ? [visibleSystemClause] : []), + Q.sortBy('ts', Q.desc), + Q.skip(0), + Q.take(count.current) + ) .observe(); } else { - const whereClause = [Q.where('rid', rid), Q.sortBy('ts', Q.desc), Q.skip(0), Q.take(count.current)] as ( - | Q.WhereDescription - | Q.Or - )[]; + const whereClause: Q.Clause[] = [ + Q.where('rid', rid), + ...(visibleSystemClause ? [visibleSystemClause] : []), + Q.sortBy('ts', Q.desc), + Q.skip(0), + Q.take(count.current) + ]; if (!showMessageInMainThread) { whereClause.push(Q.or(Q.where('tmid', null), Q.where('tshow', Q.eq(true)))); } @@ -67,24 +89,16 @@ export const useMessages = ({ } subscription.current = observable.subscribe(result => { - let newMessages: TAnyMessageModel[] = result; + const newMessages: TAnyMessageModel[] = [...result]; + if (tmid && thread.current) { newMessages.push(thread.current); } - /** - * Since 3.16.0 server version, the backend don't response with messages if - * hide system message is enabled - */ - if (compareServerVersion(serverVersion, 'lowerThan', '3.16.0') || hideSystemMessages.length) { - newMessages = newMessages.filter(m => !m.t || !hideSystemMessages?.includes(m.t)); - } - readThread(); - setMessages(newMessages); - messagesIds.current = newMessages.map(m => m.id); + setRawMessages(newMessages); }); - }, [rid, tmid, showMessageInMainThread, serverVersion, hideSystemMessages]); + }, [rid, tmid, showMessageInMainThread, hideSystemMessages]); // hideSystemMessages must be here so the DB re-queries for proper pagination const readThread = useDebounce(async () => { if (tmid) { @@ -102,11 +116,47 @@ export const useMessages = ({ return () => { unsubscribe(); }; - }, [rid, tmid, showMessageInMainThread, serverVersion, hideSystemMessages, fetchMessages]); + }, [fetchMessages]); const unsubscribe = () => { subscription.current?.unsubscribe(); }; - return [messages, messagesIds, fetchMessages] as const; + const visibleMessages = useMemo(() => { + const filtered = + !hideSystemMessages || hideSystemMessages.length === 0 + ? rawMessages + : rawMessages.filter(m => !m.t || !hideSystemMessages.includes(m.t)); + + // Derive IDs in the same memo so the ref is never out of sync + // with the visible list, even under concurrent rendering. + messagesIds.current = filtered.map(m => m.id); + + return filtered; + }, [rawMessages, hideSystemMessages]); + + /** + * Since 3.16.0 server version, the backend don't response with messages if + * hide system message is enabled + */ + useEffect(() => { + if (!compareServerVersion(serverVersion, 'greaterThanOrEqualTo', '3.16.0') || !hideSystemMessages.length) { + return; + } + + const loaderId = visibleMessages.find(m => m.t && MESSAGE_TYPE_ANY_LOAD.includes(m.t as MessageTypeLoad))?.id; + + // Only dispatch if a loader exists AND it's a different one + // from the last dispatch — prevents hammering on every message update. + if (loaderId && loaderId !== lastDispatchedLoaderId.current) { + lastDispatchedLoaderId.current = loaderId; + dispatch(roomHistoryRequest({ rid, t, loaderId })); + } + }, [serverVersion, rid, t, hideSystemMessages, visibleMessages, dispatch]); + + useEffect(() => { + lastDispatchedLoaderId.current = null; + }, [rid]); + + return [visibleMessages, messagesIds, fetchMessages] as const; }; diff --git a/app/views/RoomView/List/index.tsx b/app/views/RoomView/List/index.tsx index 2b7c3839ee9..f6d2a348db4 100644 --- a/app/views/RoomView/List/index.tsx +++ b/app/views/RoomView/List/index.tsx @@ -7,13 +7,14 @@ import { type IListContainerProps, type IListContainerRef, type IListProps } fro import { useMessages, useScroll } from './hooks'; const ListContainer = forwardRef( - ({ rid, tmid, renderRow, showMessageInMainThread, serverVersion, hideSystemMessages, listRef }, ref) => { + ({ rid, tmid, t, renderRow, showMessageInMainThread, hideSystemMessages, listRef, serverVersion }, ref) => { const [messages, messagesIds, fetchMessages] = useMessages({ rid, tmid, showMessageInMainThread, - serverVersion, - hideSystemMessages + hideSystemMessages, + t, + serverVersion }); const { jumpToBottom, diff --git a/app/views/RoomView/index.tsx b/app/views/RoomView/index.tsx index 2622040f562..056ec2dfd03 100644 --- a/app/views/RoomView/index.tsx +++ b/app/views/RoomView/index.tsx @@ -1671,6 +1671,7 @@ class RoomView extends React.Component { ref={this.list} listRef={this.flatList} rid={rid} + t={t as RoomType} tmid={this.tmid} renderRow={this.renderItem} hideSystemMessages={this.hideSystemMessages}