CRM-16692 follow-up fix, move assumptions back to calling-form

diff --git a/CRM/Batch/Form/Entry.php b/CRM/Batch/Form/Entry.php
index 7ea3a69658dfb46fb25f97bb556d16011cb37441..bd38ce5283cdcb4d9f96cdae04113814687e04a2 100755 (executable)
--- a/CRM/Batch/Form/Entry.php
+++ b/CRM/Batch/Form/Entry.php
@@ -807,6 +807,8 @@ class CRM_Batch_Form_Entry extends CRM_Core_Form {
 
           // make contribution entry
           $contrbutionParams = array_merge($value, array('membership_id' => $membership->id));
+          // @todo - calling this from here is pretty hacky since it is called from membership.create anyway
+          // This form should set the correct params & not call this fn directly.
           CRM_Member_BAO_Membership::recordMembershipContribution($contrbutionParams);
         }
         else {

diff --git a/CRM/Contribute/BAO/Contribution/Utils.php b/CRM/Contribute/BAO/Contribution/Utils.php
index 91d48abea35d267af878ea1d83521304a610b076..ff5e327e8ee8bb4903dbe1f119a6820a7b895795 100644 (file)
--- a/CRM/Contribute/BAO/Contribution/Utils.php
+++ b/CRM/Contribute/BAO/Contribution/Utils.php
@@ -103,17 +103,26 @@ class CRM_Contribute_BAO_Contribution_Utils {
         }
       }
 
+      $contributionParams = array(
+        'contact_id' => $contactID,
+        'line_item' => $lineItems,
+        'is_test' => $isTest,
+        'campaign_id' => CRM_Utils_Array::value('campaign_id', $paymentParams, CRM_Utils_Array::value('campaign_id', $form->_values)),
+        'contribution_page_id' => $form->_id,
+        'source' => CRM_Utils_Array::value('source', $paymentParams, CRM_Utils_Array::value('description', $paymentParams)),
+      );
+      $isMonetary = !empty($form->_values['is_monetary']);
+      if ($isMonetary) {
+        if (empty($paymentParams['is_pay_later'])) {
+          // @todo look up payment_instrument_id on payment processor table.
+          $contributionParams['payment_instrument_id'] = 1;
+        }
+      }
       $contribution = CRM_Contribute_Form_Contribution_Confirm::processFormContribution(
         $form,
         $paymentParams,
         NULL,
-        array(
-          'contact_id' => $contactID,
-          'line_item' => $lineItems,
-          'is_test' => $isTest,
-          'campaign_id' => CRM_Utils_Array::value('campaign_id', $paymentParams, CRM_Utils_Array::value('campaign_id', $form->_values)),
-          'contribution_page_id' => $form->_id,
-        ),
+        $contributionParams,
         $financialType,
         TRUE,
         TRUE,

diff --git a/CRM/Contribute/Form/Contribution.php b/CRM/Contribute/Form/Contribution.php
index af6d55ea3328e18c228f6fad5627cb06e4baf1f6..64c6fa81dac0d2acae03a28cef7ab9eb142b71dd 100644 (file)
--- a/CRM/Contribute/Form/Contribution.php
+++ b/CRM/Contribute/Form/Contribution.php
@@ -1197,17 +1197,25 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP
       $this->assign('is_deductible', TRUE);
       $this->set('is_deductible', TRUE);
     }
+    $contributionParams = array(
+      'contact_id' => $contactID,
+      'line_item' => $lineItem,
+      'is_test' => $isTest,
+      'campaign_id' => CRM_Utils_Array::value('campaign_id', $this->_params),
+      'contribution_page_id' => CRM_Utils_Array::value('contribution_page_id', $this->_params),
+      'source' => CRM_Utils_Array::value('source', $paymentParams, CRM_Utils_Array::value('description', $paymentParams)),
+      'thankyou_date' => CRM_Utils_Array::value('thankyou_date', $this->_params),
+    );
+
+    if (empty($paymentParams['is_pay_later'])) {
+      // @todo look up payment_instrument_id on payment processor table.
+      $contributionParams['payment_instrument_id'] = 1;
+    }
 
     $contribution = CRM_Contribute_Form_Contribution_Confirm::processFormContribution($this,
       $this->_params,
       NULL,
-      array(
-        'contact_id' => $contactID,
-        'line_item' => $lineItem,
-        'is_test' => $isTest,
-        'campaign_id' => CRM_Utils_Array::value('campaign_id', $this->_params),
-        'contribution_page_id' => CRM_Utils_Array::value('contribution_page_id', $this->_params),
-      ),
+      $contributionParams,
       $financialType,
       TRUE,
       FALSE,

diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php
index 0c3a943a44f42fecd981676d9e20e55d7dc0e08a..32a5dc2df1902741829d28a3da9de9cf2ef9e1de 100644 (file)
--- a/CRM/Contribute/Form/Contribution/Confirm.php
+++ b/CRM/Contribute/Form/Contribution/Confirm.php
@@ -56,9 +56,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
    *
    * @param array $params
    * @param int $financialTypeID
-   * @param bool $online
    * @param float $nonDeductibleAmount
-   * @param bool $isMonetary
    * @param bool $pending
    * @param array $paymentProcessorOutcome
    * @param string $receiptDate
@@ -67,7 +65,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
    * @return array
    */
   public static function getContributionParams(
-    $params, $financialTypeID, $online, $nonDeductibleAmount, $isMonetary, $pending,
+    $params, $financialTypeID, $nonDeductibleAmount, $pending,
     $paymentProcessorOutcome, $receiptDate, $recurringContributionID) {
     $contributionParams = array(
       'financial_type_id' => $financialTypeID,
@@ -78,7 +76,6 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
       'amount_level' => CRM_Utils_Array::value('amount_level', $params),
       'invoice_id' => $params['invoiceID'],
       'currency' => $params['currencyID'],
-      'source' => (!$online || !empty($params['source'])) ? CRM_Utils_Array::value('source', $params) : CRM_Utils_Array::value('description', $params),
       'is_pay_later' => CRM_Utils_Array::value('is_pay_later', $params, 0),
       //configure cancel reason, cancel date and thankyou date
       //from 'contribution' type profile if included
@@ -88,15 +85,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
       //setting to make available to hook - although seems wrong to set on form for BAO hook availability
       'skipLineItem' => CRM_Utils_Array::value('skipLineItem', $params, 0),
     );
-    if (!$online && isset($params['thankyou_date'])) {
-      $contributionParam['thankyou_date'] = $params['thankyou_date'];
-    }
-    if (!$online || $isMonetary) {
-      if (empty($params['is_pay_later'])) {
-        // @todo look up payment_instrument_id on payment processor table.
-        $contributionParams['payment_instrument_id'] = 1;
-      }
-    }
+
     if ($paymentProcessorOutcome) {
       $contributionParams['payment_processor'] = CRM_Utils_Array::value('payment_processor', $paymentProcessorOutcome);
     }
@@ -809,9 +798,22 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
    * @param array $result
    * @param array $contributionParams
    *   Parameters to be passed to contribution create action.
+   *   This differs from params in that we are currently adding params to it and 1) ensuring they are being
+   *   passed consistently & 2) documenting them here.
+   *   - contact_id
+   *   - line_item
+   *   - is_test
+   *   - campaign_id
+   *   - contribution_page_id
+   *   - source
+   *   - payment_type_id
+   *   - thankyou_date (not all forms will set this)
+   *
    * @param CRM_Financial_DAO_FinancialType $financialType
    * @param bool $pending
+   *   The intention is this should always be TRUE and we are refactoring towards any completion happening elsewhere.
    * @param bool $online
+   *   Is the form a front end form? If so set a bunch of unpredictable things that should be passed in from the form.
    *
    * @param int $billingLocationID
    *   ID of billing location type.
@@ -832,7 +834,6 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     $transaction = new CRM_Core_Transaction();
     $contactID = $contributionParams['contact_id'];
 
-    $isMonetary = !empty($form->_values['is_monetary']);
     $isEmailReceipt = !empty($form->_values['is_email_receipt']);
     // How do these vary from params? These are currently passed to
     // - custom data function....
@@ -850,7 +851,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     // add these values for the recurringContrib function ,CRM-10188
     $params['financial_type_id'] = $financialType->id;
 
-    $contribParams['address_id'] = CRM_Contribute_BAO_Contribution::createAddress($params, $billingLocationID);
+    $contributionParams['address_id'] = CRM_Contribute_BAO_Contribution::createAddress($params, $billingLocationID);
 
     //@todo - this is being set from the form to resolve CRM-10188 - an
     // eNotice caused by it not being set @ the front end
@@ -891,7 +892,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
 
     if (isset($params['amount'])) {
       $contributionParams = array_merge(self::getContributionParams(
-        $params, $financialType->id, $online, $nonDeductibleAmount, $isMonetary, $pending,
+        $params, $financialType->id, $nonDeductibleAmount, $pending,
         $result, $receiptDate,
         $recurringContributionID), $contributionParams
       );
@@ -987,7 +988,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
         $pledgeParams['original_installment_amount'] = $pledgeParams['installment_amount'];
 
         //inherit campaign from contirb page.
-        $pledgeParams['campaign_id'] = $campaignId;
+        $pledgeParams['campaign_id'] = CRM_Utils_Array::value('campaign_id', $contributionParams);
 
         $pledge = CRM_Pledge_BAO_Pledge::create($pledgeParams);
 
@@ -1775,17 +1776,26 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     //so for differentiating membership contribution from
     //main contribution.
     $form->_params['separate_membership_payment'] = 1;
+    $contributionParams = array(
+      'contact_id' => $contactID,
+      'line_item' => $lineItems,
+      'is_test' => $isTest,
+      'campaign_id' => CRM_Utils_Array::value('campaign_id', $tempParams, CRM_Utils_Array::value('campaign_id',
+        $form->_values)),
+      'contribution_page_id' => $form->_id,
+      'source' => CRM_Utils_Array::value('source', $tempParams, CRM_Utils_Array::value('description', $tempParams)),
+    );
+    $isMonetary = !empty($form->_values['is_monetary']);
+    if ($isMonetary) {
+      if (empty($paymentParams['is_pay_later'])) {
+        // @todo look up payment_instrument_id on payment processor table.
+        $contributionParams['payment_instrument_id'] = 1;
+      }
+    }
     $membershipContribution = CRM_Contribute_Form_Contribution_Confirm::processFormContribution($form,
       $tempParams,
       $result,
-      array(
-        'contact_id' => $contactID,
-        'line_item' => $lineItems,
-        'is_test' => $isTest,
-        'campaign_id' => CRM_Utils_Array::value('campaign_id', $tempParams, CRM_Utils_Array::value('campaign_id',
-          $form->_values)),
-        'contribution_page_id' => $form->_id,
-      ),
+      $contributionParams,
       $financialType,
       $pending,
       TRUE,

diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php
index 165d457529cd29f9e13ee93c58fcf4451a6c2ff0..a5ed82ca84f4d9d647438a0b99cfd15232efb166 100644 (file)
--- a/CRM/Member/Form/Membership.php
+++ b/CRM/Member/Form/Membership.php
@@ -1491,18 +1491,20 @@ class CRM_Member_Form_Membership extends CRM_Member_Form {
 
         $contribution = CRM_Contribute_Form_Contribution_Confirm::processFormContribution($this,
           $paymentParams,
-          $result,
+          NULL,
           array(
             'contact_id' => $this->_contributorContactID,
             'line_item' => $lineItem,
             'is_test' => $isTest,
             'campaign_id' => CRM_Utils_Array::value('campaign_id', $paymentParams),
             'contribution_page_id' => CRM_Utils_Array::value('contribution_page_id', $this->_params),
+            'source' => CRM_Utils_Array::value('source', $paymentParams, CRM_Utils_Array::value('description', $paymentParams)),
+            'thankyou_date' => CRM_Utils_Array::value('thankyou_date', $paymentParams),
+            'payment_instrument_id' => $this->_paymentProcessor['payment_instrument_id'],
           ),
           $financialType,
           TRUE,
           FALSE,
-          $isTest,
           $this->_bltID
         );