From 6c33ebb19808e2a9d81ac0e3b64fad09684744f1 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Tue, 16 May 2023 12:42:58 +1200
Subject: [PATCH] Only call transitionComponents when transitioning pending/pay
 later to completed

Rather than early return when called in error, don't call it
---
 CRM/Contribute/BAO/Contribution.php  | 13 ++-----------
 CRM/Contribute/Form/Contribution.php |  7 ++++---
 CRM/Contribute/Form/Task/Batch.php   |  9 +++++----
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 5b45ee305d..4c209004d0 100644
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -1786,16 +1786,13 @@ LEFT JOIN  civicrm_contribution contribution ON ( componentPayment.contribution_
    *
    */
   public static function transitionComponents($params) {
-    $contributionStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $params['contribution_status_id']);
-    $previousStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $params['previous_contribution_status_id']);
     // @todo fix the one place that calls this function to use Payment.create
     // remove this.
     // get minimum required values.
     $contributionId = $params['contribution_id'];
-    $contributionStatusId = $params['contribution_status_id'];
 
     // we process only ( Completed, Cancelled, or Failed ) contributions.
-    if (!$contributionId || $contributionStatus !== 'Completed') {
+    if (!$contributionId) {
       return;
     }
 
@@ -1880,12 +1877,6 @@ LEFT JOIN  civicrm_contribution contribution ON ( componentPayment.contribution_
       );
     }
 
-    // only pending contribution related object processed.
-    if (!in_array($previousStatus, ['Pending', 'Partially paid'])) {
-      // this is case when we already processed contribution object.
-      return;
-    }
-
     if (is_array($memberships)) {
       foreach ($memberships as $membership) {
         if ($membership) {
@@ -2039,7 +2030,7 @@ LEFT JOIN  civicrm_contribution contribution ON ( componentPayment.contribution_
     }
 
     if ($pledgePayment) {
-      CRM_Pledge_BAO_PledgePayment::updatePledgePaymentStatus($pledgeID, $pledgePaymentIDs, $contributionStatusId);
+      CRM_Pledge_BAO_PledgePayment::updatePledgePaymentStatus($pledgeID, $pledgePaymentIDs, CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed'));
     }
 
   }
diff --git a/CRM/Contribute/Form/Contribution.php b/CRM/Contribute/Form/Contribution.php
index 3c6bfa76bc..20888411a6 100644
--- a/CRM/Contribute/Form/Contribution.php
+++ b/CRM/Contribute/Form/Contribution.php
@@ -1791,15 +1791,16 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP
       }
       $contribution = CRM_Contribute_BAO_Contribution::create($params);
 
+      $previousStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $this->_values['contribution_status_id'] ?? NULL);
       // process associated membership / participant, CRM-4395
-      if ($contribution->id && $action & CRM_Core_Action::UPDATE) {
+      if ($contribution->id && $action & CRM_Core_Action::UPDATE
+        && in_array($previousStatus, ['Pending', 'Partially paid'], TRUE)
+        && 'Completed' === CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $this->getSubmittedValue('contribution_status_id'))) {
         // @todo use Payment.create to do this, remove transitioncomponents function
         // if contribution is being created with a completed status it should be
         // created pending & then Payment.create adds the payment
         CRM_Contribute_BAO_Contribution::transitionComponents([
           'contribution_id' => $contribution->id,
-          'contribution_status_id' => $contribution->contribution_status_id,
-          'previous_contribution_status_id' => $this->_values['contribution_status_id'] ?? NULL,
           'receive_date' => $contribution->receive_date,
         ]);
       }
diff --git a/CRM/Contribute/Form/Task/Batch.php b/CRM/Contribute/Form/Task/Batch.php
index 5381f47c91..4d50e0b5a0 100644
--- a/CRM/Contribute/Form/Task/Batch.php
+++ b/CRM/Contribute/Form/Task/Batch.php
@@ -208,13 +208,14 @@ class CRM_Contribute_Form_Task_Batch extends CRM_Contribute_Form_Task {
         $contribution = civicrm_api3('Contribution', 'create', $value);
         $contribution = $contribution['values'][$contributionID];
 
-        // @todo add check as to whether the status is updated.
-        if (!empty($value['contribution_status_id'])) {
+        $currentStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $value['contribution_status_id'] ?? NULL);
+        $previousStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $this->_defaultValues["field[{$contributionID}][contribution_status_id]"] ?? NULL);
+
+        if ('Completed' === $currentStatus &&
+          in_array($previousStatus, ['Pending', 'Partially paid'], TRUE)) {
           // @todo - use completeorder api or make api call do this.
           CRM_Contribute_BAO_Contribution::transitionComponents([
             'contribution_id' => $contribution['id'],
-            'contribution_status_id' => $value['contribution_status_id'],
-            'previous_contribution_status_id' => CRM_Utils_Array::value("field[{$contributionID}][contribution_status_id]", $this->_defaultValues),
             'receive_date' => $contribution['receive_date'],
           ]);
         }
-- 
2.25.1