CIVIMM-296: Fix Total Amount Calculation

[shahrukh-compuco]

Overview

Currently the calculation for total amount is done in a way that we take the contribution total that has been calculated by civi and add the each line item's tax to it assuming that the contribution total from civi does not include tax but for some strange reason civi core calculates the total in a strange manner i.e as can be seen here civi removes the tax from contribution only if its related to a participant or membership otherwise the total amount already has the tax in it thus when we add line item tax for a participant or membership contribution it doubles up the vat.

This PR fixes the issue by calculating the total based solely on line items that is sum of each line item's price + each line item's vat.

Before

After

Technical Details

This PR is partially included in https://lab.civicrm.org/extensions/lineitemedit/-/merge_requests/76 but it looks different from core PR because the core PR was created some time back but it did not get merged and our code already had some part of the core PR that was merged here #6

[jamienovick]

@shahrukh-compuco I see this is a PR to our line item extension fork. Should we also submit it to the extension maintainer?

[shahrukh-compuco]

@shahrukh-compuco I see this is a PR to our line item extension fork. Should we also submit it to the extension maintainer?

@jamienovick I dont think we submit the prs for this extension back to the original maintainer as the original repo is on civicrm's gitlab

[jamienovick]

Why can't we raise a PR to gitlab? @shahrukh-compuco

[shahrukh-compuco]

Why can't we raise a PR to gitlab? @shahrukh-compuco

because I dont think we have write access to civicrm's gitlab @erawat correct me if I am wrong

[erawat]

Why can't we raise a PR to gitlab? @shahrukh-compuco

@jamienovick Just checked this, we have started maintain our own fork, version 2.5-patches since 2023. There have been a lot of patches since.

We would have to review this, with Compuclient 5.x or 6.x and start applying the fixes back to the core extension.

please see some histories here.

#1
#5

@shahrukh-compuco The fixes should go to the core extension, first.

Can you check if there is a fix in the core as we may need to update the core from the core to our fork, or we should get rid of this fork. See how it was done in the past.

Please also see the histories of the reason we forked it.

#1
#5

[erawat]

Comment was added here

#20 (comment)

[shahrukh-compuco]

Why can't we raise a PR to gitlab? @shahrukh-compuco

@jamienovick Just checked this, we have started maintain our own fork, version 2.5-patches since 2023. There have been a lot of patches since.

We would have to review this, with Compuclient 5.x or 6.x and start applying the fixes back to the core extension.

please see some histories here.

#1 #5

@shahrukh-compuco The fixes should go to the core extension, first.

Can you check if there is a fix in the core as we may need to update the core from the core to our fork, or we should get rid of this fork. See how it was done in the past.

Please also see the histories of the reason we forked it.

#1 #5

@erawat @jamienovick I checked that this issue is not there in original extension and it was introduced while working on https://compucorp.atlassian.net/browse/BTHAB-190 in this commit eed6cbb but the issue described in https://compucorp.atlassian.net/browse/BTHAB-190 does exist in original extension so I think we will have to patch the original extension with both the prs

[erawat]

Why can't we raise a PR to gitlab? @shahrukh-compuco

@jamienovick Just checked this, we have started maintain our own fork, version 2.5-patches since 2023. There have been a lot of patches since.

We would have to review this, with Compuclient 5.x or 6.x and start applying the fixes back to the core extension.
please see some histories here.
#1 #5
@shahrukh-compuco The fixes should go to the core extension, first.
Can you check if there is a fix in the core as we may need to update the core from the core to our fork, or we should get rid of this fork. See how it was done in the past.
Please also see the histories of the reason we forked it.
#1 #5

@erawat @jamienovick I checked that this issue is not there in original extension and it was introduced while working on https://compucorp.atlassian.net/browse/BTHAB-190 in this commit eed6cbb but the issue described in https://compucorp.atlassian.net/browse/BTHAB-190 does exist in original extension so I think we will have to patch the original extension with both the prs

Yes, can you patch the original repo with our PRs, and perhaps update the patches. This process should be similar to civicrm-core.