From 3f01a459ac317a5fef7ac6b753bfcf6eca074bf8 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 26 Sep 2014 12:17:55 +1000
Subject: [PATCH 01/37] Adding a standard variant upon initialisation of a new
 instance of Spree::Product

---
 app/models/spree/product_decorator.rb | 4 ++++
 spec/models/spree/product_spec.rb     | 9 ++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index cf7d02bf46..034bbed043 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -32,6 +32,7 @@ Spree::Product.class_eval do
   validates_presence_of :variant_unit_name,
                         if: -> p { p.variant_unit == 'items' }
 
+  after_initialize :ensure_first_standard_variant, :if => :new_record?
   after_initialize :set_available_on_to_now, :if => :new_record?
   after_save :update_units
   after_touch :touch_distributors
@@ -205,4 +206,7 @@ Spree::Product.class_eval do
     Spree::OptionType.where('name LIKE ?', 'unit_%%')
   end
 
+  def ensure_first_standard_variant
+    self.variants << Spree::Variant.new
+  end
 end
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 400f92754b..73c6f2fe10 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -85,6 +85,14 @@ module Spree
           end
         end
 
+        context "instatiating a new product" do
+          let!(:product) { Spree::Product.new }
+
+          it "creates a standard (non-master) variant when created" do
+            product.variants.should_not be_empty
+          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'
@@ -122,7 +130,6 @@ module Spree
         end
       end
     end
-    
 
     describe "scopes" do
       describe "in_supplier" do

From dfb513cce7943fcb59dab66d35daaeb0a405383d Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 26 Sep 2014 13:11:32 +1000
Subject: [PATCH 02/37] Use after_create callback to duplicate master variant

---
 app/models/spree/product_decorator.rb |  9 ++++++---
 spec/models/spree/product_spec.rb     | 18 ++++++++++++++----
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 034bbed043..aae90f41c4 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -32,7 +32,7 @@ Spree::Product.class_eval do
   validates_presence_of :variant_unit_name,
                         if: -> p { p.variant_unit == 'items' }
 
-  after_initialize :ensure_first_standard_variant, :if => :new_record?
+  after_create :ensure_one_standard_variant
   after_initialize :set_available_on_to_now, :if => :new_record?
   after_save :update_units
   after_touch :touch_distributors
@@ -206,7 +206,10 @@ Spree::Product.class_eval do
     Spree::OptionType.where('name LIKE ?', 'unit_%%')
   end
 
-  def ensure_first_standard_variant
-    self.variants << Spree::Variant.new
+  def ensure_one_standard_variant
+    variant = self.master.dup
+    variant.product = self
+    variant.is_master = false
+    self.variants << variant
   end
 end
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 73c6f2fe10..e3a72f19c5 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -85,11 +85,21 @@ module Spree
           end
         end
 
-        context "instatiating a new product" do
-          let!(:product) { Spree::Product.new }
+        context "saving a new product" do
+          let!(:product){ Spree::Product.new }
 
-          it "creates a standard (non-master) variant when created" do
-            product.variants.should_not be_empty
+          before do
+            product.primary_taxon = create(:taxon)
+            product.supplier = create(:supplier_enterprise)
+            product.name = "Product1"
+            product.on_hand = 3
+            product.price = 4.27
+            product.save!
+          end
+
+          it "copies the properties on master variant to the first standard variant" do
+            standard_variant = product.variants(:reload).first
+            expect(standard_variant.price).to eq product.master.price
           end
         end
 

From 0a7b01ff07f36891f39a99fa8850ca143fa96b56 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 2 Oct 2014 16:13:03 +1000
Subject: [PATCH 03/37] Product requires variant_unit and master requires
 unit_value and/or unit_desc

---
 app/models/spree/product_decorator.rb |   6 +-
 app/models/spree/variant_decorator.rb |   6 +-
 spec/factories.rb                     |   4 +
 spec/models/spree/product_spec.rb     | 106 +++++---------------------
 spec/models/spree/variant_spec.rb     |  62 ++-------------
 5 files changed, 35 insertions(+), 149 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index aae90f41c4..b6cd0bb221 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -25,14 +25,14 @@ Spree::Product.class_eval do
   validates_presence_of :supplier
   validates :primary_taxon, presence: { message: "^Product Category can't be blank" }
   validates :tax_category_id, presence: { message: "^Tax Category can't be blank" }, if: "Spree::Config.products_require_tax_category"
-  
-  validates_presence_of :variant_unit, if: :has_variants?
+
+  validates_presence_of :variant_unit
   validates_presence_of :variant_unit_scale,
                         if: -> p { %w(weight volume).include? p.variant_unit }
   validates_presence_of :variant_unit_name,
                         if: -> p { p.variant_unit == 'items' }
 
-  after_create :ensure_one_standard_variant
+  #after_create :ensure_one_standard_variant
   after_initialize :set_available_on_to_now, :if => :new_record?
   after_save :update_units
   after_touch :touch_distributors
diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb
index 86a0b5fd69..703111c884 100644
--- a/app/models/spree/variant_decorator.rb
+++ b/app/models/spree/variant_decorator.rb
@@ -12,12 +12,10 @@ Spree::Variant.class_eval do
   accepts_nested_attributes_for :images
 
   validates_presence_of :unit_value,
-    if: -> v { %w(weight volume).include? v.product.andand.variant_unit },
-    unless: :is_master
+    if: -> v { %w(weight volume).include? v.product.andand.variant_unit }
 
   validates_presence_of :unit_description,
-    if: -> v { v.product.andand.variant_unit.present? && v.unit_value.nil? },
-    unless: :is_master
+    if: -> v { v.product.andand.variant_unit.present? && v.unit_value.nil? }
 
   before_validation :update_weight_from_unit_value, if: -> v { v.product.present? }
   after_save :update_units
diff --git a/spec/factories.rb b/spec/factories.rb
index aa86af8563..7effe49a9d 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -214,6 +214,10 @@ end
 
 
 FactoryGirl.modify do
+  factory :base_product do
+    unit_value 1
+    unit_description ''
+  end
   factory :product do
     primary_taxon { Spree::Taxon.first || FactoryGirl.create(:taxon) }
   end
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index e3a72f19c5..aca9404fb7 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -85,23 +85,24 @@ module Spree
           end
         end
 
-        context "saving a new product" do
-          let!(:product){ Spree::Product.new }
-
-          before do
-            product.primary_taxon = create(:taxon)
-            product.supplier = create(:supplier_enterprise)
-            product.name = "Product1"
-            product.on_hand = 3
-            product.price = 4.27
-            product.save!
-          end
-
-          it "copies the properties on master variant to the first standard variant" do
-            standard_variant = product.variants(:reload).first
-            expect(standard_variant.price).to eq product.master.price
-          end
-        end
+        # context "saving a new product" do
+        #   let!(:product){ Spree::Product.new }
+        #
+        #   before do
+        #     product.primary_taxon = create(:taxon)
+        #     product.supplier = create(:supplier_enterprise)
+        #     product.name = "Product1"
+        #     product.variant_unit = "weight"
+        #     product.on_hand = 3
+        #     product.price = 4.27
+        #     product.save!
+        #   end
+        #
+        #   it "copies the properties on master variant to the first standard variant" do
+        #     standard_variant = product.variants(:reload).first
+        #     expect(standard_variant.price).to eq product.master.price
+        #   end
+        # end
 
         context "when the unit is items" do
           it "is valid when unit name is set and unit scale is not" do
@@ -128,7 +129,7 @@ module Spree
           product.variant_unit_name = nil
           product.variant_unit_scale = nil
 
-          product.should be_valid
+          expect(product).to be_invalid
         end
 
         it "requires a unit scale when variant unit is weight" do
@@ -412,63 +413,6 @@ module Spree
 
 
     describe "variant units" do
-      context "when the product initially has no variant unit" do
-        let!(:p) { create(:simple_product,
-                          variant_unit: nil,
-                          variant_unit_scale: nil,
-                          variant_unit_name: nil) }
-
-        context "when the required option type does not exist" do
-          it "creates the option type and assigns it to the product" do
-            expect {
-              p.update_attributes!(variant_unit: 'weight', variant_unit_scale: 1000)
-            }.to change(Spree::OptionType, :count).by(1)
-
-            ot = Spree::OptionType.last
-            ot.name.should == 'unit_weight'
-            ot.presentation.should == 'Weight'
-
-            p.option_types.should == [ot]
-          end
-
-          it "does the same with volume" do
-            expect {
-              p.update_attributes!(variant_unit: 'volume', variant_unit_scale: 1000)
-            }.to change(Spree::OptionType, :count).by(1)
-
-            ot = Spree::OptionType.last
-            ot.name.should == 'unit_volume'
-            ot.presentation.should == 'Volume'
-
-            p.option_types.should == [ot]
-          end
-
-          it "does the same with items" do
-            expect {
-              p.update_attributes!(variant_unit: 'items', variant_unit_name: 'packet')
-            }.to change(Spree::OptionType, :count).by(1)
-
-            ot = Spree::OptionType.last
-            ot.name.should == 'unit_items'
-            ot.presentation.should == 'Items'
-
-            p.option_types.should == [ot]
-          end
-        end
-
-        context "when the required option type already exists" do
-          let!(:ot) { create(:option_type, name: 'unit_weight', presentation: 'Weight') }
-
-          it "looks up the option type and assigns it to the product" do
-            expect {
-              p.update_attributes!(variant_unit: 'weight', variant_unit_scale: 1000)
-            }.to change(Spree::OptionType, :count).by(0)
-
-            p.option_types.should == [ot]
-          end
-        end
-      end
-
       context "when the product already has a variant unit set (and all required option types exist)" do
         let!(:p) { create(:simple_product,
                           variant_unit: 'weight',
@@ -482,11 +426,6 @@ module Spree
           p.option_types.should == [ot_volume]
         end
 
-        it "leaves option type unassigned if none is provided" do
-          p.update_attributes!(variant_unit: nil, variant_unit_scale: nil)
-          p.option_types.should == []
-        end
-
         it "does not remove and re-add the option type if it is not changed" do
           p.option_types.should_receive(:delete).never
           p.update_attributes!(name: 'foo')
@@ -523,13 +462,6 @@ module Spree
         end
       end
 
-      describe "returning the variant unit option type" do
-        it "returns nil when variant_unit is not set" do
-          p = create(:simple_product, variant_unit: nil)
-          p.variant_unit_option_type.should be_nil
-        end
-      end
-
       it "finds all variant unit option types" do
         ot1 = create(:option_type, name: 'unit_weight', presentation: 'Weight')
         ot2 = create(:option_type, name: 'unit_volume', presentation: 'Volume')
diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb
index 5a82f04ba7..aa3296e5dc 100644
--- a/spec/models/spree/variant_spec.rb
+++ b/spec/models/spree/variant_spec.rb
@@ -240,12 +240,14 @@ module Spree
     end
 
     context "when the product does not have variants" do
-      let(:product) { create(:simple_product, variant_unit: nil) }
+      let(:product) { create(:simple_product) }
       let(:variant) { product.master }
 
-      it "does not require unit value or unit description when the product's unit is empty" do
+      it "requires unit value and/or unit description" do
         variant.unit_value = nil
         variant.unit_description = nil
+        variant.should be_invalid
+        variant.unit_value = 1
         variant.should be_valid
       end
     end
@@ -283,7 +285,7 @@ module Spree
 
       describe "setting the variant's weight from the unit value" do
         it "sets the variant's weight when unit is weight" do
-          p = create(:simple_product, variant_unit: nil, variant_unit_scale: nil)
+          p = create(:simple_product, variant_unit: 'volume')
           v = create(:variant, product: p, weight: nil)
 
           p.update_attributes! variant_unit: 'weight', variant_unit_scale: 1
@@ -293,7 +295,7 @@ module Spree
         end
 
         it "does nothing when unit is not weight" do
-          p = create(:simple_product, variant_unit: nil, variant_unit_scale: nil)
+          p = create(:simple_product, variant_unit: 'volume')
           v = create(:variant, product: p, weight: 123)
 
           p.update_attributes! variant_unit: 'volume', variant_unit_scale: 1
@@ -303,7 +305,7 @@ module Spree
         end
 
         it "does nothing when unit_value is not set" do
-          p = create(:simple_product, variant_unit: nil, variant_unit_scale: nil)
+          p = create(:simple_product, variant_unit: 'volume')
           v = create(:variant, product: p, weight: 123)
 
           p.update_attributes! variant_unit: 'weight', variant_unit_scale: 1
@@ -316,56 +318,6 @@ module Spree
         end
       end
 
-      context "when the variant initially has no value" do
-        context "when the required option value does not exist" do
-          let!(:p) { create(:simple_product, variant_unit: nil, variant_unit_scale: nil) }
-          let!(:v) { create(:variant, product: p, unit_value: nil, unit_description: nil) }
-
-          before do
-            p.update_attributes!(variant_unit: 'weight', variant_unit_scale: 1)
-            @ot = Spree::OptionType.find_by_name 'unit_weight'
-          end
-
-          it "creates the option value and assigns it to the variant" do
-            expect {
-              v.update_attributes!(unit_value: 10, unit_description: 'foo')
-            }.to change(Spree::OptionValue, :count).by(1)
-
-            ov = Spree::OptionValue.last
-            ov.option_type.should == @ot
-            ov.name.should == '10g foo'
-            ov.presentation.should == '10g foo'
-
-            v.option_values.should include ov
-          end
-        end
-
-        context "when the required option value already exists" do
-          let!(:p_orig) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) }
-          let!(:v_orig) { create(:variant, product: p_orig, unit_value: 10, unit_description: 'foo') }
-
-          let!(:p) { create(:simple_product, variant_unit: nil, variant_unit_scale: nil) }
-          let!(:v) { create(:variant, product: p, unit_value: nil, unit_description: nil) }
-
-          before do
-            p.update_attributes!(variant_unit: 'weight', variant_unit_scale: 1)
-            @ot = Spree::OptionType.find_by_name 'unit_weight'
-          end
-
-          it "looks up the option value and assigns it to the variant" do
-            expect {
-              v.update_attributes!(unit_value: 10, unit_description: 'foo')
-            }.to change(Spree::OptionValue, :count).by(0)
-
-            ov = v.option_values.last
-            ov.option_type.should == @ot
-            ov.name.should == '10g foo'
-            ov.presentation.should == '10g foo'
-
-            v_orig.option_values.should include ov
-          end
-        end
-      end
       context "when the variant already has a value set (and all required option values exist)" do
         let!(:p0) { create(:simple_product, variant_unit: 'weight', variant_unit_scale: 1) }
         let!(:v0) { create(:variant, product: p0, unit_value: 10, unit_description: 'foo') }

From 13a910c3727755bbf174cd3ef869ccb40979459e Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 2 Oct 2014 17:08:18 +1000
Subject: [PATCH 04/37] Replace validates_associated on master with current
 spree method for error reporting master saves

---
 app/models/spree/product_decorator.rb | 20 +++++++++++++++++++-
 spec/models/spree/product_spec.rb     |  5 +++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index b6cd0bb221..258d1e0df2 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -21,7 +21,6 @@ Spree::Product.class_eval do
   attr_accessible :supplier_id, :primary_taxon_id, :distributor_ids, :product_distributions_attributes, :group_buy, :group_buy_unit_size
   attr_accessible :variant_unit, :variant_unit_scale, :variant_unit_name, :unit_value, :unit_description, :notes, :images_attributes, :display_as
 
-  validates_associated :master, message: "^Price and On Hand must be valid"
   validates_presence_of :supplier
   validates :primary_taxon, presence: { message: "^Product Category can't be blank" }
   validates :tax_category_id, presence: { message: "^Tax Category can't be blank" }, if: "Spree::Config.products_require_tax_category"
@@ -212,4 +211,23 @@ Spree::Product.class_eval do
     variant.is_master = false
     self.variants << variant
   end
+
+  # Override Spree's old save_master method and replace it with the most recent method from spree repository
+  # This fixes any problems arising from failing master saves, without the need for a validates_associated on
+  # master, while giving us more specific errors as to why saving failed
+  def save_master
+    begin
+      if master && (master.changed? || master.new_record? || (master.default_price && (master.default_price.changed? || master.default_price.new_record?)))
+        master.save!
+      end
+
+    # If the master cannot be saved, the Product object will get its errors
+    # and will be destroyed
+    rescue ActiveRecord::RecordInvalid
+      master.errors.each do |att, error|
+        self.errors.add(att, error)
+      end
+      raise
+    end
+  end
 end
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index aca9404fb7..70bfb38227 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -25,9 +25,10 @@ module Spree
       it "does not save when master is invalid" do
         s = create(:supplier_enterprise)
         t = create(:taxon)
-        product = Product.new supplier_id: s.id, name: "Apples", price: 1, primary_taxon_id: t.id
+        product = Product.new supplier_id: s.id, name: "Apples", price: 1, primary_taxon_id: t.id, variant_unit: "weight", variant_unit_scale: 1000, unit_value: 1
         product.on_hand = "10,000"
-        product.save.should be_false
+        expect(product.save).to be_false
+        expect(product.errors[:count_on_hand]).to include "is not a number"
       end
 
       it "defaults available_on to now" do

From 869551a17c2b033061c197e9cbd48ed2e302658a Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 2 Oct 2014 17:22:30 +1000
Subject: [PATCH 05/37] Adding a standard variant again

---
 app/models/spree/product_decorator.rb |  5 ++--
 spec/models/spree/product_spec.rb     | 39 ++++++++++++++-------------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 258d1e0df2..c83ffd0c0a 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -21,6 +21,8 @@ Spree::Product.class_eval do
   attr_accessible :supplier_id, :primary_taxon_id, :distributor_ids, :product_distributions_attributes, :group_buy, :group_buy_unit_size
   attr_accessible :variant_unit, :variant_unit_scale, :variant_unit_name, :unit_value, :unit_description, :notes, :images_attributes, :display_as
 
+  before_validation :ensure_standard_variant
+
   validates_presence_of :supplier
   validates :primary_taxon, presence: { message: "^Product Category can't be blank" }
   validates :tax_category_id, presence: { message: "^Tax Category can't be blank" }, if: "Spree::Config.products_require_tax_category"
@@ -31,7 +33,6 @@ Spree::Product.class_eval do
   validates_presence_of :variant_unit_name,
                         if: -> p { p.variant_unit == 'items' }
 
-  #after_create :ensure_one_standard_variant
   after_initialize :set_available_on_to_now, :if => :new_record?
   after_save :update_units
   after_touch :touch_distributors
@@ -205,7 +206,7 @@ Spree::Product.class_eval do
     Spree::OptionType.where('name LIKE ?', 'unit_%%')
   end
 
-  def ensure_one_standard_variant
+  def ensure_standard_variant
     variant = self.master.dup
     variant.product = self
     variant.is_master = false
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 70bfb38227..2160b42b5b 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -86,24 +86,27 @@ module Spree
           end
         end
 
-        # context "saving a new product" do
-        #   let!(:product){ Spree::Product.new }
-        #
-        #   before do
-        #     product.primary_taxon = create(:taxon)
-        #     product.supplier = create(:supplier_enterprise)
-        #     product.name = "Product1"
-        #     product.variant_unit = "weight"
-        #     product.on_hand = 3
-        #     product.price = 4.27
-        #     product.save!
-        #   end
-        #
-        #   it "copies the properties on master variant to the first standard variant" do
-        #     standard_variant = product.variants(:reload).first
-        #     expect(standard_variant.price).to eq product.master.price
-        #   end
-        # end
+        context "saving a new product" do
+          let!(:product){ Spree::Product.new }
+
+          before do
+            product.primary_taxon = create(:taxon)
+            product.supplier = create(:supplier_enterprise)
+            product.name = "Product1"
+            product.variant_unit = "weight"
+            product.variant_unit_scale = 1000
+            product.unit_value = 1
+            product.on_hand = 3
+            product.price = 4.27
+            product.save!
+          end
+
+          it "copies the properties on master variant to the first standard variant" do
+            expect(product.variants(:reload).length).to eq 1
+            standard_variant = product.variants(:reload).first
+            expect(standard_variant.price).to eq product.master.price
+          end
+        end
 
         context "when the unit is items" do
           it "is valid when unit name is set and unit scale is not" do

From 28486f9e76130e6381d45bd9df91b5031e446849 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 2 Oct 2014 17:24:10 +1000
Subject: [PATCH 06/37] Only adds standard variant on create

---
 app/models/spree/product_decorator.rb | 2 +-
 spec/models/spree/product_spec.rb     | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index c83ffd0c0a..bb246ace59 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -21,7 +21,7 @@ Spree::Product.class_eval do
   attr_accessible :supplier_id, :primary_taxon_id, :distributor_ids, :product_distributions_attributes, :group_buy, :group_buy_unit_size
   attr_accessible :variant_unit, :variant_unit_scale, :variant_unit_name, :unit_value, :unit_description, :notes, :images_attributes, :display_as
 
-  before_validation :ensure_standard_variant
+  before_validation :ensure_standard_variant, if: :new_record?
 
   validates_presence_of :supplier
   validates :primary_taxon, presence: { message: "^Product Category can't be blank" }
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 2160b42b5b..7eb5b777ac 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -106,6 +106,11 @@ module Spree
             standard_variant = product.variants(:reload).first
             expect(standard_variant.price).to eq product.master.price
           end
+
+          it "only duplicates master variant on initial save" do
+            product.save!
+            expect(product.variants(:reload).length).to eq 1
+          end
         end
 
         context "when the unit is items" do

From 971723964e05d8d576d20be9aeb2dd5d1e02a5db Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 2 Oct 2014 17:49:57 +1000
Subject: [PATCH 07/37] Update outdated spec

---
 spec/models/spree/product_spec.rb | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 7eb5b777ac..57327b75ad 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -28,7 +28,12 @@ module Spree
         product = Product.new supplier_id: s.id, name: "Apples", price: 1, primary_taxon_id: t.id, variant_unit: "weight", variant_unit_scale: 1000, unit_value: 1
         product.on_hand = "10,000"
         expect(product.save).to be_false
-        expect(product.errors[:count_on_hand]).to include "is not a number"
+
+        # It is expected that master variant :count_on_hand should cause the failure to save, and it does. However, this broke with the addition of
+        # the :ensure_standard_variant callback. Evidently, normal variants are checked and their errors added to the product model before the
+        # save_master callback on product is run. Therefore, the error that is raised here is one on a normal variant, rather than one on master.
+        # expect(product.errors[:count_on_hand]).to include "is not a number"
+        expect(product.errors[:"variants.count_on_hand"]).to include "is not a number"
       end
 
       it "defaults available_on to now" do

From 89afbc80a6953a984854eb17f4e4975087d1f043 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 2 Oct 2014 18:01:46 +1000
Subject: [PATCH 08/37] Set initial on_hand to 0

---
 spec/models/spree/variant_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb
index aa3296e5dc..eec2868453 100644
--- a/spec/models/spree/variant_spec.rb
+++ b/spec/models/spree/variant_spec.rb
@@ -14,7 +14,7 @@ module Spree
 
       describe "finding variants in stock" do
         before do
-          p = create(:product)
+          p = create(:product, on_hand: 0)
           @v_in_stock = create(:variant, product: p)
           @v_on_demand = create(:variant, product: p, on_demand: true)
           @v_no_stock = create(:variant, product: p)

From 5e2fe56c228ed1f1a7d7b40fc22f4de74529fba3 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 2 Oct 2014 18:05:24 +1000
Subject: [PATCH 09/37] Cleanup

---
 spec/models/spree/product_spec.rb | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 57327b75ad..07b67ce872 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -135,10 +135,10 @@ module Spree
         end
       end
 
-      context "when product does not have variants" do
+      context "a basic product" do
         let(:product) { create(:simple_product) }
 
-        it "does not require any variant unit fields" do
+        it "requires variant unit fields" do
           product.variant_unit = nil
           product.variant_unit_name = nil
           product.variant_unit_scale = nil
@@ -523,16 +523,6 @@ module Spree
       end
 
       describe "finding products in stock for a particular distribution" do
-        it "returns in-stock products without variants" do
-          p = create(:simple_product)
-          p.master.update_attribute(:count_on_hand, 1)
-          d = create(:distributor_enterprise)
-          oc = create(:simple_order_cycle, distributors: [d])
-          oc.exchanges.outgoing.first.variants << p.master
-
-          p.should have_stock_for_distribution(oc, d)
-        end
-
         it "returns on-demand products" do
           p = create(:simple_product, on_demand: true)
           p.master.update_attribute(:count_on_hand, 0)

From a223a2d662a44896b7fa1615dccda1751895bf50 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 2 Oct 2014 18:32:40 +1000
Subject: [PATCH 10/37] Cannot remove all variants from a product

---
 app/models/spree/product_decorator.rb |  1 +
 spec/models/spree/product_spec.rb     | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index bb246ace59..3d35700b7c 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -23,6 +23,7 @@ Spree::Product.class_eval do
 
   before_validation :ensure_standard_variant, if: :new_record?
 
+  validates_presence_of :variants, message: "Product must have at least one variant"
   validates_presence_of :supplier
   validates :primary_taxon, presence: { message: "^Product Category can't be blank" }
   validates :tax_category_id, presence: { message: "^Tax Category can't be blank" }, if: "Spree::Config.products_require_tax_category"
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 07b67ce872..35dd847d1f 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -8,7 +8,7 @@ module Spree
       it { should belong_to(:primary_taxon) }
       it { should have_many(:product_distributions) }
     end
-    
+
     describe "validations and defaults" do
       it "is valid when built from factory" do
         build(:product).should be_valid
@@ -61,6 +61,14 @@ module Spree
       end
 
 
+      it "does not allow only standard variant to be deleted" do
+        product = create(:simple_product)
+        expect(product.variants(:reload).length).to eq 1
+        product.variants = []
+        expect(product.save).to be_false
+        expect(product.errors[:variants]).to include "Product must have at least one variant"
+      end
+
       context "when the product has variants" do
         let(:product) do
           product = create(:simple_product)

From fcb3bc894b18fb6e43a1b00f5b4413b19a3173b6 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 2 Oct 2014 18:35:37 +1000
Subject: [PATCH 11/37] Cleanup

---
 spec/models/spree/variant_spec.rb | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb
index eec2868453..c038bf4598 100644
--- a/spec/models/spree/variant_spec.rb
+++ b/spec/models/spree/variant_spec.rb
@@ -239,19 +239,6 @@ module Spree
       end
     end
 
-    context "when the product does not have variants" do
-      let(:product) { create(:simple_product) }
-      let(:variant) { product.master }
-
-      it "requires unit value and/or unit description" do
-        variant.unit_value = nil
-        variant.unit_description = nil
-        variant.should be_invalid
-        variant.unit_value = 1
-        variant.should be_valid
-      end
-    end
-
     describe "unit value/description" do
       describe "getting name for display" do
         it "returns display_name if present" do

From 2b47c9145a9b18f346dd186ef4254dd062eb0648 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 3 Oct 2014 13:59:34 +1000
Subject: [PATCH 12/37] Cannot delete last variant of product

---
 app/models/spree/variant_decorator.rb |  8 ++++++++
 spec/models/spree/variant_spec.rb     | 15 +++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb
index 703111c884..ee05b3b2b8 100644
--- a/app/models/spree/variant_decorator.rb
+++ b/app/models/spree/variant_decorator.rb
@@ -18,6 +18,7 @@ Spree::Variant.class_eval do
     if: -> v { v.product.andand.variant_unit.present? && v.unit_value.nil? }
 
   before_validation :update_weight_from_unit_value, if: -> v { v.product.present? }
+  before_destroy :check_product_variants
   after_save :update_units
 
   scope :with_order_cycles_inner, joins(exchanges: :order_cycle)
@@ -123,4 +124,11 @@ Spree::Variant.class_eval do
       option_value_namer.name
     end
   end
+
+  def check_product_variants
+    if product.variants == [self] # Only variant left on product
+      errors.add :product, "must have at least one variant"
+      return false
+    end
+  end
 end
diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb
index c038bf4598..2c37065916 100644
--- a/spec/models/spree/variant_spec.rb
+++ b/spec/models/spree/variant_spec.rb
@@ -399,5 +399,20 @@ module Spree
       v.destroy
       e.reload.variant_ids.should be_empty
     end
+
+    context "as the last variant of a product" do
+      let!(:product) { create(:simple_product) }
+      let!(:first_variant) { product.variants(:reload).first }
+      let!(:extra_variant) { create(:variant, product: product) }
+
+      it "cannot be deleted" do
+        expect(product.variants(:reload).length).to eq 2
+        expect(extra_variant.destroy).to eq extra_variant
+        expect(product.variants(:reload).length).to eq 1
+        expect(first_variant.destroy).to be_false
+        expect(product.variants(:reload).length).to eq 1
+        expect(first_variant.errors[:product]).to include "must have at least one variant"
+      end
+    end
   end
 end

From 4b182f92484a138c31620977c3e5b40b6efccb9c Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 3 Oct 2014 14:08:03 +1000
Subject: [PATCH 13/37] Can't delete final variant on a product from BPE

---
 .../admin/bulk_product_update.js.coffee       | 19 +++++++++-------
 .../unit/bulk_product_update_spec.js.coffee   | 22 +++++++++++++++----
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee
index e8e14413ae..5ec6eb52f8 100644
--- a/app/assets/javascripts/admin/bulk_product_update.js.coffee
+++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee
@@ -135,15 +135,18 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout
 
 
     $scope.deleteVariant = (product, variant) ->
-      if !$scope.variantSaved(variant)
-        $scope.removeVariant(product, variant)
+      if product.variants.length > 1
+        if !$scope.variantSaved(variant)
+          $scope.removeVariant(product, variant)
+        else
+          if confirm("Are you sure?")
+            $http(
+              method: "DELETE"
+              url: "/api/products/" + product.permalink_live + "/variants/" + variant.id + "/soft_delete"
+            ).success (data) ->
+              $scope.removeVariant(product, variant)
       else
-        if confirm("Are you sure?")
-          $http(
-            method: "DELETE"
-            url: "/api/products/" + product.permalink_live + "/variants/" + variant.id + "/soft_delete"
-          ).success (data) ->
-            $scope.removeVariant(product, variant)
+        alert("The last variant cannot be deleted!")
 
     $scope.removeVariant = (product, variant) ->
       product.variants.splice product.variants.indexOf(variant), 1
diff --git a/spec/javascripts/unit/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/bulk_product_update_spec.js.coffee
index a6fb0fa432..eb3830204c 100644
--- a/spec/javascripts/unit/bulk_product_update_spec.js.coffee
+++ b/spec/javascripts/unit/bulk_product_update_spec.js.coffee
@@ -777,22 +777,31 @@ describe "AdminProductEditCtrl", ->
 
 
   describe "deleting variants", ->
+    describe "when the variant is the only one left on the product", ->
+      it "alerts the user", ->
+        spyOn(window, "alert")
+        $scope.products = [
+          {id: 1, variants: [{id: 1}]}
+        ]
+        $scope.deleteVariant $scope.products[0], $scope.products[0].variants[0]
+        expect(window.alert).toHaveBeenCalledWith "The last variant cannot be deleted!"
+
     describe "when the variant has not been saved", ->
       it "removes the variant from products and dirtyProducts", ->
         spyOn(window, "confirm").andReturn true
         $scope.products = [
-          {id: 1, variants: [{id: -1}]}
+          {id: 1, variants: [{id: -1},{id: -2}]}
         ]
         DirtyProducts.addVariantProperty 1, -1, "something", "something"
         DirtyProducts.addProductProperty 1, "something", "something"
         $scope.deleteVariant $scope.products[0], $scope.products[0].variants[0]
         expect($scope.products).toEqual([
-          {id: 1, variants: []}
+          {id: 1, variants: [{id: -2}]}
         ])
         expect(DirtyProducts.all()).toEqual
           1: { id: 1, something: 'something'}
 
-  
+
     describe "when the variant has been saved", ->
       it "deletes variants with a http delete request to /api/products/product_permalink/variants/(variant_id)/soft_delete", ->
         spyOn(window, "confirm").andReturn true
@@ -800,9 +809,14 @@ describe "AdminProductEditCtrl", ->
           {
             id: 9
             permalink_live: "apples"
-            variants: [
+            variants: [{
               id: 3
               price: 12
+            },
+            {
+              id: 4
+              price: 15
+            }
             ]
           }
           {

From 8248e382f34139b3cd51805c99c84058e66db784 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 3 Oct 2014 14:30:23 +1000
Subject: [PATCH 14/37] Greying out disabled action button

---
 .../stylesheets/admin/openfoodnetwork.css.scss    | 15 +++++++++++++++
 .../bulk_edit/_products_variant.html.haml         |  2 +-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/app/assets/stylesheets/admin/openfoodnetwork.css.scss b/app/assets/stylesheets/admin/openfoodnetwork.css.scss
index 641e6c5756..8518b47e50 100644
--- a/app/assets/stylesheets/admin/openfoodnetwork.css.scss
+++ b/app/assets/stylesheets/admin/openfoodnetwork.css.scss
@@ -249,3 +249,18 @@ span.required {
   color: red;
   font-size: 110%;
 }
+
+table td.actions {
+  .icon-trash, .icon-edit, icon-copy {
+    &.disabled {
+      border-color: #d0d0d0;
+      color: #c0c0c0;
+      background-color: #fafafa;
+      &:hover {
+        border-color: #a5a5a5;
+        color: #a5a5a5;
+        background-color: #fafafa;
+      }
+    }
+  }
+}
diff --git a/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml b/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml
index 28f6ed6321..8825cec46c 100644
--- a/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml
+++ b/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml
@@ -20,4 +20,4 @@
     %a{ 'ng-click' => 'editWarn(product,variant)', :class => "edit-variant icon-edit no-text", 'ng-show' => "variantSaved(variant)" }
   %td.actions
   %td.actions
-    %a{ 'ng-click' => 'deleteVariant(product,variant)', :class => "delete-variant icon-trash no-text" }
+    %a{ 'ng-click' => 'deleteVariant(product,variant)', "ng-class" => '{disabled: product.variants.length < 2}', :class => "delete-variant icon-trash no-text" }

From 447a5481a38aadbc21f7d5eebe15e17aaf772694 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 3 Oct 2014 17:56:05 +1000
Subject: [PATCH 15/37] WIP: Building migration to duplicate master variants

---
 ...060622_add_standard_variant_to_products.rb | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 db/migrate/20141003060622_add_standard_variant_to_products.rb

diff --git a/db/migrate/20141003060622_add_standard_variant_to_products.rb b/db/migrate/20141003060622_add_standard_variant_to_products.rb
new file mode 100644
index 0000000000..1463784fba
--- /dev/null
+++ b/db/migrate/20141003060622_add_standard_variant_to_products.rb
@@ -0,0 +1,49 @@
+class AddStandardVariantToProducts < ActiveRecord::Migration
+  def change
+    # Find products without any standard variants
+    products_with_only_master = Spree::Product.where( variants: [] )
+
+
+    products_with_only_master.each do |product|
+      # Run the callback to add a copy of the master variant as a standard variant
+      product.send(:ensure_standard_variant)
+
+      existing_master = product.master
+      new_variant = product.variants.first
+
+      # Replace any existing references to the master variant with the new standard variant
+
+      # Option Values
+      # Strategy: add all option values on existing_master to new_variant, and keep on existing_master
+      option_values = existing_master.option_values
+      option_values.each do |option_value|
+        variant_ids = option_value.variant_ids
+        variant_ids << new_variant.id
+        option_value.update_attributes(variant_ids: variant_ids)
+      end
+
+      # Inventory Units
+      # Strategy: completely replace all references to existing_master with new_variant
+      inventory_units = existing_master.inventory_units
+      inventory_units.each do |inventory_unit|
+        inventory_unit.update_attributes(variant_id: new_variant.id )
+      end
+
+      # Line Items
+      # Strategy: completely replace all references to existing_master with new_variant
+      line_items = existing_master.line_items
+      line_items.each do |line_item|
+        line_item.update_attributes(variant_id: new_variant.id )
+      end
+
+      # Prices
+      # Strategy: duplicate all prices on existing_master and assign them to new_variant
+      prices = existing_master.prices
+      new_prices = []
+      prices.each do |price|
+        new_prices << price.dup
+      end
+      new_variant.update_attributes(prices: new_prices)
+    end
+  end
+end

From 5d9e861ee428796b1b6881c28e697f041fd85b14 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Wed, 28 Jan 2015 17:13:56 +1100
Subject: [PATCH 16/37] Working migration to complete deprecation of master
 variants

---
 ...060622_add_standard_variant_to_products.rb | 44 +++++++++----------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/db/migrate/20141003060622_add_standard_variant_to_products.rb b/db/migrate/20141003060622_add_standard_variant_to_products.rb
index 1463784fba..035cbff4d7 100644
--- a/db/migrate/20141003060622_add_standard_variant_to_products.rb
+++ b/db/migrate/20141003060622_add_standard_variant_to_products.rb
@@ -1,8 +1,7 @@
 class AddStandardVariantToProducts < ActiveRecord::Migration
-  def change
+  def up
     # Find products without any standard variants
-    products_with_only_master = Spree::Product.where( variants: [] )
-
+    products_with_only_master = Spree::Product.find(:all, :include => "variants", :conditions => ["spree_variants.id IS NULL"])
 
     products_with_only_master.each do |product|
       # Run the callback to add a copy of the master variant as a standard variant
@@ -11,7 +10,15 @@ class AddStandardVariantToProducts < ActiveRecord::Migration
       existing_master = product.master
       new_variant = product.variants.first
 
-      # Replace any existing references to the master variant with the new standard variant
+      # Replace any relevant references to the master variant with the new standard variant
+
+      # Inventory Units
+      # Strategy: do nothing to inventory units pertaining to existing_master,
+      # new inventory units will be created with reference to new_variant
+
+      # Line Items
+      # Strategy: do nothing to line items pertaining to existing_master,
+      # new line items will be created with reference to new_variant
 
       # Option Values
       # Strategy: add all option values on existing_master to new_variant, and keep on existing_master
@@ -22,28 +29,19 @@ class AddStandardVariantToProducts < ActiveRecord::Migration
         option_value.update_attributes(variant_ids: variant_ids)
       end
 
-      # Inventory Units
-      # Strategy: completely replace all references to existing_master with new_variant
-      inventory_units = existing_master.inventory_units
-      inventory_units.each do |inventory_unit|
-        inventory_unit.update_attributes(variant_id: new_variant.id )
-      end
-
-      # Line Items
-      # Strategy: completely replace all references to existing_master with new_variant
-      line_items = existing_master.line_items
-      line_items.each do |line_item|
-        line_item.update_attributes(variant_id: new_variant.id )
-      end
-
       # Prices
       # Strategy: duplicate all prices on existing_master and assign them to new_variant
-      prices = existing_master.prices
-      new_prices = []
-      prices.each do |price|
-        new_prices << price.dup
+      existing_prices = existing_master.prices
+      existing_prices.each do |price|
+        new_variant.prices << price.dup
+      end
+
+      # Exchange Variants
+      # Strategy: Replace all references to existing master in exchanges with new_variant
+      exchange_variants = ExchangeVariant.where(variant_id: existing_master.id)
+      exchange_variants.each do |exchange_variant|
+        exchange_variant.update_attributes(variant_id: new_variant.id )
       end
-      new_variant.update_attributes(prices: new_prices)
     end
   end
 end

From 7596270154f45a03bd90e418c9adabfcb2457d27 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 29 Jan 2015 12:37:31 +1100
Subject: [PATCH 17/37] A few more changes to tidy up standard variant
 migration

---
 .../20141003060622_add_standard_variant_to_products.rb      | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/db/migrate/20141003060622_add_standard_variant_to_products.rb b/db/migrate/20141003060622_add_standard_variant_to_products.rb
index 035cbff4d7..7e7f752a5d 100644
--- a/db/migrate/20141003060622_add_standard_variant_to_products.rb
+++ b/db/migrate/20141003060622_add_standard_variant_to_products.rb
@@ -1,7 +1,7 @@
 class AddStandardVariantToProducts < ActiveRecord::Migration
   def up
     # Find products without any standard variants
-    products_with_only_master = Spree::Product.find(:all, :include => "variants", :conditions => ["spree_variants.id IS NULL"])
+    products_with_only_master = Spree::Product.includes(:variants).where('spree_variants.id IS NULL')
 
     products_with_only_master.each do |product|
       # Run the callback to add a copy of the master variant as a standard variant
@@ -39,9 +39,7 @@ class AddStandardVariantToProducts < ActiveRecord::Migration
       # Exchange Variants
       # Strategy: Replace all references to existing master in exchanges with new_variant
       exchange_variants = ExchangeVariant.where(variant_id: existing_master.id)
-      exchange_variants.each do |exchange_variant|
-        exchange_variant.update_attributes(variant_id: new_variant.id )
-      end
+      exchange_variants.update_all(variant_id: new_variant.id)
     end
   end
 end

From 2d7fb3fd6749e1e27cda29640dc4fd7c6f3f45dd Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 29 Jan 2015 12:43:40 +1100
Subject: [PATCH 18/37] Updating name of spec

---
 spec/models/spree/product_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 35dd847d1f..ddc8f99757 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -61,7 +61,7 @@ module Spree
       end
 
 
-      it "does not allow only standard variant to be deleted" do
+      it "does not allow the last variant to be deleted" do
         product = create(:simple_product)
         expect(product.variants(:reload).length).to eq 1
         product.variants = []

From 9ee25c4e424ac74cd5c852e8dcdcff7f09a66aab Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 29 Jan 2015 13:06:28 +1100
Subject: [PATCH 19/37] Making spec better

---
 spec/models/spree/product_spec.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index ddc8f99757..42c85e98e3 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -120,9 +120,10 @@ module Spree
             expect(standard_variant.price).to eq product.master.price
           end
 
-          it "only duplicates master variant on initial save" do
+          it "only duplicates master variant on create" do
+            expect(product).to_not receive :ensure_standard_variant
+            product.name = "Something else"
             product.save!
-            expect(product.variants(:reload).length).to eq 1
           end
         end
 

From b9f19d5777ed5034297dd3725c17bccffd39cf5d Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 17 Apr 2015 13:00:20 +1000
Subject: [PATCH 20/37] Fixing broken specs

---
 .../admin/variants_controller_decorator.rb    |   2 +-
 app/models/spree/product_decorator.rb         |   5 +-
 app/models/spree/product_set.rb               |   2 +-
 app/models/spree/variant_decorator.rb         |  18 +-
 app/views/shopping_shared/_contact.html.haml  |   4 +-
 spec/controllers/cart_controller_spec.rb      |   3 +-
 spec/controllers/shop_controller_spec.rb      |  15 +-
 .../spree/admin/variants_controller_spec.rb   |  10 +-
 spec/factories.rb                             |   2 +-
 .../admin/bulk_product_update_spec.rb         | 292 ++++--------------
 spec/features/admin/order_cycles_spec.rb      |  42 +--
 spec/features/admin/orders_spec.rb            |   2 +-
 spec/features/admin/products_spec.rb          |   5 +-
 spec/features/admin/reports_spec.rb           |  26 +-
 spec/features/admin/variants_spec.rb          |  27 +-
 .../consumer/shopping/shopping_spec.rb        |  65 ++--
 .../products_and_inventory_report_spec.rb     |  45 ++-
 spec/models/enterprise_spec.rb                |  33 +-
 spec/models/order_cycle_spec.rb               |  18 +-
 spec/requests/shop_spec.rb                    |  73 ++---
 20 files changed, 237 insertions(+), 452 deletions(-)

diff --git a/app/controllers/spree/admin/variants_controller_decorator.rb b/app/controllers/spree/admin/variants_controller_decorator.rb
index acb55327ae..f83bf69eca 100644
--- a/app/controllers/spree/admin/variants_controller_decorator.rb
+++ b/app/controllers/spree/admin/variants_controller_decorator.rb
@@ -4,7 +4,7 @@ Spree::Admin::VariantsController.class_eval do
   def search
     search_params = { :product_name_cont => params[:q], :sku_cont => params[:q] }
 
-    @variants = Spree::Variant.ransack(search_params.merge(:m => 'or')).result
+    @variants = Spree::Variant.where(is_master: false).ransack(search_params.merge(:m => 'or')).result
 
     if params[:distributor_id].present?
       distributor = Enterprise.find params[:distributor_id]
diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 3d35700b7c..f7834dc3ee 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -21,9 +21,7 @@ Spree::Product.class_eval do
   attr_accessible :supplier_id, :primary_taxon_id, :distributor_ids, :product_distributions_attributes, :group_buy, :group_buy_unit_size
   attr_accessible :variant_unit, :variant_unit_scale, :variant_unit_name, :unit_value, :unit_description, :notes, :images_attributes, :display_as
 
-  before_validation :ensure_standard_variant, if: :new_record?
-
-  validates_presence_of :variants, message: "Product must have at least one variant"
+  # validates_presence_of :variants, unless: :new_record?, message: "Product must have at least one variant"
   validates_presence_of :supplier
   validates :primary_taxon, presence: { message: "^Product Category can't be blank" }
   validates :tax_category_id, presence: { message: "^Tax Category can't be blank" }, if: "Spree::Config.products_require_tax_category"
@@ -34,6 +32,7 @@ Spree::Product.class_eval do
   validates_presence_of :variant_unit_name,
                         if: -> p { p.variant_unit == 'items' }
 
+  after_save :ensure_standard_variant, if: lambda { master.valid? && variants.empty? }
   after_initialize :set_available_on_to_now, :if => :new_record?
   after_save :update_units
   after_touch :touch_distributors
diff --git a/app/models/spree/product_set.rb b/app/models/spree/product_set.rb
index 9b4b771f37..1a73ab00fe 100644
--- a/app/models/spree/product_set.rb
+++ b/app/models/spree/product_set.rb
@@ -14,7 +14,7 @@ class Spree::ProductSet < ModelSet
     if e.nil?
       @klass.new(attributes).save unless @reject_if.andand.call(attributes)
     else
-      e.update_attributes(attributes.except(:id, :variants_attributes, :master_attributes)) and
+      ( attributes.except(:id, :variants_attributes, :master_attributes).present? ? e.update_attributes(attributes.except(:id, :variants_attributes, :master_attributes)) : true) and
       (attributes[:variants_attributes] ? update_variants_attributes(e, attributes[:variants_attributes]) : true ) and
       (attributes[:master_attributes] ? update_variant(e, attributes[:master_attributes]) : true )
     end
diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb
index ee05b3b2b8..20c4f20cf7 100644
--- a/app/models/spree/variant_decorator.rb
+++ b/app/models/spree/variant_decorator.rb
@@ -18,7 +18,6 @@ Spree::Variant.class_eval do
     if: -> v { v.product.andand.variant_unit.present? && v.unit_value.nil? }
 
   before_validation :update_weight_from_unit_value, if: -> v { v.product.present? }
-  before_destroy :check_product_variants
   after_save :update_units
 
   scope :with_order_cycles_inner, joins(exchanges: :order_cycle)
@@ -103,9 +102,13 @@ Spree::Variant.class_eval do
   end
 
   def delete
-    transaction do
-      self.update_column(:deleted_at, Time.now)
-      ExchangeVariant.where(variant_id: self).destroy_all
+    if product.variants == [self] # Only variant left on product
+      errors.add :product, "must have at least one variant"
+    else
+      transaction do
+        self.update_column(:deleted_at, Time.now)
+        ExchangeVariant.where(variant_id: self).destroy_all
+      end
     end
   end
 
@@ -124,11 +127,4 @@ Spree::Variant.class_eval do
       option_value_namer.name
     end
   end
-
-  def check_product_variants
-    if product.variants == [self] # Only variant left on product
-      errors.add :product, "must have at least one variant"
-      return false
-    end
-  end
 end
diff --git a/app/views/shopping_shared/_contact.html.haml b/app/views/shopping_shared/_contact.html.haml
index a5e7cddad9..6069d453a2 100644
--- a/app/views/shopping_shared/_contact.html.haml
+++ b/app/views/shopping_shared/_contact.html.haml
@@ -2,7 +2,7 @@
   .panel
     .row
       .small-12.large-4.columns
-        - if current_distributor.address.address1 || current_distributor.address.address2 || current_distributor.address.city || current_distributor.address.state || current_distributor.address.zipcode 
+        - if current_distributor.address.address1 || current_distributor.address.address2 || current_distributor.address.city || current_distributor.address.state || current_distributor.address.zipcode
           %div.center
             .header Address
             %strong=current_distributor.name
@@ -49,7 +49,7 @@
               - unless current_distributor.linkedin.blank?
                 %span
                   %a{href: "http://#{current_distributor.linkedin}", target: "_blank" }
-                    %i.ofn-i_042-linkedin 
+                    %i.ofn-i_042-linkedin
                     / = current_distributor.linkedin
 
               - unless current_distributor.instagram.blank?
diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb
index db311a8104..4fda0d7824 100644
--- a/spec/controllers/cart_controller_spec.rb
+++ b/spec/controllers/cart_controller_spec.rb
@@ -75,8 +75,9 @@ module OpenFoodNetwork
           it 'should add variant to new order and return the order' do
             product1.distributors << distributor
             product1.save
+            variant = product1.variants.first
 
-            put :add_variant, { cart_id: cart, variant_id: product1.master.id, quantity: (product1.master.on_hand-1), distributor_id: distributor, order_cycle_id: nil, max_quantity: nil }
+            put :add_variant, { cart_id: cart, variant_id: variant.id, quantity: (variant.on_hand-1), distributor_id: distributor, order_cycle_id: nil, max_quantity: nil }
 
             cart.orders.size.should == 1
             cart.orders.first.line_items.size.should == 1
diff --git a/spec/controllers/shop_controller_spec.rb b/spec/controllers/shop_controller_spec.rb
index 4b7c53da19..3fdc1a7ea9 100644
--- a/spec/controllers/shop_controller_spec.rb
+++ b/spec/controllers/shop_controller_spec.rb
@@ -112,10 +112,10 @@ describe ShopController do
         let!(:p4) { create(:product, name: "jkl", primary_taxon_id: t1.id) }
 
         before do
-          exchange.variants << p1.master
-          exchange.variants << p2.master
-          exchange.variants << p3.master
-          exchange.variants << p4.master
+          exchange.variants << p1.variants.first
+          exchange.variants << p2.variants.first
+          exchange.variants << p3.variants.first
+          exchange.variants << p4.variants.first
         end
 
         it "sorts products by the distributor's preferred taxon list" do
@@ -136,19 +136,20 @@ describe ShopController do
       context "RABL tests" do
         render_views
         let(:product) { create(:product) }
+        let(:variant) { product.variants.first }
 
         before do
-          exchange.variants << product.master
+          exchange.variants << variant
           controller.stub(:current_order_cycle).and_return order_cycle
         end
+
         it "only returns products for the current order cycle" do
           xhr :get, :products
           response.body.should have_content product.name
         end
 
         it "doesn't return products not in stock" do
-          product.update_attribute(:on_demand, false)
-          product.master.update_attribute(:count_on_hand, 0)
+          variant.update_attribute(:count_on_hand, 0)
           xhr :get, :products
           response.body.should_not have_content product.name
         end
diff --git a/spec/controllers/spree/admin/variants_controller_spec.rb b/spec/controllers/spree/admin/variants_controller_spec.rb
index 9c6d77194d..a0e7918d7c 100644
--- a/spec/controllers/spree/admin/variants_controller_spec.rb
+++ b/spec/controllers/spree/admin/variants_controller_spec.rb
@@ -8,22 +8,24 @@ module Spree
       describe "search action" do
         let!(:p1) { create(:simple_product, name: 'Product 1') }
         let!(:p2) { create(:simple_product, name: 'Product 2') }
+        let!(:v1) { p1.variants.first }
+        let!(:v2) { p2.variants.first }
         let!(:d)  { create(:distributor_enterprise) }
-        let!(:oc) { create(:simple_order_cycle, distributors: [d], variants: [p1.master]) }
+        let!(:oc) { create(:simple_order_cycle, distributors: [d], variants: [v1]) }
 
         it "filters by distributor" do
           spree_get :search, q: 'Prod', distributor_id: d.id.to_s
-          assigns(:variants).should == [p1.master]
+          assigns(:variants).should == [v1]
         end
 
         it "filters by order cycle" do
           spree_get :search, q: 'Prod', order_cycle_id: oc.id.to_s
-          assigns(:variants).should == [p1.master]
+          assigns(:variants).should == [v1]
         end
 
         it "does not filter when no distributor or order cycle is specified" do
           spree_get :search, q: 'Prod'
-          assigns(:variants).sort.should == [p1.master, p2.master].sort
+          assigns(:variants).sort.should == [v1,v2].sort
         end
       end
     end
diff --git a/spec/factories.rb b/spec/factories.rb
index 7effe49a9d..82db4c1520 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -45,7 +45,7 @@ FactoryGirl.define do
         image = File.open(File.expand_path('../../app/assets/images/logo.jpg', __FILE__))
         Spree::Image.create({:viewable_id => product.master.id, :viewable_type => 'Spree::Variant', :alt => "position 1", :attachment => image, :position => 1})
 
-        exchange.variants << product.master
+        exchange.variants << product.variants.first
       end
 
       variants = [ex1, ex2].map(&:variants).flatten
diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb
index debb234284..f5cc856da2 100644
--- a/spec/features/admin/bulk_product_update_spec.rb
+++ b/spec/features/admin/bulk_product_update_spec.rb
@@ -53,62 +53,18 @@ feature %q{
       expect(page).to have_field "available_on", with: p2.available_on.strftime("%F %T")
     end
 
-    it "displays a price input for each product without variants (ie. for master variant)" do
-      p1 = FactoryGirl.create(:product)
-      p2 = FactoryGirl.create(:product)
-      p3 = FactoryGirl.create(:product)
-      v = FactoryGirl.create(:variant, product: p3)
-
-      p1.update_attribute :price, 22.0
-      p2.update_attribute :price, 44.0
-      p3.update_attribute :price, 66.0
+    it "displays an on hand count in a span for each product" do
+      p1 = FactoryGirl.create(:product, on_hand: 15)
+      v1 = p1.variants.first
+      v1.on_hand = 4
+      v1.save!
 
       visit '/admin/products/bulk_edit'
 
-      expect(page).to have_field "price", with: "22.0"
-      expect(page).to have_field "price", with: "44.0"
-      expect(page).to have_no_field "price", with: "66.0", visible: true
-    end
-
-    it "displays an on hand count input for each product (ie. for master variant) if no regular variants exist" do
-      p1 = FactoryGirl.create(:product)
-      p2 = FactoryGirl.create(:product)
-      p1.on_hand = 15
-      p2.on_hand = 12
-      p1.save!
-      p2.save!
-
-      visit '/admin/products/bulk_edit'
-
-      expect(page).to have_no_selector "span[name='on_hand']", text: "0"
-      expect(page).to have_field "on_hand", with: "15"
-      expect(page).to have_field "on_hand", with: "12"
-    end
-
-    it "displays an on hand count in a span for each product (ie. for master variant) if other variants exist" do
-      p1 = FactoryGirl.create(:product)
-      p2 = FactoryGirl.create(:product)
-      v1 = FactoryGirl.create(:variant, product: p1, is_master: false, on_hand: 4)
-      p1.on_hand = 15
-      p2.on_hand = 12
-      p1.save!
-      p2.save!
-
-      visit '/admin/products/bulk_edit'
-
-      expect(page).to have_no_field "on_hand", with: "15"
-      expect(page).to have_selector "span[name='on_hand']", text: "4"
-      expect(page).to have_field "on_hand", with: "12"
-    end
-
-    it "displays 'on demand' for the on hand count when the product is available on demand" do
-      p1 = FactoryGirl.create(:product, on_demand: true)
-      p1.master.on_demand = true; p1.master.save!
-
-      visit '/admin/products/bulk_edit'
-
-      expect(page).to     have_selector "span[name='on_hand']", text: "On demand"
-      expect(page).to have_no_field "on_hand", visible: true
+      within "#p_#{p1.id}" do
+        expect(page).to have_no_field "on_hand", with: "15"
+        expect(page).to have_selector "span[name='on_hand']", text: "4"
+      end
     end
 
     it "displays 'on demand' for any variant that is available on demand" do
@@ -170,7 +126,7 @@ feature %q{
       visit '/admin/products/bulk_edit'
       all("a.view-variants").each { |e| e.trigger('click') }
 
-      expect(page).to have_selector "span[name='on_hand']", text: "21"
+      expect(page).to have_selector "span[name='on_hand']", text: p1.variants.sum{ |v| v.on_hand }.to_s
       expect(page).to have_field "variant_on_hand", with: "15"
       expect(page).to have_field "variant_on_hand", with: "6"
     end
@@ -218,7 +174,9 @@ feature %q{
     expect(page).to have_content 'NEW PRODUCT'
 
     fill_in 'product_name', :with => 'Big Bag Of Apples'
-    select(s.name, :from => 'product_supplier_id')
+    select s.name, :from => 'product_supplier_id'
+    select 'Weight (g)', from: 'product_variant_unit_with_scale'
+    fill_in 'product_unit_value_with_description', with: '100'
     fill_in 'product_price', :with => '10.00'
     select taxon.name, from: 'product_primary_taxon_id'
     click_button 'Create'
@@ -231,34 +189,25 @@ feature %q{
 
   scenario "creating new variants" do
     # Given a product without variants or a unit
-    p = FactoryGirl.create(:product, variant_unit: nil, variant_unit_scale: nil)
+    p = FactoryGirl.create(:product, variant_unit: 'weight', variant_unit_scale: 1000)
     login_to_admin_section
     visit '/admin/products/bulk_edit'
 
-    # I should not see an add variant button
-    expect(page).to have_no_selector 'a.add-variant', visible: true
-
-    # When I set the unit
-    select "Weight (kg)", from: "variant_unit_with_scale"
-
     # I should see an add variant button
-    expect(page).to have_selector 'a.add-variant', visible: true
+    page.find('a.view-variants').trigger('click')
 
     # When I add three variants
     page.find('a.add-variant', visible: true).trigger('click')
     page.find('a.add-variant', visible: true).trigger('click')
-    page.find('a.add-variant', visible: true).trigger('click')
 
-    # They should be added, and should see no edit buttons
-    variant_count = page.all("tr.variant").count
-    expect(variant_count).to eq 3
-    expect(page).to have_no_selector "a.edit-variant", visible: true
+    # They should be added, and should not see edit buttons for new variants
+    expect(page).to have_selector "tr.variant", count: 3
+    expect(page).to have_selector "a.edit-variant", count: 1
 
     # When I remove two, they should be removed
     page.all('a.delete-variant').first.click
     page.all('a.delete-variant').first.click
-    variant_count = page.all("tr.variant").count
-    expect(variant_count).to eq 1
+    expect(page).to have_selector "tr.variant", count: 1
 
     # When I fill out variant details and hit update
     fill_in "variant_display_name", with: "Case of 12 Bottles"
@@ -266,6 +215,7 @@ feature %q{
     fill_in "variant_display_as", with: "Case"
     fill_in "variant_price", with: "4.0"
     fill_in "variant_on_hand", with: "10"
+
     click_button 'Save Changes'
     expect(page.find("#status-message")).to have_content "Changes saved."
 
@@ -281,59 +231,6 @@ feature %q{
     expect(page).to have_selector "a.edit-variant", visible: true
   end
 
-
-  scenario "updating a product with no variants (except master)" do
-    s1 = FactoryGirl.create(:supplier_enterprise)
-    s2 = FactoryGirl.create(:supplier_enterprise)
-    t1 = FactoryGirl.create(:taxon)
-    t2 = FactoryGirl.create(:taxon)
-    p = FactoryGirl.create(:product, supplier: s1, available_on: Date.today, variant_unit: 'volume', variant_unit_scale: 1, primary_taxon: t2)
-    p.price = 10.0
-    p.on_hand = 6;
-    p.save!
-
-    login_to_admin_section
-
-    visit '/admin/products/bulk_edit'
-
-    first("div#columns_dropdown", :text => "COLUMNS").click
-    first("div#columns_dropdown div.menu div.menu_item", text: "Available On").click
-    first("div#columns_dropdown div.menu div.menu_item", text: "Category").click
-
-    within "tr#p_#{p.id}" do
-      expect(page).to have_field "product_name", with: p.name
-      expect(page).to have_select "producer_id", selected: s1.name
-      expect(page).to have_field "available_on", with: p.available_on.strftime("%F %T")
-      expect(page).to have_field "price", with: "10.0"
-      expect(page).to have_selector "div#s2id_p#{p.id}_category_id a.select2-choice"
-      expect(page).to have_select "variant_unit_with_scale", selected: "Volume (L)"
-      expect(page).to have_field "on_hand", with: "6"
-
-      fill_in "product_name", with: "Big Bag Of Potatoes"
-      select s2.name, :from => 'producer_id'
-      fill_in "available_on", with: (3.days.ago.beginning_of_day).strftime("%F %T")
-      fill_in "price", with: "20"
-      select "Weight (kg)", from: "variant_unit_with_scale"
-      select2_select t1.name, from: "p#{p.id}_category_id"
-      fill_in "on_hand", with: "18"
-      fill_in "display_as", with: "Big Bag"
-    end
-
-    click_button 'Save Changes'
-    expect(page.find("#status-message")).to have_content "Changes saved."
-
-    p.reload
-    expect(p.name).to eq "Big Bag Of Potatoes"
-    expect(p.supplier).to eq s2
-    expect(p.variant_unit).to eq "weight"
-    expect(p.variant_unit_scale).to eq 1000 # Kg
-    expect(p.available_on).to eq 3.days.ago.beginning_of_day
-    expect(p.master.display_as).to eq "Big Bag"
-    expect(p.price).to eq 20.0
-    expect(p.on_hand).to eq 18
-    expect(p.primary_taxon).to eq t1
-  end
-
   scenario "updating a product with a variant unit of 'items'" do
     p = FactoryGirl.create(:product, variant_unit: 'weight', variant_unit_scale: 1000)
 
@@ -355,74 +252,12 @@ feature %q{
     expect(p.variant_unit_name).to eq "loaf"
   end
 
-  scenario "setting a variant unit on a product that has none" do
-    p = FactoryGirl.create(:product, variant_unit: nil, variant_unit_scale: nil)
-    v = FactoryGirl.create(:variant, product: p, unit_value: nil, unit_description: nil)
-
-    login_to_admin_section
-
-    visit '/admin/products/bulk_edit'
-
-    expect(page).to have_select "variant_unit_with_scale", selected: ''
-
-    select "Weight (kg)", from: "variant_unit_with_scale"
-    first("a.view-variants").trigger('click')
-    fill_in "variant_unit_value_with_description", with: '123 abc'
-
-    click_button 'Save Changes'
-    expect(page.find("#status-message")).to have_content "Changes saved."
-
-    p.reload
-    expect(p.variant_unit).to eq "weight"
-    expect(p.variant_unit_scale).to eq 1000 # Kg
-    v.reload
-    expect(v.unit_value).to eq 123000 # 123 kg in g
-    expect(v.unit_description).to eq "abc"
-  end
-
-  describe "setting the master unit value for a product without variants" do
-    it "sets the master unit value" do
-      p = FactoryGirl.create(:product, variant_unit: nil, variant_unit_scale: nil)
-
-      login_to_admin_section
-
-      visit '/admin/products/bulk_edit'
-
-      expect(page).to have_select "variant_unit_with_scale", selected: ''
-      expect(page).to have_no_field "master_unit_value_with_description", visible: true
-
-      select "Weight (kg)", from: "variant_unit_with_scale"
-      fill_in "master_unit_value_with_description", with: '123 abc'
-
-      click_button 'Save Changes'
-      expect(page.find("#status-message")).to have_content "Changes saved."
-
-      p.reload
-      expect(p.variant_unit).to eq 'weight'
-      expect(p.variant_unit_scale).to eq 1000
-      expect(p.master.unit_value).to eq 123000
-      expect(p.master.unit_description).to eq 'abc'
-    end
-
-    it "does not show the field when the product has variants" do
-      p = FactoryGirl.create(:product, variant_unit: nil, variant_unit_scale: nil)
-      v = FactoryGirl.create(:variant, product: p, unit_value: nil, unit_description: nil)
-
-      login_to_admin_section
-
-      visit '/admin/products/bulk_edit'
-
-      select "Weight (kg)", from: "variant_unit_with_scale"
-      expect(page).to have_no_field "master_unit_value_with_description", visible: true
-    end
-  end
-
-
   scenario "updating a product with variants" do
     s1 = FactoryGirl.create(:supplier_enterprise)
     s2 = FactoryGirl.create(:supplier_enterprise)
-    p = FactoryGirl.create(:product, supplier: s1, available_on: Date.today, variant_unit: 'volume', variant_unit_scale: 0.001)
-    v = FactoryGirl.create(:variant, product: p, price: 3.0, on_hand: 9, unit_value: 0.25, unit_description: '(bottle)')
+    p = FactoryGirl.create(:product, supplier: s1, available_on: Date.today, variant_unit: 'volume', variant_unit_scale: 0.001,
+      price: 3.0, on_hand: 9, unit_value: 0.25, unit_description: '(bottle)' )
+    v = p.variants.first
 
     login_to_admin_section
 
@@ -433,7 +268,7 @@ feature %q{
     expect(page).to have_field "variant_price", with: "3.0"
     expect(page).to have_field "variant_unit_value_with_description", with: "250 (bottle)"
     expect(page).to have_field "variant_on_hand", with: "9"
-    expect(page).to have_selector "span[name='on_hand']", text: "9"
+    expect(page).to have_selector "span[name='on_hand']", "9"
 
     select "Volume (L)", from: "variant_unit_with_scale"
     fill_in "variant_price", with: "4.0"
@@ -464,7 +299,9 @@ feature %q{
 
     expect(page).to have_field "variant_price", with: "3.0"
 
-    fill_in "variant_price", with: "10.0"
+    within "#v_#{v.id}" do
+      fill_in "variant_price", with: "10.0"
+    end
 
     click_button 'Save Changes'
     expect(page.find("#status-message")).to have_content "Changes saved."
@@ -553,40 +390,39 @@ feature %q{
 
   describe "using action buttons" do
     describe "using delete buttons" do
-      it "shows a delete button for products, which deletes the appropriate product when clicked" do
-        p1 = FactoryGirl.create(:product)
-        p2 = FactoryGirl.create(:product)
-        p3 = FactoryGirl.create(:product)
-        login_to_admin_section
+      let!(:p1) { FactoryGirl.create(:product) }
+      let!(:p2) { FactoryGirl.create(:product) }
+      let!(:v1) { p1.variants.first }
+      let!(:v2) { p2.variants.first }
+      let!(:v3) { FactoryGirl.create(:variant, product: p2 ) }
 
+
+      before do
+        quick_login_as_admin
         visit '/admin/products/bulk_edit'
+      end
 
-        expect(page).to have_selector "a.delete-product", :count => 3
+      it "shows a delete button for products, which deletes the appropriate product when clicked" do
+        expect(page).to have_selector "a.delete-product", :count => 2
 
         within "tr#p_#{p1.id}" do
           first("a.delete-product").click
         end
 
-        expect(page).to have_selector "a.delete-product", :count => 2
+        expect(page).to have_selector "a.delete-product", :count => 1
 
         visit '/admin/products/bulk_edit'
 
-        expect(page).to have_selector "a.delete-product", :count => 2
+        expect(page).to have_selector "a.delete-product", :count => 1
       end
 
       it "shows a delete button for variants, which deletes the appropriate variant when clicked" do
-        v1 = FactoryGirl.create(:variant)
-        v2 = FactoryGirl.create(:variant)
-        v3 = FactoryGirl.create(:variant)
-        login_to_admin_section
-
-        visit '/admin/products/bulk_edit'
         expect(page).to have_selector "a.view-variants"
         all("a.view-variants").each { |e| e.trigger('click') }
 
         expect(page).to have_selector "a.delete-variant", :count => 3
 
-        within "tr#v_#{v1.id}" do
+        within "tr#v_#{v3.id}" do
           first("a.delete-variant").click
         end
 
@@ -601,15 +437,18 @@ feature %q{
     end
 
     describe "using edit buttons" do
-      it "shows an edit button for products, which takes the user to the standard edit page for that product" do
-        p1 = FactoryGirl.create(:product)
-        p2 = FactoryGirl.create(:product)
-        p3 = FactoryGirl.create(:product)
-        login_to_admin_section
+      let!(:p1) { FactoryGirl.create(:product) }
+      let!(:p2) { FactoryGirl.create(:product) }
+      let!(:v1) { p1.variants.first }
+      let!(:v2) { p2.variants.first }
 
+      before do
+        quick_login_as_admin
         visit '/admin/products/bulk_edit'
+      end
 
-        expect(page).to have_selector "a.edit-product", :count => 3
+      it "shows an edit button for products, which takes the user to the standard edit page for that product" do
+        expect(page).to have_selector "a.edit-product", :count => 2
 
         within "tr#p_#{p1.id}" do
           first("a.edit-product").click
@@ -619,16 +458,10 @@ feature %q{
       end
 
       it "shows an edit button for variants, which takes the user to the standard edit page for that variant" do
-        v1 = FactoryGirl.create(:variant)
-        v2 = FactoryGirl.create(:variant)
-        v3 = FactoryGirl.create(:variant)
-        login_to_admin_section
-
-        visit '/admin/products/bulk_edit'
         expect(page).to have_selector "a.view-variants"
         all("a.view-variants").each { |e| e.trigger('click') }
 
-        expect(page).to have_selector "a.edit-variant", :count => 3
+        expect(page).to have_selector "a.edit-variant", :count => 2
 
         within "tr#v_#{v1.id}" do
           first("a.edit-variant").click
@@ -788,6 +621,8 @@ feature %q{
       within 'fieldset#new_product' do
         fill_in 'product_name', with: 'Big Bag Of Apples'
         select supplier_permitted.name, from: 'product_supplier_id'
+        select 'Weight (g)', from: 'product_variant_unit_with_scale'
+        fill_in 'product_unit_value_with_description', with: '100'
         fill_in 'product_price', with: '10.00'
         select taxon.name, from: 'product_primary_taxon_id'
       end
@@ -800,6 +635,7 @@ feature %q{
 
     it "allows me to update a product" do
       p = product_supplied_permitted
+      v = p.variants.first
 
       visit '/admin/products/bulk_edit'
       first("div#columns_dropdown", :text => "COLUMNS").click
@@ -809,30 +645,34 @@ feature %q{
         expect(page).to have_field "product_name", with: p.name
         expect(page).to have_select "producer_id", selected: supplier_permitted.name
         expect(page).to have_field "available_on", with: p.available_on.strftime("%F %T")
-        expect(page).to have_field "price", with: "10.0"
-        expect(page).to have_field "on_hand", with: "6"
 
         fill_in "product_name", with: "Big Bag Of Potatoes"
         select supplier_managed2.name, :from => 'producer_id'
         fill_in "available_on", with: (3.days.ago.beginning_of_day).strftime("%F %T")
-        fill_in "price", with: "20"
         select "Weight (kg)", from: "variant_unit_with_scale"
-        fill_in "on_hand", with: "18"
-        fill_in "display_as", with: "Big Bag"
+
+        find("a.view-variants").trigger('click')
+      end
+
+      within "#v_#{v.id}" do
+        fill_in "variant_price", with: "20"
+        fill_in "variant_on_hand", with: "18"
+        fill_in "variant_display_as", with: "Big Bag"
       end
 
       click_button 'Save Changes'
       expect(page.find("#status-message")).to have_content "Changes saved."
 
       p.reload
+      v.reload
       expect(p.name).to eq "Big Bag Of Potatoes"
       expect(p.supplier).to eq supplier_managed2
       expect(p.variant_unit).to eq "weight"
       expect(p.variant_unit_scale).to eq 1000 # Kg
       expect(p.available_on).to eq 3.days.ago.beginning_of_day
-      expect(p.master.display_as).to eq "Big Bag"
-      expect(p.price).to eq 20.0
-      expect(p.on_hand).to eq 18
+      expect(v.display_as).to eq "Big Bag"
+      expect(v.price).to eq 20.0
+      expect(v.on_hand).to eq 18
     end
   end
 end
diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb
index cb96799fbb..bbf9fb7b36 100644
--- a/spec/features/admin/order_cycles_spec.rb
+++ b/spec/features/admin/order_cycles_spec.rb
@@ -487,7 +487,9 @@ feature %q{
     let!(:shipping_method) { create(:shipping_method, distributors: [distributor_managed, distributor_unmanaged, distributor_permitted]) }
     let!(:payment_method) { create(:payment_method, distributors: [distributor_managed, distributor_unmanaged, distributor_permitted]) }
     let!(:product_managed) { create(:product, supplier: supplier_managed) }
+    let!(:variant_managed) { product_managed.variants.first }
     let!(:product_permitted) { create(:product, supplier: supplier_permitted) }
+    let!(:variant_permitted) { product_permitted.variants.first }
 
     before do
       # Relationships required for interface to work
@@ -533,8 +535,6 @@ feature %q{
         click_link "Order Cycles"
         click_link 'New Order Cycle'
 
-        # We go straight through to the new form, because only one coordinator is available
-
         fill_in 'order_cycle_name', with: 'My order cycle'
         fill_in 'order_cycle_orders_open_at', with: '2040-11-06 06:00:00'
         fill_in 'order_cycle_orders_close_at', with: '2040-11-13 17:00:00'
@@ -544,8 +544,8 @@ feature %q{
         select 'Permitted supplier', from: 'new_supplier_id'
         click_button 'Add supplier'
 
-        select_incoming_variant supplier_managed, 0, product_managed.master
-        select_incoming_variant supplier_permitted, 1, product_permitted.master
+        select_incoming_variant supplier_managed, 0, variant_managed
+        select_incoming_variant supplier_permitted, 1, variant_permitted
 
         click_button 'Add coordinator fee'
         select 'Managed distributor fee', from: 'order_cycle_coordinator_fee_0_id'
@@ -750,7 +750,9 @@ feature %q{
     let!(:p1) { create(:simple_product, supplier: enterprise) }
     let!(:p2) { create(:simple_product, supplier: enterprise) }
     let!(:p3) { create(:simple_product, supplier: enterprise) }
-    let!(:v) { create(:variant, product: p3) }
+    let!(:v1) { p1.variants.first }
+    let!(:v2) { p2.variants.first }
+    let!(:v3) { p3.variants.first }
     let!(:fee) { create(:enterprise_fee, enterprise: enterprise, name: 'Coord fee') }
 
     before do
@@ -779,12 +781,12 @@ feature %q{
       fill_in 'order_cycle_outgoing_exchange_0_pickup_instructions', with: 'pickup instructions'
 
       # Then my products / variants should already be selected
-      page.should have_checked_field "order_cycle_incoming_exchange_0_variants_#{p1.master.id}"
-      page.should have_checked_field "order_cycle_incoming_exchange_0_variants_#{p2.master.id}"
-      page.should have_checked_field "order_cycle_incoming_exchange_0_variants_#{v.id}"
+      page.should have_checked_field "order_cycle_incoming_exchange_0_variants_#{v1.id}"
+      page.should have_checked_field "order_cycle_incoming_exchange_0_variants_#{v2.id}"
+      page.should have_checked_field "order_cycle_incoming_exchange_0_variants_#{v3.id}"
 
       # When I unselect a product
-      uncheck "order_cycle_incoming_exchange_0_variants_#{p2.master.id}"
+      uncheck "order_cycle_incoming_exchange_0_variants_#{v2.id}"
 
       # And I add a fee and save
       click_button 'Add coordinator fee'
@@ -819,7 +821,7 @@ feature %q{
     scenario "editing an order cycle" do
       # Given an order cycle with pickup time and instructions
       fee = create(:enterprise_fee, name: 'my fee', enterprise: enterprise)
-      oc = create(:simple_order_cycle, suppliers: [enterprise], coordinator: enterprise, distributors: [enterprise], variants: [p1.master], coordinator_fees: [fee])
+      oc = create(:simple_order_cycle, suppliers: [enterprise], coordinator: enterprise, distributors: [enterprise], variants: [v1], coordinator_fees: [fee])
       ex = oc.exchanges.outgoing.first
       ex.update_attributes! pickup_time: 'pickup time', pickup_instructions: 'pickup instructions'
 
@@ -837,9 +839,9 @@ feature %q{
       page.should have_field 'order_cycle_outgoing_exchange_0_pickup_instructions', with: 'pickup instructions'
 
       # And I should see the products
-      page.should have_checked_field   "order_cycle_incoming_exchange_0_variants_#{p1.master.id}"
-      page.should have_unchecked_field "order_cycle_incoming_exchange_0_variants_#{p2.master.id}"
-      page.should have_unchecked_field "order_cycle_incoming_exchange_0_variants_#{v.id}"
+      page.should have_checked_field   "order_cycle_incoming_exchange_0_variants_#{v1.id}"
+      page.should have_unchecked_field "order_cycle_incoming_exchange_0_variants_#{v2.id}"
+      page.should have_unchecked_field "order_cycle_incoming_exchange_0_variants_#{v3.id}"
 
       # And I should see the coordinator fees
       page.should have_select 'order_cycle_coordinator_fee_0_id', selected: 'my fee'
@@ -849,7 +851,7 @@ feature %q{
       # Given an order cycle with pickup time and instructions
       fee1 = create(:enterprise_fee, name: 'my fee', enterprise: enterprise)
       fee2 = create(:enterprise_fee, name: 'that fee', enterprise: enterprise)
-      oc = create(:simple_order_cycle, suppliers: [enterprise], coordinator: enterprise, distributors: [enterprise], variants: [p1.master], coordinator_fees: [fee1])
+      oc = create(:simple_order_cycle, suppliers: [enterprise], coordinator: enterprise, distributors: [enterprise], variants: [v1], coordinator_fees: [fee1])
       ex = oc.exchanges.outgoing.first
       ex.update_attributes! pickup_time: 'pickup time', pickup_instructions: 'pickup instructions'
 
@@ -866,10 +868,10 @@ feature %q{
       fill_in 'order_cycle_outgoing_exchange_0_pickup_instructions', with: 'zzy'
 
       # And I make some product selections
-      uncheck "order_cycle_incoming_exchange_0_variants_#{p1.master.id}"
-      check   "order_cycle_incoming_exchange_0_variants_#{p2.master.id}"
-      check   "order_cycle_incoming_exchange_0_variants_#{v.id}"
-      uncheck "order_cycle_incoming_exchange_0_variants_#{v.id}"
+      uncheck "order_cycle_incoming_exchange_0_variants_#{v1.id}"
+      check   "order_cycle_incoming_exchange_0_variants_#{v2.id}"
+      check   "order_cycle_incoming_exchange_0_variants_#{v3.id}"
+      uncheck "order_cycle_incoming_exchange_0_variants_#{v3.id}"
 
       # And I select some fees and update
       click_link 'order_cycle_coordinator_fee_0_remove'
@@ -887,8 +889,8 @@ feature %q{
 
       # And it should have a variant selected
       oc = OrderCycle.last
-      oc.exchanges.incoming.first.variants.should == [p2.master]
-      oc.exchanges.outgoing.first.variants.should == [p2.master]
+      oc.exchanges.incoming.first.variants.should == [v2]
+      oc.exchanges.outgoing.first.variants.should == [v2]
 
       # And it should have the fee
       oc.coordinator_fees.should == [fee2]
diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb
index bb38e5d8ba..c2a302fcbb 100644
--- a/spec/features/admin/orders_spec.rb
+++ b/spec/features/admin/orders_spec.rb
@@ -11,7 +11,7 @@ feature %q{
     @user = create(:user)
     @product = create(:simple_product)
     @distributor = create(:distributor_enterprise)
-    @order_cycle = create(:simple_order_cycle, distributors: [@distributor], variants: [@product.master])
+    @order_cycle = create(:simple_order_cycle, distributors: [@distributor], variants: [@product.variants.first])
 
     @order = create(:order_with_totals_and_distribution, user: @user, distributor: @distributor, order_cycle: @order_cycle, state: 'complete', payment_state: 'balance_due')
 
diff --git a/spec/features/admin/products_spec.rb b/spec/features/admin/products_spec.rb
index 740de12045..269c77d801 100644
--- a/spec/features/admin/products_spec.rb
+++ b/spec/features/admin/products_spec.rb
@@ -117,9 +117,8 @@ feature %q{
       end
     end
 
-    scenario "creating a new product" do
+    scenario "creating a new product", js: true do
       Spree::Config.products_require_tax_category = false
-
       click_link 'Products'
       click_link 'New Product'
 
@@ -128,6 +127,8 @@ feature %q{
 
       page.should have_selector('#product_supplier_id')
       select 'Another Supplier', :from => 'product_supplier_id'
+      select 'Weight (g)', from: 'product_variant_unit_with_scale'
+      fill_in 'product_unit_value_with_description', with: '500'
       select taxon.name, from: "product_primary_taxon_id"
       select 'None', from: "product_tax_category_id"
 
diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb
index f16b736c00..277f33593b 100644
--- a/spec/features/admin/reports_spec.rb
+++ b/spec/features/admin/reports_spec.rb
@@ -223,14 +223,16 @@ feature %q{
 
   describe "products and inventory report" do
     it "shows products and inventory report" do
-      product_1 = create(:simple_product, name: "Product Name", variant_unit: nil)
-      variant_1 = create(:variant, product: product_1, price: 100.0)
-      variant_2 = create(:variant, product: product_1, price: 80.0)
-      product_2 = create(:simple_product, name: "Product 2", price: 99.0, variant_unit: nil)
-      variant_1.update_column(:count_on_hand, 10)
-      variant_2.update_column(:count_on_hand, 20)
-      product_2.master.update_column(:count_on_hand, 9)
-      variant_1.option_values = [create(:option_value, :presentation => "Test")]
+      product1 = create(:simple_product, name: "Product Name", price: 100)
+      variant1 = product1.variants.first
+      variant2 = create(:variant, product: product1, price: 80.0)
+      product2 = create(:simple_product, name: "Product 2", price: 99.0, variant_unit: 'weight', variant_unit_scale: 1, unit_value: '100')
+      variant3 = product2.variants.first
+      variant1.update_column(:count_on_hand, 10)
+      variant2.update_column(:count_on_hand, 20)
+      variant3.update_column(:count_on_hand, 9)
+      variant1.option_values = [create(:option_value, :presentation => "Test")]
+      variant2.option_values = [create(:option_value, :presentation => "Something")]
 
       login_to_admin_section
       click_link 'Reports'
@@ -244,10 +246,10 @@ feature %q{
       table = rows.map { |r| r.all("th,td").map { |c| c.text.strip } }
 
       table.sort.should == [
-        ["Supplier",              "Producer Suburb",               "Product",      "Product Properties",            "Taxons",                    "Variant Value", "Price", "Group Buy Unit Quantity",     "Amount"],
-        [product_1.supplier.name, product_1.supplier.address.city, "Product Name", product_1.properties.join(", "), product_1.primary_taxon.name, "Test",     "100.0", product_1.group_buy_unit_size.to_s, ""],
-        [product_1.supplier.name, product_1.supplier.address.city, "Product Name", product_1.properties.join(", "), product_1.primary_taxon.name, "S",        "80.0", product_1.group_buy_unit_size.to_s, ""],
-        [product_2.supplier.name, product_1.supplier.address.city, "Product 2",    product_1.properties.join(", "), product_2.primary_taxon.name, "",         "99.0", product_1.group_buy_unit_size.to_s, ""]
+        ["Supplier",              "Producer Suburb",               "Product",      "Product Properties",            "Taxons",                   "Variant Value",  "Price",  "Group Buy Unit Quantity",     "Amount"],
+        [product1.supplier.name, product1.supplier.address.city, "Product Name", product1.properties.join(", "), product1.primary_taxon.name,  "Test",           "100.0",  product1.group_buy_unit_size.to_s, ""],
+        [product1.supplier.name, product1.supplier.address.city, "Product Name", product1.properties.join(", "), product1.primary_taxon.name,   "Something",       "80.0",   product1.group_buy_unit_size.to_s, ""],
+        [product2.supplier.name, product1.supplier.address.city, "Product 2",    product1.properties.join(", "), product2.primary_taxon.name,   "100g",           "99.0",   product1.group_buy_unit_size.to_s, ""]
       ].sort
     end
   end
diff --git a/spec/features/admin/variants_spec.rb b/spec/features/admin/variants_spec.rb
index 19767d8ea2..c947872949 100644
--- a/spec/features/admin/variants_spec.rb
+++ b/spec/features/admin/variants_spec.rb
@@ -25,10 +25,11 @@ feature %q{
   end
 
 
-  scenario "editing unit value and description for a variant" do
+  scenario "editing unit value and description for a variant", js:true do
     # Given a product with unit-related option types, with a variant
     p = create(:simple_product, variant_unit: "weight", variant_unit_scale: "1")
-    v = create(:variant, product: p, unit_value: 1, unit_description: 'foo')
+    v = p.variants.first
+    v.update_attributes( unit_value: 1, unit_description: 'foo' )
 
     # And the product has option types for the unit-related and non-unit-related option values
     p.option_types << v.option_values.first.option_type
@@ -39,7 +40,7 @@ feature %q{
     page.find('table.index .icon-edit').click
 
     # Then I should not see a traditional option value field for the unit-related option value
-    page.all("div[data-hook='presentation'] input").count.should == 1
+    expect(page).to_not have_selector "div[data-hook='presentation'] input"
 
     # And I should see unit value and description fields for the unit-related option value
     page.should have_field "variant_unit_value", with: "1"
@@ -57,27 +58,9 @@ feature %q{
     v.unit_description.should == 'bar'
   end
 
-  it "does not show unit value or description fields when the product does not have a unit-related option type" do
-    # Given a product without unit-related option types, with a variant
-    p = create(:simple_product, variant_unit: nil, variant_unit_scale: nil)
-    v = create(:variant, product: p, unit_value: nil, unit_description: nil)
-
-    # And the product has option types for the variant's option values
-    p.option_types << v.option_values.first.option_type
-
-    # When I view the variant
-    login_to_admin_section
-    visit spree.admin_product_variants_path p
-    page.find('table.index .icon-edit').click
-
-    # Then I should not see unit value and description fields
-    page.should_not have_field "variant_unit_value"
-    page.should_not have_field "variant_unit_description"
-  end
-
   it "soft-deletes variants", js: true do
     p = create(:simple_product)
-    v = create(:variant, product: p)
+    v = p.variants.first
 
     login_to_admin_section
     visit spree.admin_product_variants_path p
diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb
index a47b73ba6c..e11664c457 100644
--- a/spec/features/consumer/shopping/shopping_spec.rb
+++ b/spec/features/consumer/shopping/shopping_spec.rb
@@ -13,9 +13,10 @@ feature "As a consumer I want to shop with a distributor", js: true do
     let(:oc1) { create(:simple_order_cycle, distributors: [distributor], coordinator: create(:distributor_enterprise), orders_close_at: 2.days.from_now) }
     let(:oc2) { create(:simple_order_cycle, distributors: [distributor], coordinator: create(:distributor_enterprise), orders_close_at: 3.days.from_now) }
     let(:product) { create(:simple_product, supplier: supplier) }
+    let(:variant) { product.variants.first }
     let(:order) { create(:order, distributor: distributor) }
 
-    before do 
+    before do
       set_order order
     end
 
@@ -28,23 +29,23 @@ feature "As a consumer I want to shop with a distributor", js: true do
       visit shop_path
       page.should have_text distributor.name
       find("#tab_about a").click
-      first("distributor img")['src'].should == distributor.logo.url(:thumb) 
+      first("distributor img")['src'].should == distributor.logo.url(:thumb)
     end
 
     it "shows the producers for a distributor" do
-      exchange = Exchange.find(oc1.exchanges.to_enterprises(distributor).outgoing.first.id) 
-      exchange.variants << product.master
+      exchange = Exchange.find(oc1.exchanges.to_enterprises(distributor).outgoing.first.id)
+      exchange.variants << variant
 
       visit shop_path
       find("#tab_producers a").click
-      page.should have_content supplier.name 
+      page.should have_content supplier.name
     end
 
     describe "selecting an order cycle" do
       let(:exchange1) { Exchange.find(oc1.exchanges.to_enterprises(distributor).outgoing.first.id) }
 
       it "selects an order cycle if only one is open" do
-        exchange1.update_attribute :pickup_time, "turtles" 
+        exchange1.update_attribute :pickup_time, "turtles"
         visit shop_path
         page.should have_selector "option[selected]", text: 'turtles'
       end
@@ -52,8 +53,8 @@ feature "As a consumer I want to shop with a distributor", js: true do
       describe "with multiple order cycles" do
         let(:exchange2) { Exchange.find(oc2.exchanges.to_enterprises(distributor).outgoing.first.id) }
         before do
-          exchange1.update_attribute :pickup_time, "frogs" 
-          exchange2.update_attribute :pickup_time, "turtles" 
+          exchange1.update_attribute :pickup_time, "frogs"
+          exchange2.update_attribute :pickup_time, "turtles"
         end
 
         it "shows a select with all order cycles, but doesn't show the products by default" do
@@ -62,22 +63,22 @@ feature "As a consumer I want to shop with a distributor", js: true do
           page.should have_selector "option", text: 'turtles'
           page.should_not have_selector("input.button.right", visible: true)
         end
-        
+
         it "shows products after selecting an order cycle" do
-          product.master.update_attribute(:display_name, "kitten")
-          product.master.update_attribute(:display_as, "rabbit")
-          exchange1.variants << product.master ## add product to exchange
+          variant.update_attribute(:display_name, "kitten")
+          variant.update_attribute(:display_as, "rabbit")
+          exchange1.variants << variant ## add product to exchange
           visit shop_path
-          page.should_not have_content product.name 
+          page.should_not have_content product.name
           Spree::Order.last.order_cycle.should == nil
 
           select "frogs", :from => "order_cycle_id"
           page.should have_selector "products"
-          page.should have_content "Next order closing in 2 days" 
+          page.should have_content "Next order closing in 2 days"
           Spree::Order.last.order_cycle.should == oc1
-          page.should have_content product.name 
-          page.should have_content product.master.display_name
-          page.should have_content product.master.display_as
+          page.should have_content product.name
+          page.should have_content variant.display_name
+          page.should have_content variant.display_as
 
           open_product_modal product
           modal_should_be_open_for product
@@ -88,21 +89,16 @@ feature "As a consumer I want to shop with a distributor", js: true do
     describe "after selecting an order cycle with products visible" do
       let(:variant1) { create(:variant, product: product, price: 20) }
       let(:variant2) { create(:variant, product: product, price: 30) }
-      let(:exchange) { Exchange.find(oc1.exchanges.to_enterprises(distributor).outgoing.first.id) } 
+      let(:exchange) { Exchange.find(oc1.exchanges.to_enterprises(distributor).outgoing.first.id) }
 
       before do
-        exchange.update_attribute :pickup_time, "frogs" 
-        exchange.variants << product.master
+        exchange.update_attribute :pickup_time, "frogs"
+        exchange.variants << variant
         exchange.variants << variant1
         exchange.variants << variant2
         order.order_cycle = oc1
       end
 
-      it "should not show quantity field for product with variants" do
-        visit shop_path
-        page.should_not have_selector("#variants_#{product.master.id}", visible: true)
-      end
-
       it "uses the adjusted price" do
         enterprise_fee1 = create(:enterprise_fee, amount: 20)
         enterprise_fee2 = create(:enterprise_fee, amount:  3)
@@ -126,6 +122,7 @@ feature "As a consumer I want to shop with a distributor", js: true do
     describe "group buy products" do
       let(:exchange) { Exchange.find(oc1.exchanges.to_enterprises(distributor).outgoing.first.id) }
       let(:product) { create(:simple_product, group_buy: true, on_hand: 15) }
+      let(:variant) { product.variants.first }
       let(:product2) { create(:simple_product, group_buy: false) }
 
       describe "without variants" do
@@ -134,22 +131,11 @@ feature "As a consumer I want to shop with a distributor", js: true do
           set_order_cycle(order, oc1)
           visit shop_path
         end
-        
-        it "should save group buy data to ze cart" do
-          fill_in "variants[#{product.master.id}]", with: 5
-          fill_in "variant_attributes[#{product.master.id}][max_quantity]", with: 9
 
-          wait_until { !cart_dirty }
-
-          li = Spree::Order.order(:created_at).last.line_items.order(:created_at).last
-          li.max_quantity.should == 9
-          li.quantity.should == 5
-        end
-      
         # TODO move to controller test
         pending "adding a product with a max quantity less than quantity results in max_quantity==quantity" do
-          fill_in "variants[#{product.master.id}]", with: 5
-          fill_in "variant_attributes[#{product.master.id}][max_quantity]", with: 1
+          fill_in "variants[#{variant.id}]", with: 5
+          fill_in "variant_attributes[#{variant.id}][max_quantity]", with: 1
           add_to_cart
           page.should have_content product.name
           li = Spree::Order.order(:created_at).last.line_items.order(:created_at).last
@@ -165,7 +151,7 @@ feature "As a consumer I want to shop with a distributor", js: true do
           set_order_cycle(order, oc1)
           visit shop_path
         end
-        
+
         it "should save group buy data to the cart" do
           fill_in "variants[#{variant.id}]", with: 6
           fill_in "variant_attributes[#{variant.id}][max_quantity]", with: 7
@@ -213,4 +199,3 @@ feature "As a consumer I want to shop with a distributor", js: true do
     end
   end
 end
-
diff --git a/spec/lib/open_food_network/products_and_inventory_report_spec.rb b/spec/lib/open_food_network/products_and_inventory_report_spec.rb
index 2e23919fe6..3cde6e5f8e 100644
--- a/spec/lib/open_food_network/products_and_inventory_report_spec.rb
+++ b/spec/lib/open_food_network/products_and_inventory_report_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 module OpenFoodNetwork
   describe ProductsAndInventoryReport do
     context "As a site admin" do
-      let(:user) do 
+      let(:user) do
         user = create(:user)
         user.spree_roles << Spree::Role.find_or_create_by_name!("admin")
         user
@@ -14,7 +14,7 @@ module OpenFoodNetwork
 
       it "Should return headers" do
         subject.header.should == [
-          "Supplier", 
+          "Supplier",
           "Producer Suburb",
           "Product",
           "Product Properties",
@@ -63,7 +63,7 @@ module OpenFoodNetwork
 
     context "As an enterprise user" do
       let(:supplier) { create(:supplier_enterprise) }
-      let(:enterprise_user) do 
+      let(:enterprise_user) do
         user = create(:user)
         user.enterprise_roles.create(enterprise: supplier)
         user.spree_roles = []
@@ -76,7 +76,7 @@ module OpenFoodNetwork
       describe "fetching child variants" do
         it "returns some variants" do
           product1 = create(:simple_product, supplier: supplier)
-          variant_1 = create(:variant, product: product1)
+          variant_1 = product1.variants.first
           variant_2 = create(:variant, product: product1)
 
           subject.child_variants.sort.should == [variant_1, variant_2].sort
@@ -85,41 +85,34 @@ module OpenFoodNetwork
         it "should only return variants managed by the user" do
           product1 = create(:simple_product, supplier: create(:supplier_enterprise))
           product2 = create(:simple_product, supplier: supplier)
-          variant_1 = create(:variant, product: product1)
-          variant_2 = create(:variant, product: product2)
-          
+          variant_1 = product1.variants.first
+          variant_2 = product2.variants.first
+
           subject.child_variants.should == [variant_2]
         end
       end
 
       describe "fetching master variants" do
-        it "should only return variants managed by the user" do
-          product1 = create(:simple_product, supplier: create(:supplier_enterprise))
-          product2 = create(:simple_product, supplier: supplier)
-          
-          subject.master_variants.should == [product2.master]
-        end
-
         it "doesn't return master variants with siblings" do
           product = create(:simple_product, supplier: supplier)
-          create(:variant, product: product)  
-          
-          subject.master_variants.should be_empty 
+
+          subject.master_variants.should be_empty
         end
       end
 
       describe "Filtering variants" do
-        let(:variants) { Spree::Variant.scoped.joins(:product) }
+        let(:variants) { Spree::Variant.scoped.joins(:product).where(is_master: false) }
         it "should return unfiltered variants sans-params" do
           product1 = create(:simple_product, supplier: supplier)
           product2 = create(:simple_product, supplier: supplier)
-          subject.filter(Spree::Variant.scoped).sort.should == [product1.master, product2.master].sort
+
+          subject.filter(Spree::Variant.scoped).sort.should == [product1.master, product1.variants.first, product2.master, product2.variants.first].sort
         end
         it "should filter deleted products" do
           product1 = create(:simple_product, supplier: supplier)
           product2 = create(:simple_product, supplier: supplier)
           product2.delete
-          subject.filter(Spree::Variant.scoped).sort.should == [product1.master].sort
+          subject.filter(Spree::Variant.scoped).sort.should == [product1.master, product1.variants.first].sort
         end
         describe "based on report type" do
           it "returns only variants on hand" do
@@ -127,7 +120,7 @@ module OpenFoodNetwork
             product2 = create(:simple_product, supplier: supplier, on_hand: 0)
 
             subject.stub(:params).and_return(report_type: 'inventory')
-            subject.filter(variants).should == [product1.master]
+            subject.filter(variants).should == [product1.variants.first]
           end
         end
         it "filters to a specific supplier" do
@@ -136,7 +129,7 @@ module OpenFoodNetwork
           product2 = create(:simple_product, supplier: supplier2)
 
           subject.stub(:params).and_return(supplier_id: supplier.id)
-          subject.filter(variants).should == [product1.master]
+          subject.filter(variants).should == [product1.variants.first]
         end
         it "filters to a specific distributor" do
           distributor = create(:distributor_enterprise)
@@ -144,23 +137,23 @@ module OpenFoodNetwork
           product2 = create(:simple_product, supplier: supplier, distributors: [distributor])
 
           subject.stub(:params).and_return(distributor_id: distributor.id)
-          subject.filter(variants).should == [product2.master]
+          subject.filter(variants).should == [product2.variants.first]
         end
         it "filters to a specific order cycle" do
           distributor = create(:distributor_enterprise)
           product1 = create(:simple_product, supplier: supplier, distributors: [distributor])
           product2 = create(:simple_product, supplier: supplier, distributors: [distributor])
-          order_cycle = create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], variants: [product1.master])
+          order_cycle = create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], variants: [product1.variants.first])
 
           subject.stub(:params).and_return(order_cycle_id: order_cycle.id)
-          subject.filter(variants).should == [product1.master]
+          subject.filter(variants).should == [product1.variants.first]
         end
 
         it "should do all the filters at once" do
           distributor = create(:distributor_enterprise)
           product1 = create(:simple_product, supplier: supplier, distributors: [distributor])
           product2 = create(:simple_product, supplier: supplier, distributors: [distributor])
-          order_cycle = create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], variants: [product1.master])
+          order_cycle = create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], variants: [product1.variants.first])
 
           subject.stub(:params).and_return(
             order_cycle_id: order_cycle.id,
diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb
index 97a4b988d8..47a64c06a9 100644
--- a/spec/models/enterprise_spec.rb
+++ b/spec/models/enterprise_spec.rb
@@ -693,45 +693,38 @@ describe Enterprise do
   end
 
   describe "finding variants distributed by the enterprise" do
-    it "finds the master variant" do
+    it "finds master and other variants" do
       d = create(:distributor_enterprise)
       p = create(:product, distributors: [d])
-      d.distributed_variants.should == [p.master]
-    end
-
-    it "finds other variants" do
-      d = create(:distributor_enterprise)
-      p = create(:product, distributors: [d])
-      v = create(:variant, product: p)
+      v = p.variants.first
       d.distributed_variants.sort.should == [p.master, v].sort
     end
 
-    it "finds variants distributed by order cycle" do
+    pending "finds variants distributed by order cycle" do
+      # there isn't actually a method for this on Enterprise?
       d = create(:distributor_enterprise)
       p = create(:product)
-      oc = create(:simple_order_cycle, distributors: [d], variants: [p.master])
-      d.distributed_variants.should == [p.master]
+      v = p.variants.first
+      oc = create(:simple_order_cycle, distributors: [d], variants: [v])
+
+      # This method doesn't do what this test says it does...
+      d.distributed_variants.sort.should == [v]
     end
   end
 
   describe "finding variants distributed by the enterprise in a product distribution only" do
-    it "finds the master variant" do
+    it "finds master and other variants" do
       d = create(:distributor_enterprise)
       p = create(:product, distributors: [d])
-      d.product_distribution_variants.should == [p.master]
-    end
-
-    it "finds other variants" do
-      d = create(:distributor_enterprise)
-      p = create(:product, distributors: [d])
-      v = create(:variant, product: p)
+      v = p.variants.first
       d.product_distribution_variants.sort.should == [p.master, v].sort
     end
 
     it "does not find variants distributed by order cycle" do
       d = create(:distributor_enterprise)
       p = create(:product)
-      oc = create(:simple_order_cycle, distributors: [d], variants: [p.master])
+      v = p.variants.first
+      oc = create(:simple_order_cycle, distributors: [d], variants: [v])
       d.product_distribution_variants.should == []
     end
   end
diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb
index 817d2e8e93..5f95a09a8f 100644
--- a/spec/models/order_cycle_spec.rb
+++ b/spec/models/order_cycle_spec.rb
@@ -239,10 +239,12 @@ describe OrderCycle do
     it "returns valid products but not invalid products" do
       p_valid = create(:product)
       p_invalid = create(:product)
-      v = create(:variant, product: p_invalid)
+      v_valid = p_valid.variants.first
+      v_invalid = p_invalid.variants.first
 
       d = create(:distributor_enterprise)
-      oc = create(:simple_order_cycle, distributors: [d], variants: [p_valid.master, p_invalid.master])
+      oc = create(:simple_order_cycle, distributors: [d], variants: [v_valid, p_invalid.master])
+
       oc.valid_products_distributed_by(d).should == [p_valid]
     end
 
@@ -313,7 +315,7 @@ describe OrderCycle do
         @oc.pickup_time_for(@d2).should == '2-8pm Friday'
       end
     end
-    
+
     describe "finding pickup instructions for a distributor" do
       it "returns the pickup instructions" do
         @oc.pickup_instructions_for(@d1).should == "Come get it!"
@@ -375,7 +377,7 @@ describe OrderCycle do
 
     occ.coordinator_fee_ids.should_not be_empty
     occ.coordinator_fee_ids.should == oc.coordinator_fee_ids
-    
+
     # to_h gives us a unique hash for each exchange
     # check that the clone has no additional exchanges
     occ.exchanges.map(&:to_h).all? do |ex|
@@ -402,7 +404,7 @@ describe OrderCycle do
   describe "finding order cycles opening in the future" do
     it "should give the soonest opening order cycle for a distributor" do
       distributor = create(:distributor_enterprise)
-      oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 10.days.from_now, orders_close_at: 11.days.from_now) 
+      oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 10.days.from_now, orders_close_at: 11.days.from_now)
       OrderCycle.first_opening_for(distributor).should == oc
     end
 
@@ -411,12 +413,12 @@ describe OrderCycle do
       OrderCycle.first_opening_for(distributor).should == nil
     end
   end
-  
+
   describe "finding open order cycles" do
     it "should give the soonest closing order cycle for a distributor" do
       distributor = create(:distributor_enterprise)
-      oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 1.days.ago, orders_close_at: 11.days.from_now) 
-      oc2 = create(:simple_order_cycle, name: 'oc 2', distributors: [distributor], orders_open_at: 2.days.ago, orders_close_at: 12.days.from_now) 
+      oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 1.days.ago, orders_close_at: 11.days.from_now)
+      oc2 = create(:simple_order_cycle, name: 'oc 2', distributors: [distributor], orders_open_at: 2.days.ago, orders_close_at: 12.days.from_now)
       OrderCycle.first_closing_for(distributor).should == oc
     end
   end
diff --git a/spec/requests/shop_spec.rb b/spec/requests/shop_spec.rb
index c13cfc675f..cc8838220c 100644
--- a/spec/requests/shop_spec.rb
+++ b/spec/requests/shop_spec.rb
@@ -7,72 +7,57 @@ describe "Shop API" do
     let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) }
     let(:supplier) { create(:supplier_enterprise) }
     let(:oc1) { create(:simple_order_cycle, distributors: [distributor], coordinator: create(:distributor_enterprise), orders_close_at: 2.days.from_now) }
-    let(:p1) { create(:simple_product, on_demand: false) }
-    let(:p2) { create(:simple_product, on_demand: true) }
-    let(:p3) { create(:simple_product, on_demand: false) }
     let(:p4) { create(:simple_product, on_demand: false) }
     let(:p5) { create(:simple_product, on_demand: false) }
     let(:p6) { create(:simple_product, on_demand: false) }
     let(:p7) { create(:simple_product, on_demand: false) }
-    let(:v1) { create(:variant, product: p4, unit_value: 2) }
-    let(:v2) { create(:variant, product: p4, unit_value: 3, on_demand: false) }
-    let(:v3) { create(:variant, product: p4, unit_value: 4, on_demand: true) }
-    let(:v4) { create(:variant, product: p5) }
-    let(:v5) { create(:variant, product: p5) }
-    let(:v6) { create(:variant, product: p7) }
+    let(:v41) { p4.variants.first }
+    let(:v42) { create(:variant, product: p4, unit_value: 3, on_demand: false) }
+    let(:v43) { create(:variant, product: p4, unit_value: 4, on_demand: true) }
+    let(:v51) { p5.variants.first }
+    let(:v52) { create(:variant, product: p5) }
+    let(:v61) { p6.variants.first }
+    let(:v71) { p7.variants.first }
     let(:order) { create(:order, distributor: distributor, order_cycle: oc1) }
 
     before do
       set_order order
 
-      p1.master.update_attribute(:count_on_hand, 1)
-      p2.master.update_attribute(:count_on_hand, 0)
-      p3.master.update_attribute(:count_on_hand, 0)
-      p6.master.update_attribute(:count_on_hand, 1)
+      v61.update_attribute(:count_on_hand, 1)
       p6.delete
-      p7.master.update_attribute(:count_on_hand, 1)
-      v1.update_attribute(:count_on_hand, 1)
-      v2.update_attribute(:count_on_hand, 0)
-      v3.update_attribute(:count_on_hand, 0)
-      v4.update_attribute(:count_on_hand, 1)
-      v5.update_attribute(:count_on_hand, 0)
-      v6.update_attribute(:count_on_hand, 1)
-      v6.update_attribute(:deleted_at, Time.now)
-      exchange = Exchange.find(oc1.exchanges.to_enterprises(distributor).outgoing.first.id) 
-      exchange.update_attribute :pickup_time, "frogs" 
-      exchange.variants << p1.master
-      exchange.variants << p2.master
-      exchange.variants << p3.master
-      exchange.variants << p6.master
-      exchange.variants << v1
-      exchange.variants << v2
-      exchange.variants << v3
-      # v4 is in stock but not in distribution
-      # v5 is out of stock and in the distribution
+      v71.update_attribute(:count_on_hand, 1)
+      v41.update_attribute(:count_on_hand, 1)
+      v42.update_attribute(:count_on_hand, 0)
+      v43.update_attribute(:count_on_hand, 0)
+      v51.update_attribute(:count_on_hand, 1)
+      v52.update_attribute(:count_on_hand, 0)
+      v71.update_attribute(:count_on_hand, 1)
+      v71.update_attribute(:deleted_at, Time.now)
+      exchange = Exchange.find(oc1.exchanges.to_enterprises(distributor).outgoing.first.id)
+      exchange.update_attribute :pickup_time, "frogs"
+      exchange.variants << v61
+      exchange.variants << v41
+      exchange.variants << v42
+      exchange.variants << v43
+      # v51 is in stock but not in distribution
+      # v52 is out of stock and in the distribution
       # Neither should display, nor should their product, p5
-      exchange.variants << v5
-      exchange.variants << v6
+      exchange.variants << v52
+      exchange.variants << v71
       get products_shop_path
     end
 
     it "filters products based on availability" do
-      # It shows on hand products
-      response.body.should include p1.name
-      response.body.should include p4.name
-      # It shows on demand products
-      response.body.should include p2.name
-      # It does not show products that are neither on hand or on demand
-      response.body.should_not include p3.name
       # It shows on demand variants
-      response.body.should include v3.options_text
+      response.body.should include v43.options_text
       # It does not show variants that are neither on hand or on demand
-      response.body.should_not include v2.options_text
+      response.body.should_not include v42.options_text
       # It does not show products that have no available variants in this distribution
       response.body.should_not include p5.name
       # It does not show deleted products
       response.body.should_not include p6.name
       # It does not show deleted variants
-      response.body.should_not include v6.name
+      response.body.should_not include v71.name
       response.body.should_not include p7.name
     end
   end

From 63353ebace437410161d1456c3668537372bc525 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 17 Apr 2015 16:49:14 +1000
Subject: [PATCH 21/37] Don't try and delete the only variant, that will never
 work!

---
 spec/features/admin/variants_spec.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/spec/features/admin/variants_spec.rb b/spec/features/admin/variants_spec.rb
index c947872949..910f299386 100644
--- a/spec/features/admin/variants_spec.rb
+++ b/spec/features/admin/variants_spec.rb
@@ -60,12 +60,14 @@ feature %q{
 
   it "soft-deletes variants", js: true do
     p = create(:simple_product)
-    v = p.variants.first
+    v = create(:variant, product: p)
 
     login_to_admin_section
     visit spree.admin_product_variants_path p
 
-    page.find('a.delete-resource').click
+    within "tr#spree_variant_#{v.id}" do
+      page.find('a.delete-resource').click
+    end
     page.should_not have_content v.options_text
 
     v.reload

From 72d553ef0c12bf7cf02c4123b613e11b3dc52115 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 17 Apr 2015 16:51:02 +1000
Subject: [PATCH 22/37] Test actual deletion of variants

---
 spec/models/spree/product_spec.rb | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 42c85e98e3..d6e814fed2 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -29,11 +29,7 @@ module Spree
         product.on_hand = "10,000"
         expect(product.save).to be_false
 
-        # It is expected that master variant :count_on_hand should cause the failure to save, and it does. However, this broke with the addition of
-        # the :ensure_standard_variant callback. Evidently, normal variants are checked and their errors added to the product model before the
-        # save_master callback on product is run. Therefore, the error that is raised here is one on a normal variant, rather than one on master.
-        # expect(product.errors[:count_on_hand]).to include "is not a number"
-        expect(product.errors[:"variants.count_on_hand"]).to include "is not a number"
+        expect(product.errors[:count_on_hand]).to include "is not a number"
       end
 
       it "defaults available_on to now" do
@@ -64,9 +60,9 @@ module Spree
       it "does not allow the last variant to be deleted" do
         product = create(:simple_product)
         expect(product.variants(:reload).length).to eq 1
-        product.variants = []
-        expect(product.save).to be_false
-        expect(product.errors[:variants]).to include "Product must have at least one variant"
+        v = product.variants.last
+        v.delete
+        expect(v.errors[:product]).to include "must have at least one variant"
       end
 
       context "when the product has variants" do

From 03fd148f418b285be35c145e9f3a25937535ce38 Mon Sep 17 00:00:00 2001
From: Maikel Linke <mkllnk@web.de>
Date: Fri, 17 Apr 2015 16:58:49 +1000
Subject: [PATCH 23/37] showing profile modals on groups/hubs

---
 .../controllers/group_enterprises_controller.js.coffee         | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/darkswarm/controllers/group_enterprises_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/group_enterprises_controller.js.coffee
index db1a9a1d8b..ea652b8105 100644
--- a/app/assets/javascripts/darkswarm/controllers/group_enterprises_controller.js.coffee
+++ b/app/assets/javascripts/darkswarm/controllers/group_enterprises_controller.js.coffee
@@ -1,9 +1,10 @@
-Darkswarm.controller "GroupEnterprisesCtrl", ($scope, Search, FilterSelectorsService) ->
+Darkswarm.controller "GroupEnterprisesCtrl", ($scope, Search, FilterSelectorsService, EnterpriseModal) ->
   $scope.totalActive =  FilterSelectorsService.totalActive
   $scope.clearAll =  FilterSelectorsService.clearAll
   $scope.filterText =  FilterSelectorsService.filterText
   $scope.FilterSelectorsService =  FilterSelectorsService
   $scope.query = Search.search()
+  $scope.openModal = EnterpriseModal.open
   $scope.activeTaxons = []
   $scope.show_profiles = false
   $scope.filtersActive = false

From 893b743973fceb8bf3f610597b30ea5df1d302ee Mon Sep 17 00:00:00 2001
From: Maikel Linke <mkllnk@web.de>
Date: Fri, 17 Apr 2015 17:00:18 +1000
Subject: [PATCH 24/37] tidy (rm comment)

---
 .../darkswarm/controllers/groups_controller.js.coffee        | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/app/assets/javascripts/darkswarm/controllers/groups_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/groups_controller.js.coffee
index 4bb856ace4..aafb7597d6 100644
--- a/app/assets/javascripts/darkswarm/controllers/groups_controller.js.coffee
+++ b/app/assets/javascripts/darkswarm/controllers/groups_controller.js.coffee
@@ -1,8 +1,3 @@
 Darkswarm.controller "GroupsCtrl", ($scope, Groups, $anchorScroll, $rootScope) ->
   $scope.Groups = Groups
   $scope.order = 'position'
-
-  #$rootScope.$on "$locationChangeSuccess", (newRoute, oldRoute) ->
-    #$anchorScroll()
-    #
-      #

From ff2e6d9ca46806af706181c9087e77c603823826 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 17 Apr 2015 17:25:13 +1000
Subject: [PATCH 25/37] Test deletion rather than destruction on variant model
 spec

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

diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb
index 20c4f20cf7..88c9b14e26 100644
--- a/app/models/spree/variant_decorator.rb
+++ b/app/models/spree/variant_decorator.rb
@@ -104,10 +104,12 @@ Spree::Variant.class_eval do
   def delete
     if product.variants == [self] # Only variant left on product
       errors.add :product, "must have at least one variant"
+      false
     else
       transaction do
         self.update_column(:deleted_at, Time.now)
         ExchangeVariant.where(variant_id: self).destroy_all
+        self
       end
     end
   end
diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb
index 2c37065916..7910087dfd 100644
--- a/spec/models/spree/variant_spec.rb
+++ b/spec/models/spree/variant_spec.rb
@@ -401,15 +401,17 @@ module Spree
     end
 
     context "as the last variant of a product" do
-      let!(:product) { create(:simple_product) }
-      let!(:first_variant) { product.variants(:reload).first }
-      let!(:extra_variant) { create(:variant, product: product) }
+      let!(:extra_variant) { create(:variant) }
+      let!(:product) { extra_variant.product }
+      let!(:first_variant) { product.variants.first }
+
+      before { product.reload }
 
       it "cannot be deleted" do
-        expect(product.variants(:reload).length).to eq 2
-        expect(extra_variant.destroy).to eq extra_variant
+        expect(product.variants.length).to eq 2
+        expect(extra_variant.delete).to eq extra_variant
         expect(product.variants(:reload).length).to eq 1
-        expect(first_variant.destroy).to be_false
+        expect(first_variant.delete).to be_false
         expect(product.variants(:reload).length).to eq 1
         expect(first_variant.errors[:product]).to include "must have at least one variant"
       end

From 05c350b5ff482d8d3baf533179b8ee7dfc965694 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 24 Apr 2015 11:27:47 +1000
Subject: [PATCH 26/37] Refactoring unitsCtrl

---
 .../admin/products/units_controller.js.coffee | 12 ++--
 .../products/units_controller_spec.js.coffee  | 55 +++++++++++++++++++
 2 files changed, 63 insertions(+), 4 deletions(-)
 create mode 100644 spec/javascripts/unit/admin/products/units_controller_spec.js.coffee

diff --git a/app/assets/javascripts/admin/products/units_controller.js.coffee b/app/assets/javascripts/admin/products/units_controller.js.coffee
index db005e490a..c9c5807af3 100644
--- a/app/assets/javascripts/admin/products/units_controller.js.coffee
+++ b/app/assets/javascripts/admin/products/units_controller.js.coffee
@@ -5,6 +5,13 @@ angular.module("admin.products")
     $scope.placeholder_text = ""
 
     $scope.$watchCollection '[product.variant_unit_with_scale, product.master.unit_value_with_description]', ->
+      $scope.processVariantUnitWithScale()
+      $scope.processUnitValueWithDescription()
+      $scope.placeholder_text = new OptionValueNamer($scope.product.master).name()
+
+    $scope.variant_unit_options = VariantUnitManager.variantUnitOptions()
+
+    $scope.processVariantUnitWithScale = ->
       if $scope.product.variant_unit_with_scale
         match = $scope.product.variant_unit_with_scale.match(/^([^_]+)_([\d\.]+)$/)
         if match
@@ -16,6 +23,7 @@ angular.module("admin.products")
       else
         $scope.product.variant_unit = $scope.product.variant_unit_scale = null
 
+    $scope.processUnitValueWithDescription = ->
       if $scope.product.master.hasOwnProperty("unit_value_with_description")
         match = $scope.product.master.unit_value_with_description.match(/^([\d\.]+(?= |$)|)( |)(.*)$/)
         if match
@@ -24,10 +32,6 @@ angular.module("admin.products")
           $scope.product.master.unit_value *= $scope.product.variant_unit_scale if $scope.product.master.unit_value && $scope.product.variant_unit_scale
           $scope.product.master.unit_description = match[3]
 
-      $scope.placeholder_text = new OptionValueNamer($scope.product.master).name()
-
-    $scope.variant_unit_options = VariantUnitManager.variantUnitOptions()
-
     $scope.hasVariants = (product) ->
       Object.keys(product.variants).length > 0
 
diff --git a/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee b/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee
new file mode 100644
index 0000000000..c5f104f07a
--- /dev/null
+++ b/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee
@@ -0,0 +1,55 @@
+describe "unitsCtrl", ->
+  ctrl = null
+  scope = null
+  product = null
+
+  beforeEach ->
+    module('admin.products')
+    inject ($rootScope, $controller, VariantUnitManager) ->
+      scope = $rootScope
+      ctrl = $controller 'unitsCtrl', {$scope: scope, VariantUnitManager: VariantUnitManager}
+
+  describe "interpretting variant_unit_with_scale", ->
+    it "splits string with one underscore and stores the two parts", ->
+      scope.product.variant_unit_with_scale = "weight_1000"
+      scope.processVariantUnitWithScale()
+      expect(scope.product.variant_unit).toEqual "weight"
+      expect(scope.product.variant_unit_scale).toEqual 1000
+
+    it "interprets strings with no underscore as variant_unit", ->
+      scope.product.variant_unit_with_scale = "items"
+      scope.processVariantUnitWithScale()
+      expect(scope.product.variant_unit).toEqual "items"
+      expect(scope.product.variant_unit_scale).toEqual null
+
+    it "sets variant_unit and variant_unit_scale to null", ->
+      scope.product.variant_unit_with_scale = null
+      scope.processVariantUnitWithScale()
+      expect(scope.product.variant_unit).toEqual null
+      expect(scope.product.variant_unit_scale).toEqual null
+
+  describe "interpretting unit_value_with_description", ->
+    beforeEach ->
+      scope.product.master = {}
+
+    describe "when a variant_unit_scale is present", ->
+      beforeEach ->
+        scope.product.variant_unit_scale = 1
+
+      it "splits by whitespace in to unit_value and unit_description", ->
+        scope.product.master.unit_value_with_description = "12 boxes"
+        scope.processUnitValueWithDescription()
+        expect(scope.product.master.unit_value).toEqual 12
+        expect(scope.product.master.unit_description).toEqual "boxes"
+
+      it "uses whole string as unit_value when only numerical characters are present", ->
+        scope.product.master.unit_value_with_description = "12345"
+        scope.processUnitValueWithDescription()
+        expect(scope.product.master.unit_value).toEqual 12345
+        expect(scope.product.master.unit_description).toEqual ''
+
+      it "uses whole string as description when string does not start with a number", ->
+        scope.product.master.unit_value_with_description = "boxes 12"
+        scope.processUnitValueWithDescription()
+        expect(scope.product.master.unit_value).toEqual null
+        expect(scope.product.master.unit_description).toEqual "boxes 12"

From ed7b763ecf3152dac7ada2eb5d0d51a25425dc06 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 24 Apr 2015 12:15:35 +1000
Subject: [PATCH 27/37] UnitsCtrl can interpret unit_value_with_description
 without a separating space

---
 .../admin/products/units_controller.js.coffee        |  2 +-
 .../admin/products/units_controller_spec.js.coffee   | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/admin/products/units_controller.js.coffee b/app/assets/javascripts/admin/products/units_controller.js.coffee
index c9c5807af3..a84f048580 100644
--- a/app/assets/javascripts/admin/products/units_controller.js.coffee
+++ b/app/assets/javascripts/admin/products/units_controller.js.coffee
@@ -25,7 +25,7 @@ angular.module("admin.products")
 
     $scope.processUnitValueWithDescription = ->
       if $scope.product.master.hasOwnProperty("unit_value_with_description")
-        match = $scope.product.master.unit_value_with_description.match(/^([\d\.]+(?= |$)|)( |)(.*)$/)
+        match = $scope.product.master.unit_value_with_description.match(/^([\d\.]+(?= *|$)|)( *)(.*)$/)
         if match
           $scope.product.master.unit_value  = parseFloat(match[1])
           $scope.product.master.unit_value  = null if isNaN($scope.product.master.unit_value)
diff --git a/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee b/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee
index c5f104f07a..b846126711 100644
--- a/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee
+++ b/spec/javascripts/unit/admin/products/units_controller_spec.js.coffee
@@ -53,3 +53,15 @@ describe "unitsCtrl", ->
         scope.processUnitValueWithDescription()
         expect(scope.product.master.unit_value).toEqual null
         expect(scope.product.master.unit_description).toEqual "boxes 12"
+
+      it "does not require whitespace to split unit value and description", ->
+        scope.product.master.unit_value_with_description = "12boxes"
+        scope.processUnitValueWithDescription()
+        expect(scope.product.master.unit_value).toEqual 12
+        expect(scope.product.master.unit_description).toEqual "boxes"
+
+      it "once a whitespace occurs, all subsequent numerical characters are counted as description", ->
+        scope.product.master.unit_value_with_description = "123 54 boxes"
+        scope.processUnitValueWithDescription()
+        expect(scope.product.master.unit_value).toEqual 123
+        expect(scope.product.master.unit_description).toEqual "54 boxes"

From d43df862015f2fc7d1a959210572fbca8c6b6554 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 22 May 2015 12:20:31 +1000
Subject: [PATCH 28/37] Moving conditional logic into ensure_standard_variant

---
 app/models/spree/product_decorator.rb | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index e9b99c9b7a..82330c61cf 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -34,7 +34,7 @@ Spree::Product.class_eval do
   validates_presence_of :variant_unit_name,
                         if: -> p { p.variant_unit == 'items' }
 
-  after_save :ensure_standard_variant, if: lambda { master.valid? && variants.empty? }
+  after_save :ensure_standard_variant
   after_initialize :set_available_on_to_now, :if => :new_record?
   after_save :update_units
   after_touch :touch_distributors
@@ -211,10 +211,12 @@ Spree::Product.class_eval do
   end
 
   def ensure_standard_variant
-    variant = self.master.dup
-    variant.product = self
-    variant.is_master = false
-    self.variants << variant
+    if master.valid? && variants.empty?
+      variant = self.master.dup
+      variant.product = self
+      variant.is_master = false
+      self.variants << variant
+    end
   end
 
   # Override Spree's old save_master method and replace it with the most recent method from spree repository

From 63f3ede766216c27c460d02f2a440d5fbacb4371 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 22 May 2015 12:22:05 +1000
Subject: [PATCH 29/37] Prepare master variants for duplication as standard
 variant by ensuring they have a unit value and that the product has a variant
 unit

---
 ...060622_add_standard_variant_to_products.rb | 20 ++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/db/migrate/20141003060622_add_standard_variant_to_products.rb b/db/migrate/20141003060622_add_standard_variant_to_products.rb
index 7e7f752a5d..5242f86686 100644
--- a/db/migrate/20141003060622_add_standard_variant_to_products.rb
+++ b/db/migrate/20141003060622_add_standard_variant_to_products.rb
@@ -1,9 +1,27 @@
 class AddStandardVariantToProducts < ActiveRecord::Migration
   def up
+    # Make sure that all products have a variant_unit
+    Spree::Product.where("variant_unit IS NULL OR variant_unit = ''").update_all(variant_unit: "items", variant_unit_name: "each")
+
     # Find products without any standard variants
-    products_with_only_master = Spree::Product.includes(:variants).where('spree_variants.id IS NULL')
+    products_with_only_master = Spree::Product.includes(:variants).where('spree_variants.id IS NULL').select('DISTINCT spree_products.*')
 
     products_with_only_master.each do |product|
+      # Add a unit_value to the master variant if it doesn't have one
+      if product.unit_value.blank?
+        if product.variant_unit == "weight" && match = product.unit_description.andand.match(/^(\d+(\.\d*)?)(k?g) ?(.*)$/)
+          scale = (match[3] == "kg" ? 1000 : 1)
+          product.unit_value = (match[1].to_i*scale)
+          product.unit_description = match[4]
+          product.save!
+        else
+          unless product.variant_unit == "items" && product.unit_description.present?
+            product.unit_value = 1
+            product.save!
+          end
+        end
+      end
+
       # Run the callback to add a copy of the master variant as a standard variant
       product.send(:ensure_standard_variant)
 

From 5b65f67737b93b2c5a1b7f1f2e46d45179cd04c0 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 22 May 2015 15:02:49 +1000
Subject: [PATCH 30/37] Amending spec expect after_create when we want
 after_save

---
 spec/models/spree/product_spec.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index c30a5920ca..9423e2672c 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -116,10 +116,10 @@ module Spree
             expect(standard_variant.price).to eq product.master.price
           end
 
-          it "only duplicates master variant on create" do
-            expect(product).to_not receive :ensure_standard_variant
+          it "only duplicates master with after_save when no standard variants exist" do
+            expect(product).to receive :ensure_standard_variant
             product.name = "Something else"
-            product.save!
+            expect{product.save!}.to_not change{product.variants.count}
           end
         end
 

From 5d47dc2fdb3652d486d32c9422f90126a13c49b1 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 22 May 2015 15:03:21 +1000
Subject: [PATCH 31/37] Sort array so that order doesn't cause spec fail

---
 spec/lib/open_food_network/permissions_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb
index cf2aa1a217..9bf5ef2d9d 100644
--- a/spec/lib/open_food_network/permissions_spec.rb
+++ b/spec/lib/open_food_network/permissions_spec.rb
@@ -47,7 +47,7 @@ module OpenFoodNetwork
           expect(permissions).to receive(:managed_enterprises) { Enterprise.where(id: e1) }
           expect(permissions).to receive(:related_enterprises_granting).with(:some_permission) { Enterprise.where(id: e3) }
           expect(permissions).to receive(:related_enterprises_granted).with(:some_permission) { Enterprise.where(id: e4) }
-          expect(permissions.send(:managed_and_related_enterprises_with, :some_permission)).to eq [e1, e3, e4]
+          expect(permissions.send(:managed_and_related_enterprises_with, :some_permission).sort).to eq [e1, e3, e4].sort
         end
       end
     end

From 28dae3c6c6340b49ce65c51bf2c949c1f560ce37 Mon Sep 17 00:00:00 2001
From: Maikel Linke <mkllnk@web.de>
Date: Fri, 22 May 2015 15:23:19 +1000
Subject: [PATCH 32/37] Enterprises cannot add themselves to Groups

---
 .../admin/enterprises_controller.rb            |  5 +++++
 .../form/_primary_details.html.haml            | 18 +++++++++---------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb
index d4b3244525..a0b55ad3c8 100644
--- a/app/controllers/admin/enterprises_controller.rb
+++ b/app/controllers/admin/enterprises_controller.rb
@@ -3,6 +3,7 @@ module Admin
     before_filter :load_enterprise_set, :only => :index
     before_filter :load_countries, :except => [:index, :set_sells, :check_permalink]
     before_filter :load_methods_and_fees, :only => [:new, :edit, :update, :create]
+    before_filter :load_groups, :only => [:new, :edit, :update, :create]
     before_filter :load_taxons, :only => [:new, :edit, :update, :create]
     before_filter :check_can_change_sells, only: :update
     before_filter :check_can_change_bulk_sells, only: :bulk_update
@@ -127,6 +128,10 @@ module Admin
       @enterprise_fees = EnterpriseFee.managed_by(spree_current_user).for_enterprise(@enterprise).order(:fee_type, :name).all
     end
 
+    def load_groups
+      @groups = EnterpriseGroup.managed_by(spree_current_user) | @enterprise.groups
+    end
+
     def load_taxons
       @taxons = Spree::Taxon.order(:name)
     end
diff --git a/app/views/admin/enterprises/form/_primary_details.html.haml b/app/views/admin/enterprises/form/_primary_details.html.haml
index dce5512f40..96f4674912 100644
--- a/app/views/admin/enterprises/form/_primary_details.html.haml
+++ b/app/views/admin/enterprises/form/_primary_details.html.haml
@@ -5,15 +5,15 @@
       %span.required *
     .eight.columns.omega
       = f.text_field :name, { placeholder: "eg. Professor Plum's Biodynamic Truffles" }
-.row
-  .alpha.eleven.columns
-    .three.columns.alpha
-      = f.label :group_ids, 'Groups'
-      .with-tip{'data-powertip' => "Select any groups or regions that you are a member of. This will help customers find your enterprise."}
-        %a What's this?
-
-    .eight.columns.omega
-      = f.collection_select :group_ids, EnterpriseGroup.all, :id, :name, {}, class: "select2 fullwidth", multiple: true, placeholder: "Start typing to search available groups..."
+- if @groups.present?
+  .row
+    .alpha.eleven.columns
+      .three.columns.alpha
+        = f.label :group_ids, 'Groups'
+        .with-tip{'data-powertip' => "Select any groups or regions that you are a member of. This will help customers find your enterprise."}
+          %a What's this?
+      .eight.columns.omega
+        = f.collection_select :group_ids, @groups, :id, :name, {}, class: "select2 fullwidth", multiple: true, placeholder: "Start typing to search available groups..."
 
 .row
   .three.columns.alpha

From 19448a182ecade379bc1efaba59181f7e766f851 Mon Sep 17 00:00:00 2001
From: Maikel Linke <mkllnk@web.de>
Date: Wed, 27 May 2015 11:54:13 +1000
Subject: [PATCH 33/37] Add permalink field to enterprise groups

---
 app/models/enterprise_group.rb                |  1 +
 .../20150527004427_add_permalink_to_groups.rb | 26 +++++++++++++++++++
 db/schema.rb                                  |  6 +++--
 spec/factories.rb                             |  1 +
 4 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 db/migrate/20150527004427_add_permalink_to_groups.rb

diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb
index 9d510d8f2b..aae2af4b17 100644
--- a/app/models/enterprise_group.rb
+++ b/app/models/enterprise_group.rb
@@ -15,6 +15,7 @@ class EnterpriseGroup < ActiveRecord::Base
 
   validates :name, presence: true
   validates :description, presence: true
+  validates :permalink, uniqueness: true, presence: true
 
   attr_accessible :name, :description, :long_description, :on_front_page, :enterprise_ids
   attr_accessible :owner_id
diff --git a/db/migrate/20150527004427_add_permalink_to_groups.rb b/db/migrate/20150527004427_add_permalink_to_groups.rb
new file mode 100644
index 0000000000..ab6886cff4
--- /dev/null
+++ b/db/migrate/20150527004427_add_permalink_to_groups.rb
@@ -0,0 +1,26 @@
+class AddPermalinkToGroups < ActiveRecord::Migration
+  def up
+    add_column :enterprise_groups, :permalink, :string
+
+    EnterpriseGroup.reset_column_information
+
+    EnterpriseGroup.all.each do |group|
+      counter = 1
+      permalink = group.name.parameterize
+      permalink = "my-group-name" if permalink == ""
+      while EnterpriseGroup.find_by_permalink(permalink) do
+        permalink = group.name.parameterize + counter.to_s
+        counter += 1
+      end
+
+      group.update_column :permalink, permalink
+    end
+
+    change_column :enterprise_groups, :permalink, :string, null: false
+    add_index :enterprise_groups, :permalink, :unique => true
+  end
+
+  def down
+    remove_column :enterprise_groups, :permalink
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f9f32af7d1..6d9de020f1 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20150424151117) do
+ActiveRecord::Schema.define(:version => 20150527004427) do
 
   create_table "adjustment_metadata", :force => true do |t|
     t.integer "adjustment_id"
@@ -236,10 +236,12 @@ ActiveRecord::Schema.define(:version => 20150424151117) do
     t.string   "linkedin",                 :default => "", :null => false
     t.string   "twitter",                  :default => "", :null => false
     t.integer  "owner_id"
+    t.string   "permalink",                                :null => false
   end
 
   add_index "enterprise_groups", ["address_id"], :name => "index_enterprise_groups_on_address_id"
   add_index "enterprise_groups", ["owner_id"], :name => "index_enterprise_groups_on_owner_id"
+  add_index "enterprise_groups", ["permalink"], :name => "index_enterprise_groups_on_permalink", :unique => true
 
   create_table "enterprise_groups_enterprises", :id => false, :force => true do |t|
     t.integer "enterprise_group_id"
@@ -619,9 +621,9 @@ ActiveRecord::Schema.define(:version => 20150424151117) do
     t.string   "email"
     t.text     "special_instructions"
     t.integer  "distributor_id"
-    t.integer  "order_cycle_id"
     t.string   "currency"
     t.string   "last_ip_address"
+    t.integer  "order_cycle_id"
     t.integer  "cart_id"
   end
 
diff --git a/spec/factories.rb b/spec/factories.rb
index cb4d06b35e..f61172dc2a 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -135,6 +135,7 @@ FactoryGirl.define do
 
   factory :enterprise_group, :class => EnterpriseGroup do
     name 'Enterprise group'
+    sequence(:permalink) { |n| "group#{n}" }
     description 'this is a group'
     on_front_page false
     address { FactoryGirl.build(:address) }

From aef128f2c9282793ad55d7b9fa09a8413d06cdfc Mon Sep 17 00:00:00 2001
From: Maikel Linke <mkllnk@web.de>
Date: Wed, 27 May 2015 12:20:54 +1000
Subject: [PATCH 34/37] permalink editable

---
 app/models/enterprise_group.rb                               | 1 +
 .../admin/enterprise_groups/_form_primary_details.html.haml  | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb
index aae2af4b17..1524a80b53 100644
--- a/app/models/enterprise_group.rb
+++ b/app/models/enterprise_group.rb
@@ -19,6 +19,7 @@ class EnterpriseGroup < ActiveRecord::Base
 
   attr_accessible :name, :description, :long_description, :on_front_page, :enterprise_ids
   attr_accessible :owner_id
+  attr_accessible :permalink
   attr_accessible :logo, :promo_image
   attr_accessible :address_attributes
   attr_accessible :email, :website, :facebook, :instagram, :linkedin, :twitter
diff --git a/app/views/admin/enterprise_groups/_form_primary_details.html.haml b/app/views/admin/enterprise_groups/_form_primary_details.html.haml
index 6d326e33fa..d57333626f 100644
--- a/app/views/admin/enterprise_groups/_form_primary_details.html.haml
+++ b/app/views/admin/enterprise_groups/_form_primary_details.html.haml
@@ -19,3 +19,8 @@
     = f.label :enterprise_ids, 'Enterprises'
     %br/
     = f.collection_select :enterprise_ids, @enterprises, :id, :name, {}, {class: "select2 fullwidth", multiple: true}
+
+  = f.field_container :permalink do
+    = f.label :permalink, "Permalink (unique, no spaces)"
+    %br/
+    = f.text_field :permalink

From e4f93863fd5ce50cb089af4d43a8008a8bb24ca2 Mon Sep 17 00:00:00 2001
From: Maikel Linke <mkllnk@web.de>
Date: Thu, 28 May 2015 10:17:09 +1000
Subject: [PATCH 35/37] Finding unique permalink before validation.

---
 app/models/enterprise.rb             |  2 +-
 app/models/enterprise_group.rb       | 25 +++++++++++++
 spec/factories.rb                    |  1 -
 spec/models/enterprise_group_spec.rb | 55 ++++++++++++++++++++++++++++
 spec/models/enterprise_spec.rb       |  5 +++
 5 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb
index e921167cf0..067e800e3c 100644
--- a/app/models/enterprise.rb
+++ b/app/models/enterprise.rb
@@ -299,7 +299,7 @@ class Enterprise < ActiveRecord::Base
     test_permalink = test_permalink.parameterize
     test_permalink = "my-enterprise" if test_permalink.blank?
     existing = Enterprise.select(:permalink).order(:permalink).where("permalink LIKE ?", "#{test_permalink}%").map(&:permalink)
-    if existing.empty?
+    unless existing.include?(test_permalink)
       test_permalink
     else
       used_indices = existing.map do |p|
diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb
index 1524a80b53..a59b641d7c 100644
--- a/app/models/enterprise_group.rb
+++ b/app/models/enterprise_group.rb
@@ -15,6 +15,8 @@ class EnterpriseGroup < ActiveRecord::Base
 
   validates :name, presence: true
   validates :description, presence: true
+
+  before_validation :sanitize_permalink
   validates :permalink, uniqueness: true, presence: true
 
   attr_accessible :name, :description, :long_description, :on_front_page, :enterprise_ids
@@ -73,4 +75,27 @@ class EnterpriseGroup < ActiveRecord::Base
     address.zipcode.sub!(/^undefined$/, '')
   end
 
+  private
+
+  def self.find_available_value(existing, requested)
+    return requested unless existing.include?(requested)
+    used_indices = existing.map do |p|
+      p.slice!(/^#{requested}/)
+      p.match(/^\d+$/).to_s.to_i
+    end
+    options = (1..used_indices.length + 1).to_a - used_indices
+    requested + options.first.to_s
+  end
+
+  def find_available_permalink(requested)
+    existing = self.class.where(id: !id).where("permalink LIKE ?", "#{requested}%").pluck(:permalink)
+    self.class.find_available_value(existing, requested)
+  end
+
+  def sanitize_permalink
+    if permalink.blank? || permalink_changed?
+      requested = permalink.presence || permalink_was.presence || name.presence || 'group'
+      self.permalink = find_available_permalink(requested.parameterize)
+    end
+  end
 end
diff --git a/spec/factories.rb b/spec/factories.rb
index f61172dc2a..abaecffa47 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -97,7 +97,6 @@ FactoryGirl.define do
   factory :enterprise, :class => Enterprise do
     owner { FactoryGirl.create :user }
     sequence(:name) { |n| "Enterprise #{n}" }
-    sequence(:permalink) { |n| "enterprise#{n}" }
     sells 'any'
     description 'enterprise'
     long_description '<p>Hello, world!</p><p>This is a paragraph.</p>'
diff --git a/spec/models/enterprise_group_spec.rb b/spec/models/enterprise_group_spec.rb
index ceca13b13b..45e7b0692c 100644
--- a/spec/models/enterprise_group_spec.rb
+++ b/spec/models/enterprise_group_spec.rb
@@ -2,11 +2,32 @@ require 'spec_helper'
 
 describe EnterpriseGroup do
   describe "validations" do
+    it "pass with name, description and address" do
+      e = EnterpriseGroup.new
+      e.name = 'Test Group'
+      e.description = 'A valid test group.'
+      e.address = build(:address)
+      e.should be_valid
+    end
+
     it "is valid when built from factory" do
       e = build(:enterprise_group)
       e.should be_valid
     end
 
+    it "replace empty permalink and pass" do
+      e = build(:enterprise_group, permalink: '')
+      e.should be_valid
+      e.permalink.should == e.name.parameterize
+    end
+
+    it "restores permalink and pass" do
+      e = create(:enterprise_group, permalink: 'p')
+      e.permalink = ''
+      e.should be_valid
+      e.permalink.should == 'p'
+    end
+
     it "requires a name" do
       e = build(:enterprise_group, name: '')
       e.should_not be_valid
@@ -60,5 +81,39 @@ describe EnterpriseGroup do
 
       EnterpriseGroup.managed_by(user).should == [eg1]
     end
+
+    describe "finding a permalink" do
+      it "finds available permalink" do
+        existing = []
+        expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink"
+      end
+
+      it "finds available permalink similar to existing" do
+        existing = ["permalink1"]
+        expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink"
+      end
+
+      it "adds unique number to existing permalinks" do
+        existing = ["permalink"]
+        expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink1"
+        existing = ["permalink", "permalink1"]
+        expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink2"
+      end
+
+      it "ignores permalinks with characters after the index value" do
+        existing = ["permalink", "permalink1", "permalink2xxx"]
+        expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink2"
+      end
+
+      it "finds gaps in the indices of existing permalinks" do
+        existing = ["permalink", "permalink1", "permalink3"]
+        expect(EnterpriseGroup.find_available_value(existing, "permalink")).to eq "permalink2"
+      end
+
+      it "finds available indexed permalink" do
+        existing = ["permalink", "permalink1"]
+        expect(EnterpriseGroup.find_available_value(existing, "permalink1")).to eq "permalink11"
+      end
+    end
   end
 end
diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb
index ed4669c0c2..c81c3818df 100644
--- a/spec/models/enterprise_spec.rb
+++ b/spec/models/enterprise_spec.rb
@@ -845,6 +845,11 @@ describe Enterprise do
         expect(Enterprise.find_available_permalink("permalink")).to eq "permalink2"
       end
 
+      it "finds available permalink similar to existing" do
+        create(:enterprise, permalink: "permalink2xxx")
+        expect(Enterprise.find_available_permalink("permalink2")).to eq "permalink2"
+      end
+
       it "finds gaps in the indices of existing permalinks" do
         create(:enterprise, permalink: "permalink3")
         expect(Enterprise.find_available_permalink("permalink")).to eq "permalink2"

From ff2eed77601d0813605e703ef77f3a224faabc6d Mon Sep 17 00:00:00 2001
From: Maikel Linke <mkllnk@web.de>
Date: Thu, 28 May 2015 11:27:06 +1000
Subject: [PATCH 36/37] Using permalink in URLs pointing to groups

---
 app/controllers/admin/enterprise_groups_controller.rb | 10 ++++++++--
 app/controllers/groups_controller.rb                  |  2 +-
 app/models/enterprise_group.rb                        |  4 ++++
 app/views/groups/index.html.haml                      |  2 +-
 app/views/json/_groups.rabl                           |  2 +-
 app/views/shopping_shared/_groups.html.haml           |  2 +-
 6 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/app/controllers/admin/enterprise_groups_controller.rb b/app/controllers/admin/enterprise_groups_controller.rb
index 3f8888edfb..cb3ac80935 100644
--- a/app/controllers/admin/enterprise_groups_controller.rb
+++ b/app/controllers/admin/enterprise_groups_controller.rb
@@ -9,7 +9,7 @@ module Admin
 
     def move_up
       EnterpriseGroup.with_isolation_level_serializable do
-        @enterprise_group = EnterpriseGroup.find params[:enterprise_group_id]
+        @enterprise_group = EnterpriseGroup.find_by_permalink params[:enterprise_group_id]
         @enterprise_group.move_higher
       end
       redirect_to main_app.admin_enterprise_groups_path
@@ -17,7 +17,7 @@ module Admin
 
     def move_down
       EnterpriseGroup.with_isolation_level_serializable do
-        @enterprise_group = EnterpriseGroup.find params[:enterprise_group_id]
+        @enterprise_group = EnterpriseGroup.find_by_permalink params[:enterprise_group_id]
         @enterprise_group.move_lower
       end
       redirect_to main_app.admin_enterprise_groups_path
@@ -33,6 +33,12 @@ module Admin
     end
     alias_method_chain :build_resource, :address
 
+    # Overriding method on Spree's resource controller,
+    # so that resources are found using permalink
+    def find_resource
+      EnterpriseGroup.find_by_permalink(params[:id])
+    end
+
     private
 
     def load_data
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 8653131b5a..fe43f0a0fa 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -7,6 +7,6 @@ class GroupsController < BaseController
   end
 
   def show
-    @group = EnterpriseGroup.find params[:id]
+    @group = EnterpriseGroup.find_by_permalink(params[:id]) || EnterpriseGroup.find(params[:id])
   end
 end
diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb
index a59b641d7c..986dc07d22 100644
--- a/app/models/enterprise_group.rb
+++ b/app/models/enterprise_group.rb
@@ -75,6 +75,10 @@ class EnterpriseGroup < ActiveRecord::Base
     address.zipcode.sub!(/^undefined$/, '')
   end
 
+  def to_param
+    permalink
+  end
+
   private
 
   def self.find_available_value(existing, requested)
diff --git a/app/views/groups/index.html.haml b/app/views/groups/index.html.haml
index 901cb17ad9..710039c7fb 100644
--- a/app/views/groups/index.html.haml
+++ b/app/views/groups/index.html.haml
@@ -20,7 +20,7 @@
     .row.pad-top{bindonce: true}
       .small-12.medium-6.columns
         .groups-header
-          %a{"bo-href-i" => "/groups/{{group.id}}"}
+          %a{"bo-href-i" => "/groups/{{group.permalink}}"}
             %i.ofn-i_035-groups
             %span.group-name{"bo-text" => "group.name"}
       .small-3.medium-2.columns
diff --git a/app/views/json/_groups.rabl b/app/views/json/_groups.rabl
index a079290e23..bc50586a62 100644
--- a/app/views/json/_groups.rabl
+++ b/app/views/json/_groups.rabl
@@ -1,5 +1,5 @@
 collection @groups
-attributes :id, :name, :position, :description, :long_description, :email, :website, :facebook, :instagram, :linkedin, :twitter
+attributes :id, :permalink, :name, :position, :description, :long_description, :email, :website, :facebook, :instagram, :linkedin, :twitter
 
 child enterprises: :enterprises do
   attributes :id
diff --git a/app/views/shopping_shared/_groups.html.haml b/app/views/shopping_shared/_groups.html.haml
index 68c68882fb..d94d61359c 100644
--- a/app/views/shopping_shared/_groups.html.haml
+++ b/app/views/shopping_shared/_groups.html.haml
@@ -9,5 +9,5 @@
           %ul.bullet-list
             - for group in current_distributor.groups
               %li
-                %a{href: main_app.groups_path + "/#/#group#{group.id}"}
+                %a{href: main_app.groups_path + "/#{group.permalink}"}
                   = group.name 

From 835b56b2220d25dc0f767d52ad7a7c9f8fca722d Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 29 May 2015 09:44:57 +1000
Subject: [PATCH 37/37] Attempt to fix intermittent failures in
 spec/features/consumer/authentication_spec.rb

---
 spec/features/consumer/authentication_spec.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb
index d0302bcd6f..bfa9f141fe 100644
--- a/spec/features/consumer/authentication_spec.rb
+++ b/spec/features/consumer/authentication_spec.rb
@@ -3,6 +3,11 @@ require 'spec_helper'
 feature "Authentication", js: true do
   include UIComponentHelper
 
+  # Attempt to address intermittent failures in these specs
+  around do |example|
+    Capybara.using_wait_time(120) { example.run }
+  end
+
   describe "login" do
     let(:user) { create(:user, password: "password", password_confirmation: "password") }