[HOLD for payment 2025-01-18] [$250] LHN - Video name is shown in LHN briefly instead of attachment text

[vincdargento]

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: V9. 0.77-2
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause Internal Team
Device used: Redmi note 10s android 13
App Component: Left Hand Navigation (LHN)

Action Performed:

1. Go to https://staging.new.expensify.com
2. Open a chat
3. Upload a video and immediately navigate to LHN and note the report preview

Expected Result:

Video name must not be shown in LHN briefly and only attachment text must be shown.

Actual Result:

Video name is shown in LHN briefly instead attachment text.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021869790347889402203
• Upwork Job ID: 1869790347889402203
• Last Price Increase: 2024-12-26

Issue Owner

Current Issue Owner: @lschurr

[melvin-bot]

Triggered auto assignment to @lschurr (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.

[bernhardoj]

Proposal

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

Video name is shown when uploading a video to a chat. This happen to other attachments too except image.

What is the root cause of that problem?

When we send a new attachment, we get the optimistic HTML representation of the attachment.

App/src/libs/ReportUtils.ts

Lines 4432 to 4435 in e42622c

	const attachmentHtml = getUploadingAttachmentHtml(file);
	const htmlForNewComment = `${commentText}${commentText && attachmentHtml ? '<br /><br />' : ''}${attachmentHtml}`;
	const textForNewComment = Parser.htmlToText(htmlForNewComment);

Then, we convert it to text using htmlToText, which will later be used as the optimistic last message text.

App/src/libs/ReportUtils.ts

Line 4464 in e42622c

text: textForNewComment,

App/src/libs/actions/Report.ts

Lines 533 to 541 in e42622c

	const lastAction = attachmentAction ?? reportCommentAction;
	const currentTime = DateUtils.getDBTimeWithSkew();
	const lastComment = ReportActionsUtils.getReportActionMessage(lastAction);
	const lastCommentText = ReportUtils.formatReportLastMessageText(lastComment?.text ?? '');
	const optimisticReport: Partial<Report> = {
	lastVisibleActionCreated: lastAction?.created,
	lastMessageTranslationKey: lastComment?.translationKey ?? '',
	lastMessageText: lastCommentText,

However, htmlToText doesn't handle the video case yet. It only handles image.
https://github.com/Expensify/expensify-common/blob/706481004516cddf08b7652afdf5db951443fbea/lib/ExpensiMark.ts#L799-L803

Then, the BE return the correct last message text which contains the [Attachment] text.

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

We should add the case for video and other attachment to ExpensiMark parser too.

{
    name: 'video',
    regex: /<video[^><]*data-expensify-source\s*=\s*(['"])(\S*?)\1(.*?)>([^><]*)<\/video>*(?![^<][\s\S]*?(<\/pre>|<\/code>))/gi,
    replacement: '[Attachment]',
},
{
    name: 'otherAttachment',
    regex: /<a[^><]*data-expensify-source\s*=\s*(['"])(\S*?)\1(.*?)>([^><]*)<\/a>*(?![^<][\s\S]*?(<\/pre>|<\/code>))/gi,
    replacement: '[Attachment]',
},

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We need to add the test to expensify-common repo ExpensiMark by asserting that an attachment link (image, video, others) will be parsed to [Attachment].

[melvin-bot]

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

[melvin-bot]

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

[kaushiktd]

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

LHN - Video name is shown in LHN briefly instead of attachment text

What is the root cause of that problem?

The issue occurs because in buildOptimisticAddCommentReportAction determines the text displayed in the LHN by combining the comment's text and attachmentHtml.

getUploadingAttachmentHtml(file):

This function generates HTML for the attachment, including the file name. For a video file, this HTML includes the video name (e.g., "video.mp4").
When there’s no accompanying text, the optimistic action uses this HTML for both html and text, causing the file name to appear in the LHN.

Root Cause Summary:
The textForNewComment variable is derived from htmlForNewComment, which includes the file name for attachment-only messages. Since this variable is used to populate the LHN in the optimistic update, it briefly shows the file name instead of the intended "Attachment" placeholder.

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

For attachment-only comments, explicitly set the text to a placeholder (CONST.ATTACHMENT_MESSAGE_TEXT) instead of deriving it from the attachment's HTML.

App/src/libs/ReportUtils.ts

Line 4435 in e42622c

const textForNewComment = Parser.htmlToText(htmlForNewComment);

Replace it with

const textForNewComment = isAttachmentOnly ? CONST. ATTACHMENT_MESSAGE_TEXT:Parser.htmlToText(htmlForNewComment);

video:-
screen-capture.webm

[shubham1206agra]

@lschurr Please look into the ideal behaviour as it might be changed from #54295.
cc @puneetlath

[melvin-bot]

@akinwale, @lschurr Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@akinwale, @lschurr Eep! 4 days overdue now. Issues have feelings too...

[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]

@akinwale, @lschurr Still overdue 6 days?! Let's take care of this!

[lschurr]

Can you review the proposals here @akinwale?

[akinwale]

I initially thought there was some pending action based on this comment.

In any case, @bernhardoj's proposal works here.

🎀👀🎀 C+ reviewed.

[melvin-bot]

Triggered auto assignment to @marcaaron, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[bernhardoj]

expensify-common PR is ready

cc: @akinwale

[melvin-bot]

@akinwale, @marcaaron, @lschurr, @bernhardoj Whoops! This issue is 2 days overdue. Let's get this updated quick!

[bernhardoj]

App PR is ready, but I think we need to hold until the app is updated to include the changes from the previous expensify-common version: Expensify/expensify-common#827

cc: @akinwale

[melvin-bot]

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

[melvin-bot]

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

• Update expensify-common to 2.0.114 #54685

If no regressions arise, payment will be issued on 2025-01-18. 🎊

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

• @akinwale requires payment through NewDot Manual Requests
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

@akinwale @lschurr @akinwale The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]