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

Fix: App crashes when split payment total is small #4229

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

davecreaser
Copy link
Contributor

@davecreaser davecreaser commented Feb 3, 2025

Description

  • Fix an issue with the app crashing when the split payment type is too small.

  • Took a while to figure out the actual root cause of this, since the crash was caused by an invalid input to BigNumber, but I had to actually trace it back about 6 steps to find out why it was ending up in that format.

  • Long story short, when the user changes the percentage of a payout in a split payment, there is a function run to calculate the amount. When this amount is set, we need to use the toFixed instead of toString method from decimal.js, because otherwise when the amount is small it will be in scientific notation, which down the line (after trying to move the decimal point and convert to wei and then pass to BigNumber) causes the crash 😥

  • Small side note: I also fixed some awkward padding / styling for scientific notation amounts by wrapping the amounts in span tags in the completed action.

Testing

  • Open the Split Payment form
  • Set the Total Amount to 0.000001 (1*10 pow -6) or smaller
  • Change the Percentage Value of a recipient to anything less than 100%
  • The app shouldn't crash 🤞
Screenshot 2025-02-03 at 12 00 59
  • Just to be sure, you could also try if you wish to create a few other split payments just to ensure nothing broke

Diffs

Changes 🏗

  • use toFixed instead of toString when calculating split payment amounts

Resolves #4031

@davecreaser davecreaser requested a review from a team as a code owner February 3, 2025 12:04
@davecreaser davecreaser self-assigned this Feb 3, 2025
Copy link
Contributor

@rumzledz rumzledz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must have been quite the (pesky) debug journey @davecreaser 😂 I can confirm that the app no longer crashes when the percentage value of a member is less than 100% ✅

split-no-crash

Copy link
Contributor

@iamsamgibbs iamsamgibbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job pinning down this issue, this split payment form is a bit of a pain to work your way through!

Screenshot 2025-02-04 at 11 11 34

Screenshot 2025-02-04 at 11 12 41

All working nicely 🎉

Copy link
Member

@rdig rdig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix!

@davecreaser davecreaser merged commit 3a64cb0 into master Feb 5, 2025
2 checks passed
@davecreaser davecreaser deleted the fix/#4031-small-split-payment-amount branch February 5, 2025 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Split payment] App crashes when split payment total amount is set to a number 1x10 pow -6 and less
4 participants