[HOLD ON #35977] [$500] [MEDIUM] Attachment shown in the LHN when sending a message with image

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.13-14
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1708811957655339

Action Performed:

1. Open any chat
2. Send a message with attachment
3. Look at the LHN as a recipient

Expected Result:

In the LHN first line of the message should display

Actual Result:

Attachment is displayed

Workaround:

unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/45c13785-4037-41d3-addd-7d17341bab4a

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a1ad1258cf6daa29
• Upwork Job ID: 1761910899454898176
• Last Price Increase: 2024-03-18

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01a1ad1258cf6daa29

[melvin-bot]

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[Krishna2323]

Proposal

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

Attachment shown in the LHN when sending a message with image

What is the root cause of that problem?

We don't pass text to buildOptimisticAddCommentReportAction. And in buildOptimisticAddCommentReportAction we don't check if text is available or not for setting textForNewComment.

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

We need to pass the text to buildOptimisticAddCommentReportAction when we have file and in buildOptimisticAddCommentReportAction we need to check if text is available or not for textForNewComment. We also need to modify isAttachment variable to consider it to be a attachment if the file is available.

App/src/libs/ReportUtils.ts

Lines 2720 to 2729 in 89d5689

	function buildOptimisticAddCommentReportAction(text?: string, file?: File, actorAccountID?: number): OptimisticReportAction {
	const parser = new ExpensiMark();
	const commentText = getParsedComment(text ?? '');
	const isAttachment = !text && file !== undefined;
	const attachmentInfo = isAttachment ? file : {};
	const htmlForNewComment = isAttachment ? CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML : commentText;
	const accountID = actorAccountID ?? currentUserAccountID;
	// Remove HTML from text when applying optimistic offline comment
	const textForNewComment = isAttachment ? CONST.ATTACHMENT_MESSAGE_TEXT : parser.htmlToText(htmlForNewComment);

const isAttachment = file !== undefined;

const textForNewComment = isAttachment && !text ? CONST.ATTACHMENT_MESSAGE_TEXT : parser.htmlToText(htmlForNewComment);

Result

Alternative

Pass comment text along with the file to the backend and let backend handle the required changes.

[brandonhenry]

Updated Proposal

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

The issue is that when a message with an attachment is sent in the chat, the Left Hand Navigation (LHN) displays "[Attachment]" instead of the first line of the actual message text. This behavior is not consistent with the expected result where the LHN should display the first line of the message text, providing a quick preview of the conversation content. Confirmed this issue happens on all platforms.

What is the root cause of that problem?

I'd like to keep my original proposal in, but based on some recent comments here, I'd also like to address the separation of attachment and comments mentioned by others on this thread. It's stated that you cannot send a text comment along with an attachment (and that it is not supported by the BE). This is simply, not true. We already support sending a text comment along with the attachment as you can see here:

App/src/libs/actions/Report.ts

Line 418 in 1269110

function addAttachment(reportID: string, file: File, text = '') {

As such, if we want to truly update the LHN from saying [Attachment] we need to ensure the proper text is being sent to the BE. I can confirm that the message typed right before sending the attachment is DEFINITELY going through and should be sent to the BE.

That said, this can be fixed on the FE only.

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

To solve this issue, we should fix the addActions function in libs/actions/Report.ts so that it properly attaches the comment in draft to the image being added. This can be achieved by editing the code like so:

function addActions(reportID: string, text = '', file?: File) {
    // variables

    if (text) {
        // same code
    }

    if (file) {
        // When adding an attachment with an optional comment, use AddAttachment command.
        commandName = WRITE_COMMANDS.ADD_ATTACHMENT;

        // Include the text comment in the attachment action if present. <--- important change to fix
        const attachment = ReportUtils.buildOptimisticAddCommentReportAction(text, file);
        attachmentAction = attachment.reportAction;
        reportCommentText = text;  // Include the text in the report comment.
    }

With this version of the addActions function, when both a text comment and a file attachment are provided, the text comment is included in the attachment action. This ensures that the attachment and the accompanying text are sent together in a single report action. The reportCommentText variable is updated to include the text comment, and the parameters object for the API call includes both the comment and the file. This should address the issue where the attachment is shown in the LHN instead of the first line of the message.

[dukenv0307]

Proposal

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

• Attachment shown in the LHN when sending a message with image

What is the root cause of that problem?

• When sending an attachment with text, we consider it as two separate messages, one is text and the other one is attachment. So the current behavior is expected behavior, not a bug.
• If we want to display the text rather than [Attachment] in LHN, I think it is FEATURE REQUEST.

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

• We can consider the attachment with text as only one message. This need to fix on both BE and FE sides.

1. BE:

• When receiving "addComment" from FE, we need to save the report.lastMessageText to text rather than the [Attachment]
• With the reportAction, the stored action can be:

{
            "actionName": "ADDCOMMENT",
            "message": [
                {
                    "html": "text message",
                    "text": "text message",
                    "type": "COMMENT"
                },
                {
                    "html": "<img src=\"https://www.expensify.com/chat-attachments/1419100579578839579/w_a06983d17c1d56f1912499dd4eabaa7920efdcdae5.png.1024.jpg\" data-expensify-source=\"https://www.expensify.com/chat-attachments/1419100579578839579/w_a06983d17c1d56f19499dd4eabaa7920efdcdae5.png\" data-name=\"Screenshot_from_2024-02-23_19-06-48.png\" data-expensify-height=\"894\" data-expensify-width=\"1799\" />",
                    "text": "[Attachment]",
                    "type": "COMMENT"
                }
            ],
            "originalMessage": {
                "html": "text message <br /> <img src=\"https://www.expensify.com/chat-attachments/1419100579578839579/w_a06983d17c1d56f1949912dd4eabaa7920efdcdae5.png.1024.jpg\" data-expensify-source=\"https://www.expensify.com/chat-attachments/1419100579578839579/w_a06983d17c1d56f19499dd4eabaa7920efdcdae5.png\" data-name=\"Screenshot_from_2024-02-23_19-06-48.png\" data-expensify-height=\"894\" data-expensify-width=\"1799\" />",
                "lastModified": "2024-02-26 04:40:05.241"
            },
        }

2. FE:

• Based on the above change from BE side, we can update the optimistic data when sending attachment with text.
• Also, we can update the logic to display attachment with text by updating this one to:

    if (isAttachmentWithText(action)) {
        return (
            <View style={[styles.chatItemMessage, style]}>
                <TextCommentFragment
                    fragment={fragment[0]}
                    ...
                />
                <AttachmentCommentFragment
                    source={(action.originalMessage as OriginalMessageAddComment['originalMessage'])?.source}
                    ...
                />
            </View>
        );
    }

• isAttachmentWithText(action) is the function to check if the action is attachment with text or not.

What alternative solutions did you explore? (Optional)

Alternative solution

• Also we still can store two messages (text and attachment) separately but we need to display them as only one message. This needs to be handled on both BE and FE sides.

1. BE:

• We need to save the report.lastMessageText to text message rather than the [Attachment]
• In message text, we need to create the field hasLinkedAttachment to know, if this text message has a linked attachment or not. In this case, it is true

2. FE:

• We need to update this one to:

    const lastComment = (reportCommentAction ?? attachmentAction)?.message?.[0];

• With the above change, we will fix the issue. But it just applies "display them as only one message" to LHN. We also need to apply it to the report actions. Below is detail.

1. BE:

• When the comment DeleteComment is called, we will check if the message text has hasLinkedAttachment: true. If yes, we delete the next action, too.

2. FE:

• Hover over the attachment that has a linked text should not display the menu actions (Emojis, Reply in threads, ...). Do it by create a function, named isAttachmentHasLinkedText:

 function isAttachmentHasLinkedText(reportAction){
        return reportActions[reportAction.previousReportActionId].hasLinkedAttachment
}

[mollfpr]

I think this should be held on #35977. We currently send two messages separately, so the second message is an attachment, and on the LHN is showing [Attachment] is expected.

[mollfpr]

@brandonhenry You can't post more than one proposal. Please update your latest solution in the first proposal, and hide the second proposal.

Refrain from leaving additional comments until someone from the Contributor-Plus team and / or someone from Expensify provides feedback on your proposal (do not create a pull request yet).

• Do not leave more than one proposal.
• Do not make extensive changes to your current proposal until after it has been reviewed.
• If you want to make an entirely new proposal or update an existing proposal, please go back and edit your original proposal, then post a new comment to the issue in this format to alert everyone that it has been updated:

Ref: https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#propose-a-solution-for-the-job

---

I can confirm that the message typed right before sending the attachment is DEFINITELY going through and should be sent to the BE.

Could you proof this? I tried to send an attachment while having drafted text in the composer, and yes both of them are sent from the AddAttachment API, but the response shows that the text message and the attachment are in different actions.

Screen.Recording.2024-02-27.at.16.46.28.mp4

[brandonhenry]

@mollfpr pretty positive. And even if they are received separately, the BE supports tying a Text action to Attachment. If you were to apply the changes I proposed, you'll see that instead of them being retrieved separately, the text is applied directly to the attachment comment (instead of the text being "[Atttachment]". If the BE is sending two responses, I think that should be changed but as I look and understand the code now, we can definitely tie the draft comment to the attachment from the FE and display only that text included (as original post is wanting)

[brandonhenry]

Can upload a video a little later showing it working with my changes (and what the BE is sending) - if needed

[dukenv0307]

@mollfpr Do you have any feedback about my proposal here?

[mollfpr]

@brandonhenry You might be right that the FE already implemented the send attachment with the message, but still if the reportActions are created separately, then the last text shown in the LHN is from the last reportActions.

@dukenv0307 Your proposal seems to have the right path, but we have ongoing discussions about implementing the inline image here https://expensify.slack.com/archives/C01GTK53T8Q/p1707270017261459

@tgolen Do you mind confirming if this needs to hold for #35977 or #37246?

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@kevinksullivan, @mollfpr Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

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

[melvin-bot]

@kevinksullivan, @mollfpr 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@kevinksullivan @mollfpr this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

@kevinksullivan, @mollfpr 10 days overdue. I'm getting more depressed than Marvin.

[mollfpr]

I'll bring this to the Slack channel 👀

[mollfpr]

I'm sorry for misunderstanding the issue. Looking at the original bug report, the issue is with the message from the Concierge where they can send a text with an image in a single comment.

The feature to send text and inline images is currently under development. So it's not reproducible from user perspective.

Here's a report action example from the Concierge that sends a text with an inline image.

{
  "reportActionID": "4718340389065588544",
  "message": [
    {
      "type": "COMMENT",
      "html": "Thanks for your patience. I was able to find the flights you sent me. Are these flights acceptable?:<br /><br /><img src=\"https://s3-us-west-1.amazonaws.com/concierge-responses-expensify-com/uploads%2F1676906287486-1676906287485.png\" style=\"width:300px;\" class=\"fr-fic fr-dib\" alt=\"uploads%2F1676906287486-1676906287485.png\" />",
      "text": "Thanks for your patience. I was able to find the flights you sent me. Are these flights acceptable?:\n\n[Attachment]",
      "isEdited": false,
      "whisperedTo": [],
      "isDeletedParentAction": false,
      "reactions": []
    }
  ],
  "actionName": "ADDCOMMENT",
  "originalMessage": {
    "html": "Thanks for your patience. I was able to find the flights you sent me. Are these flights acceptable?:<br /><br /><img src=\"https://s3-us-west-1.amazonaws.com/concierge-responses-expensify-com/uploads%2F1676906287486-1676906287485.png\" style=\"width:300px;\" class=\"fr-fic fr-dib\" alt=\"uploads%2F1676906287486-1676906287485.png\" />",
    "lastModified": "2023-02-20 15:18:37.595"
  },
  "whisperedToAccountIDs": [],
  "created": "2023-02-20 15:18:37.595",
  "actorAccountID": 8392101,
  "person": [
    {
      "type": "TEXT",
      "style": "strong",
      "text": "Expensify Concierge"
    }
  ]
}

---

The problem now is hard to reproduce like the screenshot from @quinthar on the bug report, because not all users get the same message from the Concierge which is a text with the inline image.

[dukenv0307]

@mollfpr I can reproduce this bug in the normal chat

[mollfpr]

@dukenv0307 Is the message have the text and inline message on 1 report action?

[dukenv0307]

No. When sending an attachment with text, BE returns the attachment and text actions separately

[mollfpr]

I think that's the same as sending a text first and then the image on the second message. So the LHN is expected to show the last message [Attachment].

I haven't confirmed if we have the last message of an inline image that will show the [Attachment].

[dukenv0307]

I think we should hold for #35977, which we will display the attachment with text as only one action

[kevinksullivan]

Moving to VIP-VSB as this is tried to chat.

[kevinksullivan]

Also agree with the proposal to hold this

[kevinksullivan]

still held

[tgolen]

I think we should hold for #35977, which we will display the attachment with text as only one action

I've started working on this today. When there is a message that contains both text and an attachment, what should the LHN show? I am not super opinionated about it but I think it should probably show the text of the comment and not show [Attachment]. To be clear what I am proposing:

Situation	Preview in LHN
Text only	Truncated text of the comm...
Attachment only	[Attachment]
Text + Attachment	Truncated text of the comm...

Does that sound right?

[dukenv0307]

@tgolen What about the situation: "Text + Attachment + Text" or "Attachment + Text"?

[tgolen]

We'll eventually have to cover those I guess, but those aren't possible right now (or with my changes in #35977). Maybe we could:

• Replace all attachments in the comment with [File]
• Strip out all the HTML from the comment
• Truncate the remaining text (which also might cut off [File] but that's probably OK)

[kevinksullivan]

Taking this off hold @mollfpr

[kevinksullivan]

going to test this now actually.

[mollfpr]

@kevinksullivan I can't reproduce the issue in production. I think we can close this.