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

feat(details): round line item amount by 2 decimal points #4

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

p-kuen
Copy link
Contributor

@p-kuen p-kuen commented Jun 2, 2024

ref #3

@p-kuen p-kuen requested a review from d-kuen June 2, 2024 19:17
src/details.rs Outdated
@@ -84,7 +84,7 @@ impl<'a> DetailsItem<'a> {
None => Decimal::ZERO,
};

self.quantity * self.unit_price / base_quantity + reduction_and_surcharge_sum
(self.quantity * self.unit_price / base_quantity + reduction_and_surcharge_sum).round_dp(2)
Copy link
Contributor

Choose a reason for hiding this comment

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

round_dp uses Bankersrounding: 6.5 -> 6

We should use clone_with_scale here. It uses MidpointAwayFromZero rounding.

Also this may affect line_item_total_gross_amount results due to previous rounding.

I think we should round right when creating the TaxItem elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just changed it to use the round_dp_with_strategy to round with MidpointAwayFromZero strategy. I wanted to use this method instead of clone_with_scale because it is directly implemented by rust_decimal and does the same.
Regarding line_item_total_gross_amount I think it is totally fine to use rounded line items as we should always calculate with the values you see on the invoice in the end.

@p-kuen p-kuen changed the title tests(invoice): add test to show tax item rounding problem feat(details): round line item amount by 2 decimal points Jun 3, 2024
@p-kuen p-kuen requested a review from d-kuen June 3, 2024 05:28
@p-kuen p-kuen merged commit 9513f8a into main Jun 7, 2024
1 check passed
@p-kuen p-kuen deleted the p-kuen/issue3 branch June 7, 2024 15:03
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