Merge pull request #1540 from gevann/payment-methods-display-on-boolean

Payment methods display on boolean

api/app/controllers/spree/api/payments_controller.rb

@@ -11,7 +11,7 @@ def index
       end
 
       def new
-        @payment_methods = Spree::PaymentMethod.available
+        @payment_methods = Spree::PaymentMethod.available_to_users.available_to_admin
         respond_with(@payment_method)
       end
 

backend/app/controllers/spree/admin/payments_controller.rb

@@ -78,7 +78,7 @@ def object_params
 
       def load_data
         @amount = params[:amount] || load_order.total
-        @payment_methods = PaymentMethod.available(:back_end)
+        @payment_methods = PaymentMethod.available_to_admin
         if @payment && @payment.payment_method
           @payment_method = @payment.payment_method
         else

backend/app/views/spree/admin/payment_methods/_form.html.erb

@@ -25,9 +25,13 @@
 
           <div class="info warning js-gateway-settings-warning"><%= Spree.t(:provider_settings_warning) %></div>
         </div>
-        <div data-hook="display" class="field">
-          <%= 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'}) %>
+        <div data-hook="available_to_user" class="field">
+          <%= label_tag nil, Spree::PaymentMethod.human_attribute_name(:available_to_users) %>
+          <%= f.check_box :available_to_users %>
+        </div>
+        <div data-hook="available_to_user" class="field">
+          <%= label_tag nil, Spree::PaymentMethod.human_attribute_name(:available_to_admin) %>
+          <%= f.check_box :available_to_admin %>
         </div>
         <div data-hook="auto_capture" class="field">
           <%= f.label :auto_capture %>

backend/app/views/spree/admin/payment_methods/index.html.erb

@@ -19,8 +19,8 @@
       <col style="width: 10%">
       <col style="width: 13%">
       <col style="width: 33%">
-      <col style="width: 13%">
-      <col style="width: 19%">
+      <col style="width: 16%">
+      <col style="width: 16%">
       <col style="width: 19%">
       <col style="width: 13%">
     </colgroup>
@@ -29,7 +29,8 @@
         <th class="no-border"></th>
         <th><%= Spree::PaymentMethod.human_attribute_name(:name) %></th>
         <th><%= Spree::PaymentMethod.human_attribute_name(:type) %></th>
-        <th><%= Spree::PaymentMethod.human_attribute_name(:display_on) %></th>
+        <th><%= Spree::PaymentMethod.human_attribute_name(:available_to_users) %></th>
+        <th><%= Spree::PaymentMethod.human_attribute_name(:available_to_admin) %></th>
         <th><%= Spree::PaymentMethod.human_attribute_name(:active) %></th>
         <th data-hook="admin_payment_methods_index_header_actions" class="actions"></th>
       </tr>
@@ -40,7 +41,8 @@
           <td class="no-border"><span class="handle"></span></td>
           <td class="align-center"><%= method.name %></td>
           <td class="align-center"><%= method.type %></td>
-          <td class="align-center"><%= method.display_on.blank? ? Spree.t(:both) : Spree.t(method.display_on) %></td>
+          <td class="align-center"><%= method.available_to_users ? Spree.t(:say_yes) : Spree.t(:say_no) %></td>
+          <td class="align-center"><%= method.available_to_admin ? Spree.t(:say_yes) : Spree.t(:say_no) %></td>
           <td class="align-center"><%= method.active ? Spree.t(:say_yes) : Spree.t(:say_no) %></td>
           <td data-hook="admin_payment_methods_index_row_actions" class="actions">
             <% if can?(:update, method.becomes(Spree::PaymentMethod)) %>

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

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

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
 

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

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
 

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

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

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