Details
-
Type: Bug
-
Status: Done/Fixed
-
Priority: Trivial
-
Resolution: Fixed/Completed
-
Affects Version/s: 4.7
-
Fix Version/s: 4.7
-
Component/s: None
-
Labels:
-
Documentation Required?:None
-
Funding Source:Contributed Code
Description
Currently when editing a contribution to be refunded you get the option to enter a cancel date & a cancel reason (good) but not to enter the transaction ID for the transaction (bad). The refund transaction ID may differ to the original payment ID and is recorded against the financial_trxn record in the DB
I propose to
1) add a field to the contribution refund form
refund_trxn_id
This field would display along with cancel_date & cancel_reason when 'Refunded' is selected as a contribution status and the same field would be supported in the BAO & tested via api support such that
contribution.create array('id' => x, 'contribution_status_id' => 'Refunded', 'refund_trxn_id' => y')
would result in the contribution trxn_id being unchanged but the trxn_id of the refund being set to y.
In terms of loading defaults - if the status is not yet refunded, load the trxn_id value. If it is, then load the value from the (latest ) refund transaction id. At this stage latest=only - when that changes the form will need greater re-work.
The BAO code should continue to use trxn_id for both the contribution AND the refund financial trxn IF the refund_trxn_id is not set. If it is set (even if empty) then it should use that value for the refund transaction. I don't believe it should be possible to empty an existing trxn_id from a financial_trxn record via the contribution api - that role can belong to more CRUD type calls. However, an empty refund_trxn_id should prevent the trxn_id (if set) from being treated as the id for the financial_trxn record created.
Note that I did investigate switching to the payment api to support this - per CRM-16259. My main goal being to have a way for both form edits & api calls to set the transaction id for the refund in a way that is future proofed using a unit test.
However, I had strong reservations about that code being ready for this purpose. I did look at the code, provide some code review feedback & propose a fix for the test it was failing on - but the gap currently seemed too great for me to jump in & start adding the tests I needed + do the backport + update the UI to support switching to the payment api from the form layer.
OTOH the contribution.create api is already solidly tested from the point of view of updating status to refunded with appropriate financial transactions. There is already a framework for me to extend the tests for this use case.
In the longer term the payment api could & should be used much more but I didn't think it should have to be a prerequisite for setting the trxn_id for refunds correctly.
I will backport to our 4.6 rather than core.
Attachments
Issue Links
- is supplemented by
-
CRM-16259 Create Payment API
- Done/Fixed
- links to