From b718cf729a28171df3700dea739155c521350b3d Mon Sep 17 00:00:00 2001
From: Paul Mackay <pauljmackay@gmail.com>
Date: Wed, 1 Jun 2016 20:51:31 +0100
Subject: [PATCH 01/35] Set production log level to warn

---
 config/environments/production.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/environments/production.rb b/config/environments/production.rb
index a55540883d..819bb98306 100644
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@@ -31,7 +31,7 @@ Openfoodnetwork::Application.configure do
   config.force_ssl = true
 
   # See everything in the log (default is :info)
-  # config.log_level = :debug
+  config.log_level = :warn
 
   # Use a different logger for distributed setups
   # config.logger = SyslogLogger.new

From fb33be78dd79d424f15b4b6900ceae18b9892854 Mon Sep 17 00:00:00 2001
From: David Ajnered <david@bilprospekt.se>
Date: Fri, 10 Jun 2016 15:42:40 +0200
Subject: [PATCH 02/35] #591 show selected hub dropdown on customers page and
 enable easy switch to different hub

---
 app/views/admin/customers/index.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/admin/customers/index.html.haml b/app/views/admin/customers/index.html.haml
index c570632c64..7e82536526 100644
--- a/app/views/admin/customers/index.html.haml
+++ b/app/views/admin/customers/index.html.haml
@@ -14,7 +14,7 @@
 = admin_inject_shops
 
 %div{ ng: { controller: 'customersCtrl' } }
-  .row{ ng: { hide: "!RequestMonitor.loading && customers.length > 0" } }
+  .row
     .five.columns.alpha
       %h3
         =t :please_select_hub

From baa6fda3e02f21761084c68893fc9ad5cd596d5a Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Wed, 4 May 2016 15:22:52 +1000
Subject: [PATCH 03/35] Use save bar in create order cycle page

---
 .../admin/order_cycles/controllers/create.js.coffee      | 9 +++++++--
 .../order_cycles/controllers/simple_create.js.coffee     | 7 +++++--
 app/views/admin/order_cycles/_form.html.haml             | 5 ++---
 app/views/admin/order_cycles/_simple_form.html.haml      | 5 ++---
 app/views/admin/order_cycles/new.html.haml               | 5 ++++-
 .../order_cycles/controllers/simple_create.js.coffee     | 3 ++-
 spec/javascripts/unit/order_cycle_spec.js.coffee         | 4 +++-
 7 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/app/assets/javascripts/admin/order_cycles/controllers/create.js.coffee b/app/assets/javascripts/admin/order_cycles/controllers/create.js.coffee
index 3f28b27f9f..5d33b895db 100644
--- a/app/assets/javascripts/admin/order_cycles/controllers/create.js.coffee
+++ b/app/assets/javascripts/admin/order_cycles/controllers/create.js.coffee
@@ -11,6 +11,9 @@ angular.module('admin.orderCycles')
 
     $scope.StatusMessage = StatusMessage
 
+    $scope.$watch 'order_cycle_form.$dirty', (newValue) ->
+      StatusMessage.display 'notice', 'You have unsaved changes' if newValue
+
     $scope.loaded = ->
       Enterprise.loaded && EnterpriseFee.loaded && OrderCycle.loaded
 
@@ -55,6 +58,7 @@ angular.module('admin.orderCycles')
     $scope.removeExchange = ($event, exchange) ->
       $event.preventDefault()
       OrderCycle.removeExchange(exchange)
+      $scope.order_cycle_form.$dirty = true
 
     $scope.addCoordinatorFee = ($event) ->
       $event.preventDefault()
@@ -75,6 +79,7 @@ angular.module('admin.orderCycles')
     $scope.removeDistributionOfVariant = (variant_id) ->
       OrderCycle.removeDistributionOfVariant(variant_id)
 
-    $scope.submit = ($event, destination) ->
-      $event.preventDefault()
+    $scope.submit = (destination) ->
+      StatusMessage.display 'progress', "Saving..."
       OrderCycle.create(destination)
+
diff --git a/app/assets/javascripts/admin/order_cycles/controllers/simple_create.js.coffee b/app/assets/javascripts/admin/order_cycles/controllers/simple_create.js.coffee
index dfa8011167..2839e74557 100644
--- a/app/assets/javascripts/admin/order_cycles/controllers/simple_create.js.coffee
+++ b/app/assets/javascripts/admin/order_cycles/controllers/simple_create.js.coffee
@@ -7,6 +7,9 @@ angular.module('admin.orderCycles').controller "AdminSimpleCreateOrderCycleCtrl"
       $scope.init(enterprises)
     $scope.enterprise_fees = EnterpriseFee.index(coordinator_id: ocInstance.coordinator_id)
 
+  $scope.$watch 'order_cycle_form.$dirty', (newValue) ->
+      StatusMessage.display 'notice', 'You have unsaved changes' if newValue
+
   $scope.init = (enterprises) ->
     enterprise = enterprises[Object.keys(enterprises)[0]]
     OrderCycle.addSupplier enterprise.id
@@ -41,7 +44,7 @@ angular.module('admin.orderCycles').controller "AdminSimpleCreateOrderCycleCtrl"
   $scope.enterpriseFeesForEnterprise = (enterprise_id) ->
     EnterpriseFee.forEnterprise(parseInt(enterprise_id))
 
-  $scope.submit = ($event, destination) ->
-    $event.preventDefault()
+  $scope.submit = (destination) ->
+    StatusMessage.display 'progress', "Saving..."
     OrderCycle.mirrorIncomingToOutgoingProducts()
     OrderCycle.create(destination)
diff --git a/app/views/admin/order_cycles/_form.html.haml b/app/views/admin/order_cycles/_form.html.haml
index a0f0a178c8..8eff9d85dd 100644
--- a/app/views/admin/order_cycles/_form.html.haml
+++ b/app/views/admin/order_cycles/_form.html.haml
@@ -46,9 +46,8 @@
   = render 'add_exchange_form', f: f, type: 'distributor'
 
 .actions
-  - if @order_cycle.new_record?
-    = f.submit 'Create', 'ng-click' => "submit($event, '#{main_app.admin_order_cycles_path}')", 'ng-disabled' => '!loaded()'
-
+  %span{'ng-show' => 'loaded()'}
+    = link_to 'Cancel', main_app.admin_order_cycles_path
   %span{'ng-hide' => 'loaded()'} Loading...
 
 
diff --git a/app/views/admin/order_cycles/_simple_form.html.haml b/app/views/admin/order_cycles/_simple_form.html.haml
index 6bb0034c73..7acbaf19e8 100644
--- a/app/views/admin/order_cycles/_simple_form.html.haml
+++ b/app/views/admin/order_cycles/_simple_form.html.haml
@@ -21,7 +21,6 @@
 = render 'coordinator_fees', f: f
 
 .actions
-  - if @order_cycle.new_record?
-    = f.submit 'Create', 'ng-click' => "submit($event, '#{main_app.admin_order_cycles_path}')", 'ng-disabled' => '!loaded()'
-
+  %span{'ng-show' => 'loaded()'}
+    = link_to 'Cancel', main_app.admin_order_cycles_path
   %span{'ng-hide' => 'loaded()'} Loading...
diff --git a/app/views/admin/order_cycles/new.html.haml b/app/views/admin/order_cycles/new.html.haml
index a257bf3c0b..7f55c28aed 100644
--- a/app/views/admin/order_cycles/new.html.haml
+++ b/app/views/admin/order_cycles/new.html.haml
@@ -4,7 +4,10 @@
 - ng_controller = order_cycles_simple_form ? 'AdminSimpleCreateOrderCycleCtrl' : 'AdminCreateOrderCycleCtrl'
 = admin_inject_order_cycle_instance
 
-= form_for [main_app, :admin, @order_cycle], :url => '', :html => {:class => 'ng order_cycle', 'ng-app' => 'admin.orderCycles', 'ng-controller' => ng_controller} do |f|
+= form_for [main_app, :admin, @order_cycle], :url => '', :html => {:class => 'ng order_cycle', 'ng-app' => 'admin.orderCycles', 'ng-controller' => ng_controller, name: 'order_cycle_form'} do |f|
+
+  %save-bar{ buttons: "[{ text: 'Create', action: submit, param: '#{main_app.admin_order_cycles_path}' }]", form: "order_cycle_form" }
+
   - if order_cycles_simple_form
     = render 'simple_form', f: f
   - else
diff --git a/spec/javascripts/unit/admin/order_cycles/controllers/simple_create.js.coffee b/spec/javascripts/unit/admin/order_cycles/controllers/simple_create.js.coffee
index 7404bbead2..70508bc519 100644
--- a/spec/javascripts/unit/admin/order_cycles/controllers/simple_create.js.coffee
+++ b/spec/javascripts/unit/admin/order_cycles/controllers/simple_create.js.coffee
@@ -8,7 +8,8 @@ describe "AdminSimpleCreateOrderCycleCtrl", ->
   outgoing_exchange = {}
 
   beforeEach ->
-    scope = {}
+    scope =
+      $watch: jasmine.createSpy('$watch')
     order_cycle =
       coordinator_id: 123
       incoming_exchanges: [incoming_exchange]
diff --git a/spec/javascripts/unit/order_cycle_spec.js.coffee b/spec/javascripts/unit/order_cycle_spec.js.coffee
index 4a924917c8..5fec8b93f5 100644
--- a/spec/javascripts/unit/order_cycle_spec.js.coffee
+++ b/spec/javascripts/unit/order_cycle_spec.js.coffee
@@ -9,7 +9,9 @@ describe 'OrderCycle controllers', ->
     EnterpriseFee = null
 
     beforeEach ->
-      scope = {}
+      scope =
+        order_cycle_form: jasmine.createSpyObj('order_cycle_form', ['$dirty'])
+        $watch: jasmine.createSpy('$watch')
       event =
         preventDefault: jasmine.createSpy('preventDefault')
       OrderCycle =

From 7994e2594a942d1d6bf696eccfb19ec98a3cf3ea Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Wed, 4 May 2016 15:46:25 +1000
Subject: [PATCH 04/35] Update create order cycle feature test

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

diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb
index 1fa85f06e7..e843e00235 100644
--- a/spec/features/admin/order_cycles_spec.rb
+++ b/spec/features/admin/order_cycles_spec.rb
@@ -139,7 +139,7 @@ feature %q{
     select 'Distributor fee', from: 'order_cycle_outgoing_exchange_0_enterprise_fees_0_enterprise_fee_id'
 
     # And I click Create
-    click_button 'Create'
+    find_button('Create').trigger('click')
 
     # Then my order cycle should have been created
     page.should have_content 'Your order cycle has been created.'
@@ -866,7 +866,7 @@ feature %q{
       page.should_not have_select 'order_cycle_coordinator_fee_1_id'
 
       select 'Coord fee', from: 'order_cycle_coordinator_fee_0_id'
-      click_button 'Create'
+      find_button('Create').trigger("click")
 
       # Then my order cycle should have been created
       page.should have_content 'Your order cycle has been created.'

From 2065d81bb4e3bd2abd5609e1bac2b8f98ada3f3e Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Thu, 12 May 2016 09:47:11 +1000
Subject: [PATCH 05/35] Use save bar directive in enterprise editing page

---
 .../controllers/enterprise_controller.js.coffee      | 12 +++++++++++-
 app/views/admin/enterprises/_ng_form.html.haml       |  7 +++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee
index a806691465..9374ed35ed 100644
--- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee
+++ b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee
@@ -1,5 +1,5 @@
 angular.module("admin.enterprises")
-  .controller "enterpriseCtrl", ($scope, NavigationCheck, enterprise, EnterprisePaymentMethods, EnterpriseShippingMethods, SideMenu) ->
+  .controller "enterpriseCtrl", ($scope, NavigationCheck, enterprise, EnterprisePaymentMethods, EnterpriseShippingMethods, SideMenu, StatusMessage) ->
     $scope.Enterprise = enterprise
     $scope.PaymentMethods = EnterprisePaymentMethods.paymentMethods
     $scope.ShippingMethods = EnterpriseShippingMethods.shippingMethods
@@ -8,6 +8,16 @@ angular.module("admin.enterprises")
     $scope.menu = SideMenu
     $scope.newManager = { id: '', email: (t('add_manager')) }
 
+    $scope.StatusMessage = StatusMessage
+
+    $scope.$watch 'enterprise_form.$dirty', (newValue) ->
+      console.log newValue
+      StatusMessage.display 'notice', 'You have unsaved changes' if newValue
+
+    $scope.setFormDirty = ->
+      $scope.$apply ->
+        $scope.enterprise_form.$setDirty()
+
     # Provide a callback for generating warning messages displayed before leaving the page. This is passed in
     # from a directive "nav-check" in the page - if we pass it here it will be called in the test suite,
     # and on all new uses of this contoller, and we might not want that .
diff --git a/app/views/admin/enterprises/_ng_form.html.haml b/app/views/admin/enterprises/_ng_form.html.haml
index a19bc43314..9e4becc158 100644
--- a/app/views/admin/enterprises/_ng_form.html.haml
+++ b/app/views/admin/enterprises/_ng_form.html.haml
@@ -1,12 +1,15 @@
 -# Not all inputs are ng inputs, they don't make the ng-form dirty on change.
 -# ng-change is only valid for inputs, not for a form.
 -# So we use onchange and have to get the scope to access the ng controller
-= form_for [main_app, :admin, @enterprise], html: { name: "enterprise",
+= form_for [main_app, :admin, @enterprise], html: { name: "enterprise_form",
   "ng-app" => 'admin.enterprises',
   "ng-submit" => "navClear()",
   "ng-controller" => 'enterpriseCtrl',
-  'onchange' => 'angular.element(enterprise).scope().enterprise.$setDirty()',
+  'onchange' => 'angular.element(enterprise_form).scope().setFormDirty()',
   } do |f|
+
+  %save-bar{ buttons: "[{ text: 'Update', action: submit, param: null }]", form: "enterprise_form" }
+
   .row
     .sixteen.columns.alpha
       .four.columns.alpha

From d8bf66a6c9a128d1752ca4bb011c5241290f8860 Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Fri, 13 May 2016 15:12:21 +1000
Subject: [PATCH 06/35] Use save bar on enterprise editing page

---
 .../controllers/enterprise_controller.js.coffee   | 11 +++++++++--
 app/views/admin/enterprises/_ng_form.html.haml    | 15 +++++++--------
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee
index 9374ed35ed..8b5f261436 100644
--- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee
+++ b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee
@@ -1,5 +1,5 @@
 angular.module("admin.enterprises")
-  .controller "enterpriseCtrl", ($scope, NavigationCheck, enterprise, EnterprisePaymentMethods, EnterpriseShippingMethods, SideMenu, StatusMessage) ->
+  .controller "enterpriseCtrl", ($scope, $window,NavigationCheck, enterprise, EnterprisePaymentMethods, EnterpriseShippingMethods, SideMenu, StatusMessage) ->
     $scope.Enterprise = enterprise
     $scope.PaymentMethods = EnterprisePaymentMethods.paymentMethods
     $scope.ShippingMethods = EnterpriseShippingMethods.shippingMethods
@@ -11,13 +11,20 @@ angular.module("admin.enterprises")
     $scope.StatusMessage = StatusMessage
 
     $scope.$watch 'enterprise_form.$dirty', (newValue) ->
-      console.log newValue
       StatusMessage.display 'notice', 'You have unsaved changes' if newValue
 
     $scope.setFormDirty = ->
       $scope.$apply ->
         $scope.enterprise_form.$setDirty()
 
+    $scope.cancel = (destination) ->
+      $window.location = destination
+
+    $scope.submit = ->
+      $scope.navClear()
+      enterprise_form.submit()
+
+
     # Provide a callback for generating warning messages displayed before leaving the page. This is passed in
     # from a directive "nav-check" in the page - if we pass it here it will be called in the test suite,
     # and on all new uses of this contoller, and we might not want that .
diff --git a/app/views/admin/enterprises/_ng_form.html.haml b/app/views/admin/enterprises/_ng_form.html.haml
index 9e4becc158..53d8f8f995 100644
--- a/app/views/admin/enterprises/_ng_form.html.haml
+++ b/app/views/admin/enterprises/_ng_form.html.haml
@@ -1,14 +1,13 @@
 -# Not all inputs are ng inputs, they don't make the ng-form dirty on change.
 -# ng-change is only valid for inputs, not for a form.
 -# So we use onchange and have to get the scope to access the ng controller
-= form_for [main_app, :admin, @enterprise], html: { name: "enterprise_form",
+= form_for [main_app, :admin, @enterprise], html: { id: "enterprise_form", name: "enterprise_form",
   "ng-app" => 'admin.enterprises',
-  "ng-submit" => "navClear()",
   "ng-controller" => 'enterpriseCtrl',
   'onchange' => 'angular.element(enterprise_form).scope().setFormDirty()',
   } do |f|
 
-  %save-bar{ buttons: "[{ text: 'Update', action: submit, param: null }]", form: "enterprise_form" }
+  %save-bar{ buttons: "[{ text: 'Update', action: submit, param: null, class: 'red' }, { text: 'Cancel', action: cancel, param: '#{main_app.admin_enterprises_path}' }]", form: "enterprise_form" }
 
   .row
     .sixteen.columns.alpha
@@ -17,8 +16,8 @@
       .one.column &nbsp;
       .eleven.columns.omega.fullwidth_inputs
         = render 'form', f: f
-  .row
-    .five.columns.alpha
-      &nbsp;
-    .eleven.columns.alpha
-      = render partial: "spree/admin/shared/#{action}_resource_links"
+  -# .row
+  -#   .five.columns.alpha
+  -#     &nbsp;
+  -#   .eleven.columns.alpha
+  -#     = render partial: "spree/admin/shared/#{action}_resource_links"

From 33fd88507daa2a12a726a90cfd0c7fad25f37fe4 Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Wed, 25 May 2016 11:26:15 +1000
Subject: [PATCH 07/35] Fix failed tests

---
 .../controllers/enterprise_controller.js.coffee |  2 +-
 spec/features/admin/enterprises_spec.rb         | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee
index 8b5f261436..f21c96a27e 100644
--- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee
+++ b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee
@@ -1,5 +1,5 @@
 angular.module("admin.enterprises")
-  .controller "enterpriseCtrl", ($scope, $window,NavigationCheck, enterprise, EnterprisePaymentMethods, EnterpriseShippingMethods, SideMenu, StatusMessage) ->
+  .controller "enterpriseCtrl", ($scope, $window, NavigationCheck, enterprise, EnterprisePaymentMethods, EnterpriseShippingMethods, SideMenu, StatusMessage) ->
     $scope.Enterprise = enterprise
     $scope.PaymentMethods = EnterprisePaymentMethods.paymentMethods
     $scope.ShippingMethods = EnterpriseShippingMethods.shippingMethods
diff --git a/spec/features/admin/enterprises_spec.rb b/spec/features/admin/enterprises_spec.rb
index 3579421b84..4d09007584 100644
--- a/spec/features/admin/enterprises_spec.rb
+++ b/spec/features/admin/enterprises_spec.rb
@@ -61,6 +61,9 @@ feature %q{
   end
 
   scenario "editing an existing enterprise", js: true do
+    # Make the page long enough to avoid the save bar overlaying the form
+    page.driver.resize(1280, 1000)
+
     @enterprise = create(:enterprise)
     e2 = create(:enterprise)
     eg1 = create(:enterprise_group, name: 'eg1')
@@ -355,6 +358,10 @@ feature %q{
       within("tbody#e_#{distributor1.id}") { click_link 'Manage' }
 
       fill_in 'enterprise_name', :with => 'Eaterprises'
+
+      # Because poltergist does not support form onchange event
+      # We need trigger the change manually
+      page.evaluate_script("angular.element(enterprise_form).scope().setFormDirty()")
       click_button 'Update'
 
       flash_message.should == 'Enterprise "Eaterprises" has been successfully updated!'
@@ -367,6 +374,10 @@ feature %q{
         within("tbody#e_#{distributor3.id}") { click_link 'Manage' }
 
         fill_in 'enterprise_name', :with => 'Eaterprises'
+
+        # Because poltergist does not support form onchange event
+        # We need trigger the change manually
+        page.evaluate_script("angular.element(enterprise_form).scope().setFormDirty()")
         click_button 'Update'
 
         flash_message.should == 'Enterprise "Eaterprises" has been successfully updated!'
@@ -407,8 +418,14 @@ feature %q{
 
       # -- Update only
       select2_select "Certified Organic", from: 'enterprise_producer_properties_attributes_0_property_name'
+
       fill_in 'enterprise_producer_properties_attributes_0_value', with: "NASAA 12345"
+
+      # Because poltergist does not support form onchange event
+      # We need trigger the change manually
+      page.evaluate_script("angular.element(enterprise_form).scope().setFormDirty()")
       click_button 'Update'
+
       supplier1.producer_properties(true).count.should == 1
 
       # -- Destroy

From 860a537f300e304c49c57a0ba8e7be72bf21e48e Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Wed, 25 May 2016 14:36:37 +1000
Subject: [PATCH 08/35] Add save bar to bulk editing product page

---
 .../javascripts/admin/bulk_product_update.js.coffee |  1 +
 app/views/admin/enterprises/_ng_form.html.haml      |  2 +-
 app/views/spree/admin/products/bulk_edit.html.haml  |  1 -
 .../admin/products/bulk_edit/_actions.html.haml     |  5 +----
 .../admin/products/bulk_edit/_products.html.haml    | 13 ++++++++-----
 .../unit/bulk_product_update_spec.js.coffee         |  4 ++++
 6 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee
index 751734a899..8ec81fb2dc 100644
--- a/app/assets/javascripts/admin/bulk_product_update.js.coffee
+++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee
@@ -247,6 +247,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout
 
     $scope.displaySuccess = ->
       StatusMessage.display 'success',t("products_changes_saved")
+      $scope.bulk_product_form.$setPristine()
 
 
     $scope.displayFailure = (failMessage) ->
diff --git a/app/views/admin/enterprises/_ng_form.html.haml b/app/views/admin/enterprises/_ng_form.html.haml
index 53d8f8f995..656d105587 100644
--- a/app/views/admin/enterprises/_ng_form.html.haml
+++ b/app/views/admin/enterprises/_ng_form.html.haml
@@ -1,7 +1,7 @@
 -# Not all inputs are ng inputs, they don't make the ng-form dirty on change.
 -# ng-change is only valid for inputs, not for a form.
 -# So we use onchange and have to get the scope to access the ng controller
-= form_for [main_app, :admin, @enterprise], html: { id: "enterprise_form", name: "enterprise_form",
+= form_for [main_app, :admin, @enterprise], html: { name: "enterprise_form",
   "ng-app" => 'admin.enterprises',
   "ng-controller" => 'enterpriseCtrl',
   'onchange' => 'angular.element(enterprise_form).scope().setFormDirty()',
diff --git a/app/views/spree/admin/products/bulk_edit.html.haml b/app/views/spree/admin/products/bulk_edit.html.haml
index 235544cb8a..4e6cf8c704 100644
--- a/app/views/spree/admin/products/bulk_edit.html.haml
+++ b/app/views/spree/admin/products/bulk_edit.html.haml
@@ -8,4 +8,3 @@
   = render 'spree/admin/products/bulk_edit/actions'
   = render 'spree/admin/products/bulk_edit/indicators'
   = render 'spree/admin/products/bulk_edit/products'
-  = render 'spree/admin/products/bulk_edit/save_button_row'
diff --git a/app/views/spree/admin/products/bulk_edit/_actions.html.haml b/app/views/spree/admin/products/bulk_edit/_actions.html.haml
index 40f135d62e..4a0878bd5c 100644
--- a/app/views/spree/admin/products/bulk_edit/_actions.html.haml
+++ b/app/views/spree/admin/products/bulk_edit/_actions.html.haml
@@ -1,6 +1,3 @@
 .controls.sixteen.columns.alpha{ 'ng-hide' => 'loading || products.length == 0' }
-  .four.columns.alpha
-    %input.four.columns.alpha{ :type => 'button', :value => 'Save Changes', 'ng-click' => 'submitProducts()'}
-  .nine.columns
-    = render 'spree/admin/shared/status_message'
+  .thirteen.columns
   %columns-dropdown{ action: "#{controller_name}_#{action_name}" }
diff --git a/app/views/spree/admin/products/bulk_edit/_products.html.haml b/app/views/spree/admin/products/bulk_edit/_products.html.haml
index 81fb31573b..04a6592f70 100644
--- a/app/views/spree/admin/products/bulk_edit/_products.html.haml
+++ b/app/views/spree/admin/products/bulk_edit/_products.html.haml
@@ -1,9 +1,12 @@
 %div.sixteen.columns.alpha{ 'ng-hide' => 'loading || filteredProducts.length == 0' }
-  %table.index#listing_products.bulk{ "infinite-scroll" => "incrementLimit()", "infinite-scroll-distance" => "1" }
+  %form{ name: 'bulk_product_form' }
+    %save-bar{ form: "bulk_product_form", buttons: "[{ text: 'Save Changes', action: submitProducts, class: 'red' }]" }
 
-    = render 'spree/admin/products/bulk_edit/products_head'
+    %table.index#listing_products.bulk{ "infinite-scroll" => "incrementLimit()", "infinite-scroll-distance" => "1" }
 
-    %tbody{ 'ng-repeat' => 'product in filteredProducts = ( products | filter:query | producer: producerFilter | category: categoryFilter | limitTo:limit )', 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'" }
+      = render 'spree/admin/products/bulk_edit/products_head'
 
-      = render 'spree/admin/products/bulk_edit/products_product'
-      = render 'spree/admin/products/bulk_edit/products_variant'
+      %tbody{ 'ng-repeat' => 'product in filteredProducts = ( products | filter:query | producer: producerFilter | category: categoryFilter | limitTo:limit )', 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'" }
+
+        = render 'spree/admin/products/bulk_edit/products_product'
+        = render 'spree/admin/products/bulk_edit/products_variant'
diff --git a/spec/javascripts/unit/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/bulk_product_update_spec.js.coffee
index cc0b574f47..c1c34cbe6b 100644
--- a/spec/javascripts/unit/bulk_product_update_spec.js.coffee
+++ b/spec/javascripts/unit/bulk_product_update_spec.js.coffee
@@ -668,6 +668,9 @@ describe "AdminProductEditCtrl", ->
         spyOn $scope, "displaySuccess"
         spyOn BulkProducts, "updateVariantLists"
         spyOn DirtyProducts, "clear"
+
+        $scope.bulk_product_form = jasmine.createSpyObj('bulk_product_form', ['$setPristine'])
+
         $scope.products = [
           {
             id: 1
@@ -692,6 +695,7 @@ describe "AdminProductEditCtrl", ->
         $httpBackend.flush()
         $timeout.flush()
         expect($scope.displaySuccess).toHaveBeenCalled()
+        expect($scope.bulk_product_form.$setPristine).toHaveBeenCalled
         expect(DirtyProducts.clear).toHaveBeenCalled()
         expect(BulkProducts.updateVariantLists).toHaveBeenCalled()
 

From 41837eb31d249880d958626eaeeedf37e57a2f0d Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Wed, 25 May 2016 14:55:00 +1000
Subject: [PATCH 09/35] Tweak create order cycle spec

---
 app/views/admin/enterprises/_ng_form.html.haml |  5 -----
 spec/features/admin/order_cycles_spec.rb       | 12 ++++++++++--
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/app/views/admin/enterprises/_ng_form.html.haml b/app/views/admin/enterprises/_ng_form.html.haml
index 656d105587..3914cc1df2 100644
--- a/app/views/admin/enterprises/_ng_form.html.haml
+++ b/app/views/admin/enterprises/_ng_form.html.haml
@@ -16,8 +16,3 @@
       .one.column &nbsp;
       .eleven.columns.omega.fullwidth_inputs
         = render 'form', f: f
-  -# .row
-  -#   .five.columns.alpha
-  -#     &nbsp;
-  -#   .eleven.columns.alpha
-  -#     = render partial: "spree/admin/shared/#{action}_resource_links"
diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb
index e843e00235..74b2e88453 100644
--- a/spec/features/admin/order_cycles_spec.rb
+++ b/spec/features/admin/order_cycles_spec.rb
@@ -59,6 +59,8 @@ feature %q{
   end
 
   scenario "creating an order cycle", js: true do
+    page.driver.resize(1280, 2000)
+
     # Given coordinating, supplying and distributing enterprises with some products with variants
     coordinator = create(:distributor_enterprise, name: 'My coordinator')
     supplier = create(:supplier_enterprise, name: 'My supplier')
@@ -139,7 +141,7 @@ feature %q{
     select 'Distributor fee', from: 'order_cycle_outgoing_exchange_0_enterprise_fees_0_enterprise_fee_id'
 
     # And I click Create
-    find_button('Create').trigger('click')
+    click_button 'Create'
 
     # Then my order cycle should have been created
     page.should have_content 'Your order cycle has been created.'
@@ -567,6 +569,9 @@ feature %q{
       end
 
       scenario "creating a new order cycle" do
+        # Make the page long enough to avoid the save bar overlaying the form
+        page.driver.resize(1280, 2000)
+
         click_link "Order Cycles"
         click_link 'New Order Cycle'
 
@@ -839,6 +844,9 @@ feature %q{
     end
 
     it "creates order cycles", js: true do
+      # Make the page long enough to avoid the save bar overlaying the form
+      page.driver.resize(1280, 2000)
+
       # When I go to the new order cycle page
       visit admin_order_cycles_path
       click_link 'New Order Cycle'
@@ -866,7 +874,7 @@ feature %q{
       page.should_not have_select 'order_cycle_coordinator_fee_1_id'
 
       select 'Coord fee', from: 'order_cycle_coordinator_fee_0_id'
-      find_button('Create').trigger("click")
+      click_button 'Create'
 
       # Then my order cycle should have been created
       page.should have_content 'Your order cycle has been created.'

From 7aa8f2c73c87ef48cd38c3473d33277f7ae60a75 Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Wed, 25 May 2016 15:15:02 +1000
Subject: [PATCH 10/35] Fix failed tests

---
 .../admin/bulk_product_update_spec.rb         | 32 +++++++++----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb
index f7f32f6523..062735ccff 100644
--- a/spec/features/admin/bulk_product_update_spec.rb
+++ b/spec/features/admin/bulk_product_update_spec.rb
@@ -367,7 +367,10 @@ feature %q{
       fill_in "variant_price", with: "10.0"
     end
 
-    click_button 'Save Changes', match: :first
+    within "#save-bar" do
+      click_button 'Save Changes'
+    end
+
     expect(page.find("#status-message")).to have_content "Changes saved."
 
     v.reload
@@ -384,7 +387,10 @@ feature %q{
 
     fill_in "product_name", with: "new name 1"
 
-    click_button 'Save Changes', match: :first
+    within "#save-bar" do
+      click_button 'Save Changes'
+    end
+
     expect(page.find("#status-message")).to have_content "Changes saved."
     p.reload
     expect(p.name).to eq "new name 1"
@@ -415,24 +421,15 @@ feature %q{
 
     fill_in "product_name", :with => "new product name"
 
-    click_button 'Save Changes', match: :first
+    within "#save-bar" do
+      click_button 'Save Changes'
+    end
+
     expect(page.find("#status-message")).to have_content "Changes saved."
     p.reload
     expect(p.name).to eq "new product name"
   end
 
-  scenario "updating when no changes have been made" do
-    Capybara.using_wait_time(2) do
-      FactoryGirl.create(:product, :name => "product 1")
-      login_to_admin_section
-
-      visit '/admin/products/bulk_edit'
-
-      click_button 'Save Changes', match: :first
-      expect(page.find("#status-message")).to have_content "No changes to save."
-    end
-  end
-
   scenario "updating when a filter has been applied" do
     s1 = create(:supplier_enterprise)
     s2 = create(:supplier_enterprise)
@@ -447,7 +444,10 @@ feature %q{
     expect(page).to have_no_field "product_name", with: p2.name
     fill_in "product_name", :with => "new product1"
 
-    click_button 'Save Changes', match: :first
+    within "#save-bar" do
+      click_button 'Save Changes'
+    end
+
     expect(page.find("#status-message")).to have_content "Changes saved."
     p1.reload
     expect(p1.name).to eq "new product1"

From c003dcded9554cf56b563642fda3ab150cec0e4a Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Fri, 27 May 2016 11:22:23 +1000
Subject: [PATCH 11/35] Update editing enterprise page save-bar

---
 app/views/admin/enterprises/_ng_form.html.haml | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/views/admin/enterprises/_ng_form.html.haml b/app/views/admin/enterprises/_ng_form.html.haml
index 3914cc1df2..ca38b7eca4 100644
--- a/app/views/admin/enterprises/_ng_form.html.haml
+++ b/app/views/admin/enterprises/_ng_form.html.haml
@@ -7,7 +7,11 @@
   'onchange' => 'angular.element(enterprise_form).scope().setFormDirty()',
   } do |f|
 
-  %save-bar{ buttons: "[{ text: 'Update', action: submit, param: null, class: 'red' }, { text: 'Cancel', action: cancel, param: '#{main_app.admin_enterprises_path}' }]", form: "enterprise_form" }
+  %save-bar{ dirty: "enterprise_form.$dirty", persist: "true" }
+    %input.red{ type: "button", value: "Update", ng: { click: "submit()", disabled: "!enterprise_form.$dirty" } }
+    %input{ type: "button", ng: { value: "enterprise_form.$dirty ? 'Cancel' : 'Close'", click: "cancel('#{main_app.admin_enterprises_path}')" } }
+
+
 
   .row
     .sixteen.columns.alpha

From 001ae19b26ba71a9dd6cd9dc887b45e0e6180a0d Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Fri, 27 May 2016 11:46:23 +1000
Subject: [PATCH 12/35] Update create and update order cycle page save-bar

---
 .../admin/order_cycles/controllers/create.js.coffee       | 8 ++++++--
 .../order_cycles/controllers/simple_create.js.coffee      | 8 ++++++--
 .../admin/order_cycles/controllers/simple_edit.js.coffee  | 5 ++++-
 app/views/admin/order_cycles/new.html.haml                | 4 +++-
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/app/assets/javascripts/admin/order_cycles/controllers/create.js.coffee b/app/assets/javascripts/admin/order_cycles/controllers/create.js.coffee
index 5d33b895db..e0f7cf96b6 100644
--- a/app/assets/javascripts/admin/order_cycles/controllers/create.js.coffee
+++ b/app/assets/javascripts/admin/order_cycles/controllers/create.js.coffee
@@ -1,5 +1,5 @@
 angular.module('admin.orderCycles')
-  .controller 'AdminCreateOrderCycleCtrl', ($scope, $filter, OrderCycle, Enterprise, EnterpriseFee, ocInstance, StatusMessage) ->
+  .controller 'AdminCreateOrderCycleCtrl', ($scope, $filter, $window, OrderCycle, Enterprise, EnterpriseFee, ocInstance, StatusMessage) ->
     $scope.enterprises = Enterprise.index(coordinator_id: ocInstance.coordinator_id)
     $scope.supplier_enterprises = Enterprise.producer_enterprises
     $scope.distributor_enterprises = Enterprise.hub_enterprises
@@ -79,7 +79,11 @@ angular.module('admin.orderCycles')
     $scope.removeDistributionOfVariant = (variant_id) ->
       OrderCycle.removeDistributionOfVariant(variant_id)
 
-    $scope.submit = (destination) ->
+    $scope.submit = ($event, destination) ->
+      $event.preventDefault()
       StatusMessage.display 'progress', "Saving..."
       OrderCycle.create(destination)
 
+    $scope.cancel = (destination) ->
+      $window.location = destination
+
diff --git a/app/assets/javascripts/admin/order_cycles/controllers/simple_create.js.coffee b/app/assets/javascripts/admin/order_cycles/controllers/simple_create.js.coffee
index 2839e74557..7a0a03f2bf 100644
--- a/app/assets/javascripts/admin/order_cycles/controllers/simple_create.js.coffee
+++ b/app/assets/javascripts/admin/order_cycles/controllers/simple_create.js.coffee
@@ -1,4 +1,4 @@
-angular.module('admin.orderCycles').controller "AdminSimpleCreateOrderCycleCtrl", ($scope, OrderCycle, Enterprise, EnterpriseFee, StatusMessage, ocInstance) ->
+angular.module('admin.orderCycles').controller "AdminSimpleCreateOrderCycleCtrl", ($scope, $window, OrderCycle, Enterprise, EnterpriseFee, StatusMessage, ocInstance) ->
   $scope.StatusMessage = StatusMessage
   $scope.OrderCycle = OrderCycle
   $scope.order_cycle = OrderCycle.new {coordinator_id: ocInstance.coordinator_id}, =>
@@ -44,7 +44,11 @@ angular.module('admin.orderCycles').controller "AdminSimpleCreateOrderCycleCtrl"
   $scope.enterpriseFeesForEnterprise = (enterprise_id) ->
     EnterpriseFee.forEnterprise(parseInt(enterprise_id))
 
-  $scope.submit = (destination) ->
+  $scope.submit = ($event, destination) ->
+    $event.preventDefault()
     StatusMessage.display 'progress', "Saving..."
     OrderCycle.mirrorIncomingToOutgoingProducts()
     OrderCycle.create(destination)
+
+  $scope.cancel = (destination) ->
+      $window.location = destination
diff --git a/app/assets/javascripts/admin/order_cycles/controllers/simple_edit.js.coffee b/app/assets/javascripts/admin/order_cycles/controllers/simple_edit.js.coffee
index f5bae5a4d8..7df7ddff34 100644
--- a/app/assets/javascripts/admin/order_cycles/controllers/simple_edit.js.coffee
+++ b/app/assets/javascripts/admin/order_cycles/controllers/simple_edit.js.coffee
@@ -1,4 +1,4 @@
-angular.module('admin.orderCycles').controller "AdminSimpleEditOrderCycleCtrl", ($scope, $location, OrderCycle, Enterprise, EnterpriseFee, StatusMessage) ->
+angular.module('admin.orderCycles').controller "AdminSimpleEditOrderCycleCtrl", ($scope, $location, $window, OrderCycle, Enterprise, EnterpriseFee, StatusMessage) ->
   $scope.orderCycleId = ->
     $location.absUrl().match(/\/admin\/order_cycles\/(\d+)/)[1]
 
@@ -42,3 +42,6 @@ angular.module('admin.orderCycles').controller "AdminSimpleEditOrderCycleCtrl",
     StatusMessage.display 'progress', "Saving..."
     OrderCycle.mirrorIncomingToOutgoingProducts()
     OrderCycle.update(destination, $scope.order_cycle_form)
+
+  $scope.cancel = (destination) ->
+    $window.location = destination
diff --git a/app/views/admin/order_cycles/new.html.haml b/app/views/admin/order_cycles/new.html.haml
index 7f55c28aed..4e5a95544c 100644
--- a/app/views/admin/order_cycles/new.html.haml
+++ b/app/views/admin/order_cycles/new.html.haml
@@ -6,7 +6,9 @@
 
 = form_for [main_app, :admin, @order_cycle], :url => '', :html => {:class => 'ng order_cycle', 'ng-app' => 'admin.orderCycles', 'ng-controller' => ng_controller, name: 'order_cycle_form'} do |f|
 
-  %save-bar{ buttons: "[{ text: 'Create', action: submit, param: '#{main_app.admin_order_cycles_path}' }]", form: "order_cycle_form" }
+  %save-bar{ dirty: "order_cycle_form.$dirty", persist: "true" }
+    %input.red{ type: "button", value: "Create", ng: { click: "submit($event, '#{main_app.admin_order_cycles_path}')", disabled: "!order_cycle_form.$dirty" } }
+    %input{ type: "button", ng: { value: "order_cycle_form.$dirty ? 'Cancel' : 'Close'", click: "cancel('#{main_app.admin_order_cycles_path}')" } }
 
   - if order_cycles_simple_form
     = render 'simple_form', f: f

From 82dc2a8c984c31f570708be8b237b821c8b1e449 Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Fri, 27 May 2016 12:04:34 +1000
Subject: [PATCH 13/35] Update bulk products editing page

---
 app/assets/javascripts/admin/bulk_product_update.js.coffee   | 4 +++-
 app/views/spree/admin/products/bulk_edit/_products.html.haml | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee
index 8ec81fb2dc..7a9d2677b9 100644
--- a/app/assets/javascripts/admin/bulk_product_update.js.coffee
+++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee
@@ -1,4 +1,4 @@
-angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout, $http, BulkProducts, DisplayProperties, dataFetcher, DirtyProducts, VariantUnitManager, StatusMessage, producers, Taxons, SpreeApiAuth, Columns, tax_categories) ->
+angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout, $http, $window, BulkProducts, DisplayProperties, dataFetcher, DirtyProducts, VariantUnitManager, StatusMessage, producers, Taxons, SpreeApiAuth, Columns, tax_categories) ->
     $scope.loading = true
 
     $scope.StatusMessage = StatusMessage
@@ -206,6 +206,8 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout
         else
           $scope.displayFailure t("products_update_error_data") + status
 
+    $scope.cancel = (destination) ->
+      $window.location = destination
 
     $scope.packProduct = (product) ->
       if product.variant_unit_with_scale
diff --git a/app/views/spree/admin/products/bulk_edit/_products.html.haml b/app/views/spree/admin/products/bulk_edit/_products.html.haml
index 04a6592f70..b8c580af41 100644
--- a/app/views/spree/admin/products/bulk_edit/_products.html.haml
+++ b/app/views/spree/admin/products/bulk_edit/_products.html.haml
@@ -1,6 +1,8 @@
 %div.sixteen.columns.alpha{ 'ng-hide' => 'loading || filteredProducts.length == 0' }
   %form{ name: 'bulk_product_form' }
-    %save-bar{ form: "bulk_product_form", buttons: "[{ text: 'Save Changes', action: submitProducts, class: 'red' }]" }
+    %save-bar{ dirty: "bulk_product_form.$dirty", persist: "false" }
+      %input.red{ type: "button", value: "Save Changes", ng: { click: "submitProducts()", disabled: "!bulk_product_form.$dirty" } }
+      %input{ type: "button", value: "Close", 'ng-click' => "cancel('#{bulk_edit_admin_products_path}')" }
 
     %table.index#listing_products.bulk{ "infinite-scroll" => "incrementLimit()", "infinite-scroll-distance" => "1" }
 

From 18a8efed5f9370fb063638affe5840858fab611f Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Fri, 27 May 2016 12:21:14 +1000
Subject: [PATCH 14/35] Resize window to fix failed test

---
 spec/features/admin/tag_rules_spec.rb | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/spec/features/admin/tag_rules_spec.rb b/spec/features/admin/tag_rules_spec.rb
index d956ff94b6..f8f25ec181 100644
--- a/spec/features/admin/tag_rules_spec.rb
+++ b/spec/features/admin/tag_rules_spec.rb
@@ -13,6 +13,9 @@ feature 'Tag Rules', js: true do
     end
 
     it "allows creation of rules of each type" do
+      # Make the whole page visible
+      page.driver.resize(1280, 2000)
+
       click_link "Tag Rules"
 
       # Creating a new tag
@@ -120,6 +123,9 @@ feature 'Tag Rules', js: true do
     end
 
     it "saves changes to rules of each type" do
+      # Make the whole page visible
+      page.driver.resize(1280, 2000)
+
       click_link "Tag Rules"
 
       # Tag groups exist
@@ -228,6 +234,9 @@ feature 'Tag Rules', js: true do
     end
 
     it "deletes both default and customer rules from the database" do
+      # Make the whole page visible
+      page.driver.resize(1280, 2000)
+
       click_link "Tag Rules"
 
       expect do

From c83952571f60557e20e5218aa7c8cf9cd770bfc3 Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Tue, 31 May 2016 10:50:28 +1000
Subject: [PATCH 15/35] Fix failed test

---
 spec/features/admin/tag_rules_spec.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/spec/features/admin/tag_rules_spec.rb b/spec/features/admin/tag_rules_spec.rb
index f8f25ec181..270946e961 100644
--- a/spec/features/admin/tag_rules_spec.rb
+++ b/spec/features/admin/tag_rules_spec.rb
@@ -195,25 +195,25 @@ feature 'Tag Rules', js: true do
       expect(default_fsm_tag_rule.preferred_matched_shipping_methods_visibility).to eq "hidden"
 
       # FilterShippingMethods rule
-      expect(fsm_tag_rule.reload.priority).to eq 1
+      expect(fsm_tag_rule.reload.priority).to eq 4
       expect(fsm_tag_rule.preferred_customer_tags).to eq "volunteer"
       expect(fsm_tag_rule.preferred_shipping_method_tags).to eq "volunteers-only4"
       expect(fsm_tag_rule.preferred_matched_shipping_methods_visibility).to eq "visible"
 
       # FilterProducts rule
-      expect(fp_tag_rule.reload.priority).to eq 2
+      expect(fp_tag_rule.reload.priority).to eq 1
       expect(fp_tag_rule.preferred_customer_tags).to eq "volunteer"
       expect(fp_tag_rule.preferred_variant_tags).to eq "volunteers-only1"
       expect(fp_tag_rule.preferred_matched_variants_visibility).to eq "hidden"
 
       # FilterPaymentMethods rule
-      expect(fpm_tag_rule.reload.priority).to eq 3
+      expect(fpm_tag_rule.reload.priority).to eq 2
       expect(fpm_tag_rule.preferred_customer_tags).to eq "volunteer"
       expect(fpm_tag_rule.preferred_payment_method_tags).to eq "volunteers-only2"
       expect(fpm_tag_rule.preferred_matched_payment_methods_visibility).to eq "visible"
 
       # FilterOrderCycles rule
-      expect(foc_tag_rule.reload.priority).to eq 4
+      expect(foc_tag_rule.reload.priority).to eq 3
       expect(foc_tag_rule.preferred_customer_tags).to eq "volunteer"
       expect(foc_tag_rule.preferred_exchange_tags).to eq "volunteers-only3"
       expect(foc_tag_rule.preferred_matched_order_cycles_visibility).to eq "hidden"

From 8221f1f193101aeb17557952d681fff9f9a678e1 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Wed, 1 Jun 2016 13:21:13 +1000
Subject: [PATCH 16/35] Use scope. to ensure that tag rule sorting is applied

---
 .../admin/utils/directives/sortable.js.coffee | 29 ++++++++++---------
 spec/features/admin/tag_rules_spec.rb         |  8 ++---
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/app/assets/javascripts/admin/utils/directives/sortable.js.coffee b/app/assets/javascripts/admin/utils/directives/sortable.js.coffee
index 2f2f100e31..19f527bc0f 100644
--- a/app/assets/javascripts/admin/utils/directives/sortable.js.coffee
+++ b/app/assets/javascripts/admin/utils/directives/sortable.js.coffee
@@ -20,17 +20,18 @@ angular.module("admin.utils").directive "ofnSortable", ($timeout, $parse) ->
         items: scope.items
         appendTo: element
         update: (event, ui) ->
-          sortableSiblings = ($(ss) for ss in ui.item.siblings(scope.items))
-          offset = Math.min(ui.item.index(), sortableSiblings[0].index())
-          newPos = ui.item.index() - offset + 1
-          oldPos = getScopePos(ui.item.scope())
-          if newPos < oldPos
-            for sibScope in sortableSiblings.map((ss) -> ss.scope())
-              pos = getScopePos(sibScope)
-              setScopePos(sibScope, pos + 1) if pos >= newPos && pos < oldPos
-          else if newPos > oldPos
-            for sibScope in sortableSiblings.map((ss) -> ss.scope())
-              pos = getScopePos(sibScope)
-              setScopePos(sibScope, pos - 1) if pos > oldPos && pos <= newPos
-          setScopePos(ui.item.scope(), newPos)
-          scope.afterSort()
+          scope.$apply ->
+            sortableSiblings = ($(ss) for ss in ui.item.siblings(scope.items))
+            offset = Math.min(ui.item.index(), sortableSiblings[0].index())
+            newPos = ui.item.index() - offset + 1
+            oldPos = getScopePos(ui.item.scope())
+            if newPos < oldPos
+              for sibScope in sortableSiblings.map((ss) -> ss.scope())
+                pos = getScopePos(sibScope)
+                setScopePos(sibScope, pos + 1) if pos >= newPos && pos < oldPos
+            else if newPos > oldPos
+              for sibScope in sortableSiblings.map((ss) -> ss.scope())
+                pos = getScopePos(sibScope)
+                setScopePos(sibScope, pos - 1) if pos > oldPos && pos <= newPos
+            setScopePos(ui.item.scope(), newPos)
+            scope.afterSort()
diff --git a/spec/features/admin/tag_rules_spec.rb b/spec/features/admin/tag_rules_spec.rb
index 270946e961..f8f25ec181 100644
--- a/spec/features/admin/tag_rules_spec.rb
+++ b/spec/features/admin/tag_rules_spec.rb
@@ -195,25 +195,25 @@ feature 'Tag Rules', js: true do
       expect(default_fsm_tag_rule.preferred_matched_shipping_methods_visibility).to eq "hidden"
 
       # FilterShippingMethods rule
-      expect(fsm_tag_rule.reload.priority).to eq 4
+      expect(fsm_tag_rule.reload.priority).to eq 1
       expect(fsm_tag_rule.preferred_customer_tags).to eq "volunteer"
       expect(fsm_tag_rule.preferred_shipping_method_tags).to eq "volunteers-only4"
       expect(fsm_tag_rule.preferred_matched_shipping_methods_visibility).to eq "visible"
 
       # FilterProducts rule
-      expect(fp_tag_rule.reload.priority).to eq 1
+      expect(fp_tag_rule.reload.priority).to eq 2
       expect(fp_tag_rule.preferred_customer_tags).to eq "volunteer"
       expect(fp_tag_rule.preferred_variant_tags).to eq "volunteers-only1"
       expect(fp_tag_rule.preferred_matched_variants_visibility).to eq "hidden"
 
       # FilterPaymentMethods rule
-      expect(fpm_tag_rule.reload.priority).to eq 2
+      expect(fpm_tag_rule.reload.priority).to eq 3
       expect(fpm_tag_rule.preferred_customer_tags).to eq "volunteer"
       expect(fpm_tag_rule.preferred_payment_method_tags).to eq "volunteers-only2"
       expect(fpm_tag_rule.preferred_matched_payment_methods_visibility).to eq "visible"
 
       # FilterOrderCycles rule
-      expect(foc_tag_rule.reload.priority).to eq 3
+      expect(foc_tag_rule.reload.priority).to eq 4
       expect(foc_tag_rule.preferred_customer_tags).to eq "volunteer"
       expect(foc_tag_rule.preferred_exchange_tags).to eq "volunteers-only3"
       expect(foc_tag_rule.preferred_matched_order_cycles_visibility).to eq "hidden"

From c8c8f0d02daa7efbeacb740ebde006e80bfd8f7d Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Wed, 1 Jun 2016 13:27:04 +1000
Subject: [PATCH 17/35] Removing old cancel button from bottom of OC forms

---
 app/views/admin/order_cycles/_form.html.haml        | 2 --
 app/views/admin/order_cycles/_simple_form.html.haml | 2 --
 2 files changed, 4 deletions(-)

diff --git a/app/views/admin/order_cycles/_form.html.haml b/app/views/admin/order_cycles/_form.html.haml
index 8eff9d85dd..ebe9e45905 100644
--- a/app/views/admin/order_cycles/_form.html.haml
+++ b/app/views/admin/order_cycles/_form.html.haml
@@ -46,8 +46,6 @@
   = render 'add_exchange_form', f: f, type: 'distributor'
 
 .actions
-  %span{'ng-show' => 'loaded()'}
-    = link_to 'Cancel', main_app.admin_order_cycles_path
   %span{'ng-hide' => 'loaded()'} Loading...
 
 
diff --git a/app/views/admin/order_cycles/_simple_form.html.haml b/app/views/admin/order_cycles/_simple_form.html.haml
index 7acbaf19e8..a7a0363cd3 100644
--- a/app/views/admin/order_cycles/_simple_form.html.haml
+++ b/app/views/admin/order_cycles/_simple_form.html.haml
@@ -21,6 +21,4 @@
 = render 'coordinator_fees', f: f
 
 .actions
-  %span{'ng-show' => 'loaded()'}
-    = link_to 'Cancel', main_app.admin_order_cycles_path
   %span{'ng-hide' => 'loaded()'} Loading...

From d424c2eb20e1cc620ce7ceacf57bd101bfb18c3d Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Wed, 1 Jun 2016 13:36:18 +1000
Subject: [PATCH 18/35] Enterprise form SaveBar becomes available when adding a
 new rule

---
 .../admin/tag_rules/controllers/tag_rules_controller.js.coffee   | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/assets/javascripts/admin/tag_rules/controllers/tag_rules_controller.js.coffee b/app/assets/javascripts/admin/tag_rules/controllers/tag_rules_controller.js.coffee
index 45f98a2b57..460d8565eb 100644
--- a/app/assets/javascripts/admin/tag_rules/controllers/tag_rules_controller.js.coffee
+++ b/app/assets/javascripts/admin/tag_rules/controllers/tag_rules_controller.js.coffee
@@ -38,6 +38,7 @@ angular.module("admin.tagRules").controller "TagRulesCtrl", ($scope, $http, $fil
         newRule.peferred_exchange_tags = []
         newRule.preferred_matched_order_cycles_visibility = "visible"
     tagGroup.rules.push(newRule)
+    $scope.enterprise_form.$setDirty()
     $scope.updateRuleCounts()
 
   $scope.addNewTag = ->

From a39d15d68575212aa5ecead5a4d91d6cffa269b6 Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Wed, 1 Jun 2016 16:02:12 +1000
Subject: [PATCH 19/35] Fix failed adds a new tag rule js test

---
 .../tag_rules/controllers/tag_rules_controller_spec.js.coffee  | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/spec/javascripts/unit/admin/tag_rules/controllers/tag_rules_controller_spec.js.coffee b/spec/javascripts/unit/admin/tag_rules/controllers/tag_rules_controller_spec.js.coffee
index 2526016d3d..7ac5cc8fbc 100644
--- a/spec/javascripts/unit/admin/tag_rules/controllers/tag_rules_controller_spec.js.coffee
+++ b/spec/javascripts/unit/admin/tag_rules/controllers/tag_rules_controller_spec.js.coffee
@@ -15,6 +15,7 @@ describe "TagRulesCtrl", ->
 
     inject ($rootScope, $controller) ->
       scope = $rootScope
+      scope.enterprise_form = jasmine.createSpyObj('enterprise_form', ['$setDirty'])
       ctrl = $controller 'TagRulesCtrl', {$scope: scope, enterprise: enterprise}
 
   describe "tagGroup start indices", ->
@@ -27,6 +28,8 @@ describe "TagRulesCtrl", ->
       scope.addNewRuleTo(scope.tagGroups[0], "DiscountOrder")
 
     it "adds a new rule of the specified type to the rules array for the tagGroup", ->
+      expect(scope.enterprise_form.$setDirty).toHaveBeenCalled()
+
       expect(scope.tagGroups[0].rules.length).toEqual 3
       expect(scope.tagGroups[0].rules[2].type).toEqual "TagRule::DiscountOrder"
 

From 25cdd4af8e58722cb619c06762b7c86c3023be49 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 16 Jun 2016 15:10:16 +1000
Subject: [PATCH 20/35] Preventing shop from being changed when unsaved
 customer changes exist

Also making layout of filters on customer index more consistent with other pages
---
 .../customers_controller.js.coffee            | 17 +++++----
 .../directives/ofn-select2.js.coffee          |  3 ++
 .../services/pending_changes.js.coffee        |  5 +++
 app/views/admin/customers/index.html.haml     | 32 +++++++++--------
 spec/features/admin/customers_spec.rb         | 34 ++++++++++++------
 .../customers_controller_spec.js.coffee       | 35 ++++++++++++++-----
 6 files changed, 87 insertions(+), 39 deletions(-)

diff --git a/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee b/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee
index 23bfb37738..aefcab9f90 100644
--- a/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee
+++ b/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee
@@ -1,6 +1,5 @@
-angular.module("admin.customers").controller "customersCtrl", ($scope, $q, Customers, TagRuleResource, CurrentShop, RequestMonitor, Columns, pendingChanges, shops) ->
+angular.module("admin.customers").controller "customersCtrl", ($scope, $q, $filter, Customers, TagRuleResource, CurrentShop, RequestMonitor, Columns, pendingChanges, shops) ->
   $scope.shops = shops
-  $scope.CurrentShop = CurrentShop
   $scope.RequestMonitor = RequestMonitor
   $scope.submitAll = pendingChanges.submitAll
   $scope.add = Customers.add
@@ -8,15 +7,21 @@ angular.module("admin.customers").controller "customersCtrl", ($scope, $q, Custo
   $scope.customerLimit = 20
   $scope.columns = Columns.columns
 
-  $scope.$watch "CurrentShop.shop", ->
-    if $scope.CurrentShop.shop.id?
-      Customers.index({enterprise_id: $scope.CurrentShop.shop.id}).then (data) ->
+  $scope.confirmRefresh = (event) ->
+    event.preventDefault() unless pendingChanges.unsavedCount() == 0 || confirm(t("unsaved_changes_warning"))
+
+  $scope.$watch "shop_id", ->
+    if $scope.shop_id?
+      CurrentShop.shop = $filter('filter')($scope.shops, {id: $scope.shop_id})[0]
+      Customers.index({enterprise_id: $scope.shop_id}).then (data) ->
+        pendingChanges.removeAll()
+        $scope.customers_form.$setPristine()
         $scope.customers = data
 
   $scope.findTags = (query) ->
     defer = $q.defer()
     params =
-      enterprise_id: $scope.CurrentShop.shop.id
+      enterprise_id: $scope.shop_id
     TagRuleResource.mapByTag params, (data) =>
       filtered = data.filter (tag) ->
         tag.text.toLowerCase().indexOf(query.toLowerCase()) != -1
diff --git a/app/assets/javascripts/admin/index_utils/directives/ofn-select2.js.coffee b/app/assets/javascripts/admin/index_utils/directives/ofn-select2.js.coffee
index bbb5221682..d65341e6fe 100644
--- a/app/assets/javascripts/admin/index_utils/directives/ofn-select2.js.coffee
+++ b/app/assets/javascripts/admin/index_utils/directives/ofn-select2.js.coffee
@@ -7,6 +7,7 @@ angular.module("admin.indexUtils").directive "ofnSelect2", ($sanitize, $timeout,
     text: "@?"
     blank: "=?"
     filter: "=?"
+    onSelecting: "=?"
   link: (scope, element, attrs, ngModel) ->
     $timeout ->
       scope.text ||= 'name'
@@ -24,6 +25,8 @@ angular.module("admin.indexUtils").directive "ofnSelect2", ($sanitize, $timeout,
         formatResult: (item) ->
           item[scope.text]
 
+      element.on "select2-opening", scope.onSelecting || angular.noop
+
     attrs.$observe 'disabled', (value) ->
       element.select2('enable', !value)
 
diff --git a/app/assets/javascripts/admin/index_utils/services/pending_changes.js.coffee b/app/assets/javascripts/admin/index_utils/services/pending_changes.js.coffee
index 15626b1479..a3d842c16d 100644
--- a/app/assets/javascripts/admin/index_utils/services/pending_changes.js.coffee
+++ b/app/assets/javascripts/admin/index_utils/services/pending_changes.js.coffee
@@ -10,6 +10,8 @@ angular.module("admin.indexUtils").factory "pendingChanges", ($q, resources, Sta
 
     removeAll: =>
       @pendingChanges = {}
+      StatusMessage.clear()
+
 
     remove: (id, attr) =>
       if @pendingChanges.hasOwnProperty("#{id}")
@@ -40,5 +42,8 @@ angular.module("admin.indexUtils").factory "pendingChanges", ($q, resources, Sta
         @errors.push error
         change.scope.error()
 
+    unsavedCount: ->
+      Object.keys(@pendingChanges).length
+
     changeCount: (objectChanges) ->
       Object.keys(objectChanges).length
diff --git a/app/views/admin/customers/index.html.haml b/app/views/admin/customers/index.html.haml
index 7e82536526..65523cda57 100644
--- a/app/views/admin/customers/index.html.haml
+++ b/app/views/admin/customers/index.html.haml
@@ -14,21 +14,25 @@
 = admin_inject_shops
 
 %div{ ng: { controller: 'customersCtrl' } }
-  .row
-    .five.columns.alpha
-      %h3
-        =t :please_select_hub
-    .four.columns
-      %select.select2.fullwidth#shop_id{ 'ng-model' => 'CurrentShop.shop', name: 'shop_id', 'ng-options' => 'shop as shop.name for shop in shops' }
-    .seven.columns.omega &nbsp;
+  .row.filters
+    .sixteen.columns.alpha.omega
+      .filter_select.five.columns.alpha
+        %label{ :for => 'quick_search', ng: {class: '{disabled: !shop_id}'} }=t('admin.quick_search')
+        %br
+        %input.fullwidth{ :type => "text", :id => 'quick_search', ng: { model: 'quickSearch', disabled: '!shop_id' }, :placeholder => "Search by email/code..." }
+      .filter_select.four.columns
+        %label{ :for => 'hub_id', ng: { bind: "shop_id ? '#{t('admin.shop')}' : '#{t('admin.variant_overrides.index.select_a_shop')}'" } }
+        %br
+        %input.ofn-select2.fullwidth#shop_id{ 'ng-model' => 'shop_id', name: 'shop_id', data: 'shops', on: { selecting: 'confirmRefresh' } }
+      .seven.columns.omega &nbsp;
 
-  .row{ 'ng-hide' => 'RequestMonitor.loading || !CurrentShop.shop.id || customers.length == 0' }
-    .controls.sixteen.columns.alpha.omega
-      .five.columns.alpha
-        %input.fullwidth{ :type => "text", :id => 'quick_search', 'ng-model' => 'quickSearch', :placeholder => 'Quick Search' }
-      .eight.columns &nbsp;
-      %columns-dropdown{ action: "#{controller_name}_#{action_name}" }
-  .row{ 'ng-if' => 'CurrentShop.shop.id && RequestMonitor.loading' }
+  %hr.divider{ ng: { show: "!RequestMonitor.loading && filteredCustomers.length > 0" } }
+
+  .row.controls{ ng: { show: "!RequestMonitor.loading && filteredCustomers.length > 0" } }
+    .thirteen.columns.alpha &nbsp;
+    %columns-dropdown{ action: "#{controller_name}_#{action_name}" }
+
+  .row{ 'ng-if' => 'shop_id && RequestMonitor.loading' }
     .sixteen.columns.alpha#loading
       %img.spinner{ src: "/assets/spinning-circles.svg" }
       %h1
diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb
index 964608510d..3a17c06e32 100644
--- a/spec/features/admin/customers_spec.rb
+++ b/spec/features/admin/customers_spec.rb
@@ -5,14 +5,16 @@ feature 'Customers' do
   include WebHelper
 
   context "as an enterprise user" do
-    let(:user) { create_enterprise_user }
-    let(:managed_distributor) { create(:distributor_enterprise, owner: user) }
+    let(:user) { create_enterprise_user(enterprise_limit: 10) }
+    let(:managed_distributor1) { create(:distributor_enterprise, owner: user) }
+    let(:managed_distributor2) { create(:distributor_enterprise, owner: user) }
     let(:unmanaged_distributor) { create(:distributor_enterprise) }
 
     describe "using the customers index", js: true do
-      let!(:customer1) { create(:customer, enterprise: managed_distributor) }
-      let!(:customer2) { create(:customer, enterprise: managed_distributor) }
+      let!(:customer1) { create(:customer, enterprise: managed_distributor1) }
+      let!(:customer2) { create(:customer, enterprise: managed_distributor1) }
       let!(:customer3) { create(:customer, enterprise: unmanaged_distributor) }
+      let!(:customer4) { create(:customer, enterprise: managed_distributor2) }
 
       before do
         quick_login_as user
@@ -21,14 +23,15 @@ feature 'Customers' do
 
       it "passes the smoke test" do
         # Prompts for a hub for a list of my managed enterprises
-        expect(page).to have_select2 "shop_id", with_options: [managed_distributor.name], without_options: [unmanaged_distributor.name]
+        expect(page).to have_select2 "shop_id", with_options: [managed_distributor1.name,managed_distributor2.name], without_options: [unmanaged_distributor.name]
 
-        select2_select managed_distributor.name, from: "shop_id"
+        select2_select managed_distributor1.name, from: "shop_id"
 
         # Loads the right customers
         expect(page).to have_selector "tr#c_#{customer1.id}"
         expect(page).to have_selector "tr#c_#{customer2.id}"
         expect(page).to_not have_selector "tr#c_#{customer3.id}"
+        expect(page).to_not have_selector "tr#c_#{customer4.id}"
 
         # Searching
         fill_in "quick_search", with: customer2.email
@@ -43,10 +46,19 @@ feature 'Customers' do
         first("div#columns-dropdown div.menu div.menu_item", text: "Email").click
         expect(page).to_not have_selector "th.email"
         expect(page).to_not have_content customer1.email
+
+        # Changing Shops
+        select2_select managed_distributor2.name, from: "shop_id"
+
+        # Loads the right customers
+        expect(page).to_not have_selector "tr#c_#{customer1.id}"
+        expect(page).to_not have_selector "tr#c_#{customer2.id}"
+        expect(page).to_not have_selector "tr#c_#{customer3.id}"
+        expect(page).to have_selector "tr#c_#{customer4.id}"
       end
 
       it "allows updating of attributes" do
-        select2_select managed_distributor.name, from: "shop_id"
+        select2_select managed_distributor1.name, from: "shop_id"
 
         within "tr#c_#{customer1.id}" do
           fill_in "code", with: "new-customer-code"
@@ -78,7 +90,7 @@ feature 'Customers' do
 
         context "when a shop is selected" do
           before do
-            select2_select managed_distributor.name, from: "shop_id"
+            select2_select managed_distributor1.name, from: "shop_id"
           end
 
           it "creates customers when the email provided is valid" do
@@ -88,21 +100,21 @@ feature 'Customers' do
               fill_in 'email', with: "not_an_email"
               click_button 'Add Customer'
               expect(page).to have_selector "#new-customer-dialog .error", text: "Please enter a valid email address"
-            }.to_not change{Customer.of(managed_distributor).count}
+            }.to_not change{Customer.of(managed_distributor1).count}
 
             # When an existing email is used
             expect{
               fill_in 'email', with: customer1.email
               click_button 'Add Customer'
               expect(page).to have_selector "#new-customer-dialog .error", text: "Email is associated with an existing customer"
-            }.to_not change{Customer.of(managed_distributor).count}
+            }.to_not change{Customer.of(managed_distributor1).count}
 
             # When a new valid email is used
             expect{
               fill_in 'email', with: "new@email.com"
               click_button 'Add Customer'
               expect(page).not_to have_selector "#new-customer-dialog"
-            }.to change{Customer.of(managed_distributor).count}.from(2).to(3)
+            }.to change{Customer.of(managed_distributor1).count}.from(2).to(3)
           end
         end
       end
diff --git a/spec/javascripts/unit/admin/customers/controllers/customers_controller_spec.js.coffee b/spec/javascripts/unit/admin/customers/controllers/customers_controller_spec.js.coffee
index 7854700dfd..fca4c6744f 100644
--- a/spec/javascripts/unit/admin/customers/controllers/customers_controller_spec.js.coffee
+++ b/spec/javascripts/unit/admin/customers/controllers/customers_controller_spec.js.coffee
@@ -1,6 +1,7 @@
 describe "CustomersCtrl", ->
   scope = null
   http = null
+  shops = null
 
   beforeEach ->
     module('admin.customers')
@@ -8,28 +9,46 @@ describe "CustomersCtrl", ->
       $provide.value 'columns', []
       null
 
+    shops = [
+      { name: "Shop 1", id: 1 }
+      { name: "Shop 2", id: 2 }
+      { name: "Shop 3", id: 3 }
+    ]
+
+
     inject ($controller, $rootScope, _CustomerResource_, $httpBackend) ->
       scope = $rootScope
       http = $httpBackend
-      $controller 'customersCtrl', {$scope: scope, CustomerResource: _CustomerResource_, shops: {}}
+      $controller 'customersCtrl', {$scope: scope, CustomerResource: _CustomerResource_, shops: shops}
     jasmine.addMatchers
       toDeepEqual: (util, customEqualityTesters) ->
         compare: (actual, expected) ->
           { pass: angular.equals(actual, expected) }
 
-  it "has no shop pre-selected", ->
-    expect(scope.CurrentShop.shop).toEqual {}
+  it "has no shop pre-selected", inject (CurrentShop) ->
+    expect(CurrentShop.shop).toEqual {}
 
   describe "setting the shop on scope", ->
     customer = { id: 5, email: 'someone@email.com'}
     customers = [customer]
 
-    beforeEach ->
-      http.expectGET('/admin/customers.json?enterprise_id=1').respond 200, customers
+    beforeEach inject (pendingChanges) ->
+      spyOn(pendingChanges, "removeAll")
+      scope.customers_form = jasmine.createSpyObj('customers_form', ['$setPristine'])
+      http.expectGET('/admin/customers.json?enterprise_id=3').respond 200, customers
       scope.$apply ->
-        scope.CurrentShop.shop = {id: 1}
+        scope.shop_id = 3
       http.flush()
 
+    it "sets the CurrentShop", inject (CurrentShop) ->
+      expect(CurrentShop.shop).toEqual shops[2]
+
+    it "sets the form state to pristine", ->
+      expect(scope.customers_form.$setPristine).toHaveBeenCalled()
+
+    it "clears all changes", inject (pendingChanges) ->
+      expect(pendingChanges.removeAll).toHaveBeenCalled()
+
     it "retrievs the list of customers", ->
       expect(scope.customers).toDeepEqual customers
 
@@ -38,7 +57,7 @@ describe "CustomersCtrl", ->
         email = "customer@example.org"
         newCustomer = {id: 6, email: email}
         customers.unshift(newCustomer)
-        http.expectPOST('/admin/customers.json?email=' + email + '&enterprise_id=1').respond 200, newCustomer
+        http.expectPOST('/admin/customers.json?email=' + email + '&enterprise_id=3').respond 200, newCustomer
         scope.add(email)
         http.flush()
         expect(scope.customers).toDeepEqual customers
@@ -60,7 +79,7 @@ describe "CustomersCtrl", ->
         { text: 'three' }
       ]
       beforeEach ->
-        http.expectGET('/admin/tag_rules/map_by_tag.json?enterprise_id=1').respond 200, tags
+        http.expectGET('/admin/tag_rules/map_by_tag.json?enterprise_id=3').respond 200, tags
 
       it "retrieves the tag list", ->
         promise = scope.findTags('')

From 2a4737147ff2c6bfec2411c1180b89318b51e142 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 16 Jun 2016 16:32:10 +1000
Subject: [PATCH 21/35] Tweaking the way new customer form error messages are
 display for latest version of AngularJS

---
 .../directives/new_customer_dialog.js.coffee          | 11 ++++++-----
 .../templates/admin/new_customer_dialog.html.haml     |  6 +++---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/app/assets/javascripts/admin/customers/directives/new_customer_dialog.js.coffee b/app/assets/javascripts/admin/customers/directives/new_customer_dialog.js.coffee
index 46292ae85c..f2dae5836b 100644
--- a/app/assets/javascripts/admin/customers/directives/new_customer_dialog.js.coffee
+++ b/app/assets/javascripts/admin/customers/directives/new_customer_dialog.js.coffee
@@ -3,18 +3,19 @@ angular.module("admin.customers").directive 'newCustomerDialog', ($compile, $inj
   scope: true
   link: (scope, element, attr) ->
     scope.CurrentShop = CurrentShop
-    scope.submitted = null
+    scope.submitted = false
     scope.email = ""
     scope.errors = []
 
-    scope.addCustomer = (valid) ->
-      scope.submitted = scope.email
+    scope.addCustomer = ->
+      scope.new_customer_form.$setPristine()
+      scope.submitted = true
       scope.errors = []
-      if valid
+      if scope.new_customer_form.$valid
         Customers.add(scope.email).$promise.then (data) ->
           if data.id
             scope.email = ""
-            scope.submitted = null
+            scope.submitted = false
             template.dialog('close')
         , (response) ->
           if response.data.errors
diff --git a/app/assets/javascripts/templates/admin/new_customer_dialog.html.haml b/app/assets/javascripts/templates/admin/new_customer_dialog.html.haml
index e30cfcd607..525e777eac 100644
--- a/app/assets/javascripts/templates/admin/new_customer_dialog.html.haml
+++ b/app/assets/javascripts/templates/admin/new_customer_dialog.html.haml
@@ -2,14 +2,14 @@
   .text-normal.margin-bottom-30.text-center
     = t('admin.customers.index.add_a_new_customer_for', shop_name: "{{ CurrentShop.shop.name }}:")
 
-  %form{ name: 'new_customer_form', novalidate: true }
+  %form{ name: 'new_customer_form', novalidate: true, ng: { submit: "addCustomer()" }}
 
     .text-center.margin-bottom-30
       %input.fullwidth{ type: 'email', name: 'email', required: true, placeholder: t('admin.customers.index.customer_placeholder'), ng: { model: "email" } }
-      %div{ ng: { show: "email == submitted" } }
+      %div{ ng: { show: "submitted && new_customer_form.$pristine" } }
         .error{ ng: { show: "(new_customer_form.email.$error.email || new_customer_form.email.$error.required)" } }
           = t('admin.customers.index.valid_email_error')
         .error{ ng: { repeat: "error in errors", bind: "error" } }
 
     .text-center
-      %input.button.red.icon-plus{ type: 'submit', value: t('admin.customers.index.add_customer'), ng: { click: 'addCustomer(new_customer_form.email.$valid)' } }
+      %input.button.red.icon-plus{ type: 'submit', value: t('admin.customers.index.add_customer') }

From 61969f5c84aab9b156a59a440ea75db42ba9a46f Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 16 Jun 2016 22:42:53 +1000
Subject: [PATCH 22/35] Ensure CustomersController#update.json gives an
 appropriate response

pendingChanges submits an empty string when a field is blank (rather than undefined)
---
 .../directives/obj_for_update.js.coffee       |  2 +-
 app/controllers/admin/customers_controller.rb |  8 +++
 .../admin/customers_controller_spec.rb        |  3 ++
 spec/features/admin/customers_spec.rb         | 50 ++++++++++++++++++-
 4 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee b/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee
index 254fa5c438..81cf58fd1e 100644
--- a/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee
+++ b/app/assets/javascripts/admin/index_utils/directives/obj_for_update.js.coffee
@@ -15,7 +15,7 @@ angular.module("admin.indexUtils").directive "objForUpdate", (switchClass, pendi
           object: scope.object()
           type: scope.type
           attr: scope.attr
-          value: value
+          value: if value? then value else ""
           scope: scope
         scope.pending()
         pendingChanges.add(scope.object().id, scope.attr, change)
diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb
index fe975aa038..433efbf69e 100644
--- a/app/controllers/admin/customers_controller.rb
+++ b/app/controllers/admin/customers_controller.rb
@@ -3,6 +3,14 @@ module Admin
     before_filter :load_managed_shops, only: :index, if: :html_request?
     respond_to :json
 
+    respond_override update: { json: {
+      success: lambda {
+        tag_rule_mapping = TagRule.mapping_for(Enterprise.where(id: @customer.enterprise))
+        render_as_json @customer, tag_rule_mapping: tag_rule_mapping
+      },
+      failure: lambda { render json: { errors: @customer.errors.full_messages }, status: :unprocessable_entity }
+    } }
+
     def index
       respond_to do |format|
         format.html
diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb
index f64a8057e8..3c75929972 100644
--- a/spec/controllers/admin/customers_controller_spec.rb
+++ b/spec/controllers/admin/customers_controller_spec.rb
@@ -69,12 +69,15 @@ describe Admin::CustomersController, type: :controller do
       let!(:customer) { create(:customer, enterprise: enterprise) }
 
       context "where I manage the customer's enterprise" do
+        render_views
+
         before do
           controller.stub spree_current_user: enterprise.owner
         end
 
         it "allows me to update the customer" do
           spree_put :update, format: :json, id: customer.id, customer: { email: 'new.email@gmail.com' }
+          expect(JSON.parse(response.body)["id"]).to eq customer.id
           expect(assigns(:customer)).to eq customer
           expect(customer.reload.email).to eq 'new.email@gmail.com'
         end
diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb
index e0e88af063..0e2665b39b 100644
--- a/spec/features/admin/customers_spec.rb
+++ b/spec/features/admin/customers_spec.rb
@@ -11,8 +11,8 @@ feature 'Customers' do
     let(:unmanaged_distributor) { create(:distributor_enterprise) }
 
     describe "using the customers index", js: true do
-      let!(:customer1) { create(:customer, enterprise: managed_distributor1) }
-      let!(:customer2) { create(:customer, enterprise: managed_distributor1) }
+      let!(:customer1) { create(:customer, enterprise: managed_distributor1, code: nil) }
+      let!(:customer2) { create(:customer, enterprise: managed_distributor1, code: nil) }
       let!(:customer3) { create(:customer, enterprise: unmanaged_distributor) }
       let!(:customer4) { create(:customer, enterprise: managed_distributor2) }
 
@@ -77,6 +77,52 @@ feature 'Customers' do
         # And it actually did
         expect(customer1.reload.code).to eq "new-customer-code"
         expect(customer1.tag_list).to eq ["awesome"]
+
+        # Clearing attributes
+        within "tr#c_#{customer1.id}" do
+          fill_in "code", with: ""
+          expect(page).to have_css "input[name=code].update-pending"
+        end
+        within "tr#c_#{customer1.id}" do
+          find("tags-input li.tag-item a.remove-button").trigger('click')
+          expect(page).to have_css ".tag_watcher.update-pending"
+        end
+        click_button "Save Changes"
+
+        # Every says it updated
+        expect(page).to have_css "input[name=code].update-success"
+        expect(page).to have_css ".tag_watcher.update-success"
+
+        # And it actually did
+        expect(customer1.reload.code).to be nil
+        expect(customer1.tag_list).to eq []
+      end
+
+      it "prevents duplicate codes from being saved" do
+        select2_select managed_distributor1.name, from: "shop_id"
+
+        within "tr#c_#{customer1.id}" do
+          fill_in "code", with: "new-customer-code"
+          expect(page).to have_css "input[name=code].update-pending"
+        end
+        within "tr#c_#{customer2.id}" do
+          fill_in "code", with: "new-customer-code"
+          expect(page).to have_content "This code is used already."
+        end
+        click_button "Save Changes"
+
+        within "tr#c_#{customer1.id}" do
+          expect(page).to have_css "input[name=code].update-success"
+        end
+
+        within "tr#c_#{customer2.id}" do
+          expect(page).to have_css "input[name=code].update-error"
+        end
+
+        expect(page).to have_content "Oh no! I was unable to save your changes"
+
+        expect(customer1.reload.code).to eq "new-customer-code"
+        expect(customer2.reload.code).to be nil
       end
 
       describe "creating a new customer" do

From d9b3366a5cc0c21cfa4abc56a3f7b732bc3aa4e1 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 17 Jun 2016 11:28:20 +1000
Subject: [PATCH 23/35] Auto-select shop on customers index when only one
 available

---
 .../admin/customers/controllers/customers_controller.js.coffee  | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee b/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee
index 50f8301cfc..2721c92961 100644
--- a/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee
+++ b/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee
@@ -18,6 +18,8 @@ angular.module("admin.customers").controller "customersCtrl", ($scope, $q, $filt
         $scope.customers_form.$setPristine()
         $scope.customers = data
 
+  $scope.shop_id = shops[0].id if shops.length == 1
+
   $scope.checkForDuplicateCodes = ->
     delete this.customer.code unless this.customer.code
     this.duplicate = $scope.isDuplicateCode(this.customer.code)

From 6586e67a5c4eec459557c19f64bc45e3de75bc8c Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 17 Jun 2016 14:47:04 +1000
Subject: [PATCH 24/35] Better messaging around deletion of customers

---
 .../customers/services/customers.js.coffee    |  8 ++++-
 .../utils/services/info_dialog.js.coffee      | 12 +++++++
 .../templates/admin/info_dialog.html.haml     |  9 ++++++
 .../admin/components/info_dialog.css.scss     | 28 +++++++++++++++++
 app/controllers/admin/customers_controller.rb |  1 +
 app/models/customer.rb                        |  8 +++++
 spec/features/admin/customers_spec.rb         | 31 ++++++++++++++-----
 7 files changed, 89 insertions(+), 8 deletions(-)
 create mode 100644 app/assets/javascripts/admin/utils/services/info_dialog.js.coffee
 create mode 100644 app/assets/javascripts/templates/admin/info_dialog.html.haml
 create mode 100644 app/assets/stylesheets/admin/components/info_dialog.css.scss

diff --git a/app/assets/javascripts/admin/customers/services/customers.js.coffee b/app/assets/javascripts/admin/customers/services/customers.js.coffee
index a9f4f0102f..cd4c9bbb44 100644
--- a/app/assets/javascripts/admin/customers/services/customers.js.coffee
+++ b/app/assets/javascripts/admin/customers/services/customers.js.coffee
@@ -1,4 +1,4 @@
-angular.module("admin.customers").factory "Customers", ($q, RequestMonitor, CustomerResource, CurrentShop) ->
+angular.module("admin.customers").factory "Customers", ($q, InfoDialog, RequestMonitor, CustomerResource, CurrentShop) ->
   new class Customers
     customers: []
 
@@ -14,6 +14,12 @@ angular.module("admin.customers").factory "Customers", ($q, RequestMonitor, Cust
       CustomerResource.destroy params, =>
         i = @customers.indexOf customer
         @customers.splice i, 1 unless i < 0
+      , (response) =>
+        errors = response.data.errors
+        if errors?
+          InfoDialog.open 'error', errors[0]
+        else
+          InfoDialog.open 'error', "Could not delete customer: #{customer.email}"
 
     index: (params) ->
       request = CustomerResource.index(params, (data) => @customers = data)
diff --git a/app/assets/javascripts/admin/utils/services/info_dialog.js.coffee b/app/assets/javascripts/admin/utils/services/info_dialog.js.coffee
new file mode 100644
index 0000000000..bb63eb7d20
--- /dev/null
+++ b/app/assets/javascripts/admin/utils/services/info_dialog.js.coffee
@@ -0,0 +1,12 @@
+angular.module("admin.customers").factory 'InfoDialog', ($rootScope, $compile, $injector, $templateCache, DialogDefaults) ->
+  new class InfoDialog
+    open: (type, message) ->
+      scope = $rootScope.$new()
+      scope.message = message
+      scope.dialog_class = type
+      template = $compile($templateCache.get('admin/info_dialog.html'))(scope)
+      template.dialog(DialogDefaults)
+      template.dialog('open')
+      scope.close = ->
+        template.dialog('close')
+        null
diff --git a/app/assets/javascripts/templates/admin/info_dialog.html.haml b/app/assets/javascripts/templates/admin/info_dialog.html.haml
new file mode 100644
index 0000000000..2755715e46
--- /dev/null
+++ b/app/assets/javascripts/templates/admin/info_dialog.html.haml
@@ -0,0 +1,9 @@
+#info-dialog{ ng: { class: "dialog_class" } }
+  .message.clearfix.margin-bottom-30
+    .icon.text-center
+      %i.icon-exclamation-sign
+    .text
+      {{ message }}
+  .action-buttons.text-center
+    %button{ ng: { click: "close()" } }
+      OK
diff --git a/app/assets/stylesheets/admin/components/info_dialog.css.scss b/app/assets/stylesheets/admin/components/info_dialog.css.scss
new file mode 100644
index 0000000000..d8295c6813
--- /dev/null
+++ b/app/assets/stylesheets/admin/components/info_dialog.css.scss
@@ -0,0 +1,28 @@
+#info-dialog {
+  .message {
+    .text, .icon {
+      position: relative;
+      float:left;
+      display: inline;
+    }
+
+    .text {
+      padding-top: 10px;
+      width: 80%;
+      font-size: 1rem;
+    }
+
+    .icon {
+      width: 20%;
+      font-size: 2rem;
+    }
+  }
+
+  &.error {
+    .message {
+      .icon {
+        color: #da5354;
+      }
+    }
+  }
+}
diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb
index 433efbf69e..c1f5f6bf9a 100644
--- a/app/controllers/admin/customers_controller.rb
+++ b/app/controllers/admin/customers_controller.rb
@@ -48,6 +48,7 @@ module Admin
         invoke_callbacks(:destroy, :fails)
         respond_with(@object) do |format|
           format.html { redirect_to location_after_destroy }
+          format.json { render json: { errors: @object.errors.full_messages }, status: :conflict }
         end
       end
     end
diff --git a/app/models/customer.rb b/app/models/customer.rb
index b2141df9ec..c208c619ba 100644
--- a/app/models/customer.rb
+++ b/app/models/customer.rb
@@ -3,6 +3,8 @@ class Customer < ActiveRecord::Base
 
   belongs_to :enterprise
   belongs_to :user, class_name: Spree.user_class
+  has_many :orders, class_name: Spree::Order
+  before_destroy :check_for_orders
 
   before_validation :downcase_email
   before_validation :empty_code
@@ -28,4 +30,10 @@ class Customer < ActiveRecord::Base
   def associate_user
     self.user = user || Spree::User.find_by_email(email)
   end
+
+  def check_for_orders
+    return true unless orders.any?
+    errors[:base] << "Delete failed: customer has associated orders"
+    false
+  end
 end
diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb
index 0e2665b39b..61caae507f 100644
--- a/spec/features/admin/customers_spec.rb
+++ b/spec/features/admin/customers_spec.rb
@@ -25,6 +25,15 @@ feature 'Customers' do
         # Prompts for a hub for a list of my managed enterprises
         expect(page).to have_select2 "shop_id", with_options: [managed_distributor1.name,managed_distributor2.name], without_options: [unmanaged_distributor.name]
 
+        select2_select managed_distributor2.name, from: "shop_id"
+
+        # Loads the right customers
+        expect(page).to_not have_selector "tr#c_#{customer1.id}"
+        expect(page).to_not have_selector "tr#c_#{customer2.id}"
+        expect(page).to_not have_selector "tr#c_#{customer3.id}"
+        expect(page).to have_selector "tr#c_#{customer4.id}"
+
+        # Changing Shops
         select2_select managed_distributor1.name, from: "shop_id"
 
         # Loads the right customers
@@ -47,14 +56,22 @@ feature 'Customers' do
         expect(page).to_not have_selector "th.email"
         expect(page).to_not have_content customer1.email
 
-        # Changing Shops
-        select2_select managed_distributor2.name, from: "shop_id"
+        # Deleting
+        create(:order, customer: customer1)
+        expect{
+          within "tr#c_#{customer1.id}" do
+            find("a.delete-customer").click
+          end
+          expect(page).to have_selector "#info-dialog .text", text: "Delete failed: customer has associated orders"
+          click_button "OK"
+        }.to_not change{Customer.count}
 
-        # Loads the right customers
-        expect(page).to_not have_selector "tr#c_#{customer1.id}"
-        expect(page).to_not have_selector "tr#c_#{customer2.id}"
-        expect(page).to_not have_selector "tr#c_#{customer3.id}"
-        expect(page).to have_selector "tr#c_#{customer4.id}"
+        expect{
+          within "tr#c_#{customer2.id}" do
+            find("a.delete-customer").click
+          end
+          expect(page).to_not have_selector "tr#c_#{customer2.id}"
+        }.to change{Customer.count}.by(-1)
       end
 
       it "allows updating of attributes" do

From 1e142aa6285dda5ebd7365a62bdda1f3e945cbf6 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 17 Jun 2016 10:48:18 +1000
Subject: [PATCH 25/35] Refactoring OrderCycleFormApplicator logic for improved
 update speed

---
 .../order_cycle_form_applicator.rb            |  74 ++----
 spec/factories.rb                             |   6 +-
 .../order_cycle_form_applicator_spec.rb       | 214 +++++++++---------
 3 files changed, 130 insertions(+), 164 deletions(-)

diff --git a/lib/open_food_network/order_cycle_form_applicator.rb b/lib/open_food_network/order_cycle_form_applicator.rb
index 2cccc4a4fa..1df429b4fb 100644
--- a/lib/open_food_network/order_cycle_form_applicator.rb
+++ b/lib/open_food_network/order_cycle_form_applicator.rb
@@ -132,71 +132,43 @@ module OpenFoodNetwork
       editable_variants_for_outgoing_exchanges_to(receiver).pluck(:id)
     end
 
-    def find_incoming_exchange(attrs)
-      @order_cycle.exchanges.
-      where(:sender_id => attrs[:enterprise_id], :receiver_id => @order_cycle.coordinator_id, :incoming => true).first
-    end
-
-    def find_outgoing_exchange(attrs)
-      @order_cycle.exchanges.
-      where(:sender_id => @order_cycle.coordinator_id, :receiver_id => attrs[:enterprise_id], :incoming => false).first
-    end
-
-    def persisted_variants_hash(exchange)
-      return {} unless exchange
-
-      # When we have permission to edit a variant, mark it for removal here, assuming it will be included again if that is what the use wants
-      # When we don't have permission to edit a variant and it is already in the exchange, keep it in the exchange.
-      method_name = "editable_variant_ids_for_#{ exchange.incoming? ? 'incoming' : 'outgoing' }_exchange_between"
-      editable = send(method_name, exchange.sender, exchange.receiver)
-      Hash[ exchange.variants.map { |v| [v.id, editable.exclude?(v.id)] } ]
+    def find_exchange(sender_id, receiver_id, incoming)
+      @order_cycle.exchanges.find_by_sender_id_and_receiver_id_and_incoming(sender_id, receiver_id, incoming)
     end
 
     def incoming_exchange_variant_ids(attrs)
-      exchange = find_incoming_exchange(attrs)
-      variants = persisted_variants_hash(exchange)
-
-      sender = exchange.andand.sender || Enterprise.find(attrs[:enterprise_id])
+      sender = Enterprise.find(attrs[:enterprise_id])
       receiver = @order_cycle.coordinator
-      permitted = editable_variant_ids_for_incoming_exchange_between(sender, receiver)
+      exchange = find_exchange(sender.id, receiver.id, true)
 
-      # Only change visibility for variants I have permission to edit
-      attrs[:variants].each do |variant_id, value|
-        variants[variant_id.to_i] = value  if permitted.include?(variant_id.to_i)
-      end
+      requested_ids = attrs[:variants].select{ |k,v| v }.keys.map(&:to_i) # Only the ids the user has requested
+      existing_ids = exchange.present? ? exchange.variants.pluck(:id) : [] # The ids that already exist
+      editable_ids = editable_variant_ids_for_incoming_exchange_between(sender, receiver) # The ids we are allowed to add/remove
 
-      variants_to_a variants
+      result = existing_ids
+
+      result |= (requested_ids & editable_ids) # add any requested & editable ids that are not yet in the exchange
+      result -= ((result & editable_ids) - requested_ids) # remove any editable ids that were not specifically mentioned in the request
+
+      result
     end
 
     def outgoing_exchange_variant_ids(attrs)
-      exchange = find_outgoing_exchange(attrs)
-      variants = persisted_variants_hash(exchange)
-
       sender = @order_cycle.coordinator
-      receiver = exchange.andand.receiver || Enterprise.find(attrs[:enterprise_id])
-      permitted = editable_variant_ids_for_outgoing_exchange_between(sender, receiver)
+      receiver = Enterprise.find(attrs[:enterprise_id])
+      exchange = find_exchange(sender.id, receiver.id, false)
 
-      # Only change visibility for variants I have permission to edit
-      attrs[:variants].each do |variant_id, value|
-        variant_id = variant_id.to_i
+      requested_ids = attrs[:variants].select{ |k,v| v }.keys.map(&:to_i) # Only the ids the user has requested
+      existing_ids = exchange.present? ? exchange.variants.pluck(:id) : [] # The ids that already exist
+      editable_ids = editable_variant_ids_for_outgoing_exchange_between(sender, receiver) # The ids we are allowed to add/remove
 
-        variants = update_outgoing_variants(variants, permitted, variant_id, value)
-      end
+      result = existing_ids
 
-      variants_to_a variants
-    end
+      result |= (requested_ids & editable_ids) # add any requested & editable ids that are not yet in the exchange
+      result -= (result - incoming_variant_ids) # remove any ids not in incoming exchanges
+      result -= ((result & editable_ids) - requested_ids) # remove any editable ids that were not specifically mentioned in the request
 
-    def update_outgoing_variants(variants, permitted, variant_id, value)
-      if !incoming_variant_ids.include? variant_id
-        # When a variant has been removed from incoming but remains
-        # in outgoing, remove it from outgoing too
-        variants[variant_id] = false
-
-      elsif permitted.include? variant_id
-        variants[variant_id] = value
-      end
-
-      variants
+      result
     end
 
     def incoming_variant_ids
diff --git a/spec/factories.rb b/spec/factories.rb
index 6d931ee2d0..5729d6c12f 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -105,10 +105,10 @@ FactoryGirl.define do
   end
 
   factory :exchange, :class => Exchange do
-    order_cycle { OrderCycle.first || FactoryGirl.create(:simple_order_cycle) }
-    sender      { FactoryGirl.create(:enterprise) }
-    receiver    { FactoryGirl.create(:enterprise) }
     incoming    false
+    order_cycle { OrderCycle.first || FactoryGirl.create(:simple_order_cycle) }
+    sender      { incoming ? FactoryGirl.create(:enterprise) : order_cycle.coordinator }
+    receiver    { incoming ? order_cycle.coordinator : FactoryGirl.create(:enterprise) }
   end
 
   factory :variant_override, :class => VariantOverride do
diff --git a/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb b/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb
index fe3155c5e5..80fef2b60e 100644
--- a/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb
+++ b/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb
@@ -144,128 +144,122 @@ module OpenFoodNetwork
         end
       end
 
-      describe "finding alterable exchange variants" do
-        let(:coordinator_mock) { double(:enterprise) }
-        let(:oc) { double(:oc, coordinator: coordinator_mock ) }
+      describe "updating the list of variants for a given outgoing exchange" do
+        let!(:v1) { create(:variant) } # Not Existing + Request Add + Editable + Incoming
+        let!(:v2) { create(:variant) } # Not Existing + Request Add + Not Editable + Incoming
+        let!(:v3) { create(:variant) } # Existing + Request Add + Editable + Incoming
+        let!(:v4) { create(:variant) } # Existing + Not mentioned + Editable + Incoming
+        let!(:v5) { create(:variant) } # Existing + Request Remove + Editable + Incoming
+        let!(:v6) { create(:variant) } # Existing + Request Remove + Not Editable + Incoming
+        let!(:v7) { create(:variant) } # Existing + Request Add + Not Editable + Not Incoming
+        let!(:v8) { create(:variant) } # Existing + Request Add + Editable + Not Incoming
+        let!(:v9) { create(:variant) } # Not Existing + Request Add + Editable + Not Incoming
+        let!(:exchange) { create(:exchange, incoming: false, variant_ids: [v3.id, v4.id, v5.id, v6.id, v7.id, v8.id]) }
+        let!(:oc) { exchange.order_cycle }
+        let!(:enterprise) { exchange.receiver }
+        let!(:coordinator) { oc.coordinator }
         let!(:applicator) { OrderCycleFormApplicator.new(oc, user) }
-
-        describe "converting the existing variants of an exchange to a hash" do
-          context "when nil is passed in" do
-            let(:hash) { applicator.send(:persisted_variants_hash, nil) }
-
-            it "returns an empty hash" do
-              expect(hash).to eq({})
-            end
-          end
-
-          context "when an exchange is passed in" do
-            let(:v1) { create(:variant) }
-            let(:v2) { create(:variant) }
-            let(:v3) { create(:variant) }
-            let(:exchange) { create(:exchange, variants: [v1, v2, v3]) }
-            let(:hash) { applicator.send(:persisted_variants_hash, exchange) }
-
-            before do
-              allow(applicator).to receive(:editable_variant_ids_for_outgoing_exchange_between) { [ v1.id, v2.id ] }
-            end
-
-            it "returns a hash with variant ids as keys" do
-              expect(hash.length).to be 3
-              expect(hash.keys).to include v1.id, v2.id, v3.id
-             end
-
-             it "editable variant ids are set to false" do
-               expect(hash[v1.id]).to be false
-               expect(hash[v2.id]).to be false
-             end
-
-             it "and non-editable variant ids are set to true" do
-               expect(hash[v3.id]).to be true
-             end
-          end
+        let(:ids) do
+          applicator.send(:outgoing_exchange_variant_ids, {
+            :enterprise_id => enterprise.id,
+            :variants => {
+              "#{v1.id}" => true,
+              "#{v2.id}" => true,
+              "#{v3.id}" => true,
+              "#{v5.id}" => false,
+              "#{v6.id}" => false,
+              "#{v7.id}" => true,
+              "#{v8.id}" => true,
+              "#{v9.id}" => true
+            }
+          })
         end
 
-        context "where a matching exchange does not exist" do
-          let(:enterprise_mock) { double(:enterprise) }
 
-          before do
-            applicator.stub(:find_outgoing_exchange) { nil }
-            applicator.stub(:incoming_variant_ids) { [1, 2, 3, 4] }
-            expect(applicator).to receive(:editable_variant_ids_for_outgoing_exchange_between).
-            with(coordinator_mock, enterprise_mock) { [1, 2, 3] }
-          end
-
-          it "converts exchange variant ids hash to an array of ids" do
-            applicator.stub(:persisted_variants_hash) { {} }
-            expect(Enterprise).to receive(:find) { enterprise_mock }
-            ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '2' => false, '3' => true}})
-            expect(ids).to eq [1, 3]
-          end
-
-          it "restricts exchange variant ids to those editable by the current user" do
-            applicator.stub(:persisted_variants_hash) { {4 => true} }
-            expect(Enterprise).to receive(:find) { enterprise_mock }
-            ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '2' => false, '3' => true, '4' => false}})
-            expect(ids).to eq [1, 3, 4]
-          end
-
-          it "overrides existing variants based on submitted data, when user has permission" do
-            applicator.stub(:persisted_variants_hash) { {2 => true} }
-            expect(Enterprise).to receive(:find) { enterprise_mock}
-            ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '2' => false, '3' => true}})
-            expect(ids).to eq [1, 3]
-          end
+        before do
+          allow(applicator).to receive(:incoming_variant_ids) { [v1.id, v2.id, v3.id, v4.id, v5.id, v6.id] }
+          allow(applicator).to receive(:editable_variant_ids_for_outgoing_exchange_between) { [v1.id, v3.id, v4.id, v5.id, v8.id, v9.id]}
         end
 
-        context "where a matching exchange exists" do
-          let(:enterprise_mock) { double(:enterprise) }
-          let(:exchange_mock) { double(:exchange) }
+        it "updates the list of variants for the exchange" do
+          # Adds variants that are editable
+          expect(ids).to include v1.id
 
-          before do
-            applicator.stub(:find_outgoing_exchange) { exchange_mock }
-            applicator.stub(:incoming_variant_ids) { [1, 2, 3, 4] }
-            allow(applicator).to receive(:editable_variant_ids_for_outgoing_exchange_between).
-            with(coordinator_mock, enterprise_mock) { [1, 2, 3] }
-          end
+          # Does not add variants that are not editable
+          expect(ids).to_not include v2.id
 
-          it "converts exchange variant ids hash to an array of ids" do
-            applicator.stub(:persisted_variants_hash) { {} }
-            expect(exchange_mock).to receive(:receiver) { enterprise_mock }
-            ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '2' => false, '3' => true}})
-            expect(ids).to eq [1, 3]
-          end
+          # Keeps existing variants, when they are explicitly mentioned in the request
+          expect(ids).to include v3.id
 
-          it "restricts exchange variant ids to those editable by the current user" do
-            applicator.stub(:persisted_variants_hash) { {4 => true} }
-            expect(exchange_mock).to receive(:receiver) { enterprise_mock }
-            ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '2' => false, '3' => true, '4' => false}})
-            expect(ids).to eq [1, 3, 4]
-          end
+          # Removes existing variants that are editable, when they are not mentioned in the request
+          expect(ids).to_not include v4.id
 
-          it "overrides existing variants based on submitted data, when user has permission" do
-            applicator.stub(:persisted_variants_hash) { {2 => true} }
-            expect(exchange_mock).to receive(:receiver) { enterprise_mock }
-            ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '2' => false, '3' => true}})
-            expect(ids).to eq [1, 3]
-          end
+          # Removes existing variants that are editable, when the request explicitly removes them
+          expect(ids).to_not include v5.id
 
-          it "removes variants which the user has permission to remove and that are not included in the submitted data" do
-            allow(exchange_mock).to receive(:incoming?) { false }
-            allow(exchange_mock).to receive(:variants) { [double(:variant, id: 1), double(:variant, id: 2), double(:variant, id: 3)] }
-            allow(exchange_mock).to receive(:sender) { coordinator_mock }
-            allow(exchange_mock).to receive(:receiver) { enterprise_mock }
-            applicator.stub(:incoming_variant_ids) { [1, 2, 3] }
-            ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '3' => true}})
-            expect(ids).to eq [1, 3]
-          end
+          # Keeps existing variants that are not editable
+          expect(ids).to include v6.id
 
-          it "removes variants which are not included in incoming exchanges" do
-            applicator.stub(:incoming_variant_ids) { [1, 2] }
-            applicator.stub(:persisted_variants_hash) { {3 => true} }
-            expect(exchange_mock).to receive(:receiver) { enterprise_mock }
-            ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '2' => false, '3' => true}})
-            expect(ids).to eq [1]
-          end
+          # Removes existing variants that are not in an incoming exchange, regardless of whether they are not editable
+          expect(ids).to_not include v7.id, v8.id
+
+          # Does not add variants that are not in an incoming exchange
+          expect(ids).to_not include v9.id
+        end
+      end
+
+      describe "updating the list of variants for a given incoming exchange" do
+        let!(:v1) { create(:variant) } # Not Existing + Request Add + Editable
+        let!(:v2) { create(:variant) } # Not Existing + Request Add + Not Editable
+        let!(:v3) { create(:variant) } # Existing + Request Add + Editable
+        let!(:v4) { create(:variant) } # Existing + Request Remove + Not Editable
+        let!(:v5) { create(:variant) } # Existing + Request Remove + Editable
+        let!(:v6) { create(:variant) } # Existing + Request Remove + Not Editable
+        let!(:v7) { create(:variant) } # Existing + Not mentioned + Editable
+        let!(:exchange) { create(:exchange, incoming: true, variant_ids: [v3.id, v4.id, v5.id, v6.id, v7.id]) }
+        let!(:oc) { exchange.order_cycle }
+        let!(:enterprise) { exchange.sender }
+        let!(:coordinator) { oc.coordinator }
+        let!(:applicator) { OrderCycleFormApplicator.new(oc, user) }
+        let(:ids) do
+          applicator.send(:incoming_exchange_variant_ids, {
+            :enterprise_id => enterprise.id,
+            :variants => {
+              "#{v1.id}" => true,
+              "#{v2.id}" => true,
+              "#{v3.id}" => true,
+              "#{v4.id}" => false,
+              "#{v5.id}" => false,
+              "#{v6.id}" => false
+            }
+          })
+        end
+
+        before do
+          allow(applicator).to receive(:editable_variant_ids_for_incoming_exchange_between) { [v1.id, v3.id, v5.id, v7.id]}
+        end
+
+        it "updates the list of variants for the exchange" do
+          # Adds variants that are editable
+          expect(ids).to include v1.id
+
+          # Does not add variants that are not editable
+          expect(ids).to_not include v2.id
+
+          # Keeps existing variants, if they are editable and requested
+          expect(ids).to include v3.id
+
+          # Keeps existing variants if they are non-editable, regardless of request
+          expect(ids).to include v4.id
+
+          # Removes existing variants that are editable, when the request explicitly removes them
+          expect(ids).to_not include v5.id
+
+          # Keeps existing variants that are not editable
+          expect(ids).to include v6.id
+
+          # Removes existing variants that are editable, when they are not mentioned in the request
+          expect(ids).to_not include v7.id
         end
       end
 

From e40ecae681421652d92217551db1d0c55c94bb60 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 17 Jun 2016 11:51:27 +1000
Subject: [PATCH 26/35] Removing inline styles for links dropdown

---
 .../javascripts/templates/admin/links_dropdown.html.haml  | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/assets/javascripts/templates/admin/links_dropdown.html.haml b/app/assets/javascripts/templates/admin/links_dropdown.html.haml
index 1f44f2418c..6d0ba060e2 100644
--- a/app/assets/javascripts/templates/admin/links_dropdown.html.haml
+++ b/app/assets/javascripts/templates/admin/links_dropdown.html.haml
@@ -3,8 +3,8 @@
     %i.icon-check
     Actions
     %i{ 'ng-class' => "expanded && 'icon-caret-up' || !expanded && 'icon-caret-down'" }
-  %div.menu{ 'ng-show' => "expanded", style: 'width: 200px' }
-    %a.menu_item{ 'ng-repeat' => "link in links", href: '{{link.url}}', target: "{{link.target || '_self'}}", data: { method: "{{ link.method || 'get' }}", confirm: "{{link.confirm}}" }, style: 'display: inline-block; width: 100%' }
-      %span{ :style => 'text-align: center; display: inline-block; width: 20%'}
+  %div.menu{ 'ng-show' => "expanded" }
+    %a.menu_item{ 'ng-repeat' => "link in links", href: '{{link.url}}', target: "{{link.target || '_self'}}", data: { method: "{{ link.method || 'get' }}", confirm: "{{link.confirm}}" } }
+      %span
         %i{ ng: { class: "link.icon" } }
-      %span{ style: "display: inline-block; width: auto"} {{ link.name }}
+      %span {{ link.name }}

From d28c0159ab87ec949e641caab6f4f4323521e2e3 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 17 Jun 2016 12:06:01 +1000
Subject: [PATCH 27/35] Use have_selector x, count: y; instead of
 all(x).count.should == y

---
 spec/features/admin/payment_method_spec.rb       | 2 +-
 spec/features/admin/reports_spec.rb              | 2 +-
 spec/features/admin/shipping_methods_spec.rb     | 2 +-
 spec/features/consumer/shopping/checkout_spec.rb | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/spec/features/admin/payment_method_spec.rb b/spec/features/admin/payment_method_spec.rb
index fba8e69401..45703a4d07 100644
--- a/spec/features/admin/payment_method_spec.rb
+++ b/spec/features/admin/payment_method_spec.rb
@@ -135,7 +135,7 @@ feature %q{
       pm2
 
       visit spree.admin_payment_methods_path
-      page.all('td', text: 'Two').count.should == 1
+      page.should have_selector 'td', text: 'Two', count: 1
     end
 
 
diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb
index 2bd39a3e38..e2c991a185 100644
--- a/spec/features/admin/reports_spec.rb
+++ b/spec/features/admin/reports_spec.rb
@@ -124,7 +124,7 @@ feature %q{
       table.sort.should == [
         ["Hub", "Code", "First Name", "Last Name", "Supplier", "Product", "Variant", "Quantity", "TempControlled?"]
       ].sort
-      all('table#listing_orders tbody tr').count.should == 5 # Totals row per order
+      page.should have_selector 'table#listing_orders tbody tr', count: 5 # Totals row per order
     end
 
     scenario "Pack By Supplier" do
diff --git a/spec/features/admin/shipping_methods_spec.rb b/spec/features/admin/shipping_methods_spec.rb
index 646b383c21..46816f5737 100644
--- a/spec/features/admin/shipping_methods_spec.rb
+++ b/spec/features/admin/shipping_methods_spec.rb
@@ -123,7 +123,7 @@ feature 'shipping methods' do
 
       visit spree.admin_shipping_methods_path
 
-      page.all('td', text: 'Two').count.should == 1
+      page.should have_selector 'td', text: 'Two', count: 1
     end
 
     pending "shows me only shipping methods for the enterprise I select" do
diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb
index cbb4d12554..10147b21a4 100644
--- a/spec/features/consumer/shopping/checkout_spec.rb
+++ b/spec/features/consumer/shopping/checkout_spec.rb
@@ -300,7 +300,7 @@ feature "As a consumer I want to check out my cart", js: true do
 
               # Does not show duplicate shipping fee
               visit checkout_path
-              page.all("th", text: "Shipping").count.should == 1
+              page.should have_selector "th", text: "Shipping", count: 1
             end
           end
         end

From fa0cc6f2c82abcf36032a10ed370ade52a98ae50 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Wed, 15 Jun 2016 10:10:40 +1000
Subject: [PATCH 28/35] Add spec for filtering producers by taxon

---
 spec/features/consumer/producers_spec.rb | 57 +++++++++++++++++++-----
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/spec/features/consumer/producers_spec.rb b/spec/features/consumer/producers_spec.rb
index 8a26eaf026..20676c7dbc 100644
--- a/spec/features/consumer/producers_spec.rb
+++ b/spec/features/consumer/producers_spec.rb
@@ -5,22 +5,43 @@ feature %q{
     I want to see a list of producers
     So that I can shop at hubs distributing their products
 }, js: true do
+  include WebHelper
   include UIComponentHelper
-  let!(:producer) { create(:supplier_enterprise) }
-  let!(:invisible_producer) { create(:supplier_enterprise, visible: false) }
-  let(:taxon) { create(:taxon) }
-  let!(:product) { create(:simple_product, supplier: producer, taxons: [taxon]) }
-  let(:shop) { create(:distributor_enterprise) }
-  let!(:er) { create(:enterprise_relationship, parent: shop, child: producer) }
 
-  before do
-    visit producers_path
+  let!(:producer1) { create(:supplier_enterprise) }
+  let!(:producer2) { create(:supplier_enterprise) }
+  let!(:invisible_producer) { create(:supplier_enterprise, visible: false) }
+
+  let(:taxon_fruit) { create(:taxon, name: 'Fruit') }
+  let(:taxon_veg) { create(:taxon, name: 'Vegetables') }
+
+  let!(:product1) { create(:simple_product, supplier: producer1, taxons: [taxon_fruit]) }
+  let!(:product2) { create(:simple_product, supplier: producer2, taxons: [taxon_veg]) }
+
+  let(:shop) { create(:distributor_enterprise) }
+  let!(:er) { create(:enterprise_relationship, parent: shop, child: producer1) }
+
+  before { visit producers_path }
+
+  it "filters by taxon" do
+    toggle_filters
+
+    toggle_filter 'Vegetables'
+
+    page.should_not have_content producer1.name
+    page.should     have_content producer2.name
+
+    toggle_filter 'Vegetables'
+    toggle_filter 'Fruit'
+
+    page.should     have_content producer1.name
+    page.should_not have_content producer2.name
   end
 
   it "shows all producers with expandable details" do
-    page.should have_content producer.name
-    expand_active_table_node producer.name
-    page.should have_content producer.supplied_taxons.first.name.split.map(&:capitalize).join(' ')
+    page.should have_content producer1.name
+    expand_active_table_node producer1.name
+    page.should have_content producer1.supplied_taxons.first.name.split.map(&:capitalize).join(' ')
   end
 
   it "doesn't show invisible producers" do
@@ -28,7 +49,19 @@ feature %q{
   end
 
   it "links to places to buy produce" do
-    expand_active_table_node producer.name
+    expand_active_table_node producer1.name
     page.should have_link shop.name
   end
+
+
+  private
+
+  def toggle_filters
+    find('a.filterbtn').click
+  end
+
+  def toggle_filter(name)
+    page.find('span', text: name).click
+  end
+
 end

From 4338f632f6af67534f9c28ca09012c538108c894 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 16 Jun 2016 10:43:53 +1000
Subject: [PATCH 29/35] Add scope: Spree::Property.applied_by

---
 app/models/spree/property_decorator.rb |  6 ++++++
 spec/models/spree/property_spec.rb     | 30 ++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/app/models/spree/property_decorator.rb b/app/models/spree/property_decorator.rb
index 5b8e53338c..f6eaefc75f 100644
--- a/app/models/spree/property_decorator.rb
+++ b/app/models/spree/property_decorator.rb
@@ -1,5 +1,11 @@
 module Spree
   Property.class_eval do
+    scope :applied_by, ->(enterprise) {
+      select('DISTINCT spree_properties.*').
+        joins(:product_properties).
+        where('spree_product_properties.product_id IN (?)', enterprise.supplied_product_ids)
+    }
+
     after_save :refresh_products_cache
 
     # When a Property is destroyed, dependent-destroy will destroy all ProductProperties,
diff --git a/spec/models/spree/property_spec.rb b/spec/models/spree/property_spec.rb
index a86513b5a9..6867b9de1f 100644
--- a/spec/models/spree/property_spec.rb
+++ b/spec/models/spree/property_spec.rb
@@ -2,6 +2,36 @@ require 'spec_helper'
 
 module Spree
   describe Property do
+    describe "scopes" do
+      describe ".applied_by" do
+        let(:producer) { create(:supplier_enterprise) }
+        let(:producer_other) { create(:supplier_enterprise) }
+        let(:product) { create(:simple_product, supplier: producer) }
+        let(:product_other_producer) { create(:simple_product, supplier: producer_other) }
+        let(:product_other_property) { create(:simple_product, supplier: producer) }
+        let(:property) { product.properties.last }
+        let(:property_other) { product_other_producer.properties.last }
+
+        before do
+          product.set_property                'Organic', 'NASAA 12345'
+          product_other_property.set_property 'Organic', 'NASAA 12345'
+          product_other_producer.set_property 'Biodynamic', 'ASDF 1234'
+        end
+
+        it "returns properties applied to supplied products" do
+          expect(Spree::Property.applied_by(producer)).to eq [property]
+        end
+
+        it "doesn't return properties not applied" do
+          expect(Spree::Property.applied_by(producer)).not_to include property_other
+        end
+
+        it "doesn't return duplicates" do
+          expect(Spree::Property.applied_by(producer).to_a.count).to eq 1
+        end
+      end
+    end
+
     describe "callbacks" do
       let(:property) { product_property.property }
       let(:product) { product_property.product }

From aae1689a27a082c111e93ed43e3810eef848d9d7 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 16 Jun 2016 11:00:20 +1000
Subject: [PATCH 30/35] Show product properties on producers page

---
 .../darkswarm/producer_node.css.sass            | 17 +++++++----------
 .../stylesheets/darkswarm/taxons.css.sass       |  4 ++--
 app/serializers/api/enterprise_serializer.rb    |  7 ++++++-
 app/serializers/api/property_serializer.rb      |  2 +-
 app/views/producers/_fat.html.haml              |  6 ++++++
 spec/features/consumer/producers_spec.rb        | 11 +++++++++--
 6 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/app/assets/stylesheets/darkswarm/producer_node.css.sass b/app/assets/stylesheets/darkswarm/producer_node.css.sass
index 7bac2dde6d..78209845c5 100644
--- a/app/assets/stylesheets/darkswarm/producer_node.css.sass
+++ b/app/assets/stylesheets/darkswarm/producer_node.css.sass
@@ -5,7 +5,7 @@
   .active_table .active_table_node
 
     // Header row
-    @media all and (max-width: 640px) 
+    @media all and (max-width: 640px)
       .skinny-head
         background-color: $clr-turquoise-light
         @include border-radius-mixed(0.5em, 0.5em, 0, 0)
@@ -16,7 +16,7 @@
       .follow-icons
         &, & *
           font-size: 1.5rem
-        
+
 
     // Producer icons
     i.ofn-i_059-producer, i.ofn-i_060-producer-reversed
@@ -41,11 +41,11 @@
       &:hover, &:focus, &:active
         &.secondary
           color: #666
-          .hub-name, .button-address 
+          .hub-name, .button-address
             border-bottom: 1px solid #999
         &.primary
           color: $clr-brick-bright
-          .hub-name, .button-address 
+          .hub-name, .button-address
             border-bottom: 1px solid $clr-brick-bright
 
     p.word-wrap
@@ -53,7 +53,7 @@
       &:last-child
         margin-bottom: 1rem
 
-    .fat-taxons
+    .fat-taxons, .fat-properties
       background-color: $clr-turquoise-light
 
     .producer-name
@@ -72,7 +72,7 @@
           max-height: 160px
           width: auto
           &.left
-            padding: 0.25rem 1rem 0.25rem 0 
+            padding: 0.25rem 1rem 0.25rem 0
           &.right
             padding: 0.25rem 0.5rem 0.25rem 2rem
 
@@ -87,10 +87,7 @@
     &.closed
       .active_table_row.closed
         border: 1px solid transparent
-        @media all and (max-width: 640px) 
+        @media all and (max-width: 640px)
           border-color: $clr-turquoise-light
         &:hover, &:active, &:focus
           border-color: $clr-turquoise
-
-
-
diff --git a/app/assets/stylesheets/darkswarm/taxons.css.sass b/app/assets/stylesheets/darkswarm/taxons.css.sass
index 4f23a8bc3a..1d1e5b652f 100644
--- a/app/assets/stylesheets/darkswarm/taxons.css.sass
+++ b/app/assets/stylesheets/darkswarm/taxons.css.sass
@@ -1,7 +1,7 @@
 @import branding
 @import mixins
 
-.fat-taxons
+.fat-taxons, .fat-properties
   display: inline-block
   line-height: 1
   margin-right: 0.5rem
@@ -29,7 +29,7 @@
     svg
       path
         fill: $disabled-dark
-     
+
 .product-header
   render-svg
     svg
diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb
index 3210c1d2af..b0f4438e90 100644
--- a/app/serializers/api/enterprise_serializer.rb
+++ b/app/serializers/api/enterprise_serializer.rb
@@ -47,7 +47,7 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer
   attributes :taxons, :supplied_taxons
 
   has_one :address, serializer: Api::AddressSerializer
-
+  has_many :properties, serializer: Api::PropertySerializer
 
   def taxons
     ids_to_objs options[:data].distributed_taxons[object.id]
@@ -57,6 +57,11 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer
     ids_to_objs options[:data].supplied_taxons[object.id]
   end
 
+  def properties
+    # This results in 2 queries per enterprise
+    Spree::Property.applied_by(object)
+  end
+
   def pickup
     services = options[:data].shipping_method_services[object.id]
     services ? services[:pickup] : false
diff --git a/app/serializers/api/property_serializer.rb b/app/serializers/api/property_serializer.rb
index ce04480d30..7da4fce990 100644
--- a/app/serializers/api/property_serializer.rb
+++ b/app/serializers/api/property_serializer.rb
@@ -1,3 +1,3 @@
 class Api::PropertySerializer < ActiveModel::Serializer
-
+  attributes :id, :name, :presentation
 end
diff --git a/app/views/producers/_fat.html.haml b/app/views/producers/_fat.html.haml
index 6b665b4f34..0fc082b361 100644
--- a/app/views/producers/_fat.html.haml
+++ b/app/views/producers/_fat.html.haml
@@ -20,6 +20,12 @@
           %render-svg{path: "{{taxon.icon}}"}
           %span{"ng-bind" => "::taxon.name"}
 
+    %div
+      %label Product properties
+      %p.trans-sentence
+        %span.fat-properties{"ng-repeat" => "property in producer.properties"}
+          %span{"ng-bind" => "property.presentation"}
+
     %div.show-for-medium-up{"ng-if" => "producer.supplied_taxons.length==0"}
       &nbsp;
 
diff --git a/spec/features/consumer/producers_spec.rb b/spec/features/consumer/producers_spec.rb
index 20676c7dbc..4f3b4f7067 100644
--- a/spec/features/consumer/producers_spec.rb
+++ b/spec/features/consumer/producers_spec.rb
@@ -21,7 +21,13 @@ feature %q{
   let(:shop) { create(:distributor_enterprise) }
   let!(:er) { create(:enterprise_relationship, parent: shop, child: producer1) }
 
-  before { visit producers_path }
+  before do
+    product1.set_property 'Organic', 'NASAA 12345'
+    product2.set_property 'Biodynamic', 'ABC123'
+
+    visit producers_path
+  end
+
 
   it "filters by taxon" do
     toggle_filters
@@ -41,7 +47,8 @@ feature %q{
   it "shows all producers with expandable details" do
     page.should have_content producer1.name
     expand_active_table_node producer1.name
-    page.should have_content producer1.supplied_taxons.first.name.split.map(&:capitalize).join(' ')
+    page.should have_content 'Fruit'
+    page.should have_content 'Organic'
   end
 
   it "doesn't show invisible producers" do

From 58379a5e28a0a6b1e71696d9a8a2d591f6d4cd2e Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 17 Jun 2016 11:14:39 +1000
Subject: [PATCH 31/35] Extract property merging to lib class

---
 app/models/spree/product_decorator.rb         |  7 ++----
 lib/open_food_network/property_merge.rb       | 18 +++++++++++++
 spec/factories.rb                             |  6 +++++
 .../open_food_network/property_merge_spec.rb  | 25 +++++++++++++++++++
 spec/models/spree/product_spec.rb             |  2 +-
 5 files changed, 52 insertions(+), 6 deletions(-)
 create mode 100644 lib/open_food_network/property_merge.rb
 create mode 100644 spec/lib/open_food_network/property_merge_spec.rb

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index e6acd440c7..f8c177ceac 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -1,4 +1,5 @@
 require 'open_food_network/permalink_generator'
+require 'open_food_network/property_merge'
 
 Spree::Product.class_eval do
   include PermalinkGenerator
@@ -128,11 +129,7 @@ Spree::Product.class_eval do
     ps = product_properties.all
 
     if inherits_properties
-      supplier.producer_properties.each do |producer_property|
-        unless ps.find { |product_property| product_property.property.presentation == producer_property.property.presentation }
-          ps << producer_property
-        end
-      end
+      ps = OpenFoodNetwork::PropertyMerge.merge(ps, supplier.producer_properties)
     end
 
     ps.
diff --git a/lib/open_food_network/property_merge.rb b/lib/open_food_network/property_merge.rb
new file mode 100644
index 0000000000..9c21fb7b0d
--- /dev/null
+++ b/lib/open_food_network/property_merge.rb
@@ -0,0 +1,18 @@
+module OpenFoodNetwork
+  class PropertyMerge
+    def self.merge(primary, secondary)
+      primary + secondary.reject do |secondary_p|
+        primary.any? do |primary_p|
+          property_of(primary_p).presentation == property_of(secondary_p).presentation
+        end
+      end
+    end
+
+
+    private
+
+    def self.property_of(p)
+      p.respond_to?(:property) ? p.property : p
+    end
+  end
+end
diff --git a/spec/factories.rb b/spec/factories.rb
index 5729d6c12f..06f5dbd879 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -259,6 +259,12 @@ FactoryGirl.define do
     end
   end
 
+  factory :producer_property, class: ProducerProperty do
+    value 'abc123'
+    producer { create(:supplier_enterprise) }
+    property
+  end
+
   factory :customer, :class => Customer do
     email { Faker::Internet.email }
     enterprise
diff --git a/spec/lib/open_food_network/property_merge_spec.rb b/spec/lib/open_food_network/property_merge_spec.rb
new file mode 100644
index 0000000000..85f4200ab0
--- /dev/null
+++ b/spec/lib/open_food_network/property_merge_spec.rb
@@ -0,0 +1,25 @@
+require 'spec_helper'
+
+module OpenFoodNetwork
+  describe PropertyMerge do
+    let(:p1a) { create(:property, presentation: 'One') }
+    let(:p1b) { create(:property, presentation: 'One') }
+    let(:p2)  { create(:property, presentation: 'Two') }
+
+    describe "merging Spree::Properties" do
+      it "merges properties" do
+        expect(PropertyMerge.merge([p1a], [p1b, p2])).to eq [p1a, p2]
+      end
+    end
+
+    describe "merging ProducerProperties and Spree::ProductProperties" do
+      let(:pp1a) { create(:product_property,  property: p1a) }
+      let(:pp1b) { create(:producer_property, property: p1b) }
+      let(:pp2)  { create(:producer_property, property: p2) }
+
+      it "merges properties" do
+        expect(PropertyMerge.merge([pp1a], [pp1b, pp2])).to eq [pp1a, pp2]
+      end
+    end
+  end
+end
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 9c2ee6b664..10e67e5900 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -420,7 +420,7 @@ module Spree
         end
       end
 
-      context "when product has an inherit_properties value set to true" do
+      context "when product has an inherit_properties value set to false" do
         let(:supplier) { create(:supplier_enterprise) }
         let(:product) { create(:simple_product, supplier: supplier, inherits_properties: false) }
 

From 4134cbfc9c3e142070743dc846810d68ee22a1b4 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 17 Jun 2016 11:19:06 +1000
Subject: [PATCH 32/35] Include producer properties on producer listing

---
 app/serializers/api/enterprise_serializer.rb |  9 +++++++--
 spec/features/consumer/producers_spec.rb     | 10 +++++++++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb
index b0f4438e90..127aee1a08 100644
--- a/app/serializers/api/enterprise_serializer.rb
+++ b/app/serializers/api/enterprise_serializer.rb
@@ -1,3 +1,5 @@
+require 'open_food_network/property_merge'
+
 class Api::EnterpriseSerializer < ActiveModel::Serializer
   # We reference this here because otherwise the serializer complains about its absence
   Api::IdSerializer
@@ -58,8 +60,11 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer
   end
 
   def properties
-    # This results in 2 queries per enterprise
-    Spree::Property.applied_by(object)
+    # This results in 3 queries per enterprise
+    product_properties  = Spree::Property.applied_by(object)
+    producer_properties = object.properties
+
+    OpenFoodNetwork::PropertyMerge.merge product_properties, producer_properties
   end
 
   def pickup
diff --git a/spec/features/consumer/producers_spec.rb b/spec/features/consumer/producers_spec.rb
index 4f3b4f7067..634350af18 100644
--- a/spec/features/consumer/producers_spec.rb
+++ b/spec/features/consumer/producers_spec.rb
@@ -25,6 +25,9 @@ feature %q{
     product1.set_property 'Organic', 'NASAA 12345'
     product2.set_property 'Biodynamic', 'ABC123'
 
+    producer1.set_producer_property 'Local', 'Victoria'
+    producer2.set_producer_property 'Fair Trade', 'FT123'
+
     visit producers_path
   end
 
@@ -47,8 +50,13 @@ feature %q{
   it "shows all producers with expandable details" do
     page.should have_content producer1.name
     expand_active_table_node producer1.name
+
+    # -- Taxons
     page.should have_content 'Fruit'
-    page.should have_content 'Organic'
+
+    # -- Properties
+    page.should have_content 'Organic' # Product property
+    page.should have_content 'Local'   # Producer property
   end
 
   it "doesn't show invisible producers" do

From a5a00e9cef15e1042b16ee4eb8f725335946dd53 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 17 Jun 2016 11:53:58 +1000
Subject: [PATCH 33/35] Show taxons and properties on producer modal (seen on
 map, shop producer info)

---
 .../partials/enterprise_details.html.haml     | 28 +++++++------------
 spec/features/consumer/shops_spec.rb          | 22 +++++++++++----
 2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/app/assets/javascripts/templates/partials/enterprise_details.html.haml b/app/assets/javascripts/templates/partials/enterprise_details.html.haml
index d1d208c5c4..46a37050e2 100644
--- a/app/assets/javascripts/templates/partials/enterprise_details.html.haml
+++ b/app/assets/javascripts/templates/partials/enterprise_details.html.haml
@@ -1,26 +1,18 @@
 .row
   .small-12.large-8.columns
-    / TODO: Rob add logic for taxons and properties too:
-    / %div{"ng-if" => "enterprise.long_description.length > 0 || enterprise.logo"}
     %div
       %p.modal-header {{'label_about' | t}}
-      / TODO: Rob - add in taxons and properties and property pop-overs
+      %div
+        %span.filter-shopfront.taxon-selectors
+          %ul.inline-block
+            %li{"ng-repeat" => "taxon in enterprise.supplied_taxons"}
+              %a.button.tiny.disabled{"ng-bind" => "taxon.name"}
+
+        %span.filter-shopfront.property-selectors.pad-top
+          %ul.inline-block
+            %li{"ng-repeat" => "property in enterprise.properties"}
+              %a.button.tiny{"ng-bind" => "property.presentation"}
 
-      -# TODO: Add producer taxons and properties here
-      -# %div
-      -#   %span.filter-shopfront.taxon-selectors
-      -#     %ul.inline-block
-      -#       %li
-      -#         %a.button.tiny.disabled Grains
-      -#       %li
-      -#         %a.button.tiny.disabled Dairy
-      -#
-      -#   %span.filter-shopfront.property-selectors.pad-top
-      -#     %ul.inline-block
-      -#       %li
-      -#         %a.button.tiny Organic certified
-      -#         / TODO: Rob - need popover, use will's directive or this? http://pineconellc.github.io/angular-foundation/
-      -#
       .about-container.pad-top
         %img.enterprise-logo{"ng-src" => "{{::enterprise.logo}}", "ng-if" => "::enterprise.logo"}
         %p.text-small{"ng-bind-html" => "::enterprise.long_description"}
diff --git a/spec/features/consumer/shops_spec.rb b/spec/features/consumer/shops_spec.rb
index df44955232..8b1a709462 100644
--- a/spec/features/consumer/shops_spec.rb
+++ b/spec/features/consumer/shops_spec.rb
@@ -13,6 +13,7 @@ feature 'Shops', js: true do
   let!(:er) { create(:enterprise_relationship, parent: distributor, child: producer) }
 
   before do
+    producer.set_producer_property 'Organic', 'NASAA 12345'
     visit shops_path
   end
 
@@ -45,11 +46,22 @@ feature 'Shops', js: true do
     expect(page).to have_current_path enterprise_shop_path(distributor)
   end
 
-  it "should show hub producer modals" do
-    expand_active_table_node distributor.name
-    expect(page).to have_content producer.name
-    open_enterprise_modal producer
-    modal_should_be_open_for producer
+  describe "hub producer modal" do
+    let!(:product) { create(:simple_product, supplier: producer, taxons: [taxon]) }
+    let!(:taxon) { create(:taxon, name: 'Fruit') }
+    let!(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], coordinator: create(:distributor_enterprise), variants: [product.variants.first]) }
+
+    it "should show hub producer modals" do
+      expand_active_table_node distributor.name
+      expect(page).to have_content producer.name
+      open_enterprise_modal producer
+      modal_should_be_open_for producer
+
+      within ".reveal-modal" do
+        expect(page).to have_content 'Fruit'   # Taxon
+        expect(page).to have_content 'Organic' # Producer property
+      end
+    end
   end
 
   def click_link_and_ensure(link_text, check)

From 9cc0bb831acecd04d7579456707b8b0fe6b4a7c2 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 17 Jun 2016 17:44:46 +1000
Subject: [PATCH 34/35] Show properties alongside taxons on producer fat view

---
 .../stylesheets/darkswarm/producer_node.css.sass |  6 +++++-
 app/views/producers/_fat.html.haml               | 16 +++++++---------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/app/assets/stylesheets/darkswarm/producer_node.css.sass b/app/assets/stylesheets/darkswarm/producer_node.css.sass
index 78209845c5..24eb0467c2 100644
--- a/app/assets/stylesheets/darkswarm/producer_node.css.sass
+++ b/app/assets/stylesheets/darkswarm/producer_node.css.sass
@@ -53,9 +53,13 @@
       &:last-child
         margin-bottom: 1rem
 
-    .fat-taxons, .fat-properties
+    .fat-taxons
       background-color: $clr-turquoise-light
 
+    .fat-properties
+      background-color: $clr-turquoise-ultra-light
+      border: 1px solid $clr-turquoise-light
+
     .producer-name
       color: $clr-turquoise
 
diff --git a/app/views/producers/_fat.html.haml b/app/views/producers/_fat.html.haml
index 0fc082b361..28ed0c985f 100644
--- a/app/views/producers/_fat.html.haml
+++ b/app/views/producers/_fat.html.haml
@@ -16,15 +16,13 @@
       %label
         = t :producers_buy
       %p.trans-sentence
-        %span.fat-taxons{"ng-repeat" => "taxon in producer.supplied_taxons"}
-          %render-svg{path: "{{taxon.icon}}"}
-          %span{"ng-bind" => "::taxon.name"}
-
-    %div
-      %label Product properties
-      %p.trans-sentence
-        %span.fat-properties{"ng-repeat" => "property in producer.properties"}
-          %span{"ng-bind" => "property.presentation"}
+        %div
+          %span.fat-taxons{"ng-repeat" => "taxon in producer.supplied_taxons"}
+            %render-svg{path: "{{taxon.icon}}"}
+            %span{"ng-bind" => "::taxon.name"}
+        %div
+          %span.fat-properties{"ng-repeat" => "property in producer.properties"}
+            %span{"ng-bind" => "property.presentation"}
 
     %div.show-for-medium-up{"ng-if" => "producer.supplied_taxons.length==0"}
       &nbsp;

From 78b22c4a823f6a3db72400c417ec09c442938da4 Mon Sep 17 00:00:00 2001
From: Bing Xie <bing708@gmail.com>
Date: Wed, 22 Jun 2016 14:54:06 +1000
Subject: [PATCH 35/35] Fix incorrectly aligned columns

---
 .../orders_and_fulfillments_report.rb         |  1 +
 .../orders_and_fulfillments_report_spec.rb    | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/lib/open_food_network/orders_and_fulfillments_report.rb b/lib/open_food_network/orders_and_fulfillments_report.rb
index 089838a1b1..b4778a12aa 100644
--- a/lib/open_food_network/orders_and_fulfillments_report.rb
+++ b/lib/open_food_network/orders_and_fulfillments_report.rb
@@ -216,6 +216,7 @@ module OpenFoodNetwork
           proc { |line_items| "" },
           proc { |line_items| "" },
           proc { |line_items| "" },
+          proc { |line_items| line_items.all? { |li| li.order.paid? } ? "Yes" : "No" },
 
           proc { |line_items| line_items.first.order.shipping_method.andand.name },
           proc { |line_items| rsa.call(line_items) ? 'Y' : 'N' },
diff --git a/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb b/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb
index 55c8bd1197..f37dcefa02 100644
--- a/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb
+++ b/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb
@@ -93,5 +93,24 @@ module OpenFoodNetwork
         end
       end
     end
+
+    describe "columns are aligned" do
+      let(:d1) { create(:distributor_enterprise) }
+      let(:oc1) { create(:simple_order_cycle) }
+      let(:o1) { create(:order, completed_at: 1.day.ago, order_cycle: oc1, distributor: d1) }
+      let(:li1) { build(:line_item) }
+      let(:user) { create(:admin_user)}
+
+      before { o1.line_items << li1 }
+
+      it 'has aligned columsn' do
+        report_types = ["", "order_cycle_supplier_totals", "order_cycle_supplier_totals_by_distributor", "order_cycle_distributor_totals_by_supplier", "order_cycle_customer_totals"]
+
+        report_types.each do |report_type|
+          report = OrdersAndFulfillmentsReport.new user, report_type: report_type
+          report.header.size.should == report.columns.size
+        end
+      end
+    end
   end
 end