Merge pull request #12038 from mkllnk/dfc-tax-category

Spec requiring tax category when creating products

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)

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)
 

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

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

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

spec/support/products_helper.rb

@@ -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

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