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

Paymentez: Correct refund and void message parsing #3509

Merged

Conversation

Carrigan
Copy link
Contributor

Why?

{"status": "success", "detail": "Completed"}

What Changed?

  • Changed the Paymentez unit test void/refund payload shape to match the specs.
  • Added checks on Paymentez unit and remote tests for nil message values.
  • Added proper message handling for Paymentez void and refunds.

Test Validation

To capture the failures, the following tests were added to check for a non-nil value for the and failed at first:

Failure: test_successful_void(RemotePaymentezTest): <nil> was expected to not be nil.
Failure: test_successful_void_with_elo(RemotePaymentezTest): <nil> was expected to not be nil.
Failure: test_successful_refund(RemotePaymentezTest): <nil> was expected to not be nil.
Failure: test_successful_refund_with_elo(RemotePaymentezTest): <nil> was expected to not be nil.
Failure: test_failed_refund(PaymentezTest): <nil> was expected to not be nil.
Failure: test_failed_void(PaymentezTest): <nil> was expected to not be nil.
Failure: test_partial_refund(PaymentezTest): <nil> was expected to not be nil.
Failure: test_successful_void(PaymentezTest): <nil> was expected to not be nil.

This PR includes the changes to make these tests pass.

Test Suite Results

Unit tests:

23 tests, 99 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote tests:

26 tests, 64 assertions, 2 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
92.3077% passed

Note: the two failing remote tests are not related to these changes and are failing on master:

  • test_partial_capture
  • test_successful_authorize_and_capture_with_different_amount

@Carrigan Carrigan requested a review from a team January 22, 2020 21:56
@@ -157,6 +157,7 @@ def test_partial_refund
assert_match(/"amount":1.0/, data)
end.respond_with(successful_refund_response)
assert_success response
assert_not_nil response.message
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's worth checking the actual content of the message field throughout the unit tests since we're specifying the responses.

Suggested change
assert_not_nil response.message
assert_equal 'Completed', response.message

@Carrigan
Copy link
Contributor Author

Added fixes above. Test suite has not changed:

Unit:

23 tests, 99 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote:

26 tests, 64 assertions, 2 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
92.3077% passed

Copy link
Contributor

@leila-alderman leila-alderman left a comment

Choose a reason for hiding this comment

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

Looks good! 💯

@Carrigan
Copy link
Contributor Author

Thanks @leila-alderman

CE-236

The message field for Paymentez refund and void transactions was not
being populated correctly because it is shaped differently from other
Paymentez transactions. This change adds unit and remote tests to
check for a `nil` message value and corrects message parsing for these
operations.

Add explicit response message checking per activemerchant#3509.

Add explicit response message checking to remote tests per activemerchant#3509.
@Carrigan Carrigan force-pushed the CE-236-fix-paymentez-credit-message branch from f79c350 to 5b01ae7 Compare January 27, 2020 19:46
@Carrigan Carrigan merged commit 5b01ae7 into activemerchant:master Jan 27, 2020
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.

2 participants