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

Prefer a store specific payment method if it exists #106

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

Prefer a store specific payment method if it exists #106

wants to merge 1 commit into from

Conversation

emileswarts
Copy link

In a multi store environment it is desirable to use the store specific payment method.
If one is not defined, default back to the last Adyen payment method it can find.

@Dkendal Dkendal requested a review from luukveenis April 12, 2017 17:26
Copy link
Member

@luukveenis luukveenis left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this PR!

I'd be happy to merge this in, just 2 minor comments.

@@ -143,6 +143,12 @@ def create_missing_payment
payment
end

def payment_method_for(order)
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't need to pass the order to this method since the class is initialized with it. With it removed the method can probably also be renamed to something like hpp_payment_method.

@@ -143,6 +143,12 @@ def create_missing_payment
payment
end

def payment_method_for(order)
order.store.payment_methods.where(type: 'Spree::Gateway::AdyenHPP', active: true).last!
rescue
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a big fan of rescuing everything, I would prefer something like:
order.store.payment_methods.active.find_by(type: "Spree::Gateway::AdyenHPP") || Spree::Gateway::AdyenHPP.last

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

2 participants