From f1bee442205f31c5b9cec7280d264f0023c86b3b Mon Sep 17 00:00:00 2001 From: OtavioStasiak Date: Fri, 13 Nov 2026 15:06:23 -0300 Subject: [PATCH] fix: keep the quote when replying in direct message to a thread message --- .../MessageComposer/MessageComposer.test.tsx | 9 ++-- .../MessageComposerContainer.test.tsx | 7 +-- .../helpers/prepareQuoteMessage.test.ts | 48 +++++++++-------- .../helpers/prepareQuoteMessage.ts | 4 +- .../MessageComposer/hooks/useMessage.ts | 4 +- app/lib/database/services/Message.test.ts | 52 ++++++++++++++++++- app/lib/database/services/Message.ts | 8 +++ 7 files changed, 97 insertions(+), 35 deletions(-) diff --git a/app/containers/MessageComposer/MessageComposer.test.tsx b/app/containers/MessageComposer/MessageComposer.test.tsx index 27d9928b862..d7fef8ad071 100644 --- a/app/containers/MessageComposer/MessageComposer.test.tsx +++ b/app/containers/MessageComposer/MessageComposer.test.tsx @@ -94,8 +94,8 @@ const initialStoreState = () => { }; initialStoreState(); -jest.mock('~/lib/database/services/Message', () => ({ - getMessageById: (messageId: any) => ({ +jest.mock('~/lib/database/services/Message', () => { + const getMessage = (messageId: any) => ({ id: messageId, rid: 'rid', msg: messageId !== 'image' ? `Message ${messageId}` : undefined, @@ -107,8 +107,9 @@ jest.mock('~/lib/database/services/Message', () => ({ } ] : [] - }) -})); + }); + return { getMessageById: getMessage, getMessageOrThreadMessageById: getMessage }; +}); const initialContext = { rid: 'rid', diff --git a/app/containers/MessageComposer/__tests__/MessageComposerContainer.test.tsx b/app/containers/MessageComposer/__tests__/MessageComposerContainer.test.tsx index 06fb2983335..e398fa9c11d 100644 --- a/app/containers/MessageComposer/__tests__/MessageComposerContainer.test.tsx +++ b/app/containers/MessageComposer/__tests__/MessageComposerContainer.test.tsx @@ -57,9 +57,10 @@ jest.mock('~/lib/hooks/useAltTextSupported', () => ({ useAltTextSupported: jest.fn() })); -jest.mock('~/lib/database/services/Message', () => ({ - getMessageById: (messageId: string) => ({ id: messageId, rid: 'rid', msg: `Message ${messageId}`, attachments: [] }) -})); +jest.mock('~/lib/database/services/Message', () => { + const getMessage = (messageId: string) => ({ id: messageId, rid: 'rid', msg: `Message ${messageId}`, attachments: [] }); + return { getMessageById: getMessage, getMessageOrThreadMessageById: getMessage }; +}); jest.mock('../hooks/useMessage', () => ({ useMessage: (messageId: string) => ({ id: messageId, msg: 'quote this', u: { username: 'rocket.cat' } }) diff --git a/app/containers/MessageComposer/helpers/prepareQuoteMessage.test.ts b/app/containers/MessageComposer/helpers/prepareQuoteMessage.test.ts index 6afd25b9a44..e6b11f2dfd4 100644 --- a/app/containers/MessageComposer/helpers/prepareQuoteMessage.test.ts +++ b/app/containers/MessageComposer/helpers/prepareQuoteMessage.test.ts @@ -1,6 +1,6 @@ import { prepareQuoteMessage } from './prepareQuoteMessage'; import { getPermalinkMessage } from '~/lib/methods/getPermalinks'; -import { getMessageById } from '~/lib/database/services/Message'; +import { getMessageOrThreadMessageById } from '~/lib/database/services/Message'; import { store } from '~/lib/store/auxStore'; import { compareServerVersion } from '~/lib/methods/helpers'; @@ -10,7 +10,7 @@ jest.mock('~/lib/methods/getPermalinks', () => ({ })); jest.mock('~/lib/database/services/Message', () => ({ - getMessageById: jest.fn() + getMessageOrThreadMessageById: jest.fn() })); jest.mock('~/lib/store/auxStore', () => ({ @@ -24,7 +24,9 @@ jest.mock('~/lib/methods/helpers/compareServerVersion', () => ({ })); const mockGetPermalinkMessage = getPermalinkMessage as jest.MockedFunction; -const mockGetMessageById = getMessageById as jest.MockedFunction; +const mockGetMessageOrThreadMessageById = getMessageOrThreadMessageById as jest.MockedFunction< + typeof getMessageOrThreadMessageById +>; const mockStore = store as jest.Mocked; const mockCompareServerVersion = compareServerVersion as jest.MockedFunction; @@ -48,7 +50,7 @@ describe('prepareQuoteMessage', () => { const result = await prepareQuoteMessage(textFromInput, selectedMessages); expect(result).toBe('Hello world'); - expect(mockGetMessageById).not.toHaveBeenCalled(); + expect(mockGetMessageOrThreadMessageById).not.toHaveBeenCalled(); expect(mockGetPermalinkMessage).not.toHaveBeenCalled(); }); @@ -76,12 +78,12 @@ describe('prepareQuoteMessage', () => { const permalink = 'https://example.com/message/message1'; mockCompareServerVersion.mockReturnValue(false); // >= 5.0.0 - mockGetMessageById.mockResolvedValue(mockMessage as any); + mockGetMessageOrThreadMessageById.mockResolvedValue(mockMessage as any); mockGetPermalinkMessage.mockResolvedValue(permalink); const result = await prepareQuoteMessage(textFromInput, selectedMessages); - expect(mockGetMessageById).toHaveBeenCalledWith('message1', undefined); + expect(mockGetMessageOrThreadMessageById).toHaveBeenCalledWith('message1', undefined); expect(mockGetPermalinkMessage).toHaveBeenCalledWith(mockMessage); expect(result).toBe(`[ ](${permalink}) \nMy reply`); }); @@ -92,7 +94,7 @@ describe('prepareQuoteMessage', () => { const permalink = 'https://example.com/message/message1'; mockCompareServerVersion.mockReturnValue(true); // < 5.0.0 - mockGetMessageById.mockResolvedValue(mockMessage as any); + mockGetMessageOrThreadMessageById.mockResolvedValue(mockMessage as any); mockGetPermalinkMessage.mockResolvedValue(permalink); const result = await prepareQuoteMessage(textFromInput, selectedMessages); @@ -110,12 +112,12 @@ describe('prepareQuoteMessage', () => { const mockMessage2 = { ...mockMessage, id: 'message2' }; mockCompareServerVersion.mockReturnValue(false); // >= 5.0.0 - mockGetMessageById.mockResolvedValueOnce(mockMessage as any).mockResolvedValueOnce(mockMessage2 as any); + mockGetMessageOrThreadMessageById.mockResolvedValueOnce(mockMessage as any).mockResolvedValueOnce(mockMessage2 as any); mockGetPermalinkMessage.mockResolvedValueOnce(permalink1).mockResolvedValueOnce(permalink2); const result = await prepareQuoteMessage(textFromInput, selectedMessages); - expect(mockGetMessageById).toHaveBeenCalledTimes(2); + expect(mockGetMessageOrThreadMessageById).toHaveBeenCalledTimes(2); expect(mockGetPermalinkMessage).toHaveBeenCalledTimes(2); expect(result).toBe(`[ ](${permalink1}) \n[ ](${permalink2}) \nMy reply`); }); @@ -125,12 +127,12 @@ describe('prepareQuoteMessage', () => { const selectedMessages = ['message1', 'nonexistent']; mockCompareServerVersion.mockReturnValue(false); - mockGetMessageById.mockResolvedValueOnce(mockMessage as any).mockResolvedValueOnce(null); + mockGetMessageOrThreadMessageById.mockResolvedValueOnce(mockMessage as any).mockResolvedValueOnce(null); mockGetPermalinkMessage.mockResolvedValue('https://example.com/message/message1'); const result = await prepareQuoteMessage(textFromInput, selectedMessages); - expect(mockGetMessageById).toHaveBeenCalledTimes(2); + expect(mockGetMessageOrThreadMessageById).toHaveBeenCalledTimes(2); expect(mockGetPermalinkMessage).toHaveBeenCalledTimes(1); expect(result).toBe('[ ](https://example.com/message/message1) \nMy reply'); }); @@ -138,7 +140,7 @@ describe('prepareQuoteMessage', () => { test('should skip messages whose permalink cannot be resolved', async () => { const selectedMessages = ['message1', 'message2']; - mockGetMessageById.mockResolvedValue(mockMessage as any); + mockGetMessageOrThreadMessageById.mockResolvedValue(mockMessage as any); mockGetPermalinkMessage.mockResolvedValueOnce(null).mockResolvedValueOnce('https://example.com/message/message2'); const result = await prepareQuoteMessage('My reply', selectedMessages); @@ -152,7 +154,7 @@ describe('prepareQuoteMessage', () => { const permalink = 'https://example.com/message/message1'; mockCompareServerVersion.mockReturnValue(false); - mockGetMessageById.mockResolvedValue(mockMessage as any); + mockGetMessageOrThreadMessageById.mockResolvedValue(mockMessage as any); mockGetPermalinkMessage.mockResolvedValue(permalink); const result = await prepareQuoteMessage(textFromInput, selectedMessages); @@ -165,7 +167,7 @@ describe('prepareQuoteMessage', () => { const selectedMessages = ['message1']; mockCompareServerVersion.mockReturnValue(false); - mockGetMessageById.mockResolvedValue(mockMessage as any); + mockGetMessageOrThreadMessageById.mockResolvedValue(mockMessage as any); mockGetPermalinkMessage.mockRejectedValue(new Error('Network error')); // Should throw the error @@ -184,33 +186,33 @@ describe('prepareQuoteMessage', () => { test('should forward tmid so the thread message can be resolved', async () => { const permalink = 'https://example.com/message/message1'; - mockGetMessageById.mockResolvedValue(mockThreadMessage as any); + mockGetMessageOrThreadMessageById.mockResolvedValue(mockThreadMessage as any); mockGetPermalinkMessage.mockResolvedValue(permalink); const result = await prepareQuoteMessage('My reply', ['message1'], 'thread-id'); - expect(mockGetMessageById).toHaveBeenCalledWith('message1', 'thread-id'); + expect(mockGetMessageOrThreadMessageById).toHaveBeenCalledWith('message1', 'thread-id'); expect(mockGetPermalinkMessage).toHaveBeenCalledWith(mockThreadMessage); expect(result).toBe(`[ ](${permalink}) \nMy reply`); }); test('should forward tmid for every selected message', async () => { - mockGetMessageById.mockResolvedValue(mockThreadMessage as any); + mockGetMessageOrThreadMessageById.mockResolvedValue(mockThreadMessage as any); mockGetPermalinkMessage.mockResolvedValue('https://example.com/link'); await prepareQuoteMessage('My reply', ['message1', 'message2'], 'thread-id'); - expect(mockGetMessageById).toHaveBeenNthCalledWith(1, 'message1', 'thread-id'); - expect(mockGetMessageById).toHaveBeenNthCalledWith(2, 'message2', 'thread-id'); + expect(mockGetMessageOrThreadMessageById).toHaveBeenNthCalledWith(1, 'message1', 'thread-id'); + expect(mockGetMessageOrThreadMessageById).toHaveBeenNthCalledWith(2, 'message2', 'thread-id'); }); test('should pass no tmid when the composer is not in a thread', async () => { - mockGetMessageById.mockResolvedValue(mockThreadMessage as any); + mockGetMessageOrThreadMessageById.mockResolvedValue(mockThreadMessage as any); mockGetPermalinkMessage.mockResolvedValue('https://example.com/link'); await prepareQuoteMessage('My reply', ['message1']); - expect(mockGetMessageById).toHaveBeenCalledWith('message1', undefined); + expect(mockGetMessageOrThreadMessageById).toHaveBeenCalledWith('message1', undefined); }); }); @@ -223,7 +225,7 @@ describe('prepareQuoteMessage', () => { server: { version: '6.0.0' } } as any); mockCompareServerVersion.mockReturnValue(false); - mockGetMessageById.mockResolvedValue({ + mockGetMessageOrThreadMessageById.mockResolvedValue({ id: 'message1', msg: 'Test' } as any); @@ -242,7 +244,7 @@ describe('prepareQuoteMessage', () => { server: { version: '4.0.0' } } as any); mockCompareServerVersion.mockReturnValue(true); - mockGetMessageById.mockResolvedValue({ + mockGetMessageOrThreadMessageById.mockResolvedValue({ id: 'message1', msg: 'Test' } as any); diff --git a/app/containers/MessageComposer/helpers/prepareQuoteMessage.ts b/app/containers/MessageComposer/helpers/prepareQuoteMessage.ts index bd3a6da8664..cc5c3ee936f 100644 --- a/app/containers/MessageComposer/helpers/prepareQuoteMessage.ts +++ b/app/containers/MessageComposer/helpers/prepareQuoteMessage.ts @@ -1,5 +1,5 @@ import { getPermalinkMessage } from '~/lib/methods/getPermalinks'; -import { getMessageById } from '~/lib/database/services/Message'; +import { getMessageOrThreadMessageById } from '~/lib/database/services/Message'; import { store } from '~/lib/store/auxStore'; import { compareServerVersion } from '~/lib/methods/helpers'; @@ -9,7 +9,7 @@ export const prepareQuoteMessage = async (textFromInput: string, selectedMessage const permalinks = await Promise.all( selectedMessages.map(async messageId => { - const message = await getMessageById(messageId, tmid); + const message = await getMessageOrThreadMessageById(messageId, tmid); return message ? getPermalinkMessage(message) : undefined; }) ); diff --git a/app/containers/MessageComposer/hooks/useMessage.ts b/app/containers/MessageComposer/hooks/useMessage.ts index a1fb16edb57..a0a0a1ccbbe 100644 --- a/app/containers/MessageComposer/hooks/useMessage.ts +++ b/app/containers/MessageComposer/hooks/useMessage.ts @@ -1,7 +1,7 @@ import { useEffect, useState } from 'react'; import { type IMessage } from '~/definitions'; -import { getMessageById } from '~/lib/database/services/Message'; +import { getMessageOrThreadMessageById } from '~/lib/database/services/Message'; // TODO: Not reactive. Should we work on an official version? export const useMessage = (messageId: string, tmid?: string): IMessage | undefined => { @@ -10,7 +10,7 @@ export const useMessage = (messageId: string, tmid?: string): IMessage | undefin let cancelled = false; const load = async () => { try { - const result = await getMessageById(messageId, tmid); + const result = await getMessageOrThreadMessageById(messageId, tmid); if (!cancelled) { setMessage(result || undefined); } diff --git a/app/lib/database/services/Message.test.ts b/app/lib/database/services/Message.test.ts index 256e6ad3d01..32993e4cd47 100644 --- a/app/lib/database/services/Message.test.ts +++ b/app/lib/database/services/Message.test.ts @@ -1,7 +1,7 @@ import database from '../index'; import { MESSAGES_TABLE } from '../model/Message'; import { type TMessageModel, type TThreadMessageModel } from '~/definitions'; -import { getMessageById } from './Message'; +import { getMessageById, getMessageOrThreadMessageById } from './Message'; import { getThreadMessageById } from './ThreadMessage'; jest.mock('../index', () => ({ @@ -100,3 +100,53 @@ describe('getMessageById', () => { }); }); }); + +describe('getMessageOrThreadMessageById', () => { + let mockFind: jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + mockFind = jest.fn(); + mockGet.mockReturnValue({ find: mockFind }); + }); + + it('returns the message from the messages table', async () => { + const message = { id: 'message1' } as unknown as TMessageModel; + mockFind.mockResolvedValue(message); + + const result = await getMessageOrThreadMessageById('message1'); + + expect(result).toBe(message); + expect(mockGetThreadMessageById).not.toHaveBeenCalled(); + }); + + it('falls back to the thread messages table without tmid', async () => { + const threadMessage = { id: 'reply1', msg: 'Thread reply' } as unknown as TThreadMessageModel; + mockFind.mockRejectedValue(new Error('not found')); + mockGetThreadMessageById.mockResolvedValue(threadMessage as any); + + const result = await getMessageOrThreadMessageById('reply1'); + + expect(result).toBe(threadMessage); + expect(mockGetThreadMessageById).toHaveBeenCalledWith('reply1'); + }); + + it('does not query the thread messages table twice with tmid', async () => { + mockGetThreadMessageById.mockResolvedValue(null); + mockFind.mockRejectedValue(new Error('not found')); + + const result = await getMessageOrThreadMessageById('reply1', 'thread-id'); + + expect(result).toBeNull(); + expect(mockGetThreadMessageById).toHaveBeenCalledTimes(1); + }); + + it('returns null when the message is in neither table', async () => { + mockFind.mockRejectedValue(new Error('not found')); + mockGetThreadMessageById.mockResolvedValue(null); + + const result = await getMessageOrThreadMessageById('nonexistent'); + + expect(result).toBeNull(); + }); +}); diff --git a/app/lib/database/services/Message.ts b/app/lib/database/services/Message.ts index b6328339224..286b7695382 100644 --- a/app/lib/database/services/Message.ts +++ b/app/lib/database/services/Message.ts @@ -26,3 +26,11 @@ export const getMessageById = async (messageId: string | null, tmid?: string | n return null; } }; + +export const getMessageOrThreadMessageById = async (messageId: string | null, tmid?: string | null) => { + const message = await getMessageById(messageId, tmid); + if (message || !messageId || tmid) { + return message; + } + return getThreadMessageById(messageId); +};