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

Count refunds when calculating amount due for an invoice #16506

Merged
merged 1 commit into from
Feb 17, 2020

Conversation

mattwire
Copy link
Contributor

Overview

Refunds are not being counted. So if you do something like this:

Contribution amount = £9.30.

  1. Add payment for £9.30.
  2. Add payment for £10.
  3. Refund payment for £10.

You end up with an invoice that shows "Amount Due: £-10.00".

Before

If contribution payments include a refund, incorrect amount shown for "Amount Due":
image

After

If contribution payments include a refund, correct amount shown for "Amount Due":
image

Technical Details

Explained in overview - we need to count both "Completed" and "Refunded" as actual payments when doing the calculation.

Comments

@eileenmcnaughton @monishdeb

@civibot
Copy link

civibot bot commented Feb 11, 2020

(Standard links)

@civibot civibot bot added the master label Feb 11, 2020
@magnolia61
Copy link
Contributor

Nice catch!

@eileenmcnaughton
Copy link
Contributor

Change makes sense - @magnolia61 can you test?

@magnolia61
Copy link
Contributor

Change makes sense - @magnolia61 can you test?

yes I can do so tomorrow morning. Thanks for the continuing work on getting everything around payment create, partial payments, etcetera, work as planned and coded solid and sound :-)

I was just thinking (should I open a gitlab issue for this?) that "the amount due" would be a very very welcome contribution token. Currently 'the amount due' is only available in the event context (event balance token).

@eileenmcnaughton
Copy link
Contributor

@magnolia61 you can open a gitlab - I don't see it happening in a hurry but maybe down the track

@eileenmcnaughton
Copy link
Contributor

@mattwire doesn't all this code replicate

$paymentAmt = CRM_Contribute_BAO_Contribution::getContributionBalance($this->_contributionId);

@mattwire mattwire force-pushed the invoicedue_countrefunded branch from 8d32cc2 to 449b5e6 Compare February 15, 2020 19:30
@mattwire
Copy link
Contributor Author

@mattwire doesn't all this code replicate

$paymentAmt = CRM_Contribute_BAO_Contribution::getContributionBalance($this->_contributionId);

@eileenmcnaughton Well spotted - I've updated the PR and confirmed we can use those functions and remove a few more lines :-)

@eileenmcnaughton
Copy link
Contributor

Looks good to me - @magnolia61 if you can confirm this passes your testing we will merge

@magnolia61
Copy link
Contributor

Looks good to me - @magnolia61 if you can confirm this passes your testing we will merge

I am testing now Of course first I had to also patch with #16480

@magnolia61
Copy link
Contributor

I cannot fully replicate the BEFORE behavior.
When I create a contribution of 10.
Pay 10
Further pay 2,50
Refund 2,50
The following invoice shows:
Screenshot from 2020-02-17 11-28-29
With the following corresponding contribution/payment screen:
Screenshot from 2020-02-17 11-28-34

@mattwire
Copy link
Contributor Author

@magnolia61 Your invoice seems to be missing the "Less amount paid + Amount due" section. I created an invoice by Viewing Contribution -> Print Invoice. Do you have that in your Contributions - Invoice messagetemplate?

@magnolia61
Copy link
Contributor

magnolia61 commented Feb 17, 2020

Strange. It had to do with the status of pay_later. My contribution had pay_later=1 and therefor did not show the LESS amount paid section (I have no clue why that would seem logical).

Anyway, with pay_later manually changed to 0 I ended up with the invoice like in your "before"
Screenshot from 2020-02-17 16-45-21

And with this PR applied:
Screenshot from 2020-02-17 16-46-59

@mattwire
Copy link
Contributor Author

Thanks @magnolia61 for testing. Merging. @eileenmcnaughton may be worth looking at why pay_later is interfering with display of amount due/paid?

@mattwire mattwire merged commit 435a9dc into civicrm:master Feb 17, 2020
@mattwire mattwire deleted the invoicedue_countrefunded branch February 17, 2020 16:01
@eileenmcnaughton
Copy link
Contributor

yes - sigh

@magnolia61
Copy link
Contributor

magnolia61 commented Feb 17, 2020

I am working on the contribution invoice template. (see: #16569) So far I only did cosmetic / layout improvements but while I am at it I can take a look at the templates logic where it comes to te pay_later setting. I will report back here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants