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 rounding inaccuracies with quantity #156

Merged
merged 2 commits into from
Nov 21, 2018

Conversation

mraerino
Copy link
Contributor

@mraerino mraerino commented Nov 2, 2018

- Summary

There was a problem when using a quantity higher than 1 because
quantities were applied after rounding, therefore multiplying
rounding inaccuracies which is not acceptable.

Currently the amounts output in line items does not reflect the more
accurate values. There needs to be an API change to display single
and total amounts for items in orders.

- Test plan

Adjusted a test to take the now accurate amounts.

- Description for the changelog

Fix increase of rounding inaccuracies through item quantity

@mraerino mraerino force-pushed the fix/calculation-rounding-amounts branch from d595c35 to 27b548f Compare November 14, 2018 23:44
@mraerino mraerino changed the title WIP: Fix rounding inaccuracies with quantity Fix rounding inaccuracies with quantity Nov 14, 2018
@mraerino
Copy link
Contributor Author

@bcomnes This is rebased on the master now and ready to go.

If you want to do api changes for expressing line item amounts properly before, we can postpone this. It is not based on an existing issue but I noticed this flaw when digging into the calculation logic.

There was a problem when using a quantity higher than 1 because
quantities were applied after rounding, therefore multiplying
rounding inaccuracies which is not acceptable.

Currently the amounts output in line items does not reflect the more
accurate values. There needs to be an API change to display single
and total amounts for items in orders.
@mraerino mraerino force-pushed the fix/calculation-rounding-amounts branch from 27b548f to 61a8e7b Compare November 15, 2018 11:08
Copy link
Contributor

@bcomnes bcomnes left a comment

Choose a reason for hiding this comment

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

Lets take it for now.

@bcomnes bcomnes merged commit 5e277bf into netlify:master Nov 21, 2018
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.

2 participants