Sign in – New tab with video opens when click on download video in Welcome to Expensify modal

lanitochka17 commented 3 weeks ago

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: 9.0.22-5 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4871545 Email or phone of affected tester (no customers): applausetester+jp_319824@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

Go to https://staging.new.expensify.com/
Log in with a new Gmail account and click on Join button
Complete the Onboarding flow
In Welcome to Expensify modal video (not full screen) click on 3 dot and download video

Expected Result:

Video is downloaded

Actual Result:

New tab with video opens

Workaround:

Unknown

Platforms:

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

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/d24229cf-3a34-4e05-8115-0c81bd0d5f44

View all open jobs on GitHub

melvin-bot[bot] commented 3 weeks ago

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

lanitochka17 commented 3 weeks ago

@JmillsExpensify FYI 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

Krishna2323 commented 3 weeks ago

Proposal

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

Sign in – New tab with video opens when click on download video in Welcome to Expensify modal

What is the root cause of that problem?

The url for onboarding modal is https://d2k5nsl2zxldvw.cloudfront.net/videos/intro-1280.mp4 and due to this, the condition below !isApiUrl && !isAttachmentUrl && !isSageUrl) becomes true and a new tab opens. https://github.com/Expensify/App/blob/7b4605a9bf642bdc54b70ede88e19e6d8193198b/src/libs/fileDownload/DownloadUtils.ts#L34-L42

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

Create a new constant for checking if the url is a cloudfront url.
```
const isCloudFrontURL = url.includes(CONST.CLOUDFRONT_URL);
```

Update the condition to also include !isCloudFrontURL.

if (
    // We have two file download cases that we should allow: 1. downloading attachments 2. downloading Expensify package for Sage Intacct
    shouldOpenExternalLink ||
    (!isApiUrl && !isAttachmentUrl && !isSageUrl && !isCloudFrontURL)
) {

The cloud front server also needs to allow the app origin to avoid CORS issue, otherwise the fetch will fail and we will agin open the file in a new tab. https://github.com/Expensify/App/blob/7b4605a9bf642bdc54b70ede88e19e6d8193198b/src/libs/fileDownload/DownloadUtils.ts#L55-L70
What alternative solutions did you explore? (Optional)  ### Result

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

JmillsExpensify commented 2 weeks ago

We're deprecating the video, so I'm going to close this report.

Expensify / App