From d33e780411fcd64e76f1db5f5878d19e4cc711b8 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 12:44:39 +0100 Subject: [PATCH 01/16] Move tax_category association from product to variant --- app/models/spree/product.rb | 11 ----------- app/models/spree/variant.rb | 18 ++++++++++++++---- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index b0b65b956a..9a1cfd27ba 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -31,7 +31,6 @@ module Spree searchable_associations :supplier, :properties, :primary_taxon, :variants searchable_scopes :active, :with_properties - belongs_to :tax_category, class_name: 'Spree::TaxCategory' belongs_to :shipping_category, class_name: 'Spree::ShippingCategory' belongs_to :supplier, class_name: 'Enterprise', touch: true belongs_to :primary_taxon, class_name: 'Spree::Taxon', touch: true @@ -59,8 +58,6 @@ module Spree validates :supplier, presence: true validates :primary_taxon, presence: true - validates :tax_category, presence: true, - if: proc { Spree::Config[:products_require_tax_category] } validates :variant_unit, presence: true validates :unit_value, presence: @@ -205,14 +202,6 @@ module Spree group(column_names.map { |col_name| "#{table_name}.#{col_name}" }) end - def tax_category - if self[:tax_category_id].nil? - TaxCategory.find_by(is_default: true) - else - TaxCategory.find(self[:tax_category_id]) - end - end - # for adding products which are closely related to existing ones def duplicate duplicator = Spree::Core::ProductDuplicator.new(self) diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 0c98ba005d..16536cb60d 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -28,9 +28,10 @@ module Spree supplier_name).join('_or_')}_cont".freeze belongs_to :product, -> { with_deleted }, touch: true, class_name: 'Spree::Product' + belongs_to :tax_category, class_name: 'Spree::TaxCategory' - delegate_belongs_to :product, :name, :description, :tax_category_id, :shipping_category_id, - :meta_keywords, :tax_category, :shipping_category + delegate_belongs_to :product, :name, :description, :shipping_category_id, + :meta_keywords, :shipping_category has_many :inventory_units, inverse_of: :variant has_many :line_items, inverse_of: :variant @@ -61,8 +62,9 @@ module Spree localize_number :price, :weight validate :check_currency - validates :price, numericality: { greater_than_or_equal_to: 0 }, - presence: true + validates :price, numericality: { greater_than_or_equal_to: 0 }, presence: true + validates :tax_category, presence: true, + if: proc { Spree::Config[:products_require_tax_category] } validates :unit_value, presence: true, if: ->(variant) { %w(weight volume).include?(variant.product&.variant_unit) @@ -166,6 +168,14 @@ module Spree select("spree_variants.id")) end + def tax_category + if self[:tax_category_id].nil? + TaxCategory.find_by(is_default: true) + else + TaxCategory.find(self[:tax_category_id]) + end + end + def price_with_fees(distributor, order_cycle) price + fees_for(distributor, order_cycle) end From 4934d09a4cfba74e0d824af33a6d92e8ce99d391 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 12:56:45 +0100 Subject: [PATCH 02/16] Update delegation and method chaining between product and tax_category --- app/components/product_component.rb | 2 +- app/models/calculator/default_tax.rb | 4 ++-- app/models/spree/line_item.rb | 4 ++-- app/services/tax_rate_finder.rb | 2 +- lib/open_food_network/enterprise_fee_applicator.rb | 2 +- lib/reporting/reports/orders_and_fulfillment/base.rb | 2 +- lib/reporting/reports/products_and_inventory/lettuce_share.rb | 2 +- spec/controllers/api/v0/orders_controller_spec.rb | 2 +- .../orders_cycle_supplier_totals_report_spec.rb | 4 ++-- spec/models/spree/line_item_spec.rb | 2 +- spec/system/admin/products_spec.rb | 4 ++-- 11 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/components/product_component.rb b/app/components/product_component.rb index c3f859116c..b6edcc0986 100644 --- a/app/components/product_component.rb +++ b/app/components/product_component.rb @@ -40,7 +40,7 @@ class ProductComponent < ViewComponentReflex::Component when 'on_demand' @product.on_demand when 'tax_category' - @product.tax_category.name + @product.variant.tax_category.name when 'inherits_properties' @product.inherits_properties when 'import_date' diff --git a/app/models/calculator/default_tax.rb b/app/models/calculator/default_tax.rb index f9c118905a..b6c8b1d3c2 100644 --- a/app/models/calculator/default_tax.rb +++ b/app/models/calculator/default_tax.rb @@ -45,7 +45,7 @@ module Calculator def line_items_total(order) matched_line_items = order.line_items.select do |line_item| - line_item.product.tax_category == rate.tax_category + line_item.variant.tax_category == rate.tax_category end matched_line_items.sum(&:total) @@ -70,7 +70,7 @@ module Calculator def applicable_rate?(applicator, line_item) fee = applicator.enterprise_fee (!fee.inherits_tax_category && fee.tax_category == rate.tax_category) || - (fee.inherits_tax_category && line_item.product.tax_category == rate.tax_category) + (fee.inherits_tax_category && line_item.variant.tax_category == rate.tax_category) end # Finds relevant fees for whole order, diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index ef0ea3a86b..17d678ebdf 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -121,7 +121,7 @@ module Spree def copy_tax_category return unless variant - self.tax_category = variant.product.tax_category + self.tax_category = variant.tax_category end def copy_dimensions @@ -192,7 +192,7 @@ module Spree end def tax_rates - product.tax_category&.tax_rates || [] + variant&.tax_category&.tax_rates || [] end def price_with_adjustments diff --git a/app/services/tax_rate_finder.rb b/app/services/tax_rate_finder.rb index e49d615976..351555e444 100644 --- a/app/services/tax_rate_finder.rb +++ b/app/services/tax_rate_finder.rb @@ -43,7 +43,7 @@ class TaxRateFinder def line_item_tax_category(enterprise_fee, line_item) if enterprise_fee.inherits_tax_category? - line_item.product.tax_category + line_item.variant.tax_category else enterprise_fee.tax_category end diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index 5a46117e19..db50ceabb5 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -37,7 +37,7 @@ module OpenFoodNetwork def tax_category(target) if target.is_a?(Spree::LineItem) && enterprise_fee.inherits_tax_category? - target.product.tax_category + target.variant.tax_category else enterprise_fee.tax_category end diff --git a/lib/reporting/reports/orders_and_fulfillment/base.rb b/lib/reporting/reports/orders_and_fulfillment/base.rb index 9d20695e09..4d223be657 100644 --- a/lib/reporting/reports/orders_and_fulfillment/base.rb +++ b/lib/reporting/reports/orders_and_fulfillment/base.rb @@ -61,7 +61,7 @@ module Reporting end def product_tax_category - proc { |line_items| line_items.first.variant.product.tax_category&.name } + proc { |line_items| line_items.first.variant.tax_category&.name } end def hub_name diff --git a/lib/reporting/reports/products_and_inventory/lettuce_share.rb b/lib/reporting/reports/products_and_inventory/lettuce_share.rb index 96100b1ecb..8311ff111d 100644 --- a/lib/reporting/reports/products_and_inventory/lettuce_share.rb +++ b/lib/reporting/reports/products_and_inventory/lettuce_share.rb @@ -39,7 +39,7 @@ module Reporting private def gst(variant) - tax_category = variant.product.tax_category + tax_category = variant.tax_category if tax_category && tax_category.tax_rates.present? tax_rate = tax_category.tax_rates.first line_item = mock_line_item(variant) diff --git a/spec/controllers/api/v0/orders_controller_spec.rb b/spec/controllers/api/v0/orders_controller_spec.rb index 9f29e41b9b..773a9170b8 100644 --- a/spec/controllers/api/v0/orders_controller_spec.rb +++ b/spec/controllers/api/v0/orders_controller_spec.rb @@ -276,7 +276,7 @@ module Api expect(json_response[:line_items].first[:variant][:product_name]) .to eq order.line_items.first.variant.product.name expect(json_response[:line_items].first[:tax_category_id]) - .to eq order.line_items.first.product.tax_category_id + .to eq order.line_items.first.variant.tax_category_id end end end diff --git a/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb b/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb index ebf1d44aae..6dcba9b807 100644 --- a/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb +++ b/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb @@ -110,7 +110,7 @@ describe Reporting::Reports::OrdersAndFulfillment::OrderCycleSupplierTotals do end it "has a variant row when product belongs to a tax category" do - product_tax_category_name = order.line_items.first.variant.product.tax_category.name + product_tax_category_name = order.line_items.first.variant.tax_category.name supplier_name_field = report_table.first[0] supplier_vat_status_field = report_table.first[-2] @@ -139,7 +139,7 @@ describe Reporting::Reports::OrdersAndFulfillment::OrderCycleSupplierTotals do end it "has a variant row when product belongs to a tax category" do - product_tax_category_name = order.line_items.first.variant.product.tax_category.name + product_tax_category_name = order.line_items.first.variant.tax_category.name supplier_name_field = report_table.first[0] supplier_vat_status_field = report_table.first[-2] diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 62db1f36be..30af1dde24 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -51,7 +51,7 @@ module Spree it "copies over a variant's tax category" do line_item.tax_category = nil line_item.copy_tax_category - expect(line_item.tax_category).to eq line_item.variant.product.tax_category + expect(line_item.tax_category).to eq line_item.variant.tax_category end end diff --git a/spec/system/admin/products_spec.rb b/spec/system/admin/products_spec.rb index 88703215fc..1f22b1a71d 100644 --- a/spec/system/admin/products_spec.rb +++ b/spec/system/admin/products_spec.rb @@ -109,7 +109,7 @@ describe ' expect(product.primary_taxon_id).to eq(taxon.id) expect(product.variants.first.price.to_s).to eq('19.99') expect(product.on_hand).to eq(5) - expect(product.tax_category_id).to eq(tax_category.id) + expect(product.variants.first.tax_category_id).to eq(tax_category.id) expect(product.shipping_category).to eq(shipping_category) expect(product.description).to eq("

A description...

") expect(product.group_buy).to be_falsey @@ -285,7 +285,7 @@ describe ' 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.tax_category).to be_nil + expect(product.variants.first.tax_category).to be_nil end end end From b12c130e02762e765b3ccf8441d67d972819f516 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 14:13:49 +0100 Subject: [PATCH 03/16] Add tax_category column to variants table --- db/migrate/20230703130854_add_tax_category_to_variants.rb | 5 +++++ db/schema.rb | 3 +++ 2 files changed, 8 insertions(+) create mode 100644 db/migrate/20230703130854_add_tax_category_to_variants.rb diff --git a/db/migrate/20230703130854_add_tax_category_to_variants.rb b/db/migrate/20230703130854_add_tax_category_to_variants.rb new file mode 100644 index 0000000000..89d531f42f --- /dev/null +++ b/db/migrate/20230703130854_add_tax_category_to_variants.rb @@ -0,0 +1,5 @@ +class AddTaxCategoryToVariants < ActiveRecord::Migration[7.0] + def change + add_reference :spree_variants, :tax_category, foreign_key: { to_table: :spree_tax_categories } + end +end diff --git a/db/schema.rb b/db/schema.rb index b5700a8ee9..e66524e88f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1053,8 +1053,10 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_20_080504) do t.string "unit_presentation" t.datetime "created_at", default: -> { "now()" }, null: false t.datetime "updated_at", default: -> { "now()" }, null: false + t.bigint "tax_category_id" t.index ["product_id"], name: "index_variants_on_product_id" t.index ["sku"], name: "index_spree_variants_on_sku" + t.index ["tax_category_id"], name: "index_spree_variants_on_tax_category_id" t.check_constraint "unit_value > 0::double precision", name: "positive_unit_value" end @@ -1286,6 +1288,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_20_080504) do add_foreign_key "spree_users", "spree_addresses", column: "bill_address_id", name: "spree_users_bill_address_id_fk" add_foreign_key "spree_users", "spree_addresses", column: "ship_address_id", name: "spree_users_ship_address_id_fk" add_foreign_key "spree_variants", "spree_products", column: "product_id", name: "spree_variants_product_id_fk" + add_foreign_key "spree_variants", "spree_tax_categories", column: "tax_category_id" add_foreign_key "spree_zone_members", "spree_zones", column: "zone_id", name: "spree_zone_members_zone_id_fk" add_foreign_key "subscription_line_items", "spree_variants", column: "variant_id", name: "subscription_line_items_variant_id_fk" add_foreign_key "subscription_line_items", "subscriptions", name: "subscription_line_items_subscription_id_fk" From a52516bcbf2d589132920f392bfc43bb736f52ea Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 14:27:12 +0100 Subject: [PATCH 04/16] Move tax_category dropdown in edit pages --- .../spree/admin/variants_controller.rb | 8 ++ .../spree/admin/products/_form.html.haml | 5 -- .../spree/admin/variants/_form.html.haml | 4 + spec/system/admin/products_spec.rb | 1 - spec/system/admin/variants_spec.rb | 79 ++++++++++--------- 5 files changed, 53 insertions(+), 44 deletions(-) diff --git a/app/controllers/spree/admin/variants_controller.rb b/app/controllers/spree/admin/variants_controller.rb index 6ccdd7f0f0..f1fc43483e 100644 --- a/app/controllers/spree/admin/variants_controller.rb +++ b/app/controllers/spree/admin/variants_controller.rb @@ -7,6 +7,8 @@ module Spree class VariantsController < ::Admin::ResourceController belongs_to 'spree/product' + before_action :load_data, only: [:new, :edit] + def index @url_filters = ::ProductFilters.new.extract(request.query_parameters) end @@ -107,6 +109,12 @@ module Spree :include_out_of_stock ).to_h.with_indifferent_access end + + private + + def load_data + @tax_categories = TaxCategory.order(:name) + end end end end diff --git a/app/views/spree/admin/products/_form.html.haml b/app/views/spree/admin/products/_form.html.haml index 745b542fe2..6fb51e7c10 100644 --- a/app/views/spree/admin/products/_form.html.haml +++ b/app/views/spree/admin/products/_form.html.haml @@ -43,11 +43,6 @@ = f.collection_select(:shipping_category_id, @shipping_categories, :id, :name, {}, { :class => 'select2' }) = f.error_message_on :shipping_category - = f.field_container :tax_category do - = f.label :tax_category_id, t(:tax_category) - = f.collection_select(:tax_category_id, @tax_categories, :id, :name, { :include_blank => t(:none) }, { :class => 'select2' }) - = f.error_message_on :tax_category - .clear %div diff --git a/app/views/spree/admin/variants/_form.html.haml b/app/views/spree/admin/variants/_form.html.haml index 4fcf2d2449..3f8f8d3021 100644 --- a/app/views/spree/admin/variants/_form.html.haml +++ b/app/views/spree/admin/variants/_form.html.haml @@ -64,4 +64,8 @@ - value = number_with_precision(@variant.send(field), precision: 2) = f.number_field field, value: value, class: 'fullwidth', step: 0.01 + .field + = f.label :tax_category_id, t(:tax_category) + = f.collection_select(:tax_category_id, @tax_categories, :id, :name, { include_blank: t(:none) }, { class: 'select2 fullwidth' }) + .clear diff --git a/spec/system/admin/products_spec.rb b/spec/system/admin/products_spec.rb index 1f22b1a71d..bf5b7ec794 100644 --- a/spec/system/admin/products_spec.rb +++ b/spec/system/admin/products_spec.rb @@ -296,7 +296,6 @@ describe ' visit spree.edit_admin_product_path product select 'Permitted Supplier', from: 'product_supplier_id' - select tax_category.name, from: 'product_tax_category_id' click_button 'Update' expect(flash_message).to eq('Product "a product" has been successfully updated!') product.reload diff --git a/spec/system/admin/variants_spec.rb b/spec/system/admin/variants_spec.rb index f95bc91d95..0a7e462d03 100644 --- a/spec/system/admin/variants_spec.rb +++ b/spec/system/admin/variants_spec.rb @@ -290,53 +290,56 @@ describe ' expect(variant.reload.deleted_at).not_to be_nil end - it "editing display name for a variant" do - product = create(:simple_product) - variant = product.variants.first + describe "editing variant attributes" do + let!(:variant) { create(:variant) } + let(:product) { variant.product } + let!(:tax_category) { create(:tax_category) } - # When I view the variant - login_as_admin - visit spree.admin_product_variants_path product - page.find('table.index .icon-edit').click + before do + login_as_admin + visit spree.edit_admin_product_variant_path product, variant + end - # It should allow the display name to be changed - expect(page).to have_field "variant_display_name" - expect(page).to have_field "variant_display_as" + it "editing display name for a variant" do + # It should allow the display name to be changed + expect(page).to have_field "variant_display_name" + expect(page).to have_field "variant_display_as" - # When I update the fields and save the variant - fill_in "variant_display_name", with: "Display Name" - fill_in "variant_display_as", with: "Display As This" - click_button 'Update' - expect(page).to have_content %(Variant "#{product.name}" has been successfully updated!) + # When I update the fields and save the variant + fill_in "variant_display_name", with: "Display Name" + fill_in "variant_display_as", with: "Display As This" + click_button 'Update' + expect(page).to have_content %(Variant "#{product.name}" has been successfully updated!) - # Then the displayed values should have been saved - expect(variant.reload.display_name).to eq("Display Name") - expect(variant.display_as).to eq("Display As This") - end + # Then the displayed values should have been saved + expect(variant.reload.display_name).to eq("Display Name") + expect(variant.display_as).to eq("Display As This") + end - it "editing weight for a variant" do - product = create(:simple_product) - variant = product.variants.first + it "editing weight for a variant" do + # It should allow the weight to be changed + expect(page).to have_field "unit_value_human" - # When I view the variant - login_as_admin - visit spree.admin_product_variants_path product + # When I update the fields and save the variant with invalid value + fill_in "unit_value_human", with: "1.234" + click_button 'Update' + expect(page).not_to have_content %(Variant "#{product.name}" has been successfully updated!) - page.find('table.index .icon-edit').click + fill_in "unit_value_human", with: "1.23" + click_button 'Update' + expect(page).not_to have_content %(Variant "#{product.name}" has been successfully updated!) - # It should allow the weight to be changed - expect(page).to have_field "unit_value_human" + # Then the displayed values should have been saved + expect(variant.reload.unit_value).to eq(1.23) + end - # When I update the fields and save the variant with invalid value - fill_in "unit_value_human", with: "1.234" - click_button 'Update' - expect(page).not_to have_content %(Variant "#{product.name}" has been successfully updated!) + context "editing variant tax category" do + it "editing variant tax category" do + select2_select tax_category.name, from: 'variant_tax_category_id' + click_button 'Update' - fill_in "unit_value_human", with: "1.23" - click_button 'Update' - expect(page).not_to have_content %(Variant "#{product.name}" has been successfully updated!) - - # Then the displayed values should have been saved - expect(variant.reload.unit_value).to eq(1.23) + expect(variant.reload.tax_category).to eq tax_category + end + end end end From 363bbded434224f4ce23f8e82c85d344172c9cee Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 14:27:30 +0100 Subject: [PATCH 05/16] Update permitted attributes --- app/services/permitted_attributes/variant.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/permitted_attributes/variant.rb b/app/services/permitted_attributes/variant.rb index f6d22c42ac..8a834d4d84 100644 --- a/app/services/permitted_attributes/variant.rb +++ b/app/services/permitted_attributes/variant.rb @@ -6,7 +6,7 @@ module PermittedAttributes [ :id, :sku, :on_hand, :on_demand, :price, :unit_value, :unit_description, - :display_name, :display_as, + :display_name, :display_as, :tax_category_id, :weight, :height, :width, :depth ] end From 5a48721af46b8fc8d2b54a3ff9492cd3fdeeb95a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 14:36:10 +0100 Subject: [PATCH 06/16] Update serializers, views, and JS for bulk products edit --- app/assets/javascripts/admin/bulk_product_update.js.coffee | 7 ++++--- app/serializers/api/admin/product_serializer.rb | 4 ++-- app/serializers/api/admin/variant_serializer.rb | 2 +- .../spree/admin/products/index/_products_product.html.haml | 2 -- .../spree/admin/products/index/_products_variant.html.haml | 2 ++ 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 8d71c0ff46..9f0c5a7a81 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -135,6 +135,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout display_name: null on_hand: null price: null + tax_category_id: null DisplayProperties.setShowVariants product.id, true @@ -346,9 +347,6 @@ filterSubmitProducts = (productsToFilter) -> if product.hasOwnProperty("category_id") filteredProduct.primary_taxon_id = product.category_id hasUpdatableProperty = true - if product.hasOwnProperty("tax_category_id") - filteredProduct.tax_category_id = product.tax_category_id - hasUpdatableProperty = true if product.hasOwnProperty("inherits_properties") filteredProduct.inherits_properties = product.inherits_properties hasUpdatableProperty = true @@ -389,6 +387,9 @@ filterSubmitVariant = (variant) -> if variant.hasOwnProperty("display_name") filteredVariant.display_name = variant.display_name hasUpdatableProperty = true + if variant.hasOwnProperty("tax_category_id") + filteredVariant.tax_category_id = variant.tax_category_id + hasUpdatableProperty = true if variant.hasOwnProperty("display_as") filteredVariant.display_as = variant.display_as hasUpdatableProperty = true diff --git a/app/serializers/api/admin/product_serializer.rb b/app/serializers/api/admin/product_serializer.rb index f19b81f0a0..b3eb0225e6 100644 --- a/app/serializers/api/admin/product_serializer.rb +++ b/app/serializers/api/admin/product_serializer.rb @@ -4,8 +4,8 @@ module Api module Admin class ProductSerializer < ActiveModel::Serializer attributes :id, :name, :sku, :variant_unit, :variant_unit_scale, :variant_unit_name, - :inherits_properties, :on_hand, :price, :tax_category_id, :import_date, - :image_url, :thumb_url, :variants + :inherits_properties, :on_hand, :price, :import_date, :image_url, + :thumb_url, :variants has_one :supplier, key: :producer_id, embed: :id has_one :primary_taxon, key: :category_id, embed: :id diff --git a/app/serializers/api/admin/variant_serializer.rb b/app/serializers/api/admin/variant_serializer.rb index 4db58190f2..23b5dbd1e3 100644 --- a/app/serializers/api/admin/variant_serializer.rb +++ b/app/serializers/api/admin/variant_serializer.rb @@ -3,7 +3,7 @@ module Api module Admin class VariantSerializer < ActiveModel::Serializer - attributes :id, :name, :producer_name, :image, :sku, :import_date, + attributes :id, :name, :producer_name, :image, :sku, :import_date, :tax_category_id, :options_text, :unit_value, :unit_description, :unit_to_display, :display_as, :display_name, :name_to_display, :variant_overrides_count, :price, :on_demand, :on_hand, :in_stock, :stock_location_id, :stock_location_name diff --git a/app/views/spree/admin/products/index/_products_product.html.haml b/app/views/spree/admin/products/index/_products_product.html.haml index c5d617b336..d28782ae02 100644 --- a/app/views/spree/admin/products/index/_products_product.html.haml +++ b/app/views/spree/admin/products/index/_products_product.html.haml @@ -27,8 +27,6 @@ %td.category{ 'ng-if' => 'columns.category.visible' } %input.fullwidth{ :type => 'text', id: "p{{product.id}}_category_id", 'ng-model' => 'product.category_id', 'ofn-taxon-autocomplete' => '', 'ofn-track-product' => 'category_id', 'multiple-selection' => 'false', placeholder: 'Category' } %td.tax_category{ 'ng-if' => 'columns.tax_category.visible' } - %select.select2{ name: 'product_tax_category_id', 'ofn-track-product' => 'tax_category_id', ng: {model: 'product.tax_category_id', options: 'tax_category.id as tax_category.name for tax_category in tax_categories'} } - %option{value: ''}= t(:none) %td.inherits_properties{ 'ng-show' => 'columns.inherits_properties.visible' } %input{ 'ng-model' => 'product.inherits_properties', :name => 'inherits_properties', 'ofn-track-product' => 'inherits_properties', type: "checkbox" } %td.import_date{ 'ng-show' => 'columns.import_date.visible' } diff --git a/app/views/spree/admin/products/index/_products_variant.html.haml b/app/views/spree/admin/products/index/_products_variant.html.haml index 5cb42f051e..45a7ea24c6 100644 --- a/app/views/spree/admin/products/index/_products_variant.html.haml +++ b/app/views/spree/admin/products/index/_products_variant.html.haml @@ -22,6 +22,8 @@ %input.field{ 'ng-model' => 'variant.on_demand', :name => 'variant_on_demand', 'ofn-track-variant' => 'on_demand', :type => 'checkbox' } %td{ 'ng-show' => 'columns.category.visible' } %td{ 'ng-show' => 'columns.tax_category.visible' } + %select.select2{ name: 'variant_tax_category_id', 'ofn-track-variant': 'tax_category_id', ng: { model: 'variant.tax_category_id', options: 'tax_category.id as tax_category.name for tax_category in tax_categories' } } + %option{ value: '' }= t(:none) %td{ 'ng-show' => 'columns.inherits_properties.visible' } %td{ 'ng-show' => 'columns.import_date.visible' } %span {{variant.import_date | date:"MMMM dd, yyyy HH:mm"}} From 4cb291290b543cc51539bb5925fa7bb690c44aea Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 15:08:00 +0100 Subject: [PATCH 07/16] Update product factory --- spec/factories/product_factory.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/spec/factories/product_factory.rb b/spec/factories/product_factory.rb index 7171f18351..f09e46bb96 100644 --- a/spec/factories/product_factory.rb +++ b/spec/factories/product_factory.rb @@ -28,10 +28,13 @@ FactoryBot.define do on_hand { 5 } end - tax_category { |r| Spree::TaxCategory.first || r.association(:tax_category) } + transient do + tax_category { |r| Spree::TaxCategory.first || r.association(:tax_category) } + end after(:create) do |product, evaluator| product.variants.first.on_hand = evaluator.on_hand + product.variants.first.tax_category = evaluator.tax_category product.reload end end @@ -63,15 +66,14 @@ FactoryBot.define do tax_rate_name { "" } included_in_price { "" } zone { nil } + tax_category { create(:tax_category) } end - tax_category { create(:tax_category) } - - after(:create) do |product, proxy| + after(:create) do |_product, proxy| raise "taxed_product factory requires a zone" unless proxy.zone create(:tax_rate, amount: proxy.tax_rate_amount, - tax_category: product.tax_category, + tax_category: proxy.tax_category, included_in_price: proxy.included_in_price, calculator: Calculator::DefaultTax.new, zone: proxy.zone, From b2a7a931f3e32e6c1805f436e1a19911f6fd935c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 15:15:45 +0100 Subject: [PATCH 08/16] Update tax_category tests --- spec/controllers/admin/bulk_line_items_controller_spec.rb | 4 ++-- spec/models/spree/adjustment_spec.rb | 2 +- spec/models/spree/tax_rate_spec.rb | 8 ++++---- spec/system/admin/products_spec.rb | 1 - 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb index 194ae438b0..8283d36420 100644 --- a/spec/controllers/admin/bulk_line_items_controller_spec.rb +++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb @@ -385,8 +385,8 @@ describe Admin::BulkLineItemsController, type: :controller do before do outgoing_exchange.variants << [line_item1.variant, line_item2.variant] allow(order).to receive(:tax_zone) { zone } - line_item1.product.update_columns(tax_category_id: tax_cat5.id) - line_item2.product.update_columns(tax_category_id: tax_cat10.id) + line_item1.variant.update_columns(tax_category_id: tax_cat5.id) + line_item2.variant.update_columns(tax_category_id: tax_cat10.id) order.shipments.map(&:refresh_rates) order.select_shipping_method(shipping_method.id) diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index f1b30805b5..5e9e2197ea 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -433,7 +433,7 @@ module Spree let(:product_tax_category) { create(:tax_category, tax_rates: [product_tax_rate]) } before do - variant.product.update_attribute(:tax_category_id, product_tax_category.id) + variant.update_attribute(:tax_category_id, product_tax_category.id) order.recreate_all_fees! end diff --git a/spec/models/spree/tax_rate_spec.rb b/spec/models/spree/tax_rate_spec.rb index bfd9f07767..6cf0da7e48 100644 --- a/spec/models/spree/tax_rate_spec.rb +++ b/spec/models/spree/tax_rate_spec.rb @@ -300,8 +300,8 @@ module Spree create(:order_with_line_items, line_items_count: 2, distributor: hub, ship_address: address) } - let!(:taxable) { order.line_items.first.variant.product } - let!(:nontaxable) { order.line_items.last.variant.product } + let!(:taxable) { order.line_items.first.variant } + let!(:nontaxable) { order.line_items.last.variant } before do taxable.update(tax_category: category) @@ -310,7 +310,7 @@ module Spree end context "not taxable line item " do - let!(:line_item) { order.contents.add(nontaxable.variants.first, 1) } + let!(:line_item) { order.contents.add(nontaxable, 1) } it "should not create a tax adjustment" do Spree::TaxRate.adjust(order, order.line_items) @@ -324,7 +324,7 @@ module Spree end context "taxable line item" do - let!(:line_item) { order.contents.add(taxable.variants.first, 1) } + let!(:line_item) { order.contents.add(taxable, 1) } before do rate1.update_column(:included_in_price, true) diff --git a/spec/system/admin/products_spec.rb b/spec/system/admin/products_spec.rb index bf5b7ec794..043769815d 100644 --- a/spec/system/admin/products_spec.rb +++ b/spec/system/admin/products_spec.rb @@ -300,7 +300,6 @@ describe ' expect(flash_message).to eq('Product "a product" has been successfully updated!') product.reload expect(product.supplier).to eq(@supplier_permitted) - expect(product.tax_category).to eq(tax_category) end it "editing a product comming from the bulk product update page with filter" do From 0df121bcc80bedc150dac799744c91ea3855cc13 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 15:20:22 +0100 Subject: [PATCH 09/16] Move tax_category validations test --- spec/models/spree/product_spec.rb | 18 ------------------ spec/models/spree/variant_spec.rb | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index e04ef54528..0c3ebee9dc 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -194,24 +194,6 @@ module Spree expect(build(:simple_product, supplier: nil)).not_to be_valid 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(:product, tax_category_id: nil)).not_to be_valid - end - end - 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(:product, tax_category_id: nil)).to be_valid - end - end - end - end - context "when the product has variants" do let(:product) do product = create(:simple_product) diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index d1701b24f7..93a0e603da 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -22,6 +22,24 @@ describe Spree::Variant do variant.unit_value = 0 expect(variant).to be_invalid 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 + 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 + end + end end context "price parsing" do From a84f9ebd2ed98268e386da0d1d72db8362b736f2 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 16:24:01 +0100 Subject: [PATCH 10/16] Allow passing tax_category to new variant when creating a new product --- app/models/spree/product.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index 9a1cfd27ba..472c3d9813 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -76,7 +76,7 @@ module Spree # Transient attributes used temporarily when creating a new product, # these values are persisted on the product's variant - attr_accessor :price, :display_as, :unit_value, :unit_description + attr_accessor :price, :display_as, :unit_value, :unit_description, :tax_category_id before_save :add_primary_taxon_to_taxons @@ -309,6 +309,7 @@ module Spree variant.display_as = display_as variant.unit_value = unit_value variant.unit_description = unit_description + variant.tax_category_id = tax_category_id variants << variant end From b593e356b05380bb4e45c02cacc7584bb40d6067 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 17:17:20 +0100 Subject: [PATCH 11/16] Update product factories --- spec/factories/product_factory.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/factories/product_factory.rb b/spec/factories/product_factory.rb index f09e46bb96..06a7aef77c 100644 --- a/spec/factories/product_factory.rb +++ b/spec/factories/product_factory.rb @@ -26,9 +26,6 @@ FactoryBot.define do factory :product do transient do on_hand { 5 } - end - - transient do tax_category { |r| Spree::TaxCategory.first || r.association(:tax_category) } end @@ -69,7 +66,7 @@ FactoryBot.define do tax_category { create(:tax_category) } end - after(:create) do |_product, proxy| + after(:create) do |product, proxy| raise "taxed_product factory requires a zone" unless proxy.zone create(:tax_rate, amount: proxy.tax_rate_amount, @@ -78,6 +75,8 @@ FactoryBot.define do calculator: Calculator::DefaultTax.new, zone: proxy.zone, name: proxy.tax_rate_name) + + product.variants.first.update(tax_category: proxy.tax_category) end end end From d1209dd49bd198ef0711a25504f2e2c4f63f619c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 17:23:48 +0100 Subject: [PATCH 12/16] Update order_with_taxes factory --- spec/factories/order_factory.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/spec/factories/order_factory.rb b/spec/factories/order_factory.rb index 979b820044..0bf4e1b73b 100644 --- a/spec/factories/order_factory.rb +++ b/spec/factories/order_factory.rb @@ -192,12 +192,13 @@ FactoryBot.define do after(:create) do |order, proxy| order.distributor.update_attribute(:charges_sales_tax, true) - product = FactoryBot.create(:taxed_product, zone: proxy.zone, - price: proxy.product_price, - tax_rate_amount: proxy.tax_rate_amount, - tax_rate_name: proxy.tax_rate_name, - included_in_price: proxy.included_in_price) - FactoryBot.create(:line_item, order: order, product: product, price: product.price) + product = create(:taxed_product, zone: proxy.zone, + price: proxy.product_price, + tax_rate_amount: proxy.tax_rate_amount, + tax_rate_name: proxy.tax_rate_name, + included_in_price: proxy.included_in_price) + + create(:line_item, order: order, variant: product.variants.first, price: product.price) order.reload end end From 07774c4572294076bca97166258b833fca9de498 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 3 Jul 2023 16:38:16 +0100 Subject: [PATCH 13/16] Update tax category specs --- .../unit/admin/bulk_product_update_spec.js.coffee | 9 ++++----- .../enterprise_fee_applicator_spec.rb | 9 ++++----- .../orders_cycle_supplier_totals_report_spec.rb | 14 +++++++++++--- spec/mailers/producer_mailer_spec.rb | 2 +- spec/models/spree/adjustment_spec.rb | 4 ++-- .../services/order_tax_adjustments_fetcher_spec.rb | 4 +--- spec/system/admin/product_import_spec.rb | 2 +- .../sales_tax/sales_tax_totals_by_order_spec.rb | 6 ++---- .../sales_tax/sales_tax_totals_by_producer_spec.rb | 9 +++------ spec/system/admin/reports_spec.rb | 6 +++--- .../consumer/split_checkout_tax_not_incl_spec.rb | 2 +- 11 files changed, 33 insertions(+), 34 deletions(-) diff --git a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee index cae859743c..28ff35e1e8 100644 --- a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee @@ -187,7 +187,6 @@ describe "filtering products for submission to database", -> description: "" deleted_at: null meta_keywords: null - tax_category_id: null shipping_category_id: null created_at: null updated_at: null @@ -205,6 +204,7 @@ describe "filtering products for submission to database", -> on_hand: 2 price: 10.0 unit_value: 250 + tax_category_id: null unit_description: "(bottle)" display_as: "bottle" display_name: "nothing" @@ -221,7 +221,6 @@ describe "filtering products for submission to database", -> variant_unit: 'volume' variant_unit_scale: 1 variant_unit_name: 'loaf' - tax_category_id: null master_attributes: id: 2 unit_value: 250 @@ -231,6 +230,7 @@ describe "filtering products for submission to database", -> on_hand: 2 price: 10.0 unit_value: 250 + tax_category_id: null unit_description: "(bottle)" display_as: "bottle" display_name: "nothing" @@ -822,8 +822,8 @@ describe "AdminProductEditCtrl", -> expect(product).toEqual id: 123 variants: [ - {id: -1, price: null, unit_value: null, unit_description: null, on_demand: false, on_hand: null, display_as: null, display_name: null} - {id: -2, price: null, unit_value: null, unit_description: null, on_demand: false, on_hand: null, display_as: null, display_name: null} + {id: -1, price: null, unit_value: null, tax_category_id: null, unit_description: null, on_demand: false, on_hand: null, display_as: null, display_name: null} + {id: -2, price: null, unit_value: null, tax_category_id: null, unit_description: null, on_demand: false, on_hand: null, display_as: null, display_name: null} ] it "shows the variant(s)", -> @@ -978,7 +978,6 @@ describe "AdminProductEditCtrl", -> description: "" deleted_at: null meta_keywords: null - tax_category_id: null shipping_category_id: null created_at: null updated_at: null diff --git a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb index 23d7952067..2beec77b17 100644 --- a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb +++ b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb @@ -5,15 +5,15 @@ require 'open_food_network/enterprise_fee_applicator' module OpenFoodNetwork describe EnterpriseFeeApplicator do - let(:line_item) { create(:line_item) } + let(:line_item) { create(:line_item, variant: target_variant) } let(:inherits_tax) { true } let(:enterprise_fee) { create(:enterprise_fee, inherits_tax_category: inherits_tax, tax_category: fee_tax_category) } let(:fee_tax_category) { nil } let(:tax_category) { create(:tax_category) } - let(:product) { create(:simple_product, tax_category: tax_category) } - let(:target_variant) { product.variants.first } + let!(:target_variant) { create(:variant, tax_category: tax_category) } + let(:product) { variant.product } let(:applicator) { EnterpriseFeeApplicator.new(enterprise_fee, target_variant, 'role') } describe "#create_line_item_adjustment" do @@ -37,10 +37,9 @@ module OpenFoodNetwork end describe "#create_order_adjustment" do - let(:target_variant) { nil } let(:inherits_tax) { false } let(:fee_tax_category) { tax_category } - let(:order) { line_item.order } + let(:order) { create(:order) } it "creates an adjustment for an order" do allow(applicator).to receive(:order_adjustment_label) { 'label' } diff --git a/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb b/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb index 6dcba9b807..43f767faf8 100644 --- a/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb +++ b/spec/lib/reports/orders_and_fulfillment/orders_cycle_supplier_totals_report_spec.rb @@ -105,8 +105,11 @@ describe Reporting::Reports::OrdersAndFulfillment::OrderCycleSupplierTotals do end context "with a VAT/GST-free supplier" do - before(:each) do + let(:tax_category) { create(:tax_category) } + + before do supplier.update(charges_sales_tax: false) + order.line_items.first.variant.update(tax_category_id: tax_category.id) end it "has a variant row when product belongs to a tax category" do @@ -122,7 +125,8 @@ describe Reporting::Reports::OrdersAndFulfillment::OrderCycleSupplierTotals do end it "has a variant row when product doesn't belong to a tax category" do - order.line_items.first.variant.product.update(tax_category_id: nil) + order.line_items.first.variant.update(tax_category_id: nil) + supplier_name_field = report_table.first[0] supplier_vat_status_field = report_table.first[-2] product_tax_category_field = report_table.first[-1] @@ -134,8 +138,11 @@ describe Reporting::Reports::OrdersAndFulfillment::OrderCycleSupplierTotals do end context "with a VAT/GST-enabled supplier" do + let(:tax_category) { create(:tax_category) } + before(:each) do supplier.update(charges_sales_tax: true) + order.line_items.first.variant.update(tax_category_id: tax_category.id) end it "has a variant row when product belongs to a tax category" do @@ -151,7 +158,8 @@ describe Reporting::Reports::OrdersAndFulfillment::OrderCycleSupplierTotals do end it "has a variant row when product doesn't belong to a tax category" do - order.line_items.first.variant.product.update(tax_category_id: nil) + order.line_items.first.variant.update(tax_category_id: nil) + supplier_name_field = report_table.first[0] supplier_vat_status_field = report_table.first[-2] product_tax_category_field = report_table.first[-1] diff --git a/spec/mailers/producer_mailer_spec.rb b/spec/mailers/producer_mailer_spec.rb index 153b72aac4..8642ecdba4 100644 --- a/spec/mailers/producer_mailer_spec.rb +++ b/spec/mailers/producer_mailer_spec.rb @@ -16,7 +16,7 @@ describe ProducerMailer, type: :mailer do let(:d1) { create(:distributor_enterprise, charges_sales_tax: true) } let(:d2) { create(:distributor_enterprise) } let(:p1) { - create(:product, name: "Zebra", price: 12.34, supplier: s1, tax_category: tax_category) + create(:product, name: "Zebra", price: 12.34, supplier: s1, tax_category_id: tax_category.id) } let(:p2) { create(:product, name: "Aardvark", price: 23.45, supplier: s2) } let(:p3) { create(:product, name: "Banana", price: 34.56, supplier: s1) } diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 5e9e2197ea..b22c31a5c2 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -527,8 +527,8 @@ module Spree create(:tax_rate, included_in_price: included_in_price, zone: zone, calculator: ::Calculator::FlatRate.new(preferred_amount: 0.1)) } - let(:product) { create(:product, tax_category: tax_category) } - let(:variant) { product.variants.first } + let(:variant) { create(:variant, tax_category: tax_category) } + let(:product) { variant.product } describe "tax adjustment creation" do before do diff --git a/spec/services/order_tax_adjustments_fetcher_spec.rb b/spec/services/order_tax_adjustments_fetcher_spec.rb index 7b91f4ee1d..360fda1742 100644 --- a/spec/services/order_tax_adjustments_fetcher_spec.rb +++ b/spec/services/order_tax_adjustments_fetcher_spec.rb @@ -43,9 +43,7 @@ describe OrderTaxAdjustmentsFetcher do let(:tax_category25) { create(:tax_category, tax_rates: [tax_rate25]) } let(:tax_category30) { create(:tax_category, tax_rates: [tax_rate30]) } - let(:variant) do - create(:variant, product: create(:product, tax_category: tax_category10)) - end + let(:variant) { create(:variant, tax_category: tax_category10) } let(:enterprise_fee) do create(:enterprise_fee, enterprise: coordinator, tax_category: tax_category20, diff --git a/spec/system/admin/product_import_spec.rb b/spec/system/admin/product_import_spec.rb index f9c38aab50..fd2d1559c5 100644 --- a/spec/system/admin/product_import_spec.rb +++ b/spec/system/admin/product_import_spec.rb @@ -190,7 +190,7 @@ describe "Product Import" do expect(page).to have_no_selector '.updated-count' carrots = Spree::Product.find_by(name: 'Carrots') - expect(carrots.tax_category).to eq tax_category + expect(carrots.variants.first.tax_category).to eq tax_category expect(carrots.shipping_category).to eq shipping_category end diff --git a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb index 718ed1e3d3..f9ce7e273e 100644 --- a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb +++ b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb @@ -82,10 +82,8 @@ describe "Sales Tax Totals By order" do distributor.shipping_methods << shipping_method distributor.payment_methods << payment_method - product.update!( - tax_category_id: tax_category.id, - supplier_id: supplier.id - ) + product.update!(supplier_id: supplier.id) + variant.update!(tax_category_id: tax_category.id) order.update!( number: 'ORDER_NUMBER_1', diff --git a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_producer_spec.rb b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_producer_spec.rb index b8cc954c15..9a1a9053d1 100644 --- a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_producer_spec.rb +++ b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_producer_spec.rb @@ -56,12 +56,9 @@ describe "Sales Tax Totals By Producer" do distributor.shipping_methods << shipping_method distributor.payment_methods << payment_method - supplier.update!({ name: 'Supplier', charges_sales_tax: true }) - - product.update!({ - tax_category_id: tax_category.id, - supplier_id: supplier.id - }) + supplier.update!(name: 'Supplier', charges_sales_tax: true) + product.update!(supplier_id: supplier.id) + variant.update!(tax_category_id: tax_category.id) end context 'added tax' do diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 6264aaee39..7681e1e96c 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -294,7 +294,7 @@ describe ' } let(:enterprise_fee) { create(:enterprise_fee, enterprise: user1.enterprises.first, - tax_category: product2.tax_category, + tax_category: product2.variants.first.tax_category, calculator: Calculator::FlatRate.new(preferred_amount: 120.0)) } let(:order_cycle) { @@ -588,12 +588,12 @@ describe ' let(:enterprise_fee1) { create(:enterprise_fee, enterprise: user1.enterprises.first, - tax_category: product2.tax_category, + tax_category: product2.variants.first.tax_category, calculator: Calculator::FlatRate.new(preferred_amount: 10)) } let(:enterprise_fee2) { create(:enterprise_fee, enterprise: user1.enterprises.first, - tax_category: product2.tax_category, + tax_category: product2.variants.first.tax_category, calculator: Calculator::FlatRate.new(preferred_amount: 20)) } let(:order_cycle) { diff --git a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb index 046a6ac483..a8c34f34c9 100644 --- a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb +++ b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb @@ -32,7 +32,7 @@ describe "As a consumer, I want to see adjustment breakdown" do let(:distributor) { create(:distributor_enterprise, charges_sales_tax: true) } let(:supplier) { create(:supplier_enterprise) } let(:product_with_tax) { - create(:simple_product, supplier: supplier, price: 10, tax_category: tax_category) + create(:product, supplier: supplier, price: 10, tax_category_id: tax_category.id) } let(:variant_with_tax) { product_with_tax.variants.first } let(:order_cycle) { From e2094665c5f94f70b4139222fc42b366dd709a5d Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 4 Jul 2023 10:00:20 +0100 Subject: [PATCH 14/16] Update enterprise fee report tax_category fetching --- lib/reporting/reports/enterprise_fee_summary/scope.rb | 2 +- .../enterprise_fee_summary_report_spec.rb | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/reporting/reports/enterprise_fee_summary/scope.rb b/lib/reporting/reports/enterprise_fee_summary/scope.rb index 651ecbb035..24ea2a3b6f 100644 --- a/lib/reporting/reports/enterprise_fee_summary/scope.rb +++ b/lib/reporting/reports/enterprise_fee_summary/scope.rb @@ -236,7 +236,7 @@ module Reporting LEFT OUTER JOIN spree_tax_categories AS product_tax_categories ON ( enterprise_fees.inherits_tax_category IS TRUE - AND product_tax_categories.id = spree_products.tax_category_id + AND product_tax_categories.id = spree_variants.tax_category_id ) JOIN_STRING ) diff --git a/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb b/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb index 939b1ae1a0..4a30f5b7dc 100644 --- a/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb +++ b/spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb @@ -31,9 +31,9 @@ describe Reporting::Reports::EnterpriseFeeSummary::FeeSummary do # Set up other requirements for ordering. let!(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) } - let!(:product) { create(:product, tax_category: product_tax_category) } + let!(:product) { create(:product) } let!(:product_tax_category) { create(:tax_category, name: "Sample Product Tax") } - let!(:variant) { prepare_variant } + let!(:variant) { prepare_variant(tax_category: product_tax_category) } # Create customers. let!(:customer) { create(:customer, first_name: "Sample", last_name: "Customer") } @@ -54,7 +54,8 @@ describe Reporting::Reports::EnterpriseFeeSummary::FeeSummary do end let!(:variant) do - prepare_variant(incoming_exchange_fees: variant_incoming_exchange_fees, + prepare_variant(tax_category: product_tax_category, + incoming_exchange_fees: variant_incoming_exchange_fees, outgoing_exchange_fees: variant_outgoing_exchange_fees) end From 64b29d40a801d33b5d3faf7f5c723c43ed2b7238 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 14 Jul 2023 13:49:34 +0100 Subject: [PATCH 15/16] Migrate data for tax_category_id from products to variants --- db/migrate/20230714123324_migrate_tax_category.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 db/migrate/20230714123324_migrate_tax_category.rb diff --git a/db/migrate/20230714123324_migrate_tax_category.rb b/db/migrate/20230714123324_migrate_tax_category.rb new file mode 100644 index 0000000000..312c5659ba --- /dev/null +++ b/db/migrate/20230714123324_migrate_tax_category.rb @@ -0,0 +1,15 @@ +class MigrateTaxCategory < ActiveRecord::Migration[7.0] + def up + migrate_tax_category + end + + def migrate_tax_category + ActiveRecord::Base.connection.execute(<<-SQL + UPDATE spree_variants + SET tax_category_id = spree_products.tax_category_id + FROM spree_products + WHERE spree_variants.product_id = spree_products.id + SQL + ) + end +end From 885bc46d889c365fdb3d38ec50ee675c3cb9490b Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 1 Aug 2023 11:45:41 +0100 Subject: [PATCH 16/16] Update tax_category in product v3 table --- app/views/admin/products_v3/_table.html.haml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index a1b8b9fd6c..bab5f02c74 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -41,7 +41,6 @@ %td.align-left .line-clamp-1= product.taxons.map(&:name).join(', ') %td.align-left - .line-clamp-1= product.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string. %td.align-left .line-clamp-1= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below) - product.variants.each do |variant| @@ -61,7 +60,7 @@ %td.align-left -# empty %td.align-left - -# empty + .line-clamp-1= variant.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string. %td.align-left -# empty