Details
-
Type:
Bug
-
Status: In Progress
-
Priority:
Major
-
Resolution: Unresolved
-
Affects Version/s: 4.7.28
-
Fix Version/s: None
-
Component/s: CiviContribute
-
Labels:
-
Versioning Impact:Patch (backwards-compatible bug fixes)
-
Documentation Required?:None
-
Funding Source:Needs Funding
-
Verified?:No
Description
A bit similar to https://issues.civicrm.org/jira/browse/CRM-20259 but it can be reproduced from the UI without any API call.
Steps to reproduce:
- Enable Invoice and Tax
- Create a new financial account for taxes and add it to one financial type
- Create a price set of type Contribution with multiple taxable price option (using the financial type above)
- Create a new contribution from the backend and choose the new price set : choose several options -> save (the total and taxes seems ok)
- Edit this contribution without changing anything -> save
- All the taxes on the line items are equal to the contribution tax amount total (and i guess the financial tables are potentially wrong the same way)
Attachments
Issue Links
- supplements
-
CRM-20259 Update to Contribution with multiple priceset via api updates the total amount and tax amount
-
- Open
-
Activity
Samuel Vanhove can you test whether this works better with the line item editor extension. If so we'll freeze this change & encourage use of that extension
Eileen McNaughton Just a quick update, we indeed use an extension that solves the problem but it's https://github.com/coopsymbiotic/biz.jmaconsulting.cdntaxcalculator
line item editor extension might solves this but i don't think so because in that use case we are not editing line items.
In any case, if i understand your point, you'd rather have it solves in an extension than in core ? Is it because of the risk to regression ?
Samuel Vanhove - In some cases where we miscalculate things where the total is edited we have started to freeze the total field & suggest people use the line item editor. (it's also possible to edit single line items) and yes, that is largely because there is a lot of complexity in that code & some risk associated & I would say it is finally fairly stable.
My preference is to freeze editing the total amount field when there is more than one line item & tell people to install line item editor if they wish to alter it.
That said, if this is replicable in core with no extensions in use as you describe it sounds of concern and we have reached a point where our test cover is pretty good & the code is safer to touch.
Monish Deb can you take a look at this & give your opinion? Note - we would need a test to merge and I don't want to put the responsibility of writing that test on you
We'd prefer to focus on line item editor at this point for this kind of stuff for reasons you outline.
Ok, i understand you concerns. We might want to display a warning regarding enabling invoicing and taxes in core or add a documentation somewhere that it is strongly recommend to use [line item editor extension|https://civicrm.org/extensions/line-item-editor] but i guess this bug report is already some kind of minimal documentation.
Samuel Vanhove I would certainly look favourably on a PR that improved in app warnings and / or froze fields in favour of line item editor
First draft of a patch -> https://github.com/civicrm/civicrm-core/compare/master...samuelsov:CRM-21588#diff-7a5b0e2d131dabc49178460fc63328c0
We might need a unit test before doing the PR.