-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[$250] LHN - Video name is shown in LHN briefly instead of attachment text #54366
Comments
Triggered auto assignment to @lschurr ( |
ProposalPlease 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. Lines 4432 to 4435 in e42622c
Then, we convert it to text using Line 4464 in e42622c
App/src/libs/actions/Report.ts Lines 533 to 541 in e42622c
However, 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.
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]. |
Job added to Upwork: https://www.upwork.com/jobs/~021869790347889402203 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale ( |
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"). Root Cause Summary: 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. Line 4435 in e42622c
Replace it with
video:- |
@lschurr Please look into the ideal behaviour as it might be changed from #54295. |
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: 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:
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:
Screenshots/Videos
bug.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @akinwaleThe text was updated successfully, but these errors were encountered: