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

[HOLD for payment 2023-10-13] [HOLD for payment 2023-10-12] [$250] Distance - App crashes when tapping on the receipt thumbnail while receipt is generating #28814

Closed
2 of 6 tasks
lanitochka17 opened this issue Oct 4, 2023 · 34 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production 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

Comments

@lanitochka17
Copy link

lanitochka17 commented Oct 4, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Launch New Expensify app
  2. Go to + > Request money > Distance > Create a distance request
  3. Go to distance request details page
  4. Tap on the receipt thumbnail before the map image is generated

Expected Result:

App does not crash

Actual Result:

App crashes

Workaround:

Unknown

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: 1.3.77-2

Reproducible in staging?: Yes

Reproducible in production?: No

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6224575_1696421736016.Screen_Recording_20231004_034841_New_Expensify.mp4

log.txt

Production

Screen_Recording_20231004_214101_New.Expensify.1.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01812bc33ae4579c27
  • Upwork Job ID: 1709571574970191872
  • Last Price Increase: 2023-10-04
  • Automatic offers:
    • paultsimura | Contributor | 27024406
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 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

@OSBotify
Copy link
Contributor

OSBotify commented Oct 4, 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.

@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

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

@mountiny mountiny added the External Added to denote the issue can be worked on by a contributor label Oct 4, 2023
@melvin-bot melvin-bot bot changed the title Distance - App crashes when tapping on the receipt thumbnail while receipt is generating [$500] Distance - App crashes when tapping on the receipt thumbnail while receipt is generating Oct 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01812bc33ae4579c27

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

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

@mountiny mountiny changed the title [$500] Distance - App crashes when tapping on the receipt thumbnail while receipt is generating [$250] Distance - App crashes when tapping on the receipt thumbnail while receipt is generating Oct 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

Upwork job price has been updated to $250

@paultsimura
Copy link
Contributor

paultsimura commented Oct 4, 2023

Proposal

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

When the Distance request image is pending, opening the receipt image crashes the App.
Note: this is easier to reproduce if we simply switch to offline mode.

What is the root cause of that problem?

On native, the pending attachment uses a static image, and it has the source: 1 (that's a standard RN, see this), which is not a string so it breaks the Str.isPDF() and Str.isImage() checks.

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

In AttachmentView, we need to make a couple of safety checks:

+ if ((_.isString(source) && Str.isPDF(source)) || (file && Str.isPDF(file.name || translate('attachmentView.unknownFilename')))) {
- if (Str.isPDF(source) || (file && Str.isPDF(file.name || translate('attachmentView.unknownFilename')))) {
+    const isImage = typeof source === 'number' || Str.isImage(source);
-    const isImage = Str.isImage(source);

This will be better fixed in this PR: #27204
But as a blocker fix, it should be good enough.

Result:

distance-offline.mp4

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 4, 2023
@mountiny mountiny assigned mountiny and unassigned iwiznia Oct 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

📣 @thesahindia Please request via NewDot manual requests for the Reviewer role ($250)

@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

📣 @paultsimura 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@mountiny
Copy link
Contributor

mountiny commented Oct 4, 2023

Thanks @paultsimura can you raise a PR and make it ready for review please?

@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2023

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

@sonialiap
Copy link
Contributor

I'm OOO Oct 16-23. Adding leave buddy

Status: waiting on checklist completion and possibly need to submit a regression test request

@melvin-bot melvin-bot bot removed the Overdue label Oct 17, 2023
@Expensify Expensify deleted a comment from melvin-bot bot Oct 17, 2023
@rushatgabhane
Copy link
Member

created a manual request - https://staging.new.expensify.com/r/4428559888912375

@rushatgabhane
Copy link
Member

rushatgabhane commented Oct 17, 2023

  1. The PR that introduced the bug has been identified. Link to the PR: Show eReceipts for distance requests #27204

  2. The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/27204/files#r1383171883

  3. A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N.A.

  4. Determine if we should create a regression test for this bug. No

  5. If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again -N.A.

@JmillsExpensify
Copy link

$375 payment approved for @rushatgabhane based on BZ summary.

@sonialiap
Copy link
Contributor

back from OOO, @rushatgabhane how's that checklist coming along? 😁

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 23, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 26, 2023

@paultsimura, @sonialiap, @rushatgabhane, @mountiny Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@mountiny
Copy link
Contributor

@rushatgabhane bump

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 28, 2023
@sonialiap
Copy link
Contributor

@rushatgabhane bump

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 31, 2023
@sonialiap
Copy link
Contributor

I think he might be OOO 🤔 there was a post about a resort in bali today

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Nov 3, 2023
@sonialiap
Copy link
Contributor

@rushatgabhane bump, please complete so that we can close this out #28814 (comment)

@rushatgabhane
Copy link
Member

@sonialiap completed the checklist 😅

@sonialiap
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production 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
Projects
None yet
Development

No branches or pull requests

10 participants