Fix api Payment.create to support overpayments

We've discussed this before - it's OK to add a payment to a fully paid contribution because ... life.

When this happens there should be no financial items linked to the over payment

CRM/Financial/BAO/Payment.php

@@ -38,8 +38,8 @@ class CRM_Financial_BAO_Payment {
    */
   public static function create($params) {
     $contribution = civicrm_api3('Contribution', 'getsingle', ['id' => $params['contribution_id']]);
-    $contributionStatus = CRM_Contribute_PseudoConstant::contributionStatus($contribution['contribution_status_id'], 'name');
-    $isPaymentCompletesContribution = self::isPaymentCompletesContribution($params['contribution_id'], $params['total_amount']);
+    $contributionStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $contribution['contribution_status_id']);
+    $isPaymentCompletesContribution = self::isPaymentCompletesContribution($params['contribution_id'], $params['total_amount'], $contributionStatus);
     $lineItems = self::getPayableLineItems($params);
 
     $whiteList = ['check_number', 'payment_processor_id', 'fee_amount', 'total_amount', 'contribution_id', 'net_amount', 'card_type_id', 'pan_truncation', 'trxn_result_code', 'payment_instrument_id', 'trxn_id', 'trxn_date'];
@@ -358,14 +358,18 @@ class CRM_Financial_BAO_Payment {
   }
 
   /**
-   * Does this payment complete the contribution
+   * Does this payment complete the contribution.
    *
    * @param int $contributionID
    * @param float $paymentAmount
+   * @param string $previousStatus
    *
    * @return bool
    */
-  protected static function isPaymentCompletesContribution($contributionID, $paymentAmount) {
+  protected static function isPaymentCompletesContribution($contributionID, $paymentAmount, $previousStatus) {
+    if ($previousStatus === 'Completed') {
+      return FALSE;
+    }
     $outstandingBalance = CRM_Contribute_BAO_Contribution::getContributionBalance($contributionID);
     $cmp = bccomp($paymentAmount, $outstandingBalance, 5);
     return ($cmp == 0 || $cmp == 1);

tests/phpunit/CiviTest/CiviUnitTestCase.php

@@ -3424,6 +3424,13 @@ AND    ( TABLE_NAME LIKE 'civicrm_value_%' )
    */
   protected function validatePayments($payments) {
     foreach ($payments as $payment) {
+      $balance = CRM_Contribute_BAO_Contribution::getContributionBalance($payment['contribution_id']);
+      if ($balance < 0 && $balance + $payment['total_amount'] === 0.0) {
+        // This is an overpayment situation. there are no financial items to allocate the overpayment.
+        // This is a pretty rough way at guessing which payment is the overpayment - but
+        // for the test suite it should be enough.
+        continue;
+      }
       $items = $this->callAPISuccess('EntityFinancialTrxn', 'get', [
         'financial_trxn_id' => $payment['id'],
         'entity_table' => 'civicrm_financial_item',

tests/phpunit/api/v3/PaymentTest.php

@@ -687,6 +687,21 @@ class api_v3_PaymentTest extends CiviUnitTestCase {
     $this->validateAllPayments();
   }
 
+  /**
+   * Test that a contribution can be overpaid with the payment api.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testCreatePaymentOverPay() {
+    $contributionID = $this->contributionCreate(['contact_id' => $this->individualCreate()]);
+    $payment = $this->callAPISuccess('Payment', 'create', ['total_amount' => 5, 'order_id' => $contributionID]);
+    $contribution = $this->callAPISuccessGetSingle('Contribution', ['id' => $contributionID]);
+    $this->assertEquals('Completed', $contribution['contribution_status']);
+    $this->callAPISuccessGetCount('EntityFinancialTrxn', ['financial_trxn_id' => $payment['id'], 'entity_table' => 'civicrm_financial_item'], 0);
+    $this->validateAllPayments();
+    $this->validateAllContributions();
+  }
+
   /**
    * Test create payment api for paylater contribution
    *