diff --git a/app/containers/MessageComposer/components/Quotes/Quote.test.tsx b/app/containers/MessageComposer/components/Quotes/Quote.test.tsx new file mode 100644 index 00000000000..7e614d16b85 --- /dev/null +++ b/app/containers/MessageComposer/components/Quotes/Quote.test.tsx @@ -0,0 +1,53 @@ +import { render } from '@testing-library/react-native'; + +import { Quote } from './Quote'; + +const mockMessage: { + msg: string; + channels?: { _id: string; name: string; fname?: string }[]; + u: { username: string }; + id: string; +} = { + id: 'm1', + msg: 'see #2P3ydWKGPhoXrbxJL', + channels: [{ _id: 'r1', name: '2P3ydWKGPhoXrbxJL', fname: 'My Discussion' }], + u: { username: 'alice' } +}; + +jest.mock('../../hooks', () => ({ + useMessage: jest.fn(() => mockMessage) +})); + +jest.mock('../../../../views/RoomView/context', () => ({ + useRoomContext: jest.fn(() => ({ onRemoveQuoteMessage: jest.fn() })) +})); + +jest.mock('../../../../lib/hooks/useAppSelector', () => ({ + useAppSelector: jest.fn(() => false) +})); + +// Pulls in the composer store, which is irrelevant to what this test asserts +jest.mock('../Buttons', () => ({ + BaseButton: () => null +})); + +describe('composer Quote preview', () => { + beforeEach(() => { + mockMessage.channels = [{ _id: 'r1', name: '2P3ydWKGPhoXrbxJL', fname: 'My Discussion' }]; + }); + + it('shows the discussion mention by fname instead of the room id', () => { + const { queryByText } = render(); + + expect(queryByText('see #2P3ydWKGPhoXrbxJL')).toBeNull(); + expect(queryByText('see #My Discussion')).toBeTruthy(); + }); + + it('leaves the text alone when the message has no channels', () => { + mockMessage.channels = undefined; + + const { queryByText } = render(); + + expect(queryByText('see #2P3ydWKGPhoXrbxJL')).toBeTruthy(); + }); +}); diff --git a/app/containers/MessageComposer/components/Quotes/Quote.tsx b/app/containers/MessageComposer/components/Quotes/Quote.tsx index 6ce10c30f8c..90a9afba62a 100644 --- a/app/containers/MessageComposer/components/Quotes/Quote.tsx +++ b/app/containers/MessageComposer/components/Quotes/Quote.tsx @@ -46,7 +46,7 @@ export const Quote = ({ messageId }: { messageId: string }) => { testID={`composer-quote-remove-${message.id}`} /> - + ); }; diff --git a/app/containers/markdown/Markdown.textStyle.test.tsx b/app/containers/markdown/Markdown.textStyle.test.tsx index ae7b1b4af12..07d11421309 100644 --- a/app/containers/markdown/Markdown.textStyle.test.tsx +++ b/app/containers/markdown/Markdown.textStyle.test.tsx @@ -40,4 +40,26 @@ describe('Markdown textStyle integration', () => { expect(onLinkPress).toHaveBeenCalledWith('https://rocket.chat'); }); + + it('renders a channel mention using fname while still matching the token by name', () => { + const navToRoomInfo = jest.fn(); + + const { getByText, queryByText } = render( + + ); + + // `roomsWithHashTagSymbol` is mocked on, hence the leading `#` + expect(getByText('#My Discussion')).toBeTruthy(); + expect(queryByText('#aBcD123xyz')).toBeNull(); + }); + + it('falls back to the raw token when the channel has no fname', () => { + const { getByText } = render(); + + expect(getByText('#general')).toBeTruthy(); + }); }); diff --git a/app/containers/markdown/MarkdownPreview.channels.test.tsx b/app/containers/markdown/MarkdownPreview.channels.test.tsx new file mode 100644 index 00000000000..4fa11741483 --- /dev/null +++ b/app/containers/markdown/MarkdownPreview.channels.test.tsx @@ -0,0 +1,45 @@ +import { render } from '@testing-library/react-native'; + +import { MarkdownPreview } from '.'; + +jest.mock('../../lib/hooks/useAppSelector', () => ({ + useAppSelector: jest.fn(() => false) +})); + +jest.mock('../../lib/methods/userPreferences', () => ({ + useUserPreferences: jest.fn(() => [true]) +})); + +// Previews (composer quote, sent quote, thread previews) render plain text rather than the +// markdown tree, so they resolve discussion mentions through `channels` themselves. +describe('MarkdownPreview channel mentions', () => { + it('shows a discussion mention by its fname instead of the room id', () => { + const { queryByText } = render( + + ); + + expect(queryByText('see #My Discussion')).toBeTruthy(); + expect(queryByText('see #aBcD123xyz')).toBeNull(); + }); + + it('resolves a mention that is the entire message', () => { + const { queryByText } = render( + + ); + + expect(queryByText('#aBcD123xyz')).toBeNull(); + expect(queryByText('#My Discussion')).toBeTruthy(); + }); + + it('leaves a regular channel mention untouched', () => { + const { queryByText } = render(); + + expect(queryByText('see #general')).toBeTruthy(); + }); + + it('renders unchanged when no channels are supplied', () => { + const { queryByText } = render(); + + expect(queryByText('see #aBcD123xyz')).toBeTruthy(); + }); +}); diff --git a/app/containers/markdown/components/Preview.tsx b/app/containers/markdown/components/Preview.tsx index 250c0d28fff..a7defb06615 100644 --- a/app/containers/markdown/components/Preview.tsx +++ b/app/containers/markdown/components/Preview.tsx @@ -3,6 +3,7 @@ import { type StyleProp, Text, type TextStyle } from 'react-native'; import { themes } from '../../../lib/constants/colors'; import { useTheme } from '../../../theme'; import usePreviewFormatText from '../../../lib/hooks/usePreviewFormatText'; +import { type IUserChannel } from '../interfaces'; import styles from '../styles'; interface IMarkdownPreview { @@ -10,11 +11,12 @@ interface IMarkdownPreview { numberOfLines?: number; testID?: string; style?: StyleProp; + channels?: IUserChannel[]; } -const MarkdownPreview = ({ msg, numberOfLines = 1, style = [], testID }: IMarkdownPreview) => { +const MarkdownPreview = ({ msg, numberOfLines = 1, style = [], testID, channels }: IMarkdownPreview) => { const { theme } = useTheme(); - const formattedText = usePreviewFormatText(msg ?? ''); + const formattedText = usePreviewFormatText(msg ?? '', channels); if (!msg) { return null; diff --git a/app/containers/markdown/components/mentions/Hashtag.tsx b/app/containers/markdown/components/mentions/Hashtag.tsx index 44259738c9a..a5c4ed53664 100644 --- a/app/containers/markdown/components/mentions/Hashtag.tsx +++ b/app/containers/markdown/components/mentions/Hashtag.tsx @@ -25,12 +25,12 @@ const Hashtag = memo(({ hashtag }: IHashtag) => { const [roomsWithHashTagSymbol] = useUserPreferences(ROOM_MENTIONS_PREFERENCES_KEY, false); const isMasterDetail = useMasterDetail(); const preffix = roomsWithHashTagSymbol ? '#' : ''; + const channel = channels?.find(({ name }) => name === hashtag); const handlePress = async () => { - const index = channels?.findIndex(channel => channel.name === hashtag); - if (typeof index !== 'undefined' && navToRoomInfo) { + if (channel && navToRoomInfo) { const navParam = { t: 'c', - rid: channels?.[index]._id + rid: channel._id }; const room = navParam.rid && (await getSubscriptionByRoomId(navParam.rid)); if (room) { @@ -49,7 +49,7 @@ const Hashtag = memo(({ hashtag }: IHashtag) => { } }; - if (channels && channels.length && channels.findIndex(channel => channel.name === hashtag) !== -1) { + if (channel) { return ( { } ]} onPress={handlePress}> - {`${preffix}${hashtag}`} + {`${preffix}${channel?.fname || hashtag}`} ); } diff --git a/app/containers/markdown/interfaces.ts b/app/containers/markdown/interfaces.ts index ae5a8cf1adf..f2e202cc9d7 100644 --- a/app/containers/markdown/interfaces.ts +++ b/app/containers/markdown/interfaces.ts @@ -8,6 +8,7 @@ export interface IUserMention { export interface IUserChannel { name: string; _id: string; + fname?: string; } export type TOnLinkPress = (link: string) => void; diff --git a/app/containers/message/components/Attachments/Reply.tsx b/app/containers/message/components/Attachments/Reply.tsx index 54f0ab57c81..2ec4472eb89 100644 --- a/app/containers/message/components/Attachments/Reply.tsx +++ b/app/containers/message/components/Attachments/Reply.tsx @@ -14,6 +14,8 @@ import { Attachments } from './components'; import Quote from './Quote'; import { useBaseUrl, useMessageUser, useTimeFormat } from '../../stores/MessageRoomStore'; import { useIsEncrypted, useMessageId } from '../../stores/MessageStore'; +import { useQuotedMessageChannels } from '../../hooks/useQuotedMessageChannels'; +import { formatChannelMentions } from '../../../../lib/methods/helpers/formatChannelMentions'; import MessageActionTouchable from '../Touchable/MessageActionTouchable'; import messageStyles from '../../styles'; import dayjs from '../../../../lib/dayjs'; @@ -106,6 +108,8 @@ const Title = ({ attachment }: { attachment: IAttachment }) => { const Description = ({ attachment }: { attachment: IAttachment }) => { const user = useMessageUser(); const text = attachment.text || attachment.title; + // Only quotes whose text can contain a channel mention need the quoted message's channels + const channels = useQuotedMessageChannels(attachment.message_link, text?.includes('#') ?? false); if (!text) { return null; @@ -121,7 +125,7 @@ const Description = ({ attachment }: { attachment: IAttachment }) => { return ; } - return ; + return ; }; const UrlImage = ({ image }: { image?: string }) => { diff --git a/app/containers/message/components/Content/PreviewContent.tsx b/app/containers/message/components/Content/PreviewContent.tsx index a298820f538..cbcf505a411 100644 --- a/app/containers/message/components/Content/PreviewContent.tsx +++ b/app/containers/message/components/Content/PreviewContent.tsx @@ -1,12 +1,13 @@ import { MarkdownPreview } from '../../../markdown'; import { getPreviewMessageFromAttachment } from '../../utils'; -import { useAttachments, useMessageText } from '../../stores/MessageStore'; +import { useAttachments, useMarkdownData, useMessageText } from '../../stores/MessageStore'; import { useAutoTranslate } from '../../stores/MessageRoomStore'; import ContentWrapper from './ContentWrapper'; const PreviewContent = () => { const { messageText } = useMessageText(); const attachments = useAttachments(); + const { channels } = useMarkdownData(); const { autoTranslateLanguage } = useAutoTranslate(); const previewMsg = @@ -18,7 +19,7 @@ const PreviewContent = () => { return ( - + ); }; diff --git a/app/containers/message/components/__tests__/Reply.test.tsx b/app/containers/message/components/__tests__/Reply.test.tsx index 898539b1c17..dffeaea3115 100644 --- a/app/containers/message/components/__tests__/Reply.test.tsx +++ b/app/containers/message/components/__tests__/Reply.test.tsx @@ -10,6 +10,7 @@ import { E2E_MESSAGE_TYPE, E2E_STATUS } from '../../../../lib/constants/keys'; import { fileDownloadAndPreview } from '../../../../lib/methods/helpers'; import openLink from '../../../../lib/methods/helpers/openLink'; import { formatAttachmentUrl } from '../../../../lib/methods/helpers/formatAttachmentUrl'; +import { getMessageById } from '../../../../lib/database/services/Message'; jest.mock('../../../markdown', () => { const React = require('react'); @@ -21,6 +22,10 @@ jest.mock('../../../markdown', () => { }; }); +jest.mock('../../../../lib/database/services/Message', () => ({ + getMessageById: jest.fn(() => Promise.resolve(null)) +})); + jest.mock('../../../../lib/methods/helpers', () => ({ fileDownloadAndPreview: jest.fn(() => Promise.resolve()) })); @@ -44,6 +49,7 @@ jest.mock('expo-image', () => { const mockFileDownloadAndPreview = fileDownloadAndPreview as jest.Mock; const mockOpenLink = openLink as jest.Mock; const mockFormatAttachmentUrl = formatAttachmentUrl as jest.Mock; +const mockGetMessageById = getMessageById as jest.Mock; const buildItem = (isEncrypted?: boolean) => ({ @@ -84,6 +90,38 @@ describe('Reply', () => { beforeEach(() => { jest.clearAllMocks(); mockFormatAttachmentUrl.mockImplementation((url: string) => `formatted:${url}`); + mockGetMessageById.mockResolvedValue(null); + }); + + // A quote attachment carries the quoted text but not its channels, so they are read back off the + // cached message. The label is plain text: a quote is not a live, tappable mention. + describe('quoted discussion mentions', () => { + const quote: IAttachment = { + author_name: 'Alice', + text: 'see #aBcD123xyz', + message_link: 'https://open.rocket.chat/channel/general?msg=quoted-1' + }; + + it('renders the discussion name instead of the room id', async () => { + mockGetMessageById.mockResolvedValue({ channels: [{ _id: 'c1', name: 'aBcD123xyz', fname: 'My Discussion' }] }); + + const { getByTestId } = renderReply({ attachment: quote }); + + await waitFor(() => expect(getByTestId('reply-markdown')).toHaveTextContent('see #My Discussion')); + }); + + it('keeps the room id when the quoted message is not cached', async () => { + const { getByTestId } = renderReply({ attachment: quote }); + + await waitFor(() => expect(mockGetMessageById).toHaveBeenCalledWith('quoted-1')); + expect(getByTestId('reply-markdown')).toHaveTextContent('see #aBcD123xyz'); + }); + + it('does not hit the database when the quoted text has no mention', () => { + renderReply({ attachment: { ...quote, text: 'plain quoted text' } }); + + expect(mockGetMessageById).not.toHaveBeenCalled(); + }); }); describe('null gate', () => { diff --git a/app/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsx b/app/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsx index 3b78be202a2..636fb9779b0 100644 --- a/app/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsx +++ b/app/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsx @@ -85,6 +85,33 @@ describe('useMessageAccessibilityLabel', () => { ).toBe(`alice ${HOUR} hey alice check general.`); }); + // The screen shows the fname, so the label has to match it rather than announcing the room id + it('announces a discussion mention by its fname', () => { + expect( + renderLabel( + buildItem({ + msg: 'see #aBcD123xyz', + channels: [{ _id: 'c1', name: 'aBcD123xyz', fname: 'My Discussion' }] + }) + ) + ).toBe(`alice ${HOUR} see My Discussion.`); + }); + + // A shorter channel name must not eat the start of a longer mention + it('announces the whole mention when another channel name is a prefix of it', () => { + expect( + renderLabel( + buildItem({ + msg: 'see #abcdef', + channels: [ + { _id: 'c1', name: 'abc', fname: 'Short' }, + { _id: 'c2', name: 'abcdef', fname: 'Long' } + ] + }) + ) + ).toBe(`alice ${HOUR} see Long.`); + }); + it('appends "Message was read" when read receipts are enabled and the message is read', () => { expect(renderLabel(buildItem({ unread: false }), { isReadReceiptEnabled: true })).toBe( `alice ${HOUR} hello world. Message was read` diff --git a/app/containers/message/hooks/__tests__/useQuotedMessageChannels.test.ts b/app/containers/message/hooks/__tests__/useQuotedMessageChannels.test.ts new file mode 100644 index 00000000000..9f52cf72e14 --- /dev/null +++ b/app/containers/message/hooks/__tests__/useQuotedMessageChannels.test.ts @@ -0,0 +1,60 @@ +import { renderHook, waitFor } from '@testing-library/react-native'; + +import { useQuotedMessageChannels } from '../useQuotedMessageChannels'; +import { getMessageById } from '../../../../lib/database/services/Message'; + +jest.mock('../../../../lib/database/services/Message', () => ({ + getMessageById: jest.fn() +})); + +const mockGetMessageById = getMessageById as jest.Mock; + +const permalink = (messageId: string) => `https://open.rocket.chat/channel/general?msg=${messageId}`; +const discussion = [{ _id: 'c1', name: 'aBcD123xyz', fname: 'My Discussion' }]; + +describe('useQuotedMessageChannels', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockGetMessageById.mockResolvedValue(null); + }); + + it('returns the channels of the message the permalink points at', async () => { + mockGetMessageById.mockResolvedValue({ channels: discussion }); + + const { result } = renderHook(() => useQuotedMessageChannels(permalink('quoted-1'))); + + await waitFor(() => expect(result.current).toEqual(discussion)); + expect(mockGetMessageById).toHaveBeenCalledWith('quoted-1'); + }); + + it('returns undefined when there is no permalink', async () => { + const { result } = renderHook(() => useQuotedMessageChannels(undefined)); + + await waitFor(() => expect(mockGetMessageById).not.toHaveBeenCalled()); + expect(result.current).toBeUndefined(); + }); + + // Otherwise the previous quote's names would label this one's mentions + it('drops the previous channels when the permalink changes to a message without any', async () => { + mockGetMessageById.mockResolvedValue({ channels: discussion }); + const { result, rerender } = renderHook(({ link }: { link: string }) => useQuotedMessageChannels(link), { + initialProps: { link: permalink('quoted-1') } + }); + await waitFor(() => expect(result.current).toEqual(discussion)); + + mockGetMessageById.mockResolvedValue({ channels: [] }); + rerender({ link: permalink('quoted-2') }); + + await waitFor(() => expect(result.current).toBeUndefined()); + }); + + // The lookup reaches the database, which has no active instance while servers switch + it('leaves the channels unset when the lookup rejects', async () => { + mockGetMessageById.mockRejectedValue(new Error('no active database')); + + const { result } = renderHook(() => useQuotedMessageChannels(permalink('quoted-1'))); + + await waitFor(() => expect(mockGetMessageById).toHaveBeenCalledWith('quoted-1')); + expect(result.current).toBeUndefined(); + }); +}); diff --git a/app/containers/message/hooks/useMessageAccessibilityLabel.ts b/app/containers/message/hooks/useMessageAccessibilityLabel.ts index aa1f3c0ee7c..5dfb7fa118f 100644 --- a/app/containers/message/hooks/useMessageAccessibilityLabel.ts +++ b/app/containers/message/hooks/useMessageAccessibilityLabel.ts @@ -3,6 +3,7 @@ import translationLanguages from '../../../lib/constants/translationLanguages'; import { useImageDescriptionLabel } from './useImageDescriptionLabel'; import { getInfoMessage } from '../utils'; import { type IUserChannel, type IUserMention } from '../../../definitions'; +import { formatChannelMentions } from '../../../lib/methods/helpers/formatChannelMentions'; import { useContentData, useIsEncrypted, @@ -23,12 +24,7 @@ const stripMentions = (label: string, mentions: IUserMention[] = [], channels: I result = result.replaceAll(`@${item.username}`, item.username); } }); - channels?.forEach(item => { - if (item?.name) { - result = result.replaceAll(`#${item.name}`, item.name); - } - }); - return result; + return formatChannelMentions(result, channels, true); }; export const useMessageAccessibilityLabel = (): string => { diff --git a/app/containers/message/hooks/useQuotedMessageChannels.ts b/app/containers/message/hooks/useQuotedMessageChannels.ts new file mode 100644 index 00000000000..3daec77aaf8 --- /dev/null +++ b/app/containers/message/hooks/useQuotedMessageChannels.ts @@ -0,0 +1,45 @@ +import { useEffect, useState } from 'react'; + +import { type IUserChannel } from '../../../definitions'; +import { getMessageById } from '../../../lib/database/services/Message'; +import { getMessageIdFromPermalink } from '../../../lib/methods/helpers/getMessageIdFromPermalink'; + +/** + * A quote attachment carries the quoted message's text but not its `channels`, and the quoting + * message has none of its own because its body is just a permalink. Without this, a discussion + * mention inside a quote renders as the room id. + * + * The quoted message is normally already cached locally, so we read `channels` back off it. + */ +export const useQuotedMessageChannels = (messageLink?: string, enabled = true): IUserChannel[] | undefined => { + const [channels, setChannels] = useState(); + + useEffect(() => { + let isActive = true; + // Never let the previous quote's names label this one's mentions + setChannels(undefined); + + const load = async () => { + const messageId = getMessageIdFromPermalink(messageLink); + if (!messageId || !enabled) { + return; + } + try { + // Reads through to the database, which has no active instance while servers switch + const message = await getMessageById(messageId); + if (isActive && message?.channels?.length) { + setChannels(message.channels); + } + } catch { + // Leaving `channels` unset falls back to the raw room id, which beats crashing the quote + } + }; + load(); + + return () => { + isActive = false; + }; + }, [messageLink, enabled]); + + return channels; +}; diff --git a/app/definitions/IMessage.ts b/app/definitions/IMessage.ts index c348b22d48e..40184c3e0c2 100644 --- a/app/definitions/IMessage.ts +++ b/app/definitions/IMessage.ts @@ -41,6 +41,8 @@ export interface IUserChannel { [index: number]: string | number; name: string; _id: string; + // Discussions carry a server-generated, ID-like `name`; `fname` holds the human-readable label + fname?: string; } export interface IEditedBy { diff --git a/app/lib/hooks/usePreviewFormatText/index.tsx b/app/lib/hooks/usePreviewFormatText/index.tsx index 664177de3a0..f0be926140b 100644 --- a/app/lib/hooks/usePreviewFormatText/index.tsx +++ b/app/lib/hooks/usePreviewFormatText/index.tsx @@ -3,11 +3,14 @@ import removeMarkdown from 'remove-markdown'; import useShortnameToUnicode from '../useShortnameToUnicode'; import { formatText } from '../../helpers/formatText'; import { formatHyperlink } from '../../helpers/formatHyperlink'; +import { formatChannelMentions, type TMentionableChannel } from '../../methods/helpers/formatChannelMentions'; -const usePreviewFormatText = (msg: string) => { +const usePreviewFormatText = (msg: string, channels?: TMentionableChannel[]): string => { const { formatShortnameToUnicode } = useShortnameToUnicode(); - let m = formatText(msg); + // Resolved before the markdown is stripped, so the mention is still `#name` at this point + let m = formatChannelMentions(msg, channels); + m = formatText(m); m = formatHyperlink(m); m = formatShortnameToUnicode(m); // Removes sequential empty spaces before to use removeMarkdown, diff --git a/app/lib/methods/helpers/formatChannelMentions.test.ts b/app/lib/methods/helpers/formatChannelMentions.test.ts new file mode 100644 index 00000000000..b40b1672156 --- /dev/null +++ b/app/lib/methods/helpers/formatChannelMentions.test.ts @@ -0,0 +1,79 @@ +import { formatChannelMentions } from './formatChannelMentions'; + +describe('formatChannelMentions', () => { + it('replaces a discussion mention with its fname', () => { + expect(formatChannelMentions('see #aBcD123xyz', [{ name: 'aBcD123xyz', fname: 'My Discussion' }])).toBe('see #My Discussion'); + }); + + it('keeps the raw name when the channel has no fname', () => { + expect(formatChannelMentions('see #general', [{ name: 'general' }])).toBe('see #general'); + }); + + it('leaves hashtags that are not in channels untouched', () => { + expect(formatChannelMentions('see #unknown', [{ name: 'general', fname: 'General' }])).toBe('see #unknown'); + }); + + it('returns the message unchanged when there are no channels', () => { + expect(formatChannelMentions('see #aBcD123xyz', [])).toBe('see #aBcD123xyz'); + expect(formatChannelMentions('see #aBcD123xyz', undefined)).toBe('see #aBcD123xyz'); + }); + + it('replaces every occurrence of the same mention', () => { + expect(formatChannelMentions('#id1 then #id1', [{ name: 'id1', fname: 'Design' }])).toBe('#Design then #Design'); + }); + + it('replaces several different mentions in one message', () => { + expect( + formatChannelMentions('#id1 and #id2', [ + { name: 'id1', fname: 'Design' }, + { name: 'id2', fname: 'Product' } + ]) + ).toBe('#Design and #Product'); + }); + + // A naive replaceAll on the shorter name first would corrupt the longer mention + it('does not corrupt a longer mention that starts with a shorter channel name', () => { + expect( + formatChannelMentions('#abc and #abcdef', [ + { name: 'abc', fname: 'Short' }, + { name: 'abcdef', fname: 'Long' } + ]) + ).toBe('#Short and #Long'); + }); + + it('does not match a channel name that is only a prefix of the written mention', () => { + expect(formatChannelMentions('#abcdef', [{ name: 'abc', fname: 'Short' }])).toBe('#abcdef'); + }); + + it('handles names containing regex metacharacters', () => { + expect(formatChannelMentions('#a.b-c_d', [{ name: 'a.b-c_d', fname: 'Dotted' }])).toBe('#Dotted'); + }); + + it('still replaces a mention followed by sentence punctuation', () => { + expect(formatChannelMentions('go to #id1.', [{ name: 'id1', fname: 'Design' }])).toBe('go to #Design.'); + }); + + it('prefers the longer channel name when one is a dotted extension of another', () => { + expect( + formatChannelMentions('#a.b', [ + { name: 'a', fname: 'Short' }, + { name: 'a.b', fname: 'Long' } + ]) + ).toBe('#Long'); + }); + + // Accessibility labels announce the name alone, without the sigil + it('drops the sigil for a discussion mention with an fname', () => { + expect(formatChannelMentions('see #aBcD123xyz', [{ name: 'aBcD123xyz', fname: 'My Discussion' }], true)).toBe( + 'see My Discussion' + ); + }); + + it('drops the sigil for a channel without an fname', () => { + expect(formatChannelMentions('see #general', [{ name: 'general' }], true)).toBe('see general'); + }); + + it('returns an empty string unchanged', () => { + expect(formatChannelMentions('', [{ name: 'id1', fname: 'Design' }])).toBe(''); + }); +}); diff --git a/app/lib/methods/helpers/formatChannelMentions.ts b/app/lib/methods/helpers/formatChannelMentions.ts new file mode 100644 index 00000000000..9d95328be5c --- /dev/null +++ b/app/lib/methods/helpers/formatChannelMentions.ts @@ -0,0 +1,38 @@ +import { escapeRegExp } from 'lodash'; + +/** Structural subset of the two `IUserChannel` declarations in the codebase, so either one fits */ +export type TMentionableChannel = { name: string; fname?: string }; + +/** + * Rewrites `#` mentions to `#` so plain-text surfaces show the same label as the + * rendered message body. Discussions carry a server-generated, ID-like `name`, so without this + * they read as `#aBcD123xyz` instead of `#My Discussion`. + * + * Only names present in `channels` are considered, and longer names are matched first, so a + * mention is never partially replaced. With `dropSigil`, the `#` is left out (accessibility + * labels announce the name alone). + */ +export const formatChannelMentions = (msg: string, channels?: TMentionableChannel[], dropSigil = false): string => { + if (!msg || !channels?.length) { + return msg; + } + + // With the sigil kept, a channel whose fname matches its name would rewrite to the same text + const rewrites = (channel: TMentionableChannel) => dropSigil || (channel.fname && channel.fname !== channel.name); + const named = channels.filter(channel => channel?.name && rewrites(channel)); + if (!named.length) { + return msg; + } + + const byName = new Map(named.map(channel => [channel.name, channel.fname || channel.name])); + // Longest first so `#abcdef` never resolves against a channel merely named `abc` + const pattern = [...byName.keys()] + .sort((a, b) => b.length - a.length) + .map(escapeRegExp) + .join('|'); + // Trailing `-`/word chars would mean a longer room name, so they must not terminate a mention. + // A `.` is allowed to terminate one, since dotted names are matched by the longest-first pattern. + const mention = new RegExp(`#(${pattern})(?![\\w-])`, 'g'); + + return msg.replace(mention, (_, name: string) => `${dropSigil ? '' : '#'}${byName.get(name) ?? name}`); +}; diff --git a/app/lib/methods/helpers/getMessageIdFromPermalink.test.ts b/app/lib/methods/helpers/getMessageIdFromPermalink.test.ts new file mode 100644 index 00000000000..b78acdb8fbc --- /dev/null +++ b/app/lib/methods/helpers/getMessageIdFromPermalink.test.ts @@ -0,0 +1,40 @@ +import { getMessageIdFromPermalink } from './getMessageIdFromPermalink'; + +describe('getMessageIdFromPermalink', () => { + it('extracts the message id from a channel permalink', () => { + expect(getMessageIdFromPermalink('https://mobile.qa.rocket.chat/channel/general?msg=n5WaK5NRJN42Hg26w')).toBe( + 'n5WaK5NRJN42Hg26w' + ); + }); + + it('extracts the message id from a private group permalink', () => { + expect(getMessageIdFromPermalink('https://mobile.qa.rocket.chat/group/channel-etc?msg=cIqhbvkOSgiCOK4Wh')).toBe( + 'cIqhbvkOSgiCOK4Wh' + ); + }); + + it('extracts the message id when other query params follow', () => { + expect(getMessageIdFromPermalink('https://server.com/channel/general?msg=abc123&jump=1')).toBe('abc123'); + }); + + it('extracts the message id when other query params precede it', () => { + expect(getMessageIdFromPermalink('https://server.com/channel/general?jump=1&msg=abc123')).toBe('abc123'); + }); + + it('returns undefined when there is no msg param', () => { + expect(getMessageIdFromPermalink('https://server.com/channel/general')).toBeUndefined(); + }); + + it('does not match a param merely ending in msg', () => { + expect(getMessageIdFromPermalink('https://server.com/channel/general?tmsg=abc123')).toBeUndefined(); + }); + + it('returns undefined for an empty msg param', () => { + expect(getMessageIdFromPermalink('https://server.com/channel/general?msg=')).toBeUndefined(); + }); + + it('returns undefined for missing input', () => { + expect(getMessageIdFromPermalink(undefined)).toBeUndefined(); + expect(getMessageIdFromPermalink('')).toBeUndefined(); + }); +}); diff --git a/app/lib/methods/helpers/getMessageIdFromPermalink.ts b/app/lib/methods/helpers/getMessageIdFromPermalink.ts new file mode 100644 index 00000000000..3ea3f164030 --- /dev/null +++ b/app/lib/methods/helpers/getMessageIdFromPermalink.ts @@ -0,0 +1,7 @@ +export const getMessageIdFromPermalink = (permalink?: string): string | undefined => { + if (!permalink) { + return undefined; + } + const [, messageId] = permalink.match(/[?&]msg=([^&#]+)/) ?? []; + return messageId || undefined; +}; diff --git a/app/views/ThreadMessagesView/Item.tsx b/app/views/ThreadMessagesView/Item.tsx index 95a7e3820c9..6dffd2efa4d 100644 --- a/app/views/ThreadMessagesView/Item.tsx +++ b/app/views/ThreadMessagesView/Item.tsx @@ -88,7 +88,7 @@ const Item = ({ item, useRealName, user, badgeColor, onPress, toggleFollowThread {time} - + {badgeColor ? : null}