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 2024-03-15] [$500] Expense - App crashes when changing date, description, merchant of the request #36857

Closed
2 of 6 tasks
izarutskaya opened this issue Feb 20, 2024 · 26 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

@izarutskaya
Copy link

izarutskaya commented Feb 20, 2024

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: v1.4.43-0
Reproducible in staging?: Y
Reproducible in production?: N
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation:

Action Performed:

  1. Launch New Expensify app.
  2. Go to workspace chat.
  3. Create a manual request.
  4. Go to request details page.
  5. Tap Date.
  6. Select a new date and save it.

Expected Result:

App does not crash.

Actual Result:

App crashes. This issue also occurs when changing description and merchant of the created request.

On web app, console error shows up.

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

Bug6385307_1708402578800.Screen_Recording_20240220_084451_New_Expensify__1_.mp4

Bug6385307_1708402696196!QA.txt

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016c45b637c55448e3
  • Upwork Job ID: 1759887389787447296
  • Last Price Increase: 2024-02-20
  • Automatic offers:
    • Pujan92 | Contributor | 0
Issue OwnerCurrent Issue Owner: @johncschuster
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 20, 2024
@melvin-bot melvin-bot bot changed the title Expense - App crashes when changing date, description, merchant of the request [$500] Expense - App crashes when changing date, description, merchant of the request Feb 20, 2024
Copy link

melvin-bot bot commented Feb 20, 2024

Job added to Upwork: https://www.upwork.com/jobs/~016c45b637c55448e3

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 20, 2024
Copy link

melvin-bot bot commented Feb 20, 2024

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

Copy link

melvin-bot bot commented Feb 20, 2024

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

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Feb 20, 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.

Copy link

melvin-bot bot commented Feb 20, 2024

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

@izarutskaya
Copy link
Author

We think that this bug might be related to #wave5-free-submitters
CC @dylanexpensify

@mountiny
Copy link
Contributor

Waiting for proposals

@Pujan92
Copy link
Contributor

Pujan92 commented Feb 20, 2024

Proposal

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

App crash on updating the request details

What is the root cause of that problem?

Seems the issue after this PR where we used modifiedCurrency for cachedTotal, modifiedCurrency won't be available when we aren't updating the currency. That leads to the error by passing an empty string to the convertToDisplayString function.

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

We need to update it to consider currency as a fallback value when modifiedCurrency isn't available here.

updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, updatedTransaction?.modifiedCurrency);

@paultsimura
Copy link
Contributor

According to @cead22, it should be fixed by #36821

Copy link

melvin-bot bot commented Feb 20, 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.

@Pujan92
Copy link
Contributor

Pujan92 commented Feb 20, 2024

Thanks @paultsimura , just replied in slack

@mountiny
Copy link
Contributor

@Pujan92 Can you raise a PR

I think we should do this:

We need to update it to consider currency as a fallback value when modifiedCurrency isn't available here.

The modifiedCurrency will only be defined if the user changed the currency, which is not always. We need to default to currency

@Pujan92
Copy link
Contributor

Pujan92 commented Feb 20, 2024

Yes @mountiny

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 20, 2024
@mountiny
Copy link
Contributor

Yeah I can handle the PR review on this one thank you @rushatgabhane

@mountiny
Copy link
Contributor

@Pujan92 what is your ETA for the PR?

@Pujan92
Copy link
Contributor

Pujan92 commented Feb 20, 2024

Raising a PR in 5 mins.

@Pujan92
Copy link
Contributor

Pujan92 commented Feb 20, 2024

@mountiny PR is ready for review!

@cead22 cead22 self-assigned this Feb 20, 2024
@rushatgabhane rushatgabhane removed their assignment Feb 20, 2024
@mountiny
Copy link
Contributor

Asking for a cp

@puneetlath puneetlath removed the DeployBlockerCash This issue or pull request should block deployment label Feb 29, 2024
@melvin-bot melvin-bot bot removed the Weekly KSv2 label Mar 14, 2024
Copy link

melvin-bot bot commented Mar 14, 2024

This issue has not been updated in over 15 days. @cead22, @johncschuster, @Pujan92, @mountiny eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@melvin-bot melvin-bot bot added the Monthly KSv2 label Mar 14, 2024
@tylerkaraszewski
Copy link
Contributor

This fix was CP'ed weeks ago, can this issue be closed?

@mountiny mountiny added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review Monthly KSv2 labels Mar 15, 2024
@mountiny
Copy link
Contributor

I think we need to pay @Pujan92 right?

@mountiny mountiny changed the title [$500] Expense - App crashes when changing date, description, merchant of the request [HOLD for payment 2024-03-15] [$500] Expense - App crashes when changing date, description, merchant of the request Mar 15, 2024
@melvin-bot melvin-bot bot added the Overdue label Mar 18, 2024
Copy link

melvin-bot bot commented Mar 18, 2024

@cead22, @johncschuster, @Pujan92, @mountiny Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@johncschuster
Copy link
Contributor

Issuing payment now!

@melvin-bot melvin-bot bot removed the Overdue label Mar 18, 2024
@johncschuster
Copy link
Contributor

Payment has been issued via Upwork! Closing!

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

9 participants