From: eileen <emcnaughton@wikimedia.org>
Date: Fri, 15 Nov 2019 04:10:27 +0000 (+1300)
Subject: Deprecate creating partially paid contributions, other than by partially paying a... 
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=b048ed178c4b970341b3f931136a1b2859b65d59;p=civicrm-core.git

Deprecate creating partially paid contributions, other than by partially paying a contribution.

As https://github.com/civicrm/civicrm-core/pull/15706 signposts - creating a contribution with a status of 'Partially Paid'
is not actually a valid thing to do. There are no created financial_trxns & the balance is wrong. In some places the code does
this in conjunction with some parameters. These don't really work either. We need to deprecate & eliminate this flow.

I'm expecting some tests to fail & need fixing before this passes
---

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 0f1a0c6359..d2f34754b5 100644
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -132,7 +132,6 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
       }
     }
 
-    $contributionStatus = CRM_Contribute_PseudoConstant::contributionStatus(NULL, 'name');
     //if contribution is created with cancelled or refunded status, add credit note id
     // do the same for chargeback - this entered the code 'accidentally' but moving it to here
     // as part of cleanup maintains consistency.
@@ -141,7 +140,11 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
         $params['creditnote_id'] = self::createCreditNoteId();
       }
     }
-    if (empty($params['contribution_status_id'])) {
+    $contributionStatusID = $params['contribution_status_id'] ?? NULL;
+    if (CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', (int) $contributionStatusID) === 'Partially paid' && empty($params['is_post_payment_create'])) {
+      CRM_Core_Error::deprecatedFunctionWarning('Setting status to partially paid other than by using Payment.create is deprecated and unreliable');
+    }
+    if (!$contributionStatusID) {
       // Since the fee amount is expecting this (later on) ensure it is always set.
       // It would only not be set for an update where it is unchanged.
       $params['contribution_status_id'] = civicrm_api3('Contribution', 'getvalue', [
@@ -149,6 +152,7 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
         'return' => 'contribution_status_id',
       ]);
     }
+    $contributionStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', (int) $params['contribution_status_id']);
 
     if (!$contributionID
       && CRM_Utils_Array::value('membership_id', $params)
@@ -183,9 +187,10 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
     if ($contributionID && $setPrevContribution) {
       $params['prevContribution'] = self::getOriginalContribution($contributionID);
     }
+    $previousContributionStatus = ($contributionID && !empty($params['prevContribution'])) ? CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', (int) $params['prevContribution']->contribution_status_id) : NULL;
 
-    if ($contributionID && !empty($params['revenue_recognition_date']) && !empty($params['prevContribution'])
-      && !($contributionStatus[$params['prevContribution']->contribution_status_id] == 'Pending')
+    if ($contributionID && !empty($params['revenue_recognition_date'])
+      && !($previousContributionStatus === 'Pending')
       && !self::allowUpdateRevenueRecognitionDate($contributionID)
     ) {
       unset($params['revenue_recognition_date']);
@@ -236,7 +241,7 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
     if (self::isUpdateToRecurringContribution($params)) {
       CRM_Contribute_BAO_ContributionRecur::updateOnNewPayment(
         (!empty($params['contribution_recur_id']) ? $params['contribution_recur_id'] : $params['prevContribution']->contribution_recur_id),
-        $contributionStatus[$params['contribution_status_id']],
+        $contributionStatus,
         CRM_Utils_Array::value('receive_date', $params)
       );
     }
diff --git a/tests/phpunit/CRM/Contribute/Form/TaskTest.php b/tests/phpunit/CRM/Contribute/Form/TaskTest.php
index 104cd0dd5d..acd140f5bb 100644
--- a/tests/phpunit/CRM/Contribute/Form/TaskTest.php
+++ b/tests/phpunit/CRM/Contribute/Form/TaskTest.php
@@ -27,6 +27,8 @@ class CRM_Contribute_Form_TaskTest extends CiviUnitTestCase {
   /**
    * CRM-19722 - Check CRM_Contribute_Form_Task::preProcessCommon()
    * executes without any error after sorting the search result.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testPreProcessCommonAfterSorting() {
     $fields = [
@@ -47,6 +49,10 @@ class CRM_Contribute_Form_TaskTest extends CiviUnitTestCase {
         'financial_type_id' => $financialTypes[$i],
         'contribution_status_id' => $status[$i],
       ];
+      if ($status[$i] === 'Partially paid') {
+        $contributionParams['contribution_status_id'] = 'Pending';
+        $contributionParams['api.Payment.create'] = ['total_amount' => 50];
+      }
       $contribution = $this->callAPISuccess('Contribution', 'create', $contributionParams);
       $contributionIds[] = $contribution['id'];
     }
diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php
index 806a962543..b35c227b0c 100644
--- a/tests/phpunit/api/v3/ContributionPageTest.php
+++ b/tests/phpunit/api/v3/ContributionPageTest.php
@@ -1307,7 +1307,6 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
     $submitParams = [
       'price_' . $this->_ids['price_field'][0] => reset($this->_ids['price_field_value']),
       'id' => (int) $this->_ids['contribution_page'],
-      'amount' => 10,
       'billing_first_name' => 'Billy',
       'billing_middle_name' => 'Goat',
       'billing_last_name' => 'Gruff',