Details
-
Type:
Bug
-
Status: Done/Fixed
-
Priority:
Critical
-
Resolution: Fixed/Completed
-
Affects Version/s: 4.6.22, 4.7.12
-
Fix Version/s: 4.7.19
-
Component/s: CiviContribute, CiviMember
-
Labels:None
-
Versioning Impact:Patch (backwards-compatible bug fixes)
-
Documentation Required?:None
-
Funding Source:Needs Funding
Description
On the heels of CRM-17417 we've had a couple of clients complaining about tax issues relating to CiviCRM memberships recently.
We've tested issues on 4.7.12 as follows - all tests are using a single tax rate of 10%:
- Using a contribution page to create an "online" membership seems to work okay - a new membership is created with the correct amount and subsequent offline renewals also work.
- Adding a membership using the "Add Membership" button and a price set results in an initial contribution that has the tax rate applied twice (e.g. $50 excluding tax becomes $60.50, not $55 as it should); subsequent renewals of this membership have the correct amount. (Appears fixed in master as of Feb 26)
- Doing the same without a price set appears to work.
- Adding the membership with a contribution in Pending state results in a Contribution that when edited exhibits multiple incorrect behaviours - on the contribution edit screen the subtotals have double the tax added (as in 20% instead of 10%), however the contribution total still shows the correct amount - attached edit_pending_membership_contribution.png demonstrates. Saving the contribution results in the total having the tax cumulatively re-applied to the post-tax amount (total is 121% of the tax exclusive amount).
- (Added 2016-12-13): When adding or renewing a membership through the admin interface ("Add Membership" button or "Renew" on a search result), if the amount is changed, the tax amount is applied from the original amount display instead of being calculated from the new amount - although this is not evident in the interface; it will show a contribution with the correct amount, but if you look at the line items via the API you'll see incorrect tax amounts.
Agileware reference 23973
Attachments
Issue Links
- is supplemented by
-
-
- Done/Fixed
-
-
CRM-19585 Sales tax issues
-
- Done/Fixed
-
- supplements
-
-
- Done/Fixed
-
-
-
- Done/Fixed
-
-
-
- Done/Fixed
-
-
-
- Won't Do
-
Activity
Jamie Novick does Compucorp want to take a stab at this?
Rough estimate would be 3 - 5 hours.
Agileware tracking ref 23973
Agileware Do you want core team to start work on this?
Added the remaining test case, FYI
Agileware Do you want core team to start work on this issue?
This seems to be a reversion of https://issues.civicrm.org/jira/browse/CRM-16228
JMA is working on this in consultation with KarinG under CRM-16228.
Agileware , I was unable to replicate Adding a membership using the "Add Membership" button and a price set results in an initial contribution that has the tax rate applied twice (e.g. $50 excluding tax becomes $60.50, not $55 as it should); subsequent renewals of this membership have the correct amount. on http://dmaster.demo.civicrm.org/
Steps tried to replicate:
Create Financial Account "Sales Tax" with tax rate as 10% and assigned it to Member Dues Financial Type
Create price set with a radio html type and used Student Membership type
On Add Membership form i selected Student Radio Button from the price set option. The amount was calculated correctly as $55.
Am i missing anything?
Pradeep Nayak you missed a crucial step in your testing, you did not enable: CiviContribute Component Settings: Tax and Invoicing
With the CiviContribute Component Settings: Tax and Invoicing enabled you can see the various problems which are still present in CiviCRM 4.7.17.
Attached are screenshots which highlight the issue and here's some notes for each step:
- Step-1-Contribution-OK-with-Tax-and-Invoicing-Disabled - Appears to be working correctly, as you also noted
- Step-2-Enable-Tax-and-Invoicing - So let's enable CiviContribute Component Settings: Tax and Invoicing and see what happens next
- Step-3-Contribution-NOT-OK-with-Tax-and-Invoicing-Enabled - Wow, the same contribution from the previous screenshot is now different. Why would that be? That's really odd.
- Total amount is now $60. Should be $55.
- Step-4-Contribution-Edit-NOT-OK-with-Tax-and-Invoicing-Enabled - Editing the same contribution shows the same results. Why?
- Total amount is now $60. Should be $55.
- Step-5-Contribution-Status-Change-Pending-Completed-NOT-OK-with-Tax-and-Invoicing-Enabled - Payment has been received for this contribution, so let's edit it and change the contribution status from Pending to Completed and save. Looking at the contribution listing, we see a new total and different amounts again. Why?
- Total amount is now $60.50. Should be $55.
Let us know if you still cannot reproduce this problem, happy to help out.
Pradeep, could you look at this Tuesday? Thx.
Linking related issues.
> I had tested above all issues with CiviContribute Component Settings: Tax and Invoicing - Enabled.
Good to hear, wasn't detailed in your steps. When I checked dmaster shortly after your posting it Tax and Invoicing was not enabled. I could see the other configuration you had applied.
It is still interesting to note tax is still calculated and included in the total even with the Tax and Invoicing option disabled. As per my previous note and step 1 screenshot. This could be yet another bug.
Without Tax and Invoicing enabled, I would have expected no tax calculations at all to be performed. But hey, these days I expect the unexpected.
Yeah, that true. The code doesn't check for setting value i.e if CiviContribute Component Settings: Tax and Invoicing - Enabled. It applies tax if it has Account relationship 'Sales Tax Account is' defined for Financial type. 
Eileen McNaughton KarinG We are proposing to fix this with changes to CRM_Contribute_BAO_Contribution::checkTaxAmount() and a unit test, but no major refactoring. Are you in agreement with that approach?
With respect to Pradeep's comment of 05/Mar/17 the correct approach it seems to me would be to allow edits of any transactions that have sales taxes even when the Component Setting has been turned off for taxes. And when creating new contributions, event regs, memberships when it is disabled taxes should be ignored even if financial accounts for taxes have been set up for relevant financial types. Let's make a separate issue for that.
Joe Murray - "We are proposing to fix this" -> what is "this" exactly? We must be chipping away at sales tax issues one at a time - le'ts get in #9948 - and then see how that affects the other issues. Else we're going to run into conflicts / waste time.
Yes, there are a variety of tax issues. I think it is best to have CRM issues for them, and to have PRs against those. This issue has been supplemented by two others and supplements 3 closed issues. I'm comfortable if you would prefer to refactor the existing issues. In one case we have changed a bug to an epic and are creating issues in epic for each PR so we can better manage all of the processes. I don't think it is a good idea to only work in PRs.
"We are proposing to fix this with changes to CRM_Contribute_BAO_Contribution::checkTaxAmount() and a unit test, but no major refactoring. Are you in agreement with that approach?"
Yes, that is a good place to fix things. The way the function is called with the $isLineItem param is a bit of a bad code smell, but it is increasingly well tested.
Pradeep Nayak as a normal priority, please put this on your list for core pro bono contributions.
Agileware this is one of those tickets that has become unreadable because there are multiple issues in it. Can we close this out & get specific issues raised for the remaining issue/s
I think there have been some issues fixed but some issues / feature requests are outstanding
Oops, forgot the mention. Eileen McNaughton
Just going to tag Joe Murray into this as Joe and his team have been doing work on similar stuff here. Marc Brazeau Also Marc here as Marc has recently been looking into the Add membership and price set stuff.