From c3829ae64f587bc6f4f57f745730a64ae9c564b7 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Sun, 23 Nov 2014 15:18:16 +0000 Subject: [PATCH 1/6] Tax category dropdown on create product form --- .../spree/app_configuration_decorator.rb | 9 ++++++++ app/models/spree/product_decorator.rb | 3 ++- .../new/replace_form.html.haml.deface | 2 ++ ...ucts_require_tax_category.html.haml.deface | 6 +++++ spec/factories.rb | 5 +++++ spec/features/admin/products_spec.rb | 5 +++++ spec/models/spree/product_spec.rb | 22 +++++++++++++++++++ 7 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 app/models/spree/app_configuration_decorator.rb create mode 100644 app/overrides/spree/admin/tax_settings/edit/add_products_require_tax_category.html.haml.deface diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb new file mode 100644 index 0000000000..f4f4acc5ec --- /dev/null +++ b/app/models/spree/app_configuration_decorator.rb @@ -0,0 +1,9 @@ +Spree::AppConfiguration.class_eval do + # This file decorates the existing preferences file defined by Spree. + # It allows us to add our own global configuration variables, which + # we can allow to be modified in the UI by adding appropriate form + # elements to existing or new configuration pages. + + # Tax Preferences + preference :products_require_tax_category, :boolean, default: false +end \ No newline at end of file diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 5b3460a073..9d7eb18d80 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -20,7 +20,8 @@ Spree::Product.class_eval do validates_associated :master, message: "^Price and On Hand must be valid" validates_presence_of :supplier validates :primary_taxon, presence: { message: "^Product Category can't be blank" } - + validates :tax_category_id, presence: { message: "^Tax Category can't be blank" }, if: "Spree::Config.products_require_tax_category" + validates_presence_of :variant_unit, if: :has_variants? validates_presence_of :variant_unit_scale, if: -> p { %w(weight volume).include? p.variant_unit } diff --git a/app/overrides/spree/admin/products/new/replace_form.html.haml.deface b/app/overrides/spree/admin/products/new/replace_form.html.haml.deface index 7c627eec4e..974e31f3ee 100644 --- a/app/overrides/spree/admin/products/new/replace_form.html.haml.deface +++ b/app/overrides/spree/admin/products/new/replace_form.html.haml.deface @@ -38,6 +38,8 @@ .twelve.columns.alpha .six.columns.alpha = render 'spree/admin/products/primary_taxon_form', f: f + - if Spree::TaxCategory.any? + = render 'spree/admin/products/tax_category_form', f: f .three.columns = f.field_container :price do = f.label :price, t(:price) diff --git a/app/overrides/spree/admin/tax_settings/edit/add_products_require_tax_category.html.haml.deface b/app/overrides/spree/admin/tax_settings/edit/add_products_require_tax_category.html.haml.deface new file mode 100644 index 0000000000..588669f005 --- /dev/null +++ b/app/overrides/spree/admin/tax_settings/edit/add_products_require_tax_category.html.haml.deface @@ -0,0 +1,6 @@ +/ insert_before "[data-hook='shipment_vat']" + +%div.field.align-center{ "data-hook" => "products_require_tax_category" } + = hidden_field_tag 'preferences[products_require_tax_category]', '0' + = check_box_tag 'preferences[products_require_tax_category]', '1', Spree::Config[:products_require_tax_category] + = label_tag nil, t(:products_require_tax_category) \ No newline at end of file diff --git a/spec/factories.rb b/spec/factories.rb index 0112ea8b51..6f86f430d4 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -175,6 +175,11 @@ FactoryGirl.define do order.reload end end + + factory :simple_tax_category, :class => Spree::TaxCategory do + name "Test Tax Category" + description "Test tax category description" + end end diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb index 1f61b07d2a..c42a57ce98 100644 --- a/spec/features/admin/products_spec.rb +++ b/spec/features/admin/products_spec.rb @@ -16,11 +16,13 @@ feature %q{ describe "creating a product" do scenario "assigning a important attributes", js: true do + tax_category = create(:simple_tax_category) login_to_admin_section click_link 'Products' click_link 'New Product' + select 'Test Tax Category', from: 'product_tax_category_id' select 'New supplier', from: 'product_supplier_id' fill_in 'product_name', with: 'A new product !!!' select "Weight (kg)", from: 'product_variant_unit_with_scale' @@ -34,6 +36,7 @@ feature %q{ flash_message.should == 'Product "A new product !!!" has been successfully created!' product = Spree::Product.find_by_name('A new product !!!') + product.tax_category_id.should == tax_category.id product.supplier.should == @supplier product.variant_unit.should == 'weight' product.variant_unit_scale.should == 1000 @@ -117,6 +120,7 @@ feature %q{ page.should have_selector('#product_supplier_id') select 'Another Supplier', :from => 'product_supplier_id' select taxon.name, from: "product_primary_taxon_id" + page.should have_select 'product_tax_category_id' if Spree::TaxCategory.any? # Should only have suppliers listed which the user can manage page.should have_select 'product_supplier_id', with_options: [@supplier2.name, @supplier_permitted.name] @@ -134,6 +138,7 @@ feature %q{ visit spree.edit_admin_product_path product + page.should have_select 'product_tax_category_id' select 'Permitted Supplier', from: 'product_supplier_id' click_button 'Update' flash_message.should == 'Product "a product" has been successfully updated!' diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index c6acae0850..f40e00f856 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -8,6 +8,28 @@ module Spree it { should belong_to(:primary_taxon) } it { should have_many(:product_distributions) } end + + describe "validating tax category" do + context "when a tax category is required" do + before { Spree::Config.products_require_tax_category = true } + + it "is invalid when a tax category is not provided" do + product = create(:product) + product.tax_category_id = nil + product.should_not be_valid + end + end + + context "when a tax category is not required" do + before { Spree::Config.products_require_tax_category = false } + + it "is valid when a tax category is not provided" do + product = create(:product) + product.tax_category_id = nil + product.should be_valid + end + end + end describe "validations and defaults" do it "is valid when created from factory" do From 451dd3966f4d69efb7ffb444ec9e8427c7fb1ef6 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Sun, 23 Nov 2014 15:22:56 +0000 Subject: [PATCH 2/6] form partial --- app/views/spree/admin/products/_tax_category_form.html.haml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 app/views/spree/admin/products/_tax_category_form.html.haml diff --git a/app/views/spree/admin/products/_tax_category_form.html.haml b/app/views/spree/admin/products/_tax_category_form.html.haml new file mode 100644 index 0000000000..9ab348636c --- /dev/null +++ b/app/views/spree/admin/products/_tax_category_form.html.haml @@ -0,0 +1,6 @@ += f.field_container :tax_category_id do + = f.label :tax_category_id, t(:tax_category) + * + %br + = f.collection_select(:tax_category_id, Spree::TaxCategory.all, :id, :name, {:include_blank => Spree::TaxCategory.count > 1}, {:class => "select2 fullwidth"}) + = f.error_message_on :tax_category_id From f90ee33c8936b05266117bf23f90abc7df6874c6 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 09:27:29 +1100 Subject: [PATCH 3/6] Use the tax category factory provided by Spree --- spec/factories.rb | 5 ----- spec/features/admin/products_spec.rb | 4 ++-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 2e3c8e223a..214c61962d 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -180,11 +180,6 @@ FactoryGirl.define do order.reload end end - - factory :simple_tax_category, :class => Spree::TaxCategory do - name "Test Tax Category" - description "Test tax category description" - end end diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb index c42a57ce98..f3141b765f 100644 --- a/spec/features/admin/products_spec.rb +++ b/spec/features/admin/products_spec.rb @@ -15,8 +15,8 @@ feature %q{ end describe "creating a product" do - scenario "assigning a important attributes", js: true do - tax_category = create(:simple_tax_category) + scenario "assigning important attributes", js: true do + tax_category = create(:tax_category, name: 'Test Tax Category') login_to_admin_section click_link 'Products' From a9b91bc52a7d8778b2f4a8e8ea54658842054b45 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 09:35:18 +1100 Subject: [PATCH 4/6] Tighten spec: setting tax category should succeed --- spec/features/admin/products_spec.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb index f3141b765f..0e27930f6e 100644 --- a/spec/features/admin/products_spec.rb +++ b/spec/features/admin/products_spec.rb @@ -6,6 +6,8 @@ feature %q{ } do include AuthenticationWorkflow include WebHelper + + let!(:taxon) { create(:taxon) } background do @@ -15,8 +17,9 @@ feature %q{ end describe "creating a product" do + let!(:tax_category) { create(:tax_category, name: 'Test Tax Category') } + scenario "assigning important attributes", js: true do - tax_category = create(:tax_category, name: 'Test Tax Category') login_to_admin_section click_link 'Products' @@ -88,6 +91,7 @@ feature %q{ end context "as an enterprise user" do + let!(:tax_category) { create(:tax_category) } before do @new_user = create_enterprise_user @@ -120,7 +124,7 @@ feature %q{ page.should have_selector('#product_supplier_id') select 'Another Supplier', :from => 'product_supplier_id' select taxon.name, from: "product_primary_taxon_id" - page.should have_select 'product_tax_category_id' if Spree::TaxCategory.any? + select tax_category.name, from: "product_tax_category_id" # Should only have suppliers listed which the user can manage page.should have_select 'product_supplier_id', with_options: [@supplier2.name, @supplier_permitted.name] @@ -131,6 +135,7 @@ feature %q{ flash_message.should == 'Product "A new product !!!" has been successfully created!' product = Spree::Product.find_by_name('A new product !!!') product.supplier.should == @supplier2 + product.tax_category.should == tax_category end scenario "editing a product" do @@ -138,12 +143,13 @@ feature %q{ visit spree.edit_admin_product_path product - page.should have_select 'product_tax_category_id' select 'Permitted Supplier', from: 'product_supplier_id' + select tax_category.name, from: 'product_tax_category_id' click_button 'Update' flash_message.should == 'Product "a product" has been successfully updated!' product.reload product.supplier.should == @supplier_permitted + product.tax_category.should == tax_category end scenario "editing product distributions" do From e4efda2f961e6f5e1201052f1a617e4dbb804526 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 09:37:35 +1100 Subject: [PATCH 5/6] Move model spec into validations block --- spec/models/spree/product_spec.rb | 45 ++++++++++++++++--------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index 2a399ba30e..3e4999556d 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -9,28 +9,6 @@ module Spree it { should have_many(:product_distributions) } end - describe "validating tax category" do - context "when a tax category is required" do - before { Spree::Config.products_require_tax_category = true } - - it "is invalid when a tax category is not provided" do - product = create(:product) - product.tax_category_id = nil - product.should_not be_valid - end - end - - context "when a tax category is not required" do - before { Spree::Config.products_require_tax_category = false } - - it "is valid when a tax category is not provided" do - product = create(:product) - product.tax_category_id = nil - product.should be_valid - end - end - end - describe "validations and defaults" do it "is valid when created from factory" do create(:product).should be_valid @@ -63,6 +41,29 @@ module Spree product.available_on.should == Time.now end + describe "tax category" do + context "when a tax category is required" do + before { Spree::Config.products_require_tax_category = true } + + it "is invalid when a tax category is not provided" do + product = create(:product) + product.tax_category_id = nil + product.should_not be_valid + end + end + + context "when a tax category is not required" do + before { Spree::Config.products_require_tax_category = false } + + it "is valid when a tax category is not provided" do + product = create(:product) + product.tax_category_id = nil + product.should be_valid + end + end + end + + context "when the product has variants" do let(:product) do product = create(:simple_product) From ac59665e3cb2164fe745b6cd020a36de1816792d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 09:48:33 +1100 Subject: [PATCH 6/6] Test validations without creating models in database --- spec/models/spree/product_spec.rb | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index 3e4999556d..3b3f5ec0a0 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -10,21 +10,16 @@ module Spree end describe "validations and defaults" do - it "is valid when created from factory" do - create(:product).should be_valid + it "is valid when built from factory" do + build(:product).should be_valid end it "requires a primary taxon" do - product = create(:simple_product) - product.taxons = [] - product.primary_taxon = nil - product.should_not be_valid + build(:simple_product, taxons: [], primary_taxon: nil).should_not be_valid end it "requires a supplier" do - product = create(:simple_product) - product.supplier = nil - product.should_not be_valid + build(:simple_product, supplier: nil).should_not be_valid end it "does not save when master is invalid" do @@ -46,9 +41,7 @@ module Spree before { Spree::Config.products_require_tax_category = true } it "is invalid when a tax category is not provided" do - product = create(:product) - product.tax_category_id = nil - product.should_not be_valid + build(:product, tax_category_id: nil).should_not be_valid end end @@ -56,9 +49,7 @@ module Spree before { Spree::Config.products_require_tax_category = false } it "is valid when a tax category is not provided" do - product = create(:product) - product.tax_category_id = nil - product.should be_valid + build(:product, tax_category_id: nil).should be_valid end end end