-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add conditionals for optional fields to PxPay request #26
Conversation
Copied the existing code for the 3 TxnData fields from PxPayAuthorizeRequest.php and added to PxPostAuthorizeRequest.php.
MerchantReference, TxnId, TxnData1, TxnData2, and TxnData3 are all optional fields so instead of sending empty strings if they're not set, don't include them in the request at all.
MerchantReference, TxnId, TxnData1, TxnData2, and TxnData3 are all optional fields so instead of sending empty strings if they're not set, don't include them in the request at all.
When I branched, it already included my PxPost changes. Once the PxPost changes have been merged, I'll resubmit the PxPay changes. Sorry. |
@gdhnz thanks for this! Can you add some tests to cover it, please? |
Is updating testCompletePurchaseWithTransactionDataSuccess in PxPayCompletePurchaseSuccess.php with additional response tests for TxnData1, TxnData2, and TxnData3 enough? I can also add tests to testAuthorizeWithTransactionDataSuccess to check the request. |
Tests added to testAuthorizeWithTransactionDataSuccess to check the request and to testCompletePurchaseWithTransactionDataSuccess to check the response.
Add PxPay test if cardReference, pxPostUsername, and pxPostPassword are set.
Merged. Can someone please confirm that this works in production and then I will tag for release? |
I've confirmed this works in production. |
MerchantReference, TxnId, TxnData1, TxnData2, and TxnData3 are all optional fields so instead of sending empty strings if they're not set, don't include them in the request at all.