Fix .docx attachment detection and edit visibility#87144
Fix .docx attachment detection and edit visibility#87144
Conversation
Fix isReportMessageAttachment to detect document attachment-only messages where message.text is the filename rather than [Attachment]. Also strengthen canEditReportAction to always block editing of attachment-only messages regardless of optimistic status. Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| // For document attachments (.docx, .pdf, etc.), message.text is the filename rather than [Attachment]. | ||
| // Check if the text matches the original filename attribute — if so, it's attachment-only. | ||
| const filenameMatch = message.html.match(originalFilenameRegex); | ||
| if (filenameMatch && filenameMatch[1] === message.text) { |
There was a problem hiding this comment.
@MelvinBot message.text is not only the filename. Also contains link.
Example: Sample.docx https://staging.expensify.com/chat-attachments/8260136212361587477/w_5586f35116d3e3cbb98b95a51bfbac3bb921d804.docx
There was a problem hiding this comment.
Good catch! Fixed — changed the strict equality check to startsWith so it correctly handles the "filename URL" format. Also added a test case with the real-world format you provided.
| import type {Message} from '@src/types/onyx/ReportAction'; | ||
|
|
||
| const attachmentRegex = new RegExp(` ${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="(.*)"`, 'i'); | ||
| const originalFilenameRegex = new RegExp(` ${CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE}="([^"]*)"`, 'i'); |
There was a problem hiding this comment.
@MelvinBot it doesn't have data-name attribute.
Example html:
<a href=\"https://staging.expensify.com/chat-attachments/8260136212361587477/w_5586f35116d3e3cbb98b95a51bfbac3bb921d804.docx\" data-expensify-source=\"https://staging.expensify.com/chat-attachments/8260136212361587477/w_5586f35116d3e3cbb98b95a51bfbac3bb921d804.docx\" data-attachment-id=\"9180105352410595168\">Sample.docx</a>
There was a problem hiding this comment.
You're right — completely rewrote the approach. Instead of relying on data-name, I now extract the filename from the anchor text and the source URL from data-expensify-source, then check if message.text is exactly "filename URL". This matches the real production HTML format you shared. Also updated all tests to use realistic HTML with data-attachment-id instead of data-name.
message.text contains the filename followed by the URL (e.g., "Sample.docx https://staging.expensify.com/..."), not just the filename. Use startsWith to match correctly. Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
Real production HTML for document attachments uses data-attachment-id instead of data-name. Extract filename from anchor text and source URL from data-expensify-source, then check if message.text matches "filename URL" exactly to distinguish attachment-only from attachment+text messages. Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
2 participants
Explanation of Change
Two fixes for document attachment handling (.docx, .pdf, etc.):
isReportMessageAttachment: For document attachment-only messages,message.textis the filename (e.g.,test.docx) rather than[Attachment](which images use). The function now detects these by matchingmessage.textagainst thedata-nameattribute in the HTML. Also, for optimistic messages, thetranslationKeyis now trusted as the authoritative signal for attachment-only — it's only set whenisAttachmentOnlyis true, so the redundant text check is removed.canEditReportAction: The attachment flags check is separated so thatisAttachmentOnlyalways blocks editing (regardless of optimistic status), whileisAttachmentWithTextonly blocks editing when the action is optimistic. Previously both flags were grouped under the same optimistic guard, meaning attachment-only messages could become editable after server confirmation ifisReportMessageAttachmentfailed to detect them.Fixed Issues
$ #74031
Tests
Test matrix (2 action types x 4 attachment types = 8 cases):
[Attachment])Offline tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari