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

dev/core#1679: Ensure Paypal IPN always updates the next scheduled p… #16952

Conversation

andrew-cormick-dockery
Copy link
Contributor

@andrew-cormick-dockery andrew-cormick-dockery commented Apr 2, 2020

…ayment date when recording the latest payment

Overview

This PR aims to resolve issue dev/core 1679 by getting the Paypal IPN to always update the next scheduled contribution date for recurring payments.

Before

The Paypal IPN did not update the next scheduled contribution date, thus causing the responsibility for this action to fall to the default algorithm, which is faulty. That algorithm also fails to update this information should the transaction fall after midnight local time. After the first failure, this algorithm never allows for the ongoing update of this date.

After

The Paypal IPN now updates the next scheduled contribution date regardless of the timing of the actual contribution.

Technical Details

This solution avoids solving the actual problem, which is the faulty default algorithm to determine the next scheduled contribution date. While this PR solves the problem for Paypal, it's possible that IPNs for other payment processors may run into the same trouble by relying on this unreliable default algorithm. This algorithm appears to be on lines 861 to 865 of CRM/Contribute/BAO/ContributionRecur.php. It has the following comments, which I believe are accurate: "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."

To me, it appears that the algorithm is trying to determine whether or not the payment processor is managing the next scheduled date by comparing the receive date of the transaction to the next scheduled date. I believe there ought to be a more direct way of informing whether or not the payment processor is managing this date rather than inferring that possibility in that (faulty) way.

However, I cannot personally think of a more suitable algorithm, so therefore I am proposing avoiding it altogether for Paypal, which suits our purposes.

…ayment date when recording the latest payment
@civibot
Copy link

civibot bot commented Apr 2, 2020

(Standard links)

@eileenmcnaughton
Copy link
Contributor

Note comment #17028 (comment)

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 3, 2020
…yment date

This is from civicrm#16952 and civicrm#17028

is to not change the scheduled date if it seems to have already been intentionally changed. However, per civicrm#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
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 3, 2020
…yment date

This is from civicrm#16952 and civicrm#17028

is to not change the scheduled date if it seems to have already been intentionally changed. However, per civicrm#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
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 3, 2020
…yment date

This is from civicrm#16952 and civicrm#17028

is to not change the scheduled date if it seems to have already been intentionally changed. However, per civicrm#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
@eileenmcnaughton
Copy link
Contributor

@andrew-cormick-dockery I think this will resolve your issue - #17744

@eileenmcnaughton
Copy link
Contributor

This should be addressed through #17744

christianwach pushed a commit to christianwach/civicrm-core that referenced this pull request Jul 18, 2020
…yment date

This is from civicrm#16952 and civicrm#17028

is to not change the scheduled date if it seems to have already been intentionally changed. However, per civicrm#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants