Merge pull request #1611 from jhawthorn/shipping_method_display_on

Replace ShippingMethod#display_on with ShippingMethod#available_to_users

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

@@ -7,14 +7,6 @@
     <% end %>
   </div>
 
-  <div data-hook="admin_shipping_method_form_display_field" class="col-xs-5">
-    <%= f.field_container :display_on do %>
-      <%= f.label :display_on %><br />
-      <%= select(:shipping_method, :display_on, Spree::ShippingMethod::DISPLAY.collect { |display| [Spree.t(display), display == :both ? nil : display.to_s] }, {}, {:class => 'select2 fullwidth'}) %>
-      <%= error_message_on :shipping_method, :display_on %>
-    <% end %>
-  </div>
-
   <div data-hook="admin_shipping_method_form_internal_name_field" class="col-xs-5">
     <%= f.field_container :admin_name do %>
       <%= f.label :admin_name %><br />
@@ -56,9 +48,15 @@
   </div>
 </div>
 
+<%= f.field_container :available_to_users do %>
+  <%= f.check_box(:available_to_users) %>
+  <%= f.label :available_to_users %>
+  <%= error_message_on :shipping_method, :available_to_users %>
+<% end %>
+
 <div class="row">
   <div class="col-xs-5">
-    <div data-hook="admin_shipping_method_form_availability_fields" class="col-xs-5">
+    <div data-hook="admin_shipping_method_form_availability_fields">
       <fieldset class="categories no-border-bottom">
         <legend align="center"><%= plural_resource_name(Spree::ShippingCategory) %></legend>
         <%= f.field_container :categories do %>
@@ -73,7 +71,7 @@
       </fieldset>
     </div>
 
-    <div class="col-xs-5">
+    <div>
       <fieldset class="no-border-bottom">
         <legend align="center"><%= plural_resource_name(Spree::Zone) %></legend>
         <%= f.field_container :zones do %>
@@ -91,10 +89,10 @@
     </div>
   </div>
   <div class="col-xs-5">
-    <div data-hook="admin_shipping_method_form_calculator_fields" class="col-xs-5">
+    <div data-hook="admin_shipping_method_form_calculator_fields">
       <%= render :partial => 'spree/admin/shared/calculator_fields', :locals => { :f => f } %>
     </div>
-    <div class="col-xs-5">
+    <div>
       <fieldset class="tax_categories no-border-bottom">
         <legend align="center"><%= Spree::TaxCategory.model_name.human %></legend>
           <%= f.field_container :tax_categories do %>

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

@@ -27,7 +27,7 @@
         <th><%= Spree::ShippingMethod.human_attribute_name(:name) %></th>
         <th><%= Spree::Zone.model_name.human %></th>
         <th><%= Spree::Calculator.model_name.human %></th>
-        <th><%= Spree::ShippingMethod.human_attribute_name(:display_on) %></th>
+        <th><%= Spree::ShippingMethod.human_attribute_name(:available_to_users) %></th>
         <th data-hook="admin_shipping_methods_index_header_actions" class="actions"></th>
       </tr>
     </thead>
@@ -37,7 +37,7 @@
           <td class="align-center"><%= shipping_method.admin_name + ' / ' if shipping_method.admin_name.present? %><%= shipping_method.name %></td>
           <td class="align-center"><%= shipping_method.zones.collect(&:name).join(", ") if shipping_method.zones %></td>
           <td class="align-center"><%= shipping_method.calculator.description %></td>
-          <td class="align-center"><%= shipping_method.display_on.blank? ? Spree.t(:both) : Spree.t(shipping_method.display_on.downcase.tr(" ", "_")) %></td>
+          <td class="align-center"><%= shipping_method.available_to_users? ? Spree.t(:say_yes) : Spree.t(:say_no) %></td>
           <td data-hook="admin_shipping_methods_index_row_actions" class="actions">
             <% if can?(:update, shipping_method) %>
               <%= link_to_edit shipping_method, :no_text => true %>

backend/spec/features/admin/configuration/shipping_methods_spec.rb

@@ -26,7 +26,7 @@
         expect(column_text(1)).to eq(shipping_method.name)
         expect(column_text(2)).to eq(zone.name)
         expect(column_text(3)).to eq("Flat rate")
-        expect(column_text(4)).to eq("Both")
+        expect(column_text(4)).to eq("Yes")
       end
     end
   end

backend/spec/features/admin/orders/customer_details_spec.rb

@@ -5,7 +5,7 @@
 
   let(:country) { create(:country, name: "Kangaland") }
   let(:state) { create(:state, name: "Alabama", country: country) }
-  let!(:shipping_method) { create(:shipping_method, display_on: "front_end") }
+  let!(:shipping_method) { create(:shipping_method) }
   let!(:order) { create(:order, ship_address: ship_address, bill_address: bill_address, state: 'complete', completed_at: "2011-02-01 12:36:15") }
   let!(:product) { create(:product_in_stock) }
 

core/app/models/spree/shipping_method.rb

@@ -4,7 +4,11 @@ module Spree
   class ShippingMethod < Spree::Base
     acts_as_paranoid
     include Spree::CalculatedAdjustments
-    DISPLAY = [:both, :front_end, :back_end]
+    DISPLAY = ActiveSupport::Deprecation::DeprecatedObjectProxy.new(
+      [:both, :front_end, :back_end],
+      "Spree::ShippingMethod::DISPLAY is deprecated",
+      Spree::Deprecation
+    )
 
     has_many :shipping_method_categories, dependent: :destroy
     has_many :shipping_categories, through: :shipping_method_categories
@@ -79,10 +83,25 @@ def build_tracking_url(tracking)
       tracking_url.gsub(/:tracking/, ERB::Util.url_encode(tracking)) # :url_encode exists in 1.8.7 through 2.1.0
     end
 
+    def display_on
+      if available_to_users?
+        "both"
+      else
+        "back_end"
+      end
+    end
+    deprecate display_on: :available_to_users?, deprecator: Spree::Deprecation
+
+    def display_on=(value)
+      self.available_to_users = (value != "back_end")
+    end
+    deprecate 'display_on=': :available_to_users=, deprecator: Spree::Deprecation
+
     # Some shipping methods are only meant to be set via backend
     def frontend?
-      display_on != "back_end"
+      available_to_users?
     end
+    deprecate frontend?: :available_to_users?, deprecator: Spree::Deprecation
 
     private
 

core/app/models/spree/stock/estimator.rb

@@ -16,7 +16,7 @@ def shipping_rates(package, frontend_only = true)
         raise OrderRequired if package.shipment.order.nil?
 
         rates = calculate_shipping_rates(package)
-        rates.select! { |rate| rate.shipping_method.frontend? } if frontend_only
+        rates.select! { |rate| rate.shipping_method.available_to_users? } if frontend_only
         choose_default_shipping_rate(rates)
         Spree::Config.shipping_rate_sorter_class.new(rates).sort
       end

core/db/migrate/20161123154034_add_available_to_users_and_remove_display_on_from_shipping_methods.rb

@@ -0,0 +1,20 @@
+class AddAvailableToUsersAndRemoveDisplayOnFromShippingMethods < ActiveRecord::Migration[5.0]
+  def up
+    add_column(:spree_shipping_methods, :available_to_users, :boolean, default: true)
+    execute("UPDATE spree_shipping_methods "\
+             "SET available_to_users=#{quoted_false} "\
+             "WHERE display_on='back_end'")
+    remove_column(:spree_shipping_methods, :display_on)
+  end
+
+  def down
+    add_column(:spree_shipping_methods, :display_on, :string)
+    execute("UPDATE spree_shipping_methods "\
+            "SET display_on='both' "\
+            "WHERE (available_to_users=#{quoted_true}")
+    execute("UPDATE spree_shipping_methods "\
+            "SET display_on='back_end' "\
+            "WHERE (available_to_users=#{quoted_false})")
+    remove_column(:spree_shipping_methods, :available_to_users)
+  end
+end

core/spec/models/spree/shipping_method_spec.rb

@@ -210,4 +210,45 @@ def matching(categories)
       it { is_expected.to_not include(shipping_method) }
     end
   end
+
+  describe "display_on=" do
+    subject do
+      Spree::Deprecation.silence do
+        described_class.new(display_on: display_on).available_to_users
+      end
+    end
+
+    context "with 'back_end'" do
+      let(:display_on) { 'back_end' }
+      it { should be false }
+    end
+
+    context "with 'both'" do
+      let(:display_on) { 'both' }
+      it { should be true }
+    end
+
+    context "with 'front_end'" do
+      let(:display_on) { 'front_end' }
+      it { should be true }
+    end
+  end
+
+  describe "display_on" do
+    subject do
+      Spree::Deprecation.silence do
+        described_class.new(available_to_users: available_to_users).display_on
+      end
+    end
+
+    context "when available_to_users is true" do
+      let(:available_to_users) { true }
+      it { should == 'both' }
+    end
+
+    context "when available_to_users is false" do
+      let(:available_to_users) { false }
+      it { should == 'back_end' }
+    end
+  end
 end

core/spec/models/spree/stock/estimator_spec.rb

@@ -126,7 +126,7 @@ module Stock
 
         context "involves backend only shipping methods" do
           before{ Spree::ShippingMethod.destroy_all }
-          let!(:backend_method) { create(:shipping_method, display_on: "back_end", cost: 0.00) }
+          let!(:backend_method) { create(:shipping_method, available_to_users: false, cost: 0.00) }
           let!(:generic_method) { create(:shipping_method, cost: 5.00) }
 
           it "does not return backend rates at all" do