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

Bring in payment model #5678

Closed
wants to merge 30 commits into from
Closed

Bring in payment model #5678

Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
06aa561
Bring in Payment model from Spree
sauloperez Jun 26, 2020
abacd06
Fix credit card instance in specs
sauloperez Jun 26, 2020
e1ea5db
Fix all but the 7 last payment specs
sauloperez Jun 26, 2020
34de219
Bring in missing translation
sauloperez Jun 26, 2020
a01f601
Fix yet another spec
sauloperez Jun 26, 2020
0ad8dcc
Fix payment log entries specs
sauloperez Jun 26, 2020
9935df9
Move Pin payment method from decorator into model
sauloperez Jun 26, 2020
31d0d4b
Fix error "no parent is saved"
sauloperez Jun 26, 2020
eafaa97
Temporarily skip spec
sauloperez Jun 26, 2020
322c4d0
Move decorator's callbacks to model
sauloperez Jun 26, 2020
6d9a518
Move method from decorator to model
sauloperez Jun 26, 2020
48910ae
Move #refund! to the processing.rb
sauloperez Jun 26, 2020
8617262
Move localize_number from decorator to model
sauloperez Jun 26, 2020
3fb6193
Move adjustments logic from decorator into model
sauloperez Jun 26, 2020
cf6138d
Replace model method with its decorated version
sauloperez Jun 26, 2020
d49068c
Move method delegation from decorator to model
sauloperez Jun 26, 2020
d8b748a
Merge alias_method method and its original version
sauloperez Jun 26, 2020
8fbbb0b
Bring back our card factory modification
sauloperez Jul 10, 2020
562f397
Isolate Spree's specs into their own context
sauloperez Jul 10, 2020
2f46483
Merge decorator specs with Spree's ones
sauloperez Jul 10, 2020
6837946
Rename spec file
sauloperez Jul 10, 2020
55d52b8
Run rubocop autocorrect on payment model
sauloperez Jul 10, 2020
cf64d3a
Merge skipped callback from decorator into model
sauloperez Jul 10, 2020
3435d5a
Fix Rubocop non-metrics issues in payment model
sauloperez Jul 10, 2020
66dbd85
Run rubocop autocorrect on payment/processing.rb
sauloperez Jul 10, 2020
42658b5
Refactor `#process!` nested ifs to guard clauses
sauloperez Jul 13, 2020
a8af3a2
Fix all but Metrics Rubocop cops in processing.rb
sauloperez Jul 13, 2020
3a64cc4
Reuse #calculate_refund_amount method
sauloperez Jul 13, 2020
70afcee
Fix Spree's spec clashing with a customization
sauloperez Jul 15, 2020
dd5e679
Address code review comments
sauloperez Jul 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 68 additions & 33 deletions app/models/spree/payment/processing.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,18 @@ def capture!
protect_from_connection_error do
check_environment

if payment_method.payment_profiles_supported?
# Gateways supporting payment profiles will need access to credit card object because this stores the payment profile information
# so supply the authorization itself as well as the credit card, rather than just the authorization code
response = payment_method.capture(self, source, gateway_options)
else
# Standard ActiveMerchant capture usage
response = payment_method.capture(money.money.cents,
response = if payment_method.payment_profiles_supported?
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont like this rubocop rule, am I the only one? I always find the result to look worse than before and the solution is always to extract the if clause to a method. I am not requesting any change, just a comment.

Copy link
Contributor Author

@sauloperez sauloperez Jul 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I don't have a strong opinion on this. I do find it a bit more readable when nested because otherwise the assignment is easy to miss. As many things in Ruby though, abusing it by having a long if/else beats readability anyway, so extracting a method in those case makes sense.

# Gateways supporting payment profiles will need access to credit
# card object because this stores the payment profile information
# so supply the authorization itself as well as the credit card,
# rather than just the authorization code
payment_method.capture(self, source, gateway_options)
else
# Standard ActiveMerchant capture usage
payment_method.capture(money.money.cents,
response_code,
gateway_options)
end
end

handle_response(response, :complete, :failure)
end
Expand All @@ -60,14 +62,17 @@ def void_transaction!
protect_from_connection_error do
check_environment

if payment_method.payment_profiles_supported?
# Gateways supporting payment profiles will need access to credit card object because this stores the payment profile information
# so supply the authorization itself as well as the credit card, rather than just the authorization code
response = payment_method.void(response_code, source, gateway_options)
else
# Standard ActiveMerchant void usage
response = payment_method.void(response_code, gateway_options)
end
response = if payment_method.payment_profiles_supported?
# Gateways supporting payment profiles will need access to credit
# card object because this stores the payment profile information
# so supply the authorization itself as well as the credit card,
# rather than just the authorization code
payment_method.void(response_code, source, gateway_options)
else
# Standard ActiveMerchant void usage
payment_method.void(response_code, gateway_options)
end

record_response(response)

if response.success?
Expand All @@ -83,14 +88,28 @@ def credit!(credit_amount = nil)
protect_from_connection_error do
check_environment

credit_amount ||= credit_allowed >= order.outstanding_balance.abs ? order.outstanding_balance.abs : credit_allowed.abs
credit_amount ||= if credit_allowed >= order.outstanding_balance.abs
order.outstanding_balance.abs
else
credit_allowed.abs
end

credit_amount = credit_amount.to_f

if payment_method.payment_profiles_supported?
response = payment_method.credit((credit_amount * 100).round, source, response_code, gateway_options)
else
response = payment_method.credit((credit_amount * 100).round, response_code, gateway_options)
end
response = if payment_method.payment_profiles_supported?
payment_method.credit(
(credit_amount * 100).round,
source,
response_code,
gateway_options
)
else
payment_method.credit(
(credit_amount * 100).round,
response_code,
gateway_options
)
end

record_response(response)

Expand All @@ -115,11 +134,20 @@ def refund!(refund_amount = nil)

refund_amount = calculate_refund_amount(refund_amount)

if payment_method.payment_profiles_supported?
response = payment_method.refund((refund_amount * 100).round, source, response_code, gateway_options)
else
response = payment_method.refund((refund_amount * 100).round, response_code, gateway_options)
end
response = if payment_method.payment_profiles_supported?
payment_method.refund(
(refund_amount * 100).round,
source,
response_code,
gateway_options
)
else
payment_method.refund(
(refund_amount * 100).round,
response_code,
gateway_options
)
end

record_response(response)

Expand Down Expand Up @@ -168,17 +196,24 @@ def gateway_options
private

def calculate_refund_amount(refund_amount = nil)
refund_amount ||= credit_allowed >= order.outstanding_balance.abs ? order.outstanding_balance.abs : credit_allowed.abs
refund_amount ||= if credit_allowed >= order.outstanding_balance.abs
order.outstanding_balance.abs
else
credit_allowed.abs
end
refund_amount.to_f
end

def gateway_action(source, action, success_state)
protect_from_connection_error do
check_environment

response = payment_method.send(action, (amount * 100).round,
source,
gateway_options)
response = payment_method.public_send(
action,
(amount * 100).round,
source,
gateway_options
)
handle_response(response, success_state, :failure)
end
end
Expand All @@ -196,9 +231,9 @@ def handle_response(response, success_state, failure_state)
self.cvv_response_message = response.cvv_result['message']
end
end
send("#{success_state}!")
__send__("#{success_state}!")
else
send(failure_state)
__send__(failure_state)
gateway_error(response)
end
end
Expand Down