Prior to 4.3 CiviCRM did not really have a method for handling refunds but in 4.3 a change was made so that refunds are recorded in the financial_trxn table against the contribution they correspond to. Prior to 4.3 the civicrm_financial_trxn table was only really used for recording credit card payments but in 4.3 it was expanded to record all payments.
CiviCRM 4.2 behaviour
No real core behaviour for refunds. WMF added functionality to add a refund button in the UI and also a behaviour via scripts. The scripts
- Validated the contribution exists & is not already refunded
- Validates the refund does not exceed the original amount
- Updated the status of the original contribution to 'Refunded'
- Adds custom data to the new contribution: 'finance_only' => 1,
- Creates a new contribution with a financial type that reflects the refund type. Status is set to Completed for offline ones & potentially '"pending" or "Completed" for UI driven changes
- Copies custom data over to the new contribution
- Adds extra data to the Contribution Extra custom group - ie. 'parent_contribution_id' => $contribution->id, 'finance_only' => 1, 'no_thank_you' => 'refund', 'source_type' 'source_name' 'source_host' 'source_version' 'source_run_id' 'source_enqueued_time'
Data gaps on wmf flow
- Was the pending status being used for refunds done by Donor Services. None of the scripts appear to use this option.
- The checks potentially prevent an over-refund from being recorded but presumably unless the over-refund is a data entry error on the contribution-edit form it is the 'truth' even if it is a rort?
Assumption on WMF flow
The code handing the possibility of source = 'RFD' (last used in 2012) or total_amount=0 (described as a legacy protocol in the code comments) can be entirely disregarded as legacy.
CiviCRM 4.6 behaviour
CiviCRM tracks records in the civicrm_financial_trxn table the query is
SELECT * FROM civicrm_contribution c LEFT JOIN civicrm_entity_financial_trxn ef ON c.id = ef.entity_id AND ef.entity_table = 'civicrm_contribution' LEFT JOIN civicrm_financial_trxn f ON f.id = ef.financial_trxn_id LIMIT 2
There a reserved status of 'Refunded' with a special meaning and setting this status results in negative entries being created in the financial_trxn table. There is also a 'Pending Refund' status we can look at if Donor Services do use that concept. There is currently a bug in that flow that might need fixing.
The financial_trxn table is used for a bit more than just payments. It's also used for internal transfers - ie. if you move money from the 'Membership' account to the 'Donation' account it creates rows to reflect that. In 4.7 an extra column is_payment is added to disambiguate these. I think we might end up with the odd extra row in this table when doing data entry fixes but I don't think it will have much impact.
As part of this investigation I added the UI to be able to see the financial contributions (when looking at a contribution row you should see it + in the screen shots on this issue). It's worth noting this UI is now in 4.7 but is not on other 4.6 sites so if communicating with other CiviCRM users don't expect them to be familiar with it. Also it is because of this I noticed the oddities I mentioned in the previous paragraph & I expect others will start to notice & fix them once 4.7 goes out https://issues.civicrm.org/jira/browse/CRM-17678
4.6 Option 1 - Co-operate with CiviCRM
If we are going to respect CiviCRM's wishes for handling refunds in 4.6 I don't think we need any custom code on the UI side of things. Donor services would need to edit the contribution to have a status of 'Refunded'. That is currently very slow but there are 2 fixes now upstreamed for this & pending QA on our side https://gerrit.wikimedia.org/r/#/c/256241/ & https://gerrit.wikimedia.org/r/#/c/257271/
On the script side I believe we should fix our mark_refund function to call the CiviCRM api like this
civicrm_api3('Contribution', 'create', array( 'id' => x, 'contribution_status_id' => 'Refunded', 'trxn_id' => 'yhn', 'finance_only' => 1, 'no_thank_you' => 'refund', );
We would need to add unit tests to wmf code AND to core CiviCRM to ensure that the correct financial_trxn rows are created when that happens. We would probably also want some error handling but per my notes above I'm not that clear about the value of error handling if the data is coming in from scripts whereas it it's from the UI I suspect the form validation should & probably does take care of it.
- We lose the custom data that is specific to the refund - mostly forensic I think but I'm not really sure how important that data is.
- I am not sure if Core accurately creates the right rows when changing status to refund via the api & whether I'll need to do bug fixes and / or add tests there. I suspect it won't have correct handling for trxn_id for the refund at a guess.
- We would only have one financial type for both at the contribution level. Although the financial trxns have type as well it's less accessible .
- I'm sure we'll have a few more core bugs before we truly get to the end of this
- The Financial transactions have less visibility & the trxn_id of the refund is harder to search by (TBH I believe the contribution search should be fixed to search for that trxn_id rather than the one on the contribution table & the one on the contribution table should possibly be removed from 4.8).
- We can move responsibility to CiviCRM core & add testing so that future changes are predictable
- We are being good open source citizens
- We are not fighting the flow.
4.6 Option 2 Tell CiviCRM to stick it
This would basically mean creating a new UI button to create refunds and ignoring the financial transaction records forever more.
- If we go down this path we are kind of committed to stay on it because we would be letting our financial transaction data get dirtier & dirtier. Currently both the pre-upgrade data & the data since the upgrade need cleaning up but if we are going to do that we should do it now.
- We would be fighting the flow & in future might hit issues / have to manage things over upgrades.
- Less change
- Easier data access. If we use the 'old' approach the contributions have a 1-1 with payments which makes it a little more intuitive from contribution search etc. However, I have already gotten the gateway report to be accurate on the trxn data (https://gerrit.wikimedia.org/r/#/c/260320/)
These would need to be fixed in our DB & upstream and 'handled' when the next upgrade happens by editing the upgrade script. Starting in 4.7 CiviCRM offers more granular upgrades & pre-running upgrade actions may not require this.
- civicrm_entity_financial_trxn needs a combined index on entity_id + entity_table.
- civicm_financial_trxn needs an index on trxn_id - since no-one has done one yet we could make it non-unique & smatter it with comments about that! ( https://issues.civicrm.org/jira/browse/CRM-17752)
Here is how a contribution + financial transactions 'should' look after being refunded
When the upgrade script ran it converted completed, pending & cancelled transactions to have the financial_trxns populated - but it didn't look at the status of 'Refunded' so our upgraded ones look like
My proposal with the upgraded ones is that we add the missing financial_trxns but leave the contributions untouched.
I believe there are 16k~ of these
MariaDB [(none)]> SELECT count(*) FROM civicrm.civicrm_contribution WHERE contribution_status_id = 9; +----------+ | count(*) | +----------+ | 16382 |
New ones since the upgrade
When we upgraded CiviCRM added an additional upgrade status of 'Refunded' not 'realising' WMF already had one. The WMF code has been picking up this version rather than the earlier one. Some combination of the the WMF code + the double status + ?? maybe a bug?? means the ones that have been created since the upgrade have an extra positive financial_trxn.
There are 1186 contributions currently affected
I propose to
- remove the extra financial trxns for these rows
- update the contribution_status_id on civicrm-contribution & civicrm-financial_trxn
SET @originalRefundStatus = 9;
SET @duplicateRefundStatus = 10;
DELETE FROM civicrm.civicrm_financial_trxn WHERE status_id = @duplicateRefundStatus;
UPDATE civicrm_contribution SET contribution_status_id = @originalRefundStatus WHERE contribution_status_id = @duplicateRefundStatus;
- Remove the duplicate Refunded status (per https://gerrit.wikimedia.org/r/#/c/254364/)
Note that this will leave us with a contribution status that is Completed & one that is Refunded. The transaction records will be correct as at the $ totals. We could merge them - this would also apply to the historical ones. I'm on the fence about this but it could be a subsequent step.
On looking at Donor Services s/sheet I feel like CiviCRM should be providing that information in a usable way & we should probably work towards that. Some thoughts
- We could add gateway transaction id to quick search (the top left hand corner search box) or similar & bypass the find contributions screen.
- I think it makes sense to add back the Refund button rather than have to use Edit Contribution - but to do that in an way that we also upstream at the same time.
- It looks like there is an import csv file function - is that accessible to Michael.
- I feel like maybe a report like that spreadsheet makes sense
- I wonder it's it worth adding a little angular form with a box to search the transaction & then a button to mark refund if Donor Services do these in some sort of bulk