[HOLD for payment 2024-06-05] [$250] Chat - Text with attachment message can be edited when the attachment is uploading #40400

izarutskaya · 2024-04-18T08:22:18Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 1.4.63-0
Reproducible in staging?: Y
Reproducible in production?: N
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Go to staging.new.expensify.com
Go to chat.
Copy an image (from any source).
Enter text in the composer.
Paste the image via CMD+V.
Send the attachment together with the text.
While the attachment is uploading, right click on the message > Edit comment.
Edit the comment and save it.

Expected Result:

In Step 7, app should prevent comment to be editable when the attachment is uploading.

Actual Result:

In Step 7, app allows editing when the attachment with text is uploading.
As a result, the attachment is not uploaded and becomes "Uploading attachment..." when editing and saving the comment (Step 8).

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Bug6453282_1713425265582.text_and_attachment.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~016d10a063fea0e0dc
Upwork Job ID: 1780928057418211328
Last Price Increase: 2024-04-25
Automatic offers:

FitseTLT | Contributor | 0

Issue Owner

Current Issue Owner: @MitchExpensify

melvin-bot · 2024-04-18T08:22:27Z

Triggered auto assignment to @MariaHCD (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

melvin-bot · 2024-04-18T08:22:27Z

Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

github-actions · 2024-04-18T08:22:34Z

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

izarutskaya · 2024-04-18T08:22:48Z

@MitchExpensify I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

izarutskaya · 2024-04-18T08:22:58Z

We think this issue might be related to the #collect project.

izarutskaya · 2024-04-18T08:23:26Z

Production

bandicam.2024-04-18.10-50-01-227.mp4

Nodebrute · 2024-04-18T09:55:33Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Text with attachment message can be edited when the attachment is uploading #

What is the root cause of that problem?

Here, isAttachment will be false. That's why we can see the edit icon for comments that have text and attachments.

App/src/libs/ReportActionsUtils.ts

Line 845 in 3ec8f2f

return reportAction?.isAttachment ?? !!reportAction?.attachmentInfo ?? false;

What changes do you think we should make in order to solve the problem?

We can set isAttachment to true in reportAction for cases like these.
Or we can simply check if reportAction.attachmentInfo is true. We can do something like this

reportAction?.attachmentInfo !== undefined

App/src/libs/ReportActionsUtils.ts

Line 845 in 3ec8f2f

return reportAction?.isAttachment ?? !!reportAction?.attachmentInfo ?? false;

3. We can create a new utility function hasAttachment for cases like these.

What alternative solutions did you explore? (Optional)

shahinyan11 · 2024-04-18T10:06:45Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Chat - Text with attachment message can be edited when the attachment is uploading

What is the root cause of that problem?

Currently isReportActionAttachment function does not return true for report actions with text and attachment

What changes do you think we should make in order to solve the problem?

Update this condition like below

return (reportAction?.isAttachment || !isEmptyObject(reportAction?.attachmentInfo)) ?? false;

Add || (ReportActionsUtils.isReportActionAttachment(reportAction) && !reportAction?.isOptimisticAction) in this check

return Boolean(
    (reportAction?.actorAccountID === currentUserAccountID &&
        isCommentOrIOU &&
        canEditMoneyRequest(reportAction) && // Returns true for non-IOU actions
        !ReportActionsUtils.isReportActionAttachment(reportAction) &&
        !ReportActionsUtils.isDeletedAction(reportAction) &&
        !ReportActionsUtils.isCreatedTaskReportAction(reportAction) &&
        reportAction?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) ||
        (ReportActionsUtils.isReportActionAttachment(reportAction) && !reportAction?.isOptimisticAction),
);

What alternative solutions did you explore? (Optional)

melvin-bot · 2024-04-18T11:55:14Z

Job added to Upwork: https://www.upwork.com/jobs/~016d10a063fea0e0dc

melvin-bot · 2024-04-18T11:55:18Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr (External)

mollfpr · 2024-04-18T11:57:44Z

@Nodebrute @shahinyan11 Did you guys find the offending PR?

ShridharGoel · 2024-04-18T12:11:53Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Text with attachment message can be edited when the attachment is uploading.

What is the root cause of that problem?

Edit option shows even when attachment is uploading. It should only show after the upload is complete.

Also, the existing logic doesn't consider "text+attachments" combination as an attachment, isReportActionAttachment returns false when text is also there with the attachment.

What changes do you think we should make in order to solve the problem?

When attachment is uploading, edit action shouldn't show.
It can be shown after upload is complete.

We have to get the message HTML and check for messageHtml !== CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML.

Move the below existing method to ReportActionsUtils, so that we can use it in ReportUtils also, to get the messageHtml.

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Lines 36 to 40 in 68e6962

    
           /** Gets the HTML version of the message in an action */ 
        
           function getActionHtml(reportAction: OnyxEntry<ReportAction>): string { 
        
               const message = reportAction?.message?.at(-1) ?? null; 
        
               return message?.html ?? ''; 
        
           }

Sample code:

function canEditReportAction(reportAction: OnyxEntry<ReportAction>): boolean {
    const isCommentOrIOU = reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT || reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU;

    const isAttachmentInUploadingState = () => {
        const messageHtml = ReportActionsUtils.getActionHtml(reportAction);
        return messageHtml.includes(CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML);
    }

    const shouldShowEditForAttachment = ReportActionsUtils.isReportActionAttachment(reportAction) && !isAttachmentInUploadingState()

    return Boolean(
        reportAction?.actorAccountID === currentUserAccountID &&
            isCommentOrIOU &&
            canEditMoneyRequest(reportAction) && // Returns true for non-IOU actions
            (!ReportActionsUtils.isReportActionAttachment(reportAction) || shouldShowEditForAttachment) && // Either it's not an attachment, or if it is an attachment then it shouldn't be in uploading state
            !ReportActionsUtils.isDeletedAction(reportAction) &&
            !ReportActionsUtils.isCreatedTaskReportAction(reportAction) &&
            reportAction?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
    );
}

Also, update the condition to check for attachments in ReportActionUtils to this:

(reportAction?.isAttachment || !!reportAction?.attachmentInfo) ?? false;

Screen.Recording.2024-04-18.at.6.34.36.PM.mov

ShridharGoel · 2024-04-18T12:13:34Z

Offending PR: #39007 (It's a new feature which combines text and attachments).

FitseTLT · 2024-04-18T12:23:16Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Text with attachment message can be edited when the attachment is uploading

What is the root cause of that problem?

It is a regression from #39007 which modifies optimistic data for text+attachment but doesn't align with what BE returns b/c for text+attachment it doesn't set isAttachment
So here

App/src/libs/ReportActionsUtils.ts

Lines 845 to 846 in 3ec8f2f

    
               return reportAction?.isAttachment ?? !!reportAction?.attachmentInfo ?? false; 
        
           }

it will return false because we have set isAttachment to false here

App/src/libs/ReportUtils.ts

Lines 3223 to 3224 in 3ec8f2f

    
           const isAttachment = !text && file !== undefined; 
        
           const attachmentInfo = file ?? {};

What changes do you think we should make in order to solve the problem?

For text + attachment we should set isAttachment to undefined Change this

App/src/libs/ReportUtils.ts

Lines 3253 to 3254 in 3ec8f2f

    
           isAttachment, 
        
           attachmentInfo,

to

            ...(!(text && file !== undefined) && {isAttachment}),

What alternative solutions did you explore? (Optional)

If we want to edit text+attachment messages we should change our logic from using isReportActionAttachment to using isReportMessageAttachment because it will check that it is only attachment without message and additionally we should check if it is not isOptimisticAction when it has attachmentInfo (text+attachment) so that it will not enable editing for not yet uploaded attachment.

App/src/libs/ReportUtils.ts

Line 2557 in 3ec8f2f

!ReportActionsUtils.isReportActionAttachment(reportAction) &&

 !isReportMessageAttachment(reportAction?.message?.[0]) &&
            !(!isEmptyObject(reportAction.attachmentInfo) && reportAction.isOptimisticAction) &&

mollfpr · 2024-04-18T12:34:21Z

We can set isAttachment to true in reportAction for cases like these.

@Nodebrute isAttachment also true after the message is sent?

@shahinyan11 I try your solution but it's not working.

@FitseTLT I think your solution will block the user after the message is sent.

mollfpr · 2024-04-18T12:37:41Z

We have to get the message HTML and check for messageHtml !== CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML.

@ShridharGoel Where is this check implemented?

melvin-bot · 2024-05-13T19:17:42Z

@MariaHCD, @mollfpr, @MitchExpensify, @FitseTLT Huh... This is 4 days overdue. Who can take care of this?

mollfpr · 2024-05-13T19:18:48Z

@FitseTLT Let me know when the PR ready, thanks!

MariaHCD · 2024-05-16T05:58:21Z

Not overdue; we're waiting on a PR from @FitseTLT

melvin-bot · 2024-05-29T09:15:59Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2024-05-29T09:16:04Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.76-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Fix text attachment edit comment bug #42290

If no regressions arise, payment will be issued on 2024-06-05. 🎊

For reference, here are some details about the assignees on this issue:

@mollfpr requires payment through NewDot Manual Requests
@FitseTLT requires payment automatic offer (Contributor)

melvin-bot · 2024-05-29T09:16:05Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
[@mollfpr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@mollfpr] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@MitchExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

MitchExpensify · 2024-06-02T20:12:31Z

Reminder set to pay, thoughts on BZ steps @mollfpr ?

MitchExpensify · 2024-06-05T17:00:32Z

Payment summary:

$250 @mollfpr requires payment through NewDot Manual Requests
$250 @FitseTLT requires payment automatic offer (Contributor)

MitchExpensify · 2024-06-05T17:02:54Z

Paid and contract ended @FitseTLT

mollfpr · 2024-06-10T17:44:12Z

Sorry for the delay 🙏

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
[@mollfpr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

No offending PR was found.

[@mollfpr] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

This is an improvement for a new feature, so the regression step should be enough.

[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Go to staging.new.expensify.com
Go to chat.
Copy an image (from any source).
Enter text in the composer.
Paste the image via CMD+V.
Send the attachment together with the text.
While the attachment is uploading, right-click on the message
Verify Edit comment is not included in the context menu list
Wait until the attachment is uploaded and open the context menu on the message
Verify Edit comment menu exists and press on it
Check that now the text+attachment message can be edited
👍 or 👎

melvin-bot · 2024-06-10T18:11:26Z

@MariaHCD, @mollfpr, @MitchExpensify, @FitseTLT Huh... This is 4 days overdue. Who can take care of this?

MitchExpensify · 2024-06-11T03:05:05Z

Thank you!

JmillsExpensify · 2024-06-11T18:19:13Z

$250 approved for @mollfpr

izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 18, 2024

melvin-bot bot assigned MariaHCD and MitchExpensify Apr 18, 2024

github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Apr 18, 2024

github-actions bot mentioned this issue Apr 18, 2024

Deploy Checklist: New Expensify 2024-04-18 #40392

Closed

MariaHCD added the External Added to denote the issue can be worked on by a contributor label Apr 18, 2024

melvin-bot bot changed the title ~~Chat - Text with attachment message can be edited when the attachment is uploading~~ [$250] Chat - Text with attachment message can be edited when the attachment is uploading Apr 18, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 18, 2024

melvin-bot bot assigned mollfpr Apr 18, 2024

melvin-bot bot mentioned this issue Apr 18, 2024

[Pay 29/4] [$250] HIGH: [Polish] Combine image attachment with the unsent composed message #35977

Closed

melvin-bot bot added Overdue and removed Overdue labels May 13, 2024

melvin-bot bot removed the Overdue label May 16, 2024

FitseTLT mentioned this issue May 16, 2024

Fix text attachment edit comment bug #42290

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels May 16, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels May 29, 2024

melvin-bot bot changed the title ~~[$250] Chat - Text with attachment message can be edited when the attachment is uploading~~ [HOLD for payment 2024-06-05] [$250] Chat - Text with attachment message can be edited when the attachment is uploading May 29, 2024

melvin-bot bot removed the Reviewing Has a PR in review label May 29, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 4, 2024

melvin-bot bot added the Overdue label Jun 7, 2024

MitchExpensify closed this as completed Jun 11, 2024

melvin-bot bot removed the Overdue label Jun 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-06-05] [$250] Chat - Text with attachment message can be edited when the attachment is uploading #40400

[HOLD for payment 2024-06-05] [$250] Chat - Text with attachment message can be edited when the attachment is uploading #40400

izarutskaya commented Apr 18, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Apr 18, 2024

melvin-bot bot commented Apr 18, 2024

github-actions bot commented Apr 18, 2024

izarutskaya commented Apr 18, 2024

izarutskaya commented Apr 18, 2024

izarutskaya commented Apr 18, 2024

Nodebrute commented Apr 18, 2024

shahinyan11 commented Apr 18, 2024 •

edited

Loading

melvin-bot bot commented Apr 18, 2024

melvin-bot bot commented Apr 18, 2024

mollfpr commented Apr 18, 2024

ShridharGoel commented Apr 18, 2024 •

edited

Loading

ShridharGoel commented Apr 18, 2024

FitseTLT commented Apr 18, 2024 •

edited

Loading

mollfpr commented Apr 18, 2024

mollfpr commented Apr 18, 2024

melvin-bot bot commented May 13, 2024

mollfpr commented May 13, 2024

MariaHCD commented May 16, 2024

melvin-bot bot commented May 29, 2024

melvin-bot bot commented May 29, 2024

melvin-bot bot commented May 29, 2024

MitchExpensify commented Jun 2, 2024

MitchExpensify commented Jun 5, 2024

MitchExpensify commented Jun 5, 2024

mollfpr commented Jun 10, 2024

melvin-bot bot commented Jun 10, 2024

MitchExpensify commented Jun 11, 2024

JmillsExpensify commented Jun 11, 2024

[HOLD for payment 2024-06-05] [$250] Chat - Text with attachment message can be edited when the attachment is uploading #40400

[HOLD for payment 2024-06-05] [$250] Chat - Text with attachment message can be edited when the attachment is uploading #40400

Comments

izarutskaya commented Apr 18, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Apr 18, 2024

melvin-bot bot commented Apr 18, 2024

github-actions bot commented Apr 18, 2024

izarutskaya commented Apr 18, 2024

izarutskaya commented Apr 18, 2024

izarutskaya commented Apr 18, 2024

Nodebrute commented Apr 18, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

shahinyan11 commented Apr 18, 2024 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Apr 18, 2024

melvin-bot bot commented Apr 18, 2024

mollfpr commented Apr 18, 2024

ShridharGoel commented Apr 18, 2024 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

ShridharGoel commented Apr 18, 2024

FitseTLT commented Apr 18, 2024 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

mollfpr commented Apr 18, 2024

mollfpr commented Apr 18, 2024

melvin-bot bot commented May 13, 2024

mollfpr commented May 13, 2024

MariaHCD commented May 16, 2024

melvin-bot bot commented May 29, 2024

melvin-bot bot commented May 29, 2024

melvin-bot bot commented May 29, 2024

MitchExpensify commented Jun 2, 2024

MitchExpensify commented Jun 5, 2024

MitchExpensify commented Jun 5, 2024

mollfpr commented Jun 10, 2024

melvin-bot bot commented Jun 10, 2024

MitchExpensify commented Jun 11, 2024

JmillsExpensify commented Jun 11, 2024

izarutskaya commented Apr 18, 2024 •

edited by melvin-bot bot

Loading

shahinyan11 commented Apr 18, 2024 •

edited

Loading

ShridharGoel commented Apr 18, 2024 •

edited

Loading

FitseTLT commented Apr 18, 2024 •

edited

Loading