From a60c9a9ceafde7dc07f0aad4ee0f7b2c834bd3f5 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 9 Jan 2014 14:11:54 +1100
Subject: [PATCH] When variant already has a value set, remove old option value
 and assign a new one

---
 app/models/spree/variant_decorator.rb |  6 +++---
 spec/models/spree/variant_spec.rb     | 19 ++++++++++++++++++-
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb
index 387a747a0c..f430970ddc 100644
--- a/app/models/spree/variant_decorator.rb
+++ b/app/models/spree/variant_decorator.rb
@@ -13,8 +13,7 @@ Spree::Variant.class_eval do
 
 
   def delete_unit_option_values
-    ovs = self.option_values.where('option_type_id IN (?)',
-                                   Spree::Product.all_variant_unit_option_types)
+    ovs = self.option_values.where(option_type_id: Spree::Product.all_variant_unit_option_types)
     self.option_values.destroy ovs
   end
 
@@ -22,8 +21,9 @@ Spree::Variant.class_eval do
   private
 
   def update_units
-    option_type = self.product.variant_unit_option_type
+    delete_unit_option_values
 
+    option_type = self.product.variant_unit_option_type
     if option_type
       name = option_value_name
       ov = Spree::OptionValue.where(option_type_id: option_type, name: name, presentation: name).first || Spree::OptionValue.create!({option_type: option_type, name: name, presentation: name}, without_protection: true)
diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb
index d3ba1cae13..9506d75239 100644
--- a/spec/models/spree/variant_spec.rb
+++ b/spec/models/spree/variant_spec.rb
@@ -123,7 +123,24 @@ module Spree
         end
 
         context "when the variant already has a value set (and all required option values exist)" do
-          it "removes the old option value and assigns the new one"
+          let!(:p0) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) }
+          let!(:v0) { create(:variant, product: p0, unit_value: 10, unit_description: 'foo') }
+
+          let!(:p) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) }
+          let!(:v) { create(:variant, product: p, unit_value: 5, unit_description: 'bar') }
+
+          it "removes the old option value and assigns the new one" do
+            ov_orig = v.option_values.last
+            ov_new  = v0.option_values.last
+
+            expect {
+              v.update_attributes!(unit_value: 10, unit_description: 'foo')
+            }.to change(Spree::OptionValue, :count).by(0)
+
+            v.option_values.should_not include ov_orig
+            v.option_values.should     include ov_new
+          end
+
           it "leaves option value unassigned if none is provided"
           it "does not remove and re-add the option value if it is not changed"
         end