-
Notifications
You must be signed in to change notification settings - Fork 3k
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 2025-01-28] [HOLD for payment 2025-01-24] [$250] Reports - App crashes after cancelling payment on expenses with violation #55288
Comments
Triggered auto assignment to @adelekennedy ( |
Triggered auto assignment to @rlinoz ( |
💬 A slack conversation has been started in #expensify-open-source |
👋 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:
|
@adelekennedy 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. |
From the error, I think this line from this PR is the problem: cc: @luacmartins in case you know an easy fix |
I found a fix, will open a PR shortly |
Thanks for investigating and finding a fix! |
ProposalPlease re-state the problem that we are trying to solve in this issue.App crashes. What is the root cause of that problem?When we pay an expense, the violation is cleared but in the snapshot, we don't cover this case to store the value as an empty array. We only cover the case What changes do you think we should make in order to solve the problem?Here we should also update the snap value to the
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?What alternative solutions did you explore? (Optional)Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. |
|
@rlinoz What do you think about my proposal? |
I believe it is the same error as I reported here #54847 (comment). And the Onyx fix didn't fix it properly. @luacmartins @rlinoz there is a proposal above. What do you think about it> |
Job added to Upwork: https://www.upwork.com/jobs/~021879581827856936306 |
This is fixed in staging. Leading the issue open to process payment to @nkdengineer since they worked on a fix. Additionally, this was also a regression from #54847, which had myself as the author and @parasharrajat as reviewer. I believe no further payments are needed for us. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.86-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2025-01-24. 🎊 For reference, here are some details about the assignees on this issue:
|
@parasharrajat @adelekennedy @parasharrajat The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
I wouldn't consider this a regression from the #54847 given the following reasons.
What do you think? |
I think it was a regression. The bug didn't exist before that PR and existed after it, even though the issue came from a higher level lib, I should have taken that into account. That's just my opinion though. |
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.87-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2025-01-28. 🎊 For reference, here are some details about the assignees on this issue:
|
@parasharrajat @adelekennedy @parasharrajat The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
@rlinoz, @luacmartins, @parasharrajat, @adelekennedy, @nkdengineer Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Payment due tomorrow |
Contributor: @nkdengineer paid $250 via Upwork |
@rlinoz, @luacmartins, @parasharrajat, @adelekennedy, @nkdengineer Eep! 4 days overdue now. Issues have feelings too... |
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: 9.0.86-0
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): Mac 15.0 / Chrome
Issue reported by: Applause Internal Team
Device used: [email protected]
App Component: Search
Action Performed:
Precondition:
Expected Result:
App will not crash.
Actual Result:
App crashes.
Workaround:
Unknown
Platforms:
Screenshots/Videos
bug.mp4
bug.txt
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @adelekennedyThe text was updated successfully, but these errors were encountered: