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

[$250] WS - Profile - Share - Workspace QR code is not downloaded to the device #40080

Closed
2 of 6 tasks
izarutskaya opened this issue Apr 11, 2024 · 20 comments
Closed
2 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Reviewing Has a PR in review

Comments

@izarutskaya
Copy link

izarutskaya commented Apr 11, 2024

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.62-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4489045
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

  1. Open Expensify App on mobile device
  2. Login with any account
  3. Go to any workspace > Settings > Profile or User > Settings
  4. Click on the Share button
  5. Click on the download option
  6. Verify the QR code is downloaded to the device

Expected Result:

QR code is downloaded to the device

Actual Result:

Nothing happens. Workspace QR code is NOT downloaded to the device.

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

Bug6445227_1712791486535!Profile_trace_for_1.4.62-0.cpuprofile

Bug6445227_1712791486536.iOS-WS-QR-code-not-download.mp4

Bug6445227_1712791486543!App_Info_1.4.62-0.json
Bug6445227_1712791486538!logs-2024-04-10_23_13_01.025.txt

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~010fdd6488cdadd975
  • Upwork Job ID: 1778361122906787840
  • Last Price Increase: 2024-04-11
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 11, 2024
Copy link

melvin-bot bot commented Apr 11, 2024

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.

Copy link

melvin-bot bot commented Apr 11, 2024

Triggered auto assignment to @lakchote (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Apr 11, 2024
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@izarutskaya
Copy link
Author

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

@izarutskaya
Copy link
Author

We think this issue might be related to the #collect project.

@lakchote lakchote added the External Added to denote the issue can be worked on by a contributor label Apr 11, 2024
@melvin-bot melvin-bot bot changed the title WS - Profile - Share - Workspace QR code is not downloaded to the device [$250] WS - Profile - Share - Workspace QR code is not downloaded to the device Apr 11, 2024
@lakchote lakchote added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 11, 2024
Copy link

melvin-bot bot commented Apr 11, 2024

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

Copy link

melvin-bot bot commented Apr 11, 2024

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

@lakchote
Copy link
Contributor

lakchote commented Apr 11, 2024

Here is the error:
image

I thought it'd be a dependency update given the component's code for QRShareWithDownload hasn't changed but it doesn't seem like it.

I've tried to revert:

But that didn't fix it.

@s77rt
Copy link
Contributor

s77rt commented Apr 11, 2024

Regression from #13767. react-native-view-shot does not support new arch yet, there is this merged PR gre/react-native-view-shot#516 but no new version yet.

@s77rt
Copy link
Contributor

s77rt commented Apr 11, 2024

And it requires react-native 0.74.0 which is also not released yet

@WoLewicki
Copy link
Contributor

@s77rt did you maybe try to apply the changes from the PR to the App to see what is not yet present in RN 0.73.x? Ping me if you have problems with making it work and we will also take a look and try to make it work in App.

Copy link

melvin-bot bot commented Apr 11, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@lakchote
Copy link
Contributor

lakchote commented Apr 11, 2024

I've asked for other engineers input here.

Since this is not a critical issue as no one rely on it for critical purposes at the moment, I'm going to demote it from being a blocker.

I've posted about it in the Fabric tracking GH, along with the context of the problem.

@lakchote lakchote added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Apr 11, 2024
@Julesssss
Copy link
Contributor

I agree with not blocking on this. But to avoid confusion lets hide the download button while we're awaiting a proper fix.

@ShridharGoel
Copy link
Contributor

Proposal

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

QR code download using react-native-view-shot doesn't work after moving to the new arch.

What is the root cause of that problem?

The react-native-view-shot library has introduced the new architecture support in latest version, but we are still using 3.8.0. (gre/react-native-view-shot#516)

Also, these changes in react-native need to be included.

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

Add patch for the view-shot library with the changes for the new architecture or bump the version to 4.0.0-alpha.0.
Add patch for these changes in react-native.

@lakchote
Copy link
Contributor

Proposal

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

QR code download using react-native-view-shot doesn't work after moving to the new arch.

What is the root cause of that problem?

The react-native-view-shot library has introduced the new architecture support in latest version, but we are still using 3.8.0. (gre/react-native-view-shot#516)

Also, these changes in react-native need to be included.

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

Add patch for the view-shot library with the changes for the new architecture or bump the version to 4.0.0-alpha.0. Add patch for these changes in react-native.

I'm not a fan of patching since it's not a critical matter and the new version for react-native will fix this.

@s77rt thoughts?

@WoLewicki
Copy link
Contributor

I'm testing bumping the version of library and adding all the proper files to react-native as a patch as part of the efforts of follow-ups to this PR: #13767. I'll try to report quickly if it is a viable option.

@WoLewicki
Copy link
Contributor

From a quick look it seems to still not work correctly after bumping the version with only the one commit mentioned here: gre/react-native-view-shot#516 (comment) is not enough to make it work on Android. Also, iOS needs bump of version and probably multiple commits to start working again too. So I think we should leave till we bump the App to RN 0.74.

Copy link

melvin-bot bot commented Apr 19, 2024

@JmillsExpensify, @s77rt, @lakchote Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@s77rt
Copy link
Contributor

s77rt commented Apr 19, 2024

Oops didn't notice this on K2. Waiting for RN 0.74 seems best since the QR download button is a minor functionality. Let's close this issue since we have this tracking one #40110

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 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

7 participants