Deprecate provider in favour of payment method type #1974
Conversation
tvdeyen
added
changelog:solidus_backend
|
Will rebase on top of #1975 if we decided to make this change. |
|
|
||
| def validate_payment_method_provider | ||
| def load_payment_methods | ||
| @payment_methods = Rails.application.config.spree.payment_methods.sort_by(&:name) |
There was a problem hiding this comment.
I think having a @payment_methods ivar can be confusing since, as a developer, I expect to find currently persisted Spree::PaymentMethods there instead now it contains possible types of Spree::PaymentMethod. What about some more semantic variable name like @payment_method_types?
There was a problem hiding this comment.
I see. payment_method_types is ok, or configured_payment_methods could also work, no? @mamhoff thoughts?
There was a problem hiding this comment.
I like @payment_method_types. Or even @available_payment_method_types. I agree with @kennyadsl s sentiment that payment_methods sounds like instances, but the collection is an array of class names.
There was a problem hiding this comment.
@kennyadsl @mamhoff updated. Please re-review
| klass.name == requested_type | ||
| end | ||
| # TODO: Remove `@payment_method_type` instance var once `load_providers` gets removed. | ||
| @payment_method_type = @payment_method_type |
There was a problem hiding this comment.
If you move the comment up to new line 70, we shouldn't really need this line, should we?
There was a problem hiding this comment.
Oh, good catch. Due to the re-rename 😊
|
@mamhoff fixed |
| @@ -25,6 +25,8 @@ class PaymentMethod < Spree::Base | |||
| include Spree::Preferences::StaticallyConfigurable | |||
|
|
|||
| def self.providers | |||
| Spree::Deprecation.warn 'Spree::PaymentMethod.providers will be deleted without replacement. ' \ | |||
There was a problem hiding this comment.
It's not without replacement. The replacement is Rails.application.config.spree.payment_methods 😄
tvdeyen
force-pushed
the
rename-providers
branch
2 times, most recently
from
84984be to
52dafa0
Compare
|
@jhawthorn rebased on latest master. Might have another look? |
|
@tvdeyen Can we get a rebase on this? |
|
@gmacdougall here you go |
Payment method types get referenced as providers in the admin payment methods configuration. In order to remove confusion about what a payment method and a provider actually is we rename usages of provider to payment method type. Later on a real payment provider class will get introduced, that hold all commons for multiple payment methods, like credentials and api communication through its gateway.
Payment method types get referenced as providers in the admin payment methods configuration.
In order to remove confusion about what a payment method and a provider actually is we rename usages of provider to payment method type.
Later on a real payment provider class will get introduced, that hold all commons for multiple payment methods, like credentials and api communication through its gateway.