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] 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