From 66866f09b5d69e90e94b2a920ae06c775231cc8b Mon Sep 17 00:00:00 2001
From: Will Marshall <will@willmarshall.me>
Date: Wed, 5 Mar 2014 11:12:23 +1100
Subject: [PATCH 01/12] Adding the require ship address flag to admin UI:

---
 .../_form/add_require_ship_address.html.haml.deface       | 5 +++++
 spec/features/admin/shipping_methods_spec.rb              | 8 ++++++++
 2 files changed, 13 insertions(+)
 create mode 100644 app/overrides/spree/admin/shipping_methods/_form/add_require_ship_address.html.haml.deface

diff --git a/app/overrides/spree/admin/shipping_methods/_form/add_require_ship_address.html.haml.deface b/app/overrides/spree/admin/shipping_methods/_form/add_require_ship_address.html.haml.deface
new file mode 100644
index 0000000000..b7bffd4c84
--- /dev/null
+++ b/app/overrides/spree/admin/shipping_methods/_form/add_require_ship_address.html.haml.deface
@@ -0,0 +1,5 @@
+/ insert_bottom "[data-hook='admin_shipping_method_form_availability_fields'] > fieldset"
+
+= f.field_container :shipping_requirements do
+  = f.label :require_ship_address, "Requires shipping address?"
+  = f.check_box :require_ship_address
diff --git a/spec/features/admin/shipping_methods_spec.rb b/spec/features/admin/shipping_methods_spec.rb
index 575e2b7da4..ab7940acc2 100644
--- a/spec/features/admin/shipping_methods_spec.rb
+++ b/spec/features/admin/shipping_methods_spec.rb
@@ -70,6 +70,14 @@ feature 'shipping methods' do
       login_to_admin_as enterprise_user
     end
 
+    it "lets me choose whether a shipping address is required" do
+      click_link "Enterprises"
+      within(".enterprise-#{distributor1.id}") { click_link 'Shipping Methods' }
+      click_link 'New Shipping Method'
+
+      page.should have_content "Requires shipping address?"
+    end
+
     it "creates shipping methods" do
       click_link 'Enterprises'
       within(".enterprise-#{distributor1.id}") { click_link 'Shipping Methods' }

From 19350eeade487dc05474dcc063271d90119ec3dc Mon Sep 17 00:00:00 2001
From: Will Marshall <will@willmarshall.me>
Date: Wed, 5 Mar 2014 12:29:49 +1100
Subject: [PATCH 02/12] Adding the 'same as billing address' JS

---
 .../controllers/checkout_controller.js.coffee |  1 +
 app/views/shop/checkout/_form.html.haml       | 73 ++++++++++++-------
 .../consumer/shopping/checkout_spec.rb        | 28 ++++++-
 3 files changed, 73 insertions(+), 29 deletions(-)

diff --git a/app/assets/javascripts/darkswarm/controllers/checkout_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/checkout_controller.js.coffee
index 6c4ec5d2cb..3b6e56f679 100644
--- a/app/assets/javascripts/darkswarm/controllers/checkout_controller.js.coffee
+++ b/app/assets/javascripts/darkswarm/controllers/checkout_controller.js.coffee
@@ -2,6 +2,7 @@ angular.module("Checkout").controller "CheckoutCtrl", ($scope, $rootScope) ->
   $scope.require_ship_address = false
   $scope.shipping_method = -1
   $scope.payment_method = -1
+  $scope.same_as_billing = true
 
   $scope.shippingMethodChanged = ->
     $scope.require_ship_address = $("#order_shipping_method_id_" + $scope.shipping_method).attr("data-require-ship-address")
diff --git a/app/views/shop/checkout/_form.html.haml b/app/views/shop/checkout/_form.html.haml
index 2add9a5c39..5b0d377071 100644
--- a/app/views/shop/checkout/_form.html.haml
+++ b/app/views/shop/checkout/_form.html.haml
@@ -21,20 +21,27 @@
           = f.fields_for :bill_address do |ba|
             .row
               .large-12.columns
-                = ba.text_field :address1, label: "Billing Address"
+                = ba.text_field :address1, label: "Billing Address",
+                  "ng-model" => "billing_address1"
             .row
               .large-12.columns
-                = ba.text_field :address2
+                = ba.text_field :address2,
+                  "ng-model" => "billing_address2"
             .row
               .large-6.columns
-                = ba.text_field :city
+                = ba.text_field :city,
+                  "ng-model" => "billing_city"
+
               .large-6.columns
-                = ba.select :country_id, Spree::Country.order(:name).select([:id, :name]).map{|c| [c.name, c.id]}
+                = ba.select :country_id, Spree::Country.order(:name).select([:id, :name]).map{|c| [c.name, c.id]},
+                  "ng-model" => "billing_country_id"
             .row
               .large-6.columns
-                = ba.select :state_id, Spree::State.order(:name).select([:id, :name]).map{|c| [c.name, c.id]}
+                = ba.select :state_id, Spree::State.order(:name).select([:id, :name]).map{|c| [c.name, c.id]},
+                  "ng-model" => "billing_state_id"
               .large-6.columns.right
-                = ba.text_field :zipcode
+                = ba.text_field :zipcode,
+                  "ng-model" => "billing_zipcode"
 
       %fieldset#shipping
 
@@ -52,26 +59,42 @@
 
         = f.fields_for :ship_address do |sa|
           #ship_address{"ng-show" => "require_ship_address"}
-            .row
-              .large-12.columns
-                = sa.text_field :address1
-            .row
-              .large-12.columns
-                = sa.text_field :address2
+            %label
+              = check_box_tag :same_as_billing, true, true, 
+                "ng-model" => "same_as_billing" 
+              Shipping address same as billing address?
+
+            %div.visible{"ng-show" => "!same_as_billing"}
+              .row
+                .large-12.columns
+                  = sa.text_field :address1
+              .row
+                .large-12.columns
+                  = sa.text_field :address2
+
+              .row
+                .large-6.columns
+                  = sa.text_field :city
+                .large-6.columns
+                  = sa.select :country_id, Spree::Country.order(:name).select([:id, :name]).map{|c| [c.name, c.id]}
+              .row
+                .large-6.columns.right
+                  = sa.text_field :zipcode
+              .row
+                .large-6.columns
+                  = sa.text_field :firstname
+                .large-6.columns
+                  = sa.text_field :lastname
+
+            #ship_address_hidden{"ng-show" => "same_as_billing"}
+              = sa.hidden_field :address1, "ng-value" => "billing_address1"
+              = sa.hidden_field :address2, "ng-value" => "billing_address2"
+              = sa.hidden_field :city, "ng-value" => "billing_city"
+              = sa.hidden_field :country_id, "ng-value" => "billing_country_id"
+              = sa.hidden_field :zipcode, "ng-value" => "billing_zipcode"
+              = sa.hidden_field :firstname, "ng-value" => "billing_firstname"
+              = sa.hidden_field :lastname, "ng-value" => "billing_lastname"
 
-            .row
-              .large-6.columns
-                = sa.text_field :city
-              .large-6.columns
-                = sa.select :country_id, Spree::Country.order(:name).select([:id, :name]).map{|c| [c.name, c.id]}
-            .row
-              .large-6.columns.right
-                = sa.text_field :zipcode
-            .row
-              .large-6.columns
-                = sa.text_field :firstname
-              .large-6.columns
-                = sa.text_field :lastname
       %fieldset#payment
         %legend Payment Details
         - current_order.available_payment_methods.each do |method|
diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb
index 4e08b9d5e4..6c33dde5d5 100644
--- a/spec/features/consumer/shopping/checkout_spec.rb
+++ b/spec/features/consumer/shopping/checkout_spec.rb
@@ -133,15 +133,35 @@ feature "As a consumer I want to check out my cart", js: true do
         page.should have_content "Donkeys"
       end
 
-      it "doesn't show ship address forms " do
+      it "doesn't show ship address forms when a shipping method wants no address" do
         choose(sm2.name)
         find("#ship_address").visible?.should be_false
       end
 
-      it "shows ship address forms when selected shipping method requires one" do
+      context "When shipping method requires an address" do
+        before do
+          choose(sm1.name)
+        end
+        it "shows the hidden ship address fields by default" do
+          check "Shipping address same as billing address?"
+          find("#ship_address_hidden").visible?.should be_true
+          find("#ship_address > div.visible").visible?.should be_false
+        end
+
+        it "shows ship address forms when 'same as billing address' is unchecked" do
+          uncheck "Shipping address same as billing address?"
+          find("#ship_address_hidden").visible?.should be_false
+          find("#ship_address > div.visible").visible?.should be_true
+        end
+      end
+
+      it "copies billing address to hidden shipping address fields" do
         choose(sm1.name)
-        save_and_open_page
-        find("#ship_address").visible?.should be_true
+        check "Shipping address same as billing address?"
+        fill_in "Billing Address", with: "testy"
+        within "#ship_address_hidden" do
+          find("#order_ship_address_attributes_address1").value.should == "testy"
+        end
       end
 
       describe "with payment methods" do

From ec6f70c145df6221374f20646325416d48249756 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 28 Feb 2014 11:44:21 +1100
Subject: [PATCH 03/12] On product list view, only show variants that are a
 member of the current order cycle

---
 app/controllers/shop/shop_controller.rb |  1 +
 app/models/order_cycle.rb               | 12 +++++--
 app/models/spree/product_decorator.rb   |  6 +++-
 app/models/spree/variant_decorator.rb   |  3 ++
 app/views/shop/shop/products.rabl       | 20 +++++------
 spec/models/spree/product_spec.rb       | 45 +++++++++++++++++++++++++
 6 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/app/controllers/shop/shop_controller.rb b/app/controllers/shop/shop_controller.rb
index e0862d205e..f8a799154d 100644
--- a/app/controllers/shop/shop_controller.rb
+++ b/app/controllers/shop/shop_controller.rb
@@ -14,6 +14,7 @@ class Shop::ShopController < BaseController
       .products_distributed_by(@distributor).andand
       .select(&:has_stock?).andand
       .sort_by {|p| p.name }
+
       render json: "", status: 404 
     end
   end
diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb
index 519e693b1e..48646fc42a 100644
--- a/app/models/order_cycle.rb
+++ b/app/models/order_cycle.rb
@@ -94,8 +94,11 @@ class OrderCycle < ActiveRecord::Base
   end
 
   def variants_distributed_by(distributor)
-    self.exchanges.where(:sender_id => self.coordinator, :receiver_id => distributor).
-      map(&:variants).flatten.uniq
+    Spree::Variant.
+      joins(:exchanges).
+      merge(Exchange.outgoing).
+      where('exchanges.order_cycle_id = ?', self).
+      where('exchanges.receiver_id = ?', distributor)
   end
 
   def products_distributed_by(distributor)
@@ -145,6 +148,11 @@ class OrderCycle < ActiveRecord::Base
 
 
   # -- Fees
+
+  # TODO: The boundary of this class is ill-defined here. OrderCycle should not know about
+  # EnterpriseFeeApplicator. Clients should be able to query it for relevant EnterpriseFees.
+  # This logic should be in another service object.
+
   def fees_for(variant, distributor)
     per_item_enterprise_fee_applicators_for(variant, distributor).sum do |applicator|
       # Spree's Calculator interface accepts Orders or LineItems,
diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 33ccf73a81..2d111e9a36 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -99,7 +99,11 @@ Spree::Product.class_eval do
   def product_distribution_for(distributor)
     self.product_distributions.find_by_distributor_id(distributor)
   end
-  
+
+  def variants_for(order_cycle, distributor)
+    self.variants_including_master.where('spree_variants.id IN (?)', order_cycle.variants_distributed_by(distributor))
+  end
+
   # overriding to check self.on_demand as well
   def has_stock?
     has_variants? ? variants.any?(&:in_stock?) : (on_demand || master.in_stock?)
diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb
index 12a4ca1a0e..00ad88e47b 100644
--- a/app/models/spree/variant_decorator.rb
+++ b/app/models/spree/variant_decorator.rb
@@ -1,4 +1,7 @@
 Spree::Variant.class_eval do
+  has_many :exchange_variants
+  has_many :exchanges, through: :exchange_variants
+
   attr_accessible :unit_value, :unit_description
 
   validates_presence_of :unit_value,
diff --git a/app/views/shop/shop/products.rabl b/app/views/shop/shop/products.rabl
index 7d5f1711fe..6d5cfa3129 100644
--- a/app/views/shop/shop/products.rabl
+++ b/app/views/shop/shop/products.rabl
@@ -22,19 +22,17 @@ child :master => :master do
   end
 end
 
-child :variants => :variants do |variant|
-  attributes :id, :is_master, :count_on_hand, :options_text, :count_on_hand, :on_demand, :group_buy
-  node do |variant|
-    {
-      price: variant.price_with_fees(current_distributor, current_order_cycle)  
+node :variants do |product|
+  product.variants_for(current_order_cycle, current_distributor).map do |v|
+    {id: v.id,
+     is_master: v.is_master,
+     count_on_hand: v.count_on_hand,
+     options_text: v.options_text,
+     on_demand: v.on_demand,
+     price: v.price_with_fees(current_distributor, current_order_cycle),
+     images: v.images.map { |i| {id: i.id, alt: i.alt, small_url: i.attachment.url(:small, false)} }
     }
   end
-  child :images => :images do
-    attributes :id, :alt
-    node do |img|
-      {:small_url => img.attachment.url(:small, false)}
-    end
-  end
 end
 
 child :taxons => :taxons do |taxon|
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 7e8f679f26..f4f5a9a1c2 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -303,6 +303,51 @@ module Spree
       end
     end
 
+    describe "finding variants for an order cycle and hub" do
+      it "returns variants in the order cycle and distributor" do
+        # Given a product and a distributor in an order cycle
+        oc = create(:order_cycle)
+        ex = oc.exchanges.outgoing.first
+        p = ex.variants.first.product
+        d = ex.receiver
+
+        p.variants_for(oc, d).should == [p.master]
+      end
+
+      it "does not return variants in the order cycle but not the distributor" do
+        oc = create(:simple_order_cycle)
+        s = create(:supplier_enterprise)
+        d1 = create(:distributor_enterprise)
+        d2 = create(:distributor_enterprise)
+
+        p1 = create(:simple_product)
+        p2 = create(:simple_product)
+
+        ex_in = create(:exchange, order_cycle: oc, sender: s, receiver: oc.coordinator,
+                       variants: [p1.master, p2.master])
+        ex_out1 = create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d1,
+                         variants: [p1.master])
+        ex_out1 = create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d2,
+                         variants: [p2.master])
+
+        p1.variants_for(oc, d1).should == [p1.master]
+        p1.variants_for(oc, d2).should be_empty
+        p2.variants_for(oc, d1).should be_empty
+        p2.variants_for(oc, d2).should == [p2.master]
+      end
+
+      it "does not return variants not in the order cycle" do
+        oc = create(:simple_order_cycle)
+        d = create(:distributor_enterprise)
+
+        p = create(:simple_product)
+        v = create(:variant, product: p)
+
+        p.variants_for(oc, d).should be_empty
+      end
+
+    end
+
     describe "variant units" do
       context "when the product initially has no variant unit" do
         let!(:p) { create(:simple_product,

From 823481215752cb982c3d6c71da89f9f967eee14a Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 28 Feb 2014 13:30:13 +1100
Subject: [PATCH 04/12] Do not include master when outputting variants for oc/d

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

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 2d111e9a36..69ff8a664b 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -101,7 +101,7 @@ Spree::Product.class_eval do
   end
 
   def variants_for(order_cycle, distributor)
-    self.variants_including_master.where('spree_variants.id IN (?)', order_cycle.variants_distributed_by(distributor))
+    self.variants.where('spree_variants.id IN (?)', order_cycle.variants_distributed_by(distributor))
   end
 
   # overriding to check self.on_demand as well
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index f4f5a9a1c2..d8e2e387a5 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -304,48 +304,39 @@ module Spree
     end
 
     describe "finding variants for an order cycle and hub" do
-      it "returns variants in the order cycle and distributor" do
-        # Given a product and a distributor in an order cycle
-        oc = create(:order_cycle)
-        ex = oc.exchanges.outgoing.first
-        p = ex.variants.first.product
-        d = ex.receiver
+      let(:oc) { create(:simple_order_cycle) }
+      let(:s) { create(:supplier_enterprise) }
+      let(:d1) { create(:distributor_enterprise) }
+      let(:d2) { create(:distributor_enterprise) }
 
-        p.variants_for(oc, d).should == [p.master]
+      let(:p1) { create(:simple_product) }
+      let(:p2) { create(:simple_product) }
+      let(:v1) { create(:variant, product: p1) }
+      let(:v2) { create(:variant, product: p2) }
+
+      let(:p_external) { create(:simple_product) }
+      let(:v_external) { create(:variant, product: p_external) }
+
+      let!(:ex_in) { create(:exchange, order_cycle: oc, sender: s, receiver: oc.coordinator,
+                           variants: [v1, v2]) }
+      let!(:ex_out1) { create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d1,
+                             variants: [v1]) }
+      let!(:ex_out2) { create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d2,
+                             variants: [v2]) }
+
+      it "returns variants in the order cycle and distributor" do
+        p1.variants_for(oc, d1).should == [v1]
+        p2.variants_for(oc, d2).should == [v2]
       end
 
       it "does not return variants in the order cycle but not the distributor" do
-        oc = create(:simple_order_cycle)
-        s = create(:supplier_enterprise)
-        d1 = create(:distributor_enterprise)
-        d2 = create(:distributor_enterprise)
-
-        p1 = create(:simple_product)
-        p2 = create(:simple_product)
-
-        ex_in = create(:exchange, order_cycle: oc, sender: s, receiver: oc.coordinator,
-                       variants: [p1.master, p2.master])
-        ex_out1 = create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d1,
-                         variants: [p1.master])
-        ex_out1 = create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d2,
-                         variants: [p2.master])
-
-        p1.variants_for(oc, d1).should == [p1.master]
         p1.variants_for(oc, d2).should be_empty
         p2.variants_for(oc, d1).should be_empty
-        p2.variants_for(oc, d2).should == [p2.master]
       end
 
       it "does not return variants not in the order cycle" do
-        oc = create(:simple_order_cycle)
-        d = create(:distributor_enterprise)
-
-        p = create(:simple_product)
-        v = create(:variant, product: p)
-
-        p.variants_for(oc, d).should be_empty
+        p_external.variants_for(oc, d1).should be_empty
       end
-
     end
 
     describe "variant units" do

From 17debd9fad33a52ba715099c0252947ddc2d373a Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Wed, 5 Mar 2014 11:25:07 +1100
Subject: [PATCH 05/12] On product list view, do not show variants that are out
 of stock

---
 app/models/spree/variant_decorator.rb         |  2 ++
 app/views/shop/shop/_products.html.haml       |  2 +-
 app/views/shop/shop/products.rabl             |  2 +-
 .../consumer/shopping/shopping_spec.rb        | 25 +++++++++++++++----
 spec/models/spree/variant_spec.rb             | 19 ++++++++++++++
 5 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb
index 00ad88e47b..923dce4e11 100644
--- a/app/models/spree/variant_decorator.rb
+++ b/app/models/spree/variant_decorator.rb
@@ -14,6 +14,8 @@ Spree::Variant.class_eval do
 
   after_save :update_units
 
+  scope :in_stock, where('spree_variants.count_on_hand > 0 OR spree_variants.on_demand=?', true)
+
 
   def price_with_fees(distributor, order_cycle)
     price + fees_for(distributor, order_cycle)
diff --git a/app/views/shop/shop/_products.html.haml b/app/views/shop/shop/_products.html.haml
index 3456ec29e7..500a918ad8 100644
--- a/app/views/shop/shop/_products.html.haml
+++ b/app/views/shop/shop/_products.html.haml
@@ -12,7 +12,7 @@
         %th.bulk Bulk
         %th.price.text-right Price
       %tbody{"ng-repeat" => "product in data.products | filter:query"}
-        %tr.product
+        %tr{"class" => "product product-{{ product.id }}"}
           %td.name
             %img{"ng-src" => "{{ product.master.images[0].small_url }}"}
             %div
diff --git a/app/views/shop/shop/products.rabl b/app/views/shop/shop/products.rabl
index 6d5cfa3129..662196953e 100644
--- a/app/views/shop/shop/products.rabl
+++ b/app/views/shop/shop/products.rabl
@@ -23,7 +23,7 @@ child :master => :master do
 end
 
 node :variants do |product|
-  product.variants_for(current_order_cycle, current_distributor).map do |v|
+  product.variants_for(current_order_cycle, current_distributor).in_stock.map do |v|
     {id: v.id,
      is_master: v.is_master,
      count_on_hand: v.count_on_hand,
diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb
index b7e085db10..5b94b9ff6a 100644
--- a/spec/features/consumer/shopping/shopping_spec.rb
+++ b/spec/features/consumer/shopping/shopping_spec.rb
@@ -12,6 +12,7 @@ feature "As a consumer I want to shop with a distributor", js: true do
       visit "/"
       click_link distributor.name
     end
+
     it "shows a distributor" do
       visit shop_path
       page.should have_text distributor.name
@@ -23,7 +24,7 @@ feature "As a consumer I want to shop with a distributor", js: true do
       first("#about img")['src'].should == distributor.promo_image.url(:large) 
     end
 
-    describe "With products in order cycles" do
+    describe "with products in order cycles" do
       let(:supplier) { create(:supplier_enterprise) }
       let(:product) { create(:product, supplier: supplier) }
       let(:order_cycle) { create(:order_cycle, distributors: [distributor], coordinator: create(:distributor_enterprise)) }
@@ -112,7 +113,7 @@ feature "As a consumer I want to shop with a distributor", js: true do
         end
       end
 
-      describe "After selecting an order cycle with products visible" do
+      describe "after selecting an order cycle with products visible" do
         let(:oc) { create(:simple_order_cycle, distributors: [distributor]) }
         let(:product) { create(:simple_product) }
         let(:variant) { create(:variant, product: product) }
@@ -154,13 +155,17 @@ feature "As a consumer I want to shop with a distributor", js: true do
         end
       end
 
-      describe "Filtering on hand and on demand products" do
+      describe "filtering on hand and on demand products" do
         let(:oc) { create(:simple_order_cycle, distributors: [distributor]) }
         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(:v1) { create(:variant, product: p4) }
+        let(:p5) { 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: p5) }
+        let(:v4) { create(:variant, product: p5) }
 
         before do
           p1.master.count_on_hand = 1
@@ -169,12 +174,17 @@ feature "As a consumer I want to shop with a distributor", js: true do
           p2.master.update_attribute(:count_on_hand, 0)
           p3.master.update_attribute(:count_on_hand, 0)
           v1.update_attribute(:count_on_hand, 1)
+          v2.update_attribute(:count_on_hand, 0)
+          v3.update_attribute(:count_on_hand, 1)
+          v4.update_attribute(:count_on_hand, 0)
           exchange = Exchange.find(oc.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 << v1 
+          exchange.variants << v1
+          exchange.variants << v2
+          exchange.variants << v4
           visit shop_path
           select "frogs", :from => "order_cycle_id"
           exchange
@@ -190,6 +200,11 @@ feature "As a consumer I want to shop with a distributor", js: true do
         it "does not show products that are neither on hand or on demand" do
           page.should_not have_content p3.name
         end
+
+        it "does not show variants that are neither on hand or on demand" do
+          within(".product-#{p4.id}") { find(".expand", visible: true).trigger "click" }
+          page.should_not have_content v2.options_text
+        end
       end
 
       describe "group buy products" do
diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb
index 2031a4d22b..fee4dc1edd 100644
--- a/spec/models/spree/variant_spec.rb
+++ b/spec/models/spree/variant_spec.rb
@@ -2,6 +2,25 @@ require 'spec_helper'
 
 module Spree
   describe Variant do
+    describe "scopes" do
+      describe "finding variants in stock" do
+        before do
+          p = create(:product)
+          @v_in_stock = create(:variant, product: p)
+          @v_on_demand = create(:variant, product: p, on_demand: true)
+          @v_no_stock = create(:variant, product: p)
+
+          @v_in_stock.update_attribute(:count_on_hand, 1)
+          @v_on_demand.update_attribute(:count_on_hand, 0)
+          @v_no_stock.update_attribute(:count_on_hand, 0)
+        end
+
+        it "returns variants in stock or on demand, but not those that are neither" do
+          Variant.where(is_master: false).in_stock.should == [@v_in_stock, @v_on_demand]
+        end
+      end
+    end
+
     describe "calculating the price with enterprise fees" do
       it "returns the price plus the fees" do
         distributor = double(:distributor)

From 637ccc113bd38c5e668e4f66198869e17ee679a0 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Wed, 5 Mar 2014 13:25:40 +1100
Subject: [PATCH 06/12] Do not show products that have no stock available to
 the current distribution

---
 app/controllers/shop/shop_controller.rb       |  6 +-
 app/models/order_cycle.rb                     |  2 +-
 app/models/spree/product_decorator.rb         | 11 ++++
 .../consumer/shopping/shopping_spec.rb        | 25 +++++++--
 spec/models/spree/product_spec.rb             | 55 ++++++++++++++++++-
 5 files changed, 90 insertions(+), 9 deletions(-)

diff --git a/app/controllers/shop/shop_controller.rb b/app/controllers/shop/shop_controller.rb
index f8a799154d..b7f124acfa 100644
--- a/app/controllers/shop/shop_controller.rb
+++ b/app/controllers/shop/shop_controller.rb
@@ -11,11 +11,11 @@ class Shop::ShopController < BaseController
   
   def products
     unless @products = current_order_cycle.andand
-      .products_distributed_by(@distributor).andand
-      .select(&:has_stock?).andand
+      .products_distributed_by(current_distributor).andand
+      .select { |p| p.has_stock_for_distribution?(current_order_cycle, current_distributor) }.andand
       .sort_by {|p| p.name }
 
-      render json: "", status: 404 
+      render json: "", status: 404
     end
   end
 
diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb
index 48646fc42a..1f98e8f7c4 100644
--- a/app/models/order_cycle.rb
+++ b/app/models/order_cycle.rb
@@ -151,7 +151,7 @@ class OrderCycle < ActiveRecord::Base
 
   # TODO: The boundary of this class is ill-defined here. OrderCycle should not know about
   # EnterpriseFeeApplicator. Clients should be able to query it for relevant EnterpriseFees.
-  # This logic should be in another service object.
+  # This logic would fit better in another service object.
 
   def fees_for(variant, distributor)
     per_item_enterprise_fee_applicators_for(variant, distributor).sum do |applicator|
diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 69ff8a664b..7682720708 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -109,6 +109,17 @@ Spree::Product.class_eval do
     has_variants? ? variants.any?(&:in_stock?) : (on_demand || master.in_stock?)
   end
 
+  def has_stock_for_distribution?(order_cycle, distributor)
+    # This product has stock for a distribution if it is available on-demand
+    # or if one of its variants in the distribution is in stock
+    (!has_variants? && on_demand) ||
+      variants_distributed_by(order_cycle, distributor).any? { |v| v.in_stock? }
+  end
+
+  def variants_distributed_by(order_cycle, distributor)
+    order_cycle.variants_distributed_by(distributor).where(product_id: self)
+  end
+
   # Build a product distribution for each distributor
   def build_product_distributions_for_user user
     Enterprise.is_distributor.managed_by(user).each do |distributor|
diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb
index 5b94b9ff6a..6c201bb92f 100644
--- a/spec/features/consumer/shopping/shopping_spec.rb
+++ b/spec/features/consumer/shopping/shopping_spec.rb
@@ -164,8 +164,9 @@ feature "As a consumer I want to shop with a distributor", js: true do
         let(:p5) { 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: p5) }
+        let(:v3) { create(:variant, product: p4, unit_value: 4, on_demand: true) }
         let(:v4) { create(:variant, product: p5) }
+        let(:v5) { create(:variant, product: p5) }
 
         before do
           p1.master.count_on_hand = 1
@@ -175,8 +176,9 @@ feature "As a consumer I want to shop with a distributor", js: true do
           p3.master.update_attribute(:count_on_hand, 0)
           v1.update_attribute(:count_on_hand, 1)
           v2.update_attribute(:count_on_hand, 0)
-          v3.update_attribute(:count_on_hand, 1)
-          v4.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)
           exchange = Exchange.find(oc.exchanges.to_enterprises(distributor).outgoing.first.id) 
           exchange.update_attribute :pickup_time, "frogs" 
           exchange.variants << p1.master
@@ -184,7 +186,11 @@ feature "As a consumer I want to shop with a distributor", js: true do
           exchange.variants << p3.master
           exchange.variants << v1
           exchange.variants << v2
-          exchange.variants << v4
+          exchange.variants << v3
+          # v4 is in stock but not in distribution
+          # v5 is out of stock and in the distribution
+          # Neither should display, nor should their product, p5
+          exchange.variants << v5
           visit shop_path
           select "frogs", :from => "order_cycle_id"
           exchange
@@ -194,17 +200,28 @@ feature "As a consumer I want to shop with a distributor", js: true do
           page.should have_content p1.name
           page.should have_content p4.name
         end
+
         it "shows on demand products" do
           page.should have_content p2.name
         end
+
         it "does not show products that are neither on hand or on demand" do
           page.should_not have_content p3.name
         end
 
+        it "shows on demand variants" do
+          within(".product-#{p4.id}") { find(".expand", visible: true).trigger "click" }
+          page.should have_content v3.options_text
+        end
+
         it "does not show variants that are neither on hand or on demand" do
           within(".product-#{p4.id}") { find(".expand", visible: true).trigger "click" }
           page.should_not have_content v2.options_text
         end
+
+        it "does not show products that have no available variants in this distribution" do
+          page.should_not have_content p5.name
+        end
       end
 
       describe "group buy products" do
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index d8e2e387a5..d42f811422 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -477,12 +477,65 @@ module Spree
       end
     end
 
-    describe "Stock filtering" do
+    describe "stock filtering" do
       it "considers products that are on_demand as being in stock" do
         product = create(:simple_product, on_demand: true)
         product.master.update_attribute(:count_on_hand, 0)
         product.has_stock?.should == true
       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)
+          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 products with in-stock variants" do
+          p = create(:simple_product)
+          v = create(:variant, product: p)
+          v.update_attribute(:count_on_hand, 1)
+          d = create(:distributor_enterprise)
+          oc = create(:simple_order_cycle, distributors: [d])
+          oc.exchanges.outgoing.first.variants << v
+
+          p.should have_stock_for_distribution(oc, d)
+        end
+
+        it "returns products with on-demand variants" do
+          p = create(:simple_product)
+          v = create(:variant, product: p, on_demand: true)
+          v.update_attribute(:count_on_hand, 0)
+          d = create(:distributor_enterprise)
+          oc = create(:simple_order_cycle, distributors: [d])
+          oc.exchanges.outgoing.first.variants << v
+
+          p.should have_stock_for_distribution(oc, d)
+        end
+
+        it "does not return products that have stock not in the distribution" do
+          p = create(:simple_product)
+          p.master.update_attribute(:count_on_hand, 1)
+          d = create(:distributor_enterprise)
+          oc = create(:simple_order_cycle, distributors: [d])
+
+          p.should_not have_stock_for_distribution(oc, d)
+        end
+      end
     end
   end
 end

From 07d979b38ff6493d24a64425aed310aa1b56eda6 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Wed, 5 Mar 2014 14:07:07 +1100
Subject: [PATCH 07/12] Collapse specs into one, halves spec runtime

---
 .../consumer/shopping/shopping_spec.rb        | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb
index 6c201bb92f..75a512ecf9 100644
--- a/spec/features/consumer/shopping/shopping_spec.rb
+++ b/spec/features/consumer/shopping/shopping_spec.rb
@@ -196,30 +196,25 @@ feature "As a consumer I want to shop with a distributor", js: true do
           exchange
         end
 
-        it "shows on hand products" do
+        it "filters products based on availability" do
+          # It shows on hand products
           page.should have_content p1.name
           page.should have_content p4.name
-        end
 
-        it "shows on demand products" do
+          # It shows on demand products
           page.should have_content p2.name
-        end
 
-        it "does not show products that are neither on hand or on demand" do
+          # It does not show products that are neither on hand or on demand
           page.should_not have_content p3.name
-        end
 
-        it "shows on demand variants" do
+          # It shows on demand variants
           within(".product-#{p4.id}") { find(".expand", visible: true).trigger "click" }
           page.should have_content v3.options_text
-        end
 
-        it "does not show variants that are neither on hand or on demand" do
-          within(".product-#{p4.id}") { find(".expand", visible: true).trigger "click" }
+          # It does not show variants that are neither on hand or on demand
           page.should_not have_content v2.options_text
-        end
 
-        it "does not show products that have no available variants in this distribution" do
+          # It does not show products that have no available variants in this distribution
           page.should_not have_content p5.name
         end
       end

From b89c84e8bdaeada9c9626708927c1e55eb41a67f Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Wed, 5 Mar 2014 15:08:54 +1100
Subject: [PATCH 08/12] BPE: Do not show product price when product has
 variants

---
 app/views/spree/admin/products/bulk_edit.html.haml |  2 +-
 spec/features/admin/bulk_product_update_spec.rb    | 13 ++++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/app/views/spree/admin/products/bulk_edit.html.haml b/app/views/spree/admin/products/bulk_edit.html.haml
index 28cefe564f..faac29fa79 100644
--- a/app/views/spree/admin/products/bulk_edit.html.haml
+++ b/app/views/spree/admin/products/bulk_edit.html.haml
@@ -122,7 +122,7 @@
             %input{ 'ng-model' => 'product.master.unit_value_with_description', :name => 'master_unit_value_with_description', 'ofn-track-product' => 'master.unit_value_with_description', :type => 'text', :placeholder => 'value', 'ng-show' => "!hasVariants(product) && hasUnit(product)" }
             %input{ 'ng-model' => 'product.variant_unit_name', :name => 'variant_unit_name', 'ofn-track-product' => 'variant_unit_name', :placeholder => 'unit', 'ng-show' => "product.variant_unit_with_scale == 'items'", :type => 'text' }
           %td{ 'ng-show' => 'columns.price.visible' }
-            %input{ 'ng-model' => 'product.price', 'ofn-decimal' => :true, :name => 'price', 'ofn-track-product' => 'price', :type => 'text' }
+            %input{ 'ng-model' => 'product.price', 'ofn-decimal' => :true, :name => 'price', 'ofn-track-product' => 'price', :type => 'text', 'ng-hide' => 'hasVariants(product)' }
           %td{ 'ng-show' => 'columns.on_hand.visible' }
             %span{ 'ng-bind' => 'product.on_hand', :name => 'on_hand', 'ng-show' => '!hasOnDemandVariants(product) && (hasVariants(product) || product.on_demand)' }
             %input.field{ 'ng-model' => 'product.on_hand', :name => 'on_hand', 'ofn-track-product' => 'on_hand', 'ng-hide' => 'hasVariants(product) || product.on_demand', :type => 'number' }
diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb
index 4644aae8c1..336792a3ae 100644
--- a/spec/features/admin/bulk_product_update_spec.rb
+++ b/spec/features/admin/bulk_product_update_spec.rb
@@ -85,18 +85,21 @@ feature %q{
       page.should have_field "available_on", with: p2.available_on.strftime("%F %T")
     end
 
-    it "displays a price input for each product (ie. for master variant)" do
+    it "displays a price input for each product without variants (ie. for master variant)" do
       p1 = FactoryGirl.create(:product)
       p2 = FactoryGirl.create(:product)
-      p1.price = 22.00
-      p2.price = 44.00
-      p1.save!
-      p2.save!
+      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
 
       visit '/admin/products/bulk_edit'
 
       page.should have_field "price", with: "22.0"
       page.should have_field "price", with: "44.0"
+      page.should_not have_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

From 922d817748257111fad26afe6ba49101c0fdea80 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Wed, 5 Mar 2014 16:16:42 +1100
Subject: [PATCH 09/12] Product list view: For product with variants, show
 product price as min(variant prices)

---
 .../controllers/products_controller.js.coffee |  7 +++++
 app/views/shop/shop/_products.html.haml       |  4 +--
 .../consumer/shopping/shopping_spec.rb        | 29 ++++++++++++++-----
 .../unit/darkswarm/controllers_spec.js.coffee | 28 +++++++++++-------
 4 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee
index f1fdb224b0..784aed539c 100644
--- a/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee
+++ b/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee
@@ -2,3 +2,10 @@ angular.module("Shop").controller "ProductsCtrl", ($scope, $rootScope, Product,
   $scope.data = Product.data
   $scope.order_cycle = OrderCycle.order_cycle
   Product.update()
+
+  $scope.productPrice = (product) ->
+    if product.variants.length > 0
+      prices = (v.price for v in product.variants)
+      Math.min.apply(null, prices)
+    else
+      product.price
diff --git a/app/views/shop/shop/_products.html.haml b/app/views/shop/shop/_products.html.haml
index 500a918ad8..434ba9718c 100644
--- a/app/views/shop/shop/_products.html.haml
+++ b/app/views/shop/shop/_products.html.haml
@@ -46,9 +46,9 @@
               name: "variant_attributes[{{product.master.id}}][max_quantity]"} 
           %td.price.text-right
             %small{"ng-show" => "(product.variants.length > 0)"} from
-            {{ product.price | currency }}
+            {{ productPrice(product) | currency }}
         %tr.product-description
           %td{colspan: 2}{{ product.notes | truncate:80 }}
-        %tr{"ng-repeat" => "variant in product.variants", "ng-show" => "product.show_variants"}
+        %tr.variant{"ng-repeat" => "variant in product.variants", "ng-show" => "product.show_variants"}
           = render partial: "shop/shop/variant"
     %input.button.right{type: :submit, value: "Check Out"}
diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb
index 75a512ecf9..4476f69571 100644
--- a/spec/features/consumer/shopping/shopping_spec.rb
+++ b/spec/features/consumer/shopping/shopping_spec.rb
@@ -115,14 +115,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(:oc) { create(:simple_order_cycle, distributors: [distributor]) }
-        let(:product) { create(:simple_product) }
-        let(:variant) { create(:variant, product: product) }
+        let(:product) { create(:simple_product, price: 10) }
+        let(:variant1) { create(:variant, product: product, price: 20) }
+        let(:variant2) { create(:variant, product: product, price: 30) }
         let(:exchange) { Exchange.find(oc.exchanges.to_enterprises(distributor).outgoing.first.id) } 
 
         before do
           exchange.update_attribute :pickup_time, "frogs" 
           exchange.variants << product.master
-          exchange.variants << variant
+          exchange.variants << variant1
+          exchange.variants << variant2
           visit shop_path
           select "frogs", :from => "order_cycle_id"
           exchange
@@ -133,14 +135,14 @@ feature "As a consumer I want to shop with a distributor", js: true do
         end
 
         it "collapses variants by default" do
-          page.should_not have_text variant.options_text
+          page.should_not have_text variant1.options_text
         end
 
         it "expands variants" do
           find(".expand").trigger "click"
-          page.should have_text variant.options_text
+          page.should have_text variant1.options_text
           find(".collapse").trigger "click"
-          page.should_not have_text variant.options_text
+          page.should_not have_text variant1.options_text
         end
 
         it "uses the adjusted price" do
@@ -151,7 +153,20 @@ feature "As a consumer I want to shop with a distributor", js: true do
 
           visit shop_path
           select "frogs", :from => "order_cycle_id"
-          page.should have_content "$#{(product.price + 23.00)}"
+
+          # All prices are as above plus $23 in fees
+
+          # Page should not have product.price (with or without fee)
+          page.should_not have_selector 'tr.product > td', text: "from $10.00"
+          page.should_not have_selector 'tr.product > td', text: "from $33.00"
+
+          # Page should have variant prices (with fee)
+          find(".expand").trigger 'click'
+          page.should have_selector 'tr.variant > td.price', text: "$43.00"
+          page.should have_selector 'tr.variant > td.price', text: "$53.00"
+
+          # Product price should be listed as the lesser of these
+          page.should have_selector 'tr.product > td', text: "from $43.00"
         end
       end
 
diff --git a/spec/javascripts/unit/darkswarm/controllers_spec.js.coffee b/spec/javascripts/unit/darkswarm/controllers_spec.js.coffee
index 0ea7488225..f7717b6737 100644
--- a/spec/javascripts/unit/darkswarm/controllers_spec.js.coffee
+++ b/spec/javascripts/unit/darkswarm/controllers_spec.js.coffee
@@ -3,7 +3,6 @@ describe 'All controllers', ->
     ctrl = null
     scope = null
     event = null
-    rootScope = null
     Product = null
 
     beforeEach ->
@@ -12,29 +11,38 @@ describe 'All controllers', ->
         all: ->
         update: ->
         data: "testy mctest"
+      OrderCycle =
+        order_cycle: {}
           
-      inject ($controller, $rootScope) ->
-        rootScope = $rootScope
-        scope = $rootScope.$new()
-        ctrl = $controller 'ProductsCtrl', {$scope: scope, Product : Product} 
+      inject ($controller) ->
+        scope = {}
+        ctrl = $controller 'ProductsCtrl', {$scope: scope, Product: Product, OrderCycle: OrderCycle}
 
-    it 'Fetches products from Product', ->
+    it 'fetches products from Product', ->
       expect(scope.data).toEqual 'testy mctest'
 
+    describe "determining the price to display for a product", ->
+      it "displays the product price when the product does not have variants", ->
+        product = {variants: [], price: 12.34}
+        expect(scope.productPrice(product)).toEqual 12.34
+
+      it "displays the minimum variant price when the product has variants", ->
+        product =
+          price: 11
+          variants: [{price: 22}, {price: 33}]
+        expect(scope.productPrice(product)).toEqual 22
   
   describe 'OrderCycleCtrl', ->
     ctrl = null
     scope = null
     event = null
-    rootScope = null
     product_ctrl = null
     OrderCycle = null
 
     beforeEach ->
       module 'Shop'
       scope = {}
-      inject ($controller, $rootScope) ->
-        rootScope = $rootScope
-        scope = $rootScope.$new()
+      inject ($controller) ->
+        scope = {}
         ctrl = $controller 'OrderCycleCtrl', {$scope: scope}
 

From 07caf9948894d182831be6beb582b72a17ef6eef Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Wed, 5 Mar 2014 16:52:06 +1100
Subject: [PATCH 10/12] Enterprise user can delete product properties

---
 app/models/spree/ability_decorator.rb |  2 +-
 spec/features/admin/variants_spec.rb  | 30 +++++++++++++++++++++++++++
 spec/models/spree/ability_spec.rb     |  2 +-
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb
index c6407c1f98..668791f555 100644
--- a/app/models/spree/ability_decorator.rb
+++ b/app/models/spree/ability_decorator.rb
@@ -15,7 +15,7 @@ class AbilityDecorator
       end
 
       can [:admin, :index, :read, :create, :edit, :update, :search, :destroy], Spree::Variant
-      can [:admin, :index, :read, :create, :edit], Spree::ProductProperty
+      can [:admin, :index, :read, :create, :edit, :destroy], Spree::ProductProperty
       can [:admin, :index, :read, :create, :edit], Spree::Image
 
       can [:admin, :index, :read, :search], Spree::Taxon
diff --git a/spec/features/admin/variants_spec.rb b/spec/features/admin/variants_spec.rb
index d05aa75ef8..7202867bd3 100644
--- a/spec/features/admin/variants_spec.rb
+++ b/spec/features/admin/variants_spec.rb
@@ -83,4 +83,34 @@ feature %q{
     page.should_not have_field "variant_unit_value"
     page.should_not have_field "variant_unit_description"
   end
+
+
+  context "as an enterprise user" do
+    before(:each) do
+      @new_user = create_enterprise_user
+      @supplier = create(:supplier_enterprise)
+      @new_user.enterprise_roles.build(enterprise: @supplier).save
+
+      login_to_admin_as @new_user
+    end
+
+    scenario "deleting product properties", js: true do
+      # Given a product with a property
+      p = create(:simple_product, supplier: @supplier)
+      p.set_property('fooprop', 'fooval')
+
+      # When I navigate to the product properties page
+      visit spree.admin_product_product_properties_path(p)
+      page.should have_field 'product_product_properties_attributes_0_property_name', with: 'fooprop', visible: true
+      page.should have_field 'product_product_properties_attributes_0_value', with: 'fooval', visible: true
+
+      # And I delete the property
+      page.all('a.remove_fields').first.click
+
+      # Then the property should have been deleted
+      page.should_not have_field 'product_product_properties_attributes_0_property_name', with: 'fooprop', visible: true
+      page.should_not have_field 'product_product_properties_attributes_0_value', with: 'fooval', visible: true
+      p.reload.property('fooprop').should be_nil
+    end
+  end
 end
diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb
index 35020aede5..193a6c0fd5 100644
--- a/spec/models/spree/ability_spec.rb
+++ b/spec/models/spree/ability_spec.rb
@@ -48,7 +48,7 @@ module Spree
         end
 
         it "should be able to read/write their enterprises' product properties" do
-          should have_ability([:admin, :index, :read, :create, :edit], for: Spree::ProductProperty)
+          should have_ability([:admin, :index, :read, :create, :edit, :destroy], for: Spree::ProductProperty)
         end
 
         it "should be able to read/write their enterprises' product images" do

From 24d97bd754df5dd83c1be560aee52b42059c3a39 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 6 Mar 2014 10:36:49 +1100
Subject: [PATCH 11/12] Fix spec race condition

---
 spec/features/admin/variants_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/features/admin/variants_spec.rb b/spec/features/admin/variants_spec.rb
index 7202867bd3..51aefd9e69 100644
--- a/spec/features/admin/variants_spec.rb
+++ b/spec/features/admin/variants_spec.rb
@@ -106,11 +106,11 @@ feature %q{
 
       # And I delete the property
       page.all('a.remove_fields').first.click
+      wait_until { p.reload.property('fooprop').nil? }
 
       # Then the property should have been deleted
       page.should_not have_field 'product_product_properties_attributes_0_property_name', with: 'fooprop', visible: true
       page.should_not have_field 'product_product_properties_attributes_0_value', with: 'fooval', visible: true
-      p.reload.property('fooprop').should be_nil
     end
   end
 end

From 068a6ebd95c56fce097dcc08eda3ada24a1c2eff Mon Sep 17 00:00:00 2001
From: Rob H <oeoeaio@gmail.com>
Date: Thu, 6 Mar 2014 12:49:29 +1100
Subject: [PATCH 12/12] Fix checkout specs

---
 spec/features/consumer/shopping/checkout_spec.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb
index 6c33dde5d5..8393543031 100644
--- a/spec/features/consumer/shopping/checkout_spec.rb
+++ b/spec/features/consumer/shopping/checkout_spec.rb
@@ -224,7 +224,6 @@ def select_order_cycle
 end
 
 def add_product_to_cart
-
-  fill_in "variants[#{product.master.id}]", with: 5
+  fill_in "variants[#{product.master.id}]", with: product.master.on_hand - 1
   first("form.custom > input.button.right").click 
 end