Skip to content
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

[$1000] mp4 files sent by Concierge display in the DM as: "Your browser does not support HTML5 video" #19964

Closed
6 tasks done
trjExpensify opened this issue Jun 1, 2023 · 48 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@trjExpensify
Copy link
Contributor

trjExpensify commented Jun 1, 2023

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


Action Performed:

  1. Navigate to the Concierge DM
  2. Request that Concierge send you a mp4 file for testing using drag and drop

Expected Result:

The mp4 file appears in the Concierge DM and is available to download

Actual Result:

Displays as "Your browser does not support HTML5 video" in written text

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: v1.3.21-2
Reproducible in staging?: Y
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail: Found when executing the QA of this PR but might not be related.
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
uploads_1685567315876-2023-05-31_14-08-33
image

Expensify/Expensify Issue URL:
Issue reported by: @trjExpensify
Slack conversation:

CC: @kidroca @hayata-suenaga

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0187897be4677fb987
  • Upwork Job ID: 1668383853323608064
  • Last Price Increase: 2023-06-15
@trjExpensify trjExpensify added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 1, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 1, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 1, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@kidroca
Copy link
Contributor

kidroca commented Jun 1, 2023

@trjExpensify It seems there's a specific way which has to be following to lead to this result

When I ask Concierge to send me a mp4 file, I don't get the described bug, but a link where I can download the file from:
image

@trjExpensify
Copy link
Contributor Author

Nope, not that I'm aware of. Though @alexpensify was my Concierge buddy. Did you send the mp4 file a specific way?

@alexpensify
Copy link
Contributor

I'm using drag and drop into the chat. @trjExpensify - I sent another test to you right now, can you check your test account? Thanks!

@trjExpensify
Copy link
Contributor Author

Same:

image

@strepanier03
Copy link
Contributor

Started testing steps now and will follow up shortly.

@melvin-bot melvin-bot bot added the Overdue label Jun 5, 2023
@kidroca
Copy link
Contributor

kidroca commented Jun 5, 2023

We were able to recreate the problem in a Concierge chat, but we had to follow the exact same steps regarding drag and dropping the video file

The resulting message "Your browser does not support HTML5 video." is just a plain message
image

It seems the bug is not in App, but in whatever this drag & drop flow involves

@melvin-bot
Copy link

melvin-bot bot commented Jun 5, 2023

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

@trjExpensify
Copy link
Contributor Author

Intereeesting! @chiragsalian as a resident Concierge expert might know what's what here. If it's not an issue in /App though, seems like this will likely need to go internal.

@strepanier03
Copy link
Contributor

strepanier03 commented Jun 6, 2023

I was having some trouble in Concierge but got it resolved today.

Confirming that using drag and drop this was easily recreatable.

On the Concierge side:
image

In Echat:
image

I updated the repro steps to include drag-and-drop instructions. This is the only way I was able to recreate the issue.

@melvin-bot melvin-bot bot removed the Overdue label Jun 6, 2023
@strepanier03
Copy link
Contributor

@chiragsalian - I didn't set Internal or External until you had a chance to weigh in. Feel free to let me know which is appropriate or toss the label on yourself, either way, I'll continue following this and help move it forward. Thank you!

@strepanier03
Copy link
Contributor

@chiragsalian - Can you review #19964 (comment) and weigh in?

Should this go internal?

@melvin-bot melvin-bot bot added the Overdue label Jun 12, 2023
@strepanier03 strepanier03 added the Internal Requires API changes or must be handled by Expensify staff label Jun 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 12, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 12, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @narefyev91 (Internal)

@strepanier03 strepanier03 added Help Wanted Apply this label when an issue is open to proposals by contributors Engineering and removed Help Wanted Apply this label when an issue is open to proposals by contributors labels Jun 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 12, 2023

Triggered auto assignment to @MariaHCD (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@strepanier03
Copy link
Contributor

Not 100% sure if this should be internal or external. @MariaHCD or @narefyev91 can you give it a set of eyes and help me figure out if this should be handled internally or externally?

If it should be external please let me know and I can quickly make the change.

@melvin-bot melvin-bot bot added the Overdue label Jul 5, 2023
@muttmuure
Copy link
Contributor

No update needed

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 5, 2023
@muttmuure
Copy link
Contributor

@MariaHCD I know you've got other high priorities but do you think you'll be able to push this one along a bit this week?

@melvin-bot melvin-bot bot removed the Overdue label Jul 10, 2023
@MariaHCD
Copy link
Contributor

I've got the upload to S3 working on the backend:

Screenshot 2023-07-10 at 7 27 28 PM

And the report action in the app now has the video tag with the link to the uploaded video on S3:

Screenshot 2023-07-10 at 7 36 07 PM

But I think there will be some changes required to the frontend, since the report action is not being parsed correctly, it is showing up as an empty message, strangely:

Screenshot 2023-07-10 at 7 39 34 PM

@MariaHCD
Copy link
Contributor

Asked in #engineering-chat if the App supports the <video> tag and if we should just convert to <a> instead: https://expensify.slack.com/archives/C03TQ48KC/p1689004812375609

@melvin-bot melvin-bot bot added the Overdue label Jul 12, 2023
@MariaHCD
Copy link
Contributor

Going to polish the PR and send it out for review today.

@melvin-bot melvin-bot bot removed the Overdue label Jul 13, 2023
@MariaHCD
Copy link
Contributor

Ah, actually, I want to look into this more: #19964 (comment)

And see if the app does support <video> tags or not.

@melvin-bot melvin-bot bot added the Overdue label Jul 17, 2023
@MariaHCD
Copy link
Contributor

MariaHCD commented Jul 17, 2023

Continuing on the PR this week but prioritizing some other dailies, chores, & projects first.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 17, 2023
@muttmuure
Copy link
Contributor

Not overdue

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 20, 2023
@MariaHCD
Copy link
Contributor

Going to aim to push the PR out this week.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 24, 2023
@muttmuure
Copy link
Contributor

Not overdue

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 27, 2023
@MariaHCD
Copy link
Contributor

Polishing the PR today.

@melvin-bot melvin-bot bot removed the Overdue label Jul 31, 2023
@MariaHCD MariaHCD added the Reviewing Has a PR in review label Jul 31, 2023
@MariaHCD
Copy link
Contributor

MariaHCD commented Aug 2, 2023

The fix was deployed to production: https://github.com/Expensify/Web-Expensify/pull/37951#issuecomment-1661180185

One limitation is that dragging and dropping multiple video files at once does not work as expected. I found that using the attachment picker to upload multiple video files does not work either.

I'm not sure if this is something that would be valuable to fix since Concierge agents can upload one video at a time. But if this would be a big QOL improvement, feel free to reopen, I can try to look further into it.

@MariaHCD MariaHCD closed this as completed Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

7 participants