From 592a53b6f552bd86c4ff3e8b2dad5699e7bec1bc Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Fri, 13 Sep 2019 20:28:09 -0500 Subject: [PATCH 01/13] Remove blank option from shipping category dropdown --- .../spree/admin/products/_shipping_category_form.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/products/_shipping_category_form.html.haml b/app/views/spree/admin/products/_shipping_category_form.html.haml index 1e324a25b1..0f3635d698 100644 --- a/app/views/spree/admin/products/_shipping_category_form.html.haml +++ b/app/views/spree/admin/products/_shipping_category_form.html.haml @@ -1,4 +1,4 @@ = f.field_container :shipping_categories do = f.label :shipping_category_id, t(:shipping_category) - = f.collection_select(:shipping_category_id, Spree::ShippingCategory.all, :id, :name, {:include_blank => true}, {:class => 'select2 fullwidth'}) + = f.collection_select(:shipping_category_id, Spree::ShippingCategory.all, :id, :name, {:include_blank => false}, {:class => 'select2 fullwidth'}) = f.error_message_on :shipping_category_id From 35f89a975078960f9e752a0eea2c8a8ff20fd9c4 Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Sun, 15 Sep 2019 16:25:00 -0500 Subject: [PATCH 02/13] Update spec to prefill shipping category in Create form --- spec/features/admin/products_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb index 1756cdc539..1565b3b546 100644 --- a/spec/features/admin/products_spec.rb +++ b/spec/features/admin/products_spec.rb @@ -32,6 +32,8 @@ feature ' click_link 'Products' click_link 'New Product' + expect(find_field('product_shipping_category_id').text).to eq(shipping_category.name) + select 'New supplier', from: 'product_supplier_id' fill_in 'product_name', with: 'A new product !!!' select "Weight (kg)", from: 'product_variant_unit_with_scale' @@ -40,7 +42,6 @@ feature ' fill_in 'product_price', with: '19.99' fill_in 'product_on_hand', with: 5 select 'Test Tax Category', from: 'product_tax_category_id' - select 'Test Shipping Category', from: 'product_shipping_category_id' page.find("div[id^='taTextElement']").native.send_keys('A description...') click_button 'Create' From b4be2cc2d41ebbce5f22852b874ca0457124b034 Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Tue, 17 Sep 2019 23:02:19 -0500 Subject: [PATCH 03/13] Add default shipping category service and update create product form --- app/services/default_shipping_category.rb | 13 +++++++++++++ .../products/_shipping_category_form.html.haml | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 app/services/default_shipping_category.rb diff --git a/app/services/default_shipping_category.rb b/app/services/default_shipping_category.rb new file mode 100644 index 0000000000..ae515fa59a --- /dev/null +++ b/app/services/default_shipping_category.rb @@ -0,0 +1,13 @@ +# Encapsulates the concept of default stock location in creation of a product or a shipping method. + +class DefaultShippingCategory + NAME = 'Default'.freeze + + def self.create! + Spree::ShippingCategory.create!(name: NAME) + end + + def self.find_or_create + Spree::ShippingCategory.find_or_create_by_name(NAME) + end +end \ No newline at end of file diff --git a/app/views/spree/admin/products/_shipping_category_form.html.haml b/app/views/spree/admin/products/_shipping_category_form.html.haml index 0f3635d698..6e8e1a5dbd 100644 --- a/app/views/spree/admin/products/_shipping_category_form.html.haml +++ b/app/views/spree/admin/products/_shipping_category_form.html.haml @@ -1,4 +1,4 @@ = f.field_container :shipping_categories do = f.label :shipping_category_id, t(:shipping_category) - = f.collection_select(:shipping_category_id, Spree::ShippingCategory.all, :id, :name, {:include_blank => false}, {:class => 'select2 fullwidth'}) + = f.collection_select(:shipping_category_id, Spree::ShippingCategory.all, :id, :name, {:include_blank => false, :selected => DefaultShippingCategory.find_or_create.id}, {:class => 'select2 fullwidth'}) = f.error_message_on :shipping_category_id From 89873a2640fd9f66aaab34c0b903c3e7608cd1ec Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Tue, 17 Sep 2019 23:39:14 -0500 Subject: [PATCH 04/13] Add and auto-check default category in shipping method create --- app/views/spree/admin/shipping_methods/_form.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/shipping_methods/_form.html.haml b/app/views/spree/admin/shipping_methods/_form.html.haml index dfef7418a1..43f5af4d20 100644 --- a/app/views/spree/admin/shipping_methods/_form.html.haml +++ b/app/views/spree/admin/shipping_methods/_form.html.haml @@ -53,7 +53,7 @@ = f.field_container :categories do - Spree::ShippingCategory.all.each do |category| = label_tag do - = check_box_tag('shipping_method[shipping_categories][]', category.id, @shipping_method.shipping_categories.include?(category)) + = check_box_tag('shipping_method[shipping_categories][]', category.id, category == DefaultShippingCategory.find_or_create) = category.name %br/ = error_message_on :shipping_method, :shipping_category_id From e2d341c9c26e9e761c7592a77a6b9702add1e534 Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Tue, 17 Sep 2019 23:41:37 -0500 Subject: [PATCH 05/13] Add default category to seeds --- db/seeds.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/seeds.rb b/db/seeds.rb index 1cb78e94ff..75d351b054 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -52,3 +52,4 @@ end require File.join(File.dirname(__FILE__), 'default', 'users') DefaultStockLocation.find_or_create +DefaultShippingCategory.find_or_create \ No newline at end of file From 48cd542138f2927743423b8c192e141300363d2f Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Wed, 18 Sep 2019 01:30:24 -0500 Subject: [PATCH 06/13] Service spec --- .../default_shipping_category_spec.rb | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 spec/services/default_shipping_category_spec.rb diff --git a/spec/services/default_shipping_category_spec.rb b/spec/services/default_shipping_category_spec.rb new file mode 100644 index 0000000000..a2e3dd0ad1 --- /dev/null +++ b/spec/services/default_shipping_category_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe DefaultShippingCategory do + describe '.create!' do + it "names the location 'Default'" do + shipping_category = described_class.create! + expect(shipping_category.name).to eq('Default') + end + end + + describe 'find_or_create' do + context 'when a Default category already exists' do + let!(:category) do + Spree::ShippingCategory.create!(name: 'Default') + end + + it 'returns the category' do + expect(described_class.find_or_create).to eq(category) + end + + it 'does not create another category' do + expect { described_class.find_or_create }.not_to change(Spree::ShippingCategory, :count) + end + end + + context 'when a Default category does not exist' do + it 'returns the category' do + category = described_class.find_or_create + expect(category.name).to eq('Default') + end + + it 'does not create another category' do + expect { described_class.find_or_create } + .to change(Spree::ShippingCategory, :count).from(0).to(1) + end + end + end + +end \ No newline at end of file From b082d3301b6ae91acc1d88ffd15ee85ad64b9d5d Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Wed, 18 Sep 2019 18:41:10 -0500 Subject: [PATCH 07/13] Add prechecked category to shipping method spec --- spec/features/admin/shipping_methods_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/features/admin/shipping_methods_spec.rb b/spec/features/admin/shipping_methods_spec.rb index 53e58c595a..3b1c9386ea 100644 --- a/spec/features/admin/shipping_methods_spec.rb +++ b/spec/features/admin/shipping_methods_spec.rb @@ -75,6 +75,7 @@ feature 'shipping methods' do let(:sm1) { create(:shipping_method, name: 'One', distributors: [distributor1]) } let(:sm2) { create(:shipping_method, name: 'Two', distributors: [distributor1, distributor2]) } let(:sm3) { create(:shipping_method, name: 'Three', distributors: [distributor3]) } + let(:shipping_category) { DefaultShippingCategory.find_or_create } before(:each) do enterprise_user.enterprise_roles.build(enterprise: distributor1).save @@ -88,6 +89,7 @@ feature 'shipping methods' do within(".side_menu") do click_link "Shipping Methods" end + DefaultShippingCategory.find_or_create click_link 'Create One Now' # Show the correct fields @@ -97,10 +99,12 @@ feature 'shipping methods' do expect(page).to have_css 'div#shipping_method_zones_field' expect(page).to have_field 'shipping_method_require_ship_address_true', checked: true + # Auto-check default shipping category + expect(page).to have_field shipping_category.name, checked: true + fill_in 'shipping_method_name', with: 'Teleport' check "shipping_method_distributor_ids_#{distributor1.id}" - check "shipping_method_shipping_categories_" find(:css, "tags-input .tags input").set "local\n" within(".tags .tag-list") do expect(page).to have_css '.tag-item', text: "local" From d6022062e1a1b5537cbd0bccf2f7ca9aec60f2e3 Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Wed, 18 Sep 2019 18:52:50 -0500 Subject: [PATCH 08/13] Use default for create product spec; auto-fill field --- spec/features/admin/products_spec.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb index 1565b3b546..9e1d0a4895 100644 --- a/spec/features/admin/products_spec.rb +++ b/spec/features/admin/products_spec.rb @@ -9,7 +9,7 @@ feature ' let!(:taxon) { create(:taxon) } let!(:stock_location) { create(:stock_location, backorderable_default: false) } - let!(:shipping_category) { create(:shipping_category, name: 'Test Shipping Category') } + let!(:shipping_category) { DefaultShippingCategory.find_or_create } background do @supplier = create(:supplier_enterprise, name: 'New supplier') @@ -81,7 +81,6 @@ feature ' fill_in 'product_on_hand', with: 0 check 'product_on_demand' select 'Test Tax Category', from: 'product_tax_category_id' - select 'Test Shipping Category', from: 'product_shipping_category_id' page.find("div[id^='taTextElement']").native.send_keys('In demand, and on_demand! The hottest cakes in town.') click_button 'Create' @@ -123,7 +122,6 @@ feature ' select 'Weight (g)', from: 'product_variant_unit_with_scale' fill_in 'product_unit_value_with_description', with: '500' select taxon.name, from: "product_primary_taxon_id" - select 'Test Shipping Category', from: 'product_shipping_category_id' select 'None', from: "product_tax_category_id" # Should only have suppliers listed which the user can manage From dbf34da87b245fa4bc6d6b99d50203c4ce1edb01 Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Wed, 18 Sep 2019 19:19:17 -0500 Subject: [PATCH 09/13] Rubocop fixes --- app/services/default_shipping_category.rb | 16 +++--- db/seeds.rb | 2 +- .../default_shipping_category_spec.rb | 49 +++++++++---------- 3 files changed, 33 insertions(+), 34 deletions(-) diff --git a/app/services/default_shipping_category.rb b/app/services/default_shipping_category.rb index ae515fa59a..10959de659 100644 --- a/app/services/default_shipping_category.rb +++ b/app/services/default_shipping_category.rb @@ -1,13 +1,13 @@ # Encapsulates the concept of default stock location in creation of a product or a shipping method. class DefaultShippingCategory - NAME = 'Default'.freeze + NAME = 'Default'.freeze - def self.create! - Spree::ShippingCategory.create!(name: NAME) - end + def self.create! + Spree::ShippingCategory.create!(name: NAME) + end - def self.find_or_create - Spree::ShippingCategory.find_or_create_by_name(NAME) - end -end \ No newline at end of file + def self.find_or_create + Spree::ShippingCategory.find_or_create_by_name(NAME) + end +end diff --git a/db/seeds.rb b/db/seeds.rb index 75d351b054..e6cac36c2c 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -52,4 +52,4 @@ end require File.join(File.dirname(__FILE__), 'default', 'users') DefaultStockLocation.find_or_create -DefaultShippingCategory.find_or_create \ No newline at end of file +DefaultShippingCategory.find_or_create diff --git a/spec/services/default_shipping_category_spec.rb b/spec/services/default_shipping_category_spec.rb index a2e3dd0ad1..24a718a262 100644 --- a/spec/services/default_shipping_category_spec.rb +++ b/spec/services/default_shipping_category_spec.rb @@ -1,18 +1,18 @@ require 'spec_helper' describe DefaultShippingCategory do - describe '.create!' do - it "names the location 'Default'" do - shipping_category = described_class.create! - expect(shipping_category.name).to eq('Default') - end - end + describe '.create!' do + it "names the location 'Default'" do + shipping_category = described_class.create! + expect(shipping_category.name).to eq('Default') + end + end - describe 'find_or_create' do - context 'when a Default category already exists' do - let!(:category) do - Spree::ShippingCategory.create!(name: 'Default') - end + describe 'find_or_create' do + context 'when a Default category already exists' do + let!(:category) do + Spree::ShippingCategory.create!(name: 'Default') + end it 'returns the category' do expect(described_class.find_or_create).to eq(category) @@ -21,19 +21,18 @@ describe DefaultShippingCategory do it 'does not create another category' do expect { described_class.find_or_create }.not_to change(Spree::ShippingCategory, :count) end - end + end - context 'when a Default category does not exist' do - it 'returns the category' do - category = described_class.find_or_create - expect(category.name).to eq('Default') - end + context 'when a Default category does not exist' do + it 'returns the category' do + category = described_class.find_or_create + expect(category.name).to eq('Default') + end - it 'does not create another category' do - expect { described_class.find_or_create } - .to change(Spree::ShippingCategory, :count).from(0).to(1) - end - end - end - -end \ No newline at end of file + it 'does not create another category' do + expect { described_class.find_or_create } + .to change(Spree::ShippingCategory, :count).from(0).to(1) + end + end + end +end From 560fa6b9490c30befd4a34dbff8989d23f768d78 Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Wed, 25 Sep 2019 12:38:06 -0500 Subject: [PATCH 10/13] Update shipping category factory with default --- lib/tasks/sample_data/product_factory.rb | 2 +- spec/factories.rb | 5 +++++ spec/features/admin/shipping_methods_spec.rb | 3 +-- spec/services/default_shipping_category_spec.rb | 8 ++++---- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/tasks/sample_data/product_factory.rb b/lib/tasks/sample_data/product_factory.rb index 09cc60d3d3..01e92dddb7 100644 --- a/lib/tasks/sample_data/product_factory.rb +++ b/lib/tasks/sample_data/product_factory.rb @@ -72,7 +72,7 @@ class ProductFactory variant_unit: "weight", variant_unit_scale: 1, unit_value: 1, - shipping_category: Spree::ShippingCategory.find_or_create_by_name('Default') + shipping_category: DefaultShippingCategory.find_or_create ) product = Spree::Product.create_with(params).find_or_create_by_name!(params[:name]) product.variants.first.update_attribute :on_demand, true diff --git a/spec/factories.rb b/spec/factories.rb index 450984f89c..4f9c6478f5 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -194,4 +194,9 @@ FactoryBot.modify do # sets the default value for variant.on_demand backorderable_default false end + + factory :shipping_category, class: Spree::ShippingCategory do + initialize_with { DefaultShippingCategory.find_or_create } + transient { name 'Default' } + end end diff --git a/spec/features/admin/shipping_methods_spec.rb b/spec/features/admin/shipping_methods_spec.rb index 3b1c9386ea..ca00af3457 100644 --- a/spec/features/admin/shipping_methods_spec.rb +++ b/spec/features/admin/shipping_methods_spec.rb @@ -75,7 +75,7 @@ feature 'shipping methods' do let(:sm1) { create(:shipping_method, name: 'One', distributors: [distributor1]) } let(:sm2) { create(:shipping_method, name: 'Two', distributors: [distributor1, distributor2]) } let(:sm3) { create(:shipping_method, name: 'Three', distributors: [distributor3]) } - let(:shipping_category) { DefaultShippingCategory.find_or_create } + let(:shipping_category) { create(:shipping_category) } before(:each) do enterprise_user.enterprise_roles.build(enterprise: distributor1).save @@ -89,7 +89,6 @@ feature 'shipping methods' do within(".side_menu") do click_link "Shipping Methods" end - DefaultShippingCategory.find_or_create click_link 'Create One Now' # Show the correct fields diff --git a/spec/services/default_shipping_category_spec.rb b/spec/services/default_shipping_category_spec.rb index 24a718a262..0afef46cbe 100644 --- a/spec/services/default_shipping_category_spec.rb +++ b/spec/services/default_shipping_category_spec.rb @@ -4,18 +4,18 @@ describe DefaultShippingCategory do describe '.create!' do it "names the location 'Default'" do shipping_category = described_class.create! - expect(shipping_category.name).to eq('Default') + expect(shipping_category.name).to eq 'Default' end end describe 'find_or_create' do context 'when a Default category already exists' do - let!(:category) do + let! :category do Spree::ShippingCategory.create!(name: 'Default') end it 'returns the category' do - expect(described_class.find_or_create).to eq(category) + expect(described_class.find_or_create).to eq category end it 'does not create another category' do @@ -26,7 +26,7 @@ describe DefaultShippingCategory do context 'when a Default category does not exist' do it 'returns the category' do category = described_class.find_or_create - expect(category.name).to eq('Default') + expect(category.name).to eq 'Default' end it 'does not create another category' do From 543e275d2e3d2bb1964668899ebc2d2fb4a82ec7 Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Thu, 26 Sep 2019 22:21:04 -0500 Subject: [PATCH 11/13] Add custom shipping category to pass package spec --- spec/models/stock/package_spec.rb | 2 +- spec/services/default_shipping_category_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/stock/package_spec.rb b/spec/models/stock/package_spec.rb index 6b3fe4012a..726eed6c37 100644 --- a/spec/models/stock/package_spec.rb +++ b/spec/models/stock/package_spec.rb @@ -48,7 +48,7 @@ module Stock describe '#shipping_categories' do it "returns shipping categories that are not shipping categories of the order's products" do package - other_shipping_category = create(:shipping_category) + other_shipping_category = Spree::ShippingCategory.create(name: "Custom") expect(package.shipping_categories).to eq [shipping_method1.shipping_categories.first, other_shipping_category] diff --git a/spec/services/default_shipping_category_spec.rb b/spec/services/default_shipping_category_spec.rb index 0afef46cbe..cab005df8a 100644 --- a/spec/services/default_shipping_category_spec.rb +++ b/spec/services/default_shipping_category_spec.rb @@ -10,7 +10,7 @@ describe DefaultShippingCategory do describe 'find_or_create' do context 'when a Default category already exists' do - let! :category do + let!(:category) do Spree::ShippingCategory.create!(name: 'Default') end From 9b7139fd45f7a680246d5f45cbe3e36de6a216ad Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Sat, 5 Oct 2019 01:11:12 -0500 Subject: [PATCH 12/13] Add default shipping category during object creation; revert forms --- app/controllers/spree/admin/products_controller_decorator.rb | 5 +++++ .../spree/admin/products/_shipping_category_form.html.haml | 2 +- app/views/spree/admin/shipping_methods/_form.html.haml | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index e05ba14792..22f5337ee5 100644 --- a/app/controllers/spree/admin/products_controller_decorator.rb +++ b/app/controllers/spree/admin/products_controller_decorator.rb @@ -30,6 +30,11 @@ Spree::Admin::ProductsController.class_eval do @show_latest_import = params[:latest_import] || false end + def new + @object.shipping_category = DefaultShippingCategory.find_or_create + super + end + def create delete_stock_params_and_set_after do super diff --git a/app/views/spree/admin/products/_shipping_category_form.html.haml b/app/views/spree/admin/products/_shipping_category_form.html.haml index 6e8e1a5dbd..0f3635d698 100644 --- a/app/views/spree/admin/products/_shipping_category_form.html.haml +++ b/app/views/spree/admin/products/_shipping_category_form.html.haml @@ -1,4 +1,4 @@ = f.field_container :shipping_categories do = f.label :shipping_category_id, t(:shipping_category) - = f.collection_select(:shipping_category_id, Spree::ShippingCategory.all, :id, :name, {:include_blank => false, :selected => DefaultShippingCategory.find_or_create.id}, {:class => 'select2 fullwidth'}) + = f.collection_select(:shipping_category_id, Spree::ShippingCategory.all, :id, :name, {:include_blank => false}, {:class => 'select2 fullwidth'}) = f.error_message_on :shipping_category_id diff --git a/app/views/spree/admin/shipping_methods/_form.html.haml b/app/views/spree/admin/shipping_methods/_form.html.haml index 43f5af4d20..dfef7418a1 100644 --- a/app/views/spree/admin/shipping_methods/_form.html.haml +++ b/app/views/spree/admin/shipping_methods/_form.html.haml @@ -53,7 +53,7 @@ = f.field_container :categories do - Spree::ShippingCategory.all.each do |category| = label_tag do - = check_box_tag('shipping_method[shipping_categories][]', category.id, category == DefaultShippingCategory.find_or_create) + = check_box_tag('shipping_method[shipping_categories][]', category.id, @shipping_method.shipping_categories.include?(category)) = category.name %br/ = error_message_on :shipping_method, :shipping_category_id From bdcadf9fc60fc4324b7e6f5664446cc11a1e6b4f Mon Sep 17 00:00:00 2001 From: Dan Ingenthron Date: Tue, 8 Oct 2019 18:35:20 -0500 Subject: [PATCH 13/13] Update changes to reflect new non-Spree shipping method controller --- app/controllers/spree/admin/shipping_methods_controller.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/controllers/spree/admin/shipping_methods_controller.rb b/app/controllers/spree/admin/shipping_methods_controller.rb index d4c9c29898..b561759757 100644 --- a/app/controllers/spree/admin/shipping_methods_controller.rb +++ b/app/controllers/spree/admin/shipping_methods_controller.rb @@ -19,6 +19,11 @@ module Spree collection end + def new + @object.shipping_categories = [DefaultShippingCategory.find_or_create] + super + end + def destroy # Our reports are not adapted to soft deleted shipping_methods so here we prevent # the deletion (even soft) of shipping_methods that are referenced in orders