-
Notifications
You must be signed in to change notification settings - Fork 13.5k
fix: thread content disappears after jumping to recent messages #39859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
b833369
9b9c8ca
cccb1f2
e105384
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import type { IMessage, MessageAttachment } from '@rocket.chat/core-typings'; | ||
| import { createPredicateFromFilter } from '@rocket.chat/mongo-adapter'; | ||
| import type { QueryClient } from '@tanstack/react-query'; | ||
| import type { Condition, Filter } from 'mongodb'; | ||
|
|
||
| import { queryClient as defaultQueryClient } from '../queryClient'; | ||
| import { roomsQueryKeys } from '../queryKeys'; | ||
|
|
||
| export type NotifyRoomRidDeleteBulkEvent = { | ||
| rid: IMessage['rid']; | ||
| excludePinned: boolean; | ||
| ignoreDiscussion: boolean; | ||
| ts: Condition<Date>; | ||
| users: string[]; | ||
| ids?: string[]; | ||
| showDeletedStatus?: boolean; | ||
| } & ( | ||
| | { | ||
| filesOnly: true; | ||
| replaceFileAttachmentsWith?: MessageAttachment; | ||
| } | ||
| | { | ||
| filesOnly?: false; | ||
| } | ||
| ); | ||
|
|
||
| export const createDeleteCriteria = (params: NotifyRoomRidDeleteBulkEvent): ((message: IMessage) => boolean) => { | ||
| const query: Filter<IMessage> = {}; | ||
|
|
||
| if (params.ids) { | ||
| query._id = { $in: params.ids }; | ||
| } else { | ||
| query.ts = params.ts; | ||
| } | ||
|
|
||
| if (params.excludePinned) { | ||
| query.pinned = { $ne: true }; | ||
| } | ||
|
|
||
| if (params.ignoreDiscussion) { | ||
| query.drid = { $exists: false }; | ||
| } | ||
| if (params.users?.length) { | ||
| query['u.username'] = { $in: params.users }; | ||
| } | ||
|
|
||
| return createPredicateFromFilter(query); | ||
| }; | ||
|
|
||
| export const upsertThreadMessageInCache = ( | ||
| message: IMessage, | ||
| rid: IMessage['rid'], | ||
| tmid: IMessage['_id'], | ||
| client: QueryClient = defaultQueryClient, | ||
| ): void => { | ||
| const queryKey = roomsQueryKeys.threadMessages(rid, tmid); | ||
| client.setQueryData<IMessage[]>(queryKey, (old) => { | ||
| if (!old) { | ||
| return [message]; | ||
| } | ||
| const idx = old.findIndex((m) => m._id === message._id); | ||
| if (idx >= 0) { | ||
| const updated = [...old]; | ||
| updated[idx] = message; | ||
| return updated; | ||
| } | ||
| return [...old, message].sort((a, b) => new Date(a.ts).getTime() - new Date(b.ts).getTime()); | ||
| }); | ||
| }; |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import { isThreadMessage, type IMessage, type IRoom, type IThreadMainMessage, type IThreadMessage } from '@rocket.chat/core-typings'; | ||
| import { useMethod, useStream } from '@rocket.chat/ui-contexts'; | ||
| import { useQuery, useQueryClient } from '@tanstack/react-query'; | ||
| import { useEffect } from 'react'; | ||
|
|
||
| import { onClientMessageReceived } from '../../../../../lib/onClientMessageReceived'; | ||
| import { roomsQueryKeys } from '../../../../../lib/queryKeys'; | ||
| import { modifyMessageOnFilesDelete } from '../../../../../lib/utils/modifyMessageOnFilesDelete'; | ||
| import { createDeleteCriteria, upsertThreadMessageInCache } from '../../../../../lib/utils/threadMessageUtils'; | ||
| import { useRoom } from '../../../contexts/RoomContext'; | ||
|
|
||
| const processMessages = async (messages: IMessage[]): Promise<IMessage[]> => { | ||
| return Promise.all(messages.map((msg) => onClientMessageReceived(msg))); | ||
| }; | ||
|
|
||
| export const useThreadMessagesQuery = (tmid: IThreadMainMessage['_id'], rid?: IRoom['_id']) => { | ||
| const room = useRoom(); | ||
| const roomId = rid ?? room._id; | ||
|
|
||
| const queryClient = useQueryClient(); | ||
| const queryKey = roomsQueryKeys.threadMessages(roomId, tmid); | ||
| const getThreadMessages = useMethod('getThreadMessages'); | ||
|
|
||
| const subscribeToRoomMessages = useStream('room-messages'); | ||
| const subscribeToNotifyRoom = useStream('notify-room'); | ||
|
|
||
| useEffect(() => { | ||
| const currentQueryKey = roomsQueryKeys.threadMessages(roomId, tmid); | ||
|
|
||
| const unsubscribeFromRoomMessages = subscribeToRoomMessages(roomId, async (event) => { | ||
| if (event.tmid !== tmid) { | ||
| return; | ||
| } | ||
|
|
||
| const processed = await onClientMessageReceived(event); | ||
| upsertThreadMessageInCache(processed, roomId, tmid, queryClient); | ||
| }); | ||
|
|
||
| const unsubscribeFromDeleteMessage = subscribeToNotifyRoom(`${roomId}/deleteMessage`, (event) => { | ||
| queryClient.setQueryData<IThreadMessage[]>(currentQueryKey, (old) => { | ||
| if (!old) { | ||
| return old; | ||
| } | ||
| return old.filter((m) => m._id !== event._id); | ||
| }); | ||
| }); | ||
|
|
||
| const unsubscribeFromDeleteMessageBulk = subscribeToNotifyRoom(`${roomId}/deleteMessageBulk`, (bulkParams) => { | ||
| const matchDeleteCriteria = createDeleteCriteria(bulkParams); | ||
|
|
||
| queryClient.setQueryData<IThreadMessage[]>(currentQueryKey, (old) => { | ||
| if (!old) { | ||
| return old; | ||
| } | ||
|
|
||
| if (bulkParams.filesOnly) { | ||
| return old.map((msg) => { | ||
| if (matchDeleteCriteria(msg)) { | ||
| return modifyMessageOnFilesDelete(msg, bulkParams.replaceFileAttachmentsWith); | ||
| } | ||
| return msg; | ||
| }); | ||
| } | ||
|
|
||
| return old.filter((msg) => !matchDeleteCriteria(msg)); | ||
| }); | ||
| }); | ||
|
|
||
| return () => { | ||
| unsubscribeFromRoomMessages(); | ||
| unsubscribeFromDeleteMessage(); | ||
| unsubscribeFromDeleteMessageBulk(); | ||
| }; | ||
| }, [tmid, roomId, queryClient, subscribeToRoomMessages, subscribeToNotifyRoom]); | ||
|
Comment on lines
+27
to
+74
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From what I understand we already listen to these streams for the main channel message list (here). Since we already listen to the streams, could we move this routing logic there? (I mean deciding whether or not it is a thread message and adding it to cache). I believe the other upsert can stay as optimistic update (e.g
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @aleksandernsilva, thanks for the review! I looked into this but I think keeping the subscription in useThreadMessagesQuery is the better approach here. A few reasons: LegacyRoomManager writes into the zustand Messages store which is deprecated. The whole point of this PR is to move thread messages into a react-query cache instead. Routing through LegacyRoomManager would mean coupling react-query cache updates to legacy code we're trying to move away from. useThreadMainMessageQuery already follows this same pattern. It has its own stream subscription for the main thread message. This just mirrors that for thread replies, so it's consistent. About the duplicate subscription concern - I dug into SDKClient.ts and the SDK multiplexes stream callbacks behind a single DDP subscription per stream-name/eventKey. So subscribing to The hook subscription is lifecycle-scoped to when the thread panel is open, which feels right. LegacyRoomManager runs for the entire room session. The upsertThreadMessageInCache call in sendMessage.ts stays as an optimistic update as you suggested. That part makes sense to keep there. |
||
|
|
||
| return useQuery({ | ||
| queryKey, | ||
| queryFn: async () => { | ||
| const messages = await getThreadMessages({ tmid }); | ||
| const filtered = messages.filter( | ||
| (msg): msg is IThreadMessage => isThreadMessage(msg) && msg.tmid === tmid && msg._id !== tmid && msg._hidden !== true, | ||
| ); | ||
| const sorted = filtered.sort((a, b) => a.ts.getTime() - b.ts.getTime()); | ||
| return processMessages(sorted) as Promise<Array<IThreadMessage>>; | ||
| }, | ||
| }); | ||
| }; | ||
Uh oh!
There was an error while loading. Please reload this page.