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

Jump to bottom

SagePay: Use VPSTxId from authorization for refunds #2489

Closed
wants to merge 1 commit into from
Closed

SagePay: Use VPSTxId from authorization for refunds #2489

wants to merge 1 commit into from

Conversation

dtykocki
Copy link

SagePay does not return the VPSTxId field in the response of
successful capture transactions. For refunds, the VPSTxId field from
authorization transactions must be used instead.

SagePay: Use VPSTxId from authorization for refunds
b8773ac 
SagePay does not return the `VPSTxId` field in the response of
successful capture transactions. For refunds, the `VPSTxId` field from
authorization transactions must be used instead.
@dtykocki
Copy link
Author

@davidsantoso / @curiousepic

@davidsantoso
Copy link
Member

davidsantoso commented Jun 30, 2017 •
edited
Loading

@dtykocki mind dropping the remote test run suite results in the original PR comment for posterity sake? Unfortunately I can't run them and add since it looks like there's IP whitelisting involved 😿

@dtykocki
Copy link
Author

@davidsantoso - I intended to, but the remote tests will not fully complete. I was forced to drop in a sleep 200 (sometimes 300) between each HTTP request to SagePage. Even with the sleep, some remote tests fail with 2002 : The Authorisation timed out.. If there's some secret handshake I need to perform with SagePay, I'd love to know what that is at this point.

@davidsantoso
Copy link
Member

@dtykocki ah gotcha. The typical gateway sandbox quirk! Alrighty, overall this looks good since using the || should keep previous transactions with the VPSTxId in the response the same. 👍

@davidsantoso davidsantoso self-requested a review June 30, 2017 17:37
davidsantoso
davidsantoso approved these changes Jun 30, 2017
View reviewed changes
Copy link
Member

@davidsantoso davidsantoso left a comment

Choose a reason for hiding this comment

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

🌮 !

@curiousepic
Copy link
Contributor

LG2M2 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidsantoso davidsantoso davidsantoso approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dtykocki @davidsantoso @curiousepic