- <%= f.label :display_on %>
- <%= select(:payment_method, :display_on, Spree::PaymentMethod::DISPLAY.collect { |display| [Spree.t(display), display == :both ? nil : display.to_s] }, {}, {:class => 'select2 fullwidth'}) %>
+
+ <%= label_tag nil, Spree::PaymentMethod.human_attribute_name(:available_to_users) %>
+ <%= f.check_box :available_to_users %>
+
+ <%= label_tag nil, Spree::PaymentMethod.human_attribute_name(:available_to_admin) %>
+ <%= f.check_box :available_to_admin %>
<%= f.label :auto_capture %>
diff --git a/backend/app/views/spree/admin/payment_methods/index.html.erb b/backend/app/views/spree/admin/payment_methods/index.html.erb
index a3852ef20ad..1b80d3f5306 100644
--- a/backend/app/views/spree/admin/payment_methods/index.html.erb
+++ b/backend/app/views/spree/admin/payment_methods/index.html.erb
@@ -19,8 +19,8 @@
-
-
+
+
@@ -29,7 +29,8 @@
|
<%= Spree::PaymentMethod.human_attribute_name(:name) %> |
<%= Spree::PaymentMethod.human_attribute_name(:type) %> |
-
<%= Spree::PaymentMethod.human_attribute_name(:display_on) %> |
+
<%= Spree::PaymentMethod.human_attribute_name(:available_to_users) %> |
+
<%= Spree::PaymentMethod.human_attribute_name(:available_to_admin) %> |
<%= Spree::PaymentMethod.human_attribute_name(:active) %> |
|
@@ -40,7 +41,8 @@
|
<%= method.name %> |
<%= method.type %> |
-
<%= method.display_on.blank? ? Spree.t(:both) : Spree.t(method.display_on) %> |
+
<%= method.available_to_users ? Spree.t(:say_yes) : Spree.t(:say_no) %> |
+
<%= method.available_to_admin ? Spree.t(:say_yes) : Spree.t(:say_no) %> |
<%= method.active ? Spree.t(:say_yes) : Spree.t(:say_no) %> |
<% if can?(:update, method.becomes(Spree::PaymentMethod)) %>
diff --git a/backend/spec/controllers/spree/admin/payments_controller_spec.rb b/backend/spec/controllers/spree/admin/payments_controller_spec.rb
index dd592b5694c..e75bbc06d3d 100644
--- a/backend/spec/controllers/spree/admin/payments_controller_spec.rb
+++ b/backend/spec/controllers/spree/admin/payments_controller_spec.rb
@@ -13,7 +13,7 @@ module Admin
describe '#create' do
context "with a valid credit card" do
let(:order) { create(:order_with_line_items, state: "payment") }
- let(:payment_method) { create(:credit_card_payment_method, display_on: "back_end") }
+ let(:payment_method) { create(:credit_card_payment_method, available_to_admin: true) }
let(:attributes) do
{
order_id: order.number,
@@ -76,7 +76,7 @@ module Admin
# Regression test for https://github.com/spree/spree/issues/3233
context "with a backend payment method" do
before do
- @payment_method = create(:check_payment_method, display_on: "back_end")
+ @payment_method = create(:check_payment_method, available_to_admin: true)
end
it "loads backend payment methods" do
diff --git a/backend/spec/features/admin/configuration/payment_methods_spec.rb b/backend/spec/features/admin/configuration/payment_methods_spec.rb
index 8a1296d5fb7..079a2a743d3 100644
--- a/backend/spec/features/admin/configuration/payment_methods_spec.rb
+++ b/backend/spec/features/admin/configuration/payment_methods_spec.rb
@@ -17,8 +17,9 @@
within("table#listing_payment_methods") do
expect(all("th")[1].text).to eq("Name")
expect(all("th")[2].text).to eq("Provider")
- expect(all("th")[3].text).to eq("Display")
- expect(all("th")[4].text).to eq("Active")
+ expect(all("th")[3].text).to eq("Available to users")
+ expect(all("th")[4].text).to eq("Available to admin")
+ expect(all("th")[5].text).to eq("Active")
end
within('table#listing_payment_methods') do
diff --git a/backend/spec/features/admin/orders/payments_spec.rb b/backend/spec/features/admin/orders/payments_spec.rb
index decbbc0610c..851a8ca4c04 100644
--- a/backend/spec/features/admin/orders/payments_spec.rb
+++ b/backend/spec/features/admin/orders/payments_spec.rb
@@ -27,7 +27,7 @@
create(:payment,
order: order,
amount: order.outstanding_balance,
- payment_method: create(:check_payment_method) # Check
+ payment_method: create(:check_payment_method, available_to_admin: true) # Check
)
end
diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb
index 9ca000d7739..477c357b5f6 100644
--- a/core/app/models/spree/order.rb
+++ b/core/app/models/spree/order.rb
@@ -435,12 +435,10 @@ def paid?
end
def available_payment_methods
- @available_payment_methods ||= (
- PaymentMethod.available(:front_end, store: store) +
- PaymentMethod.available(:both, store: store)
- ).
- uniq.
- sort_by(&:position)
+ @available_payment_methods ||= Spree::PaymentMethod
+ .available_to_store(store)
+ .available_to_users
+ .sort_by(&:position)
end
def insufficient_stock_lines
diff --git a/core/app/models/spree/payment_method.rb b/core/app/models/spree/payment_method.rb
index 2388c69e3df..23ea8ffb02b 100644
--- a/core/app/models/spree/payment_method.rb
+++ b/core/app/models/spree/payment_method.rb
@@ -14,6 +14,10 @@ class PaymentMethod < Spree::Base
has_many :stores, through: :store_payment_methods
scope :ordered_by_position, -> { order(:position) }
+ scope :active, -> { where(active: true) }
+ scope :available_to_users, -> { where(available_to_users: true) }
+ scope :available_to_admin, -> { where(available_to_admin: true) }
+ scope :available_to_store, -> (store) { (store.present? && store.payment_methods.empty?) ? self : store.payment_methods }
include Spree::Preferences::StaticallyConfigurable
@@ -32,11 +36,48 @@ def payment_source_class
raise ::NotImplementedError, "You must implement payment_source_class method for #{self.class}."
end
- def self.available(display_on = 'both', store: nil)
- all.select do |p|
- p.active &&
- (p.display_on == display_on.to_s || p.display_on.blank?) &&
- (store.nil? || store.payment_methods.empty? || store.payment_methods.include?(p))
+ # @deprecated Use {#available_to_users=} and {#available_to_admin=} instead
+ def display_on=(value)
+ Spree::Deprecation.warn "Spree::PaymentMethod#display_on= is deprecated."\
+ "Please use #available_to_users= and #available_to_admin= instead."
+ self.available_to_users = value.blank? || value == 'front_end'
+ self.available_to_admin = value.blank? || value == 'back_end'
+ end
+
+ # @deprecated Use {#available_to_users} and {#available_to_admin} instead
+ def display_on
+ Spree::Deprecation.warn "Spree::PaymentMethod#display_on is deprecated."\
+ "Please use #available_to_users and #available_to_admin instead."
+ if available_to_users? && available_to_admin?
+ ''
+ elsif available_to_users?
+ 'front_end'
+ elsif available_to_admin?
+ 'back_end'
+ else
+ 'none'
+ end
+ end
+
+ def self.available(display_on=nil, store: nil)
+ Spree::Deprecation.warn "Spree::PaymentMethod.available is deprecated."\
+ "Please use .active, .available_to_users, and .available_to_admin scopes instead."\
+ "For payment methods associated with a specific store, use Spree::PaymentMethod.available_to_store(your_store)"\
+ " as the base applying any further filtering"
+
+ display_on = display_on.to_s
+
+ available_payment_methods =
+ case display_on
+ when 'front_end'
+ active.available_to_users
+ when 'back_end'
+ active.available_to_admin
+ else
+ active.available_to_users.available_to_admin
+ end
+ available_payment_methods.select do |p|
+ store.nil? || store.payment_methods.empty? || store.payment_methods.include?(p)
end
end
diff --git a/core/db/migrate/20161014221052_add_available_to_columns_and_remove_display_on_from_payment_methods.rb b/core/db/migrate/20161014221052_add_available_to_columns_and_remove_display_on_from_payment_methods.rb
new file mode 100644
index 00000000000..c9a92af70c4
--- /dev/null
+++ b/core/db/migrate/20161014221052_add_available_to_columns_and_remove_display_on_from_payment_methods.rb
@@ -0,0 +1,28 @@
+class AddAvailableToColumnsAndRemoveDisplayOnFromPaymentMethods < ActiveRecord::Migration[5.0]
+ def up
+ add_column(:spree_payment_methods, :available_to_users, :boolean, default: true)
+ add_column(:spree_payment_methods, :available_to_admin, :boolean, default: true)
+ execute("UPDATE spree_payment_methods "\
+ "SET available_to_users=#{quoted_false} "\
+ "WHERE NOT (display_on='front_end' OR display_on='' OR display_on IS NULL)")
+ execute("UPDATE spree_payment_methods "\
+ "SET available_to_admin=#{quoted_false} "\
+ "WHERE NOT (display_on='back_end' OR display_on='' OR display_on IS NULL)")
+ remove_column(:spree_payment_methods, :display_on)
+ end
+
+ def down
+ add_column(:spree_payment_methods, :display_on, :string)
+ execute("UPDATE spree_payment_methods "\
+ "SET display_on='' "\
+ "WHERE (available_to_users=#{quoted_true} AND available_to_admin=#{quoted_true})")
+ execute("UPDATE spree_payment_methods "\
+ "SET display_on='front_end' "\
+ "WHERE (available_to_users=#{quoted_true} AND NOT available_to_admin=#{quoted_true})")
+ execute("UPDATE spree_payment_methods "\
+ "SET display_on='back_end' "\
+ "WHERE (available_to_admin=#{quoted_true} AND NOT available_to_users=#{quoted_true})")
+ remove_column(:spree_payment_methods, :available_to_users)
+ remove_column(:spree_payment_methods, :available_to_admin)
+ end
+end
diff --git a/core/lib/spree/testing_support/factories/payment_method_factory.rb b/core/lib/spree/testing_support/factories/payment_method_factory.rb
index 4ca4fe860b0..99faa0f7758 100644
--- a/core/lib/spree/testing_support/factories/payment_method_factory.rb
+++ b/core/lib/spree/testing_support/factories/payment_method_factory.rb
@@ -1,23 +1,30 @@
FactoryGirl.define do
factory :payment_method, aliases: [:credit_card_payment_method], class: Spree::Gateway::Bogus do
name 'Credit Card'
+ available_to_admin true
+ available_to_users true
end
factory :check_payment_method, class: Spree::PaymentMethod::Check do
name 'Check'
+ available_to_admin true
+ available_to_users true
end
# authorize.net was moved to spree_gateway.
# Leaving this factory in place with bogus in case anyone is using it.
factory :simple_credit_card_payment_method, class: Spree::Gateway::BogusSimple do
name 'Credit Card'
+ available_to_admin true
+ available_to_users true
end
factory :store_credit_payment_method, class: Spree::PaymentMethod::StoreCredit do
name "Store Credit"
description "Store Credit"
active true
- display_on 'none'
+ available_to_admin false
+ available_to_users false
auto_capture true
end
end
diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb
index 8455fecf7e7..6dfd768960a 100644
--- a/core/spec/models/spree/order_spec.rb
+++ b/core/spec/models/spree/order_spec.rb
@@ -510,7 +510,8 @@ def merge!(other_order, user = nil)
payment_method = Spree::PaymentMethod.create!({
name: "Fake",
active: true,
- display_on: "front_end"
+ available_to_users: true,
+ available_to_admin: false
})
expect(order.available_payment_methods).to include(payment_method)
end
@@ -519,16 +520,18 @@ def merge!(other_order, user = nil)
payment_method = Spree::PaymentMethod.create!({
name: "Fake",
active: true,
- display_on: "both"
+ available_to_users: true,
+ available_to_admin: true
})
expect(order.available_payment_methods).to include(payment_method)
end
- it "does not include a payment method twice if display_on is blank" do
+ it "does not include a payment method twice" do
payment_method = Spree::PaymentMethod.create!({
name: "Fake",
active: true,
- display_on: "both"
+ available_to_users: true,
+ available_to_admin: true
})
expect(order.available_payment_methods.count).to eq(1)
expect(order.available_payment_methods).to include(payment_method)
@@ -537,8 +540,10 @@ def merge!(other_order, user = nil)
context "with more than one payment method" do
subject { order.available_payment_methods }
- let!(:first_method) { FactoryGirl.create(:payment_method, display_on: :both) }
- let!(:second_method) { FactoryGirl.create(:payment_method, display_on: :both) }
+ let!(:first_method) { FactoryGirl.create(:payment_method, available_to_users: true,
+ available_to_admin: true) }
+ let!(:second_method) { FactoryGirl.create(:payment_method, available_to_users: true,
+ available_to_admin: true) }
before do
second_method.move_to_top
@@ -569,6 +574,23 @@ def merge!(other_order, user = nil)
[payment_method_with_store]
)
end
+
+ context 'and the store has an extra payment method unavailable to users' do
+ let!(:admin_only_payment_method) do create(:payment_method,
+ available_to_users: false,
+ available_to_admin: true)
+ end
+
+ before do
+ store_with_payment_methods.payment_methods << admin_only_payment_method
+ end
+
+ it 'returns only the payment methods available to users for that store' do
+ expect(order.available_payment_methods).to match_array(
+ [payment_method_with_store]
+ )
+ end
+ end
end
context 'when the store does not have payment methods' do
diff --git a/core/spec/models/spree/payment_method_spec.rb b/core/spec/models/spree/payment_method_spec.rb
index 269508a731b..326b54f52c7 100644
--- a/core/spec/models/spree/payment_method_spec.rb
+++ b/core/spec/models/spree/payment_method_spec.rb
@@ -1,30 +1,101 @@
require 'spec_helper'
describe Spree::PaymentMethod, type: :model do
- describe ".available" do
- let!(:payment_method_nil_display) { create(:payment_method, active: true, display_on: nil) }
- let!(:payment_method_both_display) { create(:payment_method, active: true, display_on: '') }
- let!(:payment_method_front_display){ create(:payment_method, active: true, display_on: 'front_end') }
- let!(:payment_method_back_display) { create(:payment_method, active: true, display_on: 'back_end') }
+ let!(:payment_method_nil_display) { create(:payment_method, active: true,
+ available_to_users: true,
+ available_to_admin: true) }
+ let!(:payment_method_both_display) { create(:payment_method, active: true,
+ available_to_users: true,
+ available_to_admin: true) }
+ let!(:payment_method_front_display){ create(:payment_method, active: true,
+ available_to_users: true,
+ available_to_admin: false) }
+ let!(:payment_method_back_display) { create(:payment_method, active: true,
+ available_to_users: false,
+ available_to_admin: true) }
+
+ describe "available_to_[, , ]" do
+ context "when searching for payment methods available to users and admins" do
+ subject { Spree::PaymentMethod.available_to_users.available_to_admin }
+
+ it { is_expected.to contain_exactly(payment_method_both_display, payment_method_nil_display) }
+
+ context "with a store" do
+ let!(:extra_payment_method_both_display) { create(:payment_method, active: true,
+ available_to_users: true,
+ available_to_admin: true) }
+ let!(:store_1) do
+ create(:store, payment_methods:[payment_method_nil_display,
+ payment_method_both_display,
+ payment_method_front_display,
+ payment_method_back_display])
+ end
+
+ subject { Spree::PaymentMethod.available_to_store( store_1 ).available_to_users.available_to_admin }
+
+ it { is_expected.to contain_exactly(payment_method_both_display, payment_method_nil_display) }
+
+ context "when the store has no payment methods" do
+ subject { Spree::PaymentMethod.available_to_store(store_without_payment_methods) }
+ let!(:store_without_payment_methods) do
+ create(:store, payment_methods:[])
+ end
+
+ it "returns all payment methods" do
+ expect(subject.all.size).to eq(5)
+ end
+
+ it "is further scopeable for admin availability" do
+ expect(subject.available_to_admin).not_to include(payment_method_front_display)
+ end
+
+ it "is further scopeable for users availability" do
+ expect(subject.available_to_users).not_to include(payment_method_back_display)
+ end
+ end
+ end
+ end
+ context "when searching for payment methods available to users" do
+ subject { Spree::PaymentMethod.available_to_users }
+
+ it { is_expected.to contain_exactly(payment_method_front_display, payment_method_both_display, payment_method_nil_display) }
+ end
+
+ context "when searching for payment methods available to admin" do
+ subject { Spree::PaymentMethod.available_to_admin }
+
+ it { is_expected.to contain_exactly(payment_method_back_display, payment_method_both_display, payment_method_nil_display)}
+ end
+ end
+
+ describe ".available" do
it "should have 4 total methods" do
expect(Spree::PaymentMethod.all.size).to eq(4)
end
it "should return all methods available to front-end/back-end when no parameter is passed" do
- expect(Spree::PaymentMethod.available.size).to eq(2)
+ Spree::Deprecation.silence do
+ expect(Spree::PaymentMethod.available.size).to eq(2)
+ end
end
- it "should return all methods available to front-end/back-end when display_on = :both" do
- expect(Spree::PaymentMethod.available(:both).size).to eq(2)
+ it "should return all methods available to front-end/back-end when passed :both" do
+ Spree::Deprecation.silence do
+ expect(Spree::PaymentMethod.available(:both).size).to eq(2)
+ end
end
- it "should return all methods available to front-end when display_on = :front_end" do
- expect(Spree::PaymentMethod.available(:front_end).size).to eq(3)
+ it "should return all methods available to front-end when passed :front_end" do
+ Spree::Deprecation.silence do
+ expect(Spree::PaymentMethod.available(:front_end).size).to eq(3)
+ end
end
- it "should return all methods available to back-end when display_on = :back_end" do
- expect(Spree::PaymentMethod.available(:back_end).size).to eq(3)
+ it "should return all methods available to back-end when passed :back_end" do
+ Spree::Deprecation.silence do
+ expect(Spree::PaymentMethod.available(:back_end).size).to eq(3)
+ end
end
context 'with stores' do
@@ -51,14 +122,33 @@
context 'when the store is specified' do
context 'when the store has payment methods' do
it 'finds the payment methods for the store' do
- expect(Spree::PaymentMethod.available(:both, store: store_1)).to match_array(
- [payment_method_nil_display, payment_method_both_display]
- )
+ Spree::Deprecation.silence do
+ expect(Spree::PaymentMethod.available(:both, store: store_1)).to match_array(
+ [payment_method_nil_display, payment_method_both_display]
+ )
+ end
end
end
context "when store does not have payment_methods" do
it "returns all matching payment methods regardless of store" do
+ Spree::Deprecation.silence do
+ expect(Spree::PaymentMethod.available(:both)).to match_array(
+ [
+ payment_method_nil_display,
+ payment_method_both_display,
+ store_2_payment_method,
+ no_store_payment_method
+ ]
+ )
+ end
+ end
+ end
+ end
+
+ context 'when the store is not specified' do
+ it "returns all matching payment methods regardless of store" do
+ Spree::Deprecation.silence do
expect(Spree::PaymentMethod.available(:both)).to match_array(
[
payment_method_nil_display,
@@ -70,19 +160,6 @@
end
end
end
-
- context 'when the store is not specified' do
- it "returns all matching payment methods regardless of store" do
- expect(Spree::PaymentMethod.available(:both)).to match_array(
- [
- payment_method_nil_display,
- payment_method_both_display,
- store_2_payment_method,
- no_store_payment_method
- ]
- )
- end
- end
end
end
@@ -143,4 +220,77 @@ def provider_class
end
end
end
+
+ describe "display_on=" do
+ around do |example|
+ Spree::Deprecation.silence do
+ example.run
+ end
+ end
+ let(:payment) { described_class.new(display_on: display_on) }
+
+ context 'with empty string' do
+ let(:display_on) { "" }
+
+ it "should be available to admins" do
+ expect(payment.available_to_admin).to be true
+ end
+
+ it "should be available to users" do
+ expect(payment.available_to_users).to be true
+ end
+
+ it "should have the same value for display_on" do
+ expect(payment.display_on).to eq ""
+ end
+ end
+
+ context 'with "back_end"' do
+ let(:display_on) { "back_end" }
+
+ it "should be available to admins" do
+ expect(payment.available_to_admin).to be true
+ end
+
+ it "should not be available to users" do
+ expect(payment.available_to_users).to be false
+ end
+
+ it "should have the same value for display_on" do
+ expect(payment.display_on).to eq "back_end"
+ end
+ end
+
+ context 'with "front_end"' do
+ let(:display_on) { "front_end" }
+
+ it "should be available to admins" do
+ expect(payment.available_to_admin).to be false
+ end
+
+ it "should not be available to users" do
+ expect(payment.available_to_users).to be true
+ end
+
+ it "should have the same value for display_on" do
+ expect(payment.display_on).to eq "front_end"
+ end
+ end
+
+ context 'with any other string' do
+ let(:display_on) { "foobar" }
+
+ it "should be available to admins" do
+ expect(payment.available_to_admin).to be false
+ end
+
+ it "should not be available to users" do
+ expect(payment.available_to_users).to be false
+ end
+
+ it "should have none for display_on" do
+ expect(payment.display_on).to eq "none"
+ end
+ end
+ end
end
|