-
-
Notifications
You must be signed in to change notification settings - Fork 824
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
Set contribution status to refunded when it has been refunded #16148
Set contribution status to refunded when it has been refunded #16148
Conversation
(Standard links)
|
@mattwire this makes sense but we'd need a test |
I have tested the new commit by @mattwire and seems to be working as expected. |
@@ -149,6 +149,11 @@ public static function create($params) { | |||
elseif ($contributionStatus === 'Pending' && $params['total_amount'] > 0) { | |||
self::updateContributionStatus($contribution['id'], 'Partially Paid'); | |||
} | |||
elseif ($contributionStatus === 'Completed' && ((float) CRM_Core_BAO_FinancialTrxn::getTotalPayments($contribution['id'], TRUE) === 0.0)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be missing something, but I'm not able to find a case where $contributionStatus
is Completed while recording a payment via this function. It should be either Pending
or Pending Refund
based on amount being sent as positive or negative? @mattwire can you mention the steps on UI so I can replicate and test this change from my side?
I tried using event registration -> change selection but I don't see any improved behavior in the contribution status.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jitendrapurohit This only happens when using the API function Payment.create in refund mode - see eg. here: https://lab.civicrm.org/extensions/stripe/blob/master/CRM/Core/Payment/StripeIPN.php#L270
Basically pass the full amount of a completed contribution in as a negative amount.
Eg. if contribution = Completed AND amount = 12 THEN
DO Payment.Create (Amount = -12)
Before this PR contribution status remains "Completed" but is fully refunded. After this PR contribution status is "Refunded" because it has been fully refunded.
@jitendrapurohit Does that help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok - I've tested this and also added a UT for the same at #16255
Noting that when we refund a partial amount, the contribution status is still kept as Completed
. This is also covered in the unit test PR.
… paid and now payments add up to 0
9f708f1
to
b87ee4c
Compare
@mattwire go ahead & merge this once @jitendrapurohit's test run completes with a (correct) fail |
Overview
When a contribution is refunded via Payment.create, has been previously marked completed and the sum of all payments for that contribution add up to 0 it should show "Refunded".
Before
Contribution shows "Completed" when it has been fully refunded.
After
Contribution shows "Refunded" when it has been fully refunded.
Technical Details
The function
CRM_Financial_BAO_Payment::create
already handles changing the contribution status to various states but does nothing with "Refunded". For end users this is really confusing!Comments
See (sort of) https://lab.civicrm.org/dev/financial/issues/87