dev/core#1679: Ensure Paypal IPN always updates the next scheduled payment date

This is from https://github.com/civicrm/civicrm-core/pull/16952 and https://github.com/civicrm/civicrm-core/pull/17028

is to not change the scheduled date if it seems to have already been intentionally changed. However, per #16952
time stamps can lead to 'now' being a day in the future - so this loosens the check from equals today to
is less then now + 1 day

diff --git a/CRM/Contribute/BAO/ContributionRecur.php b/CRM/Contribute/BAO/ContributionRecur.php
index 5862d4b777ba9ac39187b05eb707c5fcf69f0867..1b03e98f0abc406cdbbb237166b61dd54ab47dc5 100644 (file)
--- a/CRM/Contribute/BAO/ContributionRecur.php
+++ b/CRM/Contribute/BAO/ContributionRecur.php
@@ -835,7 +835,6 @@ INNER JOIN civicrm_contribution       con ON ( con.id = mp.contribution_id )
    */
   public static function updateOnNewPayment($recurringContributionID, $paymentStatus, $effectiveDate) {
 
-    $effectiveDate = $effectiveDate ? date('Y-m-d', strtotime($effectiveDate)) : date('Y-m-d');
     if (!in_array($paymentStatus, ['Completed', 'Failed'])) {
       return;
     }
@@ -864,12 +863,18 @@ INNER JOIN civicrm_contribution       con ON ( con.id = mp.contribution_id )
 
     if (!empty($existing['installments']) && self::isComplete($recurringContributionID, $existing['installments'])) {
       $params['contribution_status_id'] = 'Completed';
+      $params['next_sched_contribution_date'] = 'null';
     }
     else {
-      // Only update next sched date if it's empty or 'just now' because payment processors may be managing
-      // the scheduled date themselves as core did not previously provide any help.
-      if (empty($existing['next_sched_contribution_date']) || strtotime($existing['next_sched_contribution_date']) ==
-        strtotime($effectiveDate)) {
+      // Only update next sched date if it's empty or up to 48 hours away because payment processors may be managing
+      // the scheduled date themselves as core did not previously provide any help. This check can possibly be removed
+      // as it's unclear if it actually is helpful...
+      // We should allow payment processors to pass this value into repeattransaction in future.
+      // Note 48 hours is a bit aribtrary but means that we can hopefully ignore the time being potentially
+      // rounded down to midnight.
+      $upperDateToConsiderProcessed = strtotime('+ 48 hours', ($effectiveDate ? strtotime($effectiveDate) : time()));
+      if (empty($existing['next_sched_contribution_date']) || strtotime($existing['next_sched_contribution_date']) <=
+        $upperDateToConsiderProcessed) {
         $params['next_sched_contribution_date'] = date('Y-m-d', strtotime('+' . $existing['frequency_interval'] . ' ' . $existing['frequency_unit'], strtotime($effectiveDate)));
       }
     }

diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
index 5a21149c800d03d4aaa544f480f329204e3fdbbe..b5779446337865592541cd580a48ffa8b3c883dc 100644 (file)
--- a/tests/phpunit/api/v3/ContributionTest.php
+++ b/tests/phpunit/api/v3/ContributionTest.php
@@ -4597,4 +4597,145 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
     $contribution = $this->callAPISuccess('Contribution', 'Create', $params);
   }
 
+  /**
+   * Test repeatTransaction with installments and next_sched_contribution_date
+   *
+   * @dataProvider getRepeatTransactionNextSchedData
+   *
+   * @param array $dataSet
+   *
+   * @throws \Exception
+   */
+  public function testRepeatTransactionUpdateNextSchedContributionDate($dataSet) {
+    $paymentProcessorID = $this->paymentProcessorCreate();
+    // Create the contribution before the recur so it doesn't trigger the update of next_sched_contribution_date
+    $contribution = $this->callAPISuccess('contribution', 'create', array_merge(
+        $this->_params,
+        [
+          'contribution_status_id' => 'Completed',
+          'receive_date' => $dataSet['repeat'][0]['receive_date'],
+        ])
+    );
+    $contributionRecur = $this->callAPISuccess('contribution_recur', 'create', array_merge([
+      'contact_id' => $this->_individualId,
+      'frequency_interval' => '1',
+      'amount' => '500',
+      'contribution_status_id' => 'Pending',
+      'start_date' => '2012-01-01 00:00:00',
+      'currency' => 'USD',
+      'frequency_unit' => 'month',
+      'payment_processor_id' => $paymentProcessorID,
+    ], $dataSet['recur']));
+    // Link the existing contribution to the recur *after* creating the recur.
+    // If we just created the contribution now the next_sched_contribution_date would be automatically set
+    //   and we want to test the case when it is empty.
+    $this->callAPISuccess('contribution', 'create', [
+      'id' => $contribution['id'],
+      'contribution_recur_id' => $contributionRecur['id'],
+    ]);
+
+    $contributionRecur = $this->callAPISuccessGetSingle('ContributionRecur', [
+      'id' => $contributionRecur['id'],
+      'return' => ['next_sched_contribution_date', 'contribution_status_id'],
+    ]);
+    // Check that next_sched_contribution_date is empty
+    $this->assertEquals('', $contributionRecur['next_sched_contribution_date'] ?? '');
+
+    $this->callAPISuccess('Contribution', 'repeattransaction', [
+      'contribution_status_id' => 'Completed',
+      'contribution_recur_id' => $contributionRecur['id'],
+      'receive_date' => $dataSet['repeat'][0]['receive_date'],
+    ]);
+    $contributionRecur = $this->callAPISuccessGetSingle('ContributionRecur', [
+      'id' => $contributionRecur['id'],
+      'return' => ['next_sched_contribution_date', 'contribution_status_id'],
+    ]);
+    // Check that recur has status "In Progress"
+    $this->assertEquals(
+      (string) CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_ContributionRecur', 'contribution_status_id', $dataSet['repeat'][0]['expectedRecurStatus']),
+      $contributionRecur['contribution_status_id']
+    );
+    // Check that next_sched_contribution_date has been set to 1 period after the contribution receive date (ie. 1 month)
+    $this->assertEquals($dataSet['repeat'][0]['expectedNextSched'], $contributionRecur['next_sched_contribution_date']);
+
+    // Now call Contribution.repeattransaction again and check that the next_sched_contribution_date has moved forward by 1 period again
+    $this->callAPISuccess('Contribution', 'repeattransaction', [
+      'contribution_status_id' => 'Completed',
+      'contribution_recur_id' => $contributionRecur['id'],
+      'receive_date' => $dataSet['repeat'][1]['receive_date'],
+    ]);
+    $contributionRecur = $this->callAPISuccessGetSingle('ContributionRecur', [
+      'id' => $contributionRecur['id'],
+      'return' => ['next_sched_contribution_date', 'contribution_status_id'],
+    ]);
+    // Check that recur has status "In Progress" or "Completed" depending on whether number of installments has been reached
+    $this->assertEquals(
+      (string) CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_ContributionRecur', 'contribution_status_id', $dataSet['repeat'][1]['expectedRecurStatus']),
+      $contributionRecur['contribution_status_id']
+    );
+    // Check that next_sched_contribution_date has been set to 1 period after the contribution receive date (ie. 1 month)
+    $this->assertEquals($dataSet['repeat'][1]['expectedNextSched'], $contributionRecur['next_sched_contribution_date'] ?? '');
+  }
+
+  /**
+   * Get dates for testing.
+   *
+   * @return array
+   */
+  public function getRepeatTransactionNextSchedData() {
+    // Both these tests handle/test the case that next_sched_contribution_date is empty when Contribution.repeattransaction
+    //   is called for the first time. Historically setting it was inconsistent but on new updates it should always be set.
+    /*
+     * This tests that calling Contribution.repeattransaction with installments does the following:
+     * - For the first call to repeattransaction the recur status is In Progress and next_sched_contribution_date is updated
+     *   to match next expected receive_date.
+     * - Once the 3rd contribution is created contributionRecur status = completed and next_sched_contribution_date = ''.
+     */
+    $result['receive_date_includes_time_with_installments']['2012-01-01-1-month'] = [
+      'recur' => [
+        'start_date' => '2012-01-01',
+        'frequency_interval' => 1,
+        'installments' => '3',
+        'frequency_unit' => 'month',
+      ],
+      'repeat' => [
+        [
+          'receive_date' => '2012-02-29 16:00:00',
+          'expectedNextSched' => '2012-03-29 00:00:00',
+          'expectedRecurStatus' => 'In Progress',
+        ],
+        [
+          'receive_date' => '2012-03-29 16:00:00',
+          'expectedNextSched' => '',
+          'expectedRecurStatus' => 'Completed',
+        ],
+      ],
+    ];
+    /*
+     * This tests that calling Contribution.repeattransaction with no installments does the following:
+     * - For the each call to repeattransaction the recur status is In Progress and next_sched_contribution_date is updated
+     *   to match next expected receive_date.
+     */
+    $result['receive_date_includes_time_no_installments']['2012-01-01-1-month'] = [
+      'recur' => [
+        'start_date' => '2012-01-01',
+        'frequency_interval' => 1,
+        'frequency_unit' => 'month',
+      ],
+      'repeat' => [
+        [
+          'receive_date' => '2012-02-29 16:00:00',
+          'expectedNextSched' => '2012-03-29 00:00:00',
+          'expectedRecurStatus' => 'In Progress',
+        ],
+        [
+          'receive_date' => '2012-03-29 16:00:00',
+          'expectedNextSched' => '2012-04-29 00:00:00',
+          'expectedRecurStatus' => 'In Progress',
+        ],
+      ],
+    ];
+    return $result;
+  }
+
 }