Repository navigation
Show an edited file attachment as [Attachment] in the chat list - #102746
Merged
Merged
Conversation
mukhrr
changed the title
mukhrr
changed the title
Contributor
Author
|
Holding for #101717 |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
The server drops data-expensify-source from a file link when the comment is edited and keeps only data-attachment-id, so the parser read the link as plain text and the row showed the file name. The row text and the attachment-only check now also accept data-attachment-id. A typed link to a chat-attachments URL never gets that attribute, so it keeps its label.
Contributor
Author
|
#101717 is merged. Now working on this PR.. |
mukhrr
changed the title
The row text now treats a link as a file exactly when the chat draws it as a card: isAttachmentAnchorTag or isOwnAttachmentURL, both from AttachmentAnchorUtils. A file link edited to point at another host, or a pasted link to another message's file, keeps its label. isReportMessageAttachment goes back to main. Widening it moved edited file-only messages onto AttachmentCommentFragment instead of the TextCommentFragment path the card fix relies on, and the row text alone already reads them as [Attachment].
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
wrapAttachmentAnchorsInBlocks and replaceAttachmentAnchorsWithText both decided on their own whether an anchor is a file card, so a change to that rule had to be made twice. Both now call isFileCardAnchor. The card markup is unchanged.
# Conflicts: # tests/unit/ReportActionsUtilsTest.ts
Contributor
Author
|
@brunovjk we are good to go here. can you pls review?! thanks! |
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Contributor
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp102746_android_native.movAndroid: mWeb Chrome102746_android_web.moviOS: HybridApp102746_ios_native.moviOS: mWeb Safari102746_ios_web.movMacOS: Chrome / Safari102746_web_chrome.mov |
lakchote
approved these changes
Oct 26, 2026
lakchote
left a comment
lakchote
left a comment
Contributor
There was a problem hiding this comment.
Looks good, just one NAB.
| } | ||
| // Sometime html can be an empty string | ||
| return reportAction?.message?.reduce((acc, curr) => `${acc}${getTextFromHtml(curr?.html || curr?.text)}`, '') ?? ''; | ||
| return reportAction?.message?.reduce((acc, curr) => `${acc}${getTextFromHtml(curr?.html || curr?.text, reportAction.reportActionID)}`, '') ?? ''; |
Contributor
There was a problem hiding this comment.
NAB: when message is a single object, the early return above uses getReportActionText, which still calls Parser.htmlToText directly. Can we run replaceAttachmentAnchorsWithText there too, so the row shows [Attachment] for both shapes?
Contributor
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Contributor
|
🚧 lakchote has triggered a test Expensify/App build. You can view the workflow run here. |
Contributor
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
84 tasks done
Contributor
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.5.21-0 🚀
|
Contributor
|
No help site update is needed. This bug fix keeps the chat row preview showing [Attachment] after you edit a file comment, which is what users already expect, and it doesn't make any help article sentence incorrect. |
Contributor
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.5.23-0 🚀
|
Contributor
|
No help site update is needed. This bug fix keeps the chat row preview showing [Attachment] after you edit a file comment, which is what users already expect, and it doesn't make any help article sentence incorrect. |
Contributor
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.5.23-3 🚀
Bundle Size Analysis (Sentry): |
1 of 7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
When a file comment is edited, the server drops
data-expensify-sourcefrom the file link, so the parser no longer reads it as an attachment and the chat list row shows the file name instead of [Attachment]. The row text now swaps a file link for [Attachment] using the same check that decides whether the chat draws it as a file card.Fixed Issues
$ #101693
PROPOSAL:
Tests
https://example.com/file.csvand savesee [old sheet](<file URL>)see old sheetOffline tests
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
101693-android-app.mp4
Android: mWeb Chrome
101693-android-web.mp4
iOS: Native
101693-ios-app.mp4
iOS: mWeb Safari
101693-ios-web.mp4
MacOS: Chrome / Safari
web.mp4