Attachment - Edit comment option is shown for .doc file#76167
Attachment - Edit comment option is shown for .doc file#76167lorretheboy wants to merge 21 commits intoExpensify:mainfrom
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
trjExpensify
left a comment
There was a problem hiding this comment.
Commented on the issue, not sure we want to solve it like this.
|
@lorretheboy can you please rework on this to allow editing all attachment-only messages? |
|
@situchan If we allow to edit attachment only message, it will look like this Screen.Recording.2026-02-09.at.01.12.21.movHowever, after edit the attachment, it turns to normal message like this. It should be editable as welll right?
Also I think there is no way to upload an attachment with text if we don't use rich text |
yes, it's the production behavior. We can move forward as long as the fix doesn't cause regression which doesn't happen on production. Screen.Recording.2026-02-12.at.2.54.47.AM.mov |
|
@situchan I pushed the fix but it won't work if we reload the page, we need update BE to return correct values website.movOptimistic data: {
"reportActionID": "8020209584774169997",
"reportID": "4958621070272591",
"actionName": "ADDCOMMENT",
"actorAccountID": 21324019,
"person": [
{
"style": "strong",
"text": "Lorre",
"type": "TEXT"
}
],
"automatic": false,
"avatar": "https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/default-avatar_7.png",
"created": "2026-02-16 08:49:45.304",
"message": [
{
"html": "AAAAAAAAA<br /><br /><a href=\"https://www.expensify.com/chat-attachments/8020209584774169997/w_f57043752d7f79dd0deb22fb57a33640150deea9.doc\" data-expensify-source=\"https://www.expensify.com/chat-attachments/8020209584774169997/w_f57043752d7f79dd0deb22fb57a33640150deea9.doc\">file-sample_100kB.doc</a>",
"text": "AAAAAAAAA\n\n[Attachment]",
"type": "COMMENT",
"whisperedTo": []
}
],
"originalMessage": {
"html": "AAAAAAAAA<br /><br /><a href=\"https://www.expensify.com/chat-attachments/8020209584774169997/w_f57043752d7f79dd0deb22fb57a33640150deea9.doc\" data-expensify-source=\"https://www.expensify.com/chat-attachments/8020209584774169997/w_f57043752d7f79dd0deb22fb57a33640150deea9.doc\">file-sample_100kB.doc</a>",
"whisperedTo": [],
"idempotencyKey": "06792075-225c-ef69-0bc2-0caa9426280d",
"isNewDot": true,
"lastModified": "2026-02-16 08:49:45.304"
},
"isFirstItem": false,
"isAttachmentOnly": false,
"isAttachmentWithText": true,
"shouldShow": true,
"lastModified": "2026-02-16 08:49:45.304"
}Backend returns {
"person": [
{
"type": "TEXT",
"style": "strong",
"text": "Lorre"
}
],
"actorAccountID": 21324019,
"message": [
{
"type": "COMMENT",
"html": "AAAAAAAAA<br \/><br \/><a href=\"https:\/\/www.expensify.com\/chat-attachments\/8020209584774169997\/w_f57043752d7f79dd0deb22fb57a33640150deea9.doc\" data-expensify-source=\"https:\/\/www.expensify.com\/chat-attachments\/8020209584774169997\/w_f57043752d7f79dd0deb22fb57a33640150deea9.doc\">file-sample_100kB.doc<\/a>",
"text": "AAAAAAAAA\n\nfile-sample_100kB.doc https:\/\/www.expensify.com\/chat-attachments\/8020209584774169997\/w_f57043752d7f79dd0deb22fb57a33640150deea9.doc",
"isEdited": false,
"whisperedTo": [],
"isDeletedParentAction": false,
"deleted": "",
"reactions": []
}
],
"originalMessage": {
"html": "AAAAAAAAA<br \/><br \/><a href=\"https:\/\/www.expensify.com\/chat-attachments\/8020209584774169997\/w_f57043752d7f79dd0deb22fb57a33640150deea9.doc\" data-expensify-source=\"https:\/\/www.expensify.com\/chat-attachments\/8020209584774169997\/w_f57043752d7f79dd0deb22fb57a33640150deea9.doc\">file-sample_100kB.doc<\/a>",
"idempotencyKey": "06792075-225c-ef69-0bc2-0caa9426280d",
"isNewDot": true,
"lastModified": "2026-02-16 08:49:45.304"
},
"avatar": "https:\/\/d2k5nsl2zxldvw.cloudfront.net\/images\/avatars\/default-avatar_7.png",
"created": "2026-02-16 08:49:45.304",
"timestamp": 1771231785,
"reportActionTimestamp": 1771231785304,
"automatic": false,
"actionName": "ADDCOMMENT",
"shouldShow": true,
"reportActionID": "8020209584774169997",
"lastModified": "2026-02-16 08:49:45.304",
"whisperedToAccountIDs": []
} |
|
I think those values are optimistic only. It's expected not coming from backend. |
|
I will have another look today |
|
@lorretheboy how's it going? |
|
@situchan Sorry for the delay, I pushed the latest code. Is it matched your expectation? The logic here is that we only allow edit the msg for attachment only messages Screen.Recording.2026-03-11.at.00.34.07.mov |
|
Please merge main |
Why can't edit attachment+text messages? |
🤖 PR ReviewCI Failures1. ESLint errors (PR-related, must fix): In const attachmentElementRegex = new RegExp(
- `<img[^>]* ${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="[^"]*"[^>]*\/?>|<(?:a|video)[^>]* ${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="[^"]*"[^>]*>[\\s\\S]*?<\/(?:a|video)>`,
+ `<img[^>]* ${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="[^"]*"[^>]*/?>|<(?:a|video)[^>]* ${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="[^"]*"[^>]*>[\\s\\S]*?</(?:a|video)>`,
'gi',
);2. Full ESLint check: Same 2 errors above — the other 327 warnings are pre-existing and unrelated to this PR. 3. test (job 6): Failure in Code ReviewApproach: The refactor from flag-based detection ( Logic in (!isAttachmentMessage || isEditableAttachment)This allows editing when the message either (a) has no attachment, or (b) is attachment-only (no authored text). This means:
Note: The old code allowed editing attachment-with-text messages when they were non-optimistic (i.e., after reload from server). The new code blocks editing for all attachment-with-text messages. Based on the PR conversation, this appears to be the accepted direction, but worth confirming this is intentional.
Tests: Good coverage of the key cases (translated attachment-only, file attachment-only, attachment-with-text, non-attachment). Consider adding a video attachment test case as well. Action Items
|
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
@situchan At the beginning, we agree that we should rework to allow editing all attachment-only messages? Not allow:
Allow:
|
yes because we already support editing attachment+text messages in production. |
|
Alright I just to confirm again the logic, we are now switch back to original approach right? We will allow to edit any msg that contains attachment, am I correct? @situchan |
Correct Expected Result: attachment-only ✅, attachment+text ✅ (✅: editable, ❌: not editable) Let me know if anything is unclear. |
|
The solution is completely wrong. It causes various regressions.
Please update code in high quality.
Test both and compare this branch vs main, find & fix all regressions in 8 (2x4) cases:
Test scope: LHN last message, report action style, edit visibility, after edit |
|
I couldn't come up with new solution (the original one is outdated already). Maybe can ask for re-assign... |
|
@MelvinBot implement this in new PR |
|
🤖 Cannot proceed: |






Explanation of Change
Fixed Issues
$ #74031
PROPOSAL: #74031 (comment)
Tests
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))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
Screen.Recording.2025-11-27.at.02.24.51.mov
Android: mWeb Chrome
Screen.Recording.2025-11-27.at.02.25.42.mov
iOS: Native
Screen.Recording.2025-11-27.at.02.23.07.mov
iOS: mWeb Safari
Screen.Recording.2025-11-27.at.02.22.14.mov
MacOS: Chrome / Safari
Screen.Recording.2025-11-27.at.02.26.34.mov