diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index adfa2b479d..1d8dda53ff 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -167,11 +167,7 @@ module Spree end def tax_category - if self[:tax_category_id].nil? - TaxCategory.find_by(is_default: true) - else - TaxCategory.find(self[:tax_category_id]) - end + super || TaxCategory.find_by(is_default: true) end def price_with_fees(distributor, order_cycle) diff --git a/engines/dfc_provider/spec/requests/supplied_products_spec.rb b/engines/dfc_provider/spec/requests/supplied_products_spec.rb index 8f39d971b7..34236617ea 100644 --- a/engines/dfc_provider/spec/requests/supplied_products_spec.rb +++ b/engines/dfc_provider/spec/requests/supplied_products_spec.rb @@ -79,6 +79,11 @@ describe "SuppliedProducts", type: :request, swagger_doc: "dfc.yaml", rswag_auto end it "creates a product and variant" do |example| + # Despite requiring a tax catogory... + # https://github.com/openfoodfoundation/openfoodnetwork/issues/11212 + create(:tax_category, is_default: true) + Spree::Config.products_require_tax_category = true + expect { submit_request(example.metadata) } .to change { enterprise.supplied_products.count }.by(1) diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index 6e36c6ed46..f1d4cc7818 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -231,7 +231,6 @@ RSpec.configure do |config| config.include PreferencesHelper config.include OpenFoodNetwork::FiltersHelper config.include OpenFoodNetwork::EnterpriseGroupsHelper - config.include OpenFoodNetwork::ProductsHelper config.include OpenFoodNetwork::DistributionHelper config.include OpenFoodNetwork::HtmlHelper config.include ActionView::Helpers::DateHelper diff --git a/spec/controllers/api/v0/products_controller_spec.rb b/spec/controllers/api/v0/products_controller_spec.rb index b96081a57e..af6dbfe032 100644 --- a/spec/controllers/api/v0/products_controller_spec.rb +++ b/spec/controllers/api/v0/products_controller_spec.rb @@ -52,7 +52,20 @@ describe Api::V0::ProductsController, type: :controller do expect(response.status).to eq(404) end - include_examples "modifying product actions are restricted" + it "cannot create a new product if not an admin" do + api_post :create, product: { name: "Brand new product!" } + assert_unauthorized! + end + + it "cannot update a product" do + api_put :update, id: product.to_param, product: { name: "I hacked your store!" } + assert_unauthorized! + end + + it "cannot delete a product" do + api_delete :destroy, id: product.to_param + assert_unauthorized! + end end context "as an enterprise user" do diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index d729f08bd5..82317e9571 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -23,20 +23,35 @@ describe Spree::Variant do end describe "tax category" do - context "when a tax category is required" do - it "is invalid when a tax category is not provided" do - with_products_require_tax_category(true) do - expect(build_stubbed(:variant, tax_category_id: nil)).not_to be_valid - end - end + # `build_stubbed` avoids creating a tax category in the database. + subject(:variant) { build_stubbed(:variant) } + + it "is valid when empty by default" do + expect(variant.tax_category).to eq nil + expect(variant).to be_valid end - context "when a tax category is not required" do - it "is valid when a tax category is not provided" do - with_products_require_tax_category(false) do - expect(build_stubbed(:variant, tax_category_id: nil)).to be_valid - end - end + it "loads the default tax category" do + default = create(:tax_category, is_default: true) + + expect(variant.tax_category).to eq default + expect { + variant.tax_category = nil + }.to_not change { + variant.tax_category + } + expect(variant).to be_valid + end + + it "doesn't load any tax category" do + non_default = create(:tax_category, is_default: false) + expect(variant.tax_category).to eq nil + end + + context "when a tax category is required" do + before { Spree::Config.products_require_tax_category = true } + + it { is_expected.to validate_presence_of :tax_category } end end end diff --git a/spec/support/products_helper.rb b/spec/support/products_helper.rb deleted file mode 100644 index 1a865f3fd4..0000000000 --- a/spec/support/products_helper.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -module OpenFoodNetwork - module ProductsHelper - def with_products_require_tax_category(value) - original_value = Spree::Config.products_require_tax_category - - Spree::Config.products_require_tax_category = value - yield - ensure - Spree::Config.products_require_tax_category = original_value - end - - shared_examples "modifying product actions are restricted" do - it "cannot create a new product if not an admin" do - api_post :create, product: { name: "Brand new product!" } - assert_unauthorized! - end - - it "cannot update a product" do - api_put :update, id: product.to_param, product: { name: "I hacked your store!" } - assert_unauthorized! - end - - it "cannot delete a product" do - api_delete :destroy, id: product.to_param - assert_unauthorized! - end - end - end -end diff --git a/spec/system/admin/products_spec.rb b/spec/system/admin/products_spec.rb index 0a83938ce0..12f1cddee5 100644 --- a/spec/system/admin/products_spec.rb +++ b/spec/system/admin/products_spec.rb @@ -356,32 +356,30 @@ describe ' context "products do not require a tax category" do it "creating a new product" do - with_products_require_tax_category(false) do - visit spree.admin_products_path - click_link 'New Product' + visit spree.admin_products_path + click_link 'New Product' - fill_in 'product_name', with: 'A new product !!!' - fill_in 'product_price', with: '19.99' + fill_in 'product_name', with: 'A new product !!!' + fill_in 'product_price', with: '19.99' - expect(page).to have_selector('#product_supplier_id') - select 'Another Supplier', from: 'product_supplier_id' - select 'Weight (g)', from: 'product_variant_unit_with_scale' - fill_in 'product_unit_value', with: '500' - select taxon.name, from: "product_primary_taxon_id" - select 'None', from: "product_tax_category_id" + expect(page).to have_selector('#product_supplier_id') + select 'Another Supplier', from: 'product_supplier_id' + select 'Weight (g)', from: 'product_variant_unit_with_scale' + fill_in 'product_unit_value', with: '500' + select taxon.name, from: "product_primary_taxon_id" + select 'None', from: "product_tax_category_id" - # Should only have suppliers listed which the user can manage - expect(page).to have_select 'product_supplier_id', - with_options: [@supplier2.name, @supplier_permitted.name] - expect(page).not_to have_select 'product_supplier_id', with_options: [@supplier.name] + # Should only have suppliers listed which the user can manage + expect(page).to have_select 'product_supplier_id', + with_options: [@supplier2.name, @supplier_permitted.name] + expect(page).not_to have_select 'product_supplier_id', with_options: [@supplier.name] - click_button 'Create' + click_button 'Create' - expect(flash_message).to eq('Product "A new product !!!" has been successfully created!') - product = Spree::Product.find_by(name: 'A new product !!!') - expect(product.supplier).to eq(@supplier2) - expect(product.variants.first.tax_category).to be_nil - end + expect(flash_message).to eq('Product "A new product !!!" has been successfully created!') + product = Spree::Product.find_by(name: 'A new product !!!') + expect(product.supplier).to eq(@supplier2) + expect(product.variants.first.tax_category).to be_nil end end