From b9492d6483ee251306201f437d22eae1a64e83ca Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Wed, 12 Sep 2018 03:36:23 +1000
Subject: [PATCH 01/13] Fix setup in tests for SubscriptionValidator

---
 spec/services/subscription_validator_spec.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/spec/services/subscription_validator_spec.rb b/spec/services/subscription_validator_spec.rb
index 6670d14fa4..64dbc2637b 100644
--- a/spec/services/subscription_validator_spec.rb
+++ b/spec/services/subscription_validator_spec.rb
@@ -1,3 +1,5 @@
+require "spec_helper"
+
 describe SubscriptionValidator do
   let(:shop) { instance_double(Enterprise, name: "Shop") }
 

From 28c70c7719f6abd7c987a1d5a7482c8fce213a85 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Fri, 18 Jan 2019 19:04:32 +0800
Subject: [PATCH 02/13] Fix nesting of specs for editing subscriptions

---
 spec/features/admin/subscriptions_spec.rb | 234 +++++++++++-----------
 1 file changed, 117 insertions(+), 117 deletions(-)

diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb
index 27bfaa1bfc..6610c006ee 100644
--- a/spec/features/admin/subscriptions_spec.rb
+++ b/spec/features/admin/subscriptions_spec.rb
@@ -285,137 +285,137 @@ feature 'Subscriptions' do
         expect(subscription_line_item.variant).to eq variant2
         expect(subscription_line_item.quantity).to eq 3
       end
+    end
 
-      context 'editing an existing subscription' do
-        let!(:customer) { create(:customer, enterprise: shop) }
-        let!(:product1) { create(:product, supplier: shop) }
-        let!(:product2) { create(:product, supplier: shop) }
-        let!(:product3) { create(:product, supplier: shop) }
-        let!(:variant1) { create(:variant, product: product1, unit_value: '100', price: 12.00, option_values: []) }
-        let!(:variant2) { create(:variant, product: product2, unit_value: '1000', price: 6.00, option_values: []) }
-        let!(:variant3) { create(:variant, product: product3, unit_value: '10000', price: 22.00, option_values: []) }
-        let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) }
-        let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) }
-        let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2], enterprise_fees: [enterprise_fee]) }
-        let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
-        let!(:variant3_oc) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) }
-        let!(:variant3_ex) { variant3_oc.exchanges.create(sender: shop, receiver: shop, variants: [variant3]) }
-        let!(:payment_method) { create(:payment_method, distributors: [shop]) }
-        let!(:stripe_payment_method) { create(:stripe_payment_method, name: 'Credit Card', distributors: [shop], preferred_enterprise_id: shop.id) }
-        let!(:shipping_method) { create(:shipping_method, distributors: [shop]) }
-        let!(:subscription) {
-          create(:subscription,
-                 shop: shop,
-                 customer: customer,
-                 schedule: schedule,
-                 payment_method: payment_method,
-                 shipping_method: shipping_method,
-                 subscription_line_items: [create(:subscription_line_item, variant: variant1, quantity: 2, price_estimate: 13.75)],
-                 with_proxy_orders: true)
-        }
+    context 'editing an existing subscription' do
+      let!(:customer) { create(:customer, enterprise: shop) }
+      let!(:product1) { create(:product, supplier: shop) }
+      let!(:product2) { create(:product, supplier: shop) }
+      let!(:product3) { create(:product, supplier: shop) }
+      let!(:variant1) { create(:variant, product: product1, unit_value: '100', price: 12.00, option_values: []) }
+      let!(:variant2) { create(:variant, product: product2, unit_value: '1000', price: 6.00, option_values: []) }
+      let!(:variant3) { create(:variant, product: product3, unit_value: '10000', price: 22.00, option_values: []) }
+      let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) }
+      let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) }
+      let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2], enterprise_fees: [enterprise_fee]) }
+      let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
+      let!(:variant3_oc) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) }
+      let!(:variant3_ex) { variant3_oc.exchanges.create(sender: shop, receiver: shop, variants: [variant3]) }
+      let!(:payment_method) { create(:payment_method, distributors: [shop]) }
+      let!(:stripe_payment_method) { create(:stripe_payment_method, name: 'Credit Card', distributors: [shop], preferred_enterprise_id: shop.id) }
+      let!(:shipping_method) { create(:shipping_method, distributors: [shop]) }
+      let!(:subscription) {
+        create(:subscription,
+               shop: shop,
+               customer: customer,
+               schedule: schedule,
+               payment_method: payment_method,
+               shipping_method: shipping_method,
+               subscription_line_items: [create(:subscription_line_item, variant: variant1, quantity: 2, price_estimate: 13.75)],
+               with_proxy_orders: true)
+      }
 
-        it "passes the smoke test" do
-          visit edit_admin_subscription_path(subscription)
+      it "passes the smoke test" do
+        visit edit_admin_subscription_path(subscription)
 
-          # Customer and Schedule cannot be edited
-          click_button 'edit-details'
-          expect(page).to have_selector '#s2id_customer_id.select2-container-disabled'
-          expect(page).to have_selector '#s2id_schedule_id.select2-container-disabled'
+        # Customer and Schedule cannot be edited
+        click_button 'edit-details'
+        expect(page).to have_selector '#s2id_customer_id.select2-container-disabled'
+        expect(page).to have_selector '#s2id_schedule_id.select2-container-disabled'
 
-          # Can't use a Stripe payment method because customer does not allow it
-          select2_select stripe_payment_method.name, from: 'payment_method_id'
-          expect(page).to have_content I18n.t('admin.subscriptions.details.charges_not_allowed')
-          click_button 'Save Changes'
-          expect(page).to have_content 'Credit card charges are not allowed by this customer'
-          select2_select payment_method.name, from: 'payment_method_id'
-          click_button 'Review'
+        # Can't use a Stripe payment method because customer does not allow it
+        select2_select stripe_payment_method.name, from: 'payment_method_id'
+        expect(page).to have_content I18n.t('admin.subscriptions.details.charges_not_allowed')
+        click_button 'Save Changes'
+        expect(page).to have_content 'Credit card charges are not allowed by this customer'
+        select2_select payment_method.name, from: 'payment_method_id'
+        click_button 'Review'
 
-          # Existing products should be visible
-          click_button 'edit-products'
-          within "#sli_0" do
-            expect(page).to have_selector 'td.description', text: "#{product1.name} - #{variant1.full_name}"
-            expect(page).to have_selector 'td.price', text: "$13.75"
-            expect(page).to have_input 'quantity', with: "2"
-            expect(page).to have_selector 'td.total', text: "$27.50"
+        # Existing products should be visible
+        click_button 'edit-products'
+        within "#sli_0" do
+          expect(page).to have_selector 'td.description', text: "#{product1.name} - #{variant1.full_name}"
+          expect(page).to have_selector 'td.price', text: "$13.75"
+          expect(page).to have_input 'quantity', with: "2"
+          expect(page).to have_selector 'td.total', text: "$27.50"
 
-            # Remove variant1 from the subscription
-            find("a.delete-item").click
-          end
-
-          # Attempting to submit without a product
-          click_button 'Save Changes'
-          expect(page).to have_content 'Please add at least one product'
-
-          # Add variant2 to the subscription
-          select2_search_async product2.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop'
-          fill_in 'add_quantity', with: 1
-          click_link 'Add'
-          within "#sli_0" do
-            expect(page).to have_selector 'td.description', text: "#{product2.name} - #{variant2.full_name}"
-            expect(page).to have_selector 'td.price', text: "$7.75"
-            expect(page).to have_input 'quantity', with: "1"
-            expect(page).to have_selector 'td.total', text: "$7.75"
-          end
-
-          # Total should be $7.75
-          expect(page).to have_selector '#order_form_total', text: "$7.75"
-
-          # Add variant3 to the subscription (even though it is not available)
-          select2_search_async product3.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop'
-          fill_in 'add_quantity', with: 1
-          click_link 'Add'
-          within "#sli_1" do
-            expect(page).to have_selector 'td.description', text: "#{product3.name} - #{variant3.full_name}"
-            expect(page).to have_selector 'td.price', text: "$22.00"
-            expect(page).to have_input 'quantity', with: "1"
-            expect(page).to have_selector 'td.total', text: "$22.00"
-          end
-
-          # Total should be $29.75
-          expect(page).to have_selector '#order_form_total', text: "$29.75"
-
-          click_button 'Save Changes'
-          expect(page).to have_content "#{product3.name} - #{variant3.full_name} is not available from the selected schedule"
-
-          # Remove variant3 from the subscription
-          within '#sli_1' do
-            find("a.delete-item").click
-          end
-
-          click_button 'Save Changes'
-          expect(page).to have_current_path admin_subscriptions_path
-
-          select2_select shop.name, from: "shop_id"
-          expect(page).to have_selector "td.items.panel-toggle"
-          first("td.items.panel-toggle").click
-
-          # Total should be $7.75
-          expect(page).to have_selector '#order_form_total', text: "$7.75"
-          expect(page).to have_selector 'tr.item', count: 1
-          expect(subscription.reload.subscription_line_items.length).to eq 1
-          expect(subscription.subscription_line_items.first.variant).to eq variant2
+          # Remove variant1 from the subscription
+          find("a.delete-item").click
         end
 
-        context "with initialised order that has been changed" do
-          let(:proxy_order) { subscription.proxy_orders.first }
-          let(:order) { proxy_order.initialise_order! }
-          let(:line_item) { order.line_items.first }
+        # Attempting to submit without a product
+        click_button 'Save Changes'
+        expect(page).to have_content 'Please add at least one product'
 
-          before { line_item.update_attributes(quantity: 3) }
+        # Add variant2 to the subscription
+        select2_search_async product2.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop'
+        fill_in 'add_quantity', with: 1
+        click_link 'Add'
+        within "#sli_0" do
+          expect(page).to have_selector 'td.description', text: "#{product2.name} - #{variant2.full_name}"
+          expect(page).to have_selector 'td.price', text: "$7.75"
+          expect(page).to have_input 'quantity', with: "1"
+          expect(page).to have_selector 'td.total', text: "$7.75"
+        end
 
-          it "reports issues encountered during the update" do
-            visit edit_admin_subscription_path(subscription)
-            click_button 'edit-products'
+        # Total should be $7.75
+        expect(page).to have_selector '#order_form_total', text: "$7.75"
 
-            within "#sli_0" do
-              fill_in 'quantity', with: "1"
-            end
+        # Add variant3 to the subscription (even though it is not available)
+        select2_search_async product3.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop'
+        fill_in 'add_quantity', with: 1
+        click_link 'Add'
+        within "#sli_1" do
+          expect(page).to have_selector 'td.description', text: "#{product3.name} - #{variant3.full_name}"
+          expect(page).to have_selector 'td.price', text: "$22.00"
+          expect(page).to have_input 'quantity', with: "1"
+          expect(page).to have_selector 'td.total', text: "$22.00"
+        end
 
-            click_button 'Save Changes'
-            expect(page).to have_content 'Saved'
+        # Total should be $29.75
+        expect(page).to have_selector '#order_form_total', text: "$29.75"
 
-            expect(page).to have_selector "#order_update_issues_dialog .message", text: I18n.t("admin.subscriptions.order_update_issues_msg")
+        click_button 'Save Changes'
+        expect(page).to have_content "#{product3.name} - #{variant3.full_name} is not available from the selected schedule"
+
+        # Remove variant3 from the subscription
+        within '#sli_1' do
+          find("a.delete-item").click
+        end
+
+        click_button 'Save Changes'
+        expect(page).to have_current_path admin_subscriptions_path
+
+        select2_select shop.name, from: "shop_id"
+        expect(page).to have_selector "td.items.panel-toggle"
+        first("td.items.panel-toggle").click
+
+        # Total should be $7.75
+        expect(page).to have_selector '#order_form_total', text: "$7.75"
+        expect(page).to have_selector 'tr.item', count: 1
+        expect(subscription.reload.subscription_line_items.length).to eq 1
+        expect(subscription.subscription_line_items.first.variant).to eq variant2
+      end
+
+      context "with initialised order that has been changed" do
+        let(:proxy_order) { subscription.proxy_orders.first }
+        let(:order) { proxy_order.initialise_order! }
+        let(:line_item) { order.line_items.first }
+
+        before { line_item.update_attributes(quantity: 3) }
+
+        it "reports issues encountered during the update" do
+          visit edit_admin_subscription_path(subscription)
+          click_button 'edit-products'
+
+          within "#sli_0" do
+            fill_in 'quantity', with: "1"
           end
+
+          click_button 'Save Changes'
+          expect(page).to have_content 'Saved'
+
+          expect(page).to have_selector "#order_update_issues_dialog .message", text: I18n.t("admin.subscriptions.order_update_issues_msg")
         end
       end
     end

From 35c0bcb3df65d4604b3b95beb5fb712583d60098 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Wed, 12 Sep 2018 18:35:24 +0800
Subject: [PATCH 03/13] Add tests for eligible variants for a subscription

---
 spec/services/subscription_validator_spec.rb | 77 ++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/spec/services/subscription_validator_spec.rb b/spec/services/subscription_validator_spec.rb
index 64dbc2637b..c4878414a5 100644
--- a/spec/services/subscription_validator_spec.rb
+++ b/spec/services/subscription_validator_spec.rb
@@ -463,4 +463,81 @@ describe SubscriptionValidator do
       end
     end
   end
+
+  describe "variant eligibility for subscription" do
+    let!(:shop) { create(:distributor_enterprise) }
+    let!(:producer) { create(:supplier_enterprise) }
+    let!(:product) { create(:product, supplier: producer) }
+    let!(:variant) { product.variants.first }
+
+    let!(:order_cycle) { current_order_cycle }
+    let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
+    let!(:subscription) { create(:subscription, shop: shop, schedule: schedule) }
+    let!(:subscription_line_item) do
+      create(:subscription_line_item, subscription: subscription, variant: variant)
+    end
+
+    let(:current_order_cycle) do
+      create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.ago,
+                                  orders_close_at: 1.week.from_now)
+    end
+
+    let(:future_order_cycle) do
+      create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.from_now,
+                                  orders_close_at: 2.weeks.from_now)
+    end
+
+    let(:past_order_cycle) do
+      create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.weeks.ago,
+                                  orders_close_at: 1.week.ago)
+    end
+
+    let(:validator) { SubscriptionValidator.new(subscription) }
+
+    context "if it is not in an exchange" do
+      it "is not eligible" do
+        expect(validator.__send__(:available_variant_ids)).to_not include(variant.id)
+      end
+    end
+
+    context "if it is only in an incoming exchange" do
+      let!(:incoming_exchange) do
+        create(:exchange, order_cycle: order_cycle, sender: producer, receiver: shop,
+                          incoming: true, variants: [variant])
+      end
+
+      it "is not eligible" do
+        expect(validator.__send__(:available_variant_ids)).to_not include(variant.id)
+      end
+    end
+
+    context "if is an outgoing exchange where the shop is the receiver" do
+      let!(:outgoing_exchange) do
+        create(:exchange, order_cycle: order_cycle, sender: shop, receiver: shop,
+                          incoming: false, variants: [variant])
+      end
+
+      context "if the order cycle is currently open" do
+        it "is eligible" do
+          expect(validator.__send__(:available_variant_ids)).to include(variant.id)
+        end
+      end
+
+      context "if the order cycle opens in the future" do
+        let!(:order_cycle) { future_order_cycle }
+
+        it "is eligible" do
+          expect(validator.__send__(:available_variant_ids)).to include(variant.id)
+        end
+      end
+
+      context "if the order cycle closed in the past" do
+        let!(:order_cycle) { past_order_cycle }
+
+        it "is not eligible" do
+          expect(validator.__send__(:available_variant_ids)).to_not include(variant.id)
+        end
+      end
+    end
+  end
 end

From d3f20289f85e1fd3f4240af3fc9d03a8201768f9 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Wed, 12 Sep 2018 22:40:33 +0800
Subject: [PATCH 04/13] Move setup of feature specs for creating subscriptions

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

diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb
index 6610c006ee..be2eaa6ba5 100644
--- a/spec/features/admin/subscriptions_spec.rb
+++ b/spec/features/admin/subscriptions_spec.rb
@@ -156,12 +156,14 @@ feature 'Subscriptions' do
       let!(:payment_method) { create(:stripe_payment_method, name: 'Credit Card', distributors: [shop], preferred_enterprise_id: shop.id) }
       let!(:shipping_method) { create(:shipping_method, distributors: [shop]) }
 
-      it "passes the smoke test" do
+      before do
         visit admin_subscriptions_path
-        click_link 'New Subscription'
-        select2_select shop.name, from: 'new_subscription_shop_id'
-        click_button 'Continue'
+        click_link "New Subscription"
+        select2_select shop.name, from: "new_subscription_shop_id"
+        click_button "Continue"
+      end
 
+      it "passes the smoke test" do
         select2_select customer.email, from: 'customer_id'
         select2_select schedule.name, from: 'schedule_id'
         select2_select payment_method.name, from: 'payment_method_id'

From 57f6a7a3b9dffb7a4b3cf7eabf72a111f4804d9c Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Wed, 19 Sep 2018 19:01:32 +0800
Subject: [PATCH 05/13] Support clicking different text for select2 helper

Interaction with the variant autocomplete is not precise. The specs only
search for the product name, then click the first result that matches
the product name which they see.

This could have been the case because searching using the full variant
name does not match the variant. For example, searching "Some Product -
1kg" would not have results, while searching only "Some Product" (the
product name) would list "Some Product - 1kg".

Clicking the first match does not work in all scenarios.

This allows using a separate text for searching and for clicking.
---
 spec/support/request/web_helper.rb | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/spec/support/request/web_helper.rb b/spec/support/request/web_helper.rb
index c37014e637..912c19e445 100644
--- a/spec/support/request/web_helper.rb
+++ b/spec/support/request/web_helper.rb
@@ -105,6 +105,16 @@ module WebHelper
     targetted_select2(value, options)
   end
 
+  # Support having different texts to search for and to click in the select2
+  # field.
+  #
+  # This overrides the method in Spree.
+  def targetted_select2_search(value, options)
+    page.execute_script %Q{$('#{options[:from]}').select2('open')}
+    page.execute_script "$('#{options[:dropdown_css]} input.select2-input').val('#{value}').trigger('keyup-change');"
+    select_select2_result(options[:select_text] || value)
+  end
+
   def multi_select2_select(value, options)
     find("#s2id_#{options[:from]}").find('ul li.select2-search-field').click
     select_select2_result(value)

From 929290fc77c34b5f4205a75d9ab8ee09e76158e4 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Sun, 20 Jan 2019 17:51:37 +0800
Subject: [PATCH 06/13] Reduce restrictions for creating subscriptions

Allow the following variants:

* Variants of permitted producers
* Variants of hub
* Variants that are in outgoing exchanges where the hub is receiver
---
 .../subscription_controller.js.coffee         |   6 +-
 .../directives/variant_autocomplete.js.coffee |   1 +
 .../subscription_line_items_controller.rb     |   7 +-
 app/services/subscription_validator.rb        |  15 +-
 .../scope_variants_for_search.rb              |  27 +++-
 lib/open_food_network/subscription_service.rb |  28 ++++
 ...subscription_line_items_controller_spec.rb |   4 +-
 .../admin/subscriptions_controller_spec.rb    |   2 +-
 spec/features/admin/subscriptions_spec.rb     | 141 ++++++++++++++----
 .../subscription_service_spec.rb              |  99 ++++++++++++
 spec/services/subscription_validator_spec.rb  |  87 +----------
 11 files changed, 296 insertions(+), 121 deletions(-)
 create mode 100644 lib/open_food_network/subscription_service.rb
 create mode 100644 spec/lib/open_food_network/subscription_service_spec.rb

diff --git a/app/assets/javascripts/admin/subscriptions/controllers/subscription_controller.js.coffee b/app/assets/javascripts/admin/subscriptions/controllers/subscription_controller.js.coffee
index 3b26866125..aa635da8cc 100644
--- a/app/assets/javascripts/admin/subscriptions/controllers/subscription_controller.js.coffee
+++ b/app/assets/javascripts/admin/subscriptions/controllers/subscription_controller.js.coffee
@@ -6,7 +6,11 @@ angular.module("admin.subscriptions").controller "SubscriptionController", ($sco
   $scope.schedules = Schedules.all
   $scope.paymentMethods = PaymentMethods.all
   $scope.shippingMethods = ShippingMethods.all
-  $scope.distributor_id = $scope.subscription.shop_id # variant selector requires distributor_id
+
+  # Variant selector requires these
+  $scope.distributor_id = $scope.subscription.shop_id
+  $scope.eligible_for_subscriptions = true
+
   $scope.view = if $scope.subscription.id? then 'review' else 'details'
   $scope.nextCallbacks = {}
   $scope.backCallbacks = {}
diff --git a/app/assets/javascripts/admin/utils/directives/variant_autocomplete.js.coffee b/app/assets/javascripts/admin/utils/directives/variant_autocomplete.js.coffee
index 9b8f12d3dc..17a467e463 100644
--- a/app/assets/javascripts/admin/utils/directives/variant_autocomplete.js.coffee
+++ b/app/assets/javascripts/admin/utils/directives/variant_autocomplete.js.coffee
@@ -22,6 +22,7 @@ angular.module("admin.utils").directive "variantAutocomplete", ($timeout) ->
               q: term
               distributor_id: scope.distributor_id
               order_cycle_id: scope.order_cycle_id
+              eligible_for_subscriptions: scope.eligible_for_subscriptions
             results: (data, page) ->
               results: data
           formatResult: (variant) ->
diff --git a/app/controllers/admin/subscription_line_items_controller.rb b/app/controllers/admin/subscription_line_items_controller.rb
index d981b71fe1..8c0432578d 100644
--- a/app/controllers/admin/subscription_line_items_controller.rb
+++ b/app/controllers/admin/subscription_line_items_controller.rb
@@ -1,6 +1,7 @@
 require 'open_food_network/permissions'
 require 'open_food_network/order_cycle_permissions'
 require 'open_food_network/scope_variant_to_hub'
+require "open_food_network/subscription_service"
 
 module Admin
   class SubscriptionLineItemsController < ResourceController
@@ -26,7 +27,7 @@ module Admin
       @shop = Enterprise.managed_by(spree_current_user).find_by_id(params[:shop_id])
       @schedule = permissions.editable_schedules.find_by_id(params[:schedule_id])
       @order_cycle = @schedule.andand.current_or_next_order_cycle
-      @variant = Spree::Variant.stockable_by(@shop).find_by_id(params[:subscription_line_item][:variant_id])
+      @variant = variant_if_eligible(params[:subscription_line_item][:variant_id]) if @shop.present?
     end
 
     def new_actions
@@ -50,5 +51,9 @@ module Admin
       OpenFoodNetwork::ScopeVariantToHub.new(@shop).scope(@variant)
       @variant.price + fee_calculator.indexed_fees_for(@variant)
     end
+
+    def variant_if_eligible(variant_id)
+      OpenFoodNetwork::SubscriptionService.eligible_variants(@shop).find_by_id(variant_id)
+    end
   end
 end
diff --git a/app/services/subscription_validator.rb b/app/services/subscription_validator.rb
index 33fc2baf77..8ee43b9c4f 100644
--- a/app/services/subscription_validator.rb
+++ b/app/services/subscription_validator.rb
@@ -2,6 +2,8 @@
 # Public interface consists of #valid? method provided by ActiveModel::Validations
 # and #json_errors which compiles a serializable hash of errors
 
+require "open_food_network/subscription_service"
+
 class SubscriptionValidator
   include ActiveModel::Naming
   include ActiveModel::Conversion
@@ -97,15 +99,12 @@ class SubscriptionValidator
     errors.add(:subscription_line_items, :not_available, name: name)
   end
 
-  # TODO: Extract this into a separate class
   def available_variant_ids
-    @available_variant_ids ||=
-      Spree::Variant.joins(exchanges: { order_cycle: :schedules })
-        .where(id: subscription_line_items.map(&:variant_id))
-        .where(schedules: { id: schedule }, exchanges: { incoming: false, receiver_id: shop })
-        .merge(OrderCycle.not_closed)
-        .select('DISTINCT spree_variants.id')
-        .pluck(:id)
+    return @available_variant_ids if @available_variant_ids.present?
+
+    subscription_variant_ids = subscription_line_items.map(&:variant_id)
+    @available_variant_ids = OpenFoodNetwork::SubscriptionService.eligible_variants(shop)
+      .where(id: subscription_variant_ids).pluck(:id)
   end
 
   def build_msg_from(k, msg)
diff --git a/lib/open_food_network/scope_variants_for_search.rb b/lib/open_food_network/scope_variants_for_search.rb
index 9befef5304..c1f90df7d1 100644
--- a/lib/open_food_network/scope_variants_for_search.rb
+++ b/lib/open_food_network/scope_variants_for_search.rb
@@ -5,6 +5,8 @@ require 'open_food_network/scope_variant_to_hub'
 # Further restrictions on the schedule, order_cycle or distributor through which the
 # products are available are also possible
 
+require "open_food_network/subscription_service"
+
 module OpenFoodNetwork
   class ScopeVariantsForSearch
     def initialize(params)
@@ -33,6 +35,10 @@ module OpenFoodNetwork
       Spree::Variant.where(is_master: false).ransack(search_params.merge(m: 'or')).result
     end
 
+    def distributor
+      Enterprise.find params[:distributor_id]
+    end
+
     def scope_to_schedule
       @variants = @variants.in_schedule(params[:schedule_id])
     end
@@ -42,12 +48,29 @@ module OpenFoodNetwork
     end
 
     def scope_to_distributor
-      distributor = Enterprise.find params[:distributor_id]
+      if params[:eligible_for_subscriptions]
+        scope_to_eligible_for_subscriptions_in_distributor
+      else
+        scope_to_available_for_orders_in_distributor
+      end
+    end
+
+    def scope_to_available_for_orders_in_distributor
       @variants = @variants.in_distributor(distributor)
+      scope_variants_to_distributor(@variants, distributor)
+    end
+
+    def scope_to_eligible_for_subscriptions_in_distributor
+      eligible_variants_scope = OpenFoodNetwork::SubscriptionService.eligible_variants(distributor)
+      @variants = @variants.merge(eligible_variants_scope)
+      scope_variants_to_distributor(@variants, distributor)
+    end
+
+    def scope_variants_to_distributor(variants, distributor)
       scoper = OpenFoodNetwork::ScopeVariantToHub.new(distributor)
       # Perform scoping after all filtering is done.
       # Filtering could be a problem on scoped variants.
-      @variants.each { |v| scoper.scope(v) }
+      variants.each { |v| scoper.scope(v) }
     end
   end
 end
diff --git a/lib/open_food_network/subscription_service.rb b/lib/open_food_network/subscription_service.rb
new file mode 100644
index 0000000000..e3479275c1
--- /dev/null
+++ b/lib/open_food_network/subscription_service.rb
@@ -0,0 +1,28 @@
+module OpenFoodNetwork
+  class SubscriptionService
+    # Includes the following variants:
+    # - Variants of permitted producers
+    # - Variants of hub
+    # - Variants that are in outgoing exchanges where the hub is receiver
+    def self.eligible_variants(distributor)
+      permitted_order_cycle_enterprise_ids = EnterpriseRelationship.permitting(distributor)
+        .with_permission(:add_to_order_cycle).pluck(:parent_id)
+      permitted_producer_ids = Enterprise.is_primary_producer
+        .where('enterprises.id IN (?)', permitted_order_cycle_enterprise_ids).pluck(:id)
+
+      outgoing_exchange_variant_ids = ExchangeVariant
+        .select("DISTINCT exchange_variants.variant_id")
+        .joins(:exchange)
+        .where(exchanges: { incoming: false, receiver_id: distributor.id })
+        .pluck(:variant_id)
+
+      variant_conditions = ["spree_products.supplier_id IN (?)", permitted_producer_ids | [distributor.id]]
+      if outgoing_exchange_variant_ids.present?
+        variant_conditions[0] << " OR spree_variants.id IN (?)"
+        variant_conditions << outgoing_exchange_variant_ids
+      end
+
+      Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions)
+    end
+  end
+end
diff --git a/spec/controllers/admin/subscription_line_items_controller_spec.rb b/spec/controllers/admin/subscription_line_items_controller_spec.rb
index b2f3a0f432..2b42a20df7 100644
--- a/spec/controllers/admin/subscription_line_items_controller_spec.rb
+++ b/spec/controllers/admin/subscription_line_items_controller_spec.rb
@@ -10,9 +10,9 @@ describe Admin::SubscriptionLineItemsController, type: :controller do
     let(:unmanaged_shop) { create(:enterprise) }
     let!(:product) { create(:product) }
     let!(:variant) { create(:variant, product: product, unit_value: '100', price: 15.00, option_values: []) }
+    let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant], enterprise_fees: [enterprise_fee]) }
     let!(:enterprise_fee) { create(:enterprise_fee, amount: 3.50) }
     let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) }
-    let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant], enterprise_fees: [enterprise_fee]) }
     let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
     let(:unmanaged_schedule) { create(:schedule, order_cycles: [create(:simple_order_cycle, coordinator: unmanaged_shop)]) }
 
@@ -42,6 +42,8 @@ describe Admin::SubscriptionLineItemsController, type: :controller do
           before { params.merge!(shop_id: shop.id) }
 
           context "but the shop doesn't have permission to sell product in question" do
+            let!(:outgoing_exchange) { }
+
             it "returns an error" do
               spree_post :build, params
               json_response = JSON.parse(response.body)
diff --git a/spec/controllers/admin/subscriptions_controller_spec.rb b/spec/controllers/admin/subscriptions_controller_spec.rb
index 5f83bf0529..9385d453f3 100644
--- a/spec/controllers/admin/subscriptions_controller_spec.rb
+++ b/spec/controllers/admin/subscriptions_controller_spec.rb
@@ -341,7 +341,7 @@ describe Admin::SubscriptionsController, type: :controller do
         end
 
         context 'with subscription_line_items params' do
-          let!(:product2) { create(:product, supplier: shop) }
+          let!(:product2) { create(:product) }
           let!(:variant2) { create(:variant, product: product2, unit_value: '1000', price: 6.00, option_values: []) }
 
           before do
diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb
index be2eaa6ba5..0a03d629f4 100644
--- a/spec/features/admin/subscriptions_spec.rb
+++ b/spec/features/admin/subscriptions_spec.rb
@@ -145,13 +145,13 @@ feature 'Subscriptions' do
       let!(:customer_user) { create(:user) }
       let!(:credit_card1) { create(:credit_card, user: customer_user, cc_type: 'visa', last_digits: 1111, month: 10, year: 2030) }
       let!(:customer) { create(:customer, enterprise: shop, bill_address: address, user: customer_user, allow_charges: true) }
-      let!(:product1) { create(:product, supplier: shop) }
-      let!(:product2) { create(:product, supplier: shop) }
-      let!(:variant1) { create(:variant, product: product1, unit_value: '100', price: 12.00, option_values: []) }
-      let!(:variant2) { create(:variant, product: product2, unit_value: '1000', price: 6.00, option_values: []) }
+      let!(:test_product) { create(:product, supplier: shop, distributors: []) }
+      let!(:test_variant) { create(:variant, product: test_product, unit_value: "100", price: 12.00, option_values: []) }
+      let!(:shop_product) { create(:product, supplier: shop, distributors: [shop]) }
+      let!(:shop_variant) { create(:variant, product: shop_product, unit_value: "1000", price: 6.00, option_values: []) }
       let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) }
       let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) }
-      let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2], enterprise_fees: [enterprise_fee]) }
+      let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [test_variant, shop_variant], enterprise_fees: [enterprise_fee]) }
       let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
       let!(:payment_method) { create(:stripe_payment_method, name: 'Credit Card', distributors: [shop], preferred_enterprise_id: shop.id) }
       let!(:shipping_method) { create(:shipping_method, distributors: [shop]) }
@@ -217,11 +217,9 @@ feature 'Subscriptions' do
         expect(page).to have_content 'Please add at least one product'
 
         # Adding a product and getting a price estimate
-        select2_search_async product1.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop'
-        fill_in 'add_quantity', with: 2
-        click_link 'Add'
+        add_variant_to_subscription test_variant, 2
         within 'table#subscription-line-items tr.item', match: :first do
-          expect(page).to have_selector 'td.description', text: "#{product1.name} - #{variant1.full_name}"
+          expect(page).to have_selector '.description', text: "#{test_product.name} - #{test_variant.full_name}"
           expect(page).to have_selector 'td.price', text: "$13.75"
           expect(page).to have_input 'quantity', with: "2"
           expect(page).to have_selector 'td.total', text: "$27.50"
@@ -243,11 +241,9 @@ feature 'Subscriptions' do
         click_button('edit-products')
 
         # Adding a new product
-        select2_search_async product2.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop'
-        fill_in 'add_quantity', with: 3
-        click_link 'Add'
+        add_variant_to_subscription shop_variant, 3
         within 'table#subscription-line-items tr.item', match: :first do
-          expect(page).to have_selector 'td.description', text: "#{product2.name} - #{variant2.full_name}"
+          expect(page).to have_selector '.description', text: "#{shop_product.name} - #{shop_variant.full_name}"
           expect(page).to have_selector 'td.price', text: "$7.75"
           expect(page).to have_input 'quantity', with: "3"
           expect(page).to have_selector 'td.total', text: "$23.25"
@@ -266,7 +262,7 @@ feature 'Subscriptions' do
 
         # Prices are shown in the index
         within 'table#subscription-line-items tr.item', match: :first do
-          expect(page).to have_selector 'td.description', text: "#{product2.name} - #{variant2.full_name}"
+          expect(page).to have_selector '.description', text: "#{shop_product.name} - #{shop_variant.full_name}"
           expect(page).to have_selector 'td.price', text: "$7.75"
           expect(page).to have_input 'quantity', with: "3"
           expect(page).to have_selector 'td.total', text: "$23.25"
@@ -284,7 +280,7 @@ feature 'Subscriptions' do
         # Standing Line Items are created
         expect(subscription.subscription_line_items.count).to eq 1
         subscription_line_item = subscription.subscription_line_items.first
-        expect(subscription_line_item.variant).to eq variant2
+        expect(subscription_line_item.variant).to eq shop_variant
         expect(subscription_line_item.quantity).to eq 3
       end
     end
@@ -336,7 +332,7 @@ feature 'Subscriptions' do
         # Existing products should be visible
         click_button 'edit-products'
         within "#sli_0" do
-          expect(page).to have_selector 'td.description', text: "#{product1.name} - #{variant1.full_name}"
+          expect(page).to have_selector '.description', text: "#{product1.name} - #{variant1.full_name}"
           expect(page).to have_selector 'td.price', text: "$13.75"
           expect(page).to have_input 'quantity', with: "2"
           expect(page).to have_selector 'td.total', text: "$27.50"
@@ -350,11 +346,9 @@ feature 'Subscriptions' do
         expect(page).to have_content 'Please add at least one product'
 
         # Add variant2 to the subscription
-        select2_search_async product2.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop'
-        fill_in 'add_quantity', with: 1
-        click_link 'Add'
+        add_variant_to_subscription(variant2, 1)
         within "#sli_0" do
-          expect(page).to have_selector 'td.description', text: "#{product2.name} - #{variant2.full_name}"
+          expect(page).to have_selector '.description', text: "#{product2.name} - #{variant2.full_name}"
           expect(page).to have_selector 'td.price', text: "$7.75"
           expect(page).to have_input 'quantity', with: "1"
           expect(page).to have_selector 'td.total', text: "$7.75"
@@ -364,11 +358,9 @@ feature 'Subscriptions' do
         expect(page).to have_selector '#order_form_total', text: "$7.75"
 
         # Add variant3 to the subscription (even though it is not available)
-        select2_search_async product3.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop'
-        fill_in 'add_quantity', with: 1
-        click_link 'Add'
+        add_variant_to_subscription(variant3, 1)
         within "#sli_1" do
-          expect(page).to have_selector 'td.description', text: "#{product3.name} - #{variant3.full_name}"
+          expect(page).to have_selector '.description', text: "#{product3.name} - #{variant3.full_name}"
           expect(page).to have_selector 'td.price', text: "$22.00"
           expect(page).to have_input 'quantity', with: "1"
           expect(page).to have_selector 'td.total', text: "$22.00"
@@ -377,9 +369,6 @@ feature 'Subscriptions' do
         # Total should be $29.75
         expect(page).to have_selector '#order_form_total', text: "$29.75"
 
-        click_button 'Save Changes'
-        expect(page).to have_content "#{product3.name} - #{variant3.full_name} is not available from the selected schedule"
-
         # Remove variant3 from the subscription
         within '#sli_1' do
           find("a.delete-item").click
@@ -421,5 +410,103 @@ feature 'Subscriptions' do
         end
       end
     end
+
+    describe "allowed variants" do
+      let!(:customer) { create(:customer, enterprise: shop, allow_charges: true) }
+      let!(:credit_card) { create(:credit_card, user: customer.user) }
+      let!(:shop_product) { create(:product, supplier: shop, distributors: [shop]) }
+      let!(:shop_variant) { create(:variant, product: shop_product, unit_value: "2000") }
+      let!(:permitted_supplier) do
+        create(:supplier_enterprise).tap do |supplier|
+          create(:enterprise_relationship, child: shop, parent: supplier, permissions_list: [:add_to_order_cycle])
+        end
+      end
+      let!(:permitted_supplier_product) { create(:product, supplier: permitted_supplier, distributors: [shop]) }
+      let!(:permitted_supplier_variant) { create(:variant, product: permitted_supplier_product, unit_value: "2000") }
+      let!(:incoming_exchange_product) { create(:product, distributors: [shop]) }
+      let!(:incoming_exchange_variant) do
+        create(:variant, product: incoming_exchange_product, unit_value: "2000").tap do |variant|
+          create(:exchange, order_cycle: order_cycle, incoming: true, receiver: shop, variants: [variant])
+        end
+      end
+      let!(:outgoing_exchange_product) { create(:product, distributors: [shop]) }
+      let!(:outgoing_exchange_variant) do
+        create(:variant, product: outgoing_exchange_product, unit_value: "2000").tap do |variant|
+          create(:exchange, order_cycle: order_cycle, incoming: false, receiver: shop, variants: [variant])
+        end
+      end
+      let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) }
+      let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) }
+      let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
+      let!(:payment_method) { create(:stripe_payment_method, distributors: [shop], preferred_enterprise_id: shop.id) }
+      let!(:shipping_method) { create(:shipping_method, distributors: [shop]) }
+
+      before do
+        visit admin_subscriptions_path
+        click_link "New Subscription"
+        select2_select shop.name, from: "new_subscription_shop_id"
+        click_button "Continue"
+      end
+
+      it "permit creating and editing of the subscription" do
+        select2_select customer.email, from: "customer_id"
+        select2_select schedule.name, from: "schedule_id"
+        select2_select payment_method.name, from: "payment_method_id"
+        select2_select shipping_method.name, from: "shipping_method_id"
+        find_field("begins_at").click
+        within(".ui-datepicker-calendar") do
+          find(".ui-datepicker-today").click
+        end
+        click_button "Next"
+
+        expect(page).to have_content "BILLING ADDRESS"
+        click_button "Next"
+
+        # Add products
+        expect(page).to have_content "NAME OR SKU"
+        add_variant_to_subscription shop_variant, 3
+        add_variant_to_subscription permitted_supplier_variant, 4
+        add_variant_to_subscription incoming_exchange_variant, 5
+        add_variant_to_subscription outgoing_exchange_variant, 6
+        click_button "Next"
+
+        # Submit form
+        expect {
+          click_button "Create Subscription"
+          expect(page).to have_current_path admin_subscriptions_path
+        }.to change(Subscription, :count).by(1)
+
+        # Subscription line items are created
+        subscription = Subscription.last
+        expect(subscription.subscription_line_items.count).to eq 4
+
+        # Edit the subscription
+        visit edit_admin_subscription_path(subscription)
+
+        # Remove shop_variant from the subscription
+        click_button "edit-products"
+        within "#sli_0" do
+          expect(page).to have_selector ".description", text: shop_variant.name
+          find("a.delete-item").click
+        end
+
+        # Submit form
+        click_button "Save Changes"
+        expect(page).to have_current_path admin_subscriptions_path
+
+        # Subscription is saved
+        visit edit_admin_subscription_path(subscription)
+        expect(page).to have_selector "#subscription-line-items .item", count: 3
+      end
+    end
+  end
+
+  def add_variant_to_subscription(variant, quantity)
+    row_count = all("#subscription-line-items .item").length
+    variant_name = variant.full_name.present? ? "#{variant.name} - #{variant.full_name}" : variant.name
+    select2_search variant.name, from: I18n.t(:name_or_sku), dropdown_css: ".select2-drop", select_text: variant_name
+    fill_in "add_quantity", with: quantity
+    click_link "Add"
+    expect(page).to have_selector("#subscription-line-items .item", count: row_count + 1)
   end
 end
diff --git a/spec/lib/open_food_network/subscription_service_spec.rb b/spec/lib/open_food_network/subscription_service_spec.rb
new file mode 100644
index 0000000000..7df860cf03
--- /dev/null
+++ b/spec/lib/open_food_network/subscription_service_spec.rb
@@ -0,0 +1,99 @@
+require "spec_helper"
+require "open_food_network/subscription_service"
+
+module OpenFoodNetwork
+  describe SubscriptionService do
+    describe "variant eligibility for subscription" do
+      let!(:shop) { create(:distributor_enterprise) }
+      let!(:producer) { create(:supplier_enterprise) }
+      let!(:product) { create(:product, supplier: producer) }
+      let!(:variant) { product.variants.first }
+
+      let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
+      let!(:subscription) { create(:subscription, shop: shop, schedule: schedule) }
+      let!(:subscription_line_item) do
+        create(:subscription_line_item, subscription: subscription, variant: variant)
+      end
+
+      let(:current_order_cycle) do
+        create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.ago,
+                                    orders_close_at: 1.week.from_now)
+      end
+
+      let(:future_order_cycle) do
+        create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.from_now,
+                                    orders_close_at: 2.weeks.from_now)
+      end
+
+      let(:past_order_cycle) do
+        create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.weeks.ago,
+                                    orders_close_at: 1.week.ago)
+      end
+
+      let!(:order_cycle) { current_order_cycle }
+
+      context "if the shop is the supplier for the product" do
+        let!(:producer) { shop }
+
+        it "is eligible" do
+          expect(described_class.eligible_variants(shop)).to include(variant)
+        end
+      end
+
+      context "if the supplier is permitted for the shop" do
+        let!(:enterprise_relationship) { create(:enterprise_relationship, child: shop, parent: product.supplier, permissions_list: [:add_to_order_cycle]) }
+
+        it "is eligible" do
+          expect(described_class.eligible_variants(shop)).to include(variant)
+        end
+      end
+
+      context "if the variant is involved in an exchange" do
+        let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) }
+        let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
+
+        context "if it is an incoming exchange where the shop is the receiver" do
+          let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) }
+
+          it "is not eligible" do
+            expect(described_class.eligible_variants(shop)).to_not include(variant)
+          end
+        end
+
+        context "if it is an outgoing exchange where the shop is the receiver" do
+          let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) }
+
+          context "if the order cycle is currently open" do
+            let!(:order_cycle) { current_order_cycle }
+
+            it "is eligible" do
+              expect(described_class.eligible_variants(shop)).to include(variant)
+            end
+          end
+
+          context "if the order cycle opens in the future" do
+            let!(:order_cycle) { future_order_cycle }
+
+            it "is eligible" do
+              expect(described_class.eligible_variants(shop)).to include(variant)
+            end
+          end
+
+          context "if the order cycle closed in the past" do
+            let!(:order_cycle) { past_order_cycle }
+
+            it "is eligible" do
+              expect(described_class.eligible_variants(shop)).to include(variant)
+            end
+          end
+        end
+      end
+
+      context "if the variant is unrelated" do
+        it "is not eligible" do
+          expect(described_class.eligible_variants(shop)).to_not include(variant)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/subscription_validator_spec.rb b/spec/services/subscription_validator_spec.rb
index c4878414a5..bdcd14bea5 100644
--- a/spec/services/subscription_validator_spec.rb
+++ b/spec/services/subscription_validator_spec.rb
@@ -1,10 +1,11 @@
 require "spec_helper"
 
 describe SubscriptionValidator do
-  let(:shop) { instance_double(Enterprise, name: "Shop") }
+  let(:owner) { create(:user) }
+  let(:shop) { create(:enterprise, name: "Shop", owner: owner) }
 
   describe "delegation" do
-    let(:subscription) { create(:subscription) }
+    let(:subscription) { create(:subscription, shop: shop) }
     let(:validator) { SubscriptionValidator.new(subscription) }
 
     it "delegates to subscription" do
@@ -440,6 +441,7 @@ describe SubscriptionValidator do
 
         context "but some variants are unavailable" do
           let(:product) { instance_double(Spree::Product, name: "some_name") }
+
           before do
             allow(validator).to receive(:available_variant_ids) { [variant2.id] }
             allow(variant1).to receive(:product) { product }
@@ -453,7 +455,9 @@ describe SubscriptionValidator do
         end
 
         context "and all requested variants are available" do
-          before { allow(validator).to receive(:available_variant_ids) { [variant1.id, variant2.id] } }
+          before do
+            allow(validator).to receive(:available_variant_ids) { [variant1.id, variant2.id] }
+          end
 
           it "returns true" do
             expect(validator.valid?).to be true
@@ -463,81 +467,4 @@ describe SubscriptionValidator do
       end
     end
   end
-
-  describe "variant eligibility for subscription" do
-    let!(:shop) { create(:distributor_enterprise) }
-    let!(:producer) { create(:supplier_enterprise) }
-    let!(:product) { create(:product, supplier: producer) }
-    let!(:variant) { product.variants.first }
-
-    let!(:order_cycle) { current_order_cycle }
-    let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
-    let!(:subscription) { create(:subscription, shop: shop, schedule: schedule) }
-    let!(:subscription_line_item) do
-      create(:subscription_line_item, subscription: subscription, variant: variant)
-    end
-
-    let(:current_order_cycle) do
-      create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.ago,
-                                  orders_close_at: 1.week.from_now)
-    end
-
-    let(:future_order_cycle) do
-      create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.from_now,
-                                  orders_close_at: 2.weeks.from_now)
-    end
-
-    let(:past_order_cycle) do
-      create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.weeks.ago,
-                                  orders_close_at: 1.week.ago)
-    end
-
-    let(:validator) { SubscriptionValidator.new(subscription) }
-
-    context "if it is not in an exchange" do
-      it "is not eligible" do
-        expect(validator.__send__(:available_variant_ids)).to_not include(variant.id)
-      end
-    end
-
-    context "if it is only in an incoming exchange" do
-      let!(:incoming_exchange) do
-        create(:exchange, order_cycle: order_cycle, sender: producer, receiver: shop,
-                          incoming: true, variants: [variant])
-      end
-
-      it "is not eligible" do
-        expect(validator.__send__(:available_variant_ids)).to_not include(variant.id)
-      end
-    end
-
-    context "if is an outgoing exchange where the shop is the receiver" do
-      let!(:outgoing_exchange) do
-        create(:exchange, order_cycle: order_cycle, sender: shop, receiver: shop,
-                          incoming: false, variants: [variant])
-      end
-
-      context "if the order cycle is currently open" do
-        it "is eligible" do
-          expect(validator.__send__(:available_variant_ids)).to include(variant.id)
-        end
-      end
-
-      context "if the order cycle opens in the future" do
-        let!(:order_cycle) { future_order_cycle }
-
-        it "is eligible" do
-          expect(validator.__send__(:available_variant_ids)).to include(variant.id)
-        end
-      end
-
-      context "if the order cycle closed in the past" do
-        let!(:order_cycle) { past_order_cycle }
-
-        it "is not eligible" do
-          expect(validator.__send__(:available_variant_ids)).to_not include(variant.id)
-        end
-      end
-    end
-  end
 end

From c23002102cc4b7b1593cf4e86023a435e4e03199 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Mon, 17 Sep 2018 01:14:40 +1000
Subject: [PATCH 07/13] Add warning for unavailable subscription items

---
 .../admin/pages/subscription_form.css.scss    |  5 +++
 .../admin/pages/subscription_review.css.scss  |  5 +++
 .../subscription_line_items_controller.rb     |  3 +-
 .../subscription_line_item_serializer.rb      | 19 ++++++++++-
 .../admin/subscriptions/_review.html.haml     |  7 ++--
 .../_subscription_line_items.html.haml        |  7 ++--
 config/locales/en.yml                         |  2 ++
 lib/open_food_network/subscription_service.rb |  8 +++++
 spec/features/admin/subscriptions_spec.rb     |  9 +++++
 .../subscription_service_spec.rb              | 34 +++++++++++++++++++
 10 files changed, 93 insertions(+), 6 deletions(-)
 create mode 100644 app/assets/stylesheets/admin/pages/subscription_form.css.scss
 create mode 100644 app/assets/stylesheets/admin/pages/subscription_review.css.scss

diff --git a/app/assets/stylesheets/admin/pages/subscription_form.css.scss b/app/assets/stylesheets/admin/pages/subscription_form.css.scss
new file mode 100644
index 0000000000..02c9fd783e
--- /dev/null
+++ b/app/assets/stylesheets/admin/pages/subscription_form.css.scss
@@ -0,0 +1,5 @@
+.admin-subscription-form-subscription-line-items {
+  .not-in-open-and-upcoming-order-cycles-warning {
+    color: $warning-red;
+  }
+}
diff --git a/app/assets/stylesheets/admin/pages/subscription_review.css.scss b/app/assets/stylesheets/admin/pages/subscription_review.css.scss
new file mode 100644
index 0000000000..ba3280aa2b
--- /dev/null
+++ b/app/assets/stylesheets/admin/pages/subscription_review.css.scss
@@ -0,0 +1,5 @@
+.admin-subscription-review-subscription-line-items {
+  .not-in-open-and-upcoming-order-cycles-warning {
+    color: $warning-red;
+  }
+}
diff --git a/app/controllers/admin/subscription_line_items_controller.rb b/app/controllers/admin/subscription_line_items_controller.rb
index 8c0432578d..378b73b990 100644
--- a/app/controllers/admin/subscription_line_items_controller.rb
+++ b/app/controllers/admin/subscription_line_items_controller.rb
@@ -14,7 +14,8 @@ module Admin
     def build
       @subscription_line_item.assign_attributes(params[:subscription_line_item])
       @subscription_line_item.price_estimate = price_estimate
-      render json: @subscription_line_item, serializer: Api::Admin::SubscriptionLineItemSerializer
+      render json: @subscription_line_item, serializer: Api::Admin::SubscriptionLineItemSerializer,
+             shop: @shop, schedule: @schedule
     end
 
     private
diff --git a/app/serializers/api/admin/subscription_line_item_serializer.rb b/app/serializers/api/admin/subscription_line_item_serializer.rb
index 23f4c6bc52..ca8028d7b5 100644
--- a/app/serializers/api/admin/subscription_line_item_serializer.rb
+++ b/app/serializers/api/admin/subscription_line_item_serializer.rb
@@ -1,7 +1,8 @@
 module Api
   module Admin
     class SubscriptionLineItemSerializer < ActiveModel::Serializer
-      attributes :id, :variant_id, :quantity, :description, :price_estimate
+      attributes :id, :variant_id, :quantity, :description, :price_estimate,
+                 :in_open_and_upcoming_order_cycles
 
       def description
         "#{object.variant.product.name} - #{object.variant.full_name}"
@@ -10,6 +11,22 @@ module Api
       def price_estimate
         object.price_estimate.andand.to_f || "?"
       end
+
+      def in_open_and_upcoming_order_cycles
+        OpenFoodNetwork::SubscriptionService
+          .in_open_and_upcoming_order_cycles?(option_or_assigned_shop, option_or_assigned_schedule,
+                                              object.variant)
+      end
+
+      private
+
+      def option_or_assigned_shop
+        @options[:shop] || object.subscription.andand.shop
+      end
+
+      def option_or_assigned_schedule
+        @options[:schedule] || object.subscription.andand.schedule
+      end
     end
   end
 end
diff --git a/app/views/admin/subscriptions/_review.html.haml b/app/views/admin/subscriptions/_review.html.haml
index e1591e6210..7e97bd02aa 100644
--- a/app/views/admin/subscriptions/_review.html.haml
+++ b/app/views/admin/subscriptions/_review.html.haml
@@ -56,7 +56,7 @@
           %input#edit-products{ type: "button", value: t(:edit), ng: { click: "setView('products')" } }
       .row
         .seven.columns.alpha.omega
-          %table#subscription-line-items
+          %table#subscription-line-items.admin-subscription-review-subscription-line-items
             %colgroup
               %col{:style => "width: 62%;"}/
               %col{:style => "width: 14%;"}/
@@ -71,7 +71,10 @@
                   %span= t(:total)
             %tbody
               %tr.item{ id: "sli_{{$index}}", ng: { repeat: "item in subscription.subscription_line_items | filter:{ _destroy: '!true' }", class: { even: 'even', odd: 'odd' } } }
-                %td.description {{ item.description }}
+                %td
+                  .description {{ item.description }}
+                  .not-in-open-and-upcoming-order-cycles-warning{ ng: { if: '!item.in_open_and_upcoming_order_cycles' } }
+                    = t(".no_open_or_upcoming_order_cycle")
                 %td.price.align-center {{ item.price_estimate | currency }}
                 %td.quantity {{ item.quantity }}
                 %td.total.align-center {{ (item.price_estimate * item.quantity) | currency }}
diff --git a/app/views/admin/subscriptions/_subscription_line_items.html.haml b/app/views/admin/subscriptions/_subscription_line_items.html.haml
index 0be91f2c93..b80b95a85b 100644
--- a/app/views/admin/subscriptions/_subscription_line_items.html.haml
+++ b/app/views/admin/subscriptions/_subscription_line_items.html.haml
@@ -1,4 +1,4 @@
-%table#subscription-line-items
+%table#subscription-line-items.admin-subscription-form-subscription-line-items
   %colgroup
     %col{:style => "width: 49%;"}/
     %col{:style => "width: 14%;"}/
@@ -15,7 +15,10 @@
       %th.orders-actions.actions
   %tbody
     %tr.item{ id: "sli_{{$index}}", ng: { repeat: "item in subscription.subscription_line_items | filter:{ _destroy: '!true' }", class: { even: 'even', odd: 'odd' } } }
-      %td.description {{ item.description }}
+      %td
+        .description {{ item.description }}
+        .not-in-open-and-upcoming-order-cycles-warning{ ng: { if: '!item.in_open_and_upcoming_order_cycles' } }
+          = t(".not_in_open_and_upcoming_order_cycles_warning")
       %td.price.align-center {{ item.price_estimate | currency }}
       %td.quantity
         %input{ name: 'quantity', type: 'number', min: 0, ng: { model: 'item.quantity' } }
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 0bde4fb005..366a481bac 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1088,6 +1088,7 @@ en:
         this_is_an_estimate: |
           The displayed prices are only an estimate and calculated at the time the subscription is changed.
           If you change prices or fees, orders will be updated, but the subscription will still display the old values.
+        not_in_open_and_upcoming_order_cycles_warning: "There are no open or upcoming order cycles for this product."
       details:
         details: Details
         invalid_error: Oops! Please fill in all of the required fields...
@@ -1102,6 +1103,7 @@ en:
         details: Details
         address: Address
         products: Products
+        no_open_or_upcoming_order_cycle: "No Upcoming OC"
       product_already_in_order: This product has already been added to the order. Please edit the quantity directly.
       orders:
         number: Number
diff --git a/lib/open_food_network/subscription_service.rb b/lib/open_food_network/subscription_service.rb
index e3479275c1..e4f48aa67a 100644
--- a/lib/open_food_network/subscription_service.rb
+++ b/lib/open_food_network/subscription_service.rb
@@ -24,5 +24,13 @@ module OpenFoodNetwork
 
       Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions)
     end
+
+    def self.in_open_and_upcoming_order_cycles?(distributor, schedule, variant)
+      scope = ExchangeVariant.joins(exchange: { order_cycle: :schedules })
+        .where(variant_id: variant, exchanges: { incoming: false, receiver_id: distributor })
+        .merge(OrderCycle.not_closed)
+      scope = scope.where(schedules: { id: schedule })
+      scope.any?
+    end
   end
 end
diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb
index 0a03d629f4..f51de02e5f 100644
--- a/spec/features/admin/subscriptions_spec.rb
+++ b/spec/features/admin/subscriptions_spec.rb
@@ -465,9 +465,13 @@ feature 'Subscriptions' do
         # Add products
         expect(page).to have_content "NAME OR SKU"
         add_variant_to_subscription shop_variant, 3
+        expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 1
         add_variant_to_subscription permitted_supplier_variant, 4
+        expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 2
         add_variant_to_subscription incoming_exchange_variant, 5
+        expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 3
         add_variant_to_subscription outgoing_exchange_variant, 6
+        expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 3
         click_button "Next"
 
         # Submit form
@@ -509,4 +513,9 @@ feature 'Subscriptions' do
     click_link "Add"
     expect(page).to have_selector("#subscription-line-items .item", count: row_count + 1)
   end
+
+  def variant_not_in_open_or_upcoming_order_cycle_warning
+    I18n.t("not_in_open_and_upcoming_order_cycles_warning",
+           scope: "admin.subscriptions.subscription_line_items")
+  end
 end
diff --git a/spec/lib/open_food_network/subscription_service_spec.rb b/spec/lib/open_food_network/subscription_service_spec.rb
index 7df860cf03..63ef72707d 100644
--- a/spec/lib/open_food_network/subscription_service_spec.rb
+++ b/spec/lib/open_food_network/subscription_service_spec.rb
@@ -95,5 +95,39 @@ module OpenFoodNetwork
         end
       end
     end
+
+    describe "checking if variant in open and upcoming order cycles" do
+      let!(:shop) { create(:enterprise) }
+      let!(:product) { create(:product) }
+      let!(:variant) { product.variants.first }
+      let!(:schedule) { create(:schedule) }
+
+      context "if the variant is involved in an exchange" do
+        let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) }
+        let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
+
+        context "if it is an incoming exchange where the shop is the receiver" do
+          let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) }
+
+          it "is is false" do
+            expect(described_class).not_to be_in_open_and_upcoming_order_cycles(shop, schedule, variant)
+          end
+        end
+
+        context "if it is an outgoing exchange where the shop is the receiver" do
+          let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) }
+
+          it "is true" do
+            expect(described_class).to be_in_open_and_upcoming_order_cycles(shop, schedule, variant)
+          end
+        end
+      end
+
+      context "if the variant is unrelated" do
+        it "is false" do
+          expect(described_class).to_not be_in_open_and_upcoming_order_cycles(shop, schedule, variant)
+        end
+      end
+    end
   end
 end

From b691d727a780da4f78103f1eb67e77e2e70827bc Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Sun, 20 Jan 2019 23:18:52 +1100
Subject: [PATCH 08/13] Move OFN::SubscriptionService to
 SubscriptionVariantsService

---
 .../subscription_line_items_controller.rb     |   3 +-
 .../subscription_line_item_serializer.rb      |   6 +-
 app/services/subscription_validator.rb        |   4 +-
 app/services/subscription_variants_service.rb |  34 +++++
 .../scope_variants_for_search.rb              |   4 +-
 lib/open_food_network/subscription_service.rb |  36 -----
 .../subscription_service_spec.rb              | 133 ------------------
 .../subscription_variants_service_spec.rb     | 130 +++++++++++++++++
 8 files changed, 170 insertions(+), 180 deletions(-)
 create mode 100644 app/services/subscription_variants_service.rb
 delete mode 100644 lib/open_food_network/subscription_service.rb
 delete mode 100644 spec/lib/open_food_network/subscription_service_spec.rb
 create mode 100644 spec/services/subscription_variants_service_spec.rb

diff --git a/app/controllers/admin/subscription_line_items_controller.rb b/app/controllers/admin/subscription_line_items_controller.rb
index 378b73b990..5267374539 100644
--- a/app/controllers/admin/subscription_line_items_controller.rb
+++ b/app/controllers/admin/subscription_line_items_controller.rb
@@ -1,7 +1,6 @@
 require 'open_food_network/permissions'
 require 'open_food_network/order_cycle_permissions'
 require 'open_food_network/scope_variant_to_hub'
-require "open_food_network/subscription_service"
 
 module Admin
   class SubscriptionLineItemsController < ResourceController
@@ -54,7 +53,7 @@ module Admin
     end
 
     def variant_if_eligible(variant_id)
-      OpenFoodNetwork::SubscriptionService.eligible_variants(@shop).find_by_id(variant_id)
+      SubscriptionVariantsService.eligible_variants(@shop).find_by_id(variant_id)
     end
   end
 end
diff --git a/app/serializers/api/admin/subscription_line_item_serializer.rb b/app/serializers/api/admin/subscription_line_item_serializer.rb
index ca8028d7b5..34bc00c6c0 100644
--- a/app/serializers/api/admin/subscription_line_item_serializer.rb
+++ b/app/serializers/api/admin/subscription_line_item_serializer.rb
@@ -13,9 +13,9 @@ module Api
       end
 
       def in_open_and_upcoming_order_cycles
-        OpenFoodNetwork::SubscriptionService
-          .in_open_and_upcoming_order_cycles?(option_or_assigned_shop, option_or_assigned_schedule,
-                                              object.variant)
+        SubscriptionVariantsService.in_open_and_upcoming_order_cycles?(option_or_assigned_shop,
+                                                                       option_or_assigned_schedule,
+                                                                       object.variant)
       end
 
       private
diff --git a/app/services/subscription_validator.rb b/app/services/subscription_validator.rb
index 8ee43b9c4f..051033dd9f 100644
--- a/app/services/subscription_validator.rb
+++ b/app/services/subscription_validator.rb
@@ -2,8 +2,6 @@
 # Public interface consists of #valid? method provided by ActiveModel::Validations
 # and #json_errors which compiles a serializable hash of errors
 
-require "open_food_network/subscription_service"
-
 class SubscriptionValidator
   include ActiveModel::Naming
   include ActiveModel::Conversion
@@ -103,7 +101,7 @@ class SubscriptionValidator
     return @available_variant_ids if @available_variant_ids.present?
 
     subscription_variant_ids = subscription_line_items.map(&:variant_id)
-    @available_variant_ids = OpenFoodNetwork::SubscriptionService.eligible_variants(shop)
+    @available_variant_ids = SubscriptionVariantsService.eligible_variants(shop)
       .where(id: subscription_variant_ids).pluck(:id)
   end
 
diff --git a/app/services/subscription_variants_service.rb b/app/services/subscription_variants_service.rb
new file mode 100644
index 0000000000..eb6dcf0cf2
--- /dev/null
+++ b/app/services/subscription_variants_service.rb
@@ -0,0 +1,34 @@
+class SubscriptionVariantsService
+  # Includes the following variants:
+  # - Variants of permitted producers
+  # - Variants of hub
+  # - Variants that are in outgoing exchanges where the hub is receiver
+  def self.eligible_variants(distributor)
+    permitted_order_cycle_enterprise_ids = EnterpriseRelationship.permitting(distributor)
+      .with_permission(:add_to_order_cycle).pluck(:parent_id)
+    permitted_producer_ids = Enterprise.is_primary_producer
+      .where('enterprises.id IN (?)', permitted_order_cycle_enterprise_ids).pluck(:id)
+
+    outgoing_exchange_variant_ids = ExchangeVariant
+      .select("DISTINCT exchange_variants.variant_id")
+      .joins(:exchange)
+      .where(exchanges: { incoming: false, receiver_id: distributor.id })
+      .pluck(:variant_id)
+
+    variant_conditions = ["spree_products.supplier_id IN (?)", permitted_producer_ids | [distributor.id]]
+    if outgoing_exchange_variant_ids.present?
+      variant_conditions[0] << " OR spree_variants.id IN (?)"
+      variant_conditions << outgoing_exchange_variant_ids
+    end
+
+    Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions)
+  end
+
+  def self.in_open_and_upcoming_order_cycles?(distributor, schedule, variant)
+    scope = ExchangeVariant.joins(exchange: { order_cycle: :schedules })
+      .where(variant_id: variant, exchanges: { incoming: false, receiver_id: distributor })
+      .merge(OrderCycle.not_closed)
+    scope = scope.where(schedules: { id: schedule })
+    scope.any?
+  end
+end
diff --git a/lib/open_food_network/scope_variants_for_search.rb b/lib/open_food_network/scope_variants_for_search.rb
index c1f90df7d1..24b110fb6e 100644
--- a/lib/open_food_network/scope_variants_for_search.rb
+++ b/lib/open_food_network/scope_variants_for_search.rb
@@ -5,8 +5,6 @@ require 'open_food_network/scope_variant_to_hub'
 # Further restrictions on the schedule, order_cycle or distributor through which the
 # products are available are also possible
 
-require "open_food_network/subscription_service"
-
 module OpenFoodNetwork
   class ScopeVariantsForSearch
     def initialize(params)
@@ -61,7 +59,7 @@ module OpenFoodNetwork
     end
 
     def scope_to_eligible_for_subscriptions_in_distributor
-      eligible_variants_scope = OpenFoodNetwork::SubscriptionService.eligible_variants(distributor)
+      eligible_variants_scope = SubscriptionVariantsService.eligible_variants(distributor)
       @variants = @variants.merge(eligible_variants_scope)
       scope_variants_to_distributor(@variants, distributor)
     end
diff --git a/lib/open_food_network/subscription_service.rb b/lib/open_food_network/subscription_service.rb
deleted file mode 100644
index e4f48aa67a..0000000000
--- a/lib/open_food_network/subscription_service.rb
+++ /dev/null
@@ -1,36 +0,0 @@
-module OpenFoodNetwork
-  class SubscriptionService
-    # Includes the following variants:
-    # - Variants of permitted producers
-    # - Variants of hub
-    # - Variants that are in outgoing exchanges where the hub is receiver
-    def self.eligible_variants(distributor)
-      permitted_order_cycle_enterprise_ids = EnterpriseRelationship.permitting(distributor)
-        .with_permission(:add_to_order_cycle).pluck(:parent_id)
-      permitted_producer_ids = Enterprise.is_primary_producer
-        .where('enterprises.id IN (?)', permitted_order_cycle_enterprise_ids).pluck(:id)
-
-      outgoing_exchange_variant_ids = ExchangeVariant
-        .select("DISTINCT exchange_variants.variant_id")
-        .joins(:exchange)
-        .where(exchanges: { incoming: false, receiver_id: distributor.id })
-        .pluck(:variant_id)
-
-      variant_conditions = ["spree_products.supplier_id IN (?)", permitted_producer_ids | [distributor.id]]
-      if outgoing_exchange_variant_ids.present?
-        variant_conditions[0] << " OR spree_variants.id IN (?)"
-        variant_conditions << outgoing_exchange_variant_ids
-      end
-
-      Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions)
-    end
-
-    def self.in_open_and_upcoming_order_cycles?(distributor, schedule, variant)
-      scope = ExchangeVariant.joins(exchange: { order_cycle: :schedules })
-        .where(variant_id: variant, exchanges: { incoming: false, receiver_id: distributor })
-        .merge(OrderCycle.not_closed)
-      scope = scope.where(schedules: { id: schedule })
-      scope.any?
-    end
-  end
-end
diff --git a/spec/lib/open_food_network/subscription_service_spec.rb b/spec/lib/open_food_network/subscription_service_spec.rb
deleted file mode 100644
index 63ef72707d..0000000000
--- a/spec/lib/open_food_network/subscription_service_spec.rb
+++ /dev/null
@@ -1,133 +0,0 @@
-require "spec_helper"
-require "open_food_network/subscription_service"
-
-module OpenFoodNetwork
-  describe SubscriptionService do
-    describe "variant eligibility for subscription" do
-      let!(:shop) { create(:distributor_enterprise) }
-      let!(:producer) { create(:supplier_enterprise) }
-      let!(:product) { create(:product, supplier: producer) }
-      let!(:variant) { product.variants.first }
-
-      let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
-      let!(:subscription) { create(:subscription, shop: shop, schedule: schedule) }
-      let!(:subscription_line_item) do
-        create(:subscription_line_item, subscription: subscription, variant: variant)
-      end
-
-      let(:current_order_cycle) do
-        create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.ago,
-                                    orders_close_at: 1.week.from_now)
-      end
-
-      let(:future_order_cycle) do
-        create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.from_now,
-                                    orders_close_at: 2.weeks.from_now)
-      end
-
-      let(:past_order_cycle) do
-        create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.weeks.ago,
-                                    orders_close_at: 1.week.ago)
-      end
-
-      let!(:order_cycle) { current_order_cycle }
-
-      context "if the shop is the supplier for the product" do
-        let!(:producer) { shop }
-
-        it "is eligible" do
-          expect(described_class.eligible_variants(shop)).to include(variant)
-        end
-      end
-
-      context "if the supplier is permitted for the shop" do
-        let!(:enterprise_relationship) { create(:enterprise_relationship, child: shop, parent: product.supplier, permissions_list: [:add_to_order_cycle]) }
-
-        it "is eligible" do
-          expect(described_class.eligible_variants(shop)).to include(variant)
-        end
-      end
-
-      context "if the variant is involved in an exchange" do
-        let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) }
-        let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
-
-        context "if it is an incoming exchange where the shop is the receiver" do
-          let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) }
-
-          it "is not eligible" do
-            expect(described_class.eligible_variants(shop)).to_not include(variant)
-          end
-        end
-
-        context "if it is an outgoing exchange where the shop is the receiver" do
-          let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) }
-
-          context "if the order cycle is currently open" do
-            let!(:order_cycle) { current_order_cycle }
-
-            it "is eligible" do
-              expect(described_class.eligible_variants(shop)).to include(variant)
-            end
-          end
-
-          context "if the order cycle opens in the future" do
-            let!(:order_cycle) { future_order_cycle }
-
-            it "is eligible" do
-              expect(described_class.eligible_variants(shop)).to include(variant)
-            end
-          end
-
-          context "if the order cycle closed in the past" do
-            let!(:order_cycle) { past_order_cycle }
-
-            it "is eligible" do
-              expect(described_class.eligible_variants(shop)).to include(variant)
-            end
-          end
-        end
-      end
-
-      context "if the variant is unrelated" do
-        it "is not eligible" do
-          expect(described_class.eligible_variants(shop)).to_not include(variant)
-        end
-      end
-    end
-
-    describe "checking if variant in open and upcoming order cycles" do
-      let!(:shop) { create(:enterprise) }
-      let!(:product) { create(:product) }
-      let!(:variant) { product.variants.first }
-      let!(:schedule) { create(:schedule) }
-
-      context "if the variant is involved in an exchange" do
-        let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) }
-        let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
-
-        context "if it is an incoming exchange where the shop is the receiver" do
-          let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) }
-
-          it "is is false" do
-            expect(described_class).not_to be_in_open_and_upcoming_order_cycles(shop, schedule, variant)
-          end
-        end
-
-        context "if it is an outgoing exchange where the shop is the receiver" do
-          let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) }
-
-          it "is true" do
-            expect(described_class).to be_in_open_and_upcoming_order_cycles(shop, schedule, variant)
-          end
-        end
-      end
-
-      context "if the variant is unrelated" do
-        it "is false" do
-          expect(described_class).to_not be_in_open_and_upcoming_order_cycles(shop, schedule, variant)
-        end
-      end
-    end
-  end
-end
diff --git a/spec/services/subscription_variants_service_spec.rb b/spec/services/subscription_variants_service_spec.rb
new file mode 100644
index 0000000000..31d0ff4ca7
--- /dev/null
+++ b/spec/services/subscription_variants_service_spec.rb
@@ -0,0 +1,130 @@
+require "spec_helper"
+
+describe SubscriptionVariantsService do
+  describe "variant eligibility for subscription" do
+    let!(:shop) { create(:distributor_enterprise) }
+    let!(:producer) { create(:supplier_enterprise) }
+    let!(:product) { create(:product, supplier: producer) }
+    let!(:variant) { product.variants.first }
+
+    let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
+    let!(:subscription) { create(:subscription, shop: shop, schedule: schedule) }
+    let!(:subscription_line_item) do
+      create(:subscription_line_item, subscription: subscription, variant: variant)
+    end
+
+    let(:current_order_cycle) do
+      create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.ago,
+                                  orders_close_at: 1.week.from_now)
+    end
+
+    let(:future_order_cycle) do
+      create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.from_now,
+                                  orders_close_at: 2.weeks.from_now)
+    end
+
+    let(:past_order_cycle) do
+      create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.weeks.ago,
+                                  orders_close_at: 1.week.ago)
+    end
+
+    let!(:order_cycle) { current_order_cycle }
+
+    context "if the shop is the supplier for the product" do
+      let!(:producer) { shop }
+
+      it "is eligible" do
+        expect(described_class.eligible_variants(shop)).to include(variant)
+      end
+    end
+
+    context "if the supplier is permitted for the shop" do
+      let!(:enterprise_relationship) { create(:enterprise_relationship, child: shop, parent: product.supplier, permissions_list: [:add_to_order_cycle]) }
+
+      it "is eligible" do
+        expect(described_class.eligible_variants(shop)).to include(variant)
+      end
+    end
+
+    context "if the variant is involved in an exchange" do
+      let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) }
+      let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
+
+      context "if it is an incoming exchange where the shop is the receiver" do
+        let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) }
+
+        it "is not eligible" do
+          expect(described_class.eligible_variants(shop)).to_not include(variant)
+        end
+      end
+
+      context "if it is an outgoing exchange where the shop is the receiver" do
+        let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) }
+
+        context "if the order cycle is currently open" do
+          let!(:order_cycle) { current_order_cycle }
+
+          it "is eligible" do
+            expect(described_class.eligible_variants(shop)).to include(variant)
+          end
+        end
+
+        context "if the order cycle opens in the future" do
+          let!(:order_cycle) { future_order_cycle }
+
+          it "is eligible" do
+            expect(described_class.eligible_variants(shop)).to include(variant)
+          end
+        end
+
+        context "if the order cycle closed in the past" do
+          let!(:order_cycle) { past_order_cycle }
+
+          it "is eligible" do
+            expect(described_class.eligible_variants(shop)).to include(variant)
+          end
+        end
+      end
+    end
+
+    context "if the variant is unrelated" do
+      it "is not eligible" do
+        expect(described_class.eligible_variants(shop)).to_not include(variant)
+      end
+    end
+  end
+
+  describe "checking if variant in open and upcoming order cycles" do
+    let!(:shop) { create(:enterprise) }
+    let!(:product) { create(:product) }
+    let!(:variant) { product.variants.first }
+    let!(:schedule) { create(:schedule) }
+
+    context "if the variant is involved in an exchange" do
+      let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) }
+      let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) }
+
+      context "if it is an incoming exchange where the shop is the receiver" do
+        let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) }
+
+        it "is is false" do
+          expect(described_class).not_to be_in_open_and_upcoming_order_cycles(shop, schedule, variant)
+        end
+      end
+
+      context "if it is an outgoing exchange where the shop is the receiver" do
+        let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) }
+
+        it "is true" do
+          expect(described_class).to be_in_open_and_upcoming_order_cycles(shop, schedule, variant)
+        end
+      end
+    end
+
+    context "if the variant is unrelated" do
+      it "is false" do
+        expect(described_class).to_not be_in_open_and_upcoming_order_cycles(shop, schedule, variant)
+      end
+    end
+  end
+end

From 9965e95c65a29c6989f1d9abb0e7693f404c7837 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Sun, 20 Jan 2019 23:46:56 +1100
Subject: [PATCH 09/13] Refactor methods in SubscriptionVariantsService

---
 app/services/subscription_variants_service.rb | 33 +++++++++++--------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/app/services/subscription_variants_service.rb b/app/services/subscription_variants_service.rb
index eb6dcf0cf2..855c200303 100644
--- a/app/services/subscription_variants_service.rb
+++ b/app/services/subscription_variants_service.rb
@@ -4,21 +4,11 @@ class SubscriptionVariantsService
   # - Variants of hub
   # - Variants that are in outgoing exchanges where the hub is receiver
   def self.eligible_variants(distributor)
-    permitted_order_cycle_enterprise_ids = EnterpriseRelationship.permitting(distributor)
-      .with_permission(:add_to_order_cycle).pluck(:parent_id)
-    permitted_producer_ids = Enterprise.is_primary_producer
-      .where('enterprises.id IN (?)', permitted_order_cycle_enterprise_ids).pluck(:id)
-
-    outgoing_exchange_variant_ids = ExchangeVariant
-      .select("DISTINCT exchange_variants.variant_id")
-      .joins(:exchange)
-      .where(exchanges: { incoming: false, receiver_id: distributor.id })
-      .pluck(:variant_id)
-
-    variant_conditions = ["spree_products.supplier_id IN (?)", permitted_producer_ids | [distributor.id]]
-    if outgoing_exchange_variant_ids.present?
+    variant_conditions = ["spree_products.supplier_id IN (?)", permitted_producer_ids(distributor)]
+    exchange_variant_ids = outgoing_exchange_variant_ids(distributor)
+    if exchange_variant_ids.present?
       variant_conditions[0] << " OR spree_variants.id IN (?)"
-      variant_conditions << outgoing_exchange_variant_ids
+      variant_conditions << exchange_variant_ids
     end
 
     Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions)
@@ -31,4 +21,19 @@ class SubscriptionVariantsService
     scope = scope.where(schedules: { id: schedule })
     scope.any?
   end
+
+  def self.permitted_producer_ids(distributor)
+    other_permitted_producer_ids = EnterpriseRelationship.joins(:parent)
+      .permitting(distributor).with_permission(:add_to_order_cycle)
+      .merge(Enterprise.is_primary_producer)
+      .pluck(:parent_id)
+
+    other_permitted_producer_ids | [distributor.id]
+  end
+
+  def self.outgoing_exchange_variant_ids(distributor)
+    ExchangeVariant.select("DISTINCT exchange_variants.variant_id").joins(:exchange)
+      .where(exchanges: { incoming: false, receiver_id: distributor.id })
+      .pluck(:variant_id)
+  end
 end

From da4d6a092ac52b21fe7e95b83c201869952d8b97 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Mon, 21 Jan 2019 00:02:38 +1100
Subject: [PATCH 10/13] Add spec helper for choosing today from datepicker

---
 spec/features/admin/subscriptions_spec.rb  | 4 +---
 spec/spec_helper.rb                        | 1 +
 spec/support/features/datepicker_helper.rb | 9 +++++++++
 3 files changed, 11 insertions(+), 3 deletions(-)
 create mode 100644 spec/support/features/datepicker_helper.rb

diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb
index f51de02e5f..7626329ef2 100644
--- a/spec/features/admin/subscriptions_spec.rb
+++ b/spec/features/admin/subscriptions_spec.rb
@@ -454,9 +454,7 @@ feature 'Subscriptions' do
         select2_select payment_method.name, from: "payment_method_id"
         select2_select shipping_method.name, from: "shipping_method_id"
         find_field("begins_at").click
-        within(".ui-datepicker-calendar") do
-          find(".ui-datepicker-today").click
-        end
+        choose_today_from_datepicker
         click_button "Next"
 
         expect(page).to have_content "BILLING ADDRESS"
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 25dac67716..f404f4c95a 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -136,6 +136,7 @@ RSpec.configure do |config|
   config.extend  Spree::Api::TestingSupport::Setup, :type => :controller
   config.include Spree::Api::TestingSupport::Helpers, :type => :controller
   config.include OpenFoodNetwork::ControllerHelper, :type => :controller
+  config.include Features::DatepickerHelper, type: :feature
   config.include OpenFoodNetwork::FeatureToggleHelper
   config.include OpenFoodNetwork::FiltersHelper
   config.include OpenFoodNetwork::EnterpriseGroupsHelper
diff --git a/spec/support/features/datepicker_helper.rb b/spec/support/features/datepicker_helper.rb
new file mode 100644
index 0000000000..df966cfa05
--- /dev/null
+++ b/spec/support/features/datepicker_helper.rb
@@ -0,0 +1,9 @@
+module Features
+  module DatepickerHelper
+    def choose_today_from_datepicker
+      within(".ui-datepicker-calendar") do
+        find(".ui-datepicker-today").click
+      end
+    end
+  end
+end

From c33808e8f5659796339f8db0712e330ea112ada6 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Mon, 21 Jan 2019 00:14:12 +1100
Subject: [PATCH 11/13] Extract some actions in tests into methods

---
 spec/features/admin/subscriptions_spec.rb | 31 +++++++++++++++--------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb
index 7626329ef2..99f32162f6 100644
--- a/spec/features/admin/subscriptions_spec.rb
+++ b/spec/features/admin/subscriptions_spec.rb
@@ -449,27 +449,22 @@ feature 'Subscriptions' do
       end
 
       it "permit creating and editing of the subscription" do
-        select2_select customer.email, from: "customer_id"
-        select2_select schedule.name, from: "schedule_id"
-        select2_select payment_method.name, from: "payment_method_id"
-        select2_select shipping_method.name, from: "shipping_method_id"
-        find_field("begins_at").click
-        choose_today_from_datepicker
+        # Fill in other details
+        fill_in_subscription_basic_details
         click_button "Next"
-
         expect(page).to have_content "BILLING ADDRESS"
         click_button "Next"
 
         # Add products
         expect(page).to have_content "NAME OR SKU"
         add_variant_to_subscription shop_variant, 3
-        expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 1
+        expect_not_in_open_or_upcoming_order_cycle_warning 1
         add_variant_to_subscription permitted_supplier_variant, 4
-        expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 2
+        expect_not_in_open_or_upcoming_order_cycle_warning 2
         add_variant_to_subscription incoming_exchange_variant, 5
-        expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 3
+        expect_not_in_open_or_upcoming_order_cycle_warning 3
         add_variant_to_subscription outgoing_exchange_variant, 6
-        expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 3
+        expect_not_in_open_or_upcoming_order_cycle_warning 3
         click_button "Next"
 
         # Submit form
@@ -503,6 +498,20 @@ feature 'Subscriptions' do
     end
   end
 
+  def fill_in_subscription_basic_details
+    select2_select customer.email, from: "customer_id"
+    select2_select schedule.name, from: "schedule_id"
+    select2_select payment_method.name, from: "payment_method_id"
+    select2_select shipping_method.name, from: "shipping_method_id"
+
+    find_field("begins_at").click
+    choose_today_from_datepicker
+  end
+
+  def expect_not_in_open_or_upcoming_order_cycle_warning(count)
+    expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: count
+  end
+
   def add_variant_to_subscription(variant, quantity)
     row_count = all("#subscription-line-items .item").length
     variant_name = variant.full_name.present? ? "#{variant.name} - #{variant.full_name}" : variant.name

From 4a58aedf09b898a175132386bf82af1f22fbc6fb Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Tue, 22 Jan 2019 16:09:20 +0800
Subject: [PATCH 12/13] Use clearer translation for "No Upcoming OC"

---
 config/locales/en.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 366a481bac..f0c72f289a 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1103,7 +1103,7 @@ en:
         details: Details
         address: Address
         products: Products
-        no_open_or_upcoming_order_cycle: "No Upcoming OC"
+        no_open_or_upcoming_order_cycle: "No Upcoming Order Cycle"
       product_already_in_order: This product has already been added to the order. Please edit the quantity directly.
       orders:
         number: Number

From 5992bba97b80afa7264763c5ba5a92d647ed2c4f Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Sat, 26 Jan 2019 03:29:15 +0800
Subject: [PATCH 13/13] Import color variables in new page SCSS partials

"admin/all.css" which imports these SCSS partials already imports the
color variables in "admin/variables", so actually there should be no
need to import the variables again.

However, "application.css" calls "require_tree", which means asset
precompilation through Sprockets would attempt to compile each of the
SCSS partials individually. When compiled individually, the color
variables are not available to these partials.

This is a quick solution to allow precompilation of "application.css" to
complete.
---
 app/assets/stylesheets/admin/pages/subscription_form.css.scss   | 2 ++
 app/assets/stylesheets/admin/pages/subscription_review.css.scss | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/app/assets/stylesheets/admin/pages/subscription_form.css.scss b/app/assets/stylesheets/admin/pages/subscription_form.css.scss
index 02c9fd783e..b892c4f17e 100644
--- a/app/assets/stylesheets/admin/pages/subscription_form.css.scss
+++ b/app/assets/stylesheets/admin/pages/subscription_form.css.scss
@@ -1,3 +1,5 @@
+@import '../variables';
+
 .admin-subscription-form-subscription-line-items {
   .not-in-open-and-upcoming-order-cycles-warning {
     color: $warning-red;
diff --git a/app/assets/stylesheets/admin/pages/subscription_review.css.scss b/app/assets/stylesheets/admin/pages/subscription_review.css.scss
index ba3280aa2b..76008afc0f 100644
--- a/app/assets/stylesheets/admin/pages/subscription_review.css.scss
+++ b/app/assets/stylesheets/admin/pages/subscription_review.css.scss
@@ -1,3 +1,5 @@
+@import '../variables';
+
 .admin-subscription-review-subscription-line-items {
   .not-in-open-and-upcoming-order-cycles-warning {
     color: $warning-red;