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.

Sign up for GitHub

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

Convert Paypal Standard IPN payment_date to system's time zone #13439

Merged
merged 3 commits into from
Jan 16, 2019
Merged

Convert Paypal Standard IPN payment_date to system's time zone #13439

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f045e2e
Convert Paypal Standard IPN payment_date to system's time zone
jgillmanjr Jan 12, 2019
e878028
Utilize getTimeZoneString() method
seamuslee001 Jan 12, 2019
6800eef
Quick comment describing need for IPN TZ adjustment
jgillmanjr Jan 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CRM/Core/Payment/PayPalIPN.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,12 @@ public function getInput(&$input, &$ids) {
$paymentDate = $this->retrieve('payment_date', 'String', FALSE);
if (!empty($paymentDate)) {
$receiveDateTime = new DateTime($paymentDate);
Copy link
Contributor

Choose a reason for hiding this comment

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

won't you have to specify that its in Pacific time on creation or is it doing that sensibly already? Also do we need to do this for PayPalPro IPNs as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is from one of the test IPN messages: payment_date=22:08:43 Jan 11, 2019 PST
But yeah, if the time zone wasn't supplied, this wouldn't work.

Pro does look to require it as well, but the code structure for that was... a bit different:

https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Payment/PayPalProIPN.php#L318

    // CRM-13737 - am not aware of any reason why payment_date would not be set - this if is a belt & braces
    $objects['contribution']->receive_date = !empty($input['payment_date']) ? date('YmdHis', strtotime($input['payment_date'])) : $now;

and https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Payment/PayPalProIPN.php#L534:

$input['payment_date'] = $input['receive_date'] = self::retrieve('payment_date', 'String', 'POST', FALSE);

Since I admittedly don't have a setup for "properly" testing things, and don't have access (and have never used) PayPal pro, I didn't feel comfortable taking a completely dark stab - especially with payment_date being referenced in two locations.

Copy link
Contributor

Choose a reason for hiding this comment

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

See also https://developer.paypal.com/docs/classic/ipn/integration-guide/IPNandPDTVariables/#payment-information-variables

/**
* The `payment_date` that Paypal sends back is in their timezone. Example return: 08:23:05 Jan 11, 2019 PST
* Subsequently, we need to account for that, otherwise the recieve time will be incorrect for the local system
*/
$systemTimeZone = new DateTimeZone(CRM_Core_Config::singleton()->userSystem->getTimeZoneString());
Copy link
Contributor

Choose a reason for hiding this comment

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

@jgillmanjr Can you add a couple of lines of comments into the code here explaining why we need to do the timezone conversion (so anyone looking at this code in the future can work out what is going on and why). Eg. add a sample payment_date as it comes in from paypal etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comments in 292785c

$receiveDateTime->setTimezone($systemTimeZone);
$input['receive_date'] = $receiveDateTime->format('YmdHis');
}
}
Expand Down