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

confirmation_required should only check valid payments? Spree 1.3-stable #3970

Closed
emilkarl opened this issue Nov 15, 2013 · 3 comments
Closed

Comments

@emilkarl
Copy link

Hi

Found what you could in same cases call a bug. In Spree 1.3-stable at https://github.com/spree/spree/blob/1-3-stable/core/app/models/spree/order.rb#L167

the confirmation check verifies all payments on the order. But if you have an order were the customer choose a payment method with payment_profiles_supported set to true, that payment method fails, ex "Invoice" and the customer goes back, changes to another payment method and does not require confirmation. When that payment method completes if using order.next to complete it it will go the confirm step even tho the "confirm needed" payment method is invalid.

When a new payment is created the other payments will be set to invalid or failed. And imo invalid or failed payments should not be checked in this case?

This code

def confirmation_required?
  payments.map(&:payment_method).any?(&:payment_profiles_supported?)
end

should in my opinion be

def confirmation_required?
  payments.valid.map(&:payment_method).any?(&:payment_profiles_supported?)
end

I havent used Spree 2.0 I dont know if it could be a problem there as well?

@huoxito
Copy link
Member

huoxito commented Nov 18, 2013

Hi @emilkarl thanks for pointing that out I think it makes sense to only query valid payments when checking if confirmation is required. I'll do that change and see how the build behaves thanks!

@radar
Copy link
Contributor

radar commented Nov 19, 2013

@huoxito What is the status on this issue?

@huoxito
Copy link
Member

huoxito commented Nov 19, 2013

I got a failing spec I couldnt figure yet but should take another look soon.

Washington L Braga Jr
Developer
Spree Commerce Inc
Em 19/11/2013 01:14, "Ryan Bigg" notifications@github.com escreveu:

@huoxito https://github.com/huoxito What is the status on this issue?


Reply to this email directly or view it on GitHubhttps://github.com//issues/3970#issuecomment-28763885
.

huoxito added a commit that referenced this issue Nov 20, 2013
Prevent a confirmation step in cases where a previous selected payment
method which required confirmation failed and customer picked one that
doesnt require confirmation next

Fixes #3970
huoxito added a commit that referenced this issue Nov 20, 2013
Prevent a confirmation step in cases where a previous selected payment
method which required confirmation failed and customer picked one that
doesnt require confirmation next

Fixes #3970
huoxito added a commit that referenced this issue Nov 20, 2013
Prevent a confirmation step in cases where a previous selected payment
method which required confirmation failed and customer picked one that
doesnt require confirmation next

Fixes #3970

Conflicts:
	core/app/models/spree/order.rb
	core/spec/controllers/spree/checkout_controller_spec.rb
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

No branches or pull requests

3 participants