Move processConfirm function from Utils file back to only form still using it
authoreileen <emcnaughton@wikimedia.org>
Mon, 14 Dec 2020 19:26:42 +0000 (08:26 +1300)
committereileen <emcnaughton@wikimedia.org>
Mon, 14 Dec 2020 19:26:42 +0000 (08:26 +1300)
This function is really part of the Confirm form. People thought sharing it would
be good once. They were wrong. More specifically sharing large blocks of
code that attempt to service many diferent needs is not helpful

CRM/Contribute/BAO/Contribution/Utils.php
CRM/Contribute/Form/Contribution/Confirm.php
tests/phpunit/CRM/Contribute/Form/Contribution/ConfirmTest.php

index af50415499f95453e200e256f74fe366076e0c74..a444ad2a8d4696f25404840877fe8669e4d27f7a 100644 (file)
  */
 class CRM_Contribute_BAO_Contribution_Utils {
 
-  /**
-   * Process payment after confirmation.
-   *
-   * @param CRM_Core_Form $form
-   *   Form object.
-   * @param array $paymentParams
-   *   Array with payment related key.
-   *   value pairs
-   * @param int $contactID
-   *   Contact id.
-   * @param int $financialTypeID
-   *   Financial type id.
-   * @param bool $isTest
-   * @param bool $isRecur
-   *
-   * @throws CRM_Core_Exception
-   * @throws Exception
-   * @return array
-   *   associated array
-   *
-   */
-  public static function processConfirm(
-    &$form,
-    &$paymentParams,
-    $contactID,
-    $financialTypeID,
-    $isTest,
-    $isRecur
-  ) {
-    CRM_Core_Payment_Form::mapParams($form->_bltID, $form->_params, $paymentParams, TRUE);
-    $isPaymentTransaction = self::isPaymentTransaction($form);
-
-    $financialType = new CRM_Financial_DAO_FinancialType();
-    $financialType->id = $financialTypeID;
-    $financialType->find(TRUE);
-    if ($financialType->is_deductible) {
-      $form->assign('is_deductible', TRUE);
-      $form->set('is_deductible', TRUE);
-    }
-
-    // add some financial type details to the params list
-    // if folks need to use it
-    //CRM-15297 deprecate contributionTypeID
-    $paymentParams['financial_type_id'] = $paymentParams['financialTypeID'] = $paymentParams['contributionTypeID'] = $financialType->id;
-    //CRM-15297 - contributionType is obsolete - pass financial type as well so people can deprecate it
-    $paymentParams['financialType_name'] = $paymentParams['contributionType_name'] = $form->_params['contributionType_name'] = $financialType->name;
-    //CRM-11456
-    $paymentParams['financialType_accounting_code'] = $paymentParams['contributionType_accounting_code'] = $form->_params['contributionType_accounting_code'] = CRM_Financial_BAO_FinancialAccount::getAccountingCode($financialTypeID);
-    $paymentParams['contributionPageID'] = $form->_params['contributionPageID'] = $form->_values['id'];
-    $paymentParams['contactID'] = $form->_params['contactID'] = $contactID;
-
-    //fix for CRM-16317
-    if (empty($form->_params['receive_date'])) {
-      $form->_params['receive_date'] = date('YmdHis');
-    }
-    if (!empty($form->_params['start_date'])) {
-      $form->_params['start_date'] = date('YmdHis');
-    }
-    $form->assign('receive_date',
-      CRM_Utils_Date::mysqlToIso($form->_params['receive_date'])
-    );
-
-    if (empty($form->_values['amount'])) {
-      // If the amount is not in _values[], set it
-      $form->_values['amount'] = $form->_params['amount'];
-    }
-
-    if (isset($paymentParams['contribution_source'])) {
-      $paymentParams['source'] = $paymentParams['contribution_source'];
-    }
-    if ($isPaymentTransaction) {
-      $contributionParams = [
-        'id' => $paymentParams['contribution_id'] ?? NULL,
-        'contact_id' => $contactID,
-        'is_test' => $isTest,
-        'source' => CRM_Utils_Array::value('source', $paymentParams, CRM_Utils_Array::value('description', $paymentParams)),
-      ];
-
-      // CRM-21200: Don't overwrite contribution details during 'Pay now' payment
-      if (empty($form->_params['contribution_id'])) {
-        $contributionParams['contribution_page_id'] = $form->_id;
-        $contributionParams['campaign_id'] = CRM_Utils_Array::value('campaign_id', $paymentParams, CRM_Utils_Array::value('campaign_id', $form->_values));
-      }
-      // In case of 'Pay now' payment, append the contribution source with new text 'Paid later via page ID: N.'
-      else {
-        // contribution.source only allows 255 characters so we are using ellipsify(...) to ensure it.
-        $contributionParams['source'] = CRM_Utils_String::ellipsify(
-          ts('Paid later via page ID: %1. %2', [
-            1 => $form->_id,
-            2 => $contributionParams['source'],
-          ]),
-          // eventually activity.description append price information to source text so keep it 220 to ensure string length doesn't exceed 255 characters.
-          220
-        );
-      }
-
-      if (isset($paymentParams['line_item'])) {
-        // @todo make sure this is consisently set at this point.
-        $contributionParams['line_item'] = $paymentParams['line_item'];
-      }
-      if (!empty($form->_paymentProcessor)) {
-        $contributionParams['payment_instrument_id'] = $paymentParams['payment_instrument_id'] = $form->_paymentProcessor['payment_instrument_id'];
-      }
-
-      // @todo this is the wrong place for this - it should be done as close to form submission
-      // as possible
-      $paymentParams['amount'] = CRM_Utils_Rule::cleanMoney($paymentParams['amount']);
-      $contribution = CRM_Contribute_Form_Contribution_Confirm::processFormContribution(
-        $form,
-        $paymentParams,
-        NULL,
-        $contributionParams,
-        $financialType,
-        TRUE,
-        $form->_bltID,
-        $isRecur
-      );
-
-      $paymentParams['item_name'] = $form->_params['description'];
-
-      $paymentParams['qfKey'] = empty($paymentParams['qfKey']) ? $form->controller->_key : $paymentParams['qfKey'];
-      if ($paymentParams['skipLineItem']) {
-        // We are not processing the line item here because we are processing a membership.
-        // Do not continue with contribution processing in this function.
-        return ['contribution' => $contribution];
-      }
-
-      $paymentParams['contributionID'] = $contribution->id;
-      $paymentParams['contributionPageID'] = $contribution->contribution_page_id;
-
-      if (!empty($form->_params['is_recur']) && $contribution->contribution_recur_id) {
-        $paymentParams['contributionRecurID'] = $contribution->contribution_recur_id;
-      }
-      if (isset($paymentParams['contribution_source'])) {
-        $form->_params['source'] = $paymentParams['contribution_source'];
-      }
-
-      // get the price set values for receipt.
-      if ($form->_priceSetId && $form->_lineItem) {
-        $form->_values['lineItem'] = $form->_lineItem;
-        $form->_values['priceSetID'] = $form->_priceSetId;
-      }
-
-      $form->_values['contribution_id'] = $contribution->id;
-      $form->_values['contribution_page_id'] = $contribution->contribution_page_id;
-
-      if (!empty($form->_paymentProcessor)) {
-        try {
-          $payment = Civi\Payment\System::singleton()->getByProcessor($form->_paymentProcessor);
-          if ($form->_contributeMode == 'notify') {
-            // We want to get rid of this & make it generic - eg. by making payment processing the last thing
-            // and always calling it first.
-            $form->postProcessHook();
-          }
-          $result = $payment->doPayment($paymentParams);
-          $form->_params = array_merge($form->_params, $result);
-          $form->assign('trxn_id', CRM_Utils_Array::value('trxn_id', $result));
-          if (!empty($result['trxn_id'])) {
-            $contribution->trxn_id = $result['trxn_id'];
-          }
-          if (!empty($result['payment_status_id'])) {
-            $contribution->payment_status_id = $result['payment_status_id'];
-          }
-          $result['contribution'] = $contribution;
-          if ($result['payment_status_id'] == CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Pending')
-            && $payment->isSendReceiptForPending()) {
-            CRM_Contribute_BAO_ContributionPage::sendMail($contactID,
-              $form->_values,
-              $contribution->is_test
-            );
-          }
-          return $result;
-        }
-        catch (\Civi\Payment\Exception\PaymentProcessorException $e) {
-          // Clean up DB as appropriate.
-          if (!empty($paymentParams['contributionID'])) {
-            CRM_Contribute_BAO_Contribution::failPayment($paymentParams['contributionID'],
-              $paymentParams['contactID'], $e->getMessage());
-          }
-          if (!empty($paymentParams['contributionRecurID'])) {
-            CRM_Contribute_BAO_ContributionRecur::deleteRecurContribution($paymentParams['contributionRecurID']);
-          }
-
-          $result['is_payment_failure'] = TRUE;
-          $result['error'] = $e;
-          return $result;
-        }
-      }
-    }
-
-    // Only pay later or unpaid should reach this point, although pay later likely does not & is handled via the
-    // manual processor, so it's unclear what this set is for and whether the following send ever fires.
-    $form->set('params', $form->_params);
-
-    if ($form->_params['amount'] == 0) {
-      // This is kind of a back-up for pay-later $0 transactions.
-      // In other flows they pick up the manual processor & get dealt with above (I
-      // think that might be better...).
-      return [
-        'payment_status_id' => 1,
-        'contribution' => $contribution,
-        'payment_processor_id' => 0,
-      ];
-    }
-
-    CRM_Contribute_BAO_ContributionPage::sendMail($contactID,
-      $form->_values,
-      $contribution->is_test
-    );
-  }
-
-  /**
-   * Is a payment being made.
-   *
-   * Note that setting is_monetary on the form is somewhat legacy and the behaviour around this setting is confusing. It would be preferable
-   * to look for the amount only (assuming this cannot refer to payment in goats or other non-monetary currency
-   * @param CRM_Core_Form $form
-   *
-   * @return bool
-   */
-  protected static function isPaymentTransaction($form) {
-    return $form->_amount >= 0.0;
-  }
-
   /**
    * Get the contribution details by month of the year.
    *
index 1d17d82837d6dd909f29ee2f9809add7d0ff564e..d4ec34375381d80bebc4b6f59c71705da64860b4 100644 (file)
@@ -1442,7 +1442,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
         CRM_Price_BAO_LineItem::getLineItemArray($membershipParams);
 
       }
-      $paymentResult = CRM_Contribute_BAO_Contribution_Utils::processConfirm(
+      $paymentResult = self::processConfirm(
         $form,
         $membershipParams,
         $contactID,
@@ -2298,7 +2298,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
         }
       }
 
-      $result = CRM_Contribute_BAO_Contribution_Utils::processConfirm($this, $paymentParams,
+      $result = self::processConfirm($this, $paymentParams,
         $contactID,
         $this->wrangleFinancialTypeID($this->_values['financial_type_id']),
         ($this->_mode == 'test') ? 1 : 0,
@@ -2503,4 +2503,228 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     ));
   }
 
+  /**
+   * Is a payment being made.
+   *
+   * Note that setting is_monetary on the form is somewhat legacy and the behaviour around this setting is confusing. It would be preferable
+   * to look for the amount only (assuming this cannot refer to payment in goats or other non-monetary currency
+   * @param CRM_Core_Form $form
+   *
+   * @return bool
+   */
+  protected static function isPaymentTransaction($form) {
+    return $form->_amount >= 0.0;
+  }
+
+  /**
+   * Process payment after confirmation.
+   *
+   * @param CRM_Core_Form $form
+   *   Form object.
+   * @param array $paymentParams
+   *   Array with payment related key.
+   *   value pairs
+   * @param int $contactID
+   *   Contact id.
+   * @param int $financialTypeID
+   *   Financial type id.
+   * @param bool $isTest
+   * @param bool $isRecur
+   *
+   * @throws CRM_Core_Exception
+   * @throws Exception
+   * @return array
+   *   associated array
+   *
+   */
+  public static function processConfirm(
+    &$form,
+    &$paymentParams,
+    $contactID,
+    $financialTypeID,
+    $isTest,
+    $isRecur
+  ) {
+    CRM_Core_Payment_Form::mapParams($form->_bltID, $form->_params, $paymentParams, TRUE);
+    $isPaymentTransaction = self::isPaymentTransaction($form);
+
+    $financialType = new CRM_Financial_DAO_FinancialType();
+    $financialType->id = $financialTypeID;
+    $financialType->find(TRUE);
+    if ($financialType->is_deductible) {
+      $form->assign('is_deductible', TRUE);
+      $form->set('is_deductible', TRUE);
+    }
+
+    // add some financial type details to the params list
+    // if folks need to use it
+    //CRM-15297 deprecate contributionTypeID
+    $paymentParams['financial_type_id'] = $paymentParams['financialTypeID'] = $paymentParams['contributionTypeID'] = $financialType->id;
+    //CRM-15297 - contributionType is obsolete - pass financial type as well so people can deprecate it
+    $paymentParams['financialType_name'] = $paymentParams['contributionType_name'] = $form->_params['contributionType_name'] = $financialType->name;
+    //CRM-11456
+    $paymentParams['financialType_accounting_code'] = $paymentParams['contributionType_accounting_code'] = $form->_params['contributionType_accounting_code'] = CRM_Financial_BAO_FinancialAccount::getAccountingCode($financialTypeID);
+    $paymentParams['contributionPageID'] = $form->_params['contributionPageID'] = $form->_values['id'];
+    $paymentParams['contactID'] = $form->_params['contactID'] = $contactID;
+
+    //fix for CRM-16317
+    if (empty($form->_params['receive_date'])) {
+      $form->_params['receive_date'] = date('YmdHis');
+    }
+    if (!empty($form->_params['start_date'])) {
+      $form->_params['start_date'] = date('YmdHis');
+    }
+    $form->assign('receive_date',
+      CRM_Utils_Date::mysqlToIso($form->_params['receive_date'])
+    );
+
+    if (empty($form->_values['amount'])) {
+      // If the amount is not in _values[], set it
+      $form->_values['amount'] = $form->_params['amount'];
+    }
+
+    if (isset($paymentParams['contribution_source'])) {
+      $paymentParams['source'] = $paymentParams['contribution_source'];
+    }
+    if ($isPaymentTransaction) {
+      $contributionParams = [
+        'id' => $paymentParams['contribution_id'] ?? NULL,
+        'contact_id' => $contactID,
+        'is_test' => $isTest,
+        'source' => CRM_Utils_Array::value('source', $paymentParams, CRM_Utils_Array::value('description', $paymentParams)),
+      ];
+
+      // CRM-21200: Don't overwrite contribution details during 'Pay now' payment
+      if (empty($form->_params['contribution_id'])) {
+        $contributionParams['contribution_page_id'] = $form->_id;
+        $contributionParams['campaign_id'] = CRM_Utils_Array::value('campaign_id', $paymentParams, CRM_Utils_Array::value('campaign_id', $form->_values));
+      }
+      // In case of 'Pay now' payment, append the contribution source with new text 'Paid later via page ID: N.'
+      else {
+        // contribution.source only allows 255 characters so we are using ellipsify(...) to ensure it.
+        $contributionParams['source'] = CRM_Utils_String::ellipsify(
+          ts('Paid later via page ID: %1. %2', [
+            1 => $form->_id,
+            2 => $contributionParams['source'],
+          ]),
+          // eventually activity.description append price information to source text so keep it 220 to ensure string length doesn't exceed 255 characters.
+          220
+        );
+      }
+
+      if (isset($paymentParams['line_item'])) {
+        // @todo make sure this is consisently set at this point.
+        $contributionParams['line_item'] = $paymentParams['line_item'];
+      }
+      if (!empty($form->_paymentProcessor)) {
+        $contributionParams['payment_instrument_id'] = $paymentParams['payment_instrument_id'] = $form->_paymentProcessor['payment_instrument_id'];
+      }
+
+      // @todo this is the wrong place for this - it should be done as close to form submission
+      // as possible
+      $paymentParams['amount'] = CRM_Utils_Rule::cleanMoney($paymentParams['amount']);
+      $contribution = CRM_Contribute_Form_Contribution_Confirm::processFormContribution(
+        $form,
+        $paymentParams,
+        NULL,
+        $contributionParams,
+        $financialType,
+        TRUE,
+        $form->_bltID,
+        $isRecur
+      );
+
+      $paymentParams['item_name'] = $form->_params['description'];
+
+      $paymentParams['qfKey'] = empty($paymentParams['qfKey']) ? $form->controller->_key : $paymentParams['qfKey'];
+      if ($paymentParams['skipLineItem']) {
+        // We are not processing the line item here because we are processing a membership.
+        // Do not continue with contribution processing in this function.
+        return ['contribution' => $contribution];
+      }
+
+      $paymentParams['contributionID'] = $contribution->id;
+      $paymentParams['contributionPageID'] = $contribution->contribution_page_id;
+
+      if (!empty($form->_params['is_recur']) && $contribution->contribution_recur_id) {
+        $paymentParams['contributionRecurID'] = $contribution->contribution_recur_id;
+      }
+      if (isset($paymentParams['contribution_source'])) {
+        $form->_params['source'] = $paymentParams['contribution_source'];
+      }
+
+      // get the price set values for receipt.
+      if ($form->_priceSetId && $form->_lineItem) {
+        $form->_values['lineItem'] = $form->_lineItem;
+        $form->_values['priceSetID'] = $form->_priceSetId;
+      }
+
+      $form->_values['contribution_id'] = $contribution->id;
+      $form->_values['contribution_page_id'] = $contribution->contribution_page_id;
+
+      if (!empty($form->_paymentProcessor)) {
+        try {
+          $payment = Civi\Payment\System::singleton()->getByProcessor($form->_paymentProcessor);
+          if ($form->_contributeMode == 'notify') {
+            // We want to get rid of this & make it generic - eg. by making payment processing the last thing
+            // and always calling it first.
+            $form->postProcessHook();
+          }
+          $result = $payment->doPayment($paymentParams);
+          $form->_params = array_merge($form->_params, $result);
+          $form->assign('trxn_id', CRM_Utils_Array::value('trxn_id', $result));
+          if (!empty($result['trxn_id'])) {
+            $contribution->trxn_id = $result['trxn_id'];
+          }
+          if (!empty($result['payment_status_id'])) {
+            $contribution->payment_status_id = $result['payment_status_id'];
+          }
+          $result['contribution'] = $contribution;
+          if ($result['payment_status_id'] == CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Pending')
+            && $payment->isSendReceiptForPending()) {
+            CRM_Contribute_BAO_ContributionPage::sendMail($contactID,
+              $form->_values,
+              $contribution->is_test
+            );
+          }
+          return $result;
+        }
+        catch (\Civi\Payment\Exception\PaymentProcessorException $e) {
+          // Clean up DB as appropriate.
+          if (!empty($paymentParams['contributionID'])) {
+            CRM_Contribute_BAO_Contribution::failPayment($paymentParams['contributionID'],
+              $paymentParams['contactID'], $e->getMessage());
+          }
+          if (!empty($paymentParams['contributionRecurID'])) {
+            CRM_Contribute_BAO_ContributionRecur::deleteRecurContribution($paymentParams['contributionRecurID']);
+          }
+
+          $result['is_payment_failure'] = TRUE;
+          $result['error'] = $e;
+          return $result;
+        }
+      }
+    }
+
+    // Only pay later or unpaid should reach this point, although pay later likely does not & is handled via the
+    // manual processor, so it's unclear what this set is for and whether the following send ever fires.
+    $form->set('params', $form->_params);
+
+    if ($form->_params['amount'] == 0) {
+      // This is kind of a back-up for pay-later $0 transactions.
+      // In other flows they pick up the manual processor & get dealt with above (I
+      // think that might be better...).
+      return [
+        'payment_status_id' => 1,
+        'contribution' => $contribution,
+        'payment_processor_id' => 0,
+      ];
+    }
+
+    CRM_Contribute_BAO_ContributionPage::sendMail($contactID,
+      $form->_values,
+      $contribution->is_test
+    );
+  }
+
 }
index c5ab5f7f175e3ad993fed6ad19dd3a6b4fb9bcf4..b802f33d7f84d9d5372d8bff35e8d2f246a8177a 100644 (file)
@@ -105,7 +105,7 @@ class CRM_Contribute_Form_Contribution_ConfirmTest extends CiviUnitTestCase {
       'skipLineItem' => 0,
     ];
 
-    $processConfirmResult = CRM_Contribute_BAO_Contribution_Utils::processConfirm($form,
+    $processConfirmResult = CRM_Contribute_Form_Contribution_Confirm::processConfirm($form,
       $form->_params,
       $contactID,
       $form->_values['financial_type_id'],
@@ -155,7 +155,7 @@ class CRM_Contribute_Form_Contribution_ConfirmTest extends CiviUnitTestCase {
       'relationship_type_id' => 5,
       'is_current_employer' => 1,
     ]);
-    CRM_Contribute_BAO_Contribution_Utils::processConfirm($form,
+    CRM_Contribute_Form_Contribution_Confirm::processConfirm($form,
       $form->_params,
       $form->_params['onbehalf_contact_id'],
       $form->_values['financial_type_id'],