From: eileen <emcnaughton@wikimedia.org>
Date: Tue, 29 Nov 2016 08:55:45 +0000 (+1300)
Subject: CRM-19594 fix line items for separate payments, non-price set
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=f2bca231365f2195862f8dc20f6b569bf4842451;p=civicrm-core.git

CRM-19594 fix line items for separate payments, non-price set
---

diff --git a/CRM/Contribute/BAO/Contribution/Utils.php b/CRM/Contribute/BAO/Contribution/Utils.php
index a4bc54c941..9ceea35380 100644
--- a/CRM/Contribute/BAO/Contribution/Utils.php
+++ b/CRM/Contribute/BAO/Contribution/Utils.php
@@ -42,9 +42,8 @@ class CRM_Contribute_BAO_Contribution_Utils {
    *   value pairs
    * @param int $contactID
    *   Contact id.
-   * @param int $contributionTypeId
+   * @param int $financialTypeID
    *   Financial type id.
-   * @param int|string $component component id
    * @param bool $isTest
    * @param bool $isRecur
    *
@@ -58,8 +57,7 @@ class CRM_Contribute_BAO_Contribution_Utils {
     &$form,
     &$paymentParams,
     $contactID,
-    $contributionTypeId,
-    $component = 'contribution',
+    $financialTypeID,
     $isTest,
     $isRecur
   ) {
@@ -67,7 +65,7 @@ class CRM_Contribute_BAO_Contribution_Utils {
     $isPaymentTransaction = self::isPaymentTransaction($form);
 
     $financialType = new CRM_Financial_DAO_FinancialType();
-    $financialType->id = $contributionTypeId;
+    $financialType->id = $financialTypeID;
     $financialType->find(TRUE);
     if ($financialType->is_deductible) {
       $form->assign('is_deductible', TRUE);
@@ -79,7 +77,7 @@ class CRM_Contribute_BAO_Contribution_Utils {
     //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($contributionTypeId);
+    $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;
 
@@ -127,11 +125,12 @@ class CRM_Contribute_BAO_Contribution_Utils {
         $isRecur
       );
 
-      $paymentParams['contributionTypeID'] = $contributionTypeId;
       $paymentParams['item_name'] = $form->_params['description'];
 
       $paymentParams['qfKey'] = $form->controller->_key;
-      if ($component == 'membership') {
+      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 array('contribution' => $contribution);
       }
 
diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php
index e4fcfd4817..92d975a5d4 100644
--- a/CRM/Contribute/Form/Contribution/Confirm.php
+++ b/CRM/Contribute/Form/Contribution/Confirm.php
@@ -1374,10 +1374,9 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
    * @param array $premiumParams
    * @param array $membershipLineItems
    *   Line items specifically relating to memberships.
-   * @param bool $isPayLater
    */
   protected function processMembership($membershipParams, $contactID, $customFieldsFormatted, $fieldTypes, $premiumParams,
-                                $membershipLineItems, $isPayLater) {
+                                $membershipLineItems) {
 
     $membershipTypeIDs = (array) $membershipParams['selectMembership'];
     $membershipTypes = CRM_Member_BAO_Membership::buildMembershipTypeValues($this, $membershipTypeIDs);
@@ -1410,7 +1409,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
 
     $this->postProcessMembership($membershipParams, $contactID,
       $this, $premiumParams, $customFieldsFormatted, $fieldTypes, $membershipType, $membershipTypeIDs, $isPaidMembership, $this->_membershipId, $isProcessSeparateMembershipTransaction, $financialTypeID,
-      $membershipLineItems, $isPayLater, $isPending);
+      $membershipLineItems, $isPending);
 
     $this->assign('membership_assign', TRUE);
     $this->set('membershipTypeID', $membershipParams['selectMembership']);
@@ -1440,9 +1439,8 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
    * @param bool $isProcessSeparateMembershipTransaction
    *
    * @param int $financialTypeID
-   * @param array $membershipLineItems
-   *   Line items specific to membership payment that is separate to contribution.
-   * @param bool $isPayLater
+   * @param array $unprocessedLineItems
+   *   Line items for payment options chosen on the form.
    * @param bool $isPending
    *
    * @throws \CRM_Core_Exception
@@ -1450,13 +1448,14 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
   protected function postProcessMembership(
     $membershipParams, $contactID, &$form, $premiumParams,
     $customFieldsFormatted = NULL, $includeFieldTypes = NULL, $membershipDetails, $membershipTypeIDs, $isPaidMembership, $membershipID,
-    $isProcessSeparateMembershipTransaction, $financialTypeID, $membershipLineItems, $isPayLater, $isPending) {
+    $isProcessSeparateMembershipTransaction, $financialTypeID, $unprocessedLineItems, $isPending) {
+
     $membershipContribution = NULL;
     $isTest = CRM_Utils_Array::value('is_test', $membershipParams, FALSE);
     $errors = $paymentResults = array();
     $form->_values['isMembership'] = TRUE;
     $isRecurForFirstTransaction = CRM_Utils_Array::value('is_recur', $form->_values, CRM_Utils_Array::value('is_recur', $membershipParams));
-    $unprocessedLineItems = $form->_lineItem;
+
     $totalAmount = $membershipParams['amount'];
 
     if ($isPaidMembership) {
@@ -1469,17 +1468,21 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
       }
 
       if (!$isProcessSeparateMembershipTransaction) {
+        // Skip line items in the contribution processing transaction.
+        // We will create them with the membership for proper linking.
         $membershipParams['skipLineItem'] = 1;
       }
       else {
         $membershipParams['total_amount'] = $totalAmount;
+        $membershipParams['skipLineItem'] = 0;
         CRM_Price_BAO_LineItem::getLineItemArray($membershipParams);
 
       }
-      $paymentResult = CRM_Contribute_BAO_Contribution_Utils::processConfirm($form, $membershipParams,
+      $paymentResult = CRM_Contribute_BAO_Contribution_Utils::processConfirm(
+        $form,
+        $membershipParams,
         $contactID,
         $financialTypeID,
-        'membership',
         $isTest,
         $isRecurForFirstTransaction
       );
@@ -1496,12 +1499,12 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
 
     if ($isProcessSeparateMembershipTransaction) {
       try {
-        $form->_lineItem = $membershipLineItems;
+        $form->_lineItem = $unprocessedLineItems;
         if (empty($form->_params['auto_renew']) && !empty($membershipParams['is_recur'])) {
           unset($membershipParams['is_recur']);
         }
         list($membershipContribution, $secondPaymentResult) = $this->processSecondaryFinancialTransaction($contactID, $form, array_merge($membershipParams, array('skipLineItem' => 1)),
-          $isTest, $membershipLineItems, CRM_Utils_Array::value('minimum_fee', $membershipDetails, 0), CRM_Utils_Array::value('financial_type_id', $membershipDetails));
+          $isTest, $unprocessedLineItems, CRM_Utils_Array::value('minimum_fee', $membershipDetails, 0), CRM_Utils_Array::value('financial_type_id', $membershipDetails));
         $paymentResults[] = array('contribution_id' => $membershipContribution->id, 'result' => $secondPaymentResult);
       }
       catch (CRM_Core_Exception $e) {
@@ -2012,7 +2015,6 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
    * @return array
    */
   protected function processFormSubmission($contactID) {
-    $isPayLater = $this->_params['is_pay_later'];
     if (!isset($this->_params['payment_processor_id'])) {
       // If there is no processor we are using the pay-later manual pseudo-processor.
       // (note it might make sense to make this a row in the processor table in the db).
@@ -2266,13 +2268,15 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
       }
 
       CRM_Core_Payment_Form::mapParams($this->_bltID, $this->_params, $membershipParams, TRUE);
-      $this->doMembershipProcessing($contactID, $membershipParams, $premiumParams, $isPayLater);
+      $this->doMembershipProcessing($contactID, $membershipParams, $premiumParams, $this->_lineItem);
     }
     else {
       // at this point we've created a contact and stored its address etc
       // all the payment processors expect the name and address to be in the
       // so we copy stuff over to first_name etc.
       $paymentParams = $this->_params;
+      // Make it explict that we are letting the processConfirm function figure out the line items.
+      $paymentParams['skipLineItem'] = 0;
 
       if (!empty($paymentParams['onbehalf']) &&
         is_array($paymentParams['onbehalf'])
@@ -2287,7 +2291,6 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
       $result = CRM_Contribute_BAO_Contribution_Utils::processConfirm($this, $paymentParams,
         $contactID,
         $this->wrangleFinancialTypeID($this->_values['financial_type_id']),
-        'contribution',
         ($this->_mode == 'test') ? 1 : 0,
         CRM_Utils_Array::value('is_recur', $paymentParams)
       );
@@ -2312,9 +2315,9 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
    * @param int $contactID
    * @param array $membershipParams
    * @param array $premiumParams
-   * @param bool $isPayLater
+   * @param array $formLineItems
    */
-  protected function doMembershipProcessing($contactID, $membershipParams, $premiumParams, $isPayLater) {
+  protected function doMembershipProcessing($contactID, $membershipParams, $premiumParams, $formLineItems) {
     // This could be set by a hook.
     if (!empty($this->_params['installments'])) {
       $membershipParams['installments'] = $this->_params['installments'];
@@ -2380,8 +2383,9 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     }
     if (!empty($membershipParams['selectMembership'])) {
       // CRM-12233
-      $membershipLineItems = array();
+      $membershipLineItems = $formLineItems;
       if ($this->_separateMembershipPayment && $this->_values['amount_block_is_active']) {
+        $membershipLineItems = array();
         foreach ($this->_values['fee'] as $key => $feeValues) {
           if ($feeValues['name'] == 'membership_amount') {
             $fieldId = $this->_params['price_' . $key];
@@ -2392,7 +2396,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
         }
       }
       try {
-        $this->processMembership($membershipParams, $contactID, $customFieldsFormatted, $fieldTypes, $premiumParams, $membershipLineItems, $isPayLater);
+        $this->processMembership($membershipParams, $contactID, $customFieldsFormatted, $fieldTypes, $premiumParams, $membershipLineItems);
       }
       catch (CRM_Core_Exception $e) {
         CRM_Core_Session::singleton()->setStatus($e->getMessage());
diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php
index fbe0e5c8bc..c9edd1518f 100644
--- a/tests/phpunit/api/v3/ContributionPageTest.php
+++ b/tests/phpunit/api/v3/ContributionPageTest.php
@@ -548,6 +548,12 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
     $this->assertTrue(in_array($membershipPayment['contribution_id'], array_keys($contributions['values'])));
     $membership = $this->callAPISuccessGetSingle('membership', array('id' => $membershipPayment['membership_id']));
     $this->assertEquals($membership['contact_id'], $contributions['values'][$membershipPayment['contribution_id']]['contact_id']);
+    $lineItem = $this->callAPISuccessGetSingle('LineItem', array('entity_table' => 'civicrm_membership'));
+    $this->assertEquals($lineItem['entity_id'], $membership['id']);
+    $this->assertEquals($lineItem['contribution_id'], $membershipPayment['contribution_id']);
+    $this->assertEquals($lineItem['qty'], 1);
+    $this->assertEquals($lineItem['unit_price'], 2);
+    $this->assertEquals($lineItem['line_total'], 2);
     foreach ($contributions['values'] as $contribution) {
       $this->assertEquals(.72, $contribution['fee_amount']);
       $this->assertEquals($contribution['total_amount'] - .72, $contribution['net_amount']);