diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 4a7f954a7c..1e55d2a099 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -27,7 +27,7 @@ module Spree end def new - @object.shipping_category = DefaultShippingCategory.find_or_create + @object.shipping_category_id = DefaultShippingCategory.find_or_create.id end def edit diff --git a/app/controllers/spree/admin/variants_controller.rb b/app/controllers/spree/admin/variants_controller.rb index 85a609ae42..6033db7afe 100644 --- a/app/controllers/spree/admin/variants_controller.rb +++ b/app/controllers/spree/admin/variants_controller.rb @@ -15,6 +15,7 @@ module Spree def new @url_filters = ::ProductFilters.new.extract(request.query_parameters) + @object.shipping_category ||= DefaultShippingCategory.find_or_create end def edit @@ -114,6 +115,7 @@ module Spree def load_data @tax_categories = TaxCategory.order(:name) + @shipping_categories = ShippingCategory.order(:name) end end end diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 4226caa199..218863c065 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -29,7 +29,6 @@ module ProductImport unit_type: :variant_unit_scale, variant_unit_name: :variant_unit_name, tax_category: :tax_category_id, - shipping_category: :shipping_category_id } end diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index 68437005e8..2f946bf1f1 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -33,7 +33,6 @@ module Spree searchable_associations :supplier, :properties, :primary_taxon, :variants searchable_scopes :active, :with_properties - belongs_to :shipping_category, class_name: 'Spree::ShippingCategory' belongs_to :supplier, class_name: 'Enterprise', optional: false, touch: true belongs_to :primary_taxon, class_name: 'Spree::Taxon', optional: false, touch: true @@ -52,7 +51,6 @@ module Spree through: :variants validates :name, presence: true - validates :shipping_category, presence: true validates :variant_unit, presence: true validates :unit_value, presence: @@ -71,7 +69,8 @@ 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, :tax_category_id + attr_accessor :price, :display_as, :unit_value, :unit_description, :tax_category_id, + :shipping_category_id after_create :ensure_standard_variant after_save :update_units @@ -292,6 +291,7 @@ module Spree variant.unit_value = unit_value variant.unit_description = unit_description variant.tax_category_id = tax_category_id + variant.shipping_category_id = shipping_category_id variants << variant end diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 74a0db5e60..a130769de2 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -29,9 +29,9 @@ module Spree belongs_to :product, -> { with_deleted }, touch: true, class_name: 'Spree::Product' belongs_to :tax_category, class_name: 'Spree::TaxCategory' + belongs_to :shipping_category, class_name: 'Spree::ShippingCategory', optional: false - delegate_belongs_to :product, :name, :description, :shipping_category_id, - :meta_keywords, :shipping_category + delegate_belongs_to :product, :name, :description, :meta_keywords has_many :inventory_units, inverse_of: :variant has_many :line_items, inverse_of: :variant @@ -77,6 +77,7 @@ module Spree } before_validation :set_cost_currency + before_validation :ensure_shipping_category before_validation :ensure_unit_value before_validation :update_weight_from_unit_value, if: ->(v) { v.product.present? } @@ -248,6 +249,10 @@ module Spree self.unit_value = 1.0 end + def ensure_shipping_category + self.shipping_category ||= DefaultShippingCategory.find_or_create + end + def convert_variant_weight_to_decimal self.weight = weight.to_d end diff --git a/app/services/order_available_shipping_methods.rb b/app/services/order_available_shipping_methods.rb index 95603f9a68..9b4bcf76ba 100644 --- a/app/services/order_available_shipping_methods.rb +++ b/app/services/order_available_shipping_methods.rb @@ -3,9 +3,7 @@ class OrderAvailableShippingMethods attr_reader :order, :customer - delegate :distributor, - :order_cycle, - to: :order + delegate :distributor, :order_cycle, to: :order def initialize(order, customer = nil) @order, @customer = order, customer @@ -23,7 +21,7 @@ class OrderAvailableShippingMethods return methods unless OpenFoodNetwork::FeatureToggle.enabled?(:match_shipping_categories, distributor&.owner) - required_category_ids = order.products.pluck(:shipping_category_id).to_set + required_category_ids = order.variants.pluck(:shipping_category_id).to_set return methods if required_category_ids.empty? methods.select do |method| diff --git a/app/services/permitted_attributes/product.rb b/app/services/permitted_attributes/product.rb index ce61fe0d0c..96b9e18cf8 100644 --- a/app/services/permitted_attributes/product.rb +++ b/app/services/permitted_attributes/product.rb @@ -7,7 +7,7 @@ module PermittedAttributes :id, :name, :description, :supplier_id, :price, :variant_unit, :variant_unit_scale, :unit_value, :unit_description, :variant_unit_name, :display_as, :sku, :group_buy, :group_buy_unit_size, - :taxon_ids, :primary_taxon_id, :tax_category_id, :shipping_category_id, + :taxon_ids, :primary_taxon_id, :tax_category_id, :meta_keywords, :notes, :inherits_properties, { product_properties_attributes: [:id, :property_name, :value], variants_attributes: [PermittedAttributes::Variant.attributes], diff --git a/app/services/permitted_attributes/variant.rb b/app/services/permitted_attributes/variant.rb index 8a834d4d84..47e4481e5b 100644 --- a/app/services/permitted_attributes/variant.rb +++ b/app/services/permitted_attributes/variant.rb @@ -4,7 +4,7 @@ module PermittedAttributes class Variant def self.attributes [ - :id, :sku, :on_hand, :on_demand, + :id, :sku, :on_hand, :on_demand, :shipping_category_id, :price, :unit_value, :unit_description, :display_name, :display_as, :tax_category_id, :weight, :height, :width, :depth diff --git a/app/views/spree/admin/products/_form.html.haml b/app/views/spree/admin/products/_form.html.haml index 6fb51e7c10..2935fa0dc5 100644 --- a/app/views/spree/admin/products/_form.html.haml +++ b/app/views/spree/admin/products/_form.html.haml @@ -38,11 +38,6 @@ .clear - = f.field_container :shipping_categories do - = f.label :shipping_category_id, t(:shipping_categories) - = f.collection_select(:shipping_category_id, @shipping_categories, :id, :name, {}, { :class => 'select2' }) - = f.error_message_on :shipping_category - .clear %div diff --git a/app/views/spree/admin/variants/_form.html.haml b/app/views/spree/admin/variants/_form.html.haml index 3f8f8d3021..1e9aa79236 100644 --- a/app/views/spree/admin/variants/_form.html.haml +++ b/app/views/spree/admin/variants/_form.html.haml @@ -68,4 +68,8 @@ = 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' }) + .field + = f.label :shipping_category_id, t(:shipping_categories) + = f.collection_select(:shipping_category_id, @shipping_categories, :id, :name, {}, { class: 'select2 fullwidth' }) + .clear diff --git a/db/migrate/20230715143122_add_shipping_category_to_variants.rb b/db/migrate/20230715143122_add_shipping_category_to_variants.rb new file mode 100644 index 0000000000..3d50730ca7 --- /dev/null +++ b/db/migrate/20230715143122_add_shipping_category_to_variants.rb @@ -0,0 +1,5 @@ +class AddShippingCategoryToVariants < ActiveRecord::Migration[7.0] + def change + add_reference :spree_variants, :shipping_category, foreign_key: { to_table: :spree_shipping_categories } + end +end diff --git a/db/migrate/20230715145329_migrate_shipping_category.rb b/db/migrate/20230715145329_migrate_shipping_category.rb new file mode 100644 index 0000000000..9b8d970cae --- /dev/null +++ b/db/migrate/20230715145329_migrate_shipping_category.rb @@ -0,0 +1,15 @@ +class MigrateShippingCategory < ActiveRecord::Migration[7.0] + def up + migrate_shipping_category + end + + def migrate_shipping_category + ActiveRecord::Base.connection.execute(<<-SQL + UPDATE spree_variants + SET shipping_category_id = spree_products.shipping_category_id + FROM spree_products + WHERE spree_variants.product_id = spree_products.id + SQL + ) + end +end diff --git a/db/schema.rb b/db/schema.rb index 307bafadc9..0abe692745 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -948,7 +948,9 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_09_201542) do t.datetime "created_at", default: -> { "now()" }, null: false t.datetime "updated_at", default: -> { "now()" }, null: false t.bigint "tax_category_id" + t.bigint "shipping_category_id" t.index ["product_id"], name: "index_variants_on_product_id" + t.index ["shipping_category_id"], name: "index_spree_variants_on_shipping_category_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" @@ -1176,6 +1178,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_09_201542) 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_shipping_categories", column: "shipping_category_id" 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" diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index 40dfe0c186..02c8f3e00a 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -22,7 +22,6 @@ class SuppliedProductBuilder < DfcBuilder description: supplied_product.description, price: 0, # will be in DFC Offer primary_taxon: Spree::Taxon.first, # dummy value until we have a mapping - shipping_category: DefaultShippingCategory.find_or_create, ).tap do |product| QuantitativeValueBuilder.apply(supplied_product.quantity, product) end diff --git a/lib/reporting/queries/joins.rb b/lib/reporting/queries/joins.rb index 39d73fd05e..e0097aa3a0 100644 --- a/lib/reporting/queries/joins.rb +++ b/lib/reporting/queries/joins.rb @@ -23,8 +23,8 @@ module Reporting reflect query.join(association(Spree::Product, :supplier, supplier_alias)) end - def joins_product_shipping_category - reflect query.join(association(Spree::Product, :shipping_category)) + def joins_variant_shipping_category + reflect query.join(association(Spree::Variant, :shipping_category)) end def joins_order_and_distributor diff --git a/lib/reporting/reports/order_cycle_management/delivery.rb b/lib/reporting/reports/order_cycle_management/delivery.rb index 3208ce7624..04a5030579 100644 --- a/lib/reporting/reports/order_cycle_management/delivery.rb +++ b/lib/reporting/reports/order_cycle_management/delivery.rb @@ -26,7 +26,7 @@ module Reporting def has_temperature_controlled_items?(order) order.line_items.any? { |line_item| - line_item.product.shipping_category&.temperature_controlled + line_item.variant.shipping_category&.temperature_controlled } end end diff --git a/lib/reporting/reports/packing/base.rb b/lib/reporting/reports/packing/base.rb index 19e60155b2..8fb00736d8 100644 --- a/lib/reporting/reports/packing/base.rb +++ b/lib/reporting/reports/packing/base.rb @@ -19,7 +19,7 @@ module Reporting joins_variant. joins_variant_product. joins_product_supplier. - joins_product_shipping_category. + joins_variant_shipping_category. selecting(select_fields). ordered_by(ordering_fields) end diff --git a/lib/tasks/sample_data/product_factory.rb b/lib/tasks/sample_data/product_factory.rb index 0591617f8b..59a5dba611 100644 --- a/lib/tasks/sample_data/product_factory.rb +++ b/lib/tasks/sample_data/product_factory.rb @@ -68,7 +68,6 @@ module SampleData variant_unit: "weight", variant_unit_scale: 1, unit_value: 1, - shipping_category: DefaultShippingCategory.find_or_create, tax_category_id: find_or_create_tax_category.id ) product = Spree::Product.create_with(params).find_or_create_by!(name: params[:name]) diff --git a/spec/controllers/api/v0/products_controller_spec.rb b/spec/controllers/api/v0/products_controller_spec.rb index c48af9ba66..730c8ed8bd 100644 --- a/spec/controllers/api/v0/products_controller_spec.rb +++ b/spec/controllers/api/v0/products_controller_spec.rb @@ -102,8 +102,7 @@ describe Api::V0::ProductsController, type: :controller do expect(response.status).to eq(422) expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.") errors = json_response["errors"] - expect(errors.keys).to match_array(["name", "primary_taxon", "shipping_category", - "supplier", "variant_unit"]) + expect(errors.keys).to match_array(["name", "primary_taxon", "supplier", "variant_unit"]) end it "can update a product" do diff --git a/spec/controllers/api/v0/reports/packing_report_spec.rb b/spec/controllers/api/v0/reports/packing_report_spec.rb index 9cec0b7c7f..2dcdc9f284 100644 --- a/spec/controllers/api/v0/reports/packing_report_spec.rb +++ b/spec/controllers/api/v0/reports/packing_report_spec.rb @@ -71,7 +71,7 @@ describe Api::V0::ReportsController, type: :controller do "variant" => line_item.full_name, "quantity" => line_item.quantity, "price" => (line_item.quantity * line_item.price).to_s, - "temp_controlled" => line_item.product.shipping_category&.temperature_controlled + "temp_controlled" => line_item.variant.shipping_category&.temperature_controlled }. merge(dimensions(line_item)). merge(contacts(line_item.order.bill_address)) @@ -89,7 +89,7 @@ describe Api::V0::ReportsController, type: :controller do "variant" => line_item.full_name, "quantity" => line_item.quantity, "price" => (line_item.quantity * line_item.price).to_s, - "temp_controlled" => line_item.product.shipping_category&.temperature_controlled + "temp_controlled" => line_item.variant.shipping_category&.temperature_controlled }.merge(dimensions(line_item)) end diff --git a/spec/factories/product_factory.rb b/spec/factories/product_factory.rb index 06a7aef77c..320060563f 100644 --- a/spec/factories/product_factory.rb +++ b/spec/factories/product_factory.rb @@ -18,8 +18,6 @@ FactoryBot.define do variant_unit_scale { 1 } variant_unit_name { '' } - shipping_category { DefaultShippingCategory.find_or_create } - # ensure stock item will be created for this products master before(:create) { create(:stock_location) if Spree::StockLocation.count.zero? } diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index edda720680..f42d7c85f0 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -285,7 +285,7 @@ describe ProductImport::ProductImporter do expect(carrots.on_hand).to eq 5 expect(carrots.variants.first.price).to eq 3.20 expect(carrots.primary_taxon.name).to eq "Vegetables" - expect(carrots.shipping_category).to eq shipping_category + expect(carrots.variants.first.shipping_category).to eq shipping_category expect(carrots.supplier).to eq enterprise expect(carrots.variants.first.unit_presentation).to eq "500g" end diff --git a/spec/models/spree/order/checkout_spec.rb b/spec/models/spree/order/checkout_spec.rb index 8ed666961d..84b28216aa 100644 --- a/spec/models/spree/order/checkout_spec.rb +++ b/spec/models/spree/order/checkout_spec.rb @@ -123,8 +123,7 @@ describe Spree::Order::Checkout do let(:order) { create(:order_with_totals_and_distribution, ship_address: create(:address) ) } let(:shipping_method) { create(:shipping_method, distributors: [order.distributor]) } let(:other_shipping_category) { create(:shipping_category) } - let(:other_product) { create(:product, shipping_category: other_shipping_category ) } - let(:other_variant) { other_product.variants.first } + let(:other_variant) { create(:variant, shipping_category: other_shipping_category) } before do order.order_cycle = create(:simple_order_cycle, diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index d99fe291f9..d0f7a89eeb 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -207,6 +207,7 @@ module Spree context "saving a new product" do let!(:product){ Spree::Product.new } + let!(:shipping_category){ create(:shipping_category) } before do create(:stock_location) @@ -217,7 +218,7 @@ module Spree product.variant_unit_scale = 1000 product.unit_value = 1 product.price = 4.27 - product.shipping_category = create(:shipping_category) + product.shipping_category_id = shipping_category.id product.save! end @@ -225,6 +226,7 @@ module Spree expect(product.variants.reload.length).to eq 1 standard_variant = product.variants.reload.first expect(standard_variant.price).to eq 4.27 + expect(standard_variant.shipping_category).to eq shipping_category end end diff --git a/spec/services/order_available_shipping_methods_spec.rb b/spec/services/order_available_shipping_methods_spec.rb index 315f2397c4..16104614f9 100644 --- a/spec/services/order_available_shipping_methods_spec.rb +++ b/spec/services/order_available_shipping_methods_spec.rb @@ -217,9 +217,7 @@ describe OrderAvailableShippingMethods do context "when certain shipping categories are required" do subject { OrderAvailableShippingMethods.new(order) } - let(:order) { - build(:order, distributor: distributor, order_cycle: oc) - } + let(:order) { build(:order, distributor: distributor, order_cycle: oc) } let(:oc) { create(:order_cycle) } let(:distributor) { oc.distributors.first } let(:standard_shipping) { @@ -249,7 +247,7 @@ describe OrderAvailableShippingMethods do it "filters shipping methods for products needing refrigeration" do product = oc.products.first - product.update!(shipping_category: refrigerated) + product.variants.first.update!(shipping_category: refrigerated) order.line_items << build(:line_item, variant: product.variants.first) expect(subject.to_a).to eq [cooled_shipping] end diff --git a/spec/system/admin/product_import_spec.rb b/spec/system/admin/product_import_spec.rb index ec0a2d43de..8671daa8c7 100644 --- a/spec/system/admin/product_import_spec.rb +++ b/spec/system/admin/product_import_spec.rb @@ -182,7 +182,7 @@ describe "Product Import" do carrots = Spree::Product.find_by(name: 'Carrots') expect(carrots.variants.first.tax_category).to eq tax_category - expect(carrots.shipping_category).to eq shipping_category + expect(carrots.variants.first.shipping_category).to eq shipping_category end it "records a timestamp on import that can be viewed and filtered under Bulk Edit Products" do diff --git a/spec/system/admin/products_spec.rb b/spec/system/admin/products_spec.rb index 043769815d..386d0a5239 100644 --- a/spec/system/admin/products_spec.rb +++ b/spec/system/admin/products_spec.rb @@ -110,7 +110,7 @@ describe ' expect(product.variants.first.price.to_s).to eq('19.99') expect(product.on_hand).to eq(5) expect(product.variants.first.tax_category_id).to eq(tax_category.id) - expect(product.shipping_category).to eq(shipping_category) + expect(product.variants.first.shipping_category).to eq(shipping_category) expect(product.description).to eq("

A description...

") expect(product.group_buy).to be_falsey expect(product.variants.first.unit_presentation).to eq("5kg")