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

Update latest sdkv3 #65

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Update latest sdkv3 #65

wants to merge 9 commits into from

Conversation

pkuma525
Copy link

updated the code to latest sdk for credit card payment and check payment

@pkuma525 pkuma525 requested a review from slogsdon October 31, 2019 11:27
Copy link
Contributor

@slogsdon slogsdon left a comment

Choose a reason for hiding this comment

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

Notes added. I also do not see any changes around recurring payments. There will be class/method changes for recurring as well as one-time card/check payments.

$token = new HpsTokenData();
$token->tokenValue = ($response != null
? $response->token_value
: '');
Copy link
Contributor

Choose a reason for hiding this comment

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

Moving this into buildCardHolder and not renaming the method/variable is confusing from a maintenance perspective.

Copy link
Author

Choose a reason for hiding this comment

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

updated

->withCurrency($currency)
->withAddress($address)
->withAllowDuplicates(true)
->execute();
Copy link
Contributor

Choose a reason for hiding this comment

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

The data within $secureEcommerce and the level II / corporate purchasing card flag ($cpcReq) are no long included in the original authorization request.

Copy link
Author

Choose a reason for hiding this comment

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

updated

Copy link
Contributor

Choose a reason for hiding this comment

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

@pkuma525 I may not have conveyed the right meaning here. The $secureEcommerce and $cpcReq values can still be used in the new SDK, but your original changes did not add these values to the authorization builder, causing them to not be included in the request to the gateway. We need both to be included in the gateway request to leverage both programs.

Copy link
Author

Choose a reason for hiding this comment

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

updated as per review comment

->withCurrency($currency)
->withAddress($address)
->withAllowDuplicates(true)
->execute();
Copy link
Contributor

Choose a reason for hiding this comment

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

The data within $secureEcommerce and the level II / corporate purchasing card flag ($cpcReq) are no long included in the original authorization request.

Copy link
Author

Choose a reason for hiding this comment

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

updated

Copy link
Contributor

Choose a reason for hiding this comment

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

@pkuma525 I may not have conveyed the right meaning here. The $secureEcommerce and $cpcReq values can still be used in the new SDK, but your original changes did not add these values to the authorization builder, causing them to not be included in the request to the gateway. We need both to be included in the gateway request to leverage both programs.

Copy link
Author

Choose a reason for hiding this comment

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

updated as per review comment

@pkuma525 pkuma525 requested a review from slogsdon November 15, 2019 04:05
Copy link
Contributor

@slogsdon slogsdon left a comment

Choose a reason for hiding this comment

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

Let's rename buildCardHolder and buildCheckHolder and make sure they're being used correctly. The new SDK doesn't have a notion of a card/check holder object as this data now lives on either the payment data object (CreditCardData or ECheck) or on the Address object. We also need to ensure that the correct payment data object is being used to initiate the transaction.

@@ -1688,9 +1678,9 @@ private function buildCardHolder($feed, $submission_data, $entry)
*/
private function buildCheckHolder($feed, $submission_data, $entry)
{
$checkHolder = new HpsCheckHolder();
$checkHolder = new ECheck();
$checkHolder->address = $this->buildAddress($feed, $submission_data, $entry);
Copy link
Contributor

Choose a reason for hiding this comment

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

There should not be an address property on ECheck, so this line can be removed once the address is added to the authorization builder for check transactions.

Copy link
Author

Choose a reason for hiding this comment

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

updated as per review comment

$checkHolder->address = $this->buildAddress($feed, $submission_data, $entry);
$checkHolder->checkName = htmlspecialchars(rgar($submission_data, 'ach_check_holder')); //'check holder';
$checkHolder->checkHolderName = htmlspecialchars(rgar($submission_data, 'ach_check_holder')); //'check holder';
Copy link
Contributor

Choose a reason for hiding this comment

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

This should still be checkName. The checkHolderName property isn't mapped to the correct gateway field.

Copy link
Author

Choose a reason for hiding this comment

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

updated as per review comment

$config->versionNumber = '1916';*/
$config = new ServicesConfig();
$config->secretApiKey = $key;
$config->serviceUrl = "https://cert.api2.heartlandportico.com";
Copy link
Contributor

Choose a reason for hiding this comment

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

The configuration needs to support production as well. We can check the key to determine which environment should be used.

Copy link
Author

Choose a reason for hiding this comment

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

updated as per review comment

@pkuma525 pkuma525 requested a review from slogsdon April 9, 2020 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants