From c4f0a8277a419428e7f60aa3584d4a928456af60 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Thu, 7 Jul 2022 14:00:34 +1200
Subject: [PATCH] Fix handling of parameters in repeattransaction

The function was forcing in the value from the original contribution
into amount, rather than the value from the templateContribution
---
 CRM/Contribute/BAO/ContributionRecur.php  |  3 +--
 api/v3/Contribution.php                   | 17 +++++++----------
 tests/phpunit/api/v3/ContributionTest.php |  2 --
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/CRM/Contribute/BAO/ContributionRecur.php b/CRM/Contribute/BAO/ContributionRecur.php
index 30f590d695..f60e55dc6d 100644
--- a/CRM/Contribute/BAO/ContributionRecur.php
+++ b/CRM/Contribute/BAO/ContributionRecur.php
@@ -496,10 +496,9 @@ INNER JOIN civicrm_contribution       con ON ( con.id = mp.contribution_id )
    * @throws \API_Exception
    */
   public static function getTemplateContribution(int $id, array $inputOverrides = []): array {
-    $recurFields = ['is_test', 'financial_type_id', 'amount', 'campaign_id'];
     $recurringContribution = ContributionRecur::get(FALSE)
       ->addWhere('id', '=', $id)
-      ->setSelect($recurFields)
+      ->setSelect(['is_test', 'financial_type_id', 'amount', 'campaign_id'])
       ->execute()
       ->first();
 
diff --git a/api/v3/Contribution.php b/api/v3/Contribution.php
index 2cc31bec4c..3f01a7d4ef 100644
--- a/api/v3/Contribution.php
+++ b/api/v3/Contribution.php
@@ -625,6 +625,7 @@ function civicrm_api3_contribution_repeattransaction($params) {
   // We need a contribution to copy.
   if (empty($params['original_contribution_id'])) {
     // Find one from the given recur. A template contribution is preferred, otherwise use the latest one added.
+    // @todo this duplicates work done by CRM_Contribute_BAO_Contribution::repeatTransaction & should be removed.
     $templateContribution = CRM_Contribute_BAO_ContributionRecur::getTemplateContribution($params['contribution_recur_id']);
     if (empty($templateContribution)) {
       throw new CiviCRM_API3_Exception('Contribution.repeattransaction failed to get original_contribution_id for recur with ID: ' . $params['contribution_recur_id']);
@@ -632,6 +633,7 @@ function civicrm_api3_contribution_repeattransaction($params) {
   }
   else {
     // A template/original contribution was specified by the params. Load it.
+    // @todo this duplicates work done by CRM_Contribute_BAO_Contribution::repeatTransaction & should be removed.
     $templateContribution = Contribution::get(FALSE)
       ->addWhere('id', '=', $params['original_contribution_id'])
       ->addWhere('is_test', 'IN', [0, 1])
@@ -656,27 +658,20 @@ function civicrm_api3_contribution_repeattransaction($params) {
     'card_type_id',
     'pan_truncation',
     'payment_instrument_id',
+    'total_amount',
   ];
   $input = array_intersect_key($params, array_fill_keys($paramsToCopy, NULL));
   // Ensure certain keys exist with NULL values if they don't already (not sure if this is ACTUALLY necessary?)
   $input += array_fill_keys(['card_type_id', 'pan_truncation'], NULL);
 
-  // Set amount, use templateContribution if not in params.
-  $input['total_amount'] = $params['total_amount'] ?? $templateContribution['total_amount'];
-  // Why do we need this extra 'amount' key? It's used in
-  // CRM_Contribute_BAO_Contribution::completeOrder to pass on to
-  // CRM_Contribute_BAO_ContributionRecur::updateRecurLinkedPledge but it could
-  // possibly be removed if that code was adjusted, and/or when we move away
-  // from using completeOrder.
-  $input['amount'] = $input['total_amount'];
-
   $input['payment_processor_id'] = civicrm_api3('contributionRecur', 'getvalue', [
     'return' => 'payment_processor_id',
     'id' => $templateContribution['contribution_recur_id'],
   ]);
-
+  // @todo this duplicates work done by CRM_Contribute_BAO_Contribution::repeatTransaction & should be removed.
   $input['is_test'] = $templateContribution['is_test'];
 
+  // @todo this duplicates work done by CRM_Contribute_BAO_Contribution::repeatTransaction & should be removed.
   if (empty($templateContribution['contribution_page_id'])) {
     if (empty($domainFromEmail) && (empty($params['receipt_from_name']) || empty($params['receipt_from_email']))) {
       [$domainFromName, $domainFromEmail] = CRM_Core_BAO_Domain::getNameAndEmail(TRUE);
@@ -685,6 +680,8 @@ function civicrm_api3_contribution_repeattransaction($params) {
     $input['receipt_from_email'] = ($params['receipt_from_email'] ?? NULL) ?: $domainFromEmail;
   }
 
+  // @todo this should call CRM_Contribute_BAO_Contribution::repeatTransaction - some minor cleanup needed to separate
+  // from completeOrder
   return CRM_Contribute_BAO_Contribution::completeOrder($input,
     $templateContribution['contribution_recur_id'],
     NULL,
diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
index 39fcdf5304..6c45fb1a70 100644
--- a/tests/phpunit/api/v3/ContributionTest.php
+++ b/tests/phpunit/api/v3/ContributionTest.php
@@ -2702,8 +2702,6 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
   /**
    * CRM-16397 test appropriate action if total amount has changed for single
    * line items.
-   *
-   * @throws \CRM_Core_Exception
    */
   public function testRepeatTransactionAlteredAmount(): void {
     $paymentProcessorID = $this->paymentProcessorCreate();
-- 
2.25.1