Skip to content

Fix StripeCreditCard#try_void with Elements/V2 API#75

Merged
spaghetticode merged 1 commit into
solidusio:masterfrom
spaghetticode:spaghetticode/fix-try-void
Jul 17, 2020
Merged

Fix StripeCreditCard#try_void with Elements/V2 API#75
spaghetticode merged 1 commit into
solidusio:masterfrom
spaghetticode:spaghetticode/fix-try-void

Conversation

@spaghetticode

@spaghetticode spaghetticode commented Jul 15, 2020

Copy link
Copy Markdown
Member

Payment::Cancellation#cancel calls StripeCreditCard#try_void`.

When not using Payment Intents (ie. when using Elements or V2 API), after calling try_void, the former makes the following call:

payment.send(:handle_void_response, response)

which is already called by payment.void_transaction!, making the two incompatible, as #handle_void_response cannot be called twice here - it would raise the error:

NoMethodError: undefined method `success?' for true:TrueClass

Under the hood #void_transaction! calls payment_method.void, so the new code is rather equivalent to the previous one. The only relevant feature we're losing is that #void_transaction! wraps the call in a protect_from_connection_error block.

The modified integration spec verifies that an order created with Elements can be canceled without raising the error mentioned above.

@spaghetticode spaghetticode requested a review from kennyadsl July 15, 2020 12:47
@spaghetticode spaghetticode self-assigned this Jul 15, 2020
`Payment::Cancellation#cancel` calls StripeCreditCard#try_void`.

When not using Payment Intents (ie. when using Elements or V2 API),
after calling `try_void`, the former makes the following call:

  payment.send(:handle_void_response, response)

which is already called by `payment.void_transaction!`, making
the two incompatible, as `#handle_void_response` cannot be called
twice here - it would raise the error:

  NoMethodError: undefined method `success?' for true:TrueClass

Under the hood `#void_transaction!` calls `payment_method.void`,
so the new code is rather equivalent to the previous one. The
only relevant feature we're losing is that `#void_transaction!`
wraps the call in a `protect_from_connection_error` block.

The modified integration spec verifies that an order created with
Elements can be cancelled without raising the error mentioned
above.
@spaghetticode spaghetticode force-pushed the spaghetticode/fix-try-void branch from 98154b6 to 3156c01 Compare July 15, 2020 13:37
@spaghetticode spaghetticode added the bug Something isn't working label Jul 15, 2020
@aldesantis

Copy link
Copy Markdown
Member

@spaghetticode do you think there is a way to maintain the protect_from_connection_error behavior? That seems quite useful.

@spaghetticode

spaghetticode commented Jul 17, 2020

Copy link
Copy Markdown
Member Author

@aldesantis I share your same feeling. I was a bit hesitant is adding protect_from_connection_error here as I think it's a problem that would be better to address directly in Solidus core, first because protect_from_connection_error is a private method, then because #void_transaction! was introduced only very recently, so if we did fine until that moment without the help of protect_from_connection_error, so maybe it's not really a needed improvement?

@aldesantis

Copy link
Copy Markdown
Member

@spaghetticode yeah, I think that makes sense. Let's move forward without it and at some point we should probably extract it directly in the core.

@spaghetticode spaghetticode merged commit 874322c into solidusio:master Jul 17, 2020
@spaghetticode spaghetticode deleted the spaghetticode/fix-try-void branch July 17, 2020 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants