[REF] Clean up handling of financial_type_id override

Currently we use the primaryContributionID to determine whether the templateContribution will
have more than one line item & unset the parameter. It makes much more sense to do evaluations
on how many line items the template contribution will have when we have loaded it's line items.

The logic is that the financial_type_id can be overriden either by editing the recurring contribution
(which you are permitted to do if only one line item exists) or by passing it in via input (which
is ignored if there is more than one line item.

This change ensures the input parameter reaches repeatTrannsaction - which is the appropriate
place to determine whether to apply it.

It removes one of 2 places that refers to primaryContributionID. The other is also inappropriate
as it is basically used to look up the line items to see the number of terms for the membership. However,
we already know that for any repeat transactions as we have already loaded the line items. In
completeOrder the line items are that of the contribution being saved.

We could pass the saved contribution id into this function as a quick way to eliminate the primaryContributionID
or we could just determine num_terms rather than primaryContributionID earlier on

CRM/Contribute/BAO/Contribution.php

@@ -2611,9 +2611,9 @@ protected static function repeatTransaction(&$contribution, &$input, $contributi
         ])
       );
       $input['line_item'] = $contributionParams['line_item'] = $templateContribution['line_item'];
-
       $contributionParams['status_id'] = 'Pending';
-      if (isset($contributionParams['financial_type_id'])) {
+
+      if (isset($contributionParams['financial_type_id']) && count($input['line_item']) === 1) {
         // Give precedence to passed in type.
         $contribution->financial_type_id = $contributionParams['financial_type_id'];
       }
@@ -4440,10 +4440,8 @@ public static function completeOrder($input, &$ids, $objects, $transaction = NUL
       'contribution_status_id',
       'card_type_id',
       'pan_truncation',
+      'financial_type_id',
     ];
-    if (self::isSingleLineItem($primaryContributionID)) {
-      $inputContributionWhiteList[] = 'financial_type_id';
-    }
 
     $participant = $objects['participant'] ?? NULL;
     $recurContrib = $objects['contributionRecur'] ?? NULL;
@@ -4487,6 +4485,12 @@ public static function completeOrder($input, &$ids, $objects, $transaction = NUL
     $changeDate = CRM_Utils_Array::value('trxn_date', $input, date('YmdHis'));
 
     self::repeatTransaction($contribution, $input, $contributionParams, $paymentProcessorId);
+    // @todo this line only exists because after we save the contribution in repeatTransaction
+    // with the correct params we resave it shortly after this with ahem params.
+    // if we stop doing that per
+    // https://github.com/civicrm/civicrm-core/pull/17972
+    // this line can go. Test cover in testRepeatTransactionPassedInFinancialTypeTwoLineItems
+    // and testRepeatTransactionPassedInFinancialType
     $contributionParams['financial_type_id'] = $contribution->financial_type_id;
 
     $values = [];
@@ -4533,6 +4537,7 @@ public static function completeOrder($input, &$ids, $objects, $transaction = NUL
 
     // CRM-19309 - if you update the contribution here with financial_type_id it can/will mess with $lineItem
     // unsetting it here does NOT cause any other contribution test to fail!
+    // this will no longer be required once we stop setting it above/ stop double saving.
     unset($contributionParams['financial_type_id']);
     $contributionResult = civicrm_api3('Contribution', 'create', $contributionParams);
 

CRM/Contribute/BAO/ContributionRecur.php

@@ -442,8 +442,11 @@ public static function getTemplateContribution($id, $overrides = []) {
     }
     if ($templateContributions->count()) {
       $templateContribution = $templateContributions->first();
-      $result = array_merge($templateContribution, $overrides);
       $lineItems = CRM_Price_BAO_LineItem::getLineItemsByContributionID($templateContribution['id']);
+      if (count($lineItems) > 1 && isset($overrides['financial_type_id'])) {
+        unset($overrides['financial_type_id']);
+      }
+      $result = array_merge($templateContribution, $overrides);
       $result['line_item'] = self::reformatLineItemsForRepeatContribution($result['total_amount'], $result['financial_type_id'], $lineItems, (array) $templateContribution);
       return $result;
     }

tests/phpunit/api/v3/ContributionTest.php

@@ -2684,7 +2684,10 @@ public function testRepeatTransactionAlteredAmount() {
   }
 
   /**
-   * CRM-17718 test appropriate action if financial type has changed for single line items.
+   * Test financial_type_id override behaviour with a single line item.
+   *
+   * CRM-17718 a passed in financial_type_id is allowed to override the original contribution where there
+   * is only one line item.
    */
   public function testRepeatTransactionPassedInFinancialType() {
     $originalContribution = $this->setUpRecurringContribution();
@@ -2711,7 +2714,7 @@ public function testRepeatTransactionPassedInFinancialType() {
       ],
     ];
 
-    $this->callAPISuccessGetSingle('contribution', [
+    $this->callAPISuccessGetSingle('Contribution', [
       'total_amount' => 100,
       'financial_type_id' => 2,
     ]);
@@ -2734,6 +2737,37 @@ public function testRepeatTransactionPassedInFinancialType() {
     $this->assertEquals($expectedLineItem, $lineItem2['values'][0]);
   }
 
+  /**
+   * Test financial_type_id override behaviour with a single line item.
+   *
+   * CRM-17718 a passed in financial_type_id is not allowed to override the original contribution where there
+   * is more than one line item.
+   */
+  public function testRepeatTransactionPassedInFinancialTypeTwoLineItems() {
+    $this->_params = $this->getParticipantOrderParams();
+    $originalContribution = $this->setUpRecurringContribution();
+
+    $this->callAPISuccess('Contribution', 'repeattransaction', [
+      'original_contribution_id' => $originalContribution['id'],
+      'contribution_status_id' => 'Completed',
+      'trxn_id' => 'repeat',
+      'financial_type_id' => 2,
+    ]);
+
+    // Retrieve the new contribution and note the financial type passed in has been ignored.
+    $contribution = $this->callAPISuccessGetSingle('Contribution', [
+      'trxn_id' => 'repeat',
+    ]);
+    $this->assertEquals(4, $contribution['financial_type_id']);
+
+    $lineItems = $this->callAPISuccess('line_item', 'get', [
+      'entity_id' => $contribution['id'],
+    ])['values'];
+    foreach ($lineItems as $lineItem) {
+      $this->assertNotEquals(2, $lineItem['financial_type_id']);
+    }
+  }
+
   /**
    * CRM-17718 test appropriate action if financial type has changed for single line items.
    */
@@ -4064,6 +4098,7 @@ public function _deletedAddedPaymentInstrument() {
    *   Parameters to merge into the recur only.
    *
    * @return array|int
+   * @throws \CRM_Core_Exception
    */
   protected function setUpRecurringContribution($generalParams = [], $recurParams = []) {
     $contributionRecur = $this->callAPISuccess('contribution_recur', 'create', array_merge([
@@ -4077,12 +4112,14 @@ protected function setUpRecurringContribution($generalParams = [], $recurParams
       'frequency_unit' => 'month',
       'payment_processor_id' => $this->paymentProcessorID,
     ], $generalParams, $recurParams));
-    $originalContribution = $this->callAPISuccess('contribution', 'create', array_merge(
+    $contributionParams = array_merge(
       $this->_params,
       [
         'contribution_recur_id' => $contributionRecur['id'],
-      ], $generalParams)
-    );
+        'contribution_status_id' => 'Pending',
+      ], $generalParams);
+    $contributionParams['api.Payment.creaate'] = $contributionParams['total_amount'];
+    $originalContribution = $this->callAPISuccess('Order', 'create', $contributionParams);
     return $originalContribution;
   }