Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM-21027 Fix next recurring payment not accurately calculated #10818

Merged
merged 1 commit into from
Aug 7, 2017

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Aug 4, 2017

Overview

This pull request fixes a bug that entered the code base in May, causing payment processors that rely on CiviCRM updating the next_sched_contribution_date for recurring to not get it updated.

Before

When payments are completed using the completetransaction api with a receive_date passed in with a time component the date for the next scheduled contribution is not updated. This is a critical error to sites affected as customers will be recharged, but mitigated by the fact that relatively few use the combination of components (in my case DPS / Omnipay + recurring payments)

After

After processing a payment the next_sched_contribution_date is correctly updated

Technical Details

Rather than comparing effectiveDate with today, the date part of effectiveDate ('Y-m-d') is compared with today

Comments

I was able to replicate this in a unit test before fixing it & committing the test


@eileenmcnaughton eileenmcnaughton changed the title Recur date CRM-21027 Fix next recurring payment not accurately calculated Aug 4, 2017
@@ -99,15 +99,15 @@ public function testCancelRecur() {
*/
public function testSupportFinancialTypeChange() {
$contributionRecur = $this->callAPISuccess('contribution_recur', 'create', $this->_params);
$contribution = $this->callAPISuccess('contribution', 'create', array(
$this->callAPISuccess('contribution', 'create', array(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these changes are unrelated - I noticed these minor errors while looking in the wrong file

@petednz
Copy link

petednz commented Aug 4, 2017

In 'before' it says "as customers will be recharged". Is there a 'not' missing from that statement

@eileenmcnaughton
Copy link
Contributor Author

@petednz no - there isn't! I narrowly avoiding multi-charging yes customers due there being another bug in the Omnipay extensions meaning they weren't charged at all for 4 days

@monishdeb
Copy link
Member

The fix makes sense. Tested, working fine. Also the added use-case in unit test is accurate. Merging now.

@monishdeb monishdeb merged commit 66b8999 into civicrm:4.7.24-rc Aug 7, 2017
@eileenmcnaughton eileenmcnaughton deleted the recur_date branch August 9, 2017 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants