From: Eileen McNaughton <eileen@fuzion.co.nz>
Date: Wed, 24 Jun 2015 12:13:44 +0000 (+1200)
Subject: CRM-16417 towards not deleting contributions when a payment failes
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=06d062ce2e3c6507351c529acb3de6ecc6ca50e7;p=civicrm-core.git

CRM-16417 towards not deleting contributions when a payment failes
---

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index f40b117dd5..0aeae2a123 100644
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -870,13 +870,18 @@ INNER JOIN  civicrm_contact contact ON ( contact.id = civicrm_contribution.conta
    * Prior to CRM-16417 these were simply removed from the database but it has been agreed that seeing attempted
    * payments is important for forensic and outreach reasons.
    *
-   * This function updates the financial transaction records to failed.
-   *
-   * @todo in principle we also think it makes sense to add an activity - this part would be a second step as
-   * the first change is likely to go into the LTS.
+   * @param int $contributionID
+   * @param string $message
    */
-  public static function failPayment($contributionID, $message) {
-
+  public static function failPayment($contributionID, $contactID, $message) {
+    civicrm_api3('activity', 'create', array(
+      'activity_type_id' => 'Failed Payment',
+      'details' => $message,
+      'subject' => ts('Payment failed at payment processor'),
+      'source_record_id' => $contributionID,
+      'source_contact_id' => CRM_Core_Session::getLoggedInContactID() ? CRM_Core_Session::getLoggedInContactID() :
+        $contactID,
+    ));
   }
 
   /**
diff --git a/CRM/Contribute/BAO/Contribution/Utils.php b/CRM/Contribute/BAO/Contribution/Utils.php
index ebfa009ddd..6e5b6fc540 100644
--- a/CRM/Contribute/BAO/Contribution/Utils.php
+++ b/CRM/Contribute/BAO/Contribution/Utils.php
@@ -266,11 +266,26 @@ class CRM_Contribute_BAO_Contribution_Utils {
           $paymentParams['contributionRecurID'] = $contribution->contribution_recur_id;
         }
       }
-      if (is_object($payment)) {
-        $result = $payment->doDirectPayment($paymentParams);
-      }
-      else {
-        CRM_Core_Error::fatal($paymentObjError);
+      try {
+        $result = $payment->doPayment($paymentParams);
+      }
+      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']);
+        }
+
+        if ($component !== 'membership') {
+          // This is only called from contribution form.
+          // Not sure if there is any reason not to just throw an exception up to it.
+          $result['is_payment_failure'] = TRUE;
+          return $result;
+        }
+        $membershipResult[1] = $result;
       }
     }
 
@@ -278,26 +293,7 @@ class CRM_Contribute_BAO_Contribution_Utils {
       $membershipResult = array();
     }
 
-    if (is_a($result, 'CRM_Core_Error')) {
-      //make sure to cleanup db for recurring case.
-      //@todo this clean up has always been controversial as many orgs prefer to see failed transactions.
-      // most recent discussion has been that they should be retained and this could be altered
-      if (!empty($paymentParams['contributionID'])) {
-        CRM_Contribute_BAO_Contribution::deleteContribution($paymentParams['contributionID']);
-      }
-      if (!empty($paymentParams['contributionRecurID'])) {
-        CRM_Contribute_BAO_ContributionRecur::deleteRecurContribution($paymentParams['contributionRecurID']);
-      }
-
-      if ($component !== 'membership') {
-        CRM_Core_Error::displaySessionError($result);
-        CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contribute/transact',
-          "_qf_Main_display=true&qfKey={$form->_params['qfKey']}"
-        ));
-      }
-      $membershipResult[1] = $result;
-    }
-    elseif ($result || ($form->_amount == 0.0 && !$form->_params['is_pay_later'])) {
+    if ($result || ($form->_amount == 0.0 && !$form->_params['is_pay_later'])) {
       if ($result) {
         $form->_params = array_merge($form->_params, $result);
       }
diff --git a/CRM/Contribute/Form/Contribution.php b/CRM/Contribute/Form/Contribution.php
index 6bcb35fa5b..c65bc0c176 100644
--- a/CRM/Contribute/Form/Contribution.php
+++ b/CRM/Contribute/Form/Contribution.php
@@ -1264,7 +1264,7 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP
         }
       }
       catch (PaymentProcessorException $e) {
-        CRM_Contribute_BAO_Contribution::failPayment($contribution->id, $e->getMessage());
+        CRM_Contribute_BAO_Contribution::failPayment($contribution->id, $paymentParams['contactID'], $e->getMessage());
         throw new PaymentProcessorException($e->getMessage());
       }
     }
diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php
index ecce3a4a82..16a2dd912a 100644
--- a/CRM/Member/BAO/Membership.php
+++ b/CRM/Member/BAO/Membership.php
@@ -1136,7 +1136,7 @@ INNER JOIN  civicrm_membership_type type ON ( type.id = membership.membership_ty
   /**
    * Get all exportable fields.
    *
-   * @retun array return array of all exportable fields
+   * @return array return array of all exportable fields
    */
   public static function &exportableFields() {
     $expFieldMembership = CRM_Member_DAO_Membership::export();
@@ -1761,8 +1761,12 @@ WHERE  civicrm_membership.contact_id = civicrm_contact.id
    * @param CRM_Core_DAO $dao
    *   Membership object.
    *
-   * @return null|array
-   *   array of memberships if created
+   * @param bool $reset
+   *
+   * @return array|null
+   *   Membership details, if created.
+   *
+   * @throws \CRM_Core_Exception
    */
   public static function createRelatedMemberships(&$params, &$dao, $reset = FALSE) {
     static $relatedContactIds = array();
diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php
index a52018fe42..46a327e6eb 100644
--- a/CRM/Member/Form/Membership.php
+++ b/CRM/Member/Form/Membership.php
@@ -1555,22 +1555,23 @@ class CRM_Member_Form_Membership extends CRM_Member_Form {
 
       if ($params['total_amount'] > 0.0) {
         $payment = CRM_Core_Payment::singleton($this->_mode, $this->_paymentProcessor, $this);
-        $result = $payment->doDirectPayment($paymentParams);
-      }
-
-      if (is_a($result, 'CRM_Core_Error')) {
-        //make sure to cleanup db for recurring case.
-        if (!empty($paymentParams['contributionID'])) {
-          CRM_Contribute_BAO_Contribution::deleteContribution($paymentParams['contributionID']);
-        }
-        if (!empty($paymentParams['contributionRecurID'])) {
-          CRM_Contribute_BAO_ContributionRecur::deleteRecurContribution($paymentParams['contributionRecurID']);
+        try {
+          $result = $payment->doPayment($paymentParams);
         }
+        catch (PaymentProcessorException $e) {
+          if (!empty($paymentParams['contributionID'])) {
+            CRM_Contribute_BAO_Contribution::failPayment($paymentParams['contributionID'], $e->getMessage());
+          }
+          if (!empty($paymentParams['contributionRecurID'])) {
+            CRM_Contribute_BAO_ContributionRecur::deleteRecurContribution($paymentParams['contributionRecurID']);
+          }
 
-        CRM_Core_Error::displaySessionError($result);
-        CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contact/view/membership',
-          "reset=1&action=add&cid={$this->_contactID}&context=&mode={$this->_mode}"
-        ));
+          CRM_Core_Error::displaySessionError($result);
+          CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contact/view/membership',
+            "reset=1&action=add&cid={$this->_contactID}&context=&mode={$this->_mode}"
+          ));
+
+        }
       }
 
       if ($result) {