From f69a9ac37243bfd31132875efe853baecdb295a2 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 16 Nov 2016 12:50:50 +1300 Subject: [PATCH] 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 --- CRM/Contribute/BAO/Contribution/Utils.php | 9 +++------ CRM/Contribute/Form/Contribution/Confirm.php | 5 +---- CRM/Member/Form/Membership.php | 1 + CRM/Member/Form/MembershipRenewal.php | 1 - tests/phpunit/CiviTest/CiviUnitTestCase.php | 5 +++-- tests/phpunit/api/v3/ContributionPageTest.php | 5 +++++ tests/phpunit/api/v3/ContributionTest.php | 10 ++++++++-- 7 files changed, 21 insertions(+), 15 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution/Utils.php b/CRM/Contribute/BAO/Contribution/Utils.php index 7c29ade198b9..4fb880088775 100644 --- a/CRM/Contribute/BAO/Contribution/Utils.php +++ b/CRM/Contribute/BAO/Contribution/Utils.php @@ -110,13 +110,10 @@ public static function processConfirm( '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 b21b253a3686..51080e1a0851 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -1085,6 +1085,7 @@ public static function processRecurringContribution(&$form, &$params, $contactID $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 @@ public static function processRecurringContribution(&$form, &$params, $contactID $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 53de647a0daf..3ef459915ff4 100644 --- a/CRM/Member/Form/Membership.php +++ b/CRM/Member/Form/Membership.php @@ -1400,6 +1400,7 @@ public function submit() { $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 19c3b14fce91..5c73bca43673 100644 --- a/CRM/Member/Form/MembershipRenewal.php +++ b/CRM/Member/Form/MembershipRenewal.php @@ -552,7 +552,6 @@ protected function submit() { $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 73fa71aa9f56..86153cf004ce 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -3087,6 +3087,8 @@ public function paymentProcessorCreate($params = array()) { '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 @@ protected function createParticipantWithContribution() { 'participant_id' => $participant['id'], 'contribution_id' => $contribution['id'], ); - $ids = array(); $this->callAPISuccess('ParticipantPayment', 'create', $paymentParticipant); return array($lineItems, $contribution); } @@ -3527,7 +3528,7 @@ public function _checkFinancialRecords($params, $context) { '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 6e3c1b84a43e..218016617fd6 100644 --- a/tests/phpunit/api/v3/ContributionPageTest.php +++ b/tests/phpunit/api/v3/ContributionPageTest.php @@ -257,6 +257,7 @@ public function testSubmitRecurMultiProcessorInstantPayment() { '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 @@ public function testSubmitRecurMultiProcessorInstantPayment() { $this->_checkFinancialRecords(array( 'id' => $contribution['id'], 'total_amount' => $contribution['total_amount'], + 'payment_instrument_id' => $processor['payment_instrument_id'], ), 'online'); } @@ -599,6 +601,7 @@ public function testSubmitMembershipPriceSetPaymentPaymentProcessorRecurInstantP $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 @@ public function testSubmitMembershipPriceSetPaymentPaymentProcessorRecurInstantP '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 @@ public function testSubmitMembershipPriceSetPaymentPaymentProcessorRecurInstantP $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 692748e41730..7403a288ed05 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -1861,7 +1861,13 @@ public function testRepeatTransaction() { 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 @@ protected function setUpRepeatTransaction($recurParams = array(), $flag) { array('contribution_recur_id' => $contributionRecur['id'])) ); } - + $originalContribution['payment_processor_id'] = $paymentProcessorID; return $originalContribution; }