webform_civicrm icon indicating copy to clipboard operation
webform_civicrm copied to clipboard

Record line_item amounts without rounding.

Open agileware-fj opened this issue 4 years ago • 3 comments

Overview

Removes the rounding that is applied to line item amounts during the contribution processing. This fixes some tax round errors that have already been addressed in CiviCRM core by avoiding premature rounding.

Before

When a contribution is created from a webform submission, the line item total amount, unit price, and tax amount are rounded. This can result in a 1c difference between the line item amount and the contribution total per line item.

Example:

  • 1 x Membership with at $704.5454
  • 10% tax is added ($70.45454)
  • Contribution records this exactly, comes out to ~$775.00 after final rounding
  • Line item records $704.55 + $70.46† comes to ~$775.01
  • Contribution total and line item total do not match

† I think it's already rounded the total amount once, so I guess more work might be required here.

One place this causes issues is when you're recording a payment against a "pay later" contribution created in this way, it "Balanced owed" is the line item amount, not the contribution amount. If you record the contribution amount, the contribution is marked as "Partially paid."

After

Amounts are not rounded when creating the line item. As a result, the contribution amount and line item total / balance owed match. Actual rounding is deferred to CiviCRM to keep precision for as long as possible.

Technical Details

Removes the calls to CRM_Utils_Money::format entirely. These were only used for rounding purposes as far as I could tell.

Comments

Agileware ref CIVIWF-5

agileware-fj avatar Dec 17 '20 04:12 agileware-fj

@agileware-fj - thank you for this! I totally agree on the not rounding prematurely.

We're D8 first now - are you able to put together the PR for the 8.x branch? If you do I'm happy to add a test. We already are testing Contributions for D8 and D9 - but don't check line-items yet so I want to add that. After that we would backport (i.e. use this PR) to get it into the 7.x branch.

KarinG avatar Dec 17 '20 18:12 KarinG

@KarinG the same change applies to the to the 8.x-5.x branch without issue, so see #410 As noted there though, I don't actually have a D8 CiviCRM system to test on (and I'm a minimum of 3 weeks away from having time to set one up), so it's on the basis of "I don't see any reason this wouldn't work there, too".

agileware-fj avatar Dec 18 '20 00:12 agileware-fj

@colemanw - I've merged this for 8.x-5.x - it looks like there are more places where line items are being rounded - but at least this removes any rounding on our end.

KarinG avatar Dec 22 '20 20:12 KarinG