From 77e74c56421bbb97cc9241d8a7680c68c052c2f5 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 11 Feb 2016 17:35:15 +1100
Subject: [PATCH] OC Coordinators can opt to restrict products in an order
 cycle to those in their inventory only

---
 .../admin/advanced_settings.css.scss          | 19 ++++++
 .../admin/enterprises_controller.rb           |  8 +--
 .../admin/order_cycles_controller.rb          |  6 +-
 app/models/order_cycle.rb                     |  3 +
 app/models/spree/product_decorator.rb         |  7 ++
 .../api/admin/exchange_serializer.rb          | 45 ++++++++-----
 .../for_order_cycle/enterprise_serializer.rb  | 24 ++++++-
 .../supplied_product_serializer.rb            | 13 +++-
 .../form/_inventory_settings.html.haml        |  2 +-
 .../order_cycles/_advanced_settings.html.haml | 22 +++++++
 ...change_distributed_products_form.html.haml |  5 +-
 app/views/admin/order_cycles/edit.html.haml   | 22 ++++++-
 config/locales/en.yml                         |  7 ++
 .../admin/order_cycles_controller_spec.rb     | 26 +++++++-
 spec/models/order_cycle_spec.rb               |  4 +-
 spec/models/spree/product_spec.rb             | 16 +++++
 spec/models/spree/variant_spec.rb             |  2 +-
 .../admin/exchange_serializer_spec.rb         | 66 +++++++++++++------
 .../enterprise_serializer_spec.rb             | 49 +++++++++++---
 .../supplied_product_serializer_spec.rb       | 38 +++++++++++
 20 files changed, 322 insertions(+), 62 deletions(-)
 create mode 100644 app/assets/stylesheets/admin/advanced_settings.css.scss
 create mode 100644 app/views/admin/order_cycles/_advanced_settings.html.haml
 create mode 100644 spec/serializers/admin/for_order_cycle/supplied_product_serializer_spec.rb

diff --git a/app/assets/stylesheets/admin/advanced_settings.css.scss b/app/assets/stylesheets/admin/advanced_settings.css.scss
new file mode 100644
index 0000000000..6b48e8ce11
--- /dev/null
+++ b/app/assets/stylesheets/admin/advanced_settings.css.scss
@@ -0,0 +1,19 @@
+#advanced_settings {
+  background-color: #eff5fc;
+  border: 1px solid #cee1f4;
+  margin-bottom: 20px;
+
+  .row{
+    margin: 0px -4px;
+
+    padding: 20px 0px;
+
+    .column.alpha, .columns.alpha {
+      padding-left: 20px;
+    }
+
+    .column.omega, .columns.omega {
+      padding-right: 20px;
+    }
+  }
+}
diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb
index 13d5772385..55eaabd7d8 100644
--- a/app/controllers/admin/enterprises_controller.rb
+++ b/app/controllers/admin/enterprises_controller.rb
@@ -96,7 +96,7 @@ module Admin
     def for_order_cycle
       respond_to do |format|
         format.json do
-          render json: @collection, each_serializer: Api::Admin::ForOrderCycle::EnterpriseSerializer, spree_current_user: spree_current_user
+          render json: @collection, each_serializer: Api::Admin::ForOrderCycle::EnterpriseSerializer, order_cycle: @order_cycle, spree_current_user: spree_current_user
         end
       end
     end
@@ -138,10 +138,10 @@ module Admin
     def collection
       case action
       when :for_order_cycle
-        order_cycle = OrderCycle.find_by_id(params[:order_cycle_id]) if params[:order_cycle_id]
+        @order_cycle = OrderCycle.find_by_id(params[:order_cycle_id]) if params[:order_cycle_id]
         coordinator = Enterprise.find_by_id(params[:coordinator_id]) if params[:coordinator_id]
-        order_cycle = OrderCycle.new(coordinator: coordinator) if order_cycle.nil? && coordinator.present?
-        return OpenFoodNetwork::OrderCyclePermissions.new(spree_current_user, order_cycle).visible_enterprises
+        @order_cycle = OrderCycle.new(coordinator: coordinator) if @order_cycle.nil? && coordinator.present?
+        return OpenFoodNetwork::OrderCyclePermissions.new(spree_current_user, @order_cycle).visible_enterprises
       when :index
         if spree_current_user.admin?
           OpenFoodNetwork::Permissions.new(spree_current_user).
diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb
index 300a2ca24e..1d5d018522 100644
--- a/app/controllers/admin/order_cycles_controller.rb
+++ b/app/controllers/admin/order_cycles_controller.rb
@@ -60,8 +60,12 @@ module Admin
 
       respond_to do |format|
         if @order_cycle.update_attributes(params[:order_cycle])
-          OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, spree_current_user).go!
+          unless params[:order_cycle][:incoming_exchanges].nil? && params[:order_cycle][:outgoing_exchanges].nil?
+            # Only update apply exchange information if it is actually submmitted
+            OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, spree_current_user).go!
+          end
           flash[:notice] = 'Your order cycle has been updated.' if params[:reloading] == '1'
+          format.html { redirect_to main_app.edit_admin_order_cycle_path(@order_cycle) }
           format.json { render :json => {:success => true}  }
         else
           format.json { render :json => {:success => false} }
diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb
index 03979220d6..2594c132bf 100644
--- a/app/models/order_cycle.rb
+++ b/app/models/order_cycle.rb
@@ -11,6 +11,8 @@ class OrderCycle < ActiveRecord::Base
 
   validates_presence_of :name, :coordinator_id
 
+  preference :product_selection_from_coordinator_inventory_only, :boolean, default: false
+
   scope :active, lambda { where('order_cycles.orders_open_at <= ? AND order_cycles.orders_close_at >= ?', Time.zone.now, Time.zone.now) }
   scope :active_or_complete, lambda { where('order_cycles.orders_open_at <= ?', Time.zone.now) }
   scope :inactive, lambda { where('order_cycles.orders_open_at > ? OR order_cycles.orders_close_at < ?', Time.zone.now, Time.zone.now) }
@@ -113,6 +115,7 @@ class OrderCycle < ActiveRecord::Base
     oc.name = "COPY OF #{oc.name}"
     oc.orders_open_at = oc.orders_close_at = nil
     oc.coordinator_fee_ids = self.coordinator_fee_ids
+    oc.preferred_product_selection_from_coordinator_inventory_only = self.preferred_product_selection_from_coordinator_inventory_only
     oc.save!
     self.exchanges.each { |e| e.clone!(oc) }
     oc.reload
diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index b089c9a52b..c7e1cfb68d 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -52,6 +52,13 @@ Spree::Product.class_eval do
 
   scope :with_order_cycles_inner, joins(:variants_including_master => {:exchanges => :order_cycle})
 
+  scope :visible_for, lambda { |enterprise|
+    joins('LEFT OUTER JOIN spree_variants AS o_spree_variants ON (o_spree_variants.product_id = spree_products.id)').
+    joins('LEFT OUTER JOIN inventory_items AS o_inventory_items ON (o_spree_variants.id = o_inventory_items.variant_id)').
+    where('o_inventory_items.enterprise_id = (?) AND visible = (?)', enterprise, true).
+    select('DISTINCT spree_products.*')
+  }
+
 
   # -- Scopes
   scope :in_supplier, lambda { |supplier| where(:supplier_id => supplier) }
diff --git a/app/serializers/api/admin/exchange_serializer.rb b/app/serializers/api/admin/exchange_serializer.rb
index 4ef9b8964f..615d49f695 100644
--- a/app/serializers/api/admin/exchange_serializer.rb
+++ b/app/serializers/api/admin/exchange_serializer.rb
@@ -4,24 +4,35 @@ class Api::Admin::ExchangeSerializer < ActiveModel::Serializer
   has_many :enterprise_fees, serializer: Api::Admin::BasicEnterpriseFeeSerializer
 
   def variants
-    permitted = Spree::Variant.where("1=0")
-    if object.incoming
-      permitted = OpenFoodNetwork::OrderCyclePermissions.new(options[:current_user], object.order_cycle).
-      visible_variants_for_incoming_exchanges_from(object.sender)
+    variants = object.incoming? ? visible_incoming_variants : visible_outgoing_variants
+    Hash[ object.variants.merge(variants).map { |v| [v.id, true] } ]
+  end
+
+  private
+
+  def visible_incoming_variants
+    if object.order_cycle.prefers_product_selection_from_coordinator_inventory_only?
+      permitted_incoming_variants.visible_for(object.order_cycle.coordinator)
     else
-      # This is hopefully a temporary measure, pending the arrival of multiple named inventories
-      # for shops. We need this here to allow hubs to restrict visible variants to only those in
-      # their inventory if they so choose
-      permitted = if object.receiver.prefers_product_selection_from_inventory_only?
-        OpenFoodNetwork::OrderCyclePermissions.new(options[:current_user], object.order_cycle)
-        .visible_variants_for_outgoing_exchanges_to(object.receiver)
-        .visible_for(object.receiver)
-      else
-        OpenFoodNetwork::OrderCyclePermissions.new(options[:current_user], object.order_cycle)
-        .visible_variants_for_outgoing_exchanges_to(object.receiver)
-        .not_hidden_for(object.receiver)
-      end
+      permitted_incoming_variants
     end
-    Hash[ object.variants.merge(permitted).map { |v| [v.id, true] } ]
+  end
+
+  def visible_outgoing_variants
+    if object.receiver.prefers_product_selection_from_inventory_only?
+      permitted_outgoing_variants.visible_for(object.receiver)
+    else
+      permitted_outgoing_variants.not_hidden_for(object.receiver)
+    end
+  end
+
+  def permitted_incoming_variants
+    OpenFoodNetwork::OrderCyclePermissions.new(options[:current_user], object.order_cycle).
+    visible_variants_for_incoming_exchanges_from(object.sender)
+  end
+
+  def permitted_outgoing_variants
+    OpenFoodNetwork::OrderCyclePermissions.new(options[:current_user], object.order_cycle)
+    .visible_variants_for_outgoing_exchanges_to(object.receiver)
   end
 end
diff --git a/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb b/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
index 9dfa680476..8e4dee5f5d 100644
--- a/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
+++ b/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
@@ -6,7 +6,11 @@ class Api::Admin::ForOrderCycle::EnterpriseSerializer < ActiveModel::Serializer
   attributes :is_primary_producer, :is_distributor, :sells
 
   def issues_summary_supplier
-    OpenFoodNetwork::EnterpriseIssueValidator.new(object).issues_summary confirmation_only: true
+    issues = OpenFoodNetwork::EnterpriseIssueValidator.new(object).issues_summary confirmation_only: true
+    if issues.nil? && products.empty?
+      issues = "no products in inventory"
+    end
+    issues
   end
 
   def issues_summary_distributor
@@ -18,8 +22,22 @@ class Api::Admin::ForOrderCycle::EnterpriseSerializer < ActiveModel::Serializer
   end
 
   def supplied_products
-    objects = object.supplied_products.not_deleted
     serializer = Api::Admin::ForOrderCycle::SuppliedProductSerializer
-    ActiveModel::ArraySerializer.new(objects, each_serializer: serializer)
+    ActiveModel::ArraySerializer.new(products, each_serializer: serializer, order_cycle: order_cycle)
+  end
+
+  private
+
+  def products
+    return @products unless @products.nil?
+    @products = if order_cycle.prefers_product_selection_from_coordinator_inventory_only?
+      object.supplied_products.not_deleted.visible_for(order_cycle.coordinator)
+    else
+      object.supplied_products.not_deleted
+    end
+  end
+
+  def order_cycle
+    options[:order_cycle]
   end
 end
diff --git a/app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb b/app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb
index dbe3ec7a48..054fe44751 100644
--- a/app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb
+++ b/app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb
@@ -14,8 +14,17 @@ class Api::Admin::ForOrderCycle::SuppliedProductSerializer < ActiveModel::Serial
   end
 
   def variants
-    object.variants.map do |variant|
-      { id: variant.id, label: variant.full_name }
+    variants = if order_cycle.prefers_product_selection_from_coordinator_inventory_only?
+      object.variants.visible_for(order_cycle.coordinator)
+    else
+      object.variants
     end
+    variants.map { |variant| { id: variant.id, label: variant.full_name } }
+  end
+
+  private
+
+  def order_cycle
+    options[:order_cycle]
   end
 end
diff --git a/app/views/admin/enterprises/form/_inventory_settings.html.haml b/app/views/admin/enterprises/form/_inventory_settings.html.haml
index 021a5aabe4..ded4f6d497 100644
--- a/app/views/admin/enterprises/form/_inventory_settings.html.haml
+++ b/app/views/admin/enterprises/form/_inventory_settings.html.haml
@@ -10,7 +10,7 @@
 .row
   .alpha.eleven.columns
     .three.columns.alpha
-      = f.label "enterprise_preferred_product_selection_from_inventory_only", t(:select_products_for_order_cycle_from)
+      = f.label "enterprise_preferred_product_selection_from_inventory_only", t('admin.enterprise.select_outgoing_oc_products_from')
     .three.columns
       = radio_button :enterprise, :preferred_product_selection_from_inventory_only, "1", { 'ng-model' => 'Enterprise.preferred_product_selection_from_inventory_only' }
       = label :enterprise, :preferred_product_selection_from_inventory_only, "Inventory Only"
diff --git a/app/views/admin/order_cycles/_advanced_settings.html.haml b/app/views/admin/order_cycles/_advanced_settings.html.haml
new file mode 100644
index 0000000000..27b347880b
--- /dev/null
+++ b/app/views/admin/order_cycles/_advanced_settings.html.haml
@@ -0,0 +1,22 @@
+.row
+  .alpha.omega.sixteen.columns
+    %h3 Advanced Settings
+
+= form_for [main_app, :admin, @order_cycle] do |f|
+  .row
+    .six.columns.alpha
+      = f.label "enterprise_preferred_product_selection_from_coordinator_inventory_only", t('admin.order_cycle.choose_products_from')
+      .with-tip{'data-powertip' => "You can opt to restrict all available products (both incoming and outgoing), to only those in #{@order_cycle.coordinator.name}'s inventory."}
+        %a What's this?
+    .four.columns
+      = f.radio_button :preferred_product_selection_from_coordinator_inventory_only, true
+      = f.label :preferred_product_selection_from_coordinator_inventory_only, "Coordinator's Inventory Only"
+    .six.columns.omega
+      = f.radio_button :preferred_product_selection_from_coordinator_inventory_only, false
+      = f.label :preferred_product_selection_from_coordinator_inventory_only, "All Available Products"
+
+  .row
+    .sixteen.columns.alpha.omega.text-center
+      %input{ type: 'submit', value: 'Save and Reload Page' }
+      or
+      %a{ href: "#", onClick: "toggleSettings()" } Close
diff --git a/app/views/admin/order_cycles/_exchange_distributed_products_form.html.haml b/app/views/admin/order_cycles/_exchange_distributed_products_form.html.haml
index cff8f5d2b3..e2218599ce 100644
--- a/app/views/admin/order_cycles/_exchange_distributed_products_form.html.haml
+++ b/app/views/admin/order_cycles/_exchange_distributed_products_form.html.haml
@@ -9,8 +9,9 @@
     .exchange-product{'ng-repeat' => 'product in supplied_products | filter:productSuppliedToOrderCycle | visibleProducts:exchange:order_cycle.visible_variants_for_outgoing_exchanges | orderBy:"name"' }
       .exchange-product-details
         %label
-          = check_box_tag 'order_cycle_outgoing_exchange_{{ $parent.$index }}_variants_{{ product.master_id }}', 1, 1, 'ng-hide' => 'product.variants.length > 0', 'ng-model' => 'exchange.variants[product.master_id]', 'id' => 'order_cycle_outgoing_exchange_{{ $parent.$index }}_variants_{{ product.master_id }}',
-          'ng-disabled' => 'product.variants.length > 0 || !order_cycle.editable_variants_for_outgoing_exchanges.hasOwnProperty(exchange.enterprise_id) || order_cycle.editable_variants_for_outgoing_exchanges[exchange.enterprise_id].indexOf(product.master_id) < 0'
+          -# MASTER_VARIANTS: No longer required
+          -# = check_box_tag 'order_cycle_outgoing_exchange_{{ $parent.$index }}_variants_{{ product.master_id }}', 1, 1, 'ng-hide' => 'product.variants.length > 0', 'ng-model' => 'exchange.variants[product.master_id]', 'id' => 'order_cycle_outgoing_exchange_{{ $parent.$index }}_variants_{{ product.master_id }}',
+          -# 'ng-disabled' => 'product.variants.length > 0 || !order_cycle.editable_variants_for_outgoing_exchanges.hasOwnProperty(exchange.enterprise_id) || order_cycle.editable_variants_for_outgoing_exchanges[exchange.enterprise_id].indexOf(product.master_id) < 0'
           %img{'ng-src' => '{{ product.image_url }}'}
           .name {{ product.name }}
         .supplier {{ product.supplier_name }}
diff --git a/app/views/admin/order_cycles/edit.html.haml b/app/views/admin/order_cycles/edit.html.haml
index 12dc6238ae..d2b7605289 100644
--- a/app/views/admin/order_cycles/edit.html.haml
+++ b/app/views/admin/order_cycles/edit.html.haml
@@ -1,9 +1,27 @@
-- if can? :notify_producers, @order_cycle
-  = content_for :page_actions do
+- content_for :page_actions do
+  :javascript
+    function toggleSettings(){
+      if( $('#advanced_settings').is(":visible") ){
+        $('button#toggle_settings i').switchClass("icon-chevron-up","icon-chevron-down")
+      }
+      else {
+        $('button#toggle_settings i').switchClass("icon-chevron-down","icon-chevron-up")
+      }
+      $("#advanced_settings").slideToggle()
+    }
+
+  - if can? :notify_producers, @order_cycle
     %li
       = button_to "Notify producers", main_app.notify_producers_admin_order_cycle_path, :id => 'admin_notify_producers', :confirm => 'Are you sure?'
+  %li
+    %button#toggle_settings{ onClick: 'toggleSettings()' }
+      Advanced Settings
+      %i.icon-chevron-down
 
 
+#advanced_settings{ hidden: true }
+  = render partial: "/admin/order_cycles/advanced_settings"
+
 %h1 Edit Order Cycle
 
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index e5c0983ac2..a72afe655d 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -94,6 +94,13 @@ en:
       hidden_powertip: These products have been hidden from your inventory and will not be available to add to your shop. You can click 'Add' to add a product to you inventory.
       new_powertip: These products are available to be added to your inventory. Click 'Add' to add a product to your inventory, or 'Hide' to hide it from view. You can always change your mind later!
 
+
+    order_cycle:
+      choose_products_from: "Choose Products From:"
+
+    enterprise:
+      select_outgoing_oc_products_from: Select outgoing OC products from
+
   # Printable Invoice Columns
   invoice_column_item: "Item"
   invoice_column_qty: "Qty"
diff --git a/spec/controllers/admin/order_cycles_controller_spec.rb b/spec/controllers/admin/order_cycles_controller_spec.rb
index 9fc9ad9096..3cc29193ab 100644
--- a/spec/controllers/admin/order_cycles_controller_spec.rb
+++ b/spec/controllers/admin/order_cycles_controller_spec.rb
@@ -103,10 +103,34 @@ module Admin
       end
 
       it "does not set flash message otherwise" do
-        spree_put :update, id: order_cycle.id, reloading: '0', order_cycle: {}
         flash[:notice].should be_nil
       end
 
+      context "when updating without explicitly submitting exchanges" do
+        let(:form_applicator_mock) { double(:form_applicator) }
+        let(:incoming_exchange) { create(:exchange, order_cycle: order_cycle, incoming: true) }
+        let(:outgoing_exchange) { create(:exchange, order_cycle: order_cycle, incoming: false) }
+
+
+        before do
+          allow(OpenFoodNetwork::OrderCycleFormApplicator).to receive(:new) { form_applicator_mock }
+          allow(form_applicator_mock).to receive(:go!) { nil }
+        end
+
+         it "does not run the OrderCycleFormApplicator" do
+           expect(order_cycle.exchanges.incoming).to eq [incoming_exchange]
+           expect(order_cycle.exchanges.outgoing).to eq [outgoing_exchange]
+           expect(order_cycle.prefers_product_selection_from_coordinator_inventory_only?).to be false
+           spree_put :update, id: order_cycle.id, order_cycle: { name: 'Some new name', preferred_product_selection_from_coordinator_inventory_only: true }
+           expect(form_applicator_mock).to_not have_received(:go!)
+           order_cycle.reload
+           expect(order_cycle.exchanges.incoming).to eq [incoming_exchange]
+           expect(order_cycle.exchanges.outgoing).to eq [outgoing_exchange]
+           expect(order_cycle.name).to eq 'Some new name'
+           expect(order_cycle.prefers_product_selection_from_coordinator_inventory_only?).to be true
+         end
+      end
+
       context "as a producer supplying to an order cycle" do
         let(:producer) { create(:supplier_enterprise) }
         let(:coordinator) { order_cycle.coordinator }
diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb
index 90465fb80e..c8c76abb1d 100644
--- a/spec/models/order_cycle_spec.rb
+++ b/spec/models/order_cycle_spec.rb
@@ -380,7 +380,7 @@ describe OrderCycle do
 
   it "clones itself" do
     coordinator = create(:enterprise);
-    oc = create(:simple_order_cycle, coordinator_fees: [create(:enterprise_fee, enterprise: coordinator)])
+    oc = create(:simple_order_cycle, coordinator_fees: [create(:enterprise_fee, enterprise: coordinator)], preferred_product_selection_from_coordinator_inventory_only: true)
     ex1 = create(:exchange, order_cycle: oc)
     ex2 = create(:exchange, order_cycle: oc)
     oc.clone!
@@ -390,10 +390,12 @@ describe OrderCycle do
     occ.orders_open_at.should be_nil
     occ.orders_close_at.should be_nil
     occ.coordinator.should_not be_nil
+    occ.preferred_product_selection_from_coordinator_inventory_only.should be_true
     occ.coordinator.should == oc.coordinator
 
     occ.coordinator_fee_ids.should_not be_empty
     occ.coordinator_fee_ids.should == oc.coordinator_fee_ids
+    occ.preferred_product_selection_from_coordinator_inventory_only.should == oc.preferred_product_selection_from_coordinator_inventory_only
 
     # to_h gives us a unique hash for each exchange
     # check that the clone has no additional exchanges
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index 25913f679b..c37b49bc23 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -338,6 +338,22 @@ module Spree
           product.should include @p2
         end
       end
+
+      describe "visible_for" do
+        let(:enterprise) { create(:distributor_enterprise) }
+        let!(:new_variant) { create(:variant) }
+        let!(:hidden_variant) { create(:variant) }
+        let!(:visible_variant) { create(:variant) }
+        let!(:hidden_inventory_item) { create(:inventory_item, enterprise: enterprise, variant: hidden_variant, visible: false ) }
+        let!(:visible_inventory_item) { create(:inventory_item, enterprise: enterprise, variant: visible_variant, visible: true ) }
+
+        let!(:products) { Spree::Product.visible_for(enterprise) }
+
+        it "lists any products with variants that are listed as visible=true" do
+          expect(products).to include visible_variant.product
+          expect(products).to_not include new_variant.product, hidden_variant.product
+        end
+      end
     end
 
     describe "finders" do
diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb
index 1f5aeeba18..306381630f 100644
--- a/spec/models/spree/variant_spec.rb
+++ b/spec/models/spree/variant_spec.rb
@@ -133,7 +133,7 @@ module Spree
         context "finding variants that are visible in an enterprise's inventory" do
           let!(:variants) { Spree::Variant.visible_for(enterprise) }
 
-          it "lists any variants that are not listed as visible=false" do
+          it "lists any variants that are listed as visible=true" do
             expect(variants).to include visible_variant
             expect(variants).to_not include new_variant, hidden_variant
           end
diff --git a/spec/serializers/admin/exchange_serializer_spec.rb b/spec/serializers/admin/exchange_serializer_spec.rb
index ecb85d07d8..649a0b427e 100644
--- a/spec/serializers/admin/exchange_serializer_spec.rb
+++ b/spec/serializers/admin/exchange_serializer_spec.rb
@@ -3,65 +3,93 @@ require 'open_food_network/order_cycle_permissions'
 describe Api::Admin::ExchangeSerializer do
   let(:v1) { create(:variant) }
   let(:v2) { create(:variant) }
+  let(:v3) { create(:variant) }
   let(:permissions_mock) { double(:permissions) }
+  let(:permitted_variants) { Spree::Variant.where(id: [v1, v2]) }
   let(:serializer) { Api::Admin::ExchangeSerializer.new exchange }
 
+
   context "serializing incoming exchanges" do
-    let(:exchange) { create(:exchange, incoming: true, variants: [v1, v2]) }
-    let(:permitted_variants) { double(:permitted_variants) }
+    let(:exchange) { create(:exchange, incoming: true, variants: [v1, v2, v3]) }
+    let!(:inventory_item) { create(:inventory_item, enterprise: exchange.order_cycle.coordinator, variant: v1, visible: true) }
 
     before do
       allow(OpenFoodNetwork::OrderCyclePermissions).to receive(:new) { permissions_mock }
-      allow(permissions_mock).to receive(:visible_variants_for_incoming_exchanges_from) { Spree::Variant.where(id: [v1]) }
+      allow(permissions_mock).to receive(:visible_variants_for_incoming_exchanges_from) { permitted_variants }
+      allow(permitted_variants).to receive(:visible_for).and_call_original
     end
 
-    it "filters variants within the exchange based on permissions" do
-      visible_variants = serializer.variants
-      expect(permissions_mock).to have_received(:visible_variants_for_incoming_exchanges_from).with(exchange.sender)
-      expect(exchange.variants).to include v1, v2
-      expect(visible_variants.keys).to include v1.id
-      expect(visible_variants.keys).to_not include v2.id
+    context "when order cycle shows only variants in the coordinator's inventory" do
+      before do
+        allow(exchange.order_cycle).to receive(:prefers_product_selection_from_coordinator_inventory_only?) { true }
+      end
+
+      it "filters variants within the exchange based on permissions, and visibility in inventory" do
+        visible_variants = serializer.variants
+        expect(permissions_mock).to have_received(:visible_variants_for_incoming_exchanges_from).with(exchange.sender)
+        expect(permitted_variants).to have_received(:visible_for).with(exchange.order_cycle.coordinator)
+        expect(exchange.variants).to include v1, v2, v3
+        expect(visible_variants.keys).to include v1.id
+        expect(visible_variants.keys).to_not include v2.id, v3.id
+      end
+    end
+
+    context "when order cycle shows all available products" do
+      before do
+        allow(exchange.order_cycle).to receive(:prefers_product_selection_from_coordinator_inventory_only?) { false }
+      end
+
+      it "filters variants within the exchange based on permissions only" do
+        visible_variants = serializer.variants
+        expect(permissions_mock).to have_received(:visible_variants_for_incoming_exchanges_from).with(exchange.sender)
+        expect(permitted_variants).to_not have_received(:visible_for)
+        expect(exchange.variants).to include v1, v2, v3
+        expect(visible_variants.keys).to include v1.id, v2.id
+        expect(visible_variants.keys).to_not include v3.id
+      end
     end
   end
 
   context "serializing outgoing exchanges" do
-    let(:exchange) { create(:exchange, incoming: false, variants: [v1, v2]) }
-    let(:permitted_variants) { double(:permitted_variants) }
+    let(:exchange) { create(:exchange, incoming: false, variants: [v1, v2, v3]) }
+    let!(:inventory_item) { create(:inventory_item, enterprise: exchange.receiver, variant: v1, visible: true) }
 
     before do
       allow(OpenFoodNetwork::OrderCyclePermissions).to receive(:new) { permissions_mock }
       allow(permissions_mock).to receive(:visible_variants_for_outgoing_exchanges_to) { permitted_variants }
+      allow(permitted_variants).to receive(:visible_for).and_call_original
+      allow(permitted_variants).to receive(:not_hidden_for).and_call_original
     end
 
     context "when the receiver prefers to see all variants (not just those in their inventory)" do
       before do
         allow(exchange.receiver).to receive(:prefers_product_selection_from_inventory_only?) { false }
-        allow(permitted_variants).to receive(:not_hidden_for) { Spree::Variant.where(id: [v1]) }
       end
 
-      it "filters variants within the exchange based on permissions" do
+      it "filters variants within the exchange based on permissions only" do
         visible_variants = serializer.variants
         expect(permissions_mock).to have_received(:visible_variants_for_outgoing_exchanges_to).with(exchange.receiver)
         expect(permitted_variants).to have_received(:not_hidden_for).with(exchange.receiver)
-        expect(exchange.variants).to include v1, v2
-        expect(visible_variants.keys).to include v1.id
-        expect(visible_variants.keys).to_not include v2.id
+        expect(permitted_variants).to_not have_received(:visible_for)
+        expect(exchange.variants).to include v1, v2, v3
+        expect(visible_variants.keys).to include v1.id, v2.id
+        expect(visible_variants.keys).to_not include v3.id
       end
     end
 
     context "when the receiver prefers to restrict visible variants to only those in their inventory" do
       before do
         allow(exchange.receiver).to receive(:prefers_product_selection_from_inventory_only?) { true }
-        allow(permitted_variants).to receive(:visible_for) { Spree::Variant.where(id: [v1]) }
       end
 
       it "filters variants within the exchange based on permissions, and inventory visibility" do
         visible_variants = serializer.variants
         expect(permissions_mock).to have_received(:visible_variants_for_outgoing_exchanges_to).with(exchange.receiver)
         expect(permitted_variants).to have_received(:visible_for).with(exchange.receiver)
-        expect(exchange.variants).to include v1, v2
+        expect(permitted_variants).to_not have_received(:not_hidden_for)
+        expect(exchange.variants).to include v1, v2, v3
         expect(visible_variants.keys).to include v1.id
-        expect(visible_variants.keys).to_not include v2.id
+        expect(visible_variants.keys).to_not include v2.id, v3.id
       end
     end
   end
diff --git a/spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb b/spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb
index db476164eb..c30b6374c9 100644
--- a/spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb
+++ b/spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb
@@ -1,13 +1,46 @@
 describe Api::Admin::ForOrderCycle::EnterpriseSerializer do
-  let(:enterprise)       { create(:distributor_enterprise) }
-  let!(:product)         { create(:simple_product, supplier: enterprise) }
-  let!(:deleted_product) { create(:simple_product, supplier: enterprise, deleted_at: 24.hours.ago ) }
-  let(:serialized_enterprise) { Api::Admin::ForOrderCycle::EnterpriseSerializer.new(enterprise, spree_current_user: enterprise.owner ).to_json }
+  let(:coordinator)         { create(:distributor_enterprise) }
+  let(:order_cycle)         { double(:order_cycle, coordinator: coordinator) }
+  let(:enterprise)          { create(:distributor_enterprise) }
+  let!(:non_inventory_product) { create(:simple_product, supplier: enterprise) }
+  let!(:non_inventory_variant)  { non_inventory_product.variants.first }
+  let!(:inventory_product)  { create(:simple_product, supplier: enterprise) }
+  let!(:inventory_variant)  { inventory_product.variants.first }
+  let!(:deleted_product)    { create(:simple_product, supplier: enterprise, deleted_at: 24.hours.ago ) }
+  let!(:deleted_variant)  { deleted_product.variants.first }
+  let(:serialized_enterprise) { Api::Admin::ForOrderCycle::EnterpriseSerializer.new(enterprise, order_cycle: order_cycle, spree_current_user: enterprise.owner ).to_json }
+  let!(:inventory_item1) { create(:inventory_item, enterprise: coordinator, variant: inventory_variant, visible: true)}
+  let!(:inventory_item2) { create(:inventory_item, enterprise: coordinator, variant: deleted_variant, visible: true)}
 
-  describe "supplied products" do
-    it "does not render deleted products" do
-      expect(serialized_enterprise).to have_json_size(1).at_path 'supplied_products'
-      expect(serialized_enterprise).to be_json_eql(product.master.id).at_path 'supplied_products/0/master_id'
+  context "when order cycle shows only variants in the coordinator's inventory" do
+    before do
+      allow(order_cycle).to receive(:prefers_product_selection_from_coordinator_inventory_only?) { true }
+    end
+
+    describe "supplied products" do
+      it "renders only non-deleted variants that are in the coordinators inventory" do
+        expect(serialized_enterprise).to have_json_size(1).at_path 'supplied_products'
+        expect(serialized_enterprise).to have_json_size(1).at_path 'supplied_products/0/variants'
+        expect(serialized_enterprise).to be_json_eql(inventory_variant.id).at_path 'supplied_products/0/variants/0/id'
+      end
     end
   end
+
+
+  context "when order cycle shows all available products" do
+    before do
+      allow(order_cycle).to receive(:prefers_product_selection_from_coordinator_inventory_only?) { false }
+    end
+
+    describe "supplied products" do
+      it "renders variants that are not in the coordinators inventory but not variants of deleted products" do
+        expect(serialized_enterprise).to have_json_size(2).at_path 'supplied_products'
+        expect(serialized_enterprise).to have_json_size(1).at_path 'supplied_products/0/variants'
+        expect(serialized_enterprise).to have_json_size(1).at_path 'supplied_products/1/variants'
+        variant_ids = parse_json(serialized_enterprise)['supplied_products'].map{ |p| p['variants'].first['id'] }
+        expect(variant_ids).to include non_inventory_variant.id, inventory_variant.id
+      end
+    end
+  end
+
 end
diff --git a/spec/serializers/admin/for_order_cycle/supplied_product_serializer_spec.rb b/spec/serializers/admin/for_order_cycle/supplied_product_serializer_spec.rb
new file mode 100644
index 0000000000..16c562120e
--- /dev/null
+++ b/spec/serializers/admin/for_order_cycle/supplied_product_serializer_spec.rb
@@ -0,0 +1,38 @@
+describe Api::Admin::ForOrderCycle::EnterpriseSerializer do
+  let(:coordinator)         { create(:distributor_enterprise) }
+  let(:order_cycle)         { double(:order_cycle, coordinator: coordinator) }
+  let!(:product) { create(:simple_product) }
+  let!(:non_inventory_variant)  { product.variants.first }
+  let!(:inventory_variant)  { create(:variant, product: product.reload) }
+  let(:serialized_product) { Api::Admin::ForOrderCycle::SuppliedProductSerializer.new(product, order_cycle: order_cycle ).to_json }
+  let!(:inventory_item) { create(:inventory_item, enterprise: coordinator, variant: inventory_variant, visible: true)}
+
+  context "when order cycle shows only variants in the coordinator's inventory" do
+    before do
+      allow(order_cycle).to receive(:prefers_product_selection_from_coordinator_inventory_only?) { true }
+    end
+
+    describe "variants" do
+      it "renders only variants that are in the coordinators inventory" do
+        expect(serialized_product).to have_json_size(1).at_path 'variants'
+        expect(serialized_product).to be_json_eql(inventory_variant.id).at_path 'variants/0/id'
+      end
+    end
+  end
+
+
+  context "when order cycle shows all available products" do
+    before do
+      allow(order_cycle).to receive(:prefers_product_selection_from_coordinator_inventory_only?) { false }
+    end
+
+    describe "supplied products" do
+      it "renders variants regardless of whether they are in the coordinators inventory" do
+        expect(serialized_product).to have_json_size(2).at_path 'variants'
+        variant_ids = parse_json(serialized_product)['variants'].map{ |v| v['id'] }
+        expect(variant_ids).to include non_inventory_variant.id, inventory_variant.id
+      end
+    end
+  end
+
+end