frappe icon indicating copy to clipboard operation
frappe copied to clipboard

fix: money in words

Open barredterra opened this issue 1 year ago • 2 comments

[!NOTE] Please wait for #27178 to reduce the diff. Until then, please look at the last commit (06fd6f6) only.

Money in words does not work correctly for fractions that are not a factor of ten (10, 100, 100). For example, some currencies have five fraction units constituting one main unit, i.e. $ 1 == 5 cents and $ 0.2 == 1 cent.

This PR solves this by:

  • Using the fraction_units from the Currency definition instead of guessing them from the number format's decimal places.
  • Calculating fraction * fraction_units instead of just treating the fraction as an integer.
  • Adding a test case for this scenario.

Todo:

  • [ ] check if we can remove Number Format from Currency (rely on the fraction units only) (related bug: #26584)?

barredterra avatar Jul 22 '24 20:07 barredterra

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

stale[bot] avatar Aug 07 '24 07:08 stale[bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

stale[bot] avatar Aug 28 '24 02:08 stale[bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

stale[bot] avatar Sep 11 '24 13:09 stale[bot]

We can safely ignore the Semgrep failure in this case.

barredterra avatar Sep 21 '24 14:09 barredterra