From: eileen Date: Fri, 3 Jul 2020 08:41:29 +0000 (+1200) Subject: dev/core#1679: Ensure Paypal IPN always updates the next scheduled payment date X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=605665c7a3c4804ba8f453341763ffd74a0d48b9;p=civicrm-core.git 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 5862d4b777..1b03e98f0a 100644 --- 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 5a21149c80..b577944633 100644 --- 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; + } + }