CRM-19213 fix one instance of mis-setting payment_instrument_id.

There are several but I have only touched the one where I was already working on the test coverage

diff --git a/CRM/Contribute/BAO/Contribution/Utils.php b/CRM/Contribute/BAO/Contribution/Utils.php
index 7c29ade198b90d49a673ae05b3a182daf475707e..4fb880088775c3ea24a7a2c7f9ae84831185d244 100644 (file)
--- a/CRM/Contribute/BAO/Contribution/Utils.php
+++ b/CRM/Contribute/BAO/Contribution/Utils.php
@@ -110,13 +110,10 @@ class CRM_Contribute_BAO_Contribution_Utils {
         '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;
-        }
+      if (!empty($form->_paymentProcessor)) {
+        $contributionParams['payment_instrument_id'] = $paymentParams['payment_instrument_id'] = $form->_paymentProcessor['payment_instrument_id'];
       }
+
       $contribution = CRM_Contribute_Form_Contribution_Confirm::processFormContribution(
         $form,
         $paymentParams,

diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php
index b21b253a3686ff41ead5442ffb6c05d538b97768..51080e1a08514a1ec8160ece56e21162a9ebde37 100644 (file)
--- a/CRM/Contribute/Form/Contribution/Confirm.php
+++ b/CRM/Contribute/Form/Contribution/Confirm.php
@@ -1085,6 +1085,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     $recurParams['installments'] = CRM_Utils_Array::value('installments', $params);
     $recurParams['financial_type_id'] = CRM_Utils_Array::value('financial_type_id', $params);
     $recurParams['currency'] = CRM_Utils_Array::value('currency', $params);
+    $recurParams['payment_instrument_id'] = $params['payment_instrument_id'];
 
     // CRM-14354: For an auto-renewing membership with an additional contribution,
     // if separate payments is not enabled, make sure only the membership fee recurs
@@ -1119,10 +1120,6 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     $recurParams['trxn_id'] = CRM_Utils_Array::value('trxn_id', $params, $params['invoiceID']);
     $recurParams['financial_type_id'] = $contributionType->id;
 
-    if (!empty($form->_values['is_monetary'])) {
-      $recurParams['payment_instrument_id'] = 1;
-    }
-
     $campaignId = CRM_Utils_Array::value('campaign_id', $params, CRM_Utils_Array::value('campaign_id', $form->_values));
     $recurParams['campaign_id'] = $campaignId;
     $recurring = CRM_Contribute_BAO_ContributionRecur::add($recurParams);

diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php
index 53de647a0daff062d24faafe196b1485a7b6b6ab..3ef459915ff457c1748c2e21648a87a069cad1ca 100644 (file)
--- a/CRM/Member/Form/Membership.php
+++ b/CRM/Member/Form/Membership.php
@@ -1400,6 +1400,7 @@ class CRM_Member_Form_Membership extends CRM_Member_Form {
         $financialType->id = $params['financial_type_id'];
         $financialType->find(TRUE);
         $this->_params = $formValues;
+        $paymentParams['payment_instrument_id'] = $this->_paymentProcessor['payment_instrument_id'];
         $contribution = CRM_Contribute_Form_Contribution_Confirm::processFormContribution($this,
           $paymentParams,
           NULL,

diff --git a/CRM/Member/Form/MembershipRenewal.php b/CRM/Member/Form/MembershipRenewal.php
index 19c3b14fce91762750f0c944e37d5e687ec70984..5c73bca43673824b20e3b59f782b81e441014c59 100644 (file)
--- a/CRM/Member/Form/MembershipRenewal.php
+++ b/CRM/Member/Form/MembershipRenewal.php
@@ -552,7 +552,6 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form {
 
       $this->_params['contribution_status_id'] = $result['payment_status_id'];
       $this->_params['trxn_id'] = $result['trxn_id'];
-      $this->_params['payment_instrument_id'] = 1;
       $this->_params['is_test'] = ($this->_mode == 'live') ? 0 : 1;
       $this->set('params', $this->_params);
       $this->assign('trxn_id', $result['trxn_id']);

diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
index 73fa71aa9f567c315da5962c63bec4ce0d5b8d0f..86153cf004ce2124bbd2719680862388ff019756 100644 (file)
--- a/tests/phpunit/CiviTest/CiviUnitTestCase.php
+++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -3087,6 +3087,8 @@ AND    ( TABLE_NAME LIKE 'civicrm_value_%' )
         'billing_mode' => 3,
         'financial_type_id' => 1,
         'financial_account_id' => 12,
+         // Credit card = 1 so can pass 'by accident'.
+        'payment_instrument_id' => 'Debit Card',
       ),
       $params);
     if (!is_numeric($params['payment_processor_type_id'])) {
@@ -3378,7 +3380,6 @@ AND    ( TABLE_NAME LIKE 'civicrm_value_%' )
       'participant_id' => $participant['id'],
       'contribution_id' => $contribution['id'],
     );
-    $ids = array();
     $this->callAPISuccess('ParticipantPayment', 'create', $paymentParticipant);
     return array($lineItems, $contribution);
   }
@@ -3527,7 +3528,7 @@ AND    ( TABLE_NAME LIKE 'civicrm_value_%' )
         'to_financial_account_id' => 12,
         'total_amount' => CRM_Utils_Array::value('total_amount', $params, 100),
         'status_id' => 1,
-        'payment_instrument_id' => 1,
+        'payment_instrument_id' => CRM_Utils_Array::value('payment_instrument_id', $params, 1),
       );
     }
     elseif ($context == 'payLater') {

diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php
index 6e3c1b84a43e16e950aff9585245234517fbfd93..218016617fd6332ca14c0f6ce96d201202ca6ad9 100644 (file)
--- a/tests/phpunit/api/v3/ContributionPageTest.php
+++ b/tests/phpunit/api/v3/ContributionPageTest.php
@@ -257,6 +257,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
       'trxn_id' => 'create_first_success',
       'fee_amount' => .85,
     ));
+    $processor = $dummyPP->getPaymentProcessor();
     $this->callAPISuccess('ContributionPage', 'create', array(
       'id' => $this->_ids['contribution_page'],
       'payment_processor' => array($paymentProcessor2ID, $this->_ids['payment_processor']),
@@ -286,6 +287,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
     $this->_checkFinancialRecords(array(
       'id' => $contribution['id'],
       'total_amount' => $contribution['total_amount'],
+      'payment_instrument_id' => $processor['payment_instrument_id'],
     ), 'online');
   }
 
@@ -599,6 +601,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
     $this->setUpMembershipContributionPage();
     $dummyPP = Civi\Payment\System::singleton()->getByProcessor($this->_paymentProcessor);
     $dummyPP->setDoDirectPaymentResult(array('payment_status_id' => 1, 'trxn_id' => 'create_first_success'));
+    $processor = $dummyPP->getPaymentProcessor();
 
     $submitParams = array(
       'price_' . $this->_ids['price_field'][0] => reset($this->_ids['price_field_value']),
@@ -624,6 +627,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
       'contribution_page_id' => $this->_ids['contribution_page'],
       'contribution_status_id' => 1,
     ));
+    $this->assertEquals($processor['payment_instrument_id'], $contribution['payment_instrument_id']);
 
     $this->assertEquals('create_first_success', $contribution['trxn_id']);
     $membershipPayment = $this->callAPISuccess('membership_payment', 'getsingle', array());
@@ -646,6 +650,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
     $renewedMembership = $this->callAPISuccessGetSingle('membership', array('id' => $membershipPayment['membership_id']));
     $this->assertEquals(date('Y-m-d', strtotime('+ 1 year', strtotime($membership['end_date']))), $renewedMembership['end_date']);
     $recurringContribution = $this->callAPISuccess('contribution_recur', 'getsingle', array('id' => $contribution['contribution_recur_id']));
+    $this->assertEquals($processor['payment_instrument_id'], $recurringContribution['payment_instrument_id']);
     $this->assertEquals(5, $recurringContribution['contribution_status_id']);
   }
 

diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
index 692748e4173067b02ff3acd105ef3de7c6d303dd..7403a288ed05e9738ab3fbbf92e15df76eb77251 100644 (file)
--- a/tests/phpunit/api/v3/ContributionTest.php
+++ b/tests/phpunit/api/v3/ContributionTest.php
@@ -1861,7 +1861,13 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
     unset($lineItem1['values'][0]['id'], $lineItem1['values'][0]['entity_id']);
     unset($lineItem2['values'][0]['id'], $lineItem2['values'][0]['entity_id']);
     $this->assertEquals($lineItem1['values'][0], $lineItem2['values'][0]);
-    $this->_checkFinancialRecords(array('id' => $originalContribution['id'] + 1), 'online');
+    $this->_checkFinancialRecords(array(
+      'id' => $originalContribution['id'] + 1,
+      'payment_instrument_id' => $this->callAPISuccessGetValue('PaymentProcessor', array(
+        'id' => $originalContribution['payment_processor_id'],
+        'return' => 'payment_instrument_id',
+      )),
+    ), 'online');
     $this->quickCleanUpFinancialEntities();
   }
 
@@ -3050,7 +3056,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
           array('contribution_recur_id' => $contributionRecur['id']))
       );
     }
-
+    $originalContribution['payment_processor_id'] = $paymentProcessorID;
     return $originalContribution;
   }