diff --git a/app/models/spree/price.rb b/app/models/spree/price.rb index 668f53b6b9..0c2d12c285 100644 --- a/app/models/spree/price.rb +++ b/app/models/spree/price.rb @@ -24,10 +24,6 @@ module Spree amount end - def price_changed? - amount_changed? - end - def price=(price) self[:amount] = parse_price(price) end diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 294c1edfe0..18b7cb0d24 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -49,7 +49,7 @@ module Spree has_many :prices, class_name: 'Spree::Price', dependent: :destroy - delegate :display_price, :display_amount, :price, :price_changed?, :price=, + delegate :display_price, :display_amount, :price, :price=, :currency, :currency=, to: :find_or_build_default_price @@ -199,6 +199,11 @@ module Spree price_in(currency).try(:amount) end + def changed? + # We consider the variant changed if associated price is changed (it is saved after_save) + super || default_price.changed? + end + # can_supply? is implemented in VariantStock def in_stock?(quantity = 1) can_supply?(quantity) diff --git a/app/services/sets/product_set.rb b/app/services/sets/product_set.rb index ba3fb40240..04138eedbc 100644 --- a/app/services/sets/product_set.rb +++ b/app/services/sets/product_set.rb @@ -67,11 +67,12 @@ module Sets product.assign_attributes(product_related_attrs) + return true unless product.changed? + validate_presence_of_unit_value_in_product(product) - changed = product.changed? success = product.errors.empty? && product.save - count_result(success && changed) + count_result(success) success end @@ -104,7 +105,8 @@ module Sets def create_or_update_variant(product, variant_attributes) variant = find_model(product.variants, variant_attributes[:id]) if variant.present? - variant.update(variant_attributes.except(:id)) + variant.assign_attributes(variant_attributes.except(:id)) + variant.save if variant.changed? else variant = create_variant(product, variant_attributes) end diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 832cb6d931..3cea5ba96f 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -58,6 +58,28 @@ RSpec.describe Spree::Variant do end end + describe "#changed?" do + subject(:variant) { create(:variant) } + + it { is_expected.not_to be_changed } + + it "is changed when basic fields are changed" do + subject.display_name = "blah" + expect(subject).to be_changed + end + + describe "default_price" do + it "price" do + subject.price = 100 + expect(subject).to be_changed + end + it "currency" do + subject.currency = "USD" + expect(subject).to be_changed + end + end + end + context "price parsing" do context "price=" do context "with decimal point" do diff --git a/spec/services/sets/product_set_spec.rb b/spec/services/sets/product_set_spec.rb index b60dd0f2a5..3d2181c493 100644 --- a/spec/services/sets/product_set_spec.rb +++ b/spec/services/sets/product_set_spec.rb @@ -134,6 +134,27 @@ RSpec.describe Sets::ProductSet do end end end + + context "when product attributes are not changed" do + let(:collection_hash) { + { 0 => { id: product.id, name: product.name } } + } + + it 'returns true' do + is_expected.to eq true + end + + it 'does not increase saved_count' do + subject + expect(product_set.saved_count).to eq 0 + end + + it 'does not update any product by calling save' do + expect_any_instance_of(Spree::Product).not_to receive(:save) + + subject + end + end end describe "updating a product's variants" do @@ -190,6 +211,23 @@ RSpec.describe Sets::ProductSet do include_examples "nothing saved" end + context "when attributes are not changed" do + let(:variant_attributes) { { sku: variant.sku } } + + before { variant } + + it 'updates product by calling save' do + expect_any_instance_of(Spree::Variant).not_to receive(:save) + + subject + end + + it 'does not increase saved_count' do + subject + expect(product_set.saved_count).to eq 0 + end + end + context "when products attributes are also updated" do let(:product_attributes) { { sku: "prod_sku" }