From 523d819575c4408a0c2524a15576e931db9f4f73 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 11:09:16 +0000
Subject: [PATCH 01/12] Move and rename SubscriptionPaymentUpdater to
 Subscriptios::PaymentSetup to move to services/subscriptions and call it
 Setup instead to make explicit this is executed before the payment is
 processed

---
 app/jobs/subscription_confirm_job.rb          |  7 +++--
 .../services/subscriptions/payment_setup.rb   |  6 ++---
 .../subscriptions/payment_setup_spec.rb}      | 27 +++++++++----------
 3 files changed, 19 insertions(+), 21 deletions(-)
 rename lib/open_food_network/subscription_payment_updater.rb => app/services/subscriptions/payment_setup.rb (94%)
 rename spec/{lib/open_food_network/subscription_payment_updater_spec.rb => services/subscriptions/payment_setup_spec.rb} (89%)

diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb
index be7f589cfc..b7ed797be3 100644
--- a/app/jobs/subscription_confirm_job.rb
+++ b/app/jobs/subscription_confirm_job.rb
@@ -1,4 +1,3 @@
-require 'open_food_network/subscription_payment_updater'
 require 'open_food_network/subscription_summarizer'
 
 class SubscriptionConfirmJob
@@ -35,7 +34,7 @@ class SubscriptionConfirmJob
 
   def process!
     record_order(@order)
-    update_payment! if @order.payment_required?
+    setup_payment! if @order.payment_required?
     return send_failed_payment_email if @order.errors.present?
 
     @order.process_payments! if @order.payment_required?
@@ -44,8 +43,8 @@ class SubscriptionConfirmJob
     send_confirm_email
   end
 
-  def update_payment!
-    OpenFoodNetwork::SubscriptionPaymentUpdater.new(@order).update!
+  def setup_payment!
+    Subscriptions::PaymentSetup.new(@order).call!
   end
 
   def send_confirm_email
diff --git a/lib/open_food_network/subscription_payment_updater.rb b/app/services/subscriptions/payment_setup.rb
similarity index 94%
rename from lib/open_food_network/subscription_payment_updater.rb
rename to app/services/subscriptions/payment_setup.rb
index 4a9c8fa144..92677d6917 100644
--- a/lib/open_food_network/subscription_payment_updater.rb
+++ b/app/services/subscriptions/payment_setup.rb
@@ -1,10 +1,10 @@
-module OpenFoodNetwork
-  class SubscriptionPaymentUpdater
+module Subscriptions
+  class PaymentSetup
     def initialize(order)
       @order = order
     end
 
-    def update!
+    def call!
       create_payment
       ensure_payment_source
       return if order.errors.any?
diff --git a/spec/lib/open_food_network/subscription_payment_updater_spec.rb b/spec/services/subscriptions/payment_setup_spec.rb
similarity index 89%
rename from spec/lib/open_food_network/subscription_payment_updater_spec.rb
rename to spec/services/subscriptions/payment_setup_spec.rb
index 77be81020e..d0133ac388 100644
--- a/spec/lib/open_food_network/subscription_payment_updater_spec.rb
+++ b/spec/services/subscriptions/payment_setup_spec.rb
@@ -1,10 +1,9 @@
 require 'spec_helper'
-require 'open_food_network/subscription_payment_updater'
 
-module OpenFoodNetwork
-  describe SubscriptionPaymentUpdater do
+module Subscriptions
+  describe PaymentSetup do
     let(:order) { create(:order) }
-    let(:updater) { OpenFoodNetwork::SubscriptionPaymentUpdater.new(order) }
+    let(:updater) { Subscriptions::PaymentSetup.new(order) }
 
     describe "#payment" do
       context "when only one payment exists on the order" do
@@ -44,7 +43,7 @@ module OpenFoodNetwork
       end
     end
 
-    describe "#update!" do
+    describe "#call!" do
       let!(:payment){ create(:payment, amount: 10) }
 
       context "when no pending payments are present" do
@@ -58,7 +57,7 @@ module OpenFoodNetwork
         end
 
         it "creates a new payment on the order" do
-          expect{ updater.update! }.to change(Spree::Payment, :count).by(1)
+          expect{ updater.call! }.to change(Spree::Payment, :count).by(1)
           expect(order.payments.first.amount).to eq 5
         end
       end
@@ -76,7 +75,7 @@ module OpenFoodNetwork
           context "when the payment total doesn't match the outstanding balance on the order" do
             before { allow(order).to receive(:outstanding_balance) { 5 } }
             it "updates the payment total to reflect the outstanding balance" do
-              expect{ updater.update! }.to change(payment, :amount).from(10).to(5)
+              expect{ updater.call! }.to change(payment, :amount).from(10).to(5)
             end
           end
 
@@ -84,7 +83,7 @@ module OpenFoodNetwork
             before { allow(order).to receive(:outstanding_balance) { 10 } }
 
             it "does nothing" do
-              expect{ updater.update! }.to_not change(payment, :amount).from(10)
+              expect{ updater.call! }.to_not change(payment, :amount).from(10)
             end
           end
         end
@@ -104,7 +103,7 @@ module OpenFoodNetwork
 
               it "adds an error to the order and does not update the payment" do
                 expect(payment).to_not receive(:update_attributes)
-                expect{ updater.update! }.to change(order.errors[:base], :count).from(0).to(1)
+                expect{ updater.call! }.to change(order.errors[:base], :count).from(0).to(1)
               end
             end
 
@@ -116,7 +115,7 @@ module OpenFoodNetwork
 
               it "adds an error to the order and does not update the payment" do
                 expect(payment).to_not receive(:update_attributes)
-                expect{ updater.update! }.to change(order.errors[:base], :count).from(0).to(1)
+                expect{ updater.call! }.to change(order.errors[:base], :count).from(0).to(1)
               end
             end
 
@@ -129,7 +128,7 @@ module OpenFoodNetwork
               context "when the payment total doesn't match the outstanding balance on the order" do
                 before { allow(order).to receive(:outstanding_balance) { 5 } }
                 it "updates the payment total to reflect the outstanding balance" do
-                  expect{ updater.update! }.to change(payment, :amount).from(10).to(5)
+                  expect{ updater.call! }.to change(payment, :amount).from(10).to(5)
                 end
               end
 
@@ -137,7 +136,7 @@ module OpenFoodNetwork
                 before { allow(order).to receive(:outstanding_balance) { 10 } }
 
                 it "does nothing" do
-                  expect{ updater.update! }.to_not change(payment, :amount).from(10)
+                  expect{ updater.call! }.to_not change(payment, :amount).from(10)
                 end
               end
             end
@@ -149,7 +148,7 @@ module OpenFoodNetwork
             context "when the payment total doesn't match the outstanding balance on the order" do
               before { allow(order).to receive(:outstanding_balance) { 5 } }
               it "updates the payment total to reflect the outstanding balance" do
-                expect{ updater.update! }.to change(payment, :amount).from(10).to(5)
+                expect{ updater.call! }.to change(payment, :amount).from(10).to(5)
               end
             end
 
@@ -157,7 +156,7 @@ module OpenFoodNetwork
               before { allow(order).to receive(:outstanding_balance) { 10 } }
 
               it "does nothing" do
-                expect{ updater.update! }.to_not change(payment, :amount).from(10)
+                expect{ updater.call! }.to_not change(payment, :amount).from(10)
               end
             end
           end

From 25e3f729340bc98930d7bf211042acd117d986a5 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 11:54:53 +0000
Subject: [PATCH 02/12] Fix rubocop issues in subs payment_setup

---
 .rubocop_manual_todo.yml                      |  3 +-
 app/services/subscriptions/payment_setup.rb   |  2 +
 .../subscriptions/payment_setup_spec.rb       | 68 +++++++++++--------
 3 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index 77b598ad3c..c0ca52abbd 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -241,7 +241,6 @@ Layout/LineLength:
   - spec/lib/open_food_network/products_and_inventory_report_spec.rb
   - spec/lib/open_food_network/proxy_order_syncer_spec.rb
   - spec/lib/open_food_network/scope_variant_to_hub_spec.rb
-  - spec/lib/open_food_network/subscription_payment_updater_spec.rb
   - spec/lib/open_food_network/subscription_summarizer_spec.rb
   - spec/lib/open_food_network/tag_rule_applicator_spec.rb
   - spec/lib/open_food_network/users_and_enterprises_report_spec.rb
@@ -703,7 +702,6 @@ Metrics/ModuleLength:
   - spec/lib/open_food_network/products_and_inventory_report_spec.rb
   - spec/lib/open_food_network/proxy_order_syncer_spec.rb
   - spec/lib/open_food_network/scope_variant_to_hub_spec.rb
-  - spec/lib/open_food_network/subscription_payment_updater_spec.rb
   - spec/lib/open_food_network/tag_rule_applicator_spec.rb
   - spec/lib/open_food_network/users_and_enterprises_report_spec.rb
   - spec/models/spree/ability_spec.rb
@@ -713,6 +711,7 @@ Metrics/ModuleLength:
   - spec/models/spree/product_spec.rb
   - spec/models/spree/variant_spec.rb
   - spec/services/permissions/order_spec.rb
+  - spec/services/subscriptions/payment_setup_spec.rb
   - spec/support/request/web_helper.rb
 
 Metrics/ParameterLists:
diff --git a/app/services/subscriptions/payment_setup.rb b/app/services/subscriptions/payment_setup.rb
index 92677d6917..9448a0828c 100644
--- a/app/services/subscriptions/payment_setup.rb
+++ b/app/services/subscriptions/payment_setup.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Subscriptions
   class PaymentSetup
     def initialize(order)
diff --git a/spec/services/subscriptions/payment_setup_spec.rb b/spec/services/subscriptions/payment_setup_spec.rb
index d0133ac388..01b9720895 100644
--- a/spec/services/subscriptions/payment_setup_spec.rb
+++ b/spec/services/subscriptions/payment_setup_spec.rb
@@ -1,21 +1,23 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 module Subscriptions
   describe PaymentSetup do
     let(:order) { create(:order) }
-    let(:updater) { Subscriptions::PaymentSetup.new(order) }
+    let(:payment_setup) { Subscriptions::PaymentSetup.new(order) }
 
     describe "#payment" do
       context "when only one payment exists on the order" do
         let!(:payment) { create(:payment, order: order) }
 
         context "where the payment is pending" do
-          it { expect(updater.send(:payment)).to eq payment }
+          it { expect(payment_setup.__send__(:payment)).to eq payment }
         end
 
         context "where the payment is failed" do
           before { payment.update_attribute(:state, 'failed') }
-          it { expect(updater.send(:payment)).to be nil }
+          it { expect(payment_setup.__send__(:payment)).to be nil }
         end
       end
 
@@ -24,12 +26,12 @@ module Subscriptions
         let!(:payment2) { create(:payment, order: order) }
 
         context "where more than one payment is pending" do
-          it { expect([payment1, payment2]).to include updater.send(:payment) }
+          it { expect([payment1, payment2]).to include payment_setup.__send__(:payment) }
         end
 
         context "where only one payment is pending" do
           before { payment1.update_attribute(:state, 'failed') }
-          it { expect(updater.send(:payment)).to eq payment2 }
+          it { expect(payment_setup.__send__(:payment)).to eq payment2 }
         end
 
         context "where no payments are pending" do
@@ -38,7 +40,7 @@ module Subscriptions
             payment2.update_attribute(:state, 'failed')
           end
 
-          it { expect(updater.send(:payment)).to be nil }
+          it { expect(payment_setup.__send__(:payment)).to be nil }
         end
       end
     end
@@ -57,7 +59,7 @@ module Subscriptions
         end
 
         it "creates a new payment on the order" do
-          expect{ updater.call! }.to change(Spree::Payment, :count).by(1)
+          expect{ payment_setup.call! }.to change(Spree::Payment, :count).by(1)
           expect(order.payments.first.amount).to eq 5
         end
       end
@@ -67,15 +69,15 @@ module Subscriptions
 
         context "when a credit card is not required" do
           before do
-            allow(updater).to receive(:card_required?) { false }
-            expect(updater).to_not receive(:card_available?)
-            expect(updater).to_not receive(:ensure_credit_card)
+            allow(payment_setup).to receive(:card_required?) { false }
+            expect(payment_setup).to_not receive(:card_available?)
+            expect(payment_setup).to_not receive(:ensure_credit_card)
           end
 
           context "when the payment total doesn't match the outstanding balance on the order" do
             before { allow(order).to receive(:outstanding_balance) { 5 } }
             it "updates the payment total to reflect the outstanding balance" do
-              expect{ updater.call! }.to change(payment, :amount).from(10).to(5)
+              expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
             end
           end
 
@@ -83,18 +85,18 @@ module Subscriptions
             before { allow(order).to receive(:outstanding_balance) { 10 } }
 
             it "does nothing" do
-              expect{ updater.call! }.to_not change(payment, :amount).from(10)
+              expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
             end
           end
         end
 
         context "when a credit card is required" do
           before do
-            expect(updater).to receive(:card_required?) { true }
+            expect(payment_setup).to receive(:card_required?) { true }
           end
 
           context "and the payment source is not a credit card" do
-            before { expect(updater).to receive(:card_set?) { false } }
+            before { expect(payment_setup).to receive(:card_set?) { false } }
 
             context "and no default credit card has been set by the customer" do
               before do
@@ -103,32 +105,40 @@ module Subscriptions
 
               it "adds an error to the order and does not update the payment" do
                 expect(payment).to_not receive(:update_attributes)
-                expect{ updater.call! }.to change(order.errors[:base], :count).from(0).to(1)
+                expect{ payment_setup.call! }.to change(order.errors[:base], :count).from(0).to(1)
               end
             end
 
             context "and the customer has not authorised the shop to charge to credit cards" do
               before do
-                allow(order).to receive(:user) { instance_double(Spree::User, default_card: create(:credit_card)) }
-                allow(order).to receive(:customer) { instance_double(Customer, allow_charges?: false) }
+                allow(order).to receive(:user) {
+                  instance_double(Spree::User, default_card: create(:credit_card))
+                }
+                allow(order).to receive(:customer) {
+                  instance_double(Customer, allow_charges?: false)
+                }
               end
 
               it "adds an error to the order and does not update the payment" do
                 expect(payment).to_not receive(:update_attributes)
-                expect{ updater.call! }.to change(order.errors[:base], :count).from(0).to(1)
+                expect{ payment_setup.call! }.to change(order.errors[:base], :count).from(0).to(1)
               end
             end
 
             context "and an authorised default credit card is available to charge" do
               before do
-                allow(order).to receive(:user) { instance_double(Spree::User, default_card: create(:credit_card)) }
-                allow(order).to receive(:customer) { instance_double(Customer, allow_charges?: true) }
+                allow(order).to receive(:user) {
+                  instance_double(Spree::User, default_card: create(:credit_card))
+                }
+                allow(order).to receive(:customer) {
+                  instance_double(Customer, allow_charges?: true)
+                }
               end
 
               context "when the payment total doesn't match the outstanding balance on the order" do
                 before { allow(order).to receive(:outstanding_balance) { 5 } }
                 it "updates the payment total to reflect the outstanding balance" do
-                  expect{ updater.call! }.to change(payment, :amount).from(10).to(5)
+                  expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
                 end
               end
 
@@ -136,19 +146,19 @@ module Subscriptions
                 before { allow(order).to receive(:outstanding_balance) { 10 } }
 
                 it "does nothing" do
-                  expect{ updater.call! }.to_not change(payment, :amount).from(10)
+                  expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
                 end
               end
             end
           end
 
           context "and the payment source is already a credit card" do
-            before { expect(updater).to receive(:card_set?) { true } }
+            before { expect(payment_setup).to receive(:card_set?) { true } }
 
             context "when the payment total doesn't match the outstanding balance on the order" do
               before { allow(order).to receive(:outstanding_balance) { 5 } }
               it "updates the payment total to reflect the outstanding balance" do
-                expect{ updater.call! }.to change(payment, :amount).from(10).to(5)
+                expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
               end
             end
 
@@ -156,7 +166,7 @@ module Subscriptions
               before { allow(order).to receive(:outstanding_balance) { 10 } }
 
               it "does nothing" do
-                expect{ updater.call! }.to_not change(payment, :amount).from(10)
+                expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
               end
             end
           end
@@ -166,7 +176,7 @@ module Subscriptions
 
     describe "#ensure_credit_card" do
       let!(:payment) { create(:payment, source: nil) }
-      before { allow(updater).to receive(:payment) { payment } }
+      before { allow(payment_setup).to receive(:payment) { payment } }
 
       context "when no default credit card is found" do
         before do
@@ -175,7 +185,7 @@ module Subscriptions
 
         it "returns false and down not update the payment source" do
           expect do
-            expect(updater.send(:ensure_credit_card)).to be false
+            expect(payment_setup.__send__(:ensure_credit_card)).to be false
           end.to_not change(payment, :source).from(nil)
         end
       end
@@ -193,7 +203,7 @@ module Subscriptions
 
           it "returns false and does not update the payment source" do
             expect do
-              expect(updater.send(:ensure_credit_card)).to be false
+              expect(payment_setup.__send__(:ensure_credit_card)).to be false
             end.to_not change(payment, :source).from(nil)
           end
         end
@@ -205,7 +215,7 @@ module Subscriptions
 
           it "returns true and stores the credit card as the payment source" do
             expect do
-              expect(updater.send(:ensure_credit_card)).to be true
+              expect(payment_setup.__send__(:ensure_credit_card)).to be true
             end.to change(payment, :source_id).from(nil).to(credit_card.id)
           end
         end

From 15231a9128c8ad4322b9eed271c3940b1da9b5a1 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 12:23:05 +0000
Subject: [PATCH 03/12] Make SubsConfirmJob more readable

---
 app/jobs/subscription_confirm_job.rb       | 65 +++++++++++++---------
 spec/jobs/subscription_confirm_job_spec.rb | 46 +++++++--------
 2 files changed, 59 insertions(+), 52 deletions(-)

diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb
index b7ed797be3..016f3bc21c 100644
--- a/app/jobs/subscription_confirm_job.rb
+++ b/app/jobs/subscription_confirm_job.rb
@@ -1,16 +1,9 @@
 require 'open_food_network/subscription_summarizer'
 
+# Confirms orders of unconfirmed proxy orders in recently closed Order Cycles
 class SubscriptionConfirmJob
   def perform
-    ids = proxy_orders.pluck(:id)
-    proxy_orders.update_all(confirmed_at: Time.zone.now)
-    ProxyOrder.where(id: ids).each do |proxy_order|
-      Rails.logger.info "Confirming Order for Proxy Order #{proxy_order.id}"
-      @order = proxy_order.order
-      process!
-    end
-
-    send_confirmation_summary_emails
+    confirm_proxy_orders!
   end
 
   private
@@ -22,7 +15,23 @@ class SubscriptionConfirmJob
     @summarizer ||= OpenFoodNetwork::SubscriptionSummarizer.new
   end
 
-  def proxy_orders
+  def confirm_proxy_orders!
+    # Fetch all unconfirmed proxy orders
+    unconfirmed_proxy_orders_ids = unconfirmed_proxy_orders.pluck(:id)
+
+    # Mark these proxy orders as confirmed
+    unconfirmed_proxy_orders.update_all(confirmed_at: Time.zone.now)
+
+    # Confirm these proxy orders
+    ProxyOrder.where(id: unconfirmed_proxy_orders_ids).each do |proxy_order|
+      Rails.logger.info "Confirming Order for Proxy Order #{proxy_order.id}"
+      confirm_order!(proxy_order.order)
+    end
+
+    send_confirmation_summary_emails
+  end
+
+  def unconfirmed_proxy_orders
     ProxyOrder.not_canceled.where('confirmed_at IS NULL AND placed_at IS NOT NULL')
       .joins(:order_cycle).merge(recently_closed_order_cycles)
       .joins(:order).merge(Spree::Order.complete.not_state('canceled'))
@@ -32,30 +41,32 @@ class SubscriptionConfirmJob
     OrderCycle.closed.where('order_cycles.orders_close_at BETWEEN (?) AND (?) OR order_cycles.updated_at BETWEEN (?) AND (?)', 1.hour.ago, Time.zone.now, 1.hour.ago, Time.zone.now)
   end
 
-  def process!
-    record_order(@order)
-    setup_payment! if @order.payment_required?
-    return send_failed_payment_email if @order.errors.present?
+  # It sets up payments, processes payments and sends confirmation emails
+  def confirm_order!(order)
+    record_order(order)
 
-    @order.process_payments! if @order.payment_required?
-    return send_failed_payment_email if @order.errors.present?
+    setup_payment!(order) if order.payment_required?
+    return send_failed_payment_email(order) if order.errors.present?
 
-    send_confirm_email
+    order.process_payments! if order.payment_required?
+    return send_failed_payment_email(order) if order.errors.present?
+
+    send_confirmation_email(order)
   end
 
-  def setup_payment!
-    Subscriptions::PaymentSetup.new(@order).call!
+  def setup_payment!(order)
+    Subscriptions::PaymentSetup.new(order).call!
   end
 
-  def send_confirm_email
-    @order.update!
-    record_success(@order)
-    SubscriptionMailer.confirmation_email(@order).deliver
+  def send_confirmation_email(order)
+    order.update!
+    record_success(order)
+    SubscriptionMailer.confirmation_email(order).deliver
   end
 
-  def send_failed_payment_email
-    @order.update!
-    record_and_log_error(:failed_payment, @order)
-    SubscriptionMailer.failed_payment_email(@order).deliver
+  def send_failed_payment_email(order)
+    order.update!
+    record_and_log_error(:failed_payment, order)
+    SubscriptionMailer.failed_payment_email(order).deliver
   end
 end
diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb
index 3c433bf94d..a061d0fbd4 100644
--- a/spec/jobs/subscription_confirm_job_spec.rb
+++ b/spec/jobs/subscription_confirm_job_spec.rb
@@ -16,7 +16,7 @@ describe SubscriptionConfirmJob do
                            placed_at: 5.minutes.ago)
     end
     let!(:order) { proxy_order.initialise_order! }
-    let(:proxy_orders) { job.send(:proxy_orders) }
+    let(:proxy_orders) { job.send(:unconfirmed_proxy_orders) }
 
     before do
       AdvanceOrderService.new(order).call!
@@ -80,8 +80,8 @@ describe SubscriptionConfirmJob do
 
       before do
         proxy_order.initialise_order!
-        allow(job).to receive(:proxy_orders) { ProxyOrder.where(id: proxy_order.id) }
-        allow(job).to receive(:process!)
+        allow(job).to receive(:unconfirmed_proxy_orders) { ProxyOrder.where(id: proxy_order.id) }
+        allow(job).to receive(:confirm_order!)
         allow(job).to receive(:send_confirmation_summary_emails)
       end
 
@@ -92,8 +92,7 @@ describe SubscriptionConfirmJob do
 
       it "processes confirmable proxy_orders" do
         job.perform
-        expect(job).to have_received(:process!)
-        expect(job.instance_variable_get(:@order)).to eq proxy_order.reload.order
+        expect(job).to have_received(:confirm_order!).with(proxy_order.reload.order)
       end
 
       it "sends a summary email" do
@@ -117,7 +116,7 @@ describe SubscriptionConfirmJob do
     end
   end
 
-  describe "processing an order" do
+  describe "confirming an order" do
     let(:shop) { create(:distributor_enterprise) }
     let(:order_cycle1) { create(:simple_order_cycle, coordinator: shop) }
     let(:order_cycle2) { create(:simple_order_cycle, coordinator: shop) }
@@ -128,35 +127,34 @@ describe SubscriptionConfirmJob do
 
     before do
       AdvanceOrderService.new(order).call!
-      allow(job).to receive(:send_confirm_email).and_call_original
-      job.instance_variable_set(:@order, order)
+      allow(job).to receive(:send_confirmation_email).and_call_original
       setup_email
-      expect(job).to receive(:record_order).with(order)
+      expect(job).to receive(:record_order)
     end
 
     context "when payments need to be processed" do
       let(:payment_method) { create(:payment_method) }
-      let(:payment) { double(:payment, amount: 10) }
+      let(:payment) { create(:payment, amount: 10) }
 
       before do
-        allow(order).to receive(:payment_total) { 0 }
-        allow(order).to receive(:total) { 10 }
         allow(order).to receive(:payment_required?) { true }
         allow(order).to receive(:pending_payments) { [payment] }
       end
 
       context "and an error is added to the order when updating payments" do
-        before { expect(job).to receive(:update_payment!) { order.errors.add(:base, "a payment error") } }
+        before do
+          expect(job).to receive(:setup_payment!) { |order| order.errors.add(:base, "a payment error") }
+        end
 
         it "sends a failed payment email" do
           expect(job).to receive(:send_failed_payment_email)
-          expect(job).to_not receive(:send_confirm_email)
-          job.send(:process!)
+          expect(job).to_not receive(:send_confirmation_email)
+          job.send(:confirm_order!, order)
         end
       end
 
       context "and no errors are added when updating payments" do
-        before { expect(job).to receive(:update_payment!) { true } }
+        before { expect(job).to receive(:setup_payment!) { true } }
 
         context "when an error occurs while processing the payment" do
           before do
@@ -165,8 +163,8 @@ describe SubscriptionConfirmJob do
 
           it "sends a failed payment email" do
             expect(job).to receive(:send_failed_payment_email)
-            expect(job).to_not receive(:send_confirm_email)
-            job.send(:process!)
+            expect(job).to_not receive(:send_confirmation_email)
+            job.send(:confirm_order!, order)
           end
         end
 
@@ -182,8 +180,8 @@ describe SubscriptionConfirmJob do
 
           it "sends only a subscription confirm email, no regular confirmation emails" do
             ActionMailer::Base.deliveries.clear
-            expect{ job.send(:process!) }.to_not enqueue_job ConfirmOrderJob
-            expect(job).to have_received(:send_confirm_email).once
+            expect{ job.send(:confirm_order!, order) }.to_not enqueue_job ConfirmOrderJob
+            expect(job).to have_received(:send_confirmation_email).once
             expect(ActionMailer::Base.deliveries.count).to be 1
           end
         end
@@ -191,19 +189,18 @@ describe SubscriptionConfirmJob do
     end
   end
 
-  describe "#send_confirm_email" do
+  describe "#send_confirmation_email" do
     let(:order) { instance_double(Spree::Order) }
     let(:mail_mock) { double(:mailer_mock, deliver: true) }
 
     before do
-      job.instance_variable_set(:@order, order)
       allow(SubscriptionMailer).to receive(:confirmation_email) { mail_mock }
     end
 
     it "records a success and sends the email" do
       expect(order).to receive(:update!)
       expect(job).to receive(:record_success).with(order).once
-      job.send(:send_confirm_email)
+      job.send(:send_confirmation_email, order)
       expect(SubscriptionMailer).to have_received(:confirmation_email).with(order)
       expect(mail_mock).to have_received(:deliver)
     end
@@ -214,14 +211,13 @@ describe SubscriptionConfirmJob do
     let(:mail_mock) { double(:mailer_mock, deliver: true) }
 
     before do
-      job.instance_variable_set(:@order, order)
       allow(SubscriptionMailer).to receive(:failed_payment_email) { mail_mock }
     end
 
     it "records and logs an error and sends the email" do
       expect(order).to receive(:update!)
       expect(job).to receive(:record_and_log_error).with(:failed_payment, order).once
-      job.send(:send_failed_payment_email)
+      job.send(:send_failed_payment_email, order)
       expect(SubscriptionMailer).to have_received(:failed_payment_email).with(order)
       expect(mail_mock).to have_received(:deliver)
     end

From 3aefea9f04f27694d749bff5fa91ce2cb27b1901 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 12:38:48 +0000
Subject: [PATCH 04/12] Prepare SubsConfirmJob to receive a bit more payment
 logic

---
 app/jobs/subscription_confirm_job.rb | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb
index 016f3bc21c..9db6cd6050 100644
--- a/app/jobs/subscription_confirm_job.rb
+++ b/app/jobs/subscription_confirm_job.rb
@@ -45,13 +45,24 @@ class SubscriptionConfirmJob
   def confirm_order!(order)
     record_order(order)
 
-    setup_payment!(order) if order.payment_required?
-    return send_failed_payment_email(order) if order.errors.present?
+    if process_payment!(order)
+      send_confirmation_email(order)
+    else
+      send_failed_payment_email(order)
+    end
+  end
 
-    order.process_payments! if order.payment_required?
-    return send_failed_payment_email(order) if order.errors.present?
+  def process_payment!(order)
+    return false if order.errors.present?
+    return true unless order.payment_required?
 
-    send_confirmation_email(order)
+    setup_payment!(order)
+    return false if order.errors.present?
+
+    order.process_payments!
+    return false if order.errors.present?
+
+    true
   end
 
   def setup_payment!(order)

From 34fa2d7ad660f3c485dacc5f1cb5870297051ab1 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 17:20:38 +0000
Subject: [PATCH 05/12] Move Subscriptions::PaymentSetup to OrderManagement
 engine where all subscription code will be at some point in the future

---
 .rubocop_manual_todo.yml                                      | 2 +-
 app/jobs/subscription_confirm_job.rb                          | 2 +-
 .../services/order_management}/subscriptions/payment_setup.rb | 2 +-
 .../order_management}/subscriptions/payment_setup_spec.rb     | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)
 rename {app/services => engines/order_management/app/services/order_management}/subscriptions/payment_setup.rb (97%)
 rename {spec/services => engines/order_management/spec/services/order_management}/subscriptions/payment_setup_spec.rb (98%)

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index c0ca52abbd..f61d3a9148 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -685,6 +685,7 @@ Metrics/ModuleLength:
   - app/helpers/injection_helper.rb
   - app/helpers/spree/admin/navigation_helper.rb
   - app/helpers/spree/admin/base_helper.rb
+  - engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
   - lib/open_food_network/column_preference_defaults.rb
   - spec/controllers/admin/enterprises_controller_spec.rb
   - spec/controllers/admin/order_cycles_controller_spec.rb
@@ -711,7 +712,6 @@ Metrics/ModuleLength:
   - spec/models/spree/product_spec.rb
   - spec/models/spree/variant_spec.rb
   - spec/services/permissions/order_spec.rb
-  - spec/services/subscriptions/payment_setup_spec.rb
   - spec/support/request/web_helper.rb
 
 Metrics/ParameterLists:
diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb
index 9db6cd6050..2d40b02539 100644
--- a/app/jobs/subscription_confirm_job.rb
+++ b/app/jobs/subscription_confirm_job.rb
@@ -66,7 +66,7 @@ class SubscriptionConfirmJob
   end
 
   def setup_payment!(order)
-    Subscriptions::PaymentSetup.new(order).call!
+    OrderManagement::Subscriptions::PaymentSetup.new(order).call!
   end
 
   def send_confirmation_email(order)
diff --git a/app/services/subscriptions/payment_setup.rb b/engines/order_management/app/services/order_management/subscriptions/payment_setup.rb
similarity index 97%
rename from app/services/subscriptions/payment_setup.rb
rename to engines/order_management/app/services/order_management/subscriptions/payment_setup.rb
index 9448a0828c..77e398c512 100644
--- a/app/services/subscriptions/payment_setup.rb
+++ b/engines/order_management/app/services/order_management/subscriptions/payment_setup.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module Subscriptions
+module OrderManagement::Subscriptions
   class PaymentSetup
     def initialize(order)
       @order = order
diff --git a/spec/services/subscriptions/payment_setup_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
similarity index 98%
rename from spec/services/subscriptions/payment_setup_spec.rb
rename to engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
index 01b9720895..f80d10cb95 100644
--- a/spec/services/subscriptions/payment_setup_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
@@ -2,10 +2,10 @@
 
 require 'spec_helper'
 
-module Subscriptions
+module OrderManagement::Subscriptions
   describe PaymentSetup do
     let(:order) { create(:order) }
-    let(:payment_setup) { Subscriptions::PaymentSetup.new(order) }
+    let(:payment_setup) { OrderManagement::Subscriptions::PaymentSetup.new(order) }
 
     describe "#payment" do
       context "when only one payment exists on the order" do

From e36b0249b9032c776368a571eee27985f7fd0d22 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 17:27:02 +0000
Subject: [PATCH 06/12] Use nested module names to fix rubocpo issue

---
 .../subscriptions/payment_setup.rb            |  94 ++---
 .../subscriptions/payment_setup_spec.rb       | 324 +++++++++---------
 2 files changed, 213 insertions(+), 205 deletions(-)

diff --git a/engines/order_management/app/services/order_management/subscriptions/payment_setup.rb b/engines/order_management/app/services/order_management/subscriptions/payment_setup.rb
index 77e398c512..86ce1f2c29 100644
--- a/engines/order_management/app/services/order_management/subscriptions/payment_setup.rb
+++ b/engines/order_management/app/services/order_management/subscriptions/payment_setup.rb
@@ -1,67 +1,69 @@
 # frozen_string_literal: true
 
-module OrderManagement::Subscriptions
-  class PaymentSetup
-    def initialize(order)
-      @order = order
-    end
+module OrderManagement
+  module Subscriptions
+    class PaymentSetup
+      def initialize(order)
+        @order = order
+      end
 
-    def call!
-      create_payment
-      ensure_payment_source
-      return if order.errors.any?
+      def call!
+        create_payment
+        ensure_payment_source
+        return if order.errors.any?
 
-      payment.update_attributes(amount: order.outstanding_balance)
-    end
+        payment.update_attributes(amount: order.outstanding_balance)
+      end
 
-    private
+      private
 
-    attr_reader :order
+      attr_reader :order
 
-    def payment
-      @payment ||= order.pending_payments.last
-    end
+      def payment
+        @payment ||= order.pending_payments.last
+      end
 
-    def create_payment
-      return if payment.present?
+      def create_payment
+        return if payment.present?
 
-      @payment = order.payments.create(
-        payment_method_id: order.subscription.payment_method_id,
-        amount: order.outstanding_balance
-      )
-    end
+        @payment = order.payments.create(
+          payment_method_id: order.subscription.payment_method_id,
+          amount: order.outstanding_balance
+        )
+      end
 
-    def card_required?
-      [Spree::Gateway::StripeConnect,
-       Spree::Gateway::StripeSCA].include? payment.payment_method.class
-    end
+      def card_required?
+        [Spree::Gateway::StripeConnect,
+         Spree::Gateway::StripeSCA].include? payment.payment_method.class
+      end
 
-    def card_set?
-      payment.source is_a? Spree::CreditCard
-    end
+      def card_set?
+        payment.source is_a? Spree::CreditCard
+      end
 
-    def ensure_payment_source
-      return unless card_required? && !card_set?
+      def ensure_payment_source
+        return unless card_required? && !card_set?
 
-      ensure_credit_card || order.errors.add(:base, :no_card)
-    end
+        ensure_credit_card || order.errors.add(:base, :no_card)
+      end
 
-    def ensure_credit_card
-      return false if saved_credit_card.blank? || !allow_charges?
+      def ensure_credit_card
+        return false if saved_credit_card.blank? || !allow_charges?
 
-      payment.update_attributes(source: saved_credit_card)
-    end
+        payment.update_attributes(source: saved_credit_card)
+      end
 
-    def allow_charges?
-      order.customer.allow_charges?
-    end
+      def allow_charges?
+        order.customer.allow_charges?
+      end
 
-    def saved_credit_card
-      order.user.default_card
-    end
+      def saved_credit_card
+        order.user.default_card
+      end
 
-    def errors_present?
-      order.errors.any?
+      def errors_present?
+        order.errors.any?
+      end
     end
   end
 end
diff --git a/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
index f80d10cb95..442fb64cb5 100644
--- a/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
@@ -2,138 +2,159 @@
 
 require 'spec_helper'
 
-module OrderManagement::Subscriptions
-  describe PaymentSetup do
-    let(:order) { create(:order) }
-    let(:payment_setup) { OrderManagement::Subscriptions::PaymentSetup.new(order) }
+module OrderManagement
+  module Subscriptions
+    describe PaymentSetup do
+      let(:order) { create(:order) }
+      let(:payment_setup) { OrderManagement::Subscriptions::PaymentSetup.new(order) }
 
-    describe "#payment" do
-      context "when only one payment exists on the order" do
-        let!(:payment) { create(:payment, order: order) }
+      describe "#payment" do
+        context "when only one payment exists on the order" do
+          let!(:payment) { create(:payment, order: order) }
 
-        context "where the payment is pending" do
-          it { expect(payment_setup.__send__(:payment)).to eq payment }
+          context "where the payment is pending" do
+            it { expect(payment_setup.__send__(:payment)).to eq payment }
+          end
+
+          context "where the payment is failed" do
+            before { payment.update_attribute(:state, 'failed') }
+            it { expect(payment_setup.__send__(:payment)).to be nil }
+          end
         end
 
-        context "where the payment is failed" do
-          before { payment.update_attribute(:state, 'failed') }
-          it { expect(payment_setup.__send__(:payment)).to be nil }
+        context "when more that one payment exists on the order" do
+          let!(:payment1) { create(:payment, order: order) }
+          let!(:payment2) { create(:payment, order: order) }
+
+          context "where more than one payment is pending" do
+            it { expect([payment1, payment2]).to include payment_setup.__send__(:payment) }
+          end
+
+          context "where only one payment is pending" do
+            before { payment1.update_attribute(:state, 'failed') }
+            it { expect(payment_setup.__send__(:payment)).to eq payment2 }
+          end
+
+          context "where no payments are pending" do
+            before do
+              payment1.update_attribute(:state, 'failed')
+              payment2.update_attribute(:state, 'failed')
+            end
+
+            it { expect(payment_setup.__send__(:payment)).to be nil }
+          end
         end
       end
 
-      context "when more that one payment exists on the order" do
-        let!(:payment1) { create(:payment, order: order) }
-        let!(:payment2) { create(:payment, order: order) }
+      describe "#call!" do
+        let!(:payment){ create(:payment, amount: 10) }
 
-        context "where more than one payment is pending" do
-          it { expect([payment1, payment2]).to include payment_setup.__send__(:payment) }
-        end
+        context "when no pending payments are present" do
+          let(:payment_method) { create(:payment_method) }
+          let(:subscription) { double(:subscription, payment_method_id: payment_method.id) }
 
-        context "where only one payment is pending" do
-          before { payment1.update_attribute(:state, 'failed') }
-          it { expect(payment_setup.__send__(:payment)).to eq payment2 }
-        end
-
-        context "where no payments are pending" do
           before do
-            payment1.update_attribute(:state, 'failed')
-            payment2.update_attribute(:state, 'failed')
+            allow(order).to receive(:pending_payments).once { [] }
+            allow(order).to receive(:outstanding_balance) { 5 }
+            allow(order).to receive(:subscription) { subscription }
           end
 
-          it { expect(payment_setup.__send__(:payment)).to be nil }
-        end
-      end
-    end
-
-    describe "#call!" do
-      let!(:payment){ create(:payment, amount: 10) }
-
-      context "when no pending payments are present" do
-        let(:payment_method) { create(:payment_method) }
-        let(:subscription) { double(:subscription, payment_method_id: payment_method.id) }
-
-        before do
-          allow(order).to receive(:pending_payments).once { [] }
-          allow(order).to receive(:outstanding_balance) { 5 }
-          allow(order).to receive(:subscription) { subscription }
-        end
-
-        it "creates a new payment on the order" do
-          expect{ payment_setup.call! }.to change(Spree::Payment, :count).by(1)
-          expect(order.payments.first.amount).to eq 5
-        end
-      end
-
-      context "when a payment is present" do
-        before { allow(order).to receive(:pending_payments).once { [payment] } }
-
-        context "when a credit card is not required" do
-          before do
-            allow(payment_setup).to receive(:card_required?) { false }
-            expect(payment_setup).to_not receive(:card_available?)
-            expect(payment_setup).to_not receive(:ensure_credit_card)
-          end
-
-          context "when the payment total doesn't match the outstanding balance on the order" do
-            before { allow(order).to receive(:outstanding_balance) { 5 } }
-            it "updates the payment total to reflect the outstanding balance" do
-              expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
-            end
-          end
-
-          context "when the payment total matches the outstanding balance on the order" do
-            before { allow(order).to receive(:outstanding_balance) { 10 } }
-
-            it "does nothing" do
-              expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
-            end
+          it "creates a new payment on the order" do
+            expect{ payment_setup.call! }.to change(Spree::Payment, :count).by(1)
+            expect(order.payments.first.amount).to eq 5
           end
         end
 
-        context "when a credit card is required" do
-          before do
-            expect(payment_setup).to receive(:card_required?) { true }
+        context "when a payment is present" do
+          before { allow(order).to receive(:pending_payments).once { [payment] } }
+
+          context "when a credit card is not required" do
+            before do
+              allow(payment_setup).to receive(:card_required?) { false }
+              expect(payment_setup).to_not receive(:card_available?)
+              expect(payment_setup).to_not receive(:ensure_credit_card)
+            end
+
+            context "when the payment total doesn't match the outstanding balance on the order" do
+              before { allow(order).to receive(:outstanding_balance) { 5 } }
+              it "updates the payment total to reflect the outstanding balance" do
+                expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
+              end
+            end
+
+            context "when the payment total matches the outstanding balance on the order" do
+              before { allow(order).to receive(:outstanding_balance) { 10 } }
+
+              it "does nothing" do
+                expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
+              end
+            end
           end
 
-          context "and the payment source is not a credit card" do
-            before { expect(payment_setup).to receive(:card_set?) { false } }
+          context "when a credit card is required" do
+            before do
+              expect(payment_setup).to receive(:card_required?) { true }
+            end
 
-            context "and no default credit card has been set by the customer" do
-              before do
-                allow(order).to receive(:user) { instance_double(Spree::User, default_card: nil) }
+            context "and the payment source is not a credit card" do
+              before { expect(payment_setup).to receive(:card_set?) { false } }
+
+              context "and no default credit card has been set by the customer" do
+                before do
+                  allow(order).to receive(:user) { instance_double(Spree::User, default_card: nil) }
+                end
+
+                it "adds an error to the order and does not update the payment" do
+                  expect(payment).to_not receive(:update_attributes)
+                  expect{ payment_setup.call! }.to change(order.errors[:base], :count).from(0).to(1)
+                end
               end
 
-              it "adds an error to the order and does not update the payment" do
-                expect(payment).to_not receive(:update_attributes)
-                expect{ payment_setup.call! }.to change(order.errors[:base], :count).from(0).to(1)
+              context "and the customer has not authorised the shop to charge to credit cards" do
+                before do
+                  allow(order).to receive(:user) {
+                    instance_double(Spree::User, default_card: create(:credit_card))
+                  }
+                  allow(order).to receive(:customer) {
+                    instance_double(Customer, allow_charges?: false)
+                  }
+                end
+
+                it "adds an error to the order and does not update the payment" do
+                  expect(payment).to_not receive(:update_attributes)
+                  expect{ payment_setup.call! }.to change(order.errors[:base], :count).from(0).to(1)
+                end
+              end
+
+              context "and an authorised default credit card is available to charge" do
+                before do
+                  allow(order).to receive(:user) {
+                    instance_double(Spree::User, default_card: create(:credit_card))
+                  }
+                  allow(order).to receive(:customer) {
+                    instance_double(Customer, allow_charges?: true)
+                  }
+                end
+
+                context "when the payment total doesn't match the order's outstanding balance" do
+                  before { allow(order).to receive(:outstanding_balance) { 5 } }
+                  it "updates the payment total to reflect the outstanding balance" do
+                    expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
+                  end
+                end
+
+                context "when the payment total matches the outstanding balance on the order" do
+                  before { allow(order).to receive(:outstanding_balance) { 10 } }
+
+                  it "does nothing" do
+                    expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
+                  end
+                end
               end
             end
 
-            context "and the customer has not authorised the shop to charge to credit cards" do
-              before do
-                allow(order).to receive(:user) {
-                  instance_double(Spree::User, default_card: create(:credit_card))
-                }
-                allow(order).to receive(:customer) {
-                  instance_double(Customer, allow_charges?: false)
-                }
-              end
-
-              it "adds an error to the order and does not update the payment" do
-                expect(payment).to_not receive(:update_attributes)
-                expect{ payment_setup.call! }.to change(order.errors[:base], :count).from(0).to(1)
-              end
-            end
-
-            context "and an authorised default credit card is available to charge" do
-              before do
-                allow(order).to receive(:user) {
-                  instance_double(Spree::User, default_card: create(:credit_card))
-                }
-                allow(order).to receive(:customer) {
-                  instance_double(Customer, allow_charges?: true)
-                }
-              end
+            context "and the payment source is already a credit card" do
+              before { expect(payment_setup).to receive(:card_set?) { true } }
 
               context "when the payment total doesn't match the outstanding balance on the order" do
                 before { allow(order).to receive(:outstanding_balance) { 5 } }
@@ -151,72 +172,57 @@ module OrderManagement::Subscriptions
               end
             end
           end
-
-          context "and the payment source is already a credit card" do
-            before { expect(payment_setup).to receive(:card_set?) { true } }
-
-            context "when the payment total doesn't match the outstanding balance on the order" do
-              before { allow(order).to receive(:outstanding_balance) { 5 } }
-              it "updates the payment total to reflect the outstanding balance" do
-                expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
-              end
-            end
-
-            context "when the payment total matches the outstanding balance on the order" do
-              before { allow(order).to receive(:outstanding_balance) { 10 } }
-
-              it "does nothing" do
-                expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
-              end
-            end
-          end
-        end
-      end
-    end
-
-    describe "#ensure_credit_card" do
-      let!(:payment) { create(:payment, source: nil) }
-      before { allow(payment_setup).to receive(:payment) { payment } }
-
-      context "when no default credit card is found" do
-        before do
-          allow(order).to receive(:user) { instance_double(Spree::User, default_card: nil) }
-        end
-
-        it "returns false and down not update the payment source" do
-          expect do
-            expect(payment_setup.__send__(:ensure_credit_card)).to be false
-          end.to_not change(payment, :source).from(nil)
         end
       end
 
-      context "when a default credit card is found" do
-        let(:credit_card) { create(:credit_card) }
-        before do
-          allow(order).to receive(:user) { instance_double(Spree::User, default_card: credit_card) }
-        end
+      describe "#ensure_credit_card" do
+        let!(:payment) { create(:payment, source: nil) }
+        before { allow(payment_setup).to receive(:payment) { payment } }
 
-        context "and charge have not been authorised by the customer" do
+        context "when no default credit card is found" do
           before do
-            allow(order).to receive(:customer) { instance_double(Customer, allow_charges?: false) }
+            allow(order).to receive(:user) { instance_double(Spree::User, default_card: nil) }
           end
 
-          it "returns false and does not update the payment source" do
+          it "returns false and down not update the payment source" do
             expect do
               expect(payment_setup.__send__(:ensure_credit_card)).to be false
             end.to_not change(payment, :source).from(nil)
           end
         end
 
-        context "and charges have been authorised by the customer" do
+        context "when a default credit card is found" do
+          let(:credit_card) { create(:credit_card) }
           before do
-            allow(order).to receive(:customer) { instance_double(Customer, allow_charges?: true) }
+            allow(order).to receive(:user) {
+              instance_double(Spree::User, default_card: credit_card)
+            }
           end
 
-          it "returns true and stores the credit card as the payment source" do
-            expect do
-              expect(payment_setup.__send__(:ensure_credit_card)).to be true
-            end.to change(payment, :source_id).from(nil).to(credit_card.id)
+          context "and charge have not been authorised by the customer" do
+            before do
+              allow(order).to receive(:customer) {
+                instance_double(Customer, allow_charges?: false)
+              }
+            end
+
+            it "returns false and does not update the payment source" do
+              expect do
+                expect(payment_setup.__send__(:ensure_credit_card)).to be false
+              end.to_not change(payment, :source).from(nil)
+            end
+          end
+
+          context "and charges have been authorised by the customer" do
+            before do
+              allow(order).to receive(:customer) { instance_double(Customer, allow_charges?: true) }
+            end
+
+            it "returns true and stores the credit card as the payment source" do
+              expect do
+                expect(payment_setup.__send__(:ensure_credit_card)).to be true
+              end.to change(payment, :source_id).from(nil).to(credit_card.id)
+            end
           end
         end
       end

From fb1c825fbccca76c95357be5e1da147b5ea5b6cc Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 17:47:38 +0000
Subject: [PATCH 07/12] Move both subscription summarizer and subscription
 summary to order management engine

---
 .rubocop_manual_todo.yml                      |   1 -
 .rubocop_todo.yml                             |  12 +-
 app/jobs/subscription_confirm_job.rb          |   4 +-
 app/jobs/subscription_placement_job.rb        |   4 +-
 .../subscriptions/subscription_summarizer.rb  |  55 ++++++++
 .../subscriptions/subscription_summary.rb     |  53 +++++++
 .../subscription_summarizer_spec.rb           | 132 ++++++++++++++++++
 .../subscription_summary_spec.rb              | 127 +++++++++++++++++
 .../subscription_summarizer.rb                |  53 -------
 lib/open_food_network/subscription_summary.rb |  49 -------
 .../subscription_summarizer_spec.rb           | 126 -----------------
 .../subscription_summary_spec.rb              | 125 -----------------
 12 files changed, 377 insertions(+), 364 deletions(-)
 create mode 100644 engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb
 create mode 100644 engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb
 create mode 100644 engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb
 create mode 100644 engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb
 delete mode 100644 lib/open_food_network/subscription_summarizer.rb
 delete mode 100644 lib/open_food_network/subscription_summary.rb
 delete mode 100644 spec/lib/open_food_network/subscription_summarizer_spec.rb
 delete mode 100644 spec/lib/open_food_network/subscription_summary_spec.rb

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index f61d3a9148..9b0fd3b537 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -241,7 +241,6 @@ Layout/LineLength:
   - spec/lib/open_food_network/products_and_inventory_report_spec.rb
   - spec/lib/open_food_network/proxy_order_syncer_spec.rb
   - spec/lib/open_food_network/scope_variant_to_hub_spec.rb
-  - spec/lib/open_food_network/subscription_summarizer_spec.rb
   - spec/lib/open_food_network/tag_rule_applicator_spec.rb
   - spec/lib/open_food_network/users_and_enterprises_report_spec.rb
   - spec/lib/open_food_network/xero_invoices_report_spec.rb
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 33970eb805..323426fda9 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -70,8 +70,8 @@ Lint/DuplicateHashKey:
 # Offense count: 4
 Lint/DuplicateMethods:
   Exclude:
+    - 'engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb'
     - 'lib/discourse/single_sign_on.rb'
-    - 'lib/open_food_network/subscription_summary.rb'
 
 # Offense count: 10
 Lint/IneffectiveAccessModifier:
@@ -882,6 +882,8 @@ Style/FrozenStringLiteralComment:
     - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/report_service.rb'
     - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb'
     - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/summarizer.rb'
+    - 'engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb'
+    - 'engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb'
     - 'engines/order_management/app/services/reports.rb'
     - 'engines/order_management/app/services/reports/authorizer.rb'
     - 'engines/order_management/app/services/reports/parameters/base.rb'
@@ -900,6 +902,8 @@ Style/FrozenStringLiteralComment:
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb'
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_data/enterprise_fee_type_total_spec.rb'
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb'
+    - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summizer_spec.rb'
+    - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb'
     - 'engines/order_management/spec/spec_helper.rb'
     - 'engines/web/app/controllers/web/angular_templates_controller.rb'
     - 'engines/web/app/controllers/web/api/cookies_consent_controller.rb'
@@ -967,8 +971,6 @@ Style/FrozenStringLiteralComment:
     - 'lib/open_food_network/scope_variants_for_search.rb'
     - 'lib/open_food_network/spree_api_key_loader.rb'
     - 'lib/open_food_network/subscription_payment_updater.rb'
-    - 'lib/open_food_network/subscription_summarizer.rb'
-    - 'lib/open_food_network/subscription_summary.rb'
     - 'lib/open_food_network/tag_rule_applicator.rb'
     - 'lib/open_food_network/user_balance_calculator.rb'
     - 'lib/open_food_network/users_and_enterprises_report.rb'
@@ -1219,8 +1221,6 @@ Style/FrozenStringLiteralComment:
     - 'spec/lib/open_food_network/scope_variant_to_hub_spec.rb'
     - 'spec/lib/open_food_network/scope_variants_to_search_spec.rb'
     - 'spec/lib/open_food_network/subscription_payment_updater_spec.rb'
-    - 'spec/lib/open_food_network/subscription_summarizer_spec.rb'
-    - 'spec/lib/open_food_network/subscription_summary_spec.rb'
     - 'spec/lib/open_food_network/tag_rule_applicator_spec.rb'
     - 'spec/lib/open_food_network/user_balance_calculator_spec.rb'
     - 'spec/lib/open_food_network/users_and_enterprises_report_spec.rb'
@@ -1542,6 +1542,7 @@ Style/Send:
   Exclude:
     - 'app/controllers/spree/checkout_controller.rb'
     - 'app/models/spree/shipping_method_decorator.rb'
+    - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb'
     - 'spec/controllers/admin/subscriptions_controller_spec.rb'
     - 'spec/controllers/checkout_controller_spec.rb'
     - 'spec/controllers/spree/admin/base_controller_spec.rb'
@@ -1559,7 +1560,6 @@ Style/Send:
     - 'spec/lib/open_food_network/products_and_inventory_report_spec.rb'
     - 'spec/lib/open_food_network/sales_tax_report_spec.rb'
     - 'spec/lib/open_food_network/subscription_payment_updater_spec.rb'
-    - 'spec/lib/open_food_network/subscription_summarizer_spec.rb'
     - 'spec/lib/open_food_network/tag_rule_applicator_spec.rb'
     - 'spec/lib/open_food_network/xero_invoices_report_spec.rb'
     - 'spec/lib/stripe/webhook_handler_spec.rb'
diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb
index 2d40b02539..9b1ae7a79f 100644
--- a/app/jobs/subscription_confirm_job.rb
+++ b/app/jobs/subscription_confirm_job.rb
@@ -1,4 +1,4 @@
-require 'open_food_network/subscription_summarizer'
+require 'order_management/subscriptions/subscription_summarizer'
 
 # Confirms orders of unconfirmed proxy orders in recently closed Order Cycles
 class SubscriptionConfirmJob
@@ -12,7 +12,7 @@ class SubscriptionConfirmJob
   delegate :record_and_log_error, :send_confirmation_summary_emails, to: :summarizer
 
   def summarizer
-    @summarizer ||= OpenFoodNetwork::SubscriptionSummarizer.new
+    @summarizer ||= OrderManagement::Subscriptions::SubscriptionSummarizer.new
   end
 
   def confirm_proxy_orders!
diff --git a/app/jobs/subscription_placement_job.rb b/app/jobs/subscription_placement_job.rb
index ecaa9208ef..8a77367217 100644
--- a/app/jobs/subscription_placement_job.rb
+++ b/app/jobs/subscription_placement_job.rb
@@ -1,4 +1,4 @@
-require 'open_food_network/subscription_summarizer'
+require 'order_management/subscriptions/subscription_summarizer'
 
 class SubscriptionPlacementJob
   def perform
@@ -17,7 +17,7 @@ class SubscriptionPlacementJob
   delegate :record_and_log_error, :send_placement_summary_emails, to: :summarizer
 
   def summarizer
-    @summarizer ||= OpenFoodNetwork::SubscriptionSummarizer.new
+    @summarizer ||= OrderManagement::Subscriptions::SubscriptionSummarizer.new
   end
 
   def proxy_orders
diff --git a/engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb b/engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb
new file mode 100644
index 0000000000..dbffcb96ab
--- /dev/null
+++ b/engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+# Used by for SubscriptionPlacementJob and SubscriptionConfirmJob to summarize the
+# result of automatic processing of subscriptions for the relevant shop owners.
+module OrderManagement
+  module Subscriptions
+    class SubscriptionSummarizer
+      def initialize
+        @summaries = {}
+      end
+
+      def record_order(order)
+        summary_for(order).record_order(order)
+      end
+
+      def record_success(order)
+        summary_for(order).record_success(order)
+      end
+
+      def record_issue(type, order, message = nil)
+        Rails.logger.info "Issue in Subscription Order #{order.id}: #{type}"
+        summary_for(order).record_issue(type, order, message)
+      end
+
+      def record_and_log_error(type, order)
+        return record_issue(type, order) unless order.errors.any?
+
+        error = "Subscription#{type.to_s.camelize}Error"
+        line1 = "#{error}: Cannot process order #{order.number} due to errors"
+        line2 = "Errors: #{order.errors.full_messages.join(', ')}"
+        Rails.logger.info("#{line1}\n#{line2}")
+        record_issue(type, order, line2)
+      end
+
+      def send_placement_summary_emails
+        @summaries.values.each do |summary|
+          SubscriptionMailer.placement_summary_email(summary).deliver
+        end
+      end
+
+      def send_confirmation_summary_emails
+        @summaries.values.each do |summary|
+          SubscriptionMailer.confirmation_summary_email(summary).deliver
+        end
+      end
+
+      private
+
+      def summary_for(order)
+        shop_id = order.distributor_id
+        @summaries[shop_id] ||= SubscriptionSummary.new(shop_id)
+      end
+    end
+  end
+end
diff --git a/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb b/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb
new file mode 100644
index 0000000000..aa9a61c469
--- /dev/null
+++ b/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module OrderManagement
+  module Subscriptions
+    class SubscriptionSummary
+      attr_reader :shop_id, :order_count, :success_count, :issues
+
+      def initialize(shop_id)
+        @shop_id = shop_id
+        @order_ids = []
+        @success_ids = []
+        @issues = {}
+      end
+
+      def record_order(order)
+        @order_ids << order.id
+      end
+
+      def record_success(order)
+        @success_ids << order.id
+      end
+
+      def record_issue(type, order, message)
+        issues[type] ||= {}
+        issues[type][order.id] = message
+      end
+
+      def order_count
+        @order_ids.count
+      end
+
+      def success_count
+        @success_ids.count
+      end
+
+      def issue_count
+        (@order_ids - @success_ids).count
+      end
+
+      def orders_affected_by(type)
+        case type
+        when :other then Spree::Order.where(id: unrecorded_ids)
+        else Spree::Order.where(id: issues[type].keys)
+        end
+      end
+
+      def unrecorded_ids
+        recorded_ids = issues.values.map(&:keys).flatten
+        @order_ids - @success_ids - recorded_ids
+      end
+    end
+  end
+end
diff --git a/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb
new file mode 100644
index 0000000000..26a79af4d1
--- /dev/null
+++ b/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb
@@ -0,0 +1,132 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+module OrderManagement
+  module Subscriptions
+    describe SubscriptionSummarizer do
+      let(:order) { create(:order) }
+      let(:summarizer) { OrderManagement::Subscriptions::SubscriptionSummarizer.new }
+
+      before { allow(Rails.logger).to receive(:info) }
+
+      describe "#summary_for" do
+        let(:order) { double(:order, distributor_id: 123) }
+
+        context "when a summary for the order's distributor doesn't already exist" do
+          it "initializes a new summary object, and returns it" do
+            expect(summarizer.instance_variable_get(:@summaries).count).to be 0
+            summary = summarizer.send(:summary_for, order)
+            expect(summary.shop_id).to be 123
+            expect(summarizer.instance_variable_get(:@summaries).count).to be 1
+          end
+        end
+
+        context "when a summary for the order's distributor already exists" do
+          let(:summary) { double(:summary) }
+
+          before do
+            summarizer.instance_variable_set(:@summaries, 123 => summary)
+          end
+
+          it "returns the existing summary object" do
+            expect(summarizer.instance_variable_get(:@summaries).count).to be 1
+            expect(summarizer.send(:summary_for, order)).to eq summary
+            expect(summarizer.instance_variable_get(:@summaries).count).to be 1
+          end
+        end
+      end
+
+      describe "recording events" do
+        let(:order) { double(:order) }
+        let(:summary) { double(:summary) }
+        before { allow(summarizer).to receive(:summary_for).with(order) { summary } }
+
+        describe "#record_order" do
+          it "requests a summary for the order and calls #record_order on it" do
+            expect(summary).to receive(:record_order).with(order).once
+            summarizer.record_order(order)
+          end
+        end
+
+        describe "#record_success" do
+          it "requests a summary for the order and calls #record_success on it" do
+            expect(summary).to receive(:record_success).with(order).once
+            summarizer.record_success(order)
+          end
+        end
+
+        describe "#record_issue" do
+          it "requests a summary for the order and calls #record_issue on it" do
+            expect(order).to receive(:id)
+            expect(summary).to receive(:record_issue).with(:type, order, "message").once
+            summarizer.record_issue(:type, order, "message")
+          end
+        end
+
+        describe "#record_and_log_error" do
+          before do
+            allow(order).to receive(:number) { "123" }
+          end
+
+          context "when errors exist on the order" do
+            before do
+              allow(order).to receive(:errors) {
+                double(:errors, any?: true, full_messages: ["Some error"])
+              }
+            end
+
+            it "sends error info to rails logger and calls #record_issue with an error message" do
+              expect(summarizer).to receive(:record_issue).with(:processing,
+                                                                order, "Errors: Some error")
+              summarizer.record_and_log_error(:processing, order)
+            end
+          end
+
+          context "when no errors exist on the order" do
+            before do
+              allow(order).to receive(:errors) { double(:errors, any?: false) }
+            end
+
+            it "falls back to calling record_issue" do
+              expect(summarizer).to receive(:record_issue).with(:processing, order)
+              summarizer.record_and_log_error(:processing, order)
+            end
+          end
+        end
+      end
+
+      describe "#send_placement_summary_emails" do
+        let(:summary1) { double(:summary) }
+        let(:summary2) { double(:summary) }
+        let(:summaries) { { 1 => summary1, 2 => summary2 } }
+        let(:mail_mock) { double(:mail, deliver: true) }
+
+        before do
+          summarizer.instance_variable_set(:@summaries, summaries)
+        end
+
+        it "sends a placement summary email for each summary" do
+          expect(SubscriptionMailer).to receive(:placement_summary_email).twice { mail_mock }
+          summarizer.send_placement_summary_emails
+        end
+      end
+
+      describe "#send_confirmation_summary_emails" do
+        let(:summary1) { double(:summary) }
+        let(:summary2) { double(:summary) }
+        let(:summaries) { { 1 => summary1, 2 => summary2 } }
+        let(:mail_mock) { double(:mail, deliver: true) }
+
+        before do
+          summarizer.instance_variable_set(:@summaries, summaries)
+        end
+
+        it "sends a placement summary email for each summary" do
+          expect(SubscriptionMailer).to receive(:confirmation_summary_email).twice { mail_mock }
+          summarizer.send_confirmation_summary_emails
+        end
+      end
+    end
+  end
+end
diff --git a/engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb
new file mode 100644
index 0000000000..e24aa6449b
--- /dev/null
+++ b/engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb
@@ -0,0 +1,127 @@
+# frozen_string_literal: true
+
+module OrderManagement
+  module Subscriptions
+    describe SubscriptionSummary do
+      let(:summary) { OrderManagement::Subscriptions::SubscriptionSummary.new(123) }
+
+      describe "#initialize" do
+        it "initializes instance variables: shop_id, order_count, success_count and issues" do
+          expect(summary.shop_id).to be 123
+          expect(summary.order_count).to be 0
+          expect(summary.success_count).to be 0
+          expect(summary.issues).to be_a Hash
+        end
+      end
+
+      describe "#record_order" do
+        let(:order) { double(:order, id: 37) }
+        it "adds the order id to the order_ids array" do
+          summary.record_order(order)
+          expect(summary.instance_variable_get(:@order_ids)).to eq [order.id]
+        end
+      end
+
+      describe "#record_success" do
+        let(:order) { double(:order, id: 37) }
+        it "adds the order id to the success_ids array" do
+          summary.record_success(order)
+          expect(summary.instance_variable_get(:@success_ids)).to eq [order.id]
+        end
+      end
+
+      describe "#record_issue" do
+        let(:order) { double(:order, id: 1) }
+
+        context "when no issues of the same type have been recorded yet" do
+          it "adds a new type to the issues hash, and stores a new issue against it" do
+            summary.record_issue(:some_type, order, "message")
+            expect(summary.issues.keys).to include :some_type
+            expect(summary.issues[:some_type][order.id]).to eq "message"
+          end
+        end
+
+        context "when an issue of the same type has already been recorded" do
+          let(:existing_issue) { double(:existing_issue) }
+
+          before { summary.issues[:some_type] = [existing_issue] }
+
+          it "stores a new issue against the existing type" do
+            summary.record_issue(:some_type, order, "message")
+            expect(summary.issues[:some_type]).to include existing_issue
+            expect(summary.issues[:some_type][order.id]).to eq "message"
+          end
+        end
+      end
+
+      describe "#order_count" do
+        let(:order_ids) { [1, 2, 3, 4, 5, 6, 7] }
+        it "counts the number of items in the order_ids instance_variable" do
+          summary.instance_variable_set(:@order_ids, order_ids)
+          expect(summary.order_count).to be 7
+        end
+      end
+
+      describe "#success_count" do
+        let(:success_ids) { [1, 2, 3, 4, 5, 6, 7] }
+        it "counts the number of items in the success_ids instance_variable" do
+          summary.instance_variable_set(:@success_ids, success_ids)
+          expect(summary.success_count).to be 7
+        end
+      end
+
+      describe "#issue_count" do
+        let(:order_ids) { [1, 3, 5, 7, 9] }
+        let(:success_ids) { [1, 2, 3, 4, 5] }
+
+        it "counts the number of items in order_ids that are not in success_ids" do
+          summary.instance_variable_set(:@order_ids, order_ids)
+          summary.instance_variable_set(:@success_ids, success_ids)
+          expect(summary.issue_count).to be 2 # 7 & 9
+        end
+      end
+
+      describe "#orders_affected_by" do
+        let(:order1) { create(:order) }
+        let(:order2) { create(:order) }
+
+        before do
+          allow(summary).to receive(:unrecorded_ids) { [order1.id] }
+          allow(summary).to receive(:issues) { { failure: { order2.id => "A message" } } }
+        end
+
+        context "when the issue type is :other" do
+          let(:orders) { summary.orders_affected_by(:other) }
+
+          it "returns orders specified by unrecorded_ids" do
+            expect(orders).to include order1
+            expect(orders).to_not include order2
+          end
+        end
+
+        context "when the issue type is :other" do
+          let(:orders) { summary.orders_affected_by(:failure) }
+
+          it "returns orders specified by the relevant issue hash" do
+            expect(orders).to include order2
+            expect(orders).to_not include order1
+          end
+        end
+      end
+
+      describe "#unrecorded_ids" do
+        let(:issues) { { type: { 7 => "message", 8 => "message" } } }
+
+        before do
+          summary.instance_variable_set(:@order_ids, [1, 3, 5, 7, 9])
+          summary.instance_variable_set(:@success_ids, [1, 2, 3, 4, 5])
+          summary.instance_variable_set(:@issues, issues)
+        end
+
+        it "returns order_ids that are not marked as an issue or a success" do
+          expect(summary.unrecorded_ids).to eq [9]
+        end
+      end
+    end
+  end
+end
diff --git a/lib/open_food_network/subscription_summarizer.rb b/lib/open_food_network/subscription_summarizer.rb
deleted file mode 100644
index 6e4b95da8b..0000000000
--- a/lib/open_food_network/subscription_summarizer.rb
+++ /dev/null
@@ -1,53 +0,0 @@
-require 'open_food_network/subscription_summary'
-
-# Used by for SubscriptionPlacementJob and SubscriptionConfirmJob to summarize the
-# result of automatic processing of subscriptions for the relevant shop owners.
-module OpenFoodNetwork
-  class SubscriptionSummarizer
-    def initialize
-      @summaries = {}
-    end
-
-    def record_order(order)
-      summary_for(order).record_order(order)
-    end
-
-    def record_success(order)
-      summary_for(order).record_success(order)
-    end
-
-    def record_issue(type, order, message = nil)
-      Rails.logger.info "Issue in Subscription Order #{order.id}: #{type}"
-      summary_for(order).record_issue(type, order, message)
-    end
-
-    def record_and_log_error(type, order)
-      return record_issue(type, order) unless order.errors.any?
-
-      error = "Subscription#{type.to_s.camelize}Error"
-      line1 = "#{error}: Cannot process order #{order.number} due to errors"
-      line2 = "Errors: #{order.errors.full_messages.join(', ')}"
-      Rails.logger.info("#{line1}\n#{line2}")
-      record_issue(type, order, line2)
-    end
-
-    def send_placement_summary_emails
-      @summaries.values.each do |summary|
-        SubscriptionMailer.placement_summary_email(summary).deliver
-      end
-    end
-
-    def send_confirmation_summary_emails
-      @summaries.values.each do |summary|
-        SubscriptionMailer.confirmation_summary_email(summary).deliver
-      end
-    end
-
-    private
-
-    def summary_for(order)
-      shop_id = order.distributor_id
-      @summaries[shop_id] ||= SubscriptionSummary.new(shop_id)
-    end
-  end
-end
diff --git a/lib/open_food_network/subscription_summary.rb b/lib/open_food_network/subscription_summary.rb
deleted file mode 100644
index fb2a1dc066..0000000000
--- a/lib/open_food_network/subscription_summary.rb
+++ /dev/null
@@ -1,49 +0,0 @@
-module OpenFoodNetwork
-  class SubscriptionSummary
-    attr_reader :shop_id, :order_count, :success_count, :issues
-
-    def initialize(shop_id)
-      @shop_id = shop_id
-      @order_ids = []
-      @success_ids = []
-      @issues = {}
-    end
-
-    def record_order(order)
-      @order_ids << order.id
-    end
-
-    def record_success(order)
-      @success_ids << order.id
-    end
-
-    def record_issue(type, order, message)
-      issues[type] ||= {}
-      issues[type][order.id] = message
-    end
-
-    def order_count
-      @order_ids.count
-    end
-
-    def success_count
-      @success_ids.count
-    end
-
-    def issue_count
-      (@order_ids - @success_ids).count
-    end
-
-    def orders_affected_by(type)
-      case type
-      when :other then Spree::Order.where(id: unrecorded_ids)
-      else Spree::Order.where(id: issues[type].keys)
-      end
-    end
-
-    def unrecorded_ids
-      recorded_ids = issues.values.map(&:keys).flatten
-      @order_ids - @success_ids - recorded_ids
-    end
-  end
-end
diff --git a/spec/lib/open_food_network/subscription_summarizer_spec.rb b/spec/lib/open_food_network/subscription_summarizer_spec.rb
deleted file mode 100644
index a0d2b3f7bf..0000000000
--- a/spec/lib/open_food_network/subscription_summarizer_spec.rb
+++ /dev/null
@@ -1,126 +0,0 @@
-require 'spec_helper'
-require 'open_food_network/subscription_summarizer'
-
-module OpenFoodNetwork
-  describe SubscriptionSummarizer do
-    let(:order) { create(:order) }
-    let(:summarizer) { OpenFoodNetwork::SubscriptionSummarizer.new }
-
-    before { allow(Rails.logger).to receive(:info) }
-
-    describe "#summary_for" do
-      let(:order) { double(:order, distributor_id: 123) }
-
-      context "when a summary for the order's distributor doesn't already exist" do
-        it "initializes a new summary object, and returns it" do
-          expect(summarizer.instance_variable_get(:@summaries).count).to be 0
-          summary = summarizer.send(:summary_for, order)
-          expect(summary.shop_id).to be 123
-          expect(summarizer.instance_variable_get(:@summaries).count).to be 1
-        end
-      end
-
-      context "when a summary for the order's distributor already exists" do
-        let(:summary) { double(:summary) }
-
-        before do
-          summarizer.instance_variable_set(:@summaries, 123 => summary)
-        end
-
-        it "returns the existing summary object" do
-          expect(summarizer.instance_variable_get(:@summaries).count).to be 1
-          expect(summarizer.send(:summary_for, order)).to eq summary
-          expect(summarizer.instance_variable_get(:@summaries).count).to be 1
-        end
-      end
-    end
-
-    describe "recording events" do
-      let(:order) { double(:order) }
-      let(:summary) { double(:summary) }
-      before { allow(summarizer).to receive(:summary_for).with(order) { summary } }
-
-      describe "#record_order" do
-        it "requests a summary for the order and calls #record_order on it" do
-          expect(summary).to receive(:record_order).with(order).once
-          summarizer.record_order(order)
-        end
-      end
-
-      describe "#record_success" do
-        it "requests a summary for the order and calls #record_success on it" do
-          expect(summary).to receive(:record_success).with(order).once
-          summarizer.record_success(order)
-        end
-      end
-
-      describe "#record_issue" do
-        it "requests a summary for the order and calls #record_issue on it" do
-          expect(order).to receive(:id)
-          expect(summary).to receive(:record_issue).with(:type, order, "message").once
-          summarizer.record_issue(:type, order, "message")
-        end
-      end
-
-      describe "#record_and_log_error" do
-        before do
-          allow(order).to receive(:number) { "123" }
-        end
-
-        context "when errors exist on the order" do
-          before do
-            allow(order).to receive(:errors) { double(:errors, any?: true, full_messages: ["Some error"]) }
-          end
-
-          it "sends error info to the rails logger and calls #record_issue on itself with an error message" do
-            expect(summarizer).to receive(:record_issue).with(:processing, order, "Errors: Some error")
-            summarizer.record_and_log_error(:processing, order)
-          end
-        end
-
-        context "when no errors exist on the order" do
-          before do
-            allow(order).to receive(:errors) { double(:errors, any?: false) }
-          end
-
-          it "falls back to calling record_issue" do
-            expect(summarizer).to receive(:record_issue).with(:processing, order)
-            summarizer.record_and_log_error(:processing, order)
-          end
-        end
-      end
-    end
-
-    describe "#send_placement_summary_emails" do
-      let(:summary1) { double(:summary) }
-      let(:summary2) { double(:summary) }
-      let(:summaries) { { 1 => summary1, 2 => summary2 } }
-      let(:mail_mock) { double(:mail, deliver: true) }
-
-      before do
-        summarizer.instance_variable_set(:@summaries, summaries)
-      end
-
-      it "sends a placement summary email for each summary" do
-        expect(SubscriptionMailer).to receive(:placement_summary_email).twice { mail_mock }
-        summarizer.send_placement_summary_emails
-      end
-    end
-
-    describe "#send_confirmation_summary_emails" do
-      let(:summary1) { double(:summary) }
-      let(:summary2) { double(:summary) }
-      let(:summaries) { { 1 => summary1, 2 => summary2 } }
-      let(:mail_mock) { double(:mail, deliver: true) }
-
-      before do
-        summarizer.instance_variable_set(:@summaries, summaries)
-      end
-
-      it "sends a placement summary email for each summary" do
-        expect(SubscriptionMailer).to receive(:confirmation_summary_email).twice { mail_mock }
-        summarizer.send_confirmation_summary_emails
-      end
-    end
-  end
-end
diff --git a/spec/lib/open_food_network/subscription_summary_spec.rb b/spec/lib/open_food_network/subscription_summary_spec.rb
deleted file mode 100644
index f5b0cd8d71..0000000000
--- a/spec/lib/open_food_network/subscription_summary_spec.rb
+++ /dev/null
@@ -1,125 +0,0 @@
-require 'open_food_network/subscription_summary'
-
-module OpenFoodNetwork
-  describe SubscriptionSummary do
-    let(:summary) { OpenFoodNetwork::SubscriptionSummary.new(123) }
-
-    describe "#initialize" do
-      it "initializes instance variables: shop_id, order_count, success_count and issues" do
-        expect(summary.shop_id).to be 123
-        expect(summary.order_count).to be 0
-        expect(summary.success_count).to be 0
-        expect(summary.issues).to be_a Hash
-      end
-    end
-
-    describe "#record_order" do
-      let(:order) { double(:order, id: 37) }
-      it "adds the order id to the order_ids array" do
-        summary.record_order(order)
-        expect(summary.instance_variable_get(:@order_ids)).to eq [order.id]
-      end
-    end
-
-    describe "#record_success" do
-      let(:order) { double(:order, id: 37) }
-      it "adds the order id to the success_ids array" do
-        summary.record_success(order)
-        expect(summary.instance_variable_get(:@success_ids)).to eq [order.id]
-      end
-    end
-
-    describe "#record_issue" do
-      let(:order) { double(:order, id: 1) }
-
-      context "when no issues of the same type have been recorded yet" do
-        it "adds a new type to the issues hash, and stores a new issue against it" do
-          summary.record_issue(:some_type, order, "message")
-          expect(summary.issues.keys).to include :some_type
-          expect(summary.issues[:some_type][order.id]).to eq "message"
-        end
-      end
-
-      context "when an issue of the same type has already been recorded" do
-        let(:existing_issue) { double(:existing_issue) }
-
-        before { summary.issues[:some_type] = [existing_issue] }
-
-        it "stores a new issue against the existing type" do
-          summary.record_issue(:some_type, order, "message")
-          expect(summary.issues[:some_type]).to include existing_issue
-          expect(summary.issues[:some_type][order.id]).to eq "message"
-        end
-      end
-    end
-
-    describe "#order_count" do
-      let(:order_ids) { [1, 2, 3, 4, 5, 6, 7] }
-      it "counts the number of items in the order_ids instance_variable" do
-        summary.instance_variable_set(:@order_ids, order_ids)
-        expect(summary.order_count).to be 7
-      end
-    end
-
-    describe "#success_count" do
-      let(:success_ids) { [1, 2, 3, 4, 5, 6, 7] }
-      it "counts the number of items in the success_ids instance_variable" do
-        summary.instance_variable_set(:@success_ids, success_ids)
-        expect(summary.success_count).to be 7
-      end
-    end
-
-    describe "#issue_count" do
-      let(:order_ids) { [1, 3, 5, 7, 9] }
-      let(:success_ids) { [1, 2, 3, 4, 5] }
-
-      it "counts the number of items in order_ids that are not in success_ids" do
-        summary.instance_variable_set(:@order_ids, order_ids)
-        summary.instance_variable_set(:@success_ids, success_ids)
-        expect(summary.issue_count).to be 2 # 7 & 9
-      end
-    end
-
-    describe "#orders_affected_by" do
-      let(:order1) { create(:order) }
-      let(:order2) { create(:order) }
-
-      before do
-        allow(summary).to receive(:unrecorded_ids) { [order1.id] }
-        allow(summary).to receive(:issues) { { failure: { order2.id => "A message" } } }
-      end
-
-      context "when the issue type is :other" do
-        let(:orders) { summary.orders_affected_by(:other) }
-
-        it "returns orders specified by unrecorded_ids" do
-          expect(orders).to include order1
-          expect(orders).to_not include order2
-        end
-      end
-
-      context "when the issue type is :other" do
-        let(:orders) { summary.orders_affected_by(:failure) }
-
-        it "returns orders specified by the relevant issue hash" do
-          expect(orders).to include order2
-          expect(orders).to_not include order1
-        end
-      end
-    end
-
-    describe "#unrecorded_ids" do
-      let(:issues) { { type: { 7 => "message", 8 => "message" } } }
-
-      before do
-        summary.instance_variable_set(:@order_ids, [1, 3, 5, 7, 9])
-        summary.instance_variable_set(:@success_ids, [1, 2, 3, 4, 5])
-        summary.instance_variable_set(:@issues, issues)
-      end
-
-      it "returns order_ids that are not marked as an issue or a success" do
-        expect(summary.unrecorded_ids).to eq [9]
-      end
-    end
-  end
-end

From ae0ceb61a169e531fe9b7f8d13e7c4c1f7273acb Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 18:22:36 +0000
Subject: [PATCH 08/12] Move ProxyOrderSyncer to OrderManagement engine

---
 .rubocop_manual_todo.yml                      |   4 +-
 .rubocop_todo.yml                             |   6 +-
 app/controllers/admin/schedules_controller.rb |   4 +-
 app/services/order_cycle_form.rb              |   4 +-
 app/services/subscription_form.rb             |   4 +-
 .../subscriptions/proxy_order_syncer.rb       |  99 +++++
 .../subscriptions/proxy_order_syncer_spec.rb  |  69 +++
 .../subscriptions/proxy_order_syncer_spec.rb  | 402 ++++++++++++++++++
 lib/open_food_network/proxy_order_syncer.rb   |  95 -----
 .../admin/schedules_controller_spec.rb        |   4 +-
 .../proxy_order_syncer_spec.rb                | 400 -----------------
 spec/performance/proxy_order_syncer_spec.rb   |  67 ---
 spec/services/order_cycle_form_spec.rb        |   6 +-
 13 files changed, 587 insertions(+), 577 deletions(-)
 create mode 100644 engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb
 create mode 100644 engines/order_management/spec/performance/order_management/subscriptions/proxy_order_syncer_spec.rb
 create mode 100644 engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
 delete mode 100644 lib/open_food_network/proxy_order_syncer.rb
 delete mode 100644 spec/lib/open_food_network/proxy_order_syncer_spec.rb
 delete mode 100644 spec/performance/proxy_order_syncer_spec.rb

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index 9b0fd3b537..9494595048 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -101,6 +101,7 @@ Layout/LineLength:
   - app/services/subscriptions_count.rb
   - app/services/variants_stock_levels.rb
   - engines/web/app/helpers/web/cookies_policy_helper.rb
+  - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
   - lib/discourse/single_sign_on.rb
   - lib/open_food_network/available_payment_method_filter.rb
   - lib/open_food_network/bulk_coop_report.rb
@@ -239,7 +240,6 @@ Layout/LineLength:
   - spec/lib/open_food_network/packing_report_spec.rb
   - spec/lib/open_food_network/permissions_spec.rb
   - spec/lib/open_food_network/products_and_inventory_report_spec.rb
-  - spec/lib/open_food_network/proxy_order_syncer_spec.rb
   - spec/lib/open_food_network/scope_variant_to_hub_spec.rb
   - spec/lib/open_food_network/tag_rule_applicator_spec.rb
   - spec/lib/open_food_network/users_and_enterprises_report_spec.rb
@@ -684,6 +684,7 @@ Metrics/ModuleLength:
   - app/helpers/injection_helper.rb
   - app/helpers/spree/admin/navigation_helper.rb
   - app/helpers/spree/admin/base_helper.rb
+  - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
   - lib/open_food_network/column_preference_defaults.rb
   - spec/controllers/admin/enterprises_controller_spec.rb
@@ -700,7 +701,6 @@ Metrics/ModuleLength:
   - spec/lib/open_food_network/order_grouper_spec.rb
   - spec/lib/open_food_network/permissions_spec.rb
   - spec/lib/open_food_network/products_and_inventory_report_spec.rb
-  - spec/lib/open_food_network/proxy_order_syncer_spec.rb
   - spec/lib/open_food_network/scope_variant_to_hub_spec.rb
   - spec/lib/open_food_network/tag_rule_applicator_spec.rb
   - spec/lib/open_food_network/users_and_enterprises_report_spec.rb
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 323426fda9..1c003c0504 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -882,6 +882,7 @@ Style/FrozenStringLiteralComment:
     - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/report_service.rb'
     - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb'
     - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/summarizer.rb'
+    - 'engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb'
     - 'engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb'
     - 'engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb'
     - 'engines/order_management/app/services/reports.rb'
@@ -895,6 +896,7 @@ Style/FrozenStringLiteralComment:
     - 'engines/order_management/lib/order_management/engine.rb'
     - 'engines/order_management/lib/order_management/version.rb'
     - 'engines/order_management/order_management.gemspec'
+    - 'engines/order_management/spec/performance/order_management/subscriptions/proxy_order_syncer_spec.rb'
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/authorizer_spec.rb'
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/parameters_spec.rb'
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/permissions_spec.rb'
@@ -902,6 +904,7 @@ Style/FrozenStringLiteralComment:
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb'
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_data/enterprise_fee_type_total_spec.rb'
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb'
+    - 'engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb'
     - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summizer_spec.rb'
     - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb'
     - 'engines/order_management/spec/spec_helper.rb'
@@ -954,7 +957,6 @@ Style/FrozenStringLiteralComment:
     - 'lib/open_food_network/products_and_inventory_report.rb'
     - 'lib/open_food_network/products_and_inventory_report_base.rb'
     - 'lib/open_food_network/property_merge.rb'
-    - 'lib/open_food_network/proxy_order_syncer.rb'
     - 'lib/open_food_network/rack_request_blocker.rb'
     - 'lib/open_food_network/referer_parser.rb'
     - 'lib/open_food_network/reports/bulk_coop_allocation_report.rb'
@@ -1212,7 +1214,6 @@ Style/FrozenStringLiteralComment:
     - 'spec/lib/open_food_network/permissions_spec.rb'
     - 'spec/lib/open_food_network/products_and_inventory_report_spec.rb'
     - 'spec/lib/open_food_network/property_merge_spec.rb'
-    - 'spec/lib/open_food_network/proxy_order_syncer_spec.rb'
     - 'spec/lib/open_food_network/referer_parser_spec.rb'
     - 'spec/lib/open_food_network/reports/report_spec.rb'
     - 'spec/lib/open_food_network/reports/row_spec.rb'
@@ -1306,7 +1307,6 @@ Style/FrozenStringLiteralComment:
     - 'spec/models/variant_override_spec.rb'
     - 'spec/performance/injection_helper_spec.rb'
     - 'spec/performance/orders_controller_spec.rb'
-    - 'spec/performance/proxy_order_syncer_spec.rb'
     - 'spec/performance/shop_controller_spec.rb'
     - 'spec/requests/checkout/failed_checkout_spec.rb'
     - 'spec/requests/checkout/paypal_spec.rb'
diff --git a/app/controllers/admin/schedules_controller.rb b/app/controllers/admin/schedules_controller.rb
index 580a6a9c72..c568bf0ad7 100644
--- a/app/controllers/admin/schedules_controller.rb
+++ b/app/controllers/admin/schedules_controller.rb
@@ -1,5 +1,5 @@
 require 'open_food_network/permissions'
-require 'open_food_network/proxy_order_syncer'
+require 'order_management/subscriptions/proxy_order_syncer'
 
 module Admin
   class SchedulesController < ResourceController
@@ -81,7 +81,7 @@ module Admin
       return unless removed_ids.any? || new_ids.any?
 
       subscriptions = Subscription.where(schedule_id: @schedule)
-      syncer = OpenFoodNetwork::ProxyOrderSyncer.new(subscriptions)
+      syncer = OrderManagement::Subscriptions::ProxyOrderSyncer.new(subscriptions)
       syncer.sync!
     end
   end
diff --git a/app/services/order_cycle_form.rb b/app/services/order_cycle_form.rb
index 2ab5d109de..7026d5f5b6 100644
--- a/app/services/order_cycle_form.rb
+++ b/app/services/order_cycle_form.rb
@@ -1,6 +1,6 @@
 require 'open_food_network/permissions'
-require 'open_food_network/proxy_order_syncer'
 require 'open_food_network/order_cycle_form_applicator'
+require 'order_management/subscriptions/proxy_order_syncer'
 
 class OrderCycleForm
   def initialize(order_cycle, params, user)
@@ -58,7 +58,7 @@ class OrderCycleForm
     return unless schedule_ids?
     return unless schedule_sync_required?
 
-    OpenFoodNetwork::ProxyOrderSyncer.new(subscriptions_to_sync).sync!
+    OrderManagement::Subscriptions::ProxyOrderSyncer.new(subscriptions_to_sync).sync!
   end
 
   def schedule_sync_required?
diff --git a/app/services/subscription_form.rb b/app/services/subscription_form.rb
index da82063b5c..fa33b216b2 100644
--- a/app/services/subscription_form.rb
+++ b/app/services/subscription_form.rb
@@ -1,4 +1,4 @@
-require 'open_food_network/proxy_order_syncer'
+require 'order_management/subscriptions/proxy_order_syncer'
 
 class SubscriptionForm
   attr_accessor :subscription, :params, :order_update_issues, :validator, :order_syncer, :estimator
@@ -29,6 +29,6 @@ class SubscriptionForm
   private
 
   def proxy_order_syncer
-    OpenFoodNetwork::ProxyOrderSyncer.new(subscription)
+    OrderManagement::Subscriptions::ProxyOrderSyncer.new(subscription)
   end
 end
diff --git a/engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb b/engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb
new file mode 100644
index 0000000000..b53085f1c1
--- /dev/null
+++ b/engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb
@@ -0,0 +1,99 @@
+# frozen_string_literal: false
+
+module OrderManagement
+  module Subscriptions
+    class ProxyOrderSyncer
+      attr_reader :subscription
+
+      delegate :order_cycles, :proxy_orders, :begins_at, :ends_at, to: :subscription
+
+      def initialize(subscriptions)
+        case subscriptions
+        when Subscription
+          @subscription = subscriptions
+        when ActiveRecord::Relation
+          @subscriptions = subscriptions.not_ended.not_canceled
+        else
+          raise "ProxyOrderSyncer must be initialized with " \
+                  "an instance of Subscription or ActiveRecord::Relation"
+        end
+      end
+
+      def sync!
+        return sync_subscriptions! if @subscriptions
+
+        return initialise_proxy_orders! unless @subscription.id
+
+        sync_subscription!
+      end
+
+      private
+
+      def sync_subscriptions!
+        @subscriptions.each do |subscription|
+          @subscription = subscription
+          sync_subscription!
+        end
+      end
+
+      def initialise_proxy_orders!
+        uninitialised_order_cycle_ids.each do |order_cycle_id|
+          Rails.logger.info "Initializing Proxy Order " \
+            "of subscription #{@subscription.id} in order cycle #{order_cycle_id}"
+          proxy_orders << ProxyOrder.new(subscription: subscription, order_cycle_id: order_cycle_id)
+        end
+      end
+
+      def sync_subscription!
+        Rails.logger.info "Syncing Proxy Orders of subscription #{@subscription.id}"
+        create_proxy_orders!
+        remove_orphaned_proxy_orders!
+      end
+
+      def create_proxy_orders!
+        return unless not_closed_in_range_order_cycles.any?
+
+        query = "INSERT INTO proxy_orders (subscription_id, order_cycle_id, updated_at, created_at)"
+        query << " VALUES #{insert_values}"
+        query << " ON CONFLICT DO NOTHING"
+
+        ActiveRecord::Base.connection.exec_query(query)
+      end
+
+      def uninitialised_order_cycle_ids
+        not_closed_in_range_order_cycles.pluck(:id) - proxy_orders.map(&:order_cycle_id)
+      end
+
+      def remove_orphaned_proxy_orders!
+        orphaned_proxy_orders.scoped.delete_all
+      end
+
+      # Remove Proxy Orders that have not been placed yet
+      #   and are in Order Cycles that are out of range
+      def orphaned_proxy_orders
+        orphaned = proxy_orders.where(placed_at: nil)
+        order_cycle_ids = in_range_order_cycles.pluck(:id)
+        return orphaned unless order_cycle_ids.any?
+
+        orphaned.where('order_cycle_id NOT IN (?)', order_cycle_ids)
+      end
+
+      def insert_values
+        now = Time.now.utc.iso8601
+        not_closed_in_range_order_cycles
+          .map{ |oc| "(#{subscription.id},#{oc.id},'#{now}','#{now}')" }
+          .join(",")
+      end
+
+      def not_closed_in_range_order_cycles
+        in_range_order_cycles.merge(OrderCycle.not_closed)
+      end
+
+      def in_range_order_cycles
+        order_cycles.where("orders_close_at >= ? AND orders_close_at <= ?",
+                           begins_at,
+                           ends_at || 100.years.from_now)
+      end
+    end
+  end
+end
diff --git a/engines/order_management/spec/performance/order_management/subscriptions/proxy_order_syncer_spec.rb b/engines/order_management/spec/performance/order_management/subscriptions/proxy_order_syncer_spec.rb
new file mode 100644
index 0000000000..b62267b714
--- /dev/null
+++ b/engines/order_management/spec/performance/order_management/subscriptions/proxy_order_syncer_spec.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+module OrderManagement
+  module Subscriptions
+    describe ProxyOrderSyncer, performance: true do
+      let(:start) { Time.zone.now.beginning_of_day }
+      let!(:schedule) { create(:schedule, order_cycles: order_cycles) }
+
+      let!(:order_cycles) do
+        Array.new(10) do |i|
+          create(:simple_order_cycle, orders_open_at: start + i.days,
+                                      orders_close_at: start + (i + 1).days )
+        end
+      end
+
+      let!(:subscriptions) do
+        Array.new(150) do |_i|
+          create(:subscription, schedule: schedule, begins_at: start, ends_at: start + 10.days)
+        end
+        Subscription.where(schedule_id: schedule)
+      end
+
+      context "measuring performance for initialisation" do
+        it "reports the average run time for adding 10 OCs to 150 subscriptions" do
+          expect(ProxyOrder.count).to be 0
+          times = []
+          10.times do
+            syncer = ProxyOrderSyncer.new(subscriptions.reload)
+
+            t1 = Time.zone.now
+            syncer.sync!
+            t2 = Time.zone.now
+            diff = t2 - t1
+            times << diff
+            puts diff.round(2)
+
+            expect(ProxyOrder.count).to be 1500
+            ProxyOrder.destroy_all
+          end
+          puts "AVG: #{(times.sum / times.count).round(2)}"
+        end
+      end
+
+      context "measuring performance for removal" do
+        it "reports the average run time for removing 8 OCs from 150 subscriptions" do
+          times = []
+          10.times do
+            syncer = ProxyOrderSyncer.new(subscriptions.reload)
+            syncer.sync!
+            expect(ProxyOrder.count).to be 1500
+            subscriptions.update_all(begins_at: start + 8.days + 1.minute)
+            syncer = ProxyOrderSyncer.new(subscriptions.reload)
+
+            t1 = Time.zone.now
+            syncer.sync!
+            t2 = Time.zone.now
+            diff = t2 - t1
+            times << diff
+            puts diff.round(2)
+
+            expect(ProxyOrder.count).to be 300
+            subscriptions.update_all(begins_at: start)
+          end
+          puts "AVG: #{(times.sum / times.count).round(2)}"
+        end
+      end
+    end
+  end
+end
diff --git a/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
new file mode 100644
index 0000000000..ab01eddc4c
--- /dev/null
+++ b/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
@@ -0,0 +1,402 @@
+# frozen_string_literal: true
+
+module OrderManagement
+  module Subscriptions
+    describe ProxyOrderSyncer do
+      describe "initialization" do
+        let!(:subscription) { create(:subscription) }
+
+        it "raises an error when initialized with an object that is not a Subscription or an ActiveRecord::Relation" do
+          expect{ ProxyOrderSyncer.new(subscription) }.to_not raise_error
+          expect{ ProxyOrderSyncer.new(Subscription.where(id: subscription.id)) }.to_not raise_error
+          expect{ ProxyOrderSyncer.new("something") }.to raise_error RuntimeError
+        end
+      end
+
+      describe "#sync!" do
+        let(:now) { Time.zone.now }
+        let(:schedule) { create(:schedule) }
+        let(:closed_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now) } # Closed
+        let(:open_oc_closes_before_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now + 59.seconds) } # Open, but closes before begins at
+        let(:open_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now + 90.seconds) } # Open & closes between begins at and ends at
+        let(:upcoming_closes_before_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 59.seconds) } # Upcoming, but closes before begins at
+        let(:upcoming_closes_on_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 1.minute) } # Upcoming & closes on begins at
+        let(:upcoming_closes_on_ends_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 2.minutes) } # Upcoming & closes on ends at
+        let(:upcoming_closes_after_ends_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 121.seconds) } # Upcoming & closes after ends at
+        let(:subscription) { build(:subscription, schedule: schedule, begins_at: now + 1.minute, ends_at: now + 2.minutes) }
+        let(:proxy_orders) { subscription.reload.proxy_orders }
+        let(:order_cycles) { proxy_orders.map(&:order_cycle) }
+        let(:syncer) { ProxyOrderSyncer.new(subscription) }
+
+        context "when the subscription is not persisted" do
+          before do
+            oc # Ensure oc is created before we attempt to sync
+            expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0)
+          end
+
+          context "and the schedule includes a closed oc (ie. closed before opens_at)" do
+            let(:oc) { closed_oc }
+            it "does not create a new proxy order for that oc" do
+              expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0)
+              expect(order_cycles).to_not include oc
+            end
+          end
+
+          context "and the schedule includes an open oc that closes before begins_at" do
+            let(:oc) { open_oc_closes_before_begins_at_oc }
+            it "does not create a new proxy order for that oc" do
+              expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0)
+              expect(order_cycles).to_not include oc
+            end
+          end
+
+          context "and the schedule includes an open oc that closes between begins_at and ends_at" do
+            let(:oc) { open_oc }
+            it "creates a new proxy order for that oc" do
+              expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1)
+              expect(order_cycles).to include oc
+            end
+          end
+
+          context "and the schedule includes upcoming oc that closes before begins_at" do
+            let(:oc) { upcoming_closes_before_begins_at_oc }
+            it "does not create a new proxy order for that oc" do
+              expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0)
+              expect(order_cycles).to_not include oc
+            end
+          end
+
+          context "and the schedule includes upcoming oc that closes on begins_at" do
+            let(:oc) { upcoming_closes_on_begins_at_oc }
+            it "creates a new proxy order for that oc" do
+              expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1)
+              expect(order_cycles).to include oc
+            end
+          end
+
+          context "and the schedule includes upcoming oc that closes after ends_at" do
+            let(:oc) { upcoming_closes_on_ends_at_oc }
+            it "creates a new proxy order for that oc" do
+              expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1)
+              expect(order_cycles).to include oc
+            end
+          end
+
+          context "and the schedule includes upcoming oc that closes after ends_at" do
+            let(:oc) { upcoming_closes_after_ends_at_oc }
+            it "does not create a new proxy order for that oc" do
+              expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0)
+              expect(order_cycles).to_not include oc
+            end
+          end
+        end
+
+        context "when the subscription is persisted" do
+          before { expect(subscription.save!).to be true }
+
+          context "when a proxy order exists" do
+            let!(:proxy_order) { create(:proxy_order, subscription: subscription, order_cycle: oc) }
+
+            context "for an oc included in the relevant schedule" do
+              context "and the proxy order has already been placed" do
+                before { proxy_order.update_attributes(placed_at: 5.minutes.ago) }
+
+                context "the oc is closed (ie. closed before opens_at)" do
+                  let(:oc) { closed_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the schedule includes an open oc that closes before begins_at" do
+                  let(:oc) { open_oc_closes_before_begins_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is open and closes between begins_at and ends_at" do
+                  let(:oc) { open_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes before begins_at" do
+                  let(:oc) { upcoming_closes_before_begins_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes on begins_at" do
+                  let(:oc) { upcoming_closes_on_begins_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes on ends_at" do
+                  let(:oc) { upcoming_closes_on_ends_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes after ends_at" do
+                  let(:oc) { upcoming_closes_after_ends_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+              end
+
+              context "and the proxy order has not already been placed" do
+                context "the oc is closed (ie. closed before opens_at)" do
+                  let(:oc) { closed_oc }
+                  it "removes the proxy order" do
+                    expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
+                    expect(proxy_orders).to_not include proxy_order
+                  end
+                end
+
+                context "and the schedule includes an open oc that closes before begins_at" do
+                  let(:oc) { open_oc_closes_before_begins_at_oc }
+                  it "removes the proxy order" do
+                    expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
+                    expect(proxy_orders).to_not include proxy_order
+                  end
+                end
+
+                context "and the oc is open and closes between begins_at and ends_at" do
+                  let(:oc) { open_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes before begins_at" do
+                  let(:oc) { upcoming_closes_before_begins_at_oc }
+                  it "removes the proxy order" do
+                    expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
+                    expect(proxy_orders).to_not include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes on begins_at" do
+                  let(:oc) { upcoming_closes_on_begins_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes on ends_at" do
+                  let(:oc) { upcoming_closes_on_ends_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes after ends_at" do
+                  let(:oc) { upcoming_closes_after_ends_at_oc }
+                  it "removes the proxy order" do
+                    expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
+                    expect(proxy_orders).to_not include proxy_order
+                  end
+                end
+              end
+            end
+
+            context "for an oc not included in the relevant schedule" do
+              let!(:proxy_order) { create(:proxy_order, subscription: subscription, order_cycle: open_oc) }
+              before do
+                open_oc.schedule_ids = []
+                expect(open_oc.save!).to be true
+              end
+
+              context "and the proxy order has already been placed" do
+                before { proxy_order.update_attributes(placed_at: 5.minutes.ago) }
+
+                context "the oc is closed (ie. closed before opens_at)" do
+                  let(:oc) { closed_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the schedule includes an open oc that closes before begins_at" do
+                  let(:oc) { open_oc_closes_before_begins_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is open and closes between begins_at and ends_at" do
+                  let(:oc) { open_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes before begins_at" do
+                  let(:oc) { upcoming_closes_before_begins_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes on begins_at" do
+                  let(:oc) { upcoming_closes_on_begins_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes on ends_at" do
+                  let(:oc) { upcoming_closes_on_ends_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes after ends_at" do
+                  let(:oc) { upcoming_closes_after_ends_at_oc }
+                  it "keeps the proxy order" do
+                    expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
+                    expect(proxy_orders).to include proxy_order
+                  end
+                end
+              end
+
+              context "and the proxy order has not already been placed" do
+                # This shouldn't really happen, but it is possible
+                context "the oc is closed (ie. closed before opens_at)" do
+                  let(:oc) { closed_oc }
+                  it "removes the proxy order" do
+                    expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
+                    expect(proxy_orders).to_not include proxy_order
+                  end
+                end
+
+                # This shouldn't really happen, but it is possible
+                context "and the oc is open and closes between begins_at and ends_at" do
+                  let(:oc) { open_oc }
+                  it "removes the proxy order" do
+                    expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
+                    expect(proxy_orders).to_not include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes before begins_at" do
+                  let(:oc) { upcoming_closes_before_begins_at_oc }
+                  it "removes the proxy order" do
+                    expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
+                    expect(proxy_orders).to_not include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes on begins_at" do
+                  let(:oc) { upcoming_closes_on_begins_at_oc }
+                  it "removes the proxy order" do
+                    expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
+                    expect(proxy_orders).to_not include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes on ends_at" do
+                  let(:oc) { upcoming_closes_on_ends_at_oc }
+                  it "removes the proxy order" do
+                    expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
+                    expect(proxy_orders).to_not include proxy_order
+                  end
+                end
+
+                context "and the oc is upcoming and closes after ends_at" do
+                  let(:oc) { upcoming_closes_after_ends_at_oc }
+                  it "removes the proxy order" do
+                    expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
+                    expect(proxy_orders).to_not include proxy_order
+                  end
+                end
+              end
+            end
+          end
+
+          context "when a proxy order does not exist" do
+            context "and the schedule includes a closed oc (ie. closed before opens_at)" do
+              let!(:oc) { closed_oc }
+              it "does not create a new proxy order for that oc" do
+                expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0)
+                expect(order_cycles).to_not include oc
+              end
+            end
+
+            context "and the schedule includes an open oc that closes before begins_at" do
+              let(:oc) { open_oc_closes_before_begins_at_oc }
+              it "does not create a new proxy order for that oc" do
+                expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0)
+                expect(order_cycles).to_not include oc
+              end
+            end
+
+            context "and the schedule includes an open oc that closes between begins_at and ends_at" do
+              let!(:oc) { open_oc }
+              it "creates a new proxy order for that oc" do
+                expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1)
+                expect(order_cycles).to include oc
+              end
+            end
+
+            context "and the schedule includes upcoming oc that closes before begins_at" do
+              let!(:oc) { upcoming_closes_before_begins_at_oc }
+              it "does not create a new proxy order for that oc" do
+                expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0)
+                expect(order_cycles).to_not include oc
+              end
+            end
+
+            context "and the schedule includes upcoming oc that closes on begins_at" do
+              let!(:oc) { upcoming_closes_on_begins_at_oc }
+              it "creates a new proxy order for that oc" do
+                expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1)
+                expect(order_cycles).to include oc
+              end
+            end
+
+            context "and the schedule includes upcoming oc that closes on ends_at" do
+              let!(:oc) { upcoming_closes_on_ends_at_oc }
+              it "creates a new proxy order for that oc" do
+                expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1)
+                expect(order_cycles).to include oc
+              end
+            end
+
+            context "and the schedule includes upcoming oc that closes after ends_at" do
+              let!(:oc) { upcoming_closes_after_ends_at_oc }
+              it "does not create a new proxy order for that oc" do
+                expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0)
+                expect(order_cycles).to_not include oc
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/open_food_network/proxy_order_syncer.rb b/lib/open_food_network/proxy_order_syncer.rb
deleted file mode 100644
index 62dc575115..0000000000
--- a/lib/open_food_network/proxy_order_syncer.rb
+++ /dev/null
@@ -1,95 +0,0 @@
-module OpenFoodNetwork
-  class ProxyOrderSyncer
-    attr_reader :subscription
-
-    delegate :order_cycles, :proxy_orders, :begins_at, :ends_at, to: :subscription
-
-    def initialize(subscriptions)
-      case subscriptions
-      when Subscription
-        @subscription = subscriptions
-      when ActiveRecord::Relation
-        @subscriptions = subscriptions.not_ended.not_canceled
-      else
-        raise "ProxyOrderSyncer must be initialized with " \
-                "an instance of Subscription or ActiveRecord::Relation"
-      end
-    end
-
-    def sync!
-      return sync_subscriptions! if @subscriptions
-
-      return initialise_proxy_orders! unless @subscription.id
-
-      sync_subscription!
-    end
-
-    private
-
-    def sync_subscriptions!
-      @subscriptions.each do |subscription|
-        @subscription = subscription
-        sync_subscription!
-      end
-    end
-
-    def initialise_proxy_orders!
-      uninitialised_order_cycle_ids.each do |order_cycle_id|
-        Rails.logger.info "Initializing Proxy Order " \
-          "of subscription #{@subscription.id} in order cycle #{order_cycle_id}"
-        proxy_orders << ProxyOrder.new(subscription: subscription, order_cycle_id: order_cycle_id)
-      end
-    end
-
-    def sync_subscription!
-      Rails.logger.info "Syncing Proxy Orders of subscription #{@subscription.id}"
-      create_proxy_orders!
-      remove_orphaned_proxy_orders!
-    end
-
-    def create_proxy_orders!
-      return unless not_closed_in_range_order_cycles.any?
-
-      query = "INSERT INTO proxy_orders (subscription_id, order_cycle_id, updated_at, created_at)"
-      query << " VALUES #{insert_values}"
-      query << " ON CONFLICT DO NOTHING"
-
-      ActiveRecord::Base.connection.exec_query(query)
-    end
-
-    def uninitialised_order_cycle_ids
-      not_closed_in_range_order_cycles.pluck(:id) - proxy_orders.map(&:order_cycle_id)
-    end
-
-    def remove_orphaned_proxy_orders!
-      orphaned_proxy_orders.scoped.delete_all
-    end
-
-    # Remove Proxy Orders that have not been placed yet
-    #   and are in Order Cycles that are out of range
-    def orphaned_proxy_orders
-      orphaned = proxy_orders.where(placed_at: nil)
-      order_cycle_ids = in_range_order_cycles.pluck(:id)
-      return orphaned unless order_cycle_ids.any?
-
-      orphaned.where('order_cycle_id NOT IN (?)', order_cycle_ids)
-    end
-
-    def insert_values
-      now = Time.now.utc.iso8601
-      not_closed_in_range_order_cycles
-        .map{ |oc| "(#{subscription.id},#{oc.id},'#{now}','#{now}')" }
-        .join(",")
-    end
-
-    def not_closed_in_range_order_cycles
-      in_range_order_cycles.merge(OrderCycle.not_closed)
-    end
-
-    def in_range_order_cycles
-      order_cycles.where("orders_close_at >= ? AND orders_close_at <= ?",
-                         begins_at,
-                         ends_at || 100.years.from_now)
-    end
-  end
-end
diff --git a/spec/controllers/admin/schedules_controller_spec.rb b/spec/controllers/admin/schedules_controller_spec.rb
index 19f2e4b032..5f213b4292 100644
--- a/spec/controllers/admin/schedules_controller_spec.rb
+++ b/spec/controllers/admin/schedules_controller_spec.rb
@@ -90,7 +90,7 @@ describe Admin::SchedulesController, type: :controller do
 
         it "syncs proxy orders when order_cycle_ids change" do
           syncer_mock = double(:syncer)
-          allow(OpenFoodNetwork::ProxyOrderSyncer).to receive(:new) { syncer_mock }
+          allow(OrderManagement::Subscriptions::ProxyOrderSyncer).to receive(:new) { syncer_mock }
           expect(syncer_mock).to receive(:sync!).exactly(2).times
 
           spree_put :update, format: :json, id: coordinated_schedule.id, schedule: { order_cycle_ids: [coordinated_order_cycle.id, coordinated_order_cycle2.id] }
@@ -150,7 +150,7 @@ describe Admin::SchedulesController, type: :controller do
 
           it "sync proxy orders" do
             syncer_mock = double(:syncer)
-            allow(OpenFoodNetwork::ProxyOrderSyncer).to receive(:new) { syncer_mock }
+            allow(OrderManagement::Subscriptions::ProxyOrderSyncer).to receive(:new) { syncer_mock }
             expect(syncer_mock).to receive(:sync!).once
 
             create_schedule params
diff --git a/spec/lib/open_food_network/proxy_order_syncer_spec.rb b/spec/lib/open_food_network/proxy_order_syncer_spec.rb
deleted file mode 100644
index 9b059d178f..0000000000
--- a/spec/lib/open_food_network/proxy_order_syncer_spec.rb
+++ /dev/null
@@ -1,400 +0,0 @@
-require 'open_food_network/proxy_order_syncer'
-
-module OpenFoodNetwork
-  describe ProxyOrderSyncer do
-    describe "initialization" do
-      let!(:subscription) { create(:subscription) }
-
-      it "raises an error when initialized with an object that is not a Subscription or an ActiveRecord::Relation" do
-        expect{ ProxyOrderSyncer.new(subscription) }.to_not raise_error
-        expect{ ProxyOrderSyncer.new(Subscription.where(id: subscription.id)) }.to_not raise_error
-        expect{ ProxyOrderSyncer.new("something") }.to raise_error RuntimeError
-      end
-    end
-
-    describe "#sync!" do
-      let(:now) { Time.zone.now }
-      let(:schedule) { create(:schedule) }
-      let(:closed_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now) } # Closed
-      let(:open_oc_closes_before_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now + 59.seconds) } # Open, but closes before begins at
-      let(:open_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now + 90.seconds) } # Open & closes between begins at and ends at
-      let(:upcoming_closes_before_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 59.seconds) } # Upcoming, but closes before begins at
-      let(:upcoming_closes_on_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 1.minute) } # Upcoming & closes on begins at
-      let(:upcoming_closes_on_ends_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 2.minutes) } # Upcoming & closes on ends at
-      let(:upcoming_closes_after_ends_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 121.seconds) } # Upcoming & closes after ends at
-      let(:subscription) { build(:subscription, schedule: schedule, begins_at: now + 1.minute, ends_at: now + 2.minutes) }
-      let(:proxy_orders) { subscription.reload.proxy_orders }
-      let(:order_cycles) { proxy_orders.map(&:order_cycle) }
-      let(:syncer) { ProxyOrderSyncer.new(subscription) }
-
-      context "when the subscription is not persisted" do
-        before do
-          oc # Ensure oc is created before we attempt to sync
-          expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0)
-        end
-
-        context "and the schedule includes a closed oc (ie. closed before opens_at)" do
-          let(:oc) { closed_oc }
-          it "does not create a new proxy order for that oc" do
-            expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0)
-            expect(order_cycles).to_not include oc
-          end
-        end
-
-        context "and the schedule includes an open oc that closes before begins_at" do
-          let(:oc) { open_oc_closes_before_begins_at_oc }
-          it "does not create a new proxy order for that oc" do
-            expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0)
-            expect(order_cycles).to_not include oc
-          end
-        end
-
-        context "and the schedule includes an open oc that closes between begins_at and ends_at" do
-          let(:oc) { open_oc }
-          it "creates a new proxy order for that oc" do
-            expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1)
-            expect(order_cycles).to include oc
-          end
-        end
-
-        context "and the schedule includes upcoming oc that closes before begins_at" do
-          let(:oc) { upcoming_closes_before_begins_at_oc }
-          it "does not create a new proxy order for that oc" do
-            expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0)
-            expect(order_cycles).to_not include oc
-          end
-        end
-
-        context "and the schedule includes upcoming oc that closes on begins_at" do
-          let(:oc) { upcoming_closes_on_begins_at_oc }
-          it "creates a new proxy order for that oc" do
-            expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1)
-            expect(order_cycles).to include oc
-          end
-        end
-
-        context "and the schedule includes upcoming oc that closes after ends_at" do
-          let(:oc) { upcoming_closes_on_ends_at_oc }
-          it "creates a new proxy order for that oc" do
-            expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1)
-            expect(order_cycles).to include oc
-          end
-        end
-
-        context "and the schedule includes upcoming oc that closes after ends_at" do
-          let(:oc) { upcoming_closes_after_ends_at_oc }
-          it "does not create a new proxy order for that oc" do
-            expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0)
-            expect(order_cycles).to_not include oc
-          end
-        end
-      end
-
-      context "when the subscription is persisted" do
-        before { expect(subscription.save!).to be true }
-
-        context "when a proxy order exists" do
-          let!(:proxy_order) { create(:proxy_order, subscription: subscription, order_cycle: oc) }
-
-          context "for an oc included in the relevant schedule" do
-            context "and the proxy order has already been placed" do
-              before { proxy_order.update_attributes(placed_at: 5.minutes.ago) }
-
-              context "the oc is closed (ie. closed before opens_at)" do
-                let(:oc) { closed_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the schedule includes an open oc that closes before begins_at" do
-                let(:oc) { open_oc_closes_before_begins_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is open and closes between begins_at and ends_at" do
-                let(:oc) { open_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes before begins_at" do
-                let(:oc) { upcoming_closes_before_begins_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes on begins_at" do
-                let(:oc) { upcoming_closes_on_begins_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes on ends_at" do
-                let(:oc) { upcoming_closes_on_ends_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes after ends_at" do
-                let(:oc) { upcoming_closes_after_ends_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-            end
-
-            context "and the proxy order has not already been placed" do
-              context "the oc is closed (ie. closed before opens_at)" do
-                let(:oc) { closed_oc }
-                it "removes the proxy order" do
-                  expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
-                  expect(proxy_orders).to_not include proxy_order
-                end
-              end
-
-              context "and the schedule includes an open oc that closes before begins_at" do
-                let(:oc) { open_oc_closes_before_begins_at_oc }
-                it "removes the proxy order" do
-                  expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
-                  expect(proxy_orders).to_not include proxy_order
-                end
-              end
-
-              context "and the oc is open and closes between begins_at and ends_at" do
-                let(:oc) { open_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes before begins_at" do
-                let(:oc) { upcoming_closes_before_begins_at_oc }
-                it "removes the proxy order" do
-                  expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
-                  expect(proxy_orders).to_not include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes on begins_at" do
-                let(:oc) { upcoming_closes_on_begins_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes on ends_at" do
-                let(:oc) { upcoming_closes_on_ends_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes after ends_at" do
-                let(:oc) { upcoming_closes_after_ends_at_oc }
-                it "removes the proxy order" do
-                  expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
-                  expect(proxy_orders).to_not include proxy_order
-                end
-              end
-            end
-          end
-
-          context "for an oc not included in the relevant schedule" do
-            let!(:proxy_order) { create(:proxy_order, subscription: subscription, order_cycle: open_oc) }
-            before do
-              open_oc.schedule_ids = []
-              expect(open_oc.save!).to be true
-            end
-
-            context "and the proxy order has already been placed" do
-              before { proxy_order.update_attributes(placed_at: 5.minutes.ago) }
-
-              context "the oc is closed (ie. closed before opens_at)" do
-                let(:oc) { closed_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the schedule includes an open oc that closes before begins_at" do
-                let(:oc) { open_oc_closes_before_begins_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is open and closes between begins_at and ends_at" do
-                let(:oc) { open_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes before begins_at" do
-                let(:oc) { upcoming_closes_before_begins_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes on begins_at" do
-                let(:oc) { upcoming_closes_on_begins_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes on ends_at" do
-                let(:oc) { upcoming_closes_on_ends_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes after ends_at" do
-                let(:oc) { upcoming_closes_after_ends_at_oc }
-                it "keeps the proxy order" do
-                  expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1)
-                  expect(proxy_orders).to include proxy_order
-                end
-              end
-            end
-
-            context "and the proxy order has not already been placed" do
-              # This shouldn't really happen, but it is possible
-              context "the oc is closed (ie. closed before opens_at)" do
-                let(:oc) { closed_oc }
-                it "removes the proxy order" do
-                  expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
-                  expect(proxy_orders).to_not include proxy_order
-                end
-              end
-
-              # This shouldn't really happen, but it is possible
-              context "and the oc is open and closes between begins_at and ends_at" do
-                let(:oc) { open_oc }
-                it "removes the proxy order" do
-                  expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
-                  expect(proxy_orders).to_not include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes before begins_at" do
-                let(:oc) { upcoming_closes_before_begins_at_oc }
-                it "removes the proxy order" do
-                  expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
-                  expect(proxy_orders).to_not include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes on begins_at" do
-                let(:oc) { upcoming_closes_on_begins_at_oc }
-                it "removes the proxy order" do
-                  expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
-                  expect(proxy_orders).to_not include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes on ends_at" do
-                let(:oc) { upcoming_closes_on_ends_at_oc }
-                it "removes the proxy order" do
-                  expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
-                  expect(proxy_orders).to_not include proxy_order
-                end
-              end
-
-              context "and the oc is upcoming and closes after ends_at" do
-                let(:oc) { upcoming_closes_after_ends_at_oc }
-                it "removes the proxy order" do
-                  expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0)
-                  expect(proxy_orders).to_not include proxy_order
-                end
-              end
-            end
-          end
-        end
-
-        context "when a proxy order does not exist" do
-          context "and the schedule includes a closed oc (ie. closed before opens_at)" do
-            let!(:oc) { closed_oc }
-            it "does not create a new proxy order for that oc" do
-              expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0)
-              expect(order_cycles).to_not include oc
-            end
-          end
-
-          context "and the schedule includes an open oc that closes before begins_at" do
-            let(:oc) { open_oc_closes_before_begins_at_oc }
-            it "does not create a new proxy order for that oc" do
-              expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0)
-              expect(order_cycles).to_not include oc
-            end
-          end
-
-          context "and the schedule includes an open oc that closes between begins_at and ends_at" do
-            let!(:oc) { open_oc }
-            it "creates a new proxy order for that oc" do
-              expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1)
-              expect(order_cycles).to include oc
-            end
-          end
-
-          context "and the schedule includes upcoming oc that closes before begins_at" do
-            let!(:oc) { upcoming_closes_before_begins_at_oc }
-            it "does not create a new proxy order for that oc" do
-              expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0)
-              expect(order_cycles).to_not include oc
-            end
-          end
-
-          context "and the schedule includes upcoming oc that closes on begins_at" do
-            let!(:oc) { upcoming_closes_on_begins_at_oc }
-            it "creates a new proxy order for that oc" do
-              expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1)
-              expect(order_cycles).to include oc
-            end
-          end
-
-          context "and the schedule includes upcoming oc that closes on ends_at" do
-            let!(:oc) { upcoming_closes_on_ends_at_oc }
-            it "creates a new proxy order for that oc" do
-              expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1)
-              expect(order_cycles).to include oc
-            end
-          end
-
-          context "and the schedule includes upcoming oc that closes after ends_at" do
-            let!(:oc) { upcoming_closes_after_ends_at_oc }
-            it "does not create a new proxy order for that oc" do
-              expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0)
-              expect(order_cycles).to_not include oc
-            end
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/spec/performance/proxy_order_syncer_spec.rb b/spec/performance/proxy_order_syncer_spec.rb
deleted file mode 100644
index 95678c68a2..0000000000
--- a/spec/performance/proxy_order_syncer_spec.rb
+++ /dev/null
@@ -1,67 +0,0 @@
-require 'open_food_network/proxy_order_syncer'
-
-module OpenFoodNetwork
-  describe ProxyOrderSyncer, performance: true do
-    let(:start) { Time.zone.now.beginning_of_day }
-    let!(:schedule) { create(:schedule, order_cycles: order_cycles) }
-
-    let!(:order_cycles) do
-      Array.new(10) do |i|
-        create(:simple_order_cycle, orders_open_at: start + i.days,
-                                    orders_close_at: start + (i + 1).days )
-      end
-    end
-
-    let!(:subscriptions) do
-      Array.new(150) do |_i|
-        create(:subscription, schedule: schedule, begins_at: start, ends_at: start + 10.days)
-      end
-      Subscription.where(schedule_id: schedule)
-    end
-
-    context "measuring performance for initialisation" do
-      it "reports the average run time for adding 10 OCs to 150 subscriptions" do
-        expect(ProxyOrder.count).to be 0
-        times = []
-        10.times do
-          syncer = ProxyOrderSyncer.new(subscriptions.reload)
-
-          t1 = Time.zone.now
-          syncer.sync!
-          t2 = Time.zone.now
-          diff = t2 - t1
-          times << diff
-          puts diff.round(2)
-
-          expect(ProxyOrder.count).to be 1500
-          ProxyOrder.destroy_all
-        end
-        puts "AVG: #{(times.sum / times.count).round(2)}"
-      end
-    end
-
-    context "measuring performance for removal" do
-      it "reports the average run time for removing 8 OCs from 150 subscriptions" do
-        times = []
-        10.times do
-          syncer = ProxyOrderSyncer.new(subscriptions.reload)
-          syncer.sync!
-          expect(ProxyOrder.count).to be 1500
-          subscriptions.update_all(begins_at: start + 8.days + 1.minute)
-          syncer = ProxyOrderSyncer.new(subscriptions.reload)
-
-          t1 = Time.zone.now
-          syncer.sync!
-          t2 = Time.zone.now
-          diff = t2 - t1
-          times << diff
-          puts diff.round(2)
-
-          expect(ProxyOrder.count).to be 300
-          subscriptions.update_all(begins_at: start)
-        end
-        puts "AVG: #{(times.sum / times.count).round(2)}"
-      end
-    end
-  end
-end
diff --git a/spec/services/order_cycle_form_spec.rb b/spec/services/order_cycle_form_spec.rb
index 476adb3e39..e4d5d144d1 100644
--- a/spec/services/order_cycle_form_spec.rb
+++ b/spec/services/order_cycle_form_spec.rb
@@ -1,3 +1,5 @@
+require 'order_management/subscriptions/proxy_order_syncer'
+
 describe OrderCycleForm do
   describe "save" do
     describe "creating a new order cycle from params" do
@@ -66,10 +68,10 @@ describe OrderCycleForm do
 
     context "where I manage the order_cycle's coordinator" do
       let(:form) { OrderCycleForm.new(coordinated_order_cycle, params, user) }
-      let(:syncer_mock) { instance_double(OpenFoodNetwork::ProxyOrderSyncer, sync!: true) }
+      let(:syncer_mock) { instance_double(OrderManagement::Subscriptions::ProxyOrderSyncer, sync!: true) }
 
       before do
-        allow(OpenFoodNetwork::ProxyOrderSyncer).to receive(:new) { syncer_mock }
+        allow(OrderManagement::Subscriptions::ProxyOrderSyncer).to receive(:new) { syncer_mock }
       end
 
       context "and I add an schedule that I own, and remove another that I own" do

From 3901c49af9009e0f0776762860f9cfb6c3f9c69d Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 18:37:34 +0000
Subject: [PATCH 09/12] Fix rubocop issues

---
 .rubocop_manual_todo.yml                      |  2 +-
 .rubocop_todo.yml                             |  8 ---
 .../admin/subscriptions_controller.rb         |  1 -
 .../subscriptions/subscription_summary.rb     |  2 +-
 .../subscriptions/proxy_order_syncer_spec.rb  | 59 +++++++++++++++----
 .../subscription_summarizer_spec.rb           |  4 +-
 6 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index 9494595048..bd710779cc 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -101,7 +101,6 @@ Layout/LineLength:
   - app/services/subscriptions_count.rb
   - app/services/variants_stock_levels.rb
   - engines/web/app/helpers/web/cookies_policy_helper.rb
-  - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
   - lib/discourse/single_sign_on.rb
   - lib/open_food_network/available_payment_method_filter.rb
   - lib/open_food_network/bulk_coop_report.rb
@@ -686,6 +685,7 @@ Metrics/ModuleLength:
   - app/helpers/spree/admin/base_helper.rb
   - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
+  - engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb
   - lib/open_food_network/column_preference_defaults.rb
   - spec/controllers/admin/enterprises_controller_spec.rb
   - spec/controllers/admin/order_cycles_controller_spec.rb
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 1c003c0504..e498ce594a 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -70,7 +70,6 @@ Lint/DuplicateHashKey:
 # Offense count: 4
 Lint/DuplicateMethods:
   Exclude:
-    - 'engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb'
     - 'lib/discourse/single_sign_on.rb'
 
 # Offense count: 10
@@ -882,9 +881,6 @@ Style/FrozenStringLiteralComment:
     - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/report_service.rb'
     - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb'
     - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/summarizer.rb'
-    - 'engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb'
-    - 'engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb'
-    - 'engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb'
     - 'engines/order_management/app/services/reports.rb'
     - 'engines/order_management/app/services/reports/authorizer.rb'
     - 'engines/order_management/app/services/reports/parameters/base.rb'
@@ -904,9 +900,6 @@ Style/FrozenStringLiteralComment:
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb'
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_data/enterprise_fee_type_total_spec.rb'
     - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb'
-    - 'engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb'
-    - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summizer_spec.rb'
-    - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb'
     - 'engines/order_management/spec/spec_helper.rb'
     - 'engines/web/app/controllers/web/angular_templates_controller.rb'
     - 'engines/web/app/controllers/web/api/cookies_consent_controller.rb'
@@ -1542,7 +1535,6 @@ Style/Send:
   Exclude:
     - 'app/controllers/spree/checkout_controller.rb'
     - 'app/models/spree/shipping_method_decorator.rb'
-    - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb'
     - 'spec/controllers/admin/subscriptions_controller_spec.rb'
     - 'spec/controllers/checkout_controller_spec.rb'
     - 'spec/controllers/spree/admin/base_controller_spec.rb'
diff --git a/app/controllers/admin/subscriptions_controller.rb b/app/controllers/admin/subscriptions_controller.rb
index da99409469..0b3952d5a6 100644
--- a/app/controllers/admin/subscriptions_controller.rb
+++ b/app/controllers/admin/subscriptions_controller.rb
@@ -1,5 +1,4 @@
 require 'open_food_network/permissions'
-require 'open_food_network/proxy_order_syncer'
 
 module Admin
   class SubscriptionsController < ResourceController
diff --git a/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb b/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb
index aa9a61c469..2faa93b6ee 100644
--- a/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb
+++ b/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb
@@ -3,7 +3,7 @@
 module OrderManagement
   module Subscriptions
     class SubscriptionSummary
-      attr_reader :shop_id, :order_count, :success_count, :issues
+      attr_reader :shop_id, :issues
 
       def initialize(shop_id)
         @shop_id = shop_id
diff --git a/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
index ab01eddc4c..501f6fa67e 100644
--- a/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
@@ -6,7 +6,8 @@ module OrderManagement
       describe "initialization" do
         let!(:subscription) { create(:subscription) }
 
-        it "raises an error when initialized with an object that is not a Subscription or an ActiveRecord::Relation" do
+        it "raises an error when initialized with an object
+            that is not a Subscription or an ActiveRecord::Relation" do
           expect{ ProxyOrderSyncer.new(subscription) }.to_not raise_error
           expect{ ProxyOrderSyncer.new(Subscription.where(id: subscription.id)) }.to_not raise_error
           expect{ ProxyOrderSyncer.new("something") }.to raise_error RuntimeError
@@ -16,14 +17,46 @@ module OrderManagement
       describe "#sync!" do
         let(:now) { Time.zone.now }
         let(:schedule) { create(:schedule) }
-        let(:closed_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now) } # Closed
-        let(:open_oc_closes_before_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now + 59.seconds) } # Open, but closes before begins at
-        let(:open_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now + 90.seconds) } # Open & closes between begins at and ends at
-        let(:upcoming_closes_before_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 59.seconds) } # Upcoming, but closes before begins at
-        let(:upcoming_closes_on_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 1.minute) } # Upcoming & closes on begins at
-        let(:upcoming_closes_on_ends_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 2.minutes) } # Upcoming & closes on ends at
-        let(:upcoming_closes_after_ends_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 121.seconds) } # Upcoming & closes after ends at
-        let(:subscription) { build(:subscription, schedule: schedule, begins_at: now + 1.minute, ends_at: now + 2.minutes) }
+        let(:closed_oc) { # Closed
+          create(:simple_order_cycle, schedules: [schedule],
+                                      orders_open_at: now - 1.minute,
+                                      orders_close_at: now)
+        }
+        let(:open_oc_closes_before_begins_at_oc) { # Open, but closes before begins at
+          create(:simple_order_cycle, schedules: [schedule],
+                                      orders_open_at: now - 1.minute,
+                                      orders_close_at: now + 59.seconds)
+        }
+        let(:open_oc) { # Open & closes between begins at and ends at
+          create(:simple_order_cycle, schedules: [schedule],
+                                      orders_open_at: now - 1.minute,
+                                      orders_close_at: now + 90.seconds)
+        }
+        let(:upcoming_closes_before_begins_at_oc) { # Upcoming, but closes before begins at
+          create(:simple_order_cycle, schedules: [schedule],
+                                      orders_open_at: now + 30.seconds,
+                                      orders_close_at: now + 59.seconds)
+        }
+        let(:upcoming_closes_on_begins_at_oc) { # Upcoming & closes on begins at
+          create(:simple_order_cycle, schedules: [schedule],
+                                      orders_open_at: now + 30.seconds,
+                                      orders_close_at: now + 1.minute)
+        }
+        let(:upcoming_closes_on_ends_at_oc) { # Upcoming & closes on ends at
+          create(:simple_order_cycle, schedules: [schedule],
+                                      orders_open_at: now + 30.seconds,
+                                      orders_close_at: now + 2.minutes)
+        }
+        let(:upcoming_closes_after_ends_at_oc) { # Upcoming & closes after ends at
+          create(:simple_order_cycle, schedules: [schedule],
+                                      orders_open_at: now + 30.seconds,
+                                      orders_close_at: now + 121.seconds)
+        }
+        let(:subscription) {
+          build(:subscription, schedule: schedule,
+                               begins_at: now + 1.minute,
+                               ends_at: now + 2.minutes)
+        }
         let(:proxy_orders) { subscription.reload.proxy_orders }
         let(:order_cycles) { proxy_orders.map(&:order_cycle) }
         let(:syncer) { ProxyOrderSyncer.new(subscription) }
@@ -50,7 +83,7 @@ module OrderManagement
             end
           end
 
-          context "and the schedule includes an open oc that closes between begins_at and ends_at" do
+          context "and the schedule has an open OC that closes between begins_at and ends_at" do
             let(:oc) { open_oc }
             it "creates a new proxy order for that oc" do
               expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1)
@@ -218,7 +251,9 @@ module OrderManagement
             end
 
             context "for an oc not included in the relevant schedule" do
-              let!(:proxy_order) { create(:proxy_order, subscription: subscription, order_cycle: open_oc) }
+              let!(:proxy_order) {
+                create(:proxy_order, subscription: subscription, order_cycle: open_oc)
+              }
               before do
                 open_oc.schedule_ids = []
                 expect(open_oc.save!).to be true
@@ -355,7 +390,7 @@ module OrderManagement
               end
             end
 
-            context "and the schedule includes an open oc that closes between begins_at and ends_at" do
+            context "and the schedule has an open oc that closes between begins_at and ends_at" do
               let!(:oc) { open_oc }
               it "creates a new proxy order for that oc" do
                 expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1)
diff --git a/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb
index 26a79af4d1..2da6d1f7fb 100644
--- a/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb
@@ -16,7 +16,7 @@ module OrderManagement
         context "when a summary for the order's distributor doesn't already exist" do
           it "initializes a new summary object, and returns it" do
             expect(summarizer.instance_variable_get(:@summaries).count).to be 0
-            summary = summarizer.send(:summary_for, order)
+            summary = summarizer.__send__(:summary_for, order)
             expect(summary.shop_id).to be 123
             expect(summarizer.instance_variable_get(:@summaries).count).to be 1
           end
@@ -31,7 +31,7 @@ module OrderManagement
 
           it "returns the existing summary object" do
             expect(summarizer.instance_variable_get(:@summaries).count).to be 1
-            expect(summarizer.send(:summary_for, order)).to eq summary
+            expect(summarizer.__send__(:summary_for, order)).to eq summary
             expect(summarizer.instance_variable_get(:@summaries).count).to be 1
           end
         end

From f68d0c2a0f73314d249a888e3e439868c88b01a8 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 19:14:43 +0000
Subject: [PATCH 10/12] Remove Subscription from the name of the subscription
 summarizer and summary because it is already in the namespace

---
 .rubocop_manual_todo.yml                                      | 2 +-
 app/jobs/subscription_confirm_job.rb                          | 4 ++--
 app/jobs/subscription_placement_job.rb                        | 4 ++--
 .../{subscription_summarizer.rb => summarizer.rb}             | 4 ++--
 .../subscriptions/{subscription_summary.rb => summary.rb}     | 2 +-
 .../{subscription_summarizer_spec.rb => summarizer_spec.rb}   | 4 ++--
 .../{subscription_summary_spec.rb => summary_spec.rb}         | 4 ++--
 7 files changed, 12 insertions(+), 12 deletions(-)
 rename engines/order_management/app/services/order_management/subscriptions/{subscription_summarizer.rb => summarizer.rb} (94%)
 rename engines/order_management/app/services/order_management/subscriptions/{subscription_summary.rb => summary.rb} (97%)
 rename engines/order_management/spec/services/order_management/subscriptions/{subscription_summarizer_spec.rb => summarizer_spec.rb} (97%)
 rename engines/order_management/spec/services/order_management/subscriptions/{subscription_summary_spec.rb => summary_spec.rb} (97%)

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index bd710779cc..0ae58c725c 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -685,7 +685,7 @@ Metrics/ModuleLength:
   - app/helpers/spree/admin/base_helper.rb
   - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
-  - engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb
+  - engines/order_management/spec/services/order_management/subscriptions/summarizer_spec.rb
   - lib/open_food_network/column_preference_defaults.rb
   - spec/controllers/admin/enterprises_controller_spec.rb
   - spec/controllers/admin/order_cycles_controller_spec.rb
diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb
index 9b1ae7a79f..3612b7e0d4 100644
--- a/app/jobs/subscription_confirm_job.rb
+++ b/app/jobs/subscription_confirm_job.rb
@@ -1,4 +1,4 @@
-require 'order_management/subscriptions/subscription_summarizer'
+require 'order_management/subscriptions/summarizer'
 
 # Confirms orders of unconfirmed proxy orders in recently closed Order Cycles
 class SubscriptionConfirmJob
@@ -12,7 +12,7 @@ class SubscriptionConfirmJob
   delegate :record_and_log_error, :send_confirmation_summary_emails, to: :summarizer
 
   def summarizer
-    @summarizer ||= OrderManagement::Subscriptions::SubscriptionSummarizer.new
+    @summarizer ||= OrderManagement::Subscriptions::Summarizer.new
   end
 
   def confirm_proxy_orders!
diff --git a/app/jobs/subscription_placement_job.rb b/app/jobs/subscription_placement_job.rb
index 8a77367217..44a4a27c75 100644
--- a/app/jobs/subscription_placement_job.rb
+++ b/app/jobs/subscription_placement_job.rb
@@ -1,4 +1,4 @@
-require 'order_management/subscriptions/subscription_summarizer'
+require 'order_management/subscriptions/summarizer'
 
 class SubscriptionPlacementJob
   def perform
@@ -17,7 +17,7 @@ class SubscriptionPlacementJob
   delegate :record_and_log_error, :send_placement_summary_emails, to: :summarizer
 
   def summarizer
-    @summarizer ||= OrderManagement::Subscriptions::SubscriptionSummarizer.new
+    @summarizer ||= OrderManagement::Subscriptions::Summarizer.new
   end
 
   def proxy_orders
diff --git a/engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb b/engines/order_management/app/services/order_management/subscriptions/summarizer.rb
similarity index 94%
rename from engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb
rename to engines/order_management/app/services/order_management/subscriptions/summarizer.rb
index dbffcb96ab..0642e22044 100644
--- a/engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb
+++ b/engines/order_management/app/services/order_management/subscriptions/summarizer.rb
@@ -4,7 +4,7 @@
 # result of automatic processing of subscriptions for the relevant shop owners.
 module OrderManagement
   module Subscriptions
-    class SubscriptionSummarizer
+    class Summarizer
       def initialize
         @summaries = {}
       end
@@ -48,7 +48,7 @@ module OrderManagement
 
       def summary_for(order)
         shop_id = order.distributor_id
-        @summaries[shop_id] ||= SubscriptionSummary.new(shop_id)
+        @summaries[shop_id] ||= Summary.new(shop_id)
       end
     end
   end
diff --git a/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb b/engines/order_management/app/services/order_management/subscriptions/summary.rb
similarity index 97%
rename from engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb
rename to engines/order_management/app/services/order_management/subscriptions/summary.rb
index 2faa93b6ee..c37272c6d3 100644
--- a/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb
+++ b/engines/order_management/app/services/order_management/subscriptions/summary.rb
@@ -2,7 +2,7 @@
 
 module OrderManagement
   module Subscriptions
-    class SubscriptionSummary
+    class Summary
       attr_reader :shop_id, :issues
 
       def initialize(shop_id)
diff --git a/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/summarizer_spec.rb
similarity index 97%
rename from engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb
rename to engines/order_management/spec/services/order_management/subscriptions/summarizer_spec.rb
index 2da6d1f7fb..09013c204c 100644
--- a/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/summarizer_spec.rb
@@ -4,9 +4,9 @@ require 'spec_helper'
 
 module OrderManagement
   module Subscriptions
-    describe SubscriptionSummarizer do
+    describe Summarizer do
       let(:order) { create(:order) }
-      let(:summarizer) { OrderManagement::Subscriptions::SubscriptionSummarizer.new }
+      let(:summarizer) { OrderManagement::Subscriptions::Summarizer.new }
 
       before { allow(Rails.logger).to receive(:info) }
 
diff --git a/engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/summary_spec.rb
similarity index 97%
rename from engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb
rename to engines/order_management/spec/services/order_management/subscriptions/summary_spec.rb
index e24aa6449b..5e6360c542 100644
--- a/engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/summary_spec.rb
@@ -2,8 +2,8 @@
 
 module OrderManagement
   module Subscriptions
-    describe SubscriptionSummary do
-      let(:summary) { OrderManagement::Subscriptions::SubscriptionSummary.new(123) }
+    describe Summary do
+      let(:summary) { OrderManagement::Subscriptions::Summary.new(123) }
 
       describe "#initialize" do
         it "initializes instance variables: shop_id, order_count, success_count and issues" do

From 29377bbff9704110a0f615e6513379a3fbe8ff50 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 19:34:14 +0000
Subject: [PATCH 11/12] Move 5 subscriptions services from app/services to the
 engines/order_management/app/services

---
 .rubocop_manual_todo.yml                      |   7 +-
 .rubocop_todo.yml                             |  12 +-
 .../admin/order_cycles_controller.rb          |   4 +-
 .../subscription_line_items_controller.rb     |   2 +-
 .../admin/subscriptions_controller.rb         |   2 +-
 .../subscription_line_item_serializer.rb      |   6 +-
 app/services/subscription_estimator.rb        |  63 ---
 app/services/subscription_form.rb             |  34 --
 app/services/subscription_validator.rb        | 127 -----
 app/services/subscription_variants_service.rb |  39 --
 app/services/subscriptions_count.rb           |  20 -
 config/locales/en.yml                         |   2 +-
 .../order_management/subscriptions/count.rb   |  30 ++
 .../subscriptions/estimator.rb                |  69 +++
 .../order_management/subscriptions/form.rb    |  41 ++
 .../subscriptions/validator.rb                | 132 +++++
 .../subscriptions/variants_list.rb            |  46 ++
 .../subscriptions/count_spec.rb               |  40 ++
 .../subscriptions/estimator_spec.rb           | 135 +++++
 .../subscriptions/form_spec.rb                | 103 ++++
 .../subscriptions/validator_spec.rb           | 476 ++++++++++++++++++
 .../subscriptions/variants_list_spec.rb       | 136 +++++
 .../scope_variants_for_search.rb              |   2 +-
 spec/services/subscription_estimator_spec.rb  | 129 -----
 spec/services/subscription_form_spec.rb       |  97 ----
 spec/services/subscription_validator_spec.rb  | 470 -----------------
 .../subscription_variants_service_spec.rb     | 130 -----
 spec/services/subscriptions_count_spec.rb     |  34 --
 28 files changed, 1219 insertions(+), 1169 deletions(-)
 delete mode 100644 app/services/subscription_estimator.rb
 delete mode 100644 app/services/subscription_form.rb
 delete mode 100644 app/services/subscription_validator.rb
 delete mode 100644 app/services/subscription_variants_service.rb
 delete mode 100644 app/services/subscriptions_count.rb
 create mode 100644 engines/order_management/app/services/order_management/subscriptions/count.rb
 create mode 100644 engines/order_management/app/services/order_management/subscriptions/estimator.rb
 create mode 100644 engines/order_management/app/services/order_management/subscriptions/form.rb
 create mode 100644 engines/order_management/app/services/order_management/subscriptions/validator.rb
 create mode 100644 engines/order_management/app/services/order_management/subscriptions/variants_list.rb
 create mode 100644 engines/order_management/spec/services/order_management/subscriptions/count_spec.rb
 create mode 100644 engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb
 create mode 100644 engines/order_management/spec/services/order_management/subscriptions/form_spec.rb
 create mode 100644 engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb
 create mode 100644 engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb
 delete mode 100644 spec/services/subscription_estimator_spec.rb
 delete mode 100644 spec/services/subscription_form_spec.rb
 delete mode 100644 spec/services/subscription_validator_spec.rb
 delete mode 100644 spec/services/subscription_variants_service_spec.rb
 delete mode 100644 spec/services/subscriptions_count_spec.rb

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index 0ae58c725c..83de12879f 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -98,7 +98,6 @@ Layout/LineLength:
   - app/services/embedded_page_service.rb
   - app/services/order_cycle_form.rb
   - app/services/order_factory.rb
-  - app/services/subscriptions_count.rb
   - app/services/variants_stock_levels.rb
   - engines/web/app/helpers/web/cookies_policy_helper.rb
   - lib/discourse/single_sign_on.rb
@@ -314,10 +313,6 @@ Layout/LineLength:
   - spec/services/permissions/order_spec.rb
   - spec/services/product_tag_rules_filterer_spec.rb
   - spec/services/products_renderer_spec.rb
-  - spec/services/subscription_estimator_spec.rb
-  - spec/services/subscription_form_spec.rb
-  - spec/services/subscription_validator_spec.rb
-  - spec/services/subscription_variants_service_spec.rb
   - spec/spec_helper.rb
   - spec/support/cancan_helper.rb
   - spec/support/delayed_job_helper.rb
@@ -408,7 +403,7 @@ Metrics/AbcSize:
   - app/services/cart_service.rb
   - app/services/create_order_cycle.rb
   - app/services/order_syncer.rb
-  - app/services/subscription_validator.rb
+  - engines/order_management/app/services/order_management/subscriptions/validator.rb
   - lib/active_merchant/billing/gateways/stripe_decorator.rb
   - lib/active_merchant/billing/gateways/stripe_payment_intents.rb
   - lib/discourse/single_sign_on.rb
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index e498ce594a..984791e600 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -158,7 +158,7 @@ Naming/MethodParameterName:
   Exclude:
     - 'app/helpers/spree/admin/base_helper_decorator.rb'
     - 'app/helpers/spree/base_helper_decorator.rb'
-    - 'app/services/subscription_validator.rb'
+    - 'engines/order_management/app/services/order_management/subscriptions/validator.rb'
     - 'lib/open_food_network/reports/bulk_coop_report.rb'
     - 'lib/open_food_network/xero_invoices_report.rb'
     - 'spec/lib/open_food_network/reports/report_spec.rb'
@@ -849,11 +849,6 @@ Style/FrozenStringLiteralComment:
     - 'app/services/reset_order_service.rb'
     - 'app/services/restart_checkout.rb'
     - 'app/services/search_orders.rb'
-    - 'app/services/subscription_estimator.rb'
-    - 'app/services/subscription_form.rb'
-    - 'app/services/subscription_validator.rb'
-    - 'app/services/subscription_variants_service.rb'
-    - 'app/services/subscriptions_count.rb'
     - 'app/services/tax_rate_finder.rb'
     - 'app/services/upload_sanitizer.rb'
     - 'app/services/variant_deleter.rb'
@@ -1350,11 +1345,6 @@ Style/FrozenStringLiteralComment:
     - 'spec/services/reset_order_service_spec.rb'
     - 'spec/services/restart_checkout_spec.rb'
     - 'spec/services/search_orders_spec.rb'
-    - 'spec/services/subscription_estimator_spec.rb'
-    - 'spec/services/subscription_form_spec.rb'
-    - 'spec/services/subscription_validator_spec.rb'
-    - 'spec/services/subscription_variants_service_spec.rb'
-    - 'spec/services/subscriptions_count_spec.rb'
     - 'spec/services/tax_rate_finder_spec.rb'
     - 'spec/services/upload_sanitizer_spec.rb'
     - 'spec/services/variants_stock_levels_spec.rb'
diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb
index 14d075514a..4a3ad149cb 100644
--- a/app/controllers/admin/order_cycles_controller.rb
+++ b/app/controllers/admin/order_cycles_controller.rb
@@ -16,7 +16,7 @@ module Admin
           render_as_json @collection,
                          ams_prefix: params[:ams_prefix],
                          current_user: spree_current_user,
-                         subscriptions_count: SubscriptionsCount.new(@collection)
+                         subscriptions_count: OrderManagement::Subscriptions::Count.new(@collection)
         end
       end
     end
@@ -74,7 +74,7 @@ module Admin
         render_as_json @order_cycles,
                        ams_prefix: 'index',
                        current_user: spree_current_user,
-                       subscriptions_count: SubscriptionsCount.new(@collection)
+                       subscriptions_count: OrderManagement::Subscriptions::Count.new(@collection)
       else
         order_cycle = order_cycle_set.collection.find{ |oc| oc.errors.present? }
         render json: { errors: order_cycle.errors.full_messages }, status: :unprocessable_entity
diff --git a/app/controllers/admin/subscription_line_items_controller.rb b/app/controllers/admin/subscription_line_items_controller.rb
index b3649696ac..f8e4945c26 100644
--- a/app/controllers/admin/subscription_line_items_controller.rb
+++ b/app/controllers/admin/subscription_line_items_controller.rb
@@ -56,7 +56,7 @@ module Admin
     end
 
     def variant_if_eligible(variant_id)
-      SubscriptionVariantsService.eligible_variants(@shop).find_by_id(variant_id)
+      OrderManagement::Subscriptions::VariantsList.eligible_variants(@shop).find_by_id(variant_id)
     end
   end
 end
diff --git a/app/controllers/admin/subscriptions_controller.rb b/app/controllers/admin/subscriptions_controller.rb
index 0b3952d5a6..867c9d2351 100644
--- a/app/controllers/admin/subscriptions_controller.rb
+++ b/app/controllers/admin/subscriptions_controller.rb
@@ -64,7 +64,7 @@ module Admin
     private
 
     def save_form_and_render(render_issues = true)
-      form = SubscriptionForm.new(@subscription, params[:subscription])
+      form = OrderManagement::Subscriptions::Form.new(@subscription, params[:subscription])
       unless form.save
         render json: { errors: form.json_errors }, status: :unprocessable_entity
         return
diff --git a/app/serializers/api/admin/subscription_line_item_serializer.rb b/app/serializers/api/admin/subscription_line_item_serializer.rb
index 34bc00c6c0..f1411e040f 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
-        SubscriptionVariantsService.in_open_and_upcoming_order_cycles?(option_or_assigned_shop,
-                                                                       option_or_assigned_schedule,
-                                                                       object.variant)
+        OrderManagement::Subscriptions::VariantsList.in_open_and_upcoming_order_cycles?(option_or_assigned_shop,
+                                                                                        option_or_assigned_schedule,
+                                                                                        object.variant)
       end
 
       private
diff --git a/app/services/subscription_estimator.rb b/app/services/subscription_estimator.rb
deleted file mode 100644
index 6e4d7b0938..0000000000
--- a/app/services/subscription_estimator.rb
+++ /dev/null
@@ -1,63 +0,0 @@
-require 'open_food_network/scope_variant_to_hub'
-
-# Responsible for estimating prices and fees for subscriptions
-# Used by SubscriptionForm as part of the create/update process
-# The values calculated here are intended to be persisted in the db
-
-class SubscriptionEstimator
-  def initialize(subscription)
-    @subscription = subscription
-  end
-
-  def estimate!
-    assign_price_estimates
-    assign_fee_estimates
-  end
-
-  private
-
-  attr_accessor :subscription
-
-  delegate :subscription_line_items, :shipping_method, :payment_method, :shop, to: :subscription
-
-  def assign_price_estimates
-    subscription_line_items.each do |item|
-      item.price_estimate =
-        price_estimate_for(item.variant, item.price_estimate_was)
-    end
-  end
-
-  def price_estimate_for(variant, fallback)
-    return fallback unless fee_calculator && variant
-
-    scoper.scope(variant)
-    fees = fee_calculator.indexed_fees_for(variant)
-    (variant.price + fees).to_d
-  end
-
-  def fee_calculator
-    return @fee_calculator unless @fee_calculator.nil?
-
-    next_oc = subscription.schedule.andand.current_or_next_order_cycle
-    return nil unless shop && next_oc
-
-    @fee_calculator = OpenFoodNetwork::EnterpriseFeeCalculator.new(shop, next_oc)
-  end
-
-  def scoper
-    OpenFoodNetwork::ScopeVariantToHub.new(shop)
-  end
-
-  def assign_fee_estimates
-    subscription.shipping_fee_estimate = shipping_fee_estimate
-    subscription.payment_fee_estimate = payment_fee_estimate
-  end
-
-  def shipping_fee_estimate
-    shipping_method.calculator.compute(subscription)
-  end
-
-  def payment_fee_estimate
-    payment_method.calculator.compute(subscription)
-  end
-end
diff --git a/app/services/subscription_form.rb b/app/services/subscription_form.rb
deleted file mode 100644
index fa33b216b2..0000000000
--- a/app/services/subscription_form.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-require 'order_management/subscriptions/proxy_order_syncer'
-
-class SubscriptionForm
-  attr_accessor :subscription, :params, :order_update_issues, :validator, :order_syncer, :estimator
-
-  delegate :json_errors, :valid?, to: :validator
-  delegate :order_update_issues, to: :order_syncer
-
-  def initialize(subscription, params = {})
-    @subscription = subscription
-    @params = params
-    @estimator = SubscriptionEstimator.new(subscription)
-    @validator = SubscriptionValidator.new(subscription)
-    @order_syncer = OrderSyncer.new(subscription)
-  end
-
-  def save
-    subscription.assign_attributes(params)
-    return false unless valid?
-
-    subscription.transaction do
-      estimator.estimate!
-      proxy_order_syncer.sync!
-      order_syncer.sync!
-      subscription.save!
-    end
-  end
-
-  private
-
-  def proxy_order_syncer
-    OrderManagement::Subscriptions::ProxyOrderSyncer.new(subscription)
-  end
-end
diff --git a/app/services/subscription_validator.rb b/app/services/subscription_validator.rb
deleted file mode 100644
index 4cce8a3af3..0000000000
--- a/app/services/subscription_validator.rb
+++ /dev/null
@@ -1,127 +0,0 @@
-# Encapsulation of all of the validation logic required for subscriptions
-# Public interface consists of #valid? method provided by ActiveModel::Validations
-# and #json_errors which compiles a serializable hash of errors
-
-class SubscriptionValidator
-  include ActiveModel::Naming
-  include ActiveModel::Conversion
-  include ActiveModel::Validations
-
-  attr_reader :subscription
-
-  validates :shop, :customer, :schedule, :shipping_method, :payment_method, presence: true
-  validates :bill_address, :ship_address, :begins_at, presence: true
-  validate :shipping_method_allowed?
-  validate :payment_method_allowed?
-  validate :payment_method_type_allowed?
-  validate :ends_at_after_begins_at?
-  validate :customer_allowed?
-  validate :schedule_allowed?
-  validate :credit_card_ok?
-  validate :subscription_line_items_present?
-  validate :requested_variants_available?
-
-  delegate :shop, :customer, :schedule, :shipping_method, :payment_method, to: :subscription
-  delegate :bill_address, :ship_address, :begins_at, :ends_at, to: :subscription
-  delegate :subscription_line_items, to: :subscription
-
-  def initialize(subscription)
-    @subscription = subscription
-  end
-
-  def json_errors
-    errors.messages.each_with_object({}) do |(k, v), errors|
-      errors[k] = v.map { |msg| build_msg_from(k, msg) }
-    end
-  end
-
-  private
-
-  def shipping_method_allowed?
-    return unless shipping_method
-    return if shipping_method.distributors.include?(shop)
-
-    errors.add(:shipping_method, :not_available_to_shop, shop: shop.name)
-  end
-
-  def payment_method_allowed?
-    return unless payment_method
-    return if payment_method.distributors.include?(shop)
-
-    errors.add(:payment_method, :not_available_to_shop, shop: shop.name)
-  end
-
-  def payment_method_type_allowed?
-    return unless payment_method
-    return if Subscription::ALLOWED_PAYMENT_METHOD_TYPES.include? payment_method.type
-
-    errors.add(:payment_method, :invalid_type)
-  end
-
-  def ends_at_after_begins_at?
-    # Only validates ends_at if it is present
-    return if begins_at.blank? || ends_at.blank?
-    return if ends_at > begins_at
-
-    errors.add(:ends_at, :after_begins_at)
-  end
-
-  def customer_allowed?
-    return unless customer
-    return if customer.enterprise == shop
-
-    errors.add(:customer, :does_not_belong_to_shop, shop: shop.name)
-  end
-
-  def schedule_allowed?
-    return unless schedule
-    return if schedule.coordinators.include?(shop)
-
-    errors.add(:schedule, :not_coordinated_by_shop, shop: shop.name)
-  end
-
-  def credit_card_ok?
-    return unless customer && payment_method
-    return unless stripe_payment_method?(payment_method)
-    return errors.add(:payment_method, :charges_not_allowed) unless customer.allow_charges
-    return if customer.user.andand.default_card.present?
-
-    errors.add(:payment_method, :no_default_card)
-  end
-
-  def stripe_payment_method?(payment_method)
-    payment_method.type == "Spree::Gateway::StripeConnect" ||
-      payment_method.type == "Spree::Gateway::StripeSCA"
-  end
-
-  def subscription_line_items_present?
-    return if subscription_line_items.reject(&:marked_for_destruction?).any?
-
-    errors.add(:subscription_line_items, :at_least_one_product)
-  end
-
-  def requested_variants_available?
-    subscription_line_items.each { |sli| verify_availability_of(sli.variant) }
-  end
-
-  def verify_availability_of(variant)
-    return if available_variant_ids.include? variant.id
-
-    name = "#{variant.product.name} - #{variant.full_name}"
-    errors.add(:subscription_line_items, :not_available, name: name)
-  end
-
-  def available_variant_ids
-    return @available_variant_ids if @available_variant_ids.present?
-
-    subscription_variant_ids = subscription_line_items.map(&:variant_id)
-    @available_variant_ids = SubscriptionVariantsService.eligible_variants(shop)
-      .where(id: subscription_variant_ids).pluck(:id)
-  end
-
-  def build_msg_from(k, msg)
-    return msg[1..-1] if msg.starts_with?("^")
-
-    errors.full_message(k, msg)
-  end
-end
diff --git a/app/services/subscription_variants_service.rb b/app/services/subscription_variants_service.rb
deleted file mode 100644
index fbdef71e56..0000000000
--- a/app/services/subscription_variants_service.rb
+++ /dev/null
@@ -1,39 +0,0 @@
-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)
-    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 << 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
-
-  def self.permitted_producer_ids(distributor)
-    other_permitted_producer_ids = EnterpriseRelationship.joins(:parent)
-      .permitting(distributor.id).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
diff --git a/app/services/subscriptions_count.rb b/app/services/subscriptions_count.rb
deleted file mode 100644
index ee1126c0d2..0000000000
--- a/app/services/subscriptions_count.rb
+++ /dev/null
@@ -1,20 +0,0 @@
-class SubscriptionsCount
-  def initialize(order_cycles)
-    @order_cycles = order_cycles
-  end
-
-  def for(order_cycle_id)
-    active[order_cycle_id] || 0
-  end
-
-  private
-
-  attr_accessor :order_cycles
-
-  def active
-    return @active unless @active.nil?
-    return @active = [] if order_cycles.blank?
-
-    @active ||= ProxyOrder.not_canceled.group(:order_cycle_id).where(order_cycle_id: order_cycles).count
-  end
-end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 7b9c07620f..8f2e0b53b1 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -74,7 +74,7 @@ en:
       messages:
         inclusion: "is not included in the list"
       models:
-        subscription_validator:
+        order_management/subscriptions/validator:          
           attributes:
             subscription_line_items:
               at_least_one_product: "^Please add at least one product"
diff --git a/engines/order_management/app/services/order_management/subscriptions/count.rb b/engines/order_management/app/services/order_management/subscriptions/count.rb
new file mode 100644
index 0000000000..80f066d209
--- /dev/null
+++ b/engines/order_management/app/services/order_management/subscriptions/count.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module OrderManagement
+  module Subscriptions
+    class Count
+      def initialize(order_cycles)
+        @order_cycles = order_cycles
+      end
+
+      def for(order_cycle_id)
+        active[order_cycle_id] || 0
+      end
+
+      private
+
+      attr_accessor :order_cycles
+
+      def active
+        return @active unless @active.nil?
+        return @active = [] if order_cycles.blank?
+
+        @active ||= ProxyOrder.
+          not_canceled.
+          group(:order_cycle_id).
+          where(order_cycle_id: order_cycles).
+          count
+      end
+    end
+  end
+end
diff --git a/engines/order_management/app/services/order_management/subscriptions/estimator.rb b/engines/order_management/app/services/order_management/subscriptions/estimator.rb
new file mode 100644
index 0000000000..e358e031bf
--- /dev/null
+++ b/engines/order_management/app/services/order_management/subscriptions/estimator.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+require 'open_food_network/scope_variant_to_hub'
+
+# Responsible for estimating prices and fees for subscriptions
+# Used by Form as part of the create/update process
+# The values calculated here are intended to be persisted in the db
+
+module OrderManagement
+  module Subscriptions
+    class Estimator
+      def initialize(subscription)
+        @subscription = subscription
+      end
+
+      def estimate!
+        assign_price_estimates
+        assign_fee_estimates
+      end
+
+      private
+
+      attr_accessor :subscription
+
+      delegate :subscription_line_items, :shipping_method, :payment_method, :shop, to: :subscription
+
+      def assign_price_estimates
+        subscription_line_items.each do |item|
+          item.price_estimate =
+            price_estimate_for(item.variant, item.price_estimate_was)
+        end
+      end
+
+      def price_estimate_for(variant, fallback)
+        return fallback unless fee_calculator && variant
+
+        scoper.scope(variant)
+        fees = fee_calculator.indexed_fees_for(variant)
+        (variant.price + fees).to_d
+      end
+
+      def fee_calculator
+        return @fee_calculator unless @fee_calculator.nil?
+
+        next_oc = subscription.schedule.andand.current_or_next_order_cycle
+        return nil unless shop && next_oc
+
+        @fee_calculator = OpenFoodNetwork::EnterpriseFeeCalculator.new(shop, next_oc)
+      end
+
+      def scoper
+        OpenFoodNetwork::ScopeVariantToHub.new(shop)
+      end
+
+      def assign_fee_estimates
+        subscription.shipping_fee_estimate = shipping_fee_estimate
+        subscription.payment_fee_estimate = payment_fee_estimate
+      end
+
+      def shipping_fee_estimate
+        shipping_method.calculator.compute(subscription)
+      end
+
+      def payment_fee_estimate
+        payment_method.calculator.compute(subscription)
+      end
+    end
+  end
+end
diff --git a/engines/order_management/app/services/order_management/subscriptions/form.rb b/engines/order_management/app/services/order_management/subscriptions/form.rb
new file mode 100644
index 0000000000..02994e3b50
--- /dev/null
+++ b/engines/order_management/app/services/order_management/subscriptions/form.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require 'order_management/subscriptions/proxy_order_syncer'
+
+module OrderManagement
+  module Subscriptions
+    class Form
+      attr_accessor :subscription, :params, :order_update_issues,
+                    :validator, :order_syncer, :estimator
+
+      delegate :json_errors, :valid?, to: :validator
+      delegate :order_update_issues, to: :order_syncer
+
+      def initialize(subscription, params = {})
+        @subscription = subscription
+        @params = params
+        @estimator = OrderManagement::Subscriptions::Estimator.new(subscription)
+        @validator = OrderManagement::Subscriptions::Validator.new(subscription)
+        @order_syncer = OrderSyncer.new(subscription)
+      end
+
+      def save
+        subscription.assign_attributes(params)
+        return false unless valid?
+
+        subscription.transaction do
+          estimator.estimate!
+          proxy_order_syncer.sync!
+          order_syncer.sync!
+          subscription.save!
+        end
+      end
+
+      private
+
+      def proxy_order_syncer
+        OrderManagement::Subscriptions::ProxyOrderSyncer.new(subscription)
+      end
+    end
+  end
+end
diff --git a/engines/order_management/app/services/order_management/subscriptions/validator.rb b/engines/order_management/app/services/order_management/subscriptions/validator.rb
new file mode 100644
index 0000000000..a8bc6b3086
--- /dev/null
+++ b/engines/order_management/app/services/order_management/subscriptions/validator.rb
@@ -0,0 +1,132 @@
+# frozen_string_literal: true
+
+# Encapsulation of all of the validation logic required for subscriptions
+# Public interface consists of #valid? method provided by ActiveModel::Validations
+# and #json_errors which compiles a serializable hash of errors
+module OrderManagement
+  module Subscriptions
+    class Validator
+      include ActiveModel::Naming
+      include ActiveModel::Conversion
+      include ActiveModel::Validations
+
+      attr_reader :subscription
+
+      validates :shop, :customer, :schedule, :shipping_method, :payment_method, presence: true
+      validates :bill_address, :ship_address, :begins_at, presence: true
+      validate :shipping_method_allowed?
+      validate :payment_method_allowed?
+      validate :payment_method_type_allowed?
+      validate :ends_at_after_begins_at?
+      validate :customer_allowed?
+      validate :schedule_allowed?
+      validate :credit_card_ok?
+      validate :subscription_line_items_present?
+      validate :requested_variants_available?
+
+      delegate :shop, :customer, :schedule, :shipping_method, :payment_method, to: :subscription
+      delegate :bill_address, :ship_address, :begins_at, :ends_at, to: :subscription
+      delegate :subscription_line_items, to: :subscription
+
+      def initialize(subscription)
+        @subscription = subscription
+      end
+
+      def json_errors
+        errors.messages.each_with_object({}) do |(key, value), errors|
+          errors[key] = value.map { |msg| build_msg_from(key, msg) }
+        end
+      end
+
+      private
+
+      def shipping_method_allowed?
+        return unless shipping_method
+        return if shipping_method.distributors.include?(shop)
+
+        errors.add(:shipping_method, :not_available_to_shop, shop: shop.name)
+      end
+
+      def payment_method_allowed?
+        return unless payment_method
+        return if payment_method.distributors.include?(shop)
+
+        errors.add(:payment_method, :not_available_to_shop, shop: shop.name)
+      end
+
+      def payment_method_type_allowed?
+        return unless payment_method
+        return if Subscription::ALLOWED_PAYMENT_METHOD_TYPES.include? payment_method.type
+
+        errors.add(:payment_method, :invalid_type)
+      end
+
+      def ends_at_after_begins_at?
+        # Only validates ends_at if it is present
+        return if begins_at.blank? || ends_at.blank?
+        return if ends_at > begins_at
+
+        errors.add(:ends_at, :after_begins_at)
+      end
+
+      def customer_allowed?
+        return unless customer
+        return if customer.enterprise == shop
+
+        errors.add(:customer, :does_not_belong_to_shop, shop: shop.name)
+      end
+
+      def schedule_allowed?
+        return unless schedule
+        return if schedule.coordinators.include?(shop)
+
+        errors.add(:schedule, :not_coordinated_by_shop, shop: shop.name)
+      end
+
+      def credit_card_ok?
+        return unless customer && payment_method
+        return unless stripe_payment_method?(payment_method)
+        return errors.add(:payment_method, :charges_not_allowed) unless customer.allow_charges
+        return if customer.user.andand.default_card.present?
+
+        errors.add(:payment_method, :no_default_card)
+      end
+
+      def stripe_payment_method?(payment_method)
+        payment_method.type == "Spree::Gateway::StripeConnect" ||
+          payment_method.type == "Spree::Gateway::StripeSCA"
+      end
+
+      def subscription_line_items_present?
+        return if subscription_line_items.reject(&:marked_for_destruction?).any?
+
+        errors.add(:subscription_line_items, :at_least_one_product)
+      end
+
+      def requested_variants_available?
+        subscription_line_items.each { |sli| verify_availability_of(sli.variant) }
+      end
+
+      def verify_availability_of(variant)
+        return if available_variant_ids.include? variant.id
+
+        name = "#{variant.product.name} - #{variant.full_name}"
+        errors.add(:subscription_line_items, :not_available, name: name)
+      end
+
+      def available_variant_ids
+        return @available_variant_ids if @available_variant_ids.present?
+
+        subscription_variant_ids = subscription_line_items.map(&:variant_id)
+        @available_variant_ids = OrderManagement::Subscriptions::VariantsList.eligible_variants(shop)
+          .where(id: subscription_variant_ids).pluck(:id)
+      end
+
+      def build_msg_from(key, msg)
+        return msg[1..-1] if msg.starts_with?("^")
+
+        errors.full_message(key, msg)
+      end
+    end
+  end
+end
diff --git a/engines/order_management/app/services/order_management/subscriptions/variants_list.rb b/engines/order_management/app/services/order_management/subscriptions/variants_list.rb
new file mode 100644
index 0000000000..5c5a3fddb3
--- /dev/null
+++ b/engines/order_management/app/services/order_management/subscriptions/variants_list.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: false
+
+module OrderManagement
+  module Subscriptions
+    class VariantsList
+      # 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)
+        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 << 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
+
+      def self.permitted_producer_ids(distributor)
+        other_permitted_producer_ids = EnterpriseRelationship.joins(:parent)
+          .permitting(distributor.id).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
+  end
+end
diff --git a/engines/order_management/spec/services/order_management/subscriptions/count_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/count_spec.rb
new file mode 100644
index 0000000000..14b5a8c041
--- /dev/null
+++ b/engines/order_management/spec/services/order_management/subscriptions/count_spec.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module OrderManagement
+  module Subscriptions
+    describe Count do
+      let(:oc1) { create(:simple_order_cycle) }
+      let(:oc2) { create(:simple_order_cycle) }
+      let(:subscriptions_count) { Count.new(order_cycles) }
+
+      describe "#for" do
+        context "when the collection has not been set" do
+          let(:order_cycles) { nil }
+          it "returns 0" do
+            expect(subscriptions_count.for(oc1.id)).to eq 0
+          end
+        end
+
+        context "when the collection has been set" do
+          let(:order_cycles) { OrderCycle.where(id: [oc1]) }
+          let!(:po1) { create(:proxy_order, order_cycle: oc1) }
+          let!(:po2) { create(:proxy_order, order_cycle: oc1) }
+          let!(:po3) { create(:proxy_order, order_cycle: oc2) }
+
+          context "but the requested id is not present in the list of order cycles provided" do
+            it "returns 0" do
+              # Note that po3 applies to oc2, but oc2 in not in the collection
+              expect(subscriptions_count.for(oc2.id)).to eq 0
+            end
+          end
+
+          context "and the requested id is present in the list of order cycles provided" do
+            it "returns a count of active proxy orders associated with the requested order cycle" do
+              expect(subscriptions_count.for(oc1.id)).to eq 2
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb
new file mode 100644
index 0000000000..78020f6578
--- /dev/null
+++ b/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb
@@ -0,0 +1,135 @@
+# frozen_string_literal: true
+
+module OrderManagement
+  module Subscriptions
+    describe Estimator do
+      describe "estimating prices for subscription line items" do
+        let!(:subscription) { create(:subscription, with_items: true) }
+        let!(:sli1) { subscription.subscription_line_items.first }
+        let!(:sli2) { subscription.subscription_line_items.second }
+        let!(:sli3) { subscription.subscription_line_items.third }
+        let(:estimator) { Estimator.new(subscription) }
+
+        before do
+          sli1.update_attributes(price_estimate: 4.0)
+          sli2.update_attributes(price_estimate: 5.0)
+          sli3.update_attributes(price_estimate: 6.0)
+          sli1.variant.update_attributes(price: 1.0)
+          sli2.variant.update_attributes(price: 2.0)
+          sli3.variant.update_attributes(price: 3.0)
+
+          # Simulating assignment of attrs from params
+          sli1.assign_attributes(price_estimate: 7.0)
+          sli2.assign_attributes(price_estimate: 8.0)
+          sli3.assign_attributes(price_estimate: 9.0)
+        end
+
+        context "when a insufficient information exists to calculate price estimates" do
+          before do
+            # This might be because a shop has not been assigned yet, or no
+            # current or future order cycles exist for the schedule
+            allow(estimator).to receive(:fee_calculator) { nil }
+          end
+
+          it "resets the price estimates for all items" do
+            estimator.estimate!
+            expect(sli1.price_estimate).to eq 4.0
+            expect(sli2.price_estimate).to eq 5.0
+            expect(sli3.price_estimate).to eq 6.0
+          end
+        end
+
+        context "when sufficient information to calculate price estimates exists" do
+          let(:fee_calculator) { instance_double(OpenFoodNetwork::EnterpriseFeeCalculator) }
+
+          before do
+            allow(estimator).to receive(:fee_calculator) { fee_calculator }
+            allow(fee_calculator).to receive(:indexed_fees_for).with(sli1.variant) { 1.0 }
+            allow(fee_calculator).to receive(:indexed_fees_for).with(sli2.variant) { 0.0 }
+            allow(fee_calculator).to receive(:indexed_fees_for).with(sli3.variant) { 3.0 }
+          end
+
+          context "when no variant overrides apply" do
+            it "recalculates price_estimates based on variant prices and associated fees" do
+              estimator.estimate!
+              expect(sli1.price_estimate).to eq 2.0
+              expect(sli2.price_estimate).to eq 2.0
+              expect(sli3.price_estimate).to eq 6.0
+            end
+          end
+
+          context "when variant overrides apply" do
+            let!(:override1) { create(:variant_override, hub: subscription.shop, variant: sli1.variant, price: 1.2) }
+            let!(:override2) { create(:variant_override, hub: subscription.shop, variant: sli2.variant, price: 2.3) }
+
+            it "recalculates price_estimates based on override prices and associated fees" do
+              estimator.estimate!
+              expect(sli1.price_estimate).to eq 2.2
+              expect(sli2.price_estimate).to eq 2.3
+              expect(sli3.price_estimate).to eq 6.0
+            end
+          end
+        end
+      end
+
+      describe "updating estimates for shipping and payment fees" do
+        let(:subscription) { create(:subscription, with_items: true, payment_method: payment_method, shipping_method: shipping_method) }
+        let!(:sli1) { subscription.subscription_line_items.first }
+        let!(:sli2) { subscription.subscription_line_items.second }
+        let!(:sli3) { subscription.subscription_line_items.third }
+        let(:estimator) { OrderManagement::Subscriptions::Estimator.new(subscription) }
+
+        before do
+          allow(estimator).to receive(:assign_price_estimates)
+          sli1.update_attributes(price_estimate: 4.0)
+          sli2.update_attributes(price_estimate: 5.0)
+          sli3.update_attributes(price_estimate: 6.0)
+        end
+
+        context "using flat rate calculators" do
+          let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 12.34)) }
+          let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 9.12)) }
+
+          it "calculates fees based on the rates provided" do
+            estimator.estimate!
+            expect(subscription.shipping_fee_estimate.to_f).to eq 12.34
+            expect(subscription.payment_fee_estimate.to_f).to eq 9.12
+          end
+        end
+
+        context "using flat percent item total calculators" do
+          let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10)) }
+          let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 20)) }
+
+          it "calculates fees based on the estimated item total and percentage provided" do
+            estimator.estimate!
+            expect(subscription.shipping_fee_estimate.to_f).to eq 1.5
+            expect(subscription.payment_fee_estimate.to_f).to eq 3.0
+          end
+        end
+
+        context "using flat percent per item calculators" do
+          let(:shipping_method) { create(:shipping_method, calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 5)) }
+          let(:payment_method) { create(:payment_method, calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 10)) }
+
+          it "calculates fees based on the estimated item prices and percentage provided" do
+            estimator.estimate!
+            expect(subscription.shipping_fee_estimate.to_f).to eq 0.75
+            expect(subscription.payment_fee_estimate.to_f).to eq 1.5
+          end
+        end
+
+        context "using per item calculators" do
+          let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::PerItem.new(preferred_amount: 1.2)) }
+          let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::PerItem.new(preferred_amount: 0.3)) }
+
+          it "calculates fees based on the number of items and rate provided" do
+            estimator.estimate!
+            expect(subscription.shipping_fee_estimate.to_f).to eq 3.6
+            expect(subscription.payment_fee_estimate.to_f).to eq 0.9
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb
new file mode 100644
index 0000000000..f9c0798671
--- /dev/null
+++ b/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb
@@ -0,0 +1,103 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+module OrderManagement
+  module Subscriptions
+    describe Form do
+      describe "creating a new subscription" do
+        let!(:shop) { create(:distributor_enterprise) }
+        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: product2, unit_value: '1000', price: 2.50, option_values: [], on_hand: 1) }
+        let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) }
+        let!(:order_cycle1) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 9.days.ago, orders_close_at: 2.days.ago) }
+        let!(:order_cycle2) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.ago, orders_close_at: 5.days.from_now) }
+        let!(:order_cycle3) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 5.days.from_now, orders_close_at: 12.days.from_now) }
+        let!(:order_cycle4) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 12.days.from_now, orders_close_at: 19.days.from_now) }
+        let!(:outgoing_exchange1) { order_cycle1.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) }
+        let!(:outgoing_exchange2) { order_cycle2.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) }
+        let!(:outgoing_exchange3) { order_cycle3.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant3], enterprise_fees: []) }
+        let!(:outgoing_exchange4) { order_cycle4.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) }
+        let!(:schedule) { create(:schedule, order_cycles: [order_cycle1, order_cycle2, order_cycle3, order_cycle4]) }
+        let!(:payment_method) { create(:payment_method, distributors: [shop]) }
+        let!(:shipping_method) { create(:shipping_method, distributors: [shop]) }
+        let!(:address) { create(:address) }
+        let(:subscription) { Subscription.new }
+
+        let!(:params) {
+          {
+            shop_id: shop.id,
+            customer_id: customer.id,
+            schedule_id: schedule.id,
+            bill_address_attributes: address.clone.attributes,
+            ship_address_attributes: address.clone.attributes,
+            payment_method_id: payment_method.id,
+            shipping_method_id: shipping_method.id,
+            begins_at: 4.days.ago,
+            ends_at: 14.days.from_now,
+            subscription_line_items_attributes: [
+              { variant_id: variant1.id, quantity: 1, price_estimate: 7.0 },
+              { variant_id: variant2.id, quantity: 2, price_estimate: 8.0 },
+              { variant_id: variant3.id, quantity: 3, price_estimate: 9.0 }
+            ]
+          }
+        }
+
+        let(:form) { OrderManagement::Subscriptions::Form.new(subscription, params) }
+
+        it "creates orders for each order cycle in the schedule" do
+          expect(form.save).to be true
+
+          expect(subscription.proxy_orders.count).to be 2
+          expect(subscription.subscription_line_items.count).to be 3
+          expect(subscription.subscription_line_items[0].price_estimate).to eq 13.75
+          expect(subscription.subscription_line_items[1].price_estimate).to eq 7.75
+          expect(subscription.subscription_line_items[2].price_estimate).to eq 4.25
+
+          # This order cycle has already closed, so no order is initialized
+          proxy_order1 = subscription.proxy_orders.find_by_order_cycle_id(order_cycle1.id)
+          expect(proxy_order1).to be nil
+
+          # Currently open order cycle, closing after begins_at and before ends_at
+          proxy_order2 = subscription.proxy_orders.find_by_order_cycle_id(order_cycle2.id)
+          expect(proxy_order2).to be_a ProxyOrder
+          order2 = proxy_order2.initialise_order!
+          expect(order2.line_items.count).to eq 3
+          expect(order2.line_items.find_by_variant_id(variant3.id).quantity).to be 3
+          expect(order2.shipments.count).to eq 1
+          expect(order2.shipments.first.shipping_method).to eq shipping_method
+          expect(order2.payments.count).to eq 1
+          expect(order2.payments.first.payment_method).to eq payment_method
+          expect(order2.payments.first.state).to eq 'checkout'
+          expect(order2.total).to eq 42
+          expect(order2.completed?).to be false
+
+          # Future order cycle, closing after begins_at and before ends_at
+          # Adds line items for variants that aren't yet available from the order cycle
+          proxy_order3 = subscription.proxy_orders.find_by_order_cycle_id(order_cycle3.id)
+          expect(proxy_order3).to be_a ProxyOrder
+          order3 = proxy_order3.initialise_order!
+          expect(order3).to be_a Spree::Order
+          expect(order3.line_items.count).to eq 3
+          expect(order2.line_items.find_by_variant_id(variant3.id).quantity).to be 3
+          expect(order3.shipments.count).to eq 1
+          expect(order3.shipments.first.shipping_method).to eq shipping_method
+          expect(order3.payments.count).to eq 1
+          expect(order3.payments.first.payment_method).to eq payment_method
+          expect(order3.payments.first.state).to eq 'checkout'
+          expect(order3.total).to eq 31.50
+          expect(order3.completed?).to be false
+
+          # Future order cycle closing after ends_at
+          proxy_order4 = subscription.proxy_orders.find_by_order_cycle_id(order_cycle4.id)
+          expect(proxy_order4).to be nil
+        end
+      end
+    end
+  end
+end
diff --git a/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb
new file mode 100644
index 0000000000..a072262b47
--- /dev/null
+++ b/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb
@@ -0,0 +1,476 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+module OrderManagement
+  module Subscriptions
+    describe Validator do
+      let(:owner) { create(:user) }
+      let(:shop) { create(:enterprise, name: "Shop", owner: owner) }
+
+      describe "delegation" do
+        let(:subscription) { create(:subscription, shop: shop) }
+        let(:validator) { Validator.new(subscription) }
+
+        it "delegates to subscription" do
+          expect(validator.shop).to eq subscription.shop
+          expect(validator.customer).to eq subscription.customer
+          expect(validator.schedule).to eq subscription.schedule
+          expect(validator.shipping_method).to eq subscription.shipping_method
+          expect(validator.payment_method).to eq subscription.payment_method
+          expect(validator.bill_address).to eq subscription.bill_address
+          expect(validator.ship_address).to eq subscription.ship_address
+          expect(validator.begins_at).to eq subscription.begins_at
+          expect(validator.ends_at).to eq subscription.ends_at
+        end
+      end
+
+      describe "validations" do
+        let(:subscription_stubs) do
+          {
+            shop: shop,
+            customer: true,
+            schedule: true,
+            shipping_method: true,
+            payment_method: true,
+            bill_address: true,
+            ship_address: true,
+            begins_at: true,
+            ends_at: true,
+          }
+        end
+
+        let(:validation_stubs) do
+          {
+            shipping_method_allowed?: true,
+            payment_method_allowed?: true,
+            payment_method_type_allowed?: true,
+            ends_at_after_begins_at?: true,
+            customer_allowed?: true,
+            schedule_allowed?: true,
+            credit_card_ok?: true,
+            subscription_line_items_present?: true,
+            requested_variants_available?: true
+          }
+        end
+
+        let(:subscription) { instance_double(Subscription, subscription_stubs) }
+        let(:validator) { OrderManagement::Subscriptions::Validator.new(subscription) }
+
+        def stub_validations(validator, methods)
+          methods.each do |name, value|
+            allow(validator).to receive(name) { value }
+          end
+        end
+
+        describe "shipping method validation" do
+          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:shipping_method)) }
+          before { stub_validations(validator, validation_stubs.except(:shipping_method_allowed?)) }
+
+          context "when no shipping method is present" do
+            before { expect(subscription).to receive(:shipping_method).at_least(:once) { nil } }
+
+            it "adds an error and returns false" do
+              expect(validator.valid?).to be false
+              expect(validator.errors[:shipping_method]).to_not be_empty
+            end
+          end
+
+          context "when a shipping method is present" do
+            let(:shipping_method) { instance_double(Spree::ShippingMethod, distributors: [shop]) }
+            before { expect(subscription).to receive(:shipping_method).at_least(:once) { shipping_method } }
+
+            context "and the shipping method is not associated with the shop" do
+              before { allow(shipping_method).to receive(:distributors) { [double(:enterprise)] } }
+
+              it "adds an error and returns false" do
+                expect(validator.valid?).to be false
+                expect(validator.errors[:shipping_method]).to_not be_empty
+              end
+            end
+
+            context "and the shipping method is associated with the shop" do
+              before { allow(shipping_method).to receive(:distributors) { [shop] } }
+
+              it "returns true" do
+                expect(validator.valid?).to be true
+                expect(validator.errors[:shipping_method]).to be_empty
+              end
+            end
+          end
+        end
+
+        describe "payment method validation" do
+          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) }
+          before { stub_validations(validator, validation_stubs.except(:payment_method_allowed?)) }
+
+          context "when no payment method is present" do
+            before { expect(subscription).to receive(:payment_method).at_least(:once) { nil } }
+
+            it "adds an error and returns false" do
+              expect(validator.valid?).to be false
+              expect(validator.errors[:payment_method]).to_not be_empty
+            end
+          end
+
+          context "when a payment method is present" do
+            let(:payment_method) { instance_double(Spree::PaymentMethod, distributors: [shop]) }
+            before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } }
+
+            context "and the payment method is not associated with the shop" do
+              before { allow(payment_method).to receive(:distributors) { [double(:enterprise)] } }
+
+              it "adds an error and returns false" do
+                expect(validator.valid?).to be false
+                expect(validator.errors[:payment_method]).to_not be_empty
+              end
+            end
+
+            context "and the payment method is associated with the shop" do
+              before { allow(payment_method).to receive(:distributors) { [shop] } }
+
+              it "returns true" do
+                expect(validator.valid?).to be true
+                expect(validator.errors[:payment_method]).to be_empty
+              end
+            end
+          end
+        end
+
+        describe "payment method type validation" do
+          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) }
+          before { stub_validations(validator, validation_stubs.except(:payment_method_type_allowed?)) }
+
+          context "when a payment method is present" do
+            let(:payment_method) { instance_double(Spree::PaymentMethod, distributors: [shop]) }
+            before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } }
+
+            context "and the payment method type is not in the approved list" do
+              before { allow(payment_method).to receive(:type) { "Blah" } }
+
+              it "adds an error and returns false" do
+                expect(validator.valid?).to be false
+                expect(validator.errors[:payment_method]).to_not be_empty
+              end
+            end
+
+            context "and the payment method is in the approved list" do
+              let(:approved_type) { Subscription::ALLOWED_PAYMENT_METHOD_TYPES.first }
+              before { allow(payment_method).to receive(:type) { approved_type } }
+
+              it "returns true" do
+                expect(validator.valid?).to be true
+                expect(validator.errors[:payment_method]).to be_empty
+              end
+            end
+          end
+        end
+
+        describe "dates" do
+          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:begins_at, :ends_at)) }
+          before { stub_validations(validator, validation_stubs.except(:ends_at_after_begins_at?)) }
+          before { expect(subscription).to receive(:begins_at).at_least(:once) { begins_at } }
+
+          context "when no begins_at is present" do
+            let(:begins_at) { nil }
+
+            it "adds an error and returns false" do
+              expect(validator.valid?).to be false
+              expect(validator.errors[:begins_at]).to_not be_empty
+            end
+          end
+
+          context "when a start date is present" do
+            let(:begins_at) { Time.zone.today }
+            before { expect(subscription).to receive(:ends_at).at_least(:once) { ends_at } }
+
+            context "when no ends_at is present" do
+              let(:ends_at) { nil }
+
+              it "returns true" do
+                expect(validator.valid?).to be true
+                expect(validator.errors[:ends_at]).to be_empty
+              end
+            end
+
+            context "when ends_at is equal to begins_at" do
+              let(:ends_at) { Time.zone.today }
+              it "adds an error and returns false" do
+                expect(validator.valid?).to be false
+                expect(validator.errors[:ends_at]).to_not be_empty
+              end
+            end
+
+            context "when ends_at is before begins_at" do
+              let(:ends_at) { Time.zone.today - 1.day }
+              it "adds an error and returns false" do
+                expect(validator.valid?).to be false
+                expect(validator.errors[:ends_at]).to_not be_empty
+              end
+            end
+
+            context "when ends_at is after begins_at" do
+              let(:ends_at) { Time.zone.today + 1.day }
+              it "adds an error and returns false" do
+                expect(validator.valid?).to be true
+                expect(validator.errors[:ends_at]).to be_empty
+              end
+            end
+          end
+        end
+
+        describe "addresses" do
+          before { stub_validations(validator, validation_stubs) }
+          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:bill_address, :ship_address)) }
+          before { expect(subscription).to receive(:bill_address).at_least(:once) { bill_address } }
+          before { expect(subscription).to receive(:ship_address).at_least(:once) { ship_address } }
+
+          context "when bill_address and ship_address are not present" do
+            let(:bill_address) { nil }
+            let(:ship_address) { nil }
+
+            it "adds an error and returns false" do
+              expect(validator.valid?).to be false
+              expect(validator.errors[:bill_address]).to_not be_empty
+              expect(validator.errors[:ship_address]).to_not be_empty
+            end
+          end
+
+          context "when bill_address and ship_address are present" do
+            let(:bill_address) { instance_double(Spree::Address) }
+            let(:ship_address) { instance_double(Spree::Address) }
+
+            it "returns true" do
+              expect(validator.valid?).to be true
+              expect(validator.errors[:bill_address]).to be_empty
+              expect(validator.errors[:ship_address]).to be_empty
+            end
+          end
+        end
+
+        describe "customer" do
+          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:customer)) }
+          before { stub_validations(validator, validation_stubs.except(:customer_allowed?)) }
+          before { expect(subscription).to receive(:customer).at_least(:once) { customer } }
+
+          context "when no customer is present" do
+            let(:customer) { nil }
+
+            it "adds an error and returns false" do
+              expect(validator.valid?).to be false
+              expect(validator.errors[:customer]).to_not be_empty
+            end
+          end
+
+          context "when a customer is present" do
+            let(:customer) { instance_double(Customer) }
+
+            context "and the customer is not associated with the shop" do
+              before { allow(customer).to receive(:enterprise) { double(:enterprise) } }
+
+              it "adds an error and returns false" do
+                expect(validator.valid?).to be false
+                expect(validator.errors[:customer]).to_not be_empty
+              end
+            end
+
+            context "and the customer is associated with the shop" do
+              before { allow(customer).to receive(:enterprise) { shop } }
+
+              it "returns true" do
+                expect(validator.valid?).to be true
+                expect(validator.errors[:customer]).to be_empty
+              end
+            end
+          end
+        end
+
+        describe "schedule" do
+          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:schedule)) }
+          before { stub_validations(validator, validation_stubs.except(:schedule_allowed?)) }
+          before { expect(subscription).to receive(:schedule).at_least(:once) { schedule } }
+
+          context "when no schedule is present" do
+            let(:schedule) { nil }
+
+            it "adds an error and returns false" do
+              expect(validator.valid?).to be false
+              expect(validator.errors[:schedule]).to_not be_empty
+            end
+          end
+
+          context "when a schedule is present" do
+            let(:schedule) { instance_double(Schedule) }
+
+            context "and the schedule is not associated with the shop" do
+              before { allow(schedule).to receive(:coordinators) { [double(:enterprise)] } }
+
+              it "adds an error and returns false" do
+                expect(validator.valid?).to be false
+                expect(validator.errors[:schedule]).to_not be_empty
+              end
+            end
+
+            context "and the schedule is associated with the shop" do
+              before { allow(schedule).to receive(:coordinators) { [shop] } }
+
+              it "returns true" do
+                expect(validator.valid?).to be true
+                expect(validator.errors[:schedule]).to be_empty
+              end
+            end
+          end
+        end
+
+        describe "credit card" do
+          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) }
+          before { stub_validations(validator, validation_stubs.except(:credit_card_ok?)) }
+          before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } }
+
+          context "when using a Check payment method" do
+            let(:payment_method) { instance_double(Spree::PaymentMethod, type: "Spree::PaymentMethod::Check") }
+
+            it "returns true" do
+              expect(validator.valid?).to be true
+              expect(validator.errors[:subscription_line_items]).to be_empty
+            end
+          end
+
+          context "when using the StripeConnect payment gateway" do
+            let(:payment_method) { instance_double(Spree::PaymentMethod, type: "Spree::Gateway::StripeConnect") }
+            before { expect(subscription).to receive(:customer).at_least(:once) { customer } }
+
+            context "when the customer does not allow charges" do
+              let(:customer) { instance_double(Customer, allow_charges: false) }
+
+              it "adds an error and returns false" do
+                expect(validator.valid?).to be false
+                expect(validator.errors[:payment_method]).to_not be_empty
+              end
+            end
+
+            context "when the customer allows charges" do
+              let(:customer) { instance_double(Customer, allow_charges: true) }
+
+              context "and the customer is not associated with a user" do
+                before { allow(customer).to receive(:user) { nil } }
+
+                it "adds an error and returns false" do
+                  expect(validator.valid?).to be false
+                  expect(validator.errors[:payment_method]).to_not be_empty
+                end
+              end
+
+              context "and the customer is associated with a user" do
+                before { expect(customer).to receive(:user).once { user } }
+
+                context "and the user has no default card set" do
+                  let(:user) { instance_double(Spree::User, default_card: nil) }
+
+                  it "adds an error and returns false" do
+                    expect(validator.valid?).to be false
+                    expect(validator.errors[:payment_method]).to_not be_empty
+                  end
+                end
+
+                context "and the user has a default card set" do
+                  let(:user) { instance_double(Spree::User, default_card: 'some card') }
+
+                  it "returns true" do
+                    expect(validator.valid?).to be true
+                    expect(validator.errors[:payment_method]).to be_empty
+                  end
+                end
+              end
+            end
+          end
+        end
+
+        describe "subscription line items" do
+          let(:subscription) { instance_double(Subscription, subscription_stubs) }
+          before { stub_validations(validator, validation_stubs.except(:subscription_line_items_present?)) }
+          before { expect(subscription).to receive(:subscription_line_items).at_least(:once) { subscription_line_items } }
+
+          context "when no subscription line items are present" do
+            let(:subscription_line_items) { [] }
+
+            it "adds an error and returns false" do
+              expect(validator.valid?).to be false
+              expect(validator.errors[:subscription_line_items]).to_not be_empty
+            end
+          end
+
+          context "when subscription line items are present but they are all marked for destruction" do
+            let(:subscription_line_item1) { instance_double(SubscriptionLineItem, marked_for_destruction?: true) }
+            let(:subscription_line_items) { [subscription_line_item1] }
+
+            it "adds an error and returns false" do
+              expect(validator.valid?).to be false
+              expect(validator.errors[:subscription_line_items]).to_not be_empty
+            end
+          end
+
+          context "when subscription line items are present and some and not marked for destruction" do
+            let(:subscription_line_item1) { instance_double(SubscriptionLineItem, marked_for_destruction?: true) }
+            let(:subscription_line_item2) { instance_double(SubscriptionLineItem, marked_for_destruction?: false) }
+            let(:subscription_line_items) { [subscription_line_item1, subscription_line_item2] }
+
+            it "returns true" do
+              expect(validator.valid?).to be true
+              expect(validator.errors[:subscription_line_items]).to be_empty
+            end
+          end
+        end
+
+        describe "variant availability" do
+          let(:subscription) { instance_double(Subscription, subscription_stubs) }
+          before { stub_validations(validator, validation_stubs.except(:requested_variants_available?)) }
+          before { expect(subscription).to receive(:subscription_line_items).at_least(:once) { subscription_line_items } }
+
+          context "when no subscription line items are present" do
+            let(:subscription_line_items) { [] }
+
+            it "returns true" do
+              expect(validator.valid?).to be true
+              expect(validator.errors[:subscription_line_items]).to be_empty
+            end
+          end
+
+          context "when subscription line items are present" do
+            let(:variant1) { instance_double(Spree::Variant, id: 1) }
+            let(:variant2) { instance_double(Spree::Variant, id: 2) }
+            let(:subscription_line_item1) { instance_double(SubscriptionLineItem, variant: variant1) }
+            let(:subscription_line_item2) { instance_double(SubscriptionLineItem, variant: variant2) }
+            let(:subscription_line_items) { [subscription_line_item1] }
+
+            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 }
+                allow(variant1).to receive(:full_name) { "some name" }
+              end
+
+              it "adds an error and returns false" do
+                expect(validator.valid?).to be false
+                expect(validator.errors[:subscription_line_items]).to_not be_empty
+              end
+            end
+
+            context "and all requested variants are available" do
+              before do
+                allow(validator).to receive(:available_variant_ids) { [variant1.id, variant2.id] }
+              end
+
+              it "returns true" do
+                expect(validator.valid?).to be true
+                expect(validator.errors[:subscription_line_items]).to be_empty
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb
new file mode 100644
index 0000000000..8dc97b128e
--- /dev/null
+++ b/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb
@@ -0,0 +1,136 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+module OrderManagement
+  module Subscriptions
+    describe VariantsList 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
+end
diff --git a/lib/open_food_network/scope_variants_for_search.rb b/lib/open_food_network/scope_variants_for_search.rb
index ddf1d48771..5d16eafefe 100644
--- a/lib/open_food_network/scope_variants_for_search.rb
+++ b/lib/open_food_network/scope_variants_for_search.rb
@@ -61,7 +61,7 @@ module OpenFoodNetwork
     end
 
     def scope_to_eligible_for_subscriptions_in_distributor
-      eligible_variants_scope = SubscriptionVariantsService.eligible_variants(distributor)
+      eligible_variants_scope = OrderManagement::Subscriptions::VariantsList.eligible_variants(distributor)
       @variants = @variants.merge(eligible_variants_scope)
       scope_variants_to_distributor(@variants, distributor)
     end
diff --git a/spec/services/subscription_estimator_spec.rb b/spec/services/subscription_estimator_spec.rb
deleted file mode 100644
index 5230057888..0000000000
--- a/spec/services/subscription_estimator_spec.rb
+++ /dev/null
@@ -1,129 +0,0 @@
-describe SubscriptionEstimator do
-  describe "estimating prices for subscription line items" do
-    let!(:subscription) { create(:subscription, with_items: true) }
-    let!(:sli1) { subscription.subscription_line_items.first }
-    let!(:sli2) { subscription.subscription_line_items.second }
-    let!(:sli3) { subscription.subscription_line_items.third }
-    let(:estimator) { SubscriptionEstimator.new(subscription) }
-
-    before do
-      sli1.update_attributes(price_estimate: 4.0)
-      sli2.update_attributes(price_estimate: 5.0)
-      sli3.update_attributes(price_estimate: 6.0)
-      sli1.variant.update_attributes(price: 1.0)
-      sli2.variant.update_attributes(price: 2.0)
-      sli3.variant.update_attributes(price: 3.0)
-
-      # Simulating assignment of attrs from params
-      sli1.assign_attributes(price_estimate: 7.0)
-      sli2.assign_attributes(price_estimate: 8.0)
-      sli3.assign_attributes(price_estimate: 9.0)
-    end
-
-    context "when a insufficient information exists to calculate price estimates" do
-      before do
-        # This might be because a shop has not been assigned yet, or no
-        # current or future order cycles exist for the schedule
-        allow(estimator).to receive(:fee_calculator) { nil }
-      end
-
-      it "resets the price estimates for all items" do
-        estimator.estimate!
-        expect(sli1.price_estimate).to eq 4.0
-        expect(sli2.price_estimate).to eq 5.0
-        expect(sli3.price_estimate).to eq 6.0
-      end
-    end
-
-    context "when sufficient information to calculate price estimates exists" do
-      let(:fee_calculator) { instance_double(OpenFoodNetwork::EnterpriseFeeCalculator) }
-
-      before do
-        allow(estimator).to receive(:fee_calculator) { fee_calculator }
-        allow(fee_calculator).to receive(:indexed_fees_for).with(sli1.variant) { 1.0 }
-        allow(fee_calculator).to receive(:indexed_fees_for).with(sli2.variant) { 0.0 }
-        allow(fee_calculator).to receive(:indexed_fees_for).with(sli3.variant) { 3.0 }
-      end
-
-      context "when no variant overrides apply" do
-        it "recalculates price_estimates based on variant prices and associated fees" do
-          estimator.estimate!
-          expect(sli1.price_estimate).to eq 2.0
-          expect(sli2.price_estimate).to eq 2.0
-          expect(sli3.price_estimate).to eq 6.0
-        end
-      end
-
-      context "when variant overrides apply" do
-        let!(:override1) { create(:variant_override, hub: subscription.shop, variant: sli1.variant, price: 1.2) }
-        let!(:override2) { create(:variant_override, hub: subscription.shop, variant: sli2.variant, price: 2.3) }
-
-        it "recalculates price_estimates based on override prices and associated fees" do
-          estimator.estimate!
-          expect(sli1.price_estimate).to eq 2.2
-          expect(sli2.price_estimate).to eq 2.3
-          expect(sli3.price_estimate).to eq 6.0
-        end
-      end
-    end
-  end
-
-  describe "updating estimates for shipping and payment fees" do
-    let(:subscription) { create(:subscription, with_items: true, payment_method: payment_method, shipping_method: shipping_method) }
-    let!(:sli1) { subscription.subscription_line_items.first }
-    let!(:sli2) { subscription.subscription_line_items.second }
-    let!(:sli3) { subscription.subscription_line_items.third }
-    let(:estimator) { SubscriptionEstimator.new(subscription) }
-
-    before do
-      allow(estimator).to receive(:assign_price_estimates)
-      sli1.update_attributes(price_estimate: 4.0)
-      sli2.update_attributes(price_estimate: 5.0)
-      sli3.update_attributes(price_estimate: 6.0)
-    end
-
-    context "using flat rate calculators" do
-      let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 12.34)) }
-      let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 9.12)) }
-
-      it "calculates fees based on the rates provided" do
-        estimator.estimate!
-        expect(subscription.shipping_fee_estimate.to_f).to eq 12.34
-        expect(subscription.payment_fee_estimate.to_f).to eq 9.12
-      end
-    end
-
-    context "using flat percent item total calculators" do
-      let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10)) }
-      let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 20)) }
-
-      it "calculates fees based on the estimated item total and percentage provided" do
-        estimator.estimate!
-        expect(subscription.shipping_fee_estimate.to_f).to eq 1.5
-        expect(subscription.payment_fee_estimate.to_f).to eq 3.0
-      end
-    end
-
-    context "using flat percent per item calculators" do
-      let(:shipping_method) { create(:shipping_method, calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 5)) }
-      let(:payment_method) { create(:payment_method, calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 10)) }
-
-      it "calculates fees based on the estimated item prices and percentage provided" do
-        estimator.estimate!
-        expect(subscription.shipping_fee_estimate.to_f).to eq 0.75
-        expect(subscription.payment_fee_estimate.to_f).to eq 1.5
-      end
-    end
-
-    context "using per item calculators" do
-      let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::PerItem.new(preferred_amount: 1.2)) }
-      let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::PerItem.new(preferred_amount: 0.3)) }
-
-      it "calculates fees based on the number of items and rate provided" do
-        estimator.estimate!
-        expect(subscription.shipping_fee_estimate.to_f).to eq 3.6
-        expect(subscription.payment_fee_estimate.to_f).to eq 0.9
-      end
-    end
-  end
-end
diff --git a/spec/services/subscription_form_spec.rb b/spec/services/subscription_form_spec.rb
deleted file mode 100644
index 9a06ef7164..0000000000
--- a/spec/services/subscription_form_spec.rb
+++ /dev/null
@@ -1,97 +0,0 @@
-require 'spec_helper'
-
-describe SubscriptionForm do
-  describe "creating a new subscription" do
-    let!(:shop) { create(:distributor_enterprise) }
-    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: product2, unit_value: '1000', price: 2.50, option_values: [], on_hand: 1) }
-    let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) }
-    let!(:order_cycle1) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 9.days.ago, orders_close_at: 2.days.ago) }
-    let!(:order_cycle2) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.ago, orders_close_at: 5.days.from_now) }
-    let!(:order_cycle3) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 5.days.from_now, orders_close_at: 12.days.from_now) }
-    let!(:order_cycle4) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 12.days.from_now, orders_close_at: 19.days.from_now) }
-    let!(:outgoing_exchange1) { order_cycle1.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) }
-    let!(:outgoing_exchange2) { order_cycle2.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) }
-    let!(:outgoing_exchange3) { order_cycle3.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant3], enterprise_fees: []) }
-    let!(:outgoing_exchange4) { order_cycle4.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) }
-    let!(:schedule) { create(:schedule, order_cycles: [order_cycle1, order_cycle2, order_cycle3, order_cycle4]) }
-    let!(:payment_method) { create(:payment_method, distributors: [shop]) }
-    let!(:shipping_method) { create(:shipping_method, distributors: [shop]) }
-    let!(:address) { create(:address) }
-    let(:subscription) { Subscription.new }
-
-    let!(:params) {
-      {
-        shop_id: shop.id,
-        customer_id: customer.id,
-        schedule_id: schedule.id,
-        bill_address_attributes: address.clone.attributes,
-        ship_address_attributes: address.clone.attributes,
-        payment_method_id: payment_method.id,
-        shipping_method_id: shipping_method.id,
-        begins_at: 4.days.ago,
-        ends_at: 14.days.from_now,
-        subscription_line_items_attributes: [
-          { variant_id: variant1.id, quantity: 1, price_estimate: 7.0 },
-          { variant_id: variant2.id, quantity: 2, price_estimate: 8.0 },
-          { variant_id: variant3.id, quantity: 3, price_estimate: 9.0 }
-        ]
-      }
-    }
-
-    let(:form) { SubscriptionForm.new(subscription, params) }
-
-    it "creates orders for each order cycle in the schedule" do
-      expect(form.save).to be true
-
-      expect(subscription.proxy_orders.count).to be 2
-      expect(subscription.subscription_line_items.count).to be 3
-      expect(subscription.subscription_line_items[0].price_estimate).to eq 13.75
-      expect(subscription.subscription_line_items[1].price_estimate).to eq 7.75
-      expect(subscription.subscription_line_items[2].price_estimate).to eq 4.25
-
-      # This order cycle has already closed, so no order is initialized
-      proxy_order1 = subscription.proxy_orders.find_by_order_cycle_id(order_cycle1.id)
-      expect(proxy_order1).to be nil
-
-      # Currently open order cycle, closing after begins_at and before ends_at
-      proxy_order2 = subscription.proxy_orders.find_by_order_cycle_id(order_cycle2.id)
-      expect(proxy_order2).to be_a ProxyOrder
-      order2 = proxy_order2.initialise_order!
-      expect(order2.line_items.count).to eq 3
-      expect(order2.line_items.find_by_variant_id(variant3.id).quantity).to be 3
-      expect(order2.shipments.count).to eq 1
-      expect(order2.shipments.first.shipping_method).to eq shipping_method
-      expect(order2.payments.count).to eq 1
-      expect(order2.payments.first.payment_method).to eq payment_method
-      expect(order2.payments.first.state).to eq 'checkout'
-      expect(order2.total).to eq 42
-      expect(order2.completed?).to be false
-
-      # Future order cycle, closing after begins_at and before ends_at
-      # Adds line items for variants that aren't yet available from the order cycle
-      proxy_order3 = subscription.proxy_orders.find_by_order_cycle_id(order_cycle3.id)
-      expect(proxy_order3).to be_a ProxyOrder
-      order3 = proxy_order3.initialise_order!
-      expect(order3).to be_a Spree::Order
-      expect(order3.line_items.count).to eq 3
-      expect(order2.line_items.find_by_variant_id(variant3.id).quantity).to be 3
-      expect(order3.shipments.count).to eq 1
-      expect(order3.shipments.first.shipping_method).to eq shipping_method
-      expect(order3.payments.count).to eq 1
-      expect(order3.payments.first.payment_method).to eq payment_method
-      expect(order3.payments.first.state).to eq 'checkout'
-      expect(order3.total).to eq 31.50
-      expect(order3.completed?).to be false
-
-      # Future order cycle closing after ends_at
-      proxy_order4 = subscription.proxy_orders.find_by_order_cycle_id(order_cycle4.id)
-      expect(proxy_order4).to be nil
-    end
-  end
-end
diff --git a/spec/services/subscription_validator_spec.rb b/spec/services/subscription_validator_spec.rb
deleted file mode 100644
index bdcd14bea5..0000000000
--- a/spec/services/subscription_validator_spec.rb
+++ /dev/null
@@ -1,470 +0,0 @@
-require "spec_helper"
-
-describe SubscriptionValidator do
-  let(:owner) { create(:user) }
-  let(:shop) { create(:enterprise, name: "Shop", owner: owner) }
-
-  describe "delegation" do
-    let(:subscription) { create(:subscription, shop: shop) }
-    let(:validator) { SubscriptionValidator.new(subscription) }
-
-    it "delegates to subscription" do
-      expect(validator.shop).to eq subscription.shop
-      expect(validator.customer).to eq subscription.customer
-      expect(validator.schedule).to eq subscription.schedule
-      expect(validator.shipping_method).to eq subscription.shipping_method
-      expect(validator.payment_method).to eq subscription.payment_method
-      expect(validator.bill_address).to eq subscription.bill_address
-      expect(validator.ship_address).to eq subscription.ship_address
-      expect(validator.begins_at).to eq subscription.begins_at
-      expect(validator.ends_at).to eq subscription.ends_at
-    end
-  end
-
-  describe "validations" do
-    let(:subscription_stubs) do
-      {
-        shop: shop,
-        customer: true,
-        schedule: true,
-        shipping_method: true,
-        payment_method: true,
-        bill_address: true,
-        ship_address: true,
-        begins_at: true,
-        ends_at: true,
-      }
-    end
-
-    let(:validation_stubs) do
-      {
-        shipping_method_allowed?: true,
-        payment_method_allowed?: true,
-        payment_method_type_allowed?: true,
-        ends_at_after_begins_at?: true,
-        customer_allowed?: true,
-        schedule_allowed?: true,
-        credit_card_ok?: true,
-        subscription_line_items_present?: true,
-        requested_variants_available?: true
-      }
-    end
-
-    let(:subscription) { instance_double(Subscription, subscription_stubs) }
-    let(:validator) { SubscriptionValidator.new(subscription) }
-
-    def stub_validations(validator, methods)
-      methods.each do |name, value|
-        allow(validator).to receive(name) { value }
-      end
-    end
-
-    describe "shipping method validation" do
-      let(:subscription) { instance_double(Subscription, subscription_stubs.except(:shipping_method)) }
-      before { stub_validations(validator, validation_stubs.except(:shipping_method_allowed?)) }
-
-      context "when no shipping method is present" do
-        before { expect(subscription).to receive(:shipping_method).at_least(:once) { nil } }
-
-        it "adds an error and returns false" do
-          expect(validator.valid?).to be false
-          expect(validator.errors[:shipping_method]).to_not be_empty
-        end
-      end
-
-      context "when a shipping method is present" do
-        let(:shipping_method) { instance_double(Spree::ShippingMethod, distributors: [shop]) }
-        before { expect(subscription).to receive(:shipping_method).at_least(:once) { shipping_method } }
-
-        context "and the shipping method is not associated with the shop" do
-          before { allow(shipping_method).to receive(:distributors) { [double(:enterprise)] } }
-
-          it "adds an error and returns false" do
-            expect(validator.valid?).to be false
-            expect(validator.errors[:shipping_method]).to_not be_empty
-          end
-        end
-
-        context "and the shipping method is associated with the shop" do
-          before { allow(shipping_method).to receive(:distributors) { [shop] } }
-
-          it "returns true" do
-            expect(validator.valid?).to be true
-            expect(validator.errors[:shipping_method]).to be_empty
-          end
-        end
-      end
-    end
-
-    describe "payment method validation" do
-      let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) }
-      before { stub_validations(validator, validation_stubs.except(:payment_method_allowed?)) }
-
-      context "when no payment method is present" do
-        before { expect(subscription).to receive(:payment_method).at_least(:once) { nil } }
-
-        it "adds an error and returns false" do
-          expect(validator.valid?).to be false
-          expect(validator.errors[:payment_method]).to_not be_empty
-        end
-      end
-
-      context "when a payment method is present" do
-        let(:payment_method) { instance_double(Spree::PaymentMethod, distributors: [shop]) }
-        before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } }
-
-        context "and the payment method is not associated with the shop" do
-          before { allow(payment_method).to receive(:distributors) { [double(:enterprise)] } }
-
-          it "adds an error and returns false" do
-            expect(validator.valid?).to be false
-            expect(validator.errors[:payment_method]).to_not be_empty
-          end
-        end
-
-        context "and the payment method is associated with the shop" do
-          before { allow(payment_method).to receive(:distributors) { [shop] } }
-
-          it "returns true" do
-            expect(validator.valid?).to be true
-            expect(validator.errors[:payment_method]).to be_empty
-          end
-        end
-      end
-    end
-
-    describe "payment method type validation" do
-      let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) }
-      before { stub_validations(validator, validation_stubs.except(:payment_method_type_allowed?)) }
-
-      context "when a payment method is present" do
-        let(:payment_method) { instance_double(Spree::PaymentMethod, distributors: [shop]) }
-        before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } }
-
-        context "and the payment method type is not in the approved list" do
-          before { allow(payment_method).to receive(:type) { "Blah" } }
-
-          it "adds an error and returns false" do
-            expect(validator.valid?).to be false
-            expect(validator.errors[:payment_method]).to_not be_empty
-          end
-        end
-
-        context "and the payment method is in the approved list" do
-          let(:approved_type) { Subscription::ALLOWED_PAYMENT_METHOD_TYPES.first }
-          before { allow(payment_method).to receive(:type) { approved_type } }
-
-          it "returns true" do
-            expect(validator.valid?).to be true
-            expect(validator.errors[:payment_method]).to be_empty
-          end
-        end
-      end
-    end
-
-    describe "dates" do
-      let(:subscription) { instance_double(Subscription, subscription_stubs.except(:begins_at, :ends_at)) }
-      before { stub_validations(validator, validation_stubs.except(:ends_at_after_begins_at?)) }
-      before { expect(subscription).to receive(:begins_at).at_least(:once) { begins_at } }
-
-      context "when no begins_at is present" do
-        let(:begins_at) { nil }
-
-        it "adds an error and returns false" do
-          expect(validator.valid?).to be false
-          expect(validator.errors[:begins_at]).to_not be_empty
-        end
-      end
-
-      context "when a start date is present" do
-        let(:begins_at) { Time.zone.today }
-        before { expect(subscription).to receive(:ends_at).at_least(:once) { ends_at } }
-
-        context "when no ends_at is present" do
-          let(:ends_at) { nil }
-
-          it "returns true" do
-            expect(validator.valid?).to be true
-            expect(validator.errors[:ends_at]).to be_empty
-          end
-        end
-
-        context "when ends_at is equal to begins_at" do
-          let(:ends_at) { Time.zone.today }
-          it "adds an error and returns false" do
-            expect(validator.valid?).to be false
-            expect(validator.errors[:ends_at]).to_not be_empty
-          end
-        end
-
-        context "when ends_at is before begins_at" do
-          let(:ends_at) { Time.zone.today - 1.day }
-          it "adds an error and returns false" do
-            expect(validator.valid?).to be false
-            expect(validator.errors[:ends_at]).to_not be_empty
-          end
-        end
-
-        context "when ends_at is after begins_at" do
-          let(:ends_at) { Time.zone.today + 1.day }
-          it "adds an error and returns false" do
-            expect(validator.valid?).to be true
-            expect(validator.errors[:ends_at]).to be_empty
-          end
-        end
-      end
-    end
-
-    describe "addresses" do
-      before { stub_validations(validator, validation_stubs) }
-      let(:subscription) { instance_double(Subscription, subscription_stubs.except(:bill_address, :ship_address)) }
-      before { expect(subscription).to receive(:bill_address).at_least(:once) { bill_address } }
-      before { expect(subscription).to receive(:ship_address).at_least(:once) { ship_address } }
-
-      context "when bill_address and ship_address are not present" do
-        let(:bill_address) { nil }
-        let(:ship_address) { nil }
-
-        it "adds an error and returns false" do
-          expect(validator.valid?).to be false
-          expect(validator.errors[:bill_address]).to_not be_empty
-          expect(validator.errors[:ship_address]).to_not be_empty
-        end
-      end
-
-      context "when bill_address and ship_address are present" do
-        let(:bill_address) { instance_double(Spree::Address) }
-        let(:ship_address) { instance_double(Spree::Address) }
-
-        it "returns true" do
-          expect(validator.valid?).to be true
-          expect(validator.errors[:bill_address]).to be_empty
-          expect(validator.errors[:ship_address]).to be_empty
-        end
-      end
-    end
-
-    describe "customer" do
-      let(:subscription) { instance_double(Subscription, subscription_stubs.except(:customer)) }
-      before { stub_validations(validator, validation_stubs.except(:customer_allowed?)) }
-      before { expect(subscription).to receive(:customer).at_least(:once) { customer } }
-
-      context "when no customer is present" do
-        let(:customer) { nil }
-
-        it "adds an error and returns false" do
-          expect(validator.valid?).to be false
-          expect(validator.errors[:customer]).to_not be_empty
-        end
-      end
-
-      context "when a customer is present" do
-        let(:customer) { instance_double(Customer) }
-
-        context "and the customer is not associated with the shop" do
-          before { allow(customer).to receive(:enterprise) { double(:enterprise) } }
-
-          it "adds an error and returns false" do
-            expect(validator.valid?).to be false
-            expect(validator.errors[:customer]).to_not be_empty
-          end
-        end
-
-        context "and the customer is associated with the shop" do
-          before { allow(customer).to receive(:enterprise) { shop } }
-
-          it "returns true" do
-            expect(validator.valid?).to be true
-            expect(validator.errors[:customer]).to be_empty
-          end
-        end
-      end
-    end
-
-    describe "schedule" do
-      let(:subscription) { instance_double(Subscription, subscription_stubs.except(:schedule)) }
-      before { stub_validations(validator, validation_stubs.except(:schedule_allowed?)) }
-      before { expect(subscription).to receive(:schedule).at_least(:once) { schedule } }
-
-      context "when no schedule is present" do
-        let(:schedule) { nil }
-
-        it "adds an error and returns false" do
-          expect(validator.valid?).to be false
-          expect(validator.errors[:schedule]).to_not be_empty
-        end
-      end
-
-      context "when a schedule is present" do
-        let(:schedule) { instance_double(Schedule) }
-
-        context "and the schedule is not associated with the shop" do
-          before { allow(schedule).to receive(:coordinators) { [double(:enterprise)] } }
-
-          it "adds an error and returns false" do
-            expect(validator.valid?).to be false
-            expect(validator.errors[:schedule]).to_not be_empty
-          end
-        end
-
-        context "and the schedule is associated with the shop" do
-          before { allow(schedule).to receive(:coordinators) { [shop] } }
-
-          it "returns true" do
-            expect(validator.valid?).to be true
-            expect(validator.errors[:schedule]).to be_empty
-          end
-        end
-      end
-    end
-
-    describe "credit card" do
-      let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) }
-      before { stub_validations(validator, validation_stubs.except(:credit_card_ok?)) }
-      before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } }
-
-      context "when using a Check payment method" do
-        let(:payment_method) { instance_double(Spree::PaymentMethod, type: "Spree::PaymentMethod::Check") }
-
-        it "returns true" do
-          expect(validator.valid?).to be true
-          expect(validator.errors[:subscription_line_items]).to be_empty
-        end
-      end
-
-      context "when using the StripeConnect payment gateway" do
-        let(:payment_method) { instance_double(Spree::PaymentMethod, type: "Spree::Gateway::StripeConnect") }
-        before { expect(subscription).to receive(:customer).at_least(:once) { customer } }
-
-        context "when the customer does not allow charges" do
-          let(:customer) { instance_double(Customer, allow_charges: false) }
-
-          it "adds an error and returns false" do
-            expect(validator.valid?).to be false
-            expect(validator.errors[:payment_method]).to_not be_empty
-          end
-        end
-
-        context "when the customer allows charges" do
-          let(:customer) { instance_double(Customer, allow_charges: true) }
-
-          context "and the customer is not associated with a user" do
-            before { allow(customer).to receive(:user) { nil } }
-
-            it "adds an error and returns false" do
-              expect(validator.valid?).to be false
-              expect(validator.errors[:payment_method]).to_not be_empty
-            end
-          end
-
-          context "and the customer is associated with a user" do
-            before { expect(customer).to receive(:user).once { user } }
-
-            context "and the user has no default card set" do
-              let(:user) { instance_double(Spree::User, default_card: nil) }
-
-              it "adds an error and returns false" do
-                expect(validator.valid?).to be false
-                expect(validator.errors[:payment_method]).to_not be_empty
-              end
-            end
-
-            context "and the user has a default card set" do
-              let(:user) { instance_double(Spree::User, default_card: 'some card') }
-
-              it "returns true" do
-                expect(validator.valid?).to be true
-                expect(validator.errors[:payment_method]).to be_empty
-              end
-            end
-          end
-        end
-      end
-    end
-
-    describe "subscription line items" do
-      let(:subscription) { instance_double(Subscription, subscription_stubs) }
-      before { stub_validations(validator, validation_stubs.except(:subscription_line_items_present?)) }
-      before { expect(subscription).to receive(:subscription_line_items).at_least(:once) { subscription_line_items } }
-
-      context "when no subscription line items are present" do
-        let(:subscription_line_items) { [] }
-
-        it "adds an error and returns false" do
-          expect(validator.valid?).to be false
-          expect(validator.errors[:subscription_line_items]).to_not be_empty
-        end
-      end
-
-      context "when subscription line items are present but they are all marked for destruction" do
-        let(:subscription_line_item1) { instance_double(SubscriptionLineItem, marked_for_destruction?: true) }
-        let(:subscription_line_items) { [subscription_line_item1] }
-
-        it "adds an error and returns false" do
-          expect(validator.valid?).to be false
-          expect(validator.errors[:subscription_line_items]).to_not be_empty
-        end
-      end
-
-      context "when subscription line items are present and some and not marked for destruction" do
-        let(:subscription_line_item1) { instance_double(SubscriptionLineItem, marked_for_destruction?: true) }
-        let(:subscription_line_item2) { instance_double(SubscriptionLineItem, marked_for_destruction?: false) }
-        let(:subscription_line_items) { [subscription_line_item1, subscription_line_item2] }
-
-        it "returns true" do
-          expect(validator.valid?).to be true
-          expect(validator.errors[:subscription_line_items]).to be_empty
-        end
-      end
-    end
-
-    describe "variant availability" do
-      let(:subscription) { instance_double(Subscription, subscription_stubs) }
-      before { stub_validations(validator, validation_stubs.except(:requested_variants_available?)) }
-      before { expect(subscription).to receive(:subscription_line_items).at_least(:once) { subscription_line_items } }
-
-      context "when no subscription line items are present" do
-        let(:subscription_line_items) { [] }
-
-        it "returns true" do
-          expect(validator.valid?).to be true
-          expect(validator.errors[:subscription_line_items]).to be_empty
-        end
-      end
-
-      context "when subscription line items are present" do
-        let(:variant1) { instance_double(Spree::Variant, id: 1) }
-        let(:variant2) { instance_double(Spree::Variant, id: 2) }
-        let(:subscription_line_item1) { instance_double(SubscriptionLineItem, variant: variant1) }
-        let(:subscription_line_item2) { instance_double(SubscriptionLineItem, variant: variant2) }
-        let(:subscription_line_items) { [subscription_line_item1] }
-
-        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 }
-            allow(variant1).to receive(:full_name) { "some name" }
-          end
-
-          it "adds an error and returns false" do
-            expect(validator.valid?).to be false
-            expect(validator.errors[:subscription_line_items]).to_not be_empty
-          end
-        end
-
-        context "and all requested variants are available" do
-          before do
-            allow(validator).to receive(:available_variant_ids) { [variant1.id, variant2.id] }
-          end
-
-          it "returns true" do
-            expect(validator.valid?).to be true
-            expect(validator.errors[:subscription_line_items]).to be_empty
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/spec/services/subscription_variants_service_spec.rb b/spec/services/subscription_variants_service_spec.rb
deleted file mode 100644
index 31d0ff4ca7..0000000000
--- a/spec/services/subscription_variants_service_spec.rb
+++ /dev/null
@@ -1,130 +0,0 @@
-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
diff --git a/spec/services/subscriptions_count_spec.rb b/spec/services/subscriptions_count_spec.rb
deleted file mode 100644
index 2446bcc8bf..0000000000
--- a/spec/services/subscriptions_count_spec.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-describe SubscriptionsCount do
-  let(:oc1) { create(:simple_order_cycle) }
-  let(:oc2) { create(:simple_order_cycle) }
-  let(:subscriptions_count) { SubscriptionsCount.new(order_cycles) }
-
-  describe "#for" do
-    context "when the collection has not been set" do
-      let(:order_cycles) { nil }
-      it "returns 0" do
-        expect(subscriptions_count.for(oc1.id)).to eq 0
-      end
-    end
-
-    context "when the collection has been set" do
-      let(:order_cycles) { OrderCycle.where(id: [oc1]) }
-      let!(:po1) { create(:proxy_order, order_cycle: oc1) }
-      let!(:po2) { create(:proxy_order, order_cycle: oc1) }
-      let!(:po3) { create(:proxy_order, order_cycle: oc2) }
-
-      context "but the requested id is not present in the list of order cycles provided" do
-        it "returns 0" do
-          # Note that po3 applies to oc2, but oc2 in not in the collection
-          expect(subscriptions_count.for(oc2.id)).to eq 0
-        end
-      end
-
-      context "and the requested id is present in the list of order cycles provided" do
-        it "returns a count of active proxy orders associated with the requested order cycle" do
-          expect(subscriptions_count.for(oc1.id)).to eq 2
-        end
-      end
-    end
-  end
-end

From f8a4f00d52a13d07ff6fdb8521b555028a79bdf6 Mon Sep 17 00:00:00 2001
From: Luis Ramos <luisramos0@gmail.com>
Date: Tue, 11 Feb 2020 20:04:22 +0000
Subject: [PATCH 12/12] Fix rubocop issues in subs specs

---
 .rubocop_manual_todo.yml                      |   4 +
 .../subscriptions/estimator_spec.rb           |  58 ++++++++--
 .../subscriptions/form_spec.rb                |  69 +++++++++---
 .../subscriptions/validator_spec.rb           | 102 +++++++++++++-----
 .../subscriptions/variants_list_spec.rb       |  45 ++++++--
 5 files changed, 220 insertions(+), 58 deletions(-)

diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml
index 83de12879f..a1908cdc5d 100644
--- a/.rubocop_manual_todo.yml
+++ b/.rubocop_manual_todo.yml
@@ -678,9 +678,13 @@ Metrics/ModuleLength:
   - app/helpers/injection_helper.rb
   - app/helpers/spree/admin/navigation_helper.rb
   - app/helpers/spree/admin/base_helper.rb
+  - engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb
+  - engines/order_management/spec/services/order_management/subscriptions/form_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/summarizer_spec.rb
+  - engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb
+  - engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb
   - lib/open_food_network/column_preference_defaults.rb
   - spec/controllers/admin/enterprises_controller_spec.rb
   - spec/controllers/admin/order_cycles_controller_spec.rb
diff --git a/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb
index 78020f6578..e8fe7bf957 100644
--- a/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb
@@ -59,8 +59,12 @@ module OrderManagement
           end
 
           context "when variant overrides apply" do
-            let!(:override1) { create(:variant_override, hub: subscription.shop, variant: sli1.variant, price: 1.2) }
-            let!(:override2) { create(:variant_override, hub: subscription.shop, variant: sli2.variant, price: 2.3) }
+            let!(:override1) {
+              create(:variant_override, hub: subscription.shop, variant: sli1.variant, price: 1.2)
+            }
+            let!(:override2) {
+              create(:variant_override, hub: subscription.shop, variant: sli2.variant, price: 2.3)
+            }
 
             it "recalculates price_estimates based on override prices and associated fees" do
               estimator.estimate!
@@ -73,7 +77,11 @@ module OrderManagement
       end
 
       describe "updating estimates for shipping and payment fees" do
-        let(:subscription) { create(:subscription, with_items: true, payment_method: payment_method, shipping_method: shipping_method) }
+        let(:subscription) {
+          create(:subscription, with_items: true,
+                                payment_method: payment_method,
+                                shipping_method: shipping_method)
+        }
         let!(:sli1) { subscription.subscription_line_items.first }
         let!(:sli2) { subscription.subscription_line_items.second }
         let!(:sli3) { subscription.subscription_line_items.third }
@@ -87,8 +95,14 @@ module OrderManagement
         end
 
         context "using flat rate calculators" do
-          let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 12.34)) }
-          let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 9.12)) }
+          let(:shipping_method) {
+            create(:shipping_method,
+                   calculator: Spree::Calculator::FlatRate.new(preferred_amount: 12.34))
+          }
+          let(:payment_method) {
+            create(:payment_method,
+                   calculator: Spree::Calculator::FlatRate.new(preferred_amount: 9.12))
+          }
 
           it "calculates fees based on the rates provided" do
             estimator.estimate!
@@ -98,8 +112,18 @@ module OrderManagement
         end
 
         context "using flat percent item total calculators" do
-          let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10)) }
-          let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 20)) }
+          let(:shipping_method) {
+            create(:shipping_method,
+                   calculator: Spree::Calculator::FlatPercentItemTotal.new(
+                     preferred_flat_percent: 10
+                   ))
+          }
+          let(:payment_method) {
+            create(:payment_method,
+                   calculator: Spree::Calculator::FlatPercentItemTotal.new(
+                     preferred_flat_percent: 20
+                   ))
+          }
 
           it "calculates fees based on the estimated item total and percentage provided" do
             estimator.estimate!
@@ -109,8 +133,14 @@ module OrderManagement
         end
 
         context "using flat percent per item calculators" do
-          let(:shipping_method) { create(:shipping_method, calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 5)) }
-          let(:payment_method) { create(:payment_method, calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 10)) }
+          let(:shipping_method) {
+            create(:shipping_method,
+                   calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 5))
+          }
+          let(:payment_method) {
+            create(:payment_method,
+                   calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 10))
+          }
 
           it "calculates fees based on the estimated item prices and percentage provided" do
             estimator.estimate!
@@ -120,8 +150,14 @@ module OrderManagement
         end
 
         context "using per item calculators" do
-          let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::PerItem.new(preferred_amount: 1.2)) }
-          let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::PerItem.new(preferred_amount: 0.3)) }
+          let(:shipping_method) {
+            create(:shipping_method,
+                   calculator: Spree::Calculator::PerItem.new(preferred_amount: 1.2))
+          }
+          let(:payment_method) {
+            create(:payment_method,
+                   calculator: Spree::Calculator::PerItem.new(preferred_amount: 0.3))
+          }
 
           it "calculates fees based on the number of items and rate provided" do
             estimator.estimate!
diff --git a/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb
index f9c0798671..3e849d5c82 100644
--- a/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb
@@ -11,19 +11,64 @@ module OrderManagement
         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: product2, unit_value: '1000', price: 2.50, option_values: [], on_hand: 1) }
+        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: product2, unit_value: '1000',
+                           price: 2.50, option_values: [], on_hand: 1)
+        }
         let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) }
-        let!(:order_cycle1) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 9.days.ago, orders_close_at: 2.days.ago) }
-        let!(:order_cycle2) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.ago, orders_close_at: 5.days.from_now) }
-        let!(:order_cycle3) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 5.days.from_now, orders_close_at: 12.days.from_now) }
-        let!(:order_cycle4) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 12.days.from_now, orders_close_at: 19.days.from_now) }
-        let!(:outgoing_exchange1) { order_cycle1.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) }
-        let!(:outgoing_exchange2) { order_cycle2.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) }
-        let!(:outgoing_exchange3) { order_cycle3.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant3], enterprise_fees: []) }
-        let!(:outgoing_exchange4) { order_cycle4.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) }
-        let!(:schedule) { create(:schedule, order_cycles: [order_cycle1, order_cycle2, order_cycle3, order_cycle4]) }
+        let!(:order_cycle1) {
+          create(:simple_order_cycle, coordinator: shop,
+                                      orders_open_at: 9.days.ago,
+                                      orders_close_at: 2.days.ago)
+        }
+        let!(:order_cycle2) {
+          create(:simple_order_cycle, coordinator: shop,
+                                      orders_open_at: 2.days.ago,
+                                      orders_close_at: 5.days.from_now)
+        }
+        let!(:order_cycle3) {
+          create(:simple_order_cycle, coordinator: shop,
+                                      orders_open_at: 5.days.from_now,
+                                      orders_close_at: 12.days.from_now)
+        }
+        let!(:order_cycle4) {
+          create(:simple_order_cycle, coordinator: shop,
+                                      orders_open_at: 12.days.from_now,
+                                      orders_close_at: 19.days.from_now)
+        }
+        let!(:outgoing_exchange1) {
+          order_cycle1.exchanges.create(sender: shop,
+                                        receiver: shop,
+                                        variants: [variant1, variant2, variant3],
+                                        enterprise_fees: [enterprise_fee])
+        }
+        let!(:outgoing_exchange2) {
+          order_cycle2.exchanges.create(sender: shop,
+                                        receiver: shop,
+                                        variants: [variant1, variant2, variant3],
+                                        enterprise_fees: [enterprise_fee])
+        }
+        let!(:outgoing_exchange3) {
+          order_cycle3.exchanges.create(sender: shop,
+                                        receiver: shop,
+                                        variants: [variant1, variant3],
+                                        enterprise_fees: [])
+        }
+        let!(:outgoing_exchange4) {
+          order_cycle4.exchanges.create(sender: shop,
+                                        receiver: shop,
+                                        variants: [variant1, variant2, variant3],
+                                        enterprise_fees: [enterprise_fee])
+        }
+        let!(:schedule) {
+          create(:schedule, order_cycles: [order_cycle1, order_cycle2, order_cycle3, order_cycle4])
+        }
         let!(:payment_method) { create(:payment_method, distributors: [shop]) }
         let!(:shipping_method) { create(:shipping_method, distributors: [shop]) }
         let!(:address) { create(:address) }
diff --git a/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb
index a072262b47..7c52fba12a 100644
--- a/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb
@@ -64,7 +64,9 @@ module OrderManagement
         end
 
         describe "shipping method validation" do
-          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:shipping_method)) }
+          let(:subscription) {
+            instance_double(Subscription, subscription_stubs.except(:shipping_method))
+          }
           before { stub_validations(validator, validation_stubs.except(:shipping_method_allowed?)) }
 
           context "when no shipping method is present" do
@@ -78,7 +80,9 @@ module OrderManagement
 
           context "when a shipping method is present" do
             let(:shipping_method) { instance_double(Spree::ShippingMethod, distributors: [shop]) }
-            before { expect(subscription).to receive(:shipping_method).at_least(:once) { shipping_method } }
+            before {
+              expect(subscription).to receive(:shipping_method).at_least(:once) { shipping_method }
+            }
 
             context "and the shipping method is not associated with the shop" do
               before { allow(shipping_method).to receive(:distributors) { [double(:enterprise)] } }
@@ -101,7 +105,9 @@ module OrderManagement
         end
 
         describe "payment method validation" do
-          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) }
+          let(:subscription) {
+            instance_double(Subscription, subscription_stubs.except(:payment_method))
+          }
           before { stub_validations(validator, validation_stubs.except(:payment_method_allowed?)) }
 
           context "when no payment method is present" do
@@ -115,7 +121,9 @@ module OrderManagement
 
           context "when a payment method is present" do
             let(:payment_method) { instance_double(Spree::PaymentMethod, distributors: [shop]) }
-            before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } }
+            before {
+              expect(subscription).to receive(:payment_method).at_least(:once) { payment_method }
+            }
 
             context "and the payment method is not associated with the shop" do
               before { allow(payment_method).to receive(:distributors) { [double(:enterprise)] } }
@@ -138,12 +146,18 @@ module OrderManagement
         end
 
         describe "payment method type validation" do
-          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) }
-          before { stub_validations(validator, validation_stubs.except(:payment_method_type_allowed?)) }
+          let(:subscription) {
+            instance_double(Subscription, subscription_stubs.except(:payment_method))
+          }
+          before {
+            stub_validations(validator, validation_stubs.except(:payment_method_type_allowed?))
+          }
 
           context "when a payment method is present" do
             let(:payment_method) { instance_double(Spree::PaymentMethod, distributors: [shop]) }
-            before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } }
+            before {
+              expect(subscription).to receive(:payment_method).at_least(:once) { payment_method }
+            }
 
             context "and the payment method type is not in the approved list" do
               before { allow(payment_method).to receive(:type) { "Blah" } }
@@ -167,7 +181,9 @@ module OrderManagement
         end
 
         describe "dates" do
-          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:begins_at, :ends_at)) }
+          let(:subscription) {
+            instance_double(Subscription, subscription_stubs.except(:begins_at, :ends_at))
+          }
           before { stub_validations(validator, validation_stubs.except(:ends_at_after_begins_at?)) }
           before { expect(subscription).to receive(:begins_at).at_least(:once) { begins_at } }
 
@@ -221,7 +237,9 @@ module OrderManagement
 
         describe "addresses" do
           before { stub_validations(validator, validation_stubs) }
-          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:bill_address, :ship_address)) }
+          let(:subscription) {
+            instance_double(Subscription, subscription_stubs.except(:bill_address, :ship_address))
+          }
           before { expect(subscription).to receive(:bill_address).at_least(:once) { bill_address } }
           before { expect(subscription).to receive(:ship_address).at_least(:once) { ship_address } }
 
@@ -323,12 +341,18 @@ module OrderManagement
         end
 
         describe "credit card" do
-          let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) }
+          let(:subscription) {
+            instance_double(Subscription, subscription_stubs.except(:payment_method))
+          }
           before { stub_validations(validator, validation_stubs.except(:credit_card_ok?)) }
-          before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } }
+          before {
+            expect(subscription).to receive(:payment_method).at_least(:once) { payment_method }
+          }
 
           context "when using a Check payment method" do
-            let(:payment_method) { instance_double(Spree::PaymentMethod, type: "Spree::PaymentMethod::Check") }
+            let(:payment_method) {
+              instance_double(Spree::PaymentMethod, type: "Spree::PaymentMethod::Check")
+            }
 
             it "returns true" do
               expect(validator.valid?).to be true
@@ -337,7 +361,9 @@ module OrderManagement
           end
 
           context "when using the StripeConnect payment gateway" do
-            let(:payment_method) { instance_double(Spree::PaymentMethod, type: "Spree::Gateway::StripeConnect") }
+            let(:payment_method) {
+              instance_double(Spree::PaymentMethod, type: "Spree::Gateway::StripeConnect")
+            }
             before { expect(subscription).to receive(:customer).at_least(:once) { customer } }
 
             context "when the customer does not allow charges" do
@@ -388,10 +414,16 @@ module OrderManagement
 
         describe "subscription line items" do
           let(:subscription) { instance_double(Subscription, subscription_stubs) }
-          before { stub_validations(validator, validation_stubs.except(:subscription_line_items_present?)) }
-          before { expect(subscription).to receive(:subscription_line_items).at_least(:once) { subscription_line_items } }
+          before {
+            stub_validations(validator, validation_stubs.except(:subscription_line_items_present?))
+          }
+          before {
+            expect(subscription).to receive(:subscription_line_items).at_least(:once) {
+              subscription_line_items
+            }
+          }
 
-          context "when no subscription line items are present" do
+          context "when no subscription line items exist" do
             let(:subscription_line_items) { [] }
 
             it "adds an error and returns false" do
@@ -400,8 +432,10 @@ module OrderManagement
             end
           end
 
-          context "when subscription line items are present but they are all marked for destruction" do
-            let(:subscription_line_item1) { instance_double(SubscriptionLineItem, marked_for_destruction?: true) }
+          context "when subscription line items exist but all are marked for destruction" do
+            let(:subscription_line_item1) {
+              instance_double(SubscriptionLineItem, marked_for_destruction?: true)
+            }
             let(:subscription_line_items) { [subscription_line_item1] }
 
             it "adds an error and returns false" do
@@ -410,9 +444,13 @@ module OrderManagement
             end
           end
 
-          context "when subscription line items are present and some and not marked for destruction" do
-            let(:subscription_line_item1) { instance_double(SubscriptionLineItem, marked_for_destruction?: true) }
-            let(:subscription_line_item2) { instance_double(SubscriptionLineItem, marked_for_destruction?: false) }
+          context "when subscription line items exist and some are not marked for destruction" do
+            let(:subscription_line_item1) {
+              instance_double(SubscriptionLineItem, marked_for_destruction?: true)
+            }
+            let(:subscription_line_item2) {
+              instance_double(SubscriptionLineItem, marked_for_destruction?: false)
+            }
             let(:subscription_line_items) { [subscription_line_item1, subscription_line_item2] }
 
             it "returns true" do
@@ -424,10 +462,16 @@ module OrderManagement
 
         describe "variant availability" do
           let(:subscription) { instance_double(Subscription, subscription_stubs) }
-          before { stub_validations(validator, validation_stubs.except(:requested_variants_available?)) }
-          before { expect(subscription).to receive(:subscription_line_items).at_least(:once) { subscription_line_items } }
+          before {
+            stub_validations(validator, validation_stubs.except(:requested_variants_available?))
+          }
+          before {
+            expect(subscription).to receive(:subscription_line_items).at_least(:once) {
+              subscription_line_items
+            }
+          }
 
-          context "when no subscription line items are present" do
+          context "when no subscription line items exist" do
             let(:subscription_line_items) { [] }
 
             it "returns true" do
@@ -436,11 +480,15 @@ module OrderManagement
             end
           end
 
-          context "when subscription line items are present" do
+          context "when subscription line items exist" do
             let(:variant1) { instance_double(Spree::Variant, id: 1) }
             let(:variant2) { instance_double(Spree::Variant, id: 2) }
-            let(:subscription_line_item1) { instance_double(SubscriptionLineItem, variant: variant1) }
-            let(:subscription_line_item2) { instance_double(SubscriptionLineItem, variant: variant2) }
+            let(:subscription_line_item1) {
+              instance_double(SubscriptionLineItem, variant: variant1)
+            }
+            let(:subscription_line_item2) {
+              instance_double(SubscriptionLineItem, variant: variant2)
+            }
             let(:subscription_line_items) { [subscription_line_item1] }
 
             context "but some variants are unavailable" do
diff --git a/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb
index 8dc97b128e..2ea3206045 100644
--- a/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb
+++ b/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb
@@ -43,7 +43,11 @@ module OrderManagement
         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]) }
+          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)
@@ -55,7 +59,11 @@ module OrderManagement
           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]) }
+            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)
@@ -63,7 +71,12 @@ module OrderManagement
           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]) }
+            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 }
@@ -109,25 +122,41 @@ module OrderManagement
           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]) }
+            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)
+              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]) }
+            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)
+              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)
+            expect(described_class).to_not be_in_open_and_upcoming_order_cycles(shop,
+                                                                                schedule,
+                                                                                variant)
           end
         end
       end