diff --git a/app/helpers/enterprises_helper.rb b/app/helpers/enterprises_helper.rb index 64d78e7e938..eaaa4c3e865 100644 --- a/app/helpers/enterprises_helper.rb +++ b/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) diff --git a/app/models/spree/shipping_method_decorator.rb b/app/models/spree/shipping_method_decorator.rb index ba7836899d9..ec1b0a8dca4 100644 --- a/app/models/spree/shipping_method_decorator.rb +++ b/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}, ...} diff --git a/app/views/spree/admin/shipping_methods/_form.html.haml b/app/views/spree/admin/shipping_methods/_form.html.haml index dfef7418a14..450febb31bd 100644 --- a/app/views/spree/admin/shipping_methods/_form.html.haml +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 01ba7cebd27..e2a70ff4124 100644 --- a/config/locales/en.yml +++ b/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" diff --git a/db/migrate/20200508101630_convert_frontend_shipping_method_to_both.rb b/db/migrate/20200508101630_convert_frontend_shipping_method_to_both.rb new file mode 100644 index 00000000000..5e2418c3b85 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index e08122c5d68..99415dc9fab 100644 --- a/db/schema.rb +++ b/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" diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 61a89bc6adc..a6135bf88c3 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/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) } diff --git a/spec/helpers/enterprises_helper_spec.rb b/spec/helpers/enterprises_helper_spec.rb index 0933abb7480..2ad922a259c 100644 --- a/spec/helpers/enterprises_helper_spec.rb +++ b/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')