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

FlexCharge: Adding support fot FlexCharge gateway #5108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Heavyblade
Copy link
Collaborator

Summary:

This PR adds support for FlexCharge gateway adding the evaluate and refund operations

SER-1130

Tests

Remote Test:

Finished in 39.651458 seconds.
12 tests, 40 assertions, 1 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
91.6667% passed

Note:
Failing test happens because of account limit on refunds

Unit Tests:

Finished in 33.453236 seconds.
5855 tests, 79334 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

RuboCop:

795 files inspected, no offenses detected

Copy link
Collaborator

@gasb150 gasb150 left a comment

Choose a reason for hiding this comment

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

@Heavyblade good job! I just left a few questions to clarify my understanding.

lib/active_merchant/billing/gateways/flex_charge.rb Outdated Show resolved Hide resolved
end

def add_base_data(post, options)
post[:isDeclined] = cast_bool(options[:is_declined])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where comes this value that you should send a boolean, but it could be a set of different "false" related values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cast_bool is meant to boleans that come from GSFS, so I wanted to go full defensive covering different cases.

lib/active_merchant/billing/gateways/flex_charge.rb Outdated Show resolved Hide resolved
lib/active_merchant/billing/gateways/flex_charge.rb Outdated Show resolved Hide resolved
Summary:
------------------------------
This PR adds support for FlexCharge gateway adding the
evaluate and refund operations

[SER-1130](https://spreedly.atlassian.net/browse/SER-1130)

Remote Test:
------------------------------
Finished in 39.651458 seconds.
12 tests, 40 assertions, 1 failures, 0 errors,
0 pendings, 0 omissions, 0 notifications
91.6667% passed

*Note:*
Failing test happens because of account limit on refunds

Unit Tests:
------------------------------
Finished in 33.453236 seconds.
5855 tests, 79334 assertions, 0 failures, 0 errors,
0 pendings, 0 omissions, 0 notifications
100% passed

RuboCop:
------------------------------
795 files inspected, no offenses detected
Copy link
Contributor

@naashton naashton left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me. Only a couple of comments for your consideration.

end

def refund(money, authorization, options = {})
commit(:refund, { amountToRefund: (money.to_f / 100).round(2) }, authorization)
Copy link
Contributor

Choose a reason for hiding this comment

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

For the this money formatting, there is a helper method in the gateway.rb class called def localized_amount that you might be able to use instead.

Comment on lines +56 to +58
gsub(%r(("mid\\?"\s*:\s*\\?")[^"]*)i, '\1[FILTERED]').
gsub(%r(("siteId\\?"\s*:\s*\\?")[^"]*)i, '\1[FILTERED]').
gsub(%r(("environment\\?"\s*:\s*\\?")[^"]*)i, '\1[FILTERED]').
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to scrub these?

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.

None yet

3 participants