Skip to content

Commit b570388

Browse files
juliajforestijuliajforesti
authored
fix: prevent self-message notification in new message stream (RocketChat#39545)
1 parent dd1f620 commit b570388

3 files changed

Lines changed: 228 additions & 0 deletions

File tree

‎.changeset/polite-plums-boil.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@rocket.chat/meteor': patch
3+
---
4+
5+
Fixes the intermittent behavior where the "New messages" indicator appears incorrectly after the user sends a message

‎apps/meteor/client/views/room/body/hooks/useHasNewMessages.spec.ts‎

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
import type { IEditedMessage, IMessage } from '@rocket.chat/core-typings';
2+
import { mockAppRoot } from '@rocket.chat/mock-providers';
3+
import { clientCallbacks } from '@rocket.chat/ui-client';
4+
import { renderHook, act } from '@testing-library/react';
5+
6+
import { useHasNewMessages } from './useHasNewMessages';
7+
8+
jest.mock('../../contexts/ChatContext', () => ({
9+
useChat: () => ({
10+
composer: { focus: jest.fn() },
11+
}),
12+
}));
13+
14+
jest.mock('../../../../../app/ui-utils/client', () => ({
15+
RoomHistoryManager: {
16+
clear: jest.fn(),
17+
getMoreIfIsEmpty: jest.fn(),
18+
},
19+
}));
20+
21+
describe('useHasNewMessages', () => {
22+
const mockScrollBehavior = {
23+
sendToBottom: jest.fn(),
24+
sendToBottomIfNecessary: jest.fn(),
25+
isAtBottom: jest.fn(() => true),
26+
};
27+
28+
const atBottomRef = { current: true };
29+
const rid = 'room-id';
30+
const uid = 'current-user-id';
31+
32+
beforeEach(() => {
33+
jest.clearAllMocks();
34+
clientCallbacks.remove('streamNewMessage', rid);
35+
clientCallbacks.remove('afterSaveMessage', rid);
36+
});
37+
38+
afterEach(() => {
39+
clientCallbacks.remove('streamNewMessage', rid);
40+
clientCallbacks.remove('afterSaveMessage', rid);
41+
});
42+
43+
it('should NOT show new messages button when user sends their own message', () => {
44+
const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, mockScrollBehavior), { wrapper: mockAppRoot().build() });
45+
46+
const ownMsg: IMessage = {
47+
_id: 'msg-1',
48+
rid,
49+
u: { _id: uid, username: 'current-user', name: 'Current User' },
50+
msg: 'Hello',
51+
ts: new Date(),
52+
_updatedAt: new Date(),
53+
};
54+
55+
const callbacks = clientCallbacks.getCallbacks('streamNewMessage');
56+
act(() => {
57+
callbacks.forEach((callback) => callback(ownMsg));
58+
});
59+
expect(result.current.hasNewMessages).toBe(false);
60+
});
61+
62+
it('should NOT show new messages button when user sends own message during race condition (not at bottom)', () => {
63+
const scrollBehavior = {
64+
...mockScrollBehavior,
65+
isAtBottom: jest.fn(() => false),
66+
};
67+
68+
const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, scrollBehavior), { wrapper: mockAppRoot().build() });
69+
70+
const ownMsg: IMessage = {
71+
_id: 'msg-race',
72+
rid,
73+
u: { _id: uid, username: 'current-user', name: 'Current User' },
74+
msg: 'Message sent during scroll',
75+
ts: new Date(),
76+
_updatedAt: new Date(),
77+
};
78+
79+
const callbacks = clientCallbacks.getCallbacks('streamNewMessage');
80+
act(() => {
81+
callbacks.forEach((callback) => callback(ownMsg));
82+
});
83+
expect(result.current.hasNewMessages).toBe(false);
84+
});
85+
86+
it('should show new messages button when another user sends a message and user is NOT at bottom', () => {
87+
const scrollBehavior = {
88+
...mockScrollBehavior,
89+
isAtBottom: jest.fn(() => false),
90+
};
91+
92+
const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, scrollBehavior), { wrapper: mockAppRoot().build() });
93+
94+
const otherUserMsg: IMessage = {
95+
_id: 'msg-2',
96+
rid,
97+
u: { _id: 'other-user-id', username: 'other-user', name: 'Other User' },
98+
msg: 'Hello from another user',
99+
ts: new Date(),
100+
_updatedAt: new Date(),
101+
};
102+
103+
const callbacks = clientCallbacks.getCallbacks('streamNewMessage');
104+
act(() => {
105+
callbacks.forEach((callback) => callback(otherUserMsg));
106+
});
107+
expect(result.current.hasNewMessages).toBe(true);
108+
});
109+
110+
it('should NOT show new messages button when another user sends a message but user IS at bottom', () => {
111+
const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, mockScrollBehavior), { wrapper: mockAppRoot().build() });
112+
113+
const otherUserMsg: IMessage = {
114+
_id: 'msg-3',
115+
rid,
116+
u: { _id: 'other-user-id', username: 'other-user', name: 'Other User' },
117+
msg: 'Hello from another user',
118+
ts: new Date(),
119+
_updatedAt: new Date(),
120+
};
121+
122+
const callbacks = clientCallbacks.getCallbacks('streamNewMessage');
123+
act(() => {
124+
callbacks.forEach((callback) => callback(otherUserMsg));
125+
});
126+
expect(result.current.hasNewMessages).toBe(false);
127+
});
128+
129+
it('should ignore edited messages in streamNewMessage', () => {
130+
const scrollBehavior = {
131+
...mockScrollBehavior,
132+
isAtBottom: jest.fn(() => false),
133+
};
134+
135+
const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, scrollBehavior), { wrapper: mockAppRoot().build() });
136+
137+
const editedMsg: IEditedMessage = {
138+
_id: 'msg-4',
139+
rid,
140+
u: { _id: 'other-user-id', username: 'other-user', name: 'Other User' },
141+
msg: 'Edited message',
142+
ts: new Date(),
143+
_updatedAt: new Date(),
144+
editedAt: new Date(),
145+
editedBy: { _id: 'other-user-id', username: 'other-user' },
146+
};
147+
148+
const callbacks = clientCallbacks.getCallbacks('streamNewMessage');
149+
act(() => {
150+
callbacks.forEach((callback) => callback(editedMsg));
151+
});
152+
expect(result.current.hasNewMessages).toBe(false);
153+
});
154+
155+
it('should ignore thread messages in streamNewMessage', () => {
156+
const scrollBehavior = {
157+
...mockScrollBehavior,
158+
isAtBottom: jest.fn(() => false),
159+
};
160+
161+
const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, scrollBehavior), { wrapper: mockAppRoot().build() });
162+
163+
const threadMsg: IMessage = {
164+
_id: 'msg-5',
165+
rid,
166+
tmid: 'parent-thread-id',
167+
u: { _id: 'other-user-id', username: 'other-user', name: 'Other User' },
168+
msg: 'Thread reply',
169+
ts: new Date(),
170+
_updatedAt: new Date(),
171+
};
172+
173+
const callbacks = clientCallbacks.getCallbacks('streamNewMessage');
174+
act(() => {
175+
callbacks.forEach((callback) => callback(threadMsg));
176+
});
177+
expect(result.current.hasNewMessages).toBe(false);
178+
});
179+
180+
it('should clear hasNewMessages when afterSaveMessage fires for current user', () => {
181+
const scrollBehavior = {
182+
...mockScrollBehavior,
183+
isAtBottom: jest.fn(() => false),
184+
};
185+
186+
const { result } = renderHook(() => useHasNewMessages(rid, uid, atBottomRef, scrollBehavior), { wrapper: mockAppRoot().build() });
187+
188+
const otherUserMsg: IMessage = {
189+
_id: 'msg-6',
190+
rid,
191+
u: { _id: 'other-user-id', username: 'other-user', name: 'Other User' },
192+
msg: 'Hello',
193+
ts: new Date(),
194+
_updatedAt: new Date(),
195+
};
196+
197+
const streamCallbacks = clientCallbacks.getCallbacks('streamNewMessage');
198+
act(() => {
199+
streamCallbacks.forEach((callback) => callback(otherUserMsg));
200+
});
201+
expect(result.current.hasNewMessages).toBe(true);
202+
203+
const ownMsg: IMessage = {
204+
_id: 'msg-7',
205+
rid,
206+
u: { _id: uid, username: 'current-user', name: 'Current User' },
207+
msg: 'My reply',
208+
ts: new Date(),
209+
_updatedAt: new Date(),
210+
};
211+
212+
const afterSaveCallbacks = clientCallbacks.getCallbacks('afterSaveMessage');
213+
act(() => {
214+
afterSaveCallbacks.forEach((callback) => callback(ownMsg));
215+
});
216+
expect(result.current.hasNewMessages).toBe(false);
217+
expect(mockScrollBehavior.sendToBottom).toHaveBeenCalled();
218+
});
219+
});

‎apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ export const useHasNewMessages = (
5656
return;
5757
}
5858

59+
if (msg.u._id === uid) {
60+
return;
61+
}
62+
5963
if (!isAtBottom()) {
6064
setHasNewMessages(true);
6165
}

0 commit comments

Comments
 (0)