diff --git a/cspell.json b/cspell.json index 23720a61ff84..9e0c9381081a 100644 --- a/cspell.json +++ b/cspell.json @@ -737,6 +737,7 @@ "onloaderror", "onopentag", "onplayerror", + "ontext", "onxy", "openxmlformats", "ordinality", diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 8f89c7871ba3..136c6948bdfd 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -114,7 +114,6 @@ import {getMicroSecondOnyxErrorWithTranslationKey, isReceiptError} from './Error import getAttachmentDetails from './fileDownload/getAttachmentDetails'; import type {FormulaContext} from './Formula'; import getBase62ReportID from './getBase62ReportID'; -import {isReportMessageAttachment} from './isReportMessageAttachment'; import {formatPhoneNumber as formatPhoneNumberPhoneUtils} from './LocalePhoneNumber'; import {translateLocal} from './Localize'; import Log from './Log'; @@ -5063,13 +5062,16 @@ function canEditFieldOfMoneyRequest({ * Can only edit if: * * - It was written by the current user - * - It's an ADD_COMMENT that is not an attachment + * - It's an ADD_COMMENT or IOU action + * - It's not an optimistic attachment (still uploading) * - It's an expense where conditions for modifications are defined in canEditMoneyRequest method * - It's not pending deletion */ function canEditReportAction(reportAction: OnyxInputOrEntry, linkedTransaction: OnyxEntry): boolean { const isCommentOrIOU = reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT || reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU; - const message = reportAction ? getReportActionMessageReportUtils(reportAction) : undefined; + + // Block editing only while the attachment is still uploading; once synced, attachments are editable. + const isOptimisticAttachment = !!reportAction?.isOptimisticAction && (!!reportAction?.isAttachmentOnly || !!reportAction?.isAttachmentWithText); // For money request actions on settled/approved/closed expense reports, the action cannot be edited. // canEditMoneyRequest has an admin/manager bypass for field-level edits (via canEditFieldOfMoneyRequest), @@ -5087,9 +5089,8 @@ function canEditReportAction(reportAction: OnyxInputOrEntry, linke return !!( reportAction?.actorAccountID === deprecatedCurrentUserAccountID && isCommentOrIOU && - (!isMoneyRequestAction(reportAction) || canEditMoneyRequest(reportAction, linkedTransaction)) && // Returns true for non-IOU actions - !isReportMessageAttachment(message) && - ((!reportAction.isAttachmentWithText && !reportAction.isAttachmentOnly) || !reportAction.isOptimisticAction) && + (!isMoneyRequestAction(reportAction) || canEditMoneyRequest(reportAction, linkedTransaction)) && + !isOptimisticAttachment && !isDeletedAction(reportAction) && !isCreatedTaskReportAction(reportAction) && reportAction?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE diff --git a/src/libs/isReportMessageAttachment.ts b/src/libs/isReportMessageAttachment.ts index 662d2e19922f..275a2c8691c7 100644 --- a/src/libs/isReportMessageAttachment.ts +++ b/src/libs/isReportMessageAttachment.ts @@ -1,37 +1,88 @@ -import {Str} from 'expensify-common'; +import {Parser as HtmlParser} from 'htmlparser2'; import CONST from '@src/CONST'; import type {Message} from '@src/types/onyx/ReportAction'; -const attachmentRegex = new RegExp(` ${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="(.*)"`, 'i'); +const ATTACHMENT_TAGS = new Set(['a', 'img', 'video']); +// Leading space + `="` (not a bare substring) so a URL query param like `?data-expensify-source=` isn't a false positive. +const ATTACHMENT_SOURCE_TOKEN = ` ${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="`; -/** - * Check whether a report action is Attachment or not. - * Ignore messages containing [Attachment] as the main content. Attachments are actions with only text as [Attachment]. - * - * @param message report action's message as text, html and translationKey - */ -function isReportMessageAttachment(message: Message | undefined): boolean { - if (!message?.text || !message.html) { +// Keyed by Onyx message identity: re-renders pass the same immutable object, so parse at most once per message. +const resultCache = new WeakMap(); + +function computeIsReportMessageAttachment(html: string, text: string, translationKey: string | undefined): boolean { + // Optimistic attachment-only messages carry this translationKey; the server drops it after sync. + if (translationKey === CONST.TRANSLATION_KEYS.ATTACHMENT) { + return true; + } + + // Fast path: no source attribute → not an attachment (avoids the parser for most messages). + if (!html.includes(ATTACHMENT_SOURCE_TOKEN)) { return false; } - if (message.translationKey) { - return message.text === CONST.ATTACHMENT_MESSAGE_TEXT && message.translationKey === CONST.TRANSLATION_KEYS.ATTACHMENT; + // image/video attachment-only keep text "[Attachment]"; only .doc/.pdf (text = "filename URL") need the parser. + if (text === CONST.ATTACHMENT_MESSAGE_TEXT) { + return true; } - const hasAttachmentHtml = attachmentRegex.test(message.html); + let hasAttachment = false; + let hasOtherContent = false; + // Depth >0 = inside the attachment's subtree; its inner nodes (filename/markup) aren't user content. + let depth = 0; - if (!hasAttachmentHtml) { + const parser = new HtmlParser({ + ontext: (nodeText) => { + if (depth > 0 || !nodeText.trim()) { + return; + } + hasOtherContent = true; + }, + onopentag: (name, attribs) => { + if (depth > 0) { + depth += 1; + return; + } + if (name === 'br') { + return; + } + if (ATTACHMENT_TAGS.has(name) && !!attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]) { + hasAttachment = true; + if (name === 'a' || name === 'video') { + depth = 1; + } + return; + } + hasOtherContent = true; + }, + onclosetag: () => { + if (depth <= 0) { + return; + } + depth -= 1; + }, + }); + parser.write(html); + parser.end(); + + return hasAttachment && !hasOtherContent; +} + +/** + * Returns true for attachment-only messages (no user-typed text), false for attachment+text and non-attachment messages. + */ +function isReportMessageAttachment(message: Message | undefined): boolean { + if (!message?.text || !message.html) { return false; } - const isAttachmentMessageText = message.text === CONST.ATTACHMENT_MESSAGE_TEXT; - - if (isAttachmentMessageText) { - return true; + const cached = resultCache.get(message); + if (cached !== undefined) { + return cached; } - return Str.isVideo(message.text); + const result = computeIsReportMessageAttachment(message.html, message.text, message.translationKey); + resultCache.set(message, result); + return result; } // eslint-disable-next-line import/prefer-default-export diff --git a/tests/perf-test/isReportMessageAttachment.perf-test.ts b/tests/perf-test/isReportMessageAttachment.perf-test.ts new file mode 100644 index 000000000000..b6c5bf4024e5 --- /dev/null +++ b/tests/perf-test/isReportMessageAttachment.perf-test.ts @@ -0,0 +1,37 @@ +import {measureFunction} from 'reassure'; +import type {Message} from '@src/types/onyx/ReportAction'; +import {isReportMessageAttachment} from '../../src/libs/isReportMessageAttachment'; + +const SAMPLE_MESSAGES: Message[] = [ + // .doc attachment-only after OpenReport: text = "filename URL" (the parser-bound case) + { + text: 'sample.doc https://www.expensify.com/chat-attachments/123/w_abc.doc', + html: 'sample.doc', + type: '', + }, + // image attachment-only (translationKey path) + { + text: '[Attachment]', + html: '', + type: '', + translationKey: 'common.attachment', + }, + { + text: 'Here is the file\nsample.pdf https://www.expensify.com/chat-attachments/123/s.pdf', + html: 'Here is the file

sample.pdf', + type: '', + }, + {text: 'Just a normal chat message with some words in it', html: 'Just a normal chat message with some words in it', type: ''}, + {text: 'https://example.com', html: 'https://example.com', type: ''}, +]; + +// One classification pass over a long chat history (called per message on list renders / LHN). +const LONG_CHAT_HISTORY: Message[] = Array.from({length: 2000}, () => SAMPLE_MESSAGES).flat(); + +test('[isReportMessageAttachment] classify a 10k-message chat history', async () => { + await measureFunction(() => { + for (const message of LONG_CHAT_HISTORY) { + isReportMessageAttachment(message); + } + }); +}); diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 089fe131c9a5..805dab6bb870 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -5745,6 +5745,111 @@ describe('ReportUtils', () => { expect(canEditReportAction(moneyRequestAction, transaction)).toEqual(false); }); + + it('should return false for an optimistic attachment-only action (still uploading)', () => { + const transaction = createRandomTransaction(300); + const reportAction: ReportAction = { + reportActionID: '300', + actorAccountID: currentUserAccountID, + actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT, + message: [ + { + type: 'COMMENT', + html: 'file.doc', + text: '[Attachment]', + }, + ], + isAttachmentOnly: true, + isOptimisticAction: true, + created: '2025-03-05 16:34:27', + }; + + expect(canEditReportAction(reportAction, transaction)).toEqual(false); + }); + + it('should return false for an optimistic attachment+text action (still uploading)', () => { + const transaction = createRandomTransaction(301); + const reportAction: ReportAction = { + reportActionID: '301', + actorAccountID: currentUserAccountID, + actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT, + message: [ + { + type: 'COMMENT', + html: 'Hi

file.doc', + text: 'Hi\n[Attachment]', + }, + ], + isAttachmentWithText: true, + isOptimisticAction: true, + created: '2025-03-05 16:34:27', + }; + + expect(canEditReportAction(reportAction, transaction)).toEqual(false); + }); + + it('should return true for a synced attachment-only action (optimistic flags cleared)', () => { + const transaction = createRandomTransaction(302); + // Post-sync state: successData clears isOptimisticAction (null in Onyx, undefined here); isAttachmentOnly persists. + const reportAction: ReportAction = { + reportActionID: '302', + actorAccountID: currentUserAccountID, + actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT, + message: [ + { + type: 'COMMENT', + html: 'file.doc', + text: 'file.doc https://www.expensify.com/chat-attachments/123/file.doc', + }, + ], + isAttachmentOnly: true, + isOptimisticAction: undefined, + created: '2025-03-05 16:34:27', + }; + + expect(canEditReportAction(reportAction, transaction)).toEqual(true); + }); + + it('should return true for a synced attachment+text action', () => { + const transaction = createRandomTransaction(303); + const reportAction: ReportAction = { + reportActionID: '303', + actorAccountID: currentUserAccountID, + actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT, + message: [ + { + type: 'COMMENT', + html: 'Hi

file.doc', + text: 'Hi\nfile.doc https://www.expensify.com/chat-attachments/123/file.doc', + }, + ], + isAttachmentWithText: true, + isOptimisticAction: undefined, + created: '2025-03-05 16:34:27', + }; + + expect(canEditReportAction(reportAction, transaction)).toEqual(true); + }); + + it('should return true for an optimistic plain-text comment (no attachment)', () => { + const transaction = createRandomTransaction(304); + const reportAction: ReportAction = { + reportActionID: '304', + actorAccountID: currentUserAccountID, + actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT, + message: [ + { + type: 'COMMENT', + html: 'hello', + text: 'hello', + }, + ], + isOptimisticAction: true, + created: '2025-03-05 16:34:27', + }; + + expect(canEditReportAction(reportAction, transaction)).toEqual(true); + }); }); describe('getChatByParticipants', () => { diff --git a/tests/unit/isReportMessageAttachmentTest.ts b/tests/unit/isReportMessageAttachmentTest.ts index 75faba9c8935..32362d5071c3 100644 --- a/tests/unit/isReportMessageAttachmentTest.ts +++ b/tests/unit/isReportMessageAttachmentTest.ts @@ -1,3 +1,4 @@ +import CONST from '@src/CONST'; import type {Message} from '@src/types/onyx/ReportAction'; import {isReportMessageAttachment} from '../../src/libs/isReportMessageAttachment'; @@ -21,4 +22,156 @@ describe('isReportMessageAttachment', () => { message = {text: '[Attachment]', html: '[Attachment]', type: ''}; expect(isReportMessageAttachment(message)).toBe(false); }); + + it('returns true for optimistic attachment-only via translationKey', () => { + const message: Message = { + text: '[Attachment]', + html: 'sample.doc', + type: '', + translationKey: CONST.TRANSLATION_KEYS.ATTACHMENT, + }; + expect(isReportMessageAttachment(message)).toBe(true); + }); + + it('returns true for .doc attachment-only after OpenReport sync (text is "filename URL")', () => { + const message: Message = { + text: 'sample.doc https://staging.expensify.com/chat-attachments/4271684646135633435/w_87c70962804917e4c6866d052537133ae0a3060e.doc', + html: 'sample.doc', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(true); + }); + + it('returns true for .pdf attachment-only', () => { + const message: Message = { + text: 'report.pdf https://www.expensify.com/chat-attachments/123/report.pdf', + html: 'report.pdf', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(true); + }); + + it('returns true for video attachment-only', () => { + const message: Message = { + text: 'demo.mp4', + html: '', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(true); + }); + + it('returns false for attachment+text (user text before the attachment)', () => { + const message: Message = { + text: 'Check this file\nsample.doc https://www.expensify.com/chat-attachments/123/sample.doc', + html: 'Check this file

sample.doc', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(false); + }); + + it('returns false for a leading user URL + attachment (regression guard for codex bot P2)', () => { + const message: Message = { + text: 'https://example.com check this\nsample.doc https://www.expensify.com/chat-attachments/123/sample.doc', + html: 'https://example.com check this

sample.doc', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(false); + }); + + it('returns false for a plain user link with only data-raw-href (no attachment)', () => { + const message: Message = { + text: 'https://example.com', + html: 'https://example.com', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(false); + }); + + it('returns false for a mention before the attachment', () => { + const message: Message = { + text: '@user check this\nsample.doc https://www.expensify.com/chat-attachments/123/sample.doc', + html: '@user check this

sample.doc', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(false); + }); + + it('returns false for an attachment with a caption appended after it (regression guard for codex bot P2)', () => { + const message: Message = { + text: 'sample.doc https://www.expensify.com/chat-attachments/123/sample.doc my caption', + html: 'sample.doc my caption', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(false); + }); + + it('returns true when the anchor inner text (filename) is the only text', () => { + const message: Message = { + text: 'sample.doc https://www.expensify.com/chat-attachments/123/sample.doc', + html: 'sample.doc', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(true); + }); + + it('returns true for multiple attachments with no surrounding text', () => { + const message: Message = { + text: 'a.doc https://www.expensify.com/chat-attachments/1/a.doc\nb.pdf https://www.expensify.com/chat-attachments/2/b.pdf', + html: 'a.docb.pdf', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(true); + }); + + it('returns true for an attachment whose anchor wraps nested markup', () => { + const message: Message = { + text: 'sample.doc https://www.expensify.com/chat-attachments/123/sample.doc', + html: 'presample.doc', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(true); + }); + + it('returns false for an attachment with nested markup AND a trailing caption', () => { + const message: Message = { + text: 'sample.doc https://www.expensify.com/chat-attachments/123/sample.doc see this', + html: 'presample.doc see this', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(false); + }); + + it('returns false for undefined or empty message', () => { + expect(isReportMessageAttachment(undefined)).toBe(false); + expect(isReportMessageAttachment({text: '', html: '', type: ''})).toBe(false); + expect(isReportMessageAttachment({text: 'x', html: undefined as unknown as string, type: ''})).toBe(false); + }); + + it('falls through when translationKey is set but is not the attachment key', () => { + const message: Message = { + text: 'sample.doc https://www.expensify.com/chat-attachments/123/sample.doc', + html: 'sample.doc', + type: '', + translationKey: 'common.download', + }; + expect(isReportMessageAttachment(message)).toBe(true); + }); + + it('returns false for image attachment + text', () => { + const message: Message = { + text: 'check this image\n[Attachment]', + html: 'check this image

', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(false); + }); + + it('returns false for video attachment + text', () => { + const message: Message = { + text: 'watch this\ndemo.mp4 https://www.expensify.com/chat-attachments/123/demo.mp4', + html: 'watch this

', + type: '', + }; + expect(isReportMessageAttachment(message)).toBe(false); + }); });