Skip to content
This repository has been archived by the owner on Oct 30, 2020. It is now read-only.

Response success logic #4

Merged
merged 3 commits into from
Dec 28, 2016

Conversation

tappleby
Copy link
Collaborator

@tappleby tappleby commented Dec 9, 2016

As we were discussing previously failed CVD / AVS response doesnt necessarily mean the transaction has failed. After fighting with the moneris API more and reviewing some other libraries I believe their might be a bug with the logic for a successful response.

This PR updates the successful flag to only check if the receipt was complete + if the response code is in the range from 0-49 (Financial Response Codes).

The failed AVS + CVD flags are still being set but its up to the merchant to decide how they want to handle these (eg. void purchase in application code, manual process etc).

Some other examples of response handling out in the wild:

I was hoping to add some more functional test for Response but the moneris test APIs like to return odd result later in the day. Will try again in the morning and see what gets returned for the Penny values.

@craigpaul
Copy link
Owner

craigpaul commented Dec 22, 2016

Hey @tappleby, sorry, I never got a notification for this pull request. Is this good to go? I know the tests are failing, but thats most likely the fault of Moneris and the fact that these tests have to make actual requests to the api, making them super fragile. Notice they are passing now since I ran them one at a time. I changed Travis to limit it to 1 concurrent build at a time so these tests shouldn't be racing against each other anymore. Let me know and I'll merge it.

Also, does this require any documentation updates?

@tappleby
Copy link
Collaborator Author

Meant to try again, can you try and retigger the Travis CI test for this PR? I think only owners can do it.

@craigpaul
Copy link
Owner

@tappleby Ah I did not know that. I triggered the build again and it completed successfully.

@craigpaul craigpaul merged commit cb3b75e into craigpaul:master Dec 28, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants