diff --git a/CRM/Financial/BAO/Payment.php b/CRM/Financial/BAO/Payment.php index a76426354f15..fcbc2229fb5b 100644 --- a/CRM/Financial/BAO/Payment.php +++ b/CRM/Financial/BAO/Payment.php @@ -105,10 +105,18 @@ public static function create($params) { if ($value['allocation'] === (float) 0) { continue; } + + if (!empty($ftIds[$value['price_field_value_id']])) { + $financialItemID = $ftIds[$value['price_field_value_id']]; + } + else { + $financialItemID = self::getNewFinancialItemID($value, $params['trxn_date'], $contribution['contact_id'], $paymentTrxnParams['currency']); + } + $eftParams = [ 'entity_table' => 'civicrm_financial_item', 'financial_trxn_id' => $trxn->id, - 'entity_id' => $ftIds[$value['price_field_value_id']], + 'entity_id' => $financialItemID, 'amount' => $value['allocation'], ]; @@ -413,6 +421,7 @@ private static function updateContributionStatus(int $contributionID, string $st * @param $params * * @return array + * @throws \CiviCRM_API3_Exception */ protected static function getPayableLineItems($params): array { $lineItems = CRM_Price_BAO_LineItem::getLineItemsByContributionID($params['contribution_id']); @@ -433,6 +442,8 @@ protected static function getPayableLineItems($params): array { $ratio = 0; } foreach ($lineItems as $lineItemID => $lineItem) { + // Ideally id would be set deeper but for now just add in here. + $lineItems[$lineItemID]['id'] = $lineItemID; $lineItems[$lineItemID]['paid'] = self::getAmountOfLineItemPaid($lineItemID); $lineItems[$lineItemID]['balance'] = $lineItem['subTotal'] - $lineItems[$lineItemID]['paid']; @@ -503,4 +514,49 @@ protected static function reverseAllocationsFromPreviousPayment($params, $trxnID } } + /** + * Create a financial items & return the ID. + * + * Ideally this will never be called. + * + * However, I hit a scenario in testing where 'something' had created a pending payment with + * no financial items and that would result in a fatal error without handling here. I failed + * to replicate & am not investigating via a new test methodology + * https://github.com/civicrm/civicrm-core/pull/15706 + * + * After this is in I will do more digging & once I feel confident new instances are not being + * created I will add deprecation notices into this function with a view to removing. + * + * However, I think we want to add it in 5.20 as there is a risk of users experiencing an error + * if there is incorrect data & we need time to ensure that what I hit was not a 'thing. + * (it might be the demo site data is a bit flawed & that was the issue). + * + * @param array $lineItem + * @param string $trxn_date + * @param int $contactID + * @param string $currency + * + * @return int + * + * @throws \CiviCRM_API3_Exception + */ + protected static function getNewFinancialItemID($lineItem, $trxn_date, $contactID, $currency): int { + $financialAccount = CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship( + $lineItem['financial_type_id'], + 'Income Account Is' + ); + $itemParams = [ + 'transaction_date' => $trxn_date, + 'contact_id' => $contactID, + 'currency' => $currency, + 'amount' => $lineItem['line_total'], + 'description' => $lineItem['label'], + 'status_id' => 'Unpaid', + 'financial_account_id' => $financialAccount, + 'entity_table' => 'civicrm_line_item', + 'entity_id' => $lineItem['id'], + ]; + return (int) civicrm_api3('FinancialItem', 'create', $itemParams)['id']; + } + } diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index 0983069bbf89..95ae9cfdf8d9 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -2146,6 +2146,8 @@ public function testCheckTaxAmount($thousandSeparator) { /** * Test repeat contribution successfully creates line item. + * + * @throws \CRM_Core_Exception */ public function testRepeatTransaction() { $originalContribution = $this->setUpRepeatTransaction($recurParams = [], 'single'); diff --git a/tests/phpunit/api/v3/PaymentTest.php b/tests/phpunit/api/v3/PaymentTest.php index 76410ad8d3c5..30065f064be0 100644 --- a/tests/phpunit/api/v3/PaymentTest.php +++ b/tests/phpunit/api/v3/PaymentTest.php @@ -309,6 +309,29 @@ public function testCreatePaymentPendingOrderNoLineItems() { $this->validateAllPayments(); } + /** + * Test that Payment.create does not fail if the line items are missing. + * + * In the original spec it was anticipated that financial items would not be created + * for pending contributions in some circumstances. We've backed away from this and + * I mostly could not find a way to do it through the UI. But I did seem to once & + * I want to be sure that if they ARE missing no fatal occurs so this tests + * that in an artificial way. + * + * @throws \CRM_Core_Exception + */ + public function testAddPaymentMissingFinancialItems() { + $contribution = $this->callAPISuccess('Contribution', 'create', [ + 'total_amount' => 50, + 'financial_type_id' => 'Donation', + 'contact_id' => $this->individualCreate(), + 'contribution_status_id' => 'Pending', + ]); + CRM_Core_DAO::executeQuery('DELETE FROM civicrm_financial_item'); + $this->callAPISuccess('Payment', 'create', ['contribution_id' => $contribution['id'], 'payment_instrument_id' => 'Check', 'total_amount' => 5]); + $this->validateAllPayments(); + } + /** * Add participant with contribution *