Pending Pay Later /w Custom payment method

@monish.deb @jitendra thoughts?

Seems "to_financial_account" = Account Receivable is intentionally not considered while recording additional payments. Not sure why is the extra where clause inserted here @monish.deb @eileen

added needs:concept approval triaged type:request labels

OK - it's possible we should be blocking creating this type but will see what Monish & @JoeMurray say

That query was first introduced in codebase as a part of https://issues.civicrm.org/jira/browse/CRM-13964 PR implementation, which is correct when I correlated the financial data flow for an offline Completed contribution - https://wiki.civicrm.org/confluence/display/CRM/CiviAccounts+Data+Flow#CiviAccountsDataFlow-Offlinecompletedcontribution(%22basedataflow%22)

Which indicates that corresponding financial trxn(s) of partial/complete payment, should have to_financial_account set to 'Asset' account type

If payment_instrument_id IS NULL, retrieve default asset financial account (where financial_account_type_id is 'Asset' and is_default = 1

and the same criteria is used in this query to track the payment total, on basis of which it updates the contribution status.

In order to fix it, we should remove those FT from 'Record payment' form, which got default financial account set to 'Account receivable' for partial/pending payment.

What do you think?

hmm better get an answer from @JoeMurray - seems like this is mostly about how we let the accounts be configured

@studio136 can you provide the Asset type for the Accounts Receivable financial account that you created? It should be Asset. If you chose something else that may have led to problems.
On dmaster I created new Payment Method of Undetermined and set the financial account to Accounts Receivable. Then I registered demo contact using that payment method and status of Pending. I did not change the Event Registration Status from default value of Registered. Then I clicked Record Payment, and recorded the payment. The contribution status comes back incorrect as Partially Paid, though I was able to open the payments fieldset:

I then registered for a second event using undetermined payment method and pending status. I opened edit event registration, then from there clicked edit contribution. I changed status to completed, and clicked save. I did not change the Event Registration Status from default value of Registered. On the edit event reg form I then saw the following error:

I edited the contribution from 2, changed the status from partially pad to completed, and saved. Everything then looked fine:

I tried again by registering in 3rd event with undetermined payment method and status=pending, and tried changing registration status to Pending (Pay Later). The contribution has the correct status:

The registration has the correct status:

Rather than recording a payment (which is the best practice), I changed the contribution status to completed:

At this point everything looked correct and there are no record payment buttons available anywhere for this contribution (on view, edit or on contribution tab):

So, I think the correct workflow that is properly supported is for the backend user to make sure to change the Registration Status to Pending if they don't want to record a payment at time of registration. It appears that the code is interpreting a registration status of Completed and 'recording of payment' with contribution status of pending pay later to mean a partial payment has been made.

Looking at the bookkeeping report entries (use the time to correlate to the transactions above), the first registration seems to incorrectly have one entry with 1200 (accounts receivable) for both debit and credit account, instead of 1100 for debit like the others. I'm not sure what's going on that it would be different between the three approaches.

The Bookkeeping report showed all three contributions. If I removed the transaction status=Completed filter I got two transactions for each registration, as appropriate.

Note that the error I got on point 3 above (second registration scenario) could be the cause of the reported error above when trying to view payment.

Perhaps as an enhancement, maybe as an extension, users could be prevented from selecting a contribution status of pending pay later unless registration was set to pending pay later too.

@JoeMurray - Thanks for looking into this, the Asset type for the Accounts Receivable financial account is Asset (AR)

I think this is a bit more than a workflow problem as it affects Memberships as well.

On the last item you noted here 'Rather than recording a payment (which is the best practice), I changed the contribution status to completed', this will not work because we need to record actual payment method and Check/Transaction#

A little background: The client system in question here was originally setup as far as I can tell around 2011. They've been using custom Payment Methods since then without any issues (Pending, Comp, Cancelled), only since the most recent update to 5.1.2 (from 4.7.25) did issues arise. (Their Pending method is what we are referring to here as Undetermined).

The client was entering Pending Pay Later payments that originated on the front end of the site by users.

When doing Record Payment or Edit Contribution, they would set Status = Complete, Method = Check, upon save, the Contribution Status was set as Partially Paid, requiring client to go back into the Contribution record and set the Contribution status (again) to Complete. But... viewing the contribution at this point showed the following, notice the Received Into and the Payment Method.

I was able to replicate this by creating an event registration on the back end Checking Record Payment with Status = Pending(Pay Later), Method = Pending, with a Participant Status of Pending Pay Later. I realized that they had the custom Pending payment method set as the default. I changed this to Check, to try to resolve the issue with future online pay later contributions, because creating an event reg with Method = Check worked as expected.

The problem now is that they have many online event registrations in the system that were created with Method = Pending and still awaiting payment. These are going to end up with the same problem as shown in the image above.

For Memberships, they do back end Pay Later Memberships/Renewals in order to submit an invoice to the user for payment.

See the following screenshots from dmaster

Sorry, to boil it down, I was able to create entries properly by using a recommended workflow, and to mess things up by changing status to completed rather than Record Payment. I think you have something specific to your setup that relates to their custom setup and I'm not sure what you expect. Feel free to contact me at joe dot murray at jmaconsulting dot biz if you would like us to review and update the entries on your instance's database in some way.

closed

I wasn't able to find out the commit(s) between 4.7.25 and master which is responsible for this error. But I found out the cause of this issue, let me explain in steps:

When we do backoffice Pending Event registration with our custom Payment method (say 'Undetermined' here) which got linked Financial Account (FA) as 'Account Receivable' it creates a financial_trxn (FT) record with to_financial_account_id = AR, which is correct as per the workflow mention for pending contribution - https://wiki.civicrm.org/confluence/display/CRM/CiviAccounts+Data+Flow#CiviAccountsDataFlow-Pay-latercontributionreceived
Now when we do 'Record Payment', it is supposed to create another FT record with from_financial_account_id = AR and to_financial_account_id != AR (it should be 'Deposit Bank Account' if we are using Check/Cash payment method etc.) AND status = Completed. For later, it MUST NOT be AR which is applicable only for pending contribution. In this case, the issue arises when we submit the additional payment and CRM_Contribute_BAO_Contribution::recordAdditionalPayment() is responsible to record corresponding financial entries. What's strange is that this particular fn selectively chooses few financial parameters and ignores essentials like 'payment_instrument_id' (PI), see here. As a result of which it retrieves the linked FA of the previous PI - Undetermined, see here for to_financial_account_id which is incorrect. And eventually fails here

Proposal:

Consider the chosen PI, which will be later used for making financial entries. Fix would be:

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 6d8f3de..48da67b 100644
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -3853,6 +3853,9 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
     $params['skipLineItem'] = TRUE;
     $trxnsData['trxn_date'] = !empty($trxnsData['trxn_date']) ? $trxnsData['trxn_date'] : date('YmdHis');
     $arAccountId = CRM_Contribute_PseudoConstant::getRelationalFinancialAccount($contributionDAO->financial_type_id, 'Accounts Receivable Account is');
+    if (!empty($trxnsData['payment_instrument_id'])) {
+      $params['payment_instrument_id'] = $trxnsData['payment_instrument_id'];
+    }
 
     $completedStatusId = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed');
     $paidStatus = CRM_Core_PseudoConstant::getKey('CRM_Financial_DAO_FinancialItem', 'status_id', 'Paid');

Populate only those payment instrument which doesn't have linked Financial Account = AR and that change need to be made here
Add unit test.

Overall it would take me 4hr to submit a PR along with UT.

reopened

No effort or testing went into supporting payment methods which did not result in a payment when designing and implementing financial accounting changes. So I'm not surprised that issues are arising as we attempt to support payment instruments with 'payments' going into accounts receivable. Monish, could also check if the original purchase with Undetermined as payment method/instrument is ceating a civicrm_financial_trxn record with is_payment=True? Is there a hook somewhere that could fix that? Otherwise there will be issues with some of the reports. I'm not confident there is an inexpensive solution here.

@lcdweb and @studio136 : advice on next steps here?

To expand on point from #153 (comment 5925) the hook would be implemented to support an Undetermined Payment Method so that if the PM's Financial Account was the same as one used elsewhere in system with account relationship is 'Accounts Receivable' then set is_payment=FALSE. But the impact of this would need to be tested.

added Concept approved and removed needs:concept approval labels

closed

changed milestone to %5.9

mentioned in commit e1c5a855

mentioned in commit 6cc8f149

mentioned in commit 39110928

Pending Pay Later /w Custom payment method

Child items ...

Activity