diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index d4872b3e84..9232b78120 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -22,7 +22,7 @@ module Spree include LogDestroyPerformer self.belongs_to_required_by_default = false - self.ignored_columns += [:supplier_id] + self.ignored_columns += [:supplier_id, :variant_unit_scale, :variant_unit_name] acts_as_paranoid @@ -45,16 +45,6 @@ module Spree validates_lengths_from_database validates :name, presence: true - - validates :variant_unit, presence: true - validates :unit_value, numericality: { - greater_than: 0, - if: ->(p) { p.variant_unit.in?(%w(weight volume)) && new_record? } - } - validates :variant_unit_scale, - presence: { if: ->(p) { %w(weight volume).include? p.variant_unit } } - validates :variant_unit_name, - presence: { if: ->(p) { p.variant_unit == 'items' } } validate :validate_image validates :price, numericality: { greater_than_or_equal_to: 0, if: ->{ new_record? } } @@ -66,14 +56,14 @@ 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, - :shipping_category_id, :primary_taxon_id, :supplier_id + attr_accessor :price, :display_as, :unit_value, :unit_description, :variant_unit, + :variant_unit_name, :variant_unit_scale, :tax_category_id, :shipping_category_id, + :primary_taxon_id, :supplier_id after_validation :validate_variant_attrs, on: :create after_create :ensure_standard_variant after_update :touch_supplier, if: :saved_change_to_primary_taxon_id? around_destroy :destruction - after_save :update_units after_touch :touch_supplier # -- Scopes @@ -254,6 +244,9 @@ module Spree variant.display_as = display_as variant.unit_value = unit_value variant.unit_description = unit_description + variant.variant_unit = variant_unit + variant.variant_unit_name = variant_unit_name + variant.variant_unit_scale = variant_unit_scale variant.tax_category_id = tax_category_id variant.shipping_category_id = shipping_category_id variant.primary_taxon_id = primary_taxon_id @@ -261,25 +254,6 @@ module Spree variants << variant end - # Format as per WeightsAndMeasures (todo: re-orgnaise maybe after product/variant refactor) - def variant_unit_with_scale - # Our code is based upon English based number formatting with a period `.` - scale_clean = ActiveSupport::NumberHelper.number_to_rounded(variant_unit_scale, - precision: nil, - significant: false, - strip_insignificant_zeros: true, - locale: :en) - [variant_unit, scale_clean].compact_blank.join("_") - end - - def variant_unit_with_scale=(variant_unit_with_scale) - values = variant_unit_with_scale.split("_") - assign_attributes( - variant_unit: values[0], - variant_unit_scale: values[1] || nil - ) - end - # Remove any unsupported HTML. def description HtmlSanitizer.sanitize(super) @@ -301,18 +275,6 @@ module Spree errors.add(:supplier_id, :blank) unless Enterprise.find_by(id: supplier_id) end - def update_units - return unless saved_change_to_variant_unit? || saved_change_to_variant_unit_name? - - variants.each do |v| - if v.persisted? - v.update_units - else - v.assign_units - end - end - end - def touch_supplier return if variants.empty? @@ -324,6 +286,7 @@ module Spree # importing product. In this scenario the variant has not been updated with the supplier yet # hence the check. first_variant.supplier.touch if first_variant.supplier.present? + end def validate_image diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 6ad1cf6d37..16a555498e 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -229,9 +229,12 @@ module Spree # Format as per WeightsAndMeasures # TODO test ? def variant_unit_with_scale + # Our code is based upon English based number formatting with a period `.` scale_clean = ActiveSupport::NumberHelper.number_to_rounded(variant_unit_scale, precision: nil, - strip_insignificant_zeros: true) + strip_insignificant_zeros: true, + locale: :en + ) [variant_unit, scale_clean].compact_blank.join("_") end diff --git a/spec/factories/product_factory.rb b/spec/factories/product_factory.rb index f2fab0e39f..453a442be4 100644 --- a/spec/factories/product_factory.rb +++ b/spec/factories/product_factory.rb @@ -20,10 +20,8 @@ FactoryBot.define do unit_value { 1 } unit_description { '' } - variant_unit { 'weight' } variant_unit_scale { 1 } - variant_unit_name { '' } # ensure stock item will be created for this products master before(:create) { DefaultStockLocation.find_or_create } diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index 385588538d..130ac639a6 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -123,27 +123,7 @@ module Spree it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_length_of(:sku).is_at_most(255) } - context "unit value" do - it "requires a unit value when variant unit is weight" do - expect(build(:simple_product, variant_unit: 'weight', variant_unit_name: 'name', - unit_value: nil)).not_to be_valid - expect(build(:simple_product, variant_unit: 'weight', variant_unit_name: 'name', - unit_value: 0)).not_to be_valid - end - - it "requires a unit value when variant unit is volume" do - expect(build(:simple_product, variant_unit: 'volume', variant_unit_name: 'name', - unit_value: nil)).not_to be_valid - expect(build(:simple_product, variant_unit: 'volume', variant_unit_name: 'name', - unit_value: 0)).not_to be_valid - end - - it "does not require a unit value when variant unit is items" do - expect(build(:simple_product, variant_unit: 'items', variant_unit_name: 'name', - unit_value: nil)).to be_valid - end - end - +# context "when the product has variants" do let(:product) do product = create(:simple_product) @@ -151,30 +131,7 @@ module Spree product.reload end - it { is_expected.to validate_numericality_of(:price).is_greater_than_or_equal_to(0) } - - it "requires a unit" do - product.variant_unit = nil - expect(product).not_to be_valid - end - - %w(weight volume).each do |unit| - context "when unit is #{unit}" do - it "is valid when unit scale is set and unit name is not" do - product.variant_unit = unit - product.variant_unit_scale = 1 - product.variant_unit_name = nil - expect(product).to be_valid - end - - it "is invalid when unit scale is not set" do - product.variant_unit = unit - product.variant_unit_scale = nil - product.variant_unit_name = nil - expect(product).not_to be_valid - end - end - end + it { is_expected.to validate_numericality_of(:price).is_greater_than_or_equal_to(0) context "saving a new product" do let!(:product){ Spree::Product.new } @@ -189,6 +146,7 @@ module Spree product.variant_unit = "weight" product.variant_unit_scale = 1000 product.unit_value = 1 + product.unit_description = "some product" product.price = 4.27 product.shipping_category_id = shipping_category.id product.supplier_id = supplier.id @@ -198,49 +156,18 @@ module Spree it "copies properties to the first standard variant" do expect(product.variants.reload.length).to eq 1 standard_variant = product.variants.reload.first + expect(standard_variant).to be_valid + expect(standard_variant.variant_unit).to eq("weight") + expect(standard_variant.variant_unit_scale).to eq(1000) + expect(standard_variant.unit_value).to eq(1) + expect(standard_variant.unit_description).to eq("some product") expect(standard_variant.price).to eq 4.27 expect(standard_variant.shipping_category).to eq shipping_category expect(standard_variant.primary_taxon).to eq taxon expect(standard_variant.supplier).to eq supplier end end - - context "when the unit is items" do - it "is valid when unit name is set and unit scale is not" do - product.variant_unit = 'items' - product.variant_unit_name = 'loaf' - product.variant_unit_scale = nil - expect(product).to be_valid - end - - it "is invalid when unit name is not set" do - product.variant_unit = 'items' - product.variant_unit_name = nil - product.variant_unit_scale = nil - expect(product).not_to be_valid - end - end - end - - context "a basic product" do - let(:product) { build_stubbed(:simple_product) } - - it "requires variant unit fields" do - product.variant_unit = nil - product.variant_unit_name = nil - product.variant_unit_scale = nil - - expect(product).not_to be_valid - end - - it "requires a unit scale when variant unit is weight" do - product.variant_unit = 'weight' - product.variant_unit_scale = nil - product.variant_unit_name = nil - - expect(product).not_to be_valid - end end describe "#validate_image" do @@ -328,30 +255,6 @@ module Spree end end end - - it "updates units when saved change to variant unit" do - product.variant_unit = 'items' - product.variant_unit_scale = nil - product.variant_unit_name = 'loaf' - product.save! - - expect(product.variant_unit_name).to eq 'loaf' - - product.update(variant_unit_name: 'bag') - - expect(product.variant_unit_name).to eq 'bag' - - product.variant_unit = 'weight' - product.variant_unit_scale = 1 - product.variant_unit_name = 'g' - product.save! - - expect(product.variant_unit).to eq 'weight' - - product.update(variant_unit: 'volume') - - expect(product.variant_unit).to eq 'volume' - end end describe "scopes" do @@ -682,28 +585,6 @@ module Spree end end - describe "variant units" do - context "when the product already has a variant unit set" do - let!(:p) { - create(:simple_product, - variant_unit: 'weight', - variant_unit_scale: 1, - variant_unit_name: nil) - } - - it "updates its variants unit values" do - v = create(:variant, unit_value: 1, product: p) - p.reload - - expect(v.unit_presentation).to eq "1g" - - p.update!(variant_unit: 'volume', variant_unit_scale: 0.001) - - expect(v.reload.unit_presentation).to eq "1L" - end - end - end - describe "deletion" do let(:product) { create(:simple_product) } let(:variant) { create(:variant, product:) }