From c09aeeee8fe42f46a70024c505cd0835f620b838 Mon Sep 17 00:00:00 2001
From: Rob H <oeoeaio@gmail.com>
Date: Thu, 12 Jun 2014 12:11:20 +1000
Subject: [PATCH] Recalculate option values on variants when product variant
 unit is changed

---
 app/models/spree/product_decorator.rb |  2 +-
 app/models/spree/variant_decorator.rb | 23 +++++++++++------------
 spec/features/admin/products_spec.rb  |  2 ++
 spec/models/spree/product_spec.rb     | 24 +++++++++++++++++-------
 spec/models/spree/variant_spec.rb     |  4 ++--
 5 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 36473d4e24..f257e55c1b 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -158,7 +158,7 @@ Spree::Product.class_eval do
     if variant_unit_changed?
       option_types.delete self.class.all_variant_unit_option_types
       option_types << variant_unit_option_type if variant_unit.present?
-      variants_including_master.each { |v| v.delete_unit_option_values }
+      variants_including_master.each { |v| v.update_units }
     end
   end
 
diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb
index 2ffb07d4e1..25d09ae097 100644
--- a/app/models/spree/variant_decorator.rb
+++ b/app/models/spree/variant_decorator.rb
@@ -40,18 +40,6 @@ Spree::Variant.class_eval do
     values.to_sentence({ :words_connector => ", ", :two_words_connector => ", " })
   end
 
-  def delete_unit_option_values
-    ovs = self.option_values.where(option_type_id: Spree::Product.all_variant_unit_option_types)
-    self.option_values.destroy ovs
-  end
-
-
-  private
-
-  def update_weight_from_unit_value
-    self.weight = unit_value / 1000 if self.product.variant_unit == 'weight' && unit_value.present?
-  end
-
   def update_units
     delete_unit_option_values
 
@@ -63,6 +51,17 @@ Spree::Variant.class_eval do
     end
   end
 
+  private
+
+  def update_weight_from_unit_value
+    self.weight = unit_value / 1000 if self.product.variant_unit == 'weight' && unit_value.present?
+  end
+
+  def delete_unit_option_values
+    ovs = self.option_values.where(option_type_id: Spree::Product.all_variant_unit_option_types)
+    self.option_values.destroy ovs
+  end
+
   def option_value_name
     if display_as.present?
       display_as
diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb
index 35c6bc10cd..18aed5bf52 100644
--- a/spec/features/admin/products_spec.rb
+++ b/spec/features/admin/products_spec.rb
@@ -45,6 +45,8 @@ feature %q{
       product.on_hand.should == 5
       product.description.should == "A description..."
       product.group_buy.should be_false
+      product.master.option_values.map(&:name).should == ['5kg']
+      product.master.options_text.should == "5kg"
 
       # Distributors
       visit spree.product_distributions_admin_product_path(product)
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 08939532f6..3b21d425eb 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -442,24 +442,34 @@ module Spree
           p.update_attributes!(name: 'foo')
         end
 
-        it "removes the related option values from all its variants" do
+        it "removes the related option values from all its variants and replaces them" do
           ot = Spree::OptionType.find_by_name 'unit_weight'
-          v = create(:variant, product: p)
+          v = create(:variant, unit_value: 1, product: p)
           p.reload
 
-          expect {
+          v.option_values.map(&:name).include?("1L").should == false
+          v.option_values.map(&:name).include?("1g").should == true
+                    expect {
             p.update_attributes!(variant_unit: 'volume', variant_unit_scale: 0.001)
-          }.to change(v.option_values(true), :count).by(-1)
+          }.to change(p.master.option_values(true), :count).by(0)
+          v.reload
+          v.option_values.map(&:name).include?("1L").should == true
+          v.option_values.map(&:name).include?("1g").should == false
         end
 
-        it "removes the related option values from its master variant" do
+        it "removes the related option values from its master variant and replaces them" do
           ot = Spree::OptionType.find_by_name 'unit_weight'
           p.master.update_attributes!(unit_value: 1)
           p.reload
 
-          expect {
+          p.master.option_values.map(&:name).include?("1L").should == false
+          p.master.option_values.map(&:name).include?("1g").should == true
+                    expect {
             p.update_attributes!(variant_unit: 'volume', variant_unit_scale: 0.001)
-          }.to change(p.master.option_values(true), :count).by(-1)
+          }.to change(p.master.option_values(true), :count).by(0)
+          p.reload
+          p.master.option_values.map(&:name).include?("1L").should == true
+          p.master.option_values.map(&:name).include?("1g").should == false
         end
       end
 
diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb
index 535508ebe2..f5a3dd146d 100644
--- a/spec/models/spree/variant_spec.rb
+++ b/spec/models/spree/variant_spec.rb
@@ -264,13 +264,13 @@ module Spree
 
       it "removes option value associations for unit option types" do
         expect {
-          @v.delete_unit_option_values
+          @v.send(:delete_unit_option_values)
         }.to change(@v.option_values, :count).by(-1)
       end
 
       it "does not delete option values" do
         expect {
-          @v.delete_unit_option_values
+          @v.send(:delete_unit_option_values)
         }.to change(Spree::OptionValue, :count).by(0)
       end
     end