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

Request - IOU preview is showing as blank in Scan module and Distance Module #32593

Closed
4 of 6 tasks
lanitochka17 opened this issue Dec 6, 2023 · 21 comments
Closed
4 of 6 tasks
Assignees

Comments

@lanitochka17
Copy link

lanitochka17 commented Dec 6, 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!


Version Number: 1.4.9-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Access staging.new.expensify.com
  2. Sign into a valid account
  3. Click on + > Request > Scan > Add any receipt/photo > Submit the Request
  4. Tap on the IOU

Expected Result:

User expects to be able to see the IOU preview

Actual Result:

IOU preview shows as blank

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

Add any screenshot/video evidence

Bug6303688_1701898021571.Blank_IOU_in_Scan_flow.mp4

View all open jobs on GitHub

@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Dec 6, 2023
Copy link
Contributor

github-actions bot commented Dec 6, 2023

👋 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.

Copy link

melvin-bot bot commented Dec 6, 2023

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

@ZhenjaHorbach
Copy link
Contributor

Looks like a regression this PR
#13036

@blimpich
Copy link
Contributor

blimpich commented Dec 6, 2023

@ZhenjaHorbach Can we confirm that PR is the problem, and if so why it is causing this issue?

@ZhenjaHorbach
Copy link
Contributor

Screenshot 2023-12-07 at 00 03 48

https://github.com/Expensify/App/blob/deda6ae510f1e1ef1599c8c6ee45d91fe58660c5/src/components/Image/BaseImage.js#L6C4-L12

As far as I can tell, the problem is related to the fact that we cannot get the width and height of the image we need

@blimpich
Copy link
Contributor

blimpich commented Dec 6, 2023

Ah yeah I see, I'm currently testing locally if reverting that PR fixes the issue.

@ZhenjaHorbach
Copy link
Contributor

This will help)
I already checked)

@situchan
Copy link
Contributor

situchan commented Dec 6, 2023

Screenshot 2023-12-07 at 00 03 48 https://github.com/Expensify/App/blob/deda6ae510f1e1ef1599c8c6ee45d91fe58660c5/src/components/Image/BaseImage.js#L6C4-L12

As far as I can tell, the problem is related to the fact that we cannot get the width and height of the image we need

I got this but after npm install, issue gone

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Dec 6, 2023

Oh )
Yeah )
This really helps)
The necessary changes were in the patch))

@situchan
Copy link
Contributor

situchan commented Dec 6, 2023

cors

I am not able to reproduce on main but staging. Seems like CORS issue
cc: @kidroca

@blimpich
Copy link
Contributor

blimpich commented Dec 7, 2023

I also can't reproduce this locally. Not on main, and not when I switch to origin/staging (both 1.4.9-0 and 1.4.9-1). Can reproduce on staging.new.expensify.com. +1 to this being a CORS issue.

@blimpich
Copy link
Contributor

blimpich commented Dec 7, 2023

@situchan should we wait for @kidroca to respond or is there a path forward to unblocking the deploy? I'm a bit brain-fried and not sure how to unblock this.

@situchan
Copy link
Contributor

situchan commented Dec 7, 2023

I think we can wait for @kidroca until tomorrow. If it won't be fixed by deploy time, we can always do quick solution which is straight revert.

@kidroca
Copy link
Contributor

kidroca commented Dec 7, 2023

It seems the endpoint /receipts/{path} isn't configured for CORS the way the endpoint /chat-attachments/{path} was updated in the issue here:

I think whatever was done to make /chat-attachments/{path} comply with CORS should be done for the receipts route as well

@blimpich
Copy link
Contributor

blimpich commented Dec 7, 2023

Attempting to solve CORS issue server side right now. Might not be able to move quickly enough though to prevent reverting.

@blimpich
Copy link
Contributor

blimpich commented Dec 7, 2023

Making progress on the solution to this. Struggling with testing the solution locally to confirm I have the right solution.

@blimpich
Copy link
Contributor

blimpich commented Dec 7, 2023

Getting closer to a solution. Testing it locally is difficult.

@blimpich
Copy link
Contributor

blimpich commented Dec 7, 2023

I have a potential solution but I'm not able to successfully test it locally. I'm frustrated because this is definitely possible but I'm not able to move quickly enough to fix this. Going to revert to stop blocking the deploy. Sorry @kidroca. I will try to fix the underlying CORS issue so this can go through soon.

@blimpich
Copy link
Contributor

blimpich commented Dec 7, 2023

PR was reverted: #32700. I will work to get the CORS issue fixed and update this thread when I can.

@blimpich blimpich removed the DeployBlockerCash This issue or pull request should block deployment label Dec 8, 2023
@blimpich
Copy link
Contributor

blimpich commented Dec 8, 2023

Closing this issue since the PR was reverted. I will create a new issue for fixing the CORS and post the link to that issue here.

@blimpich
Copy link
Contributor

blimpich commented Dec 8, 2023

New issue linked above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants