Upgrade script for contribution_recur amount, fix loading
authorEileen McNaughton <emcnaughton@wikimedia.org>
Fri, 1 Jul 2022 21:13:07 +0000 (09:13 +1200)
committerEileen McNaughton <emcnaughton@wikimedia.org>
Thu, 7 Jul 2022 05:06:32 +0000 (17:06 +1200)
CRM/Contribute/BAO/Contribution.php
CRM/Contribute/BAO/ContributionRecur.php
CRM/Upgrade/Incremental/sql/5.52.alpha1.mysql.tpl
tests/phpunit/api/v3/ContributionTest.php

index e90d9eb220a71195f96bac0c965976e1496f560b..0ad9215b24429ce6d653b9b94e3d9cad8805289d 100644 (file)
@@ -2162,12 +2162,11 @@ LEFT JOIN  civicrm_contribution contribution ON ( componentPayment.contribution_
   protected static function repeatTransaction(array $input, array $contributionParams) {
     $templateContribution = CRM_Contribute_BAO_ContributionRecur::getTemplateContribution(
       (int) $contributionParams['contribution_recur_id'],
-      array_filter([
+      [
         'total_amount' => $input['total_amount'] ?? NULL,
         'financial_type_id' => $input['financial_type_id'] ?? NULL,
         'campaign_id' => $input['campaign_id'] ?? NULL,
-        // array_filter with strlen filters out NULL, '' and FALSE but not 0.
-      ], 'strlen')
+      ]
     );
     $contributionParams['line_item'] = $templateContribution['line_item'];
     $contributionParams['status_id'] = 'Pending';
index 7ede12463a3a4a724db65de0cd0e91594921120c..a8827cfd5fa8bff190a5421c579cbb27a16bf95d 100644 (file)
@@ -488,30 +488,30 @@ INNER JOIN civicrm_contribution       con ON ( con.id = mp.contribution_id )
    * Later we might merge in data stored against the contribution recur record rather than just return the contribution.
    *
    * @param int $id
-   * @param array $overrides
+   * @param array $inputOverrides
    *   Parameters that should be overridden. Add unit tests if using parameters other than total_amount & financial_type_id.
    *
    * @return array
    *
    * @throws \API_Exception
    */
-  public static function getTemplateContribution(int $id, $overrides = []): array {
-    $recurFields = ['is_test', 'financial_type_id', 'total_amount', 'campaign_id'];
+  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)
       ->execute()
       ->first();
-    // If financial_type_id or total_amount are set on the
-    // recurring they are overrides, but of lower precedence
-    // than input parameters.
+
+    // Parameters passed into the function take precedences, falling back to those loaded from
+    // the recurring contribution.
     // we filter out null, '' and FALSE but not zero - I'm on the fence about zero.
-    $overrides = array_filter(array_merge(
-      // We filter recurringContribution as we only want the fields we asked for
-      // and specifically don't want 'id' added to overrides.
-      array_intersect_key($recurringContribution, array_fill_keys($recurFields, 1)),
-      $overrides
-    ), 'strlen');
+    $overrides = array_filter([
+      'is_test' => $inputOverrides['is_test'] ?? $recurringContribution['is_test'],
+      'financial_type_id' => $inputOverrides['financial_type_id'] ?? $recurringContribution['financial_type_id'],
+      'campaign_id' => $inputOverrides['campaign_id'] ?? $recurringContribution['campaign_id'],
+      'total_amount' => $inputOverrides['total_amount'] ?? $recurringContribution['amount'],
+    ], 'strlen');
 
     // First look for new-style template contribution with is_template=1
     $templateContributions = Contribution::get(FALSE)
@@ -546,8 +546,8 @@ INNER JOIN civicrm_contribution       con ON ( con.id = mp.contribution_id )
       // The handling of the line items is managed in BAO_Order so this
       // is whether we should override on the contribution. Arguably the 2 should
       // be decoupled.
-      if (count($lineItems) > 1 && isset($overrides['financial_type_id'])) {
-        unset($overrides['financial_type_id']);
+      if (count($lineItems) > 1) {
+        unset($overrides['financial_type_id'], $overrides['total_amount']);
       }
       $result = array_merge($templateContribution, $overrides);
       // Line items aren't always written to a contribution, for mystery reasons.
index e98ea9c42139ca639fdbdb49442de21e1e10410c..6c918dce1422a4f9fc4decebb9f261c74b91fa2c 100644 (file)
@@ -1 +1,9 @@
 {* file to handle db changes in 5.52.alpha1 during upgrade *}
+--  Update any recurring contributions to have the same amount
+-- as the recurring template contribution if it exists.
+-- Some of these got out of sync over recent changes.
+UPDATE civicrm_contribution_recur r
+INNER JOIN civicrm_contribution c ON contribution_recur_id = r.id
+AND c.is_template = 1
+SET amount = total_amount
+WHERE total_amount IS NOT NULL;
index e8dfd90d195267747fe149fd897d8023a94002ae..39fcdf530403cdae57cc96131dcf49878ff696ae 100644 (file)
@@ -2769,6 +2769,16 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
 
     unset($expectedLineItem['id'], $expectedLineItem['entity_id'], $lineItem2['values'][0]['id'], $lineItem2['values'][0]['entity_id']);
     $this->assertEquals($expectedLineItem, $lineItem2['values'][0]);
+
+    $this->callAPISuccess('contribution', 'repeattransaction', [
+      'original_contribution_id' => $originalContribution['id'],
+      'contribution_status_id' => 'Completed',
+      'trxn_id' => 789,
+    ]);
+    $this->callAPISuccessGetSingle('contribution', [
+      'total_amount' => 400,
+      'trxn_id' => 789,
+    ]);
   }
 
   /**