Skip to content
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/slimy-mugs-own.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Fixed an issue where messages in an open thread panel would disappear when clicking "Jump to recent messages" in the main channel.
137 changes: 137 additions & 0 deletions apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import { RoomHistoryManager } from './RoomHistoryManager';
import type { MessageRecord } from '../../../../client/stores/Messages';
import { Messages } from '../../../../client/stores/Messages';

jest.mock('@rocket.chat/emitter');
jest.mock('date-fns');

jest.mock('../../../../client/lib/onClientMessageReceived', () => ({
onClientMessageReceived: jest.fn((msg) => msg),
}));
jest.mock('../../../../client/lib/user', () => ({
getUserId: jest.fn(),
}));
jest.mock('../../../../client/lib/utils/callWithErrorHandling', () => ({
callWithErrorHandling: jest.fn(),
}));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
jest.mock('../../../../client/lib/utils/getConfig', () => ({
getConfig: jest.fn(() => '50'),
}));
jest.mock('../../../../client/lib/utils/waitForElement');
jest.mock('../../../../client/stores', () => {
const actual = jest.requireActual('../../../../client/stores/Messages');
return {
Messages: actual.Messages,
Subscriptions: {
state: {
find: jest.fn(),
},
},
};
});
jest.mock('../../../utils/client', () => ({
getUserPreference: jest.fn(),
}));

describe('RoomHistoryManager.clear', () => {
const rid = 'test-room-id';
const otherRid = 'other-room-id';
const threadId = 'thread-123';

const createMessage = (partial: Partial<MessageRecord> & Pick<MessageRecord, '_id' | 'rid'>): MessageRecord =>
({
msg: '',
ts: new Date(),
_updatedAt: new Date(),
u: { _id: 'user-id', username: 'test-user', name: 'Test User' },
...partial,
}) as MessageRecord;

const messages: MessageRecord[] = [
createMessage({ _id: 'msg1', rid, msg: 'Main channel message 1' }),
createMessage({ _id: 'msg2', rid, msg: 'Main channel message 2' }),
createMessage({ _id: 'thread-msg-1', rid, tmid: threadId, msg: 'Thread message 1' }),
createMessage({ _id: 'thread-msg-2', rid, tmid: threadId, msg: 'Thread message 2' }),
createMessage({ _id: 'other-thread-msg', rid, tmid: 'other-thread-456', msg: 'Other thread message' }),
createMessage({ _id: 'other-room-msg', rid: otherRid, msg: 'Other room message' }),
createMessage({ _id: 'other-room-thread', rid: otherRid, tmid: threadId, msg: 'Other room thread message' }),
];

beforeEach(() => {
Messages.state.replaceAll([]);
Messages.state.storeMany(messages);
(RoomHistoryManager as any).histories = {};
});

it('should remove all messages for a room when no filter is provided', () => {
expect(Messages.state.filter((m) => m.rid === rid).length).toBe(5);

RoomHistoryManager.clear(rid);

expect(Messages.state.filter((m) => m.rid === rid).length).toBe(0);
expect(Messages.state.filter((m) => m.rid === otherRid).length).toBe(2);
expect(Messages.state.get('other-room-msg')).toBeDefined();
expect(Messages.state.get('other-room-thread')).toBeDefined();
});

it('should preserve messages that do not match the filter predicate', () => {
expect(Messages.state.filter((m) => m.rid === rid).length).toBe(5);
expect(Messages.state.filter((m) => m.rid === otherRid).length).toBe(2);

RoomHistoryManager.clear(rid, (record: MessageRecord) => !record.tmid || record.tmid !== threadId);

const remainingMessages = Messages.state.filter((m) => m.rid === rid);
expect(remainingMessages.length).toBe(2);
expect(remainingMessages.every((m) => m.tmid === threadId)).toBe(true);
expect(Messages.state.get('thread-msg-1')).toBeDefined();
expect(Messages.state.get('thread-msg-2')).toBeDefined();

expect(Messages.state.get('msg1')).toBeUndefined();
expect(Messages.state.get('msg2')).toBeUndefined();

expect(Messages.state.get('other-thread-msg')).toBeUndefined();

expect(Messages.state.filter((m) => m.rid === otherRid).length).toBe(2);
expect(Messages.state.get('other-room-msg')).toBeDefined();
expect(Messages.state.get('other-room-thread')).toBeDefined();
});

it('should remove all messages including threads when filter returns true for all', () => {
expect(Messages.state.filter((m) => m.rid === rid).length).toBe(5);

RoomHistoryManager.clear(rid, () => true);

expect(Messages.state.filter((m) => m.rid === rid).length).toBe(0);
});

it('should preserve all messages when filter returns false for all', () => {
expect(Messages.state.filter((m) => m.rid === rid).length).toBe(5);

RoomHistoryManager.clear(rid, () => false);

expect(Messages.state.filter((m) => m.rid === rid).length).toBe(5);
expect(Messages.state.get('msg1')).toBeDefined();
expect(Messages.state.get('thread-msg-1')).toBeDefined();
});

it('should handle empty message store gracefully', () => {
Messages.state.replaceAll([]);

expect(Messages.state.filter((m) => m.rid === rid).length).toBe(0);

expect(() => RoomHistoryManager.clear(rid)).not.toThrow();
expect(() => RoomHistoryManager.clear(rid, () => true)).not.toThrow();
});

it('should not affect messages from other rooms when using a filter', () => {
RoomHistoryManager.clear(rid, (record: MessageRecord) => !record.tmid);

expect(Messages.state.filter((m) => m.rid === otherRid).length).toBe(2);
expect(Messages.state.get('other-room-msg')).toBeDefined();
expect(Messages.state.get('other-room-thread')).toBeDefined();

expect(Messages.state.filter((m) => m.rid === rid).length).toBe(3);
expect(Messages.state.get('thread-msg-1')).toBeDefined();
expect(Messages.state.get('msg1')).toBeUndefined();
});
});
6 changes: 3 additions & 3 deletions apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { getUserId } from '../../../../client/lib/user';
import { callWithErrorHandling } from '../../../../client/lib/utils/callWithErrorHandling';
import { getConfig } from '../../../../client/lib/utils/getConfig';
import { waitForElement } from '../../../../client/lib/utils/waitForElement';
import { Messages, Subscriptions } from '../../../../client/stores';
import { Messages, Subscriptions, type MessageRecord } from '../../../../client/stores';
import { getUserPreference } from '../../../utils/client';

const waitAfterFlush = () => new Promise((resolve) => Tracker.afterFlush(() => resolve(void 0)));
Expand Down Expand Up @@ -285,9 +285,9 @@ class RoomHistoryManagerClass extends Emitter {
delete this.histories[rid];
}

public clear(rid: IRoom['_id']) {
public clear(rid: IRoom['_id'], shouldRemove?: (record: MessageRecord) => boolean) {
const room = this.getRoom(rid);
Messages.state.remove((record) => record.rid === rid);
Messages.state.remove((record) => record.rid === rid && (!shouldRemove || shouldRemove(record)));
room.isLoading.set(false);
room.hasMore.set(true);
room.hasMoreNext.set(false);
Expand Down
19 changes: 8 additions & 11 deletions apps/meteor/client/stores/Messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,12 @@ import type { IMessage } from '@rocket.chat/core-typings';
import { createDocumentMapStore } from '../lib/cachedStores/DocumentMapStore';
import { createGlobalStore } from '../lib/cachedStores/createGlobalStore';

export type MessageRecord = IMessage & {
ignored?: boolean;
autoTranslateFetching?: boolean;
autoTranslateShowInverse?: boolean;
temp?: boolean;
};

/** @deprecated prefer fetching data from the REST API, listening to changes via streamer events, and storing the state in a Tanstack Query */
export const Messages =
createGlobalStore(
createDocumentMapStore<
IMessage & {
ignored?: boolean;
autoTranslateFetching?: boolean;
autoTranslateShowInverse?: boolean;
temp?: boolean;
}
>(),
);
export const Messages = createGlobalStore(createDocumentMapStore<MessageRecord>());
2 changes: 1 addition & 1 deletion apps/meteor/client/stores/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { Messages } from './Messages';
export { Messages, type MessageRecord } from './Messages';
export { Permissions } from './Permissions';
export { PrivateSettings, PublicSettings } from './Settings';
export { Roles } from './Roles';
Expand Down
117 changes: 117 additions & 0 deletions apps/meteor/client/views/room/body/hooks/useHasNewMessages.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { useRouteParameter } from '@rocket.chat/ui-contexts';
import { renderHook } from '@testing-library/react';
import type { MutableRefObject } from 'react';

import { useHasNewMessages } from './useHasNewMessages';
import { RoomHistoryManager } from '../../../../../app/ui-utils/client';
import { useChat } from '../../contexts/ChatContext';

jest.mock('@rocket.chat/ui-contexts', () => ({
useRouteParameter: jest.fn(),
}));

jest.mock('../../../../../app/ui-utils/client', () => ({
RoomHistoryManager: {
clear: jest.fn(),
getMoreIfIsEmpty: jest.fn(),
},
}));

jest.mock('../../contexts/ChatContext', () => ({
useChat: jest.fn(),
}));

const mockSendToBottom = jest.fn();
const mockSendToBottomIfNecessary = jest.fn();
const mockIsAtBottom = jest.fn();

describe('useHasNewMessages', () => {
const rid = 'test-room-id';
const uid = 'test-user-id';
const atBottomRef: MutableRefObject<boolean> = { current: false };
const scrollHelpers = {
sendToBottom: mockSendToBottom,
sendToBottomIfNecessary: mockSendToBottomIfNecessary,
isAtBottom: mockIsAtBottom,
};

beforeEach(() => {
jest.clearAllMocks();
atBottomRef.current = false;
(useChat as jest.Mock).mockReturnValue({
composer: { focus: jest.fn() },
});
});

describe('handleJumpToRecentButtonClick', () => {
it('should call RoomHistoryManager.clear with a filter to exclude the opened thread messages', () => {
const openThreadId = 'thread-123';
(useRouteParameter as jest.Mock).mockImplementation((param: string) => {
if (param === 'tab') return 'thread';
if (param === 'context') return openThreadId;
return undefined;
});

const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, scrollHelpers));

result.current.handleJumpToRecentButtonClick();

expect(RoomHistoryManager.clear).toHaveBeenCalledWith(rid, expect.any(Function));
expect(RoomHistoryManager.getMoreIfIsEmpty).toHaveBeenCalledWith(rid);

const shouldRemoveFn = (RoomHistoryManager.clear as jest.Mock).mock.calls[0][1];
expect(shouldRemoveFn({ _id: openThreadId })).toBe(false);
expect(shouldRemoveFn({ _id: 'message_id', tmid: openThreadId })).toBe(false);
expect(shouldRemoveFn({ _id: 'message_id', tmid: 'other-thread' })).toBe(true);
expect(shouldRemoveFn({ _id: 'other-message-id' })).toBe(true);
});

it('should call RoomHistoryManager.clear with a filter to clear all messages of opened room when not in a thread', () => {
(useRouteParameter as jest.Mock).mockImplementation((param: string) => {
if (param === 'tab') return 'info';
if (param === 'context') return 'some-context';
return undefined;
});

const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, scrollHelpers));

result.current.handleJumpToRecentButtonClick();

expect(RoomHistoryManager.clear).toHaveBeenCalledWith(rid, expect.any(Function));
expect(RoomHistoryManager.getMoreIfIsEmpty).toHaveBeenCalledWith(rid);

const shouldRemoveFn = (RoomHistoryManager.clear as jest.Mock).mock.calls[0][1];
expect(shouldRemoveFn({ _id: 'some-message-id' })).toBe(true);
expect(shouldRemoveFn({ _id: 'some-message-id', tmid: 'any-thread' })).toBe(true);
expect(shouldRemoveFn({ tmid: 'thread-without-id' })).toBe(true);
expect(shouldRemoveFn({})).toBe(true);
});

it('should update atBottomRef when called', () => {
(useRouteParameter as jest.Mock).mockReturnValue(undefined);

const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, scrollHelpers));

result.current.handleJumpToRecentButtonClick();

expect(atBottomRef.current).toBe(true);
});

it('should clear all messages when thread tab is active but context is undefined', () => {
(useRouteParameter as jest.Mock).mockImplementation((param: string) => {
if (param === 'tab') return 'thread';
return undefined;
});

const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, scrollHelpers));

result.current.handleJumpToRecentButtonClick();

expect(RoomHistoryManager.clear).toHaveBeenCalledWith(rid, expect.any(Function));

const shouldRemoveFn = (RoomHistoryManager.clear as jest.Mock).mock.calls[0][1];
expect(shouldRemoveFn({ _id: 'any-msg' })).toBe(true);
expect(shouldRemoveFn({ _id: 'any-msg', tmid: 'any-thread' })).toBe(true);
});
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { IMessage } from '@rocket.chat/core-typings';
import { isEditedMessage } from '@rocket.chat/core-typings';
import { clientCallbacks } from '@rocket.chat/ui-client';
import { useRouteParameter } from '@rocket.chat/ui-contexts';
import type { MutableRefObject } from 'react';
import { useCallback, useEffect, useState } from 'react';

Expand Down Expand Up @@ -37,11 +38,15 @@ export const useHasNewMessages = (
chat.composer?.focus();
}, [atBottomRef, chat.composer, sendToBottomIfNecessary]);

const tab = useRouteParameter('tab');
const context = useRouteParameter('context');
const openThreadId = tab === 'thread' ? context : undefined;

const handleJumpToRecentButtonClick = useCallback(() => {
atBottomRef.current = true;
RoomHistoryManager.clear(rid);
RoomHistoryManager.clear(rid, (record) => !openThreadId || (record._id !== openThreadId && record.tmid !== openThreadId));
RoomHistoryManager.getMoreIfIsEmpty(rid);
}, [atBottomRef, rid]);
}, [atBottomRef, rid, openThreadId]);

const handleComposerResize = useCallback((): void => {
sendToBottomIfNecessary();
Expand Down
7 changes: 4 additions & 3 deletions apps/meteor/jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ export default {
setupFilesAfterEnv: [...client.setupFilesAfterEnv],

testMatch: [
'<rootDir>/app/ui/client/**/**.spec.[jt]s?(x)',
'<rootDir>/app/ui-message/client/**/**.spec.[jt]s?(x)',
'<rootDir>/app/ui-utils/client/**/**.spec.[jt]s?(x)',
'<rootDir>/client/**/**.spec.[jt]s?(x)',
'<rootDir>/ee/client/**/**.spec.[jt]s?(x)',
'<rootDir>/app/ui-message/client/**/**.spec.[jt]s?(x)',
'<rootDir>/tests/unit/client/views/**/*.spec.{ts,tsx}',
'<rootDir>/tests/unit/client/providers/**/*.spec.{ts,tsx}',
'<rootDir>/app/ui/client/**/**.spec.[jt]s?(x)',
'<rootDir>/tests/unit/client/views/**/*.spec.{ts,tsx}',
],

moduleNameMapper: {
Expand Down
Loading
Loading