Merge pull request #5392 from luisramos0/ship_method_bo

Make "backoffice only" ship methods work and remove option "frontoffice only"

app/helpers/enterprises_helper.rb

@@ -14,7 +14,7 @@ def current_customer
   def available_shipping_methods
     return [] if current_distributor.blank?
 
-    shipping_methods = current_distributor.shipping_methods
+    shipping_methods = current_distributor.shipping_methods.display_on_checkout.to_a
 
     applicator = OpenFoodNetwork::TagRuleApplicator.new(current_distributor, "FilterShippingMethods", current_customer.andand.tag_list)
     applicator.filter!(shipping_methods)

app/models/spree/shipping_method_decorator.rb

@@ -30,6 +30,7 @@
   }
 
   scope :by_name, -> { order('spree_shipping_methods.name ASC') }
+  scope :display_on_checkout, -> { where("spree_shipping_methods.display_on is null OR spree_shipping_methods.display_on = ''") }
 
   # Return the services (pickup, delivery) that different distributors provide, in the format:
   # {distributor_id => {pickup: true, delivery: false}, ...}

app/views/spree/admin/shipping_methods/_form.html.haml

@@ -18,7 +18,7 @@
       .alpha.three.columns
         = f.label :display_on, t(:display)
       .omega.eight.columns
-        = select(:shipping_method, :display_on, Spree::ShippingMethod::DISPLAY.collect { |display| [t(".#{display}"), display == :both ? nil : display.to_s] }, {}, {class: 'select2 fullwidth'})
+        = select(:shipping_method, :display_on, [[t(".both"), nil], [t(".back_end"), "back_end"]], {}, {class: 'select2 fullwidth'})
         = error_message_on :shipping_method, :display_on
 
   .row

config/locales/en.yml

@@ -3231,9 +3231,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using
           zone: "Zone"
           calculator: "Calculator"
           display: "Display"
-          both: "Both"
-          front_end: "Front End"
-          back_end: "Back End"
+          both: "Both Checkout and Back office"
+          back_end: "Back office only"
           no_shipping_methods_found: "No shipping methods found"
         new:
           new_shipping_method: "New Shipping Method"
@@ -3245,9 +3244,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using
         form:
           categories: "Categories"
           zones: "Zones"
-          both: "Both"
-          front_end: "Front End"
-          back_end: "Back End"
+          both: "Both Checkout and Back office"
+          back_end: "Back office only"
       payment_methods:
         new:
           new_payment_method: "New Payment Method"

db/migrate/20200508101630_convert_frontend_shipping_method_to_both.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class ConvertFrontendShippingMethodToBoth < ActiveRecord::Migration
+  def up
+    # The display_on value front_end is not working
+    #   (it's not being used in the back office to ignore shipping methods marked as front_end)
+    # So, here we are converting all entries to the more generic "both" option
+    #   both is represented as nil in the database
+    # # This enables us to remove the front_end option from the code
+    execute("UPDATE spree_shipping_methods SET display_on = null WHERE display_on = 'front_end'")
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20200430105459) do
+ActiveRecord::Schema.define(:version => 20200508101630) do
 
   create_table "adjustment_metadata", :force => true do |t|
     t.integer "adjustment_id"

spec/features/consumer/shopping/checkout_spec.rb

@@ -229,6 +229,15 @@
         end
       end
 
+      it "filters out 'Back office only' shipping methods" do
+        expect(page).to have_content shipping_with_fee.name
+        shipping_with_fee.update_attribute :display_on, 'back_end' # Back office only
+
+        visit checkout_path
+        checkout_as_guest
+        expect(page).not_to have_content shipping_with_fee.name
+      end
+
       context "using FilterShippingMethods" do
         let(:user) { create(:user) }
         let(:customer) { create(:customer, user: user, enterprise: distributor) }

spec/helpers/enterprises_helper_spec.rb

@@ -8,8 +8,8 @@
   before { allow(helper).to receive(:spree_current_user) { user } }
 
   describe "loading available shipping methods" do
-    let!(:sm1) { create(:shipping_method, require_ship_address: false, distributors: [distributor]) }
-    let!(:sm2) { create(:shipping_method, require_ship_address: false, distributors: [some_other_distributor]) }
+    let!(:distributor_shipping_method) { create(:shipping_method, require_ship_address: false, distributors: [distributor]) }
+    let!(:other_distributor_shipping_method) { create(:shipping_method, require_ship_address: false, distributors: [some_other_distributor]) }
 
     context "when the order has no current_distributor" do
       before do
@@ -25,8 +25,16 @@
       before { allow(helper).to receive(:current_distributor) { distributor } }
 
       it "finds the shipping methods for the current distributor" do
-        expect(helper.available_shipping_methods).to_not include sm2
-        expect(helper.available_shipping_methods).to include sm1
+        expect(helper.available_shipping_methods).to_not include other_distributor_shipping_method
+        expect(helper.available_shipping_methods).to include distributor_shipping_method
+      end
+
+      it "does not return 'back office only' shipping method" do
+        backoffice_only_shipping_method = create(:shipping_method, require_ship_address: false, distributors: [distributor], display_on: 'back_end')
+
+        expect(helper.available_shipping_methods).to_not include backoffice_only_shipping_method
+        expect(helper.available_shipping_methods).to_not include other_distributor_shipping_method
+        expect(helper.available_shipping_methods).to include distributor_shipping_method
       end
     end
 
@@ -44,8 +52,8 @@
                is_default: true,
                preferred_shipping_method_tags: "local-delivery")
       }
-      let!(:tagged_sm) { sm1 }
-      let!(:untagged_sm) { sm2 }
+      let!(:tagged_sm) { distributor_shipping_method }
+      let!(:untagged_sm) { other_distributor_shipping_method }
 
       before do
         tagged_sm.update_attribute(:tag_list, 'local-delivery')