From 5081ffc17562fa90edf0f2a8dfa0d03991480362 Mon Sep 17 00:00:00 2001
From: Alessandro Desantis <desa.alessandro@gmail.com>
Date: Thu, 19 Nov 2020 13:13:46 +0100
Subject: [PATCH 1/9] Streamline and simplify `SolidusSubscriptions::Processor`

The processor was way too complicated, taking on too many responsibilities.
As a result, it was complex to reason about and it was a very brittle part
of the extension.

The new processor simply does two things: it finds all actionable
subscriptions and ensures to take the appropriate action on them (e.g.,
cancel, deactivate), including creating the new installment for later.

Then, it finds all the actionable installments (including the ones that
were just created), and schedules them for asynchronous processing.

One side effect of this refactoring is that installments are not grouped
by address and payment method/source anymore: each installment will always
correspond to a new order. Any logistics-related optimizations should be
implemented by the individual store.
---
 lib/solidus_subscriptions/processor.rb        | 110 ++-------
 .../factories/installment_factory.rb          |   4 +
 .../solidus_subscriptions/processor_spec.rb   | 212 ++++++++----------
 3 files changed, 115 insertions(+), 211 deletions(-)

diff --git a/lib/solidus_subscriptions/processor.rb b/lib/solidus_subscriptions/processor.rb
index d56816a9..4547ceca 100644
--- a/lib/solidus_subscriptions/processor.rb
+++ b/lib/solidus_subscriptions/processor.rb
@@ -1,116 +1,36 @@
 # frozen_string_literal: true
 
-# This class is responsible for finding subscriptions and installments
-# which need to be processed. It will group them together by user and attempts
-# to process them together. Subscriptions will also be grouped by their
-# shiping address id.
-#
-# This class passes the reponsibility of actually creating the order off onto
-# the consolidated installment class.
-#
-# This class generates `ProcessInstallmentsJob`s
 module SolidusSubscriptions
   class Processor
     class << self
-      # Find all actionable subscriptions and intallments, group them together
-      # by user, and schedule a processing job for the group as a batch
       def run
-        batched_users_to_be_processed.each { |batch| new(batch).build_jobs }
+        SolidusSubscriptions::Subscription.actionable.find_each(&method(:process_subscription))
+        SolidusSubscriptions::Installment.actionable.with_active_subscription.find_each(&method(:process_installment))
       end
 
       private
 
-      def batched_users_to_be_processed
-        subscriptions = SolidusSubscriptions::Subscription.arel_table
-        installments = SolidusSubscriptions::Installment.arel_table
+      def process_subscription(subscription)
+        ActiveRecord::Base.transaction do
+          subscription.successive_skip_count = 0
+          subscription.advance_actionable_date
 
-        ::Spree.user_class.
-          joins(:subscriptions).
-          joins(
-            subscriptions.
-              join(installments, Arel::Nodes::OuterJoin).
-              on(subscriptions[:id].eq(installments[:subscription_id])).
-              join_sources
-          ).
-          where(
-            SolidusSubscriptions::Subscription.actionable.arel.constraints.reduce(:and).
-              or(SolidusSubscriptions::Installment.actionable.with_active_subscription.arel.constraints.reduce(:and))
-          ).
-          distinct.
-          find_in_batches
-      end
-    end
-
-    # @return [Array<Spree.user_class>]
-    attr_reader :users
-
-    # Get a new instance of the SolidusSubscriptions::Processor
-    #
-    # @param users [Array<Spree.user_class>] A list of users with actionable
-    #   subscriptions or installments
-    #
-    # @return [SolidusSubscriptions::Processor]
-    def initialize(users)
-      @users = users
-      @installments = {}
-    end
-
-    # Create `ProcessInstallmentsJob`s for the users used to initalize the
-    # instance
-    def build_jobs
-      users.map do |user|
-        installemts_by_address_and_user = installments(user).group_by do |i|
-          [i.subscription.shipping_address_id, i.subscription.billing_address_id]
-        end
-
-        installemts_by_address_and_user.each_value do |grouped_installments|
-          ProcessInstallmentsJob.perform_later grouped_installments.map(&:id)
-        end
-      end
-    end
+          subscription.cancel! if subscription.pending_cancellation?
+          subscription.deactivate! if subscription.can_be_deactivated?
 
-    private
-
-    def subscriptions_by_id
-      @subscriptions_by_id ||= Subscription.
-                               actionable.
-                               includes(:line_items, :user).
-                               where(user_id: user_ids).
-                               group_by(&:user_id)
-    end
-
-    def retry_installments
-      @failed_installments ||= Installment.
-                               actionable.
-                               includes(:subscription).
-                               where(solidus_subscriptions_subscriptions: { user_id: user_ids }).
-                               group_by { |i| i.subscription.user_id }
-    end
-
-    def installments(user)
-      @installments[user.id] ||= retry_installments.fetch(user.id, []) + new_installments(user)
-    end
-
-    def new_installments(user)
-      ActiveRecord::Base.transaction do
-        subscriptions_by_id.fetch(user.id, []).map do |sub|
-          sub.successive_skip_count = 0
-          sub.advance_actionable_date
-          sub.cancel! if sub.pending_cancellation?
-          sub.deactivate! if sub.can_be_deactivated?
           if SolidusSubscriptions.configuration.clear_past_installments
-            sub.installments.unfulfilled.each do |installment|
-              installment.actionable_date = nil
-              installment.save!
+            subscription.installments.unfulfilled.actionable.each do |installment|
+              installment.update!(actionable_date: nil)
             end
           end
-          sub.installments.create!
+
+          subscription.installments.create!(actionable_date: Time.zone.now)
         end
       end
-    end
 
-    def user_ids
-      @user_ids ||= users.map(&:id)
+      def process_installment(installment)
+        ProcessInstallmentsJob.perform_later(installment)
+      end
     end
   end
 end
diff --git a/lib/solidus_subscriptions/testing_support/factories/installment_factory.rb b/lib/solidus_subscriptions/testing_support/factories/installment_factory.rb
index 2eb5069d..20dcc041 100644
--- a/lib/solidus_subscriptions/testing_support/factories/installment_factory.rb
+++ b/lib/solidus_subscriptions/testing_support/factories/installment_factory.rb
@@ -21,5 +21,9 @@
         [association(:installment_detail, :success, installment: @instance, order: order)]
       end
     end
+
+    trait :actionable do
+      actionable_date { Time.zone.now }
+    end
   end
 end
diff --git a/spec/lib/solidus_subscriptions/processor_spec.rb b/spec/lib/solidus_subscriptions/processor_spec.rb
index d3bb2680..a7301f00 100644
--- a/spec/lib/solidus_subscriptions/processor_spec.rb
+++ b/spec/lib/solidus_subscriptions/processor_spec.rb
@@ -1,156 +1,136 @@
-require 'spec_helper'
-
 RSpec.describe SolidusSubscriptions::Processor, :checkout do
-  include ActiveJob::TestHelper
-  around { |e| perform_enqueued_jobs { e.run } }
-
-  let!(:user) { create(:user, :subscription_user) }
-  let!(:credit_card) {
-    card = create(:credit_card, gateway_customer_profile_id: 'BGS-123', user: user)
-    wallet_payment_source = user.wallet.add(card)
-    user.wallet.default_wallet_payment_source = wallet_payment_source
-    user.save
-    card
-  }
-
-  let!(:actionable_subscriptions) { create_list(:subscription, 2, :actionable, user: user) }
-  let!(:pending_cancellation_subscriptions) do
-    create_list(:subscription, 2, :pending_cancellation, user: user)
-  end
+  shared_examples 'processes the subscription' do
+    it 'resets the successive_skip_count' do
+      subscription
+      subscription.update_columns(successive_skip_count: 3)
 
-  let!(:expiring_subscriptions) do
-    create_list(
-      :subscription,
-      2,
-      :actionable,
-      :with_line_item,
-      user: user,
-      end_date: Date.current.tomorrow,
-    )
-  end
+      described_class.run
 
-  let!(:future_subscriptions) { create_list(:subscription, 2, :not_actionable) }
-  let!(:canceled_subscriptions) { create_list(:subscription, 2, :canceled) }
-  let!(:inactive) { create_list(:subscription, 2, :inactive) }
-
-  let!(:successful_installments) { create_list :installment, 2, :success }
-  let!(:failed_installments) do
-    create_list(
-      :installment,
-      2,
-      :failed,
-      subscription_traits: [{ user: user }]
-    )
-  end
+      expect(subscription.reload.successive_skip_count).to eq(0)
+    end
 
-  # all subscriptions and previously failed installments belong to the same user
-  let(:expected_orders) { 1 }
+    context 'with clear_past_installments set to true' do
+      it 'clears any past unfulfilled installments' do
+        stub_config(clear_past_installments: true)
+        subscription
+        installment = create(:installment, :actionable, subscription: subscription)
 
-  shared_examples 'a subscription order' do
-    let(:order_variant_ids) { Spree::Order.last.variant_ids }
-    let(:expected_ids) do
-      subs = actionable_subscriptions + pending_cancellation_subscriptions + expiring_subscriptions
-      subs_ids = subs.flat_map { |s| s.line_items.pluck(:subscribable_id) }
-      inst_ids = failed_installments.flat_map { |i| i.subscription.line_items.pluck(:subscribable_id) }
+        described_class.run
 
-      subs_ids + inst_ids
+        expect(installment.reload.actionable_date).to eq(nil)
+      end
     end
 
-    it 'creates the correct number of orders' do
-      expect { subject }.to change { Spree::Order.complete.count }.by expected_orders
-    end
+    context 'with clear_past_installments set to false' do
+      it 'does not clear any past unfulfilled installments' do
+        stub_config(clear_past_installments: false)
+        subscription
+        installment = create(:installment, :actionable, subscription: subscription)
+
+        described_class.run
 
-    it 'creates the correct order' do
-      subject
-      expect(order_variant_ids).to match_array expected_ids
+        expect(installment.reload.actionable_date).not_to be_nil
+      end
     end
 
-    it 'advances the subscription actionable dates' do
-      subscription = actionable_subscriptions.first
+    it 'creates a new installment' do
+      subscription
 
-      current_date = subscription.actionable_date
-      expected_date = subscription.next_actionable_date
+      described_class.run
 
-      expect { subject }.
-        to change { subscription.reload.actionable_date }.
-        from(current_date).to(expected_date)
+      expect(subscription.installments.count).to eq(1)
     end
 
-    it 'cancels subscriptions pending cancellation' do
-      subs = pending_cancellation_subscriptions.first
-      expect { subject }.
-        to change { subs.reload.state }.
-        from('pending_cancellation').to('canceled')
-    end
+    it 'schedules the newly created installment for processing' do
+      subscription
 
-    it 'resets the subscription successive skip count' do
-      subs = pending_cancellation_subscriptions.first
-      expect { subject }.
-        to change { subs.reload.state }.
-        from('pending_cancellation').to('canceled')
+      described_class.run
+
+      expect(SolidusSubscriptions::ProcessInstallmentsJob).to have_been_enqueued
+        .with([subscription.installments.last])
     end
 
-    it 'deactivates expired subscriptions' do
-      sub = expiring_subscriptions.first
+    it 'schedules other actionable installments for processing' do
+      actionable_installment = create(:installment, :actionable)
+
+      described_class.run
 
-      expect { subject }.
-        to change { sub.reload.state }.
-        from('active').to('inactive')
+      expect(SolidusSubscriptions::ProcessInstallmentsJob).to have_been_enqueued
+        .with([actionable_installment])
     end
+  end
 
-    context 'the subscriptions have different shipping addresses' do
-      let!(:sub_to_different_address) do
-        create(:subscription, :actionable, :with_shipping_address, user: user)
-      end
+  shared_examples 'schedules the subscription for reprocessing' do
+    it 'advances the actionable_date' do
+      subscription
+      subscription.update_columns(interval_length: 1, interval_units: 'week')
+      old_actionable_date = subscription.actionable_date
 
-      it 'creates an order for each shipping address' do
-        expect { subject }.to change { Spree::Order.complete.count }.by 2
-      end
+      described_class.run
+
+      expect(subscription.reload.actionable_date.to_date).to eq((old_actionable_date + 1.week).to_date)
     end
+  end
 
-    context "when the config 'clear_past_installments' is enabled" do
-      it 'clears the past failed installments' do
-        stub_config(clear_past_installments: true)
+  context 'with a regular subscription' do
+    let(:subscription) { create(:subscription, :actionable) }
 
-        subject
+    include_examples 'processes the subscription'
+    include_examples 'schedules the subscription for reprocessing'
+  end
 
-        failed_installments.each do |fi|
-          expect(fi.reload.actionable_date).to eq(nil)
-        end
-      end
+  context 'with a subscription that is pending deactivation' do
+    let(:subscription) do
+      create(
+        :subscription,
+        :actionable,
+        interval_units: 'week',
+        interval_length: 2,
+        end_date: 3.days.from_now,
+      )
     end
 
-    context 'the subscriptions have different billing addresses' do
-      let!(:sub_to_different_address) do
-        create(:subscription, :actionable, :with_billing_address, user: user)
-      end
+    include_examples 'processes the subscription'
+    include_examples 'schedules the subscription for reprocessing'
 
-      it 'creates an order for each billing address' do
-        expect { subject }.to change { Spree::Order.complete.count }.by 2
-      end
-    end
+    it 'deactivates the subscription' do
+      subscription
 
-    context 'the subscription is cancelled with pending installments' do
-      let!(:cancelled_installment) do
-        installment = create(:installment, actionable_date: Time.zone.today)
-        installment.subscription.cancel!
-      end
+      described_class.run
 
-      it 'does not process the installment' do
-        expect { subject }.to change { Spree::Order.complete.count }.by expected_orders
-      end
+      expect(subscription.reload.state).to eq('inactive')
     end
   end
 
-  describe '.run' do
-    subject { described_class.run }
+  context 'with a subscription that is pending cancellation' do
+    let(:subscription) do
+      create(
+        :subscription,
+        :actionable,
+        :pending_cancellation,
+      )
+    end
+
+    include_examples 'processes the subscription'
+
+    it 'cancels the subscription' do
+      subscription
 
-    it_behaves_like 'a subscription order'
+      described_class.run
+
+      expect(subscription.reload.state).to eq('canceled')
+    end
   end
 
-  describe '#build_jobs' do
-    subject { described_class.new([user]).build_jobs }
+  context 'with a cancelled subscription with pending installments' do
+    it 'does not process the installment' do
+      subscription = create(:subscription)
+      create(:installment, subscription: subscription, actionable_date: Time.zone.today)
+      subscription.cancel!
 
-    it_behaves_like 'a subscription order'
+      described_class.run
+
+      expect(SolidusSubscriptions::ProcessInstallmentJob).not_to have_been_enqueued
+    end
   end
 end

From 0a0659d1150a7df8f0ca23e4876672e95a103f51 Mon Sep 17 00:00:00 2001
From: Alessandro Desantis <desa.alessandro@gmail.com>
Date: Thu, 19 Nov 2020 13:14:14 +0100
Subject: [PATCH 2/9] Process one installment at a time in background jobs

Instead of attempting to process multiple installments at a time, which
increases the chances something might go wrong, we are now only processing
one installment at a time.

This also improves the extension's performance, because the installments
can be processed in parallel.
---
 .../process_installment_job.rb                | 11 ++++++++
 .../process_installments_job.rb               | 24 ----------------
 lib/solidus_subscriptions/processor.rb        |  2 +-
 .../process_installment_job_spec.rb           | 11 ++++++++
 .../process_installments_job_spec.rb          | 28 -------------------
 .../solidus_subscriptions/processor_spec.rb   |  8 +++---
 6 files changed, 27 insertions(+), 57 deletions(-)
 create mode 100644 app/jobs/solidus_subscriptions/process_installment_job.rb
 delete mode 100644 app/jobs/solidus_subscriptions/process_installments_job.rb
 create mode 100644 spec/jobs/solidus_subscriptions/process_installment_job_spec.rb
 delete mode 100644 spec/jobs/solidus_subscriptions/process_installments_job_spec.rb

diff --git a/app/jobs/solidus_subscriptions/process_installment_job.rb b/app/jobs/solidus_subscriptions/process_installment_job.rb
new file mode 100644
index 00000000..6beeec6a
--- /dev/null
+++ b/app/jobs/solidus_subscriptions/process_installment_job.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module SolidusSubscriptions
+  class ProcessInstallmentJob < ApplicationJob
+    queue_as SolidusSubscriptions.configuration.processing_queue
+
+    def perform(installment)
+      Checkout.new([installment]).process
+    end
+  end
+end
diff --git a/app/jobs/solidus_subscriptions/process_installments_job.rb b/app/jobs/solidus_subscriptions/process_installments_job.rb
deleted file mode 100644
index e7e1f639..00000000
--- a/app/jobs/solidus_subscriptions/process_installments_job.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-# frozen_string_literal: true
-
-# This job is responsible for creating a consolidated installment from a
-# list of installments and processing it.
-
-module SolidusSubscriptions
-  class ProcessInstallmentsJob < ApplicationJob
-    queue_as SolidusSubscriptions.configuration.processing_queue
-
-    # Process a collection of installments
-    #
-    # @param installment_ids [Array<Integer>] The ids of the
-    #   installments to be processed together and fulfilled by the same order
-    #
-    # @return [Spree::Order] The order which fulfills the list of installments
-    def perform(installment_ids)
-      return if installment_ids.empty?
-
-      installments = SolidusSubscriptions::Installment.where(id: installment_ids).
-                     includes(subscription: [:line_items, :user])
-      Checkout.new(installments).process
-    end
-  end
-end
diff --git a/lib/solidus_subscriptions/processor.rb b/lib/solidus_subscriptions/processor.rb
index 4547ceca..b5aaa707 100644
--- a/lib/solidus_subscriptions/processor.rb
+++ b/lib/solidus_subscriptions/processor.rb
@@ -29,7 +29,7 @@ def process_subscription(subscription)
       end
 
       def process_installment(installment)
-        ProcessInstallmentsJob.perform_later(installment)
+        ProcessInstallmentJob.perform_later(installment)
       end
     end
   end
diff --git a/spec/jobs/solidus_subscriptions/process_installment_job_spec.rb b/spec/jobs/solidus_subscriptions/process_installment_job_spec.rb
new file mode 100644
index 00000000..4a33819c
--- /dev/null
+++ b/spec/jobs/solidus_subscriptions/process_installment_job_spec.rb
@@ -0,0 +1,11 @@
+RSpec.describe SolidusSubscriptions::ProcessInstallmentJob do
+  it 'processes checkout for the installment' do
+    installment = build_stubbed(:installment)
+    checkout = instance_spy(SolidusSubscriptions::Checkout)
+    allow(SolidusSubscriptions::Checkout).to receive(:new).with(installment).and_return(checkout)
+
+    described_class.perform_now(installment)
+
+    expect(checkout).to have_received(:process)
+  end
+end
diff --git a/spec/jobs/solidus_subscriptions/process_installments_job_spec.rb b/spec/jobs/solidus_subscriptions/process_installments_job_spec.rb
deleted file mode 100644
index 111fc058..00000000
--- a/spec/jobs/solidus_subscriptions/process_installments_job_spec.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-require 'spec_helper'
-
-RSpec.describe SolidusSubscriptions::ProcessInstallmentsJob do
-  let(:root_order) { create :completed_order_with_pending_payment }
-  let(:installments) do
-    traits = {
-      subscription_traits: [{
-        user: root_order.user,
-        line_item_traits: [{
-          spree_line_item: root_order.line_items.first
-        }]
-      }]
-    }
-
-    create_list(:installment, 2, traits)
-  end
-
-  describe '#perform' do
-    subject { described_class.new.perform(installments) }
-
-    it 'processes the consolidated installment' do
-      expect_any_instance_of(SolidusSubscriptions::Checkout).
-        to receive(:process).once
-
-      subject
-    end
-  end
-end
diff --git a/spec/lib/solidus_subscriptions/processor_spec.rb b/spec/lib/solidus_subscriptions/processor_spec.rb
index a7301f00..03be168c 100644
--- a/spec/lib/solidus_subscriptions/processor_spec.rb
+++ b/spec/lib/solidus_subscriptions/processor_spec.rb
@@ -46,8 +46,8 @@
 
       described_class.run
 
-      expect(SolidusSubscriptions::ProcessInstallmentsJob).to have_been_enqueued
-        .with([subscription.installments.last])
+      expect(SolidusSubscriptions::ProcessInstallmentJob).to have_been_enqueued
+        .with(subscription.installments.last)
     end
 
     it 'schedules other actionable installments for processing' do
@@ -55,8 +55,8 @@
 
       described_class.run
 
-      expect(SolidusSubscriptions::ProcessInstallmentsJob).to have_been_enqueued
-        .with([actionable_installment])
+      expect(SolidusSubscriptions::ProcessInstallmentJob).to have_been_enqueued
+        .with(actionable_installment)
     end
   end
 

From 16dd27dab051bfd7fd8edd7721391e3bd80a62ea Mon Sep 17 00:00:00 2001
From: Alessandro Desantis <desa.alessandro@gmail.com>
Date: Thu, 19 Nov 2020 13:14:38 +0100
Subject: [PATCH 3/9] Streamline and simplify `SolidusSubscriptions::Checkout`

This service object contained a lot of indirection and took on too
many responsibilities. The new version is much more streamlined and
the flow of operations should be much clearer.
---
 .../process_installment_job.rb                |   2 +-
 .../solidus_subscriptions/checkout.rb         | 170 ++-----
 .../solidus_subscriptions/order_builder.rb    |  42 --
 .../user_mismatch_error.rb                    |  17 -
 .../solidus_subscriptions/checkout_spec.rb    | 431 ++++--------------
 .../order_builder_spec.rb                     |  53 ---
 6 files changed, 127 insertions(+), 588 deletions(-)
 delete mode 100644 app/services/solidus_subscriptions/order_builder.rb
 delete mode 100644 app/services/solidus_subscriptions/user_mismatch_error.rb
 delete mode 100644 spec/services/solidus_subscriptions/order_builder_spec.rb

diff --git a/app/jobs/solidus_subscriptions/process_installment_job.rb b/app/jobs/solidus_subscriptions/process_installment_job.rb
index 6beeec6a..17eb446c 100644
--- a/app/jobs/solidus_subscriptions/process_installment_job.rb
+++ b/app/jobs/solidus_subscriptions/process_installment_job.rb
@@ -5,7 +5,7 @@ class ProcessInstallmentJob < ApplicationJob
     queue_as SolidusSubscriptions.configuration.processing_queue
 
     def perform(installment)
-      Checkout.new([installment]).process
+      Checkout.new(installment).process
     end
   end
 end
diff --git a/app/services/solidus_subscriptions/checkout.rb b/app/services/solidus_subscriptions/checkout.rb
index c40849a3..e348547b 100644
--- a/app/services/solidus_subscriptions/checkout.rb
+++ b/app/services/solidus_subscriptions/checkout.rb
@@ -1,155 +1,73 @@
 # frozen_string_literal: true
 
-# This class takes a collection of installments and populates a new spree
-# order with the correct contents based on the subscriptions associated to the
-# intallments. This is to group together subscriptions being
-# processed on the same day for a specific user
 module SolidusSubscriptions
   class Checkout
-    # @return [Array<Installment>] The collection of installments to be used
-    #   when generating a new order
-    attr_reader :installments
+    attr_reader :installment
 
-    delegate :user, to: :subscription
-
-    # Get a new instance of a Checkout
-    #
-    # @param installments [Array<Installment>] The collection of installments
-    # to be used when generating a new order
-    def initialize(installments)
-      @installments = installments
-      raise UserMismatchError.new(installments) if different_owners?
+    def initialize(installment)
+      @installment = installment
     end
 
-    # Generate a new Spree::Order based on the information associated to the
-    # installments
-    #
-    # @return [Spree::Order]
     def process
-      populate
-
-      # Installments are removed and set for future processing if they are
-      # out of stock. If there are no line items left there is nothing to do
-      return if installments.empty?
-
-      if checkout
-        SolidusSubscriptions.configuration.success_dispatcher_class.new(installments, order).dispatch
-        return order
+      order = create_order
+
+      begin
+        populate_order(order)
+        finalize_order(order)
+
+        SolidusSubscriptions.configuration.success_dispatcher_class.new([installment], order).dispatch
+      rescue StateMachines::InvalidTransition
+        if order.payments.any?(&:failed?)
+          SolidusSubscriptions.configuration.payment_failed_dispatcher_class.new([installment], order).dispatch
+        else
+          SolidusSubscriptions.configuration.failure_dispatcher_class.new([installment], order).dispatch
+        end
+      rescue ::Spree::Order::InsufficientStock
+        SolidusSubscriptions.configuration.out_of_stock_dispatcher_class.new([installment], order).dispatch
       end
 
-      # A new order will only have 1 payment that we created
-      if order.payments.any?(&:failed?)
-        SolidusSubscriptions.configuration.payment_failed_dispatcher_class.new(installments, order).dispatch
-        installments.clear
-        nil
-      end
-    ensure
-      # Any installments that failed to be processed will be reprocessed
-      unfulfilled_installments = installments.select(&:unfulfilled?)
-      if unfulfilled_installments.any?
-        SolidusSubscriptions.configuration.failure_dispatcher_class.
-          new(unfulfilled_installments, order).dispatch
-      end
+      order
     end
 
-    # The order fulfilling the consolidated installment
-    #
-    # @return [Spree::Order]
-    def order
-      @order ||= ::Spree::Order.create(
-        user: user,
-        email: user.email,
-        store: subscription.store || ::Spree::Store.default,
+    private
+
+    def create_order
+      ::Spree::Order.create(
+        user: installment.subscription.user,
+        email: installment.subscription.user.email,
+        store: installment.subscription.store || ::Spree::Store.default,
         subscription_order: true,
-        subscription: subscription
+        subscription: installment.subscription
       )
     end
 
-    private
+    def populate_order(order)
+      installment.subscription.line_items.each do |line_item|
+        order.contents.add(line_item.subscribable, line_item.quantity)
+      end
+    end
 
-    def checkout
+    def finalize_order(order)
+      ::Spree::PromotionHandler::Cart.new(order).activate
       order.recalculate
-      apply_promotions
 
       order.checkout_steps[0...-1].each do
         case order.state
-        when "address"
-          order.ship_address = ship_address
-          order.bill_address = bill_address
-        when "payment"
-          create_payment
+        when 'address'
+          order.ship_address = installment.subscription.shipping_address_to_use
+          order.bill_address = installment.subscription.billing_address_to_use
+        when 'payment'
+          order.payments.create(
+            payment_method: installment.subscription.payment_method_to_use,
+            source: installment.subscription.payment_source_to_use,
+            amount: order.total,
+          )
         end
 
         order.next!
       end
 
-      # Do this as a separate "quiet" transition so that it returns true or
-      # false rather than raising a failed transition error
-      order.complete
-    end
-
-    def populate
-      unfulfilled_installments = []
-
-      order_line_items = installments.flat_map do |installment|
-        line_items = installment.line_item_builder.spree_line_items
-
-        unfulfilled_installments.push(installment) if line_items.empty?
-
-        line_items
-      end
-
-      # Remove installments which had no stock from the active list
-      # They will be reprocessed later
-      @installments -= unfulfilled_installments
-      if unfulfilled_installments.any?
-        SolidusSubscriptions.configuration.out_of_stock_dispatcher_class.new(unfulfilled_installments).dispatch
-      end
-
-      return if installments.empty?
-
-      order_builder.add_line_items(order_line_items)
-    end
-
-    def order_builder
-      @order_builder ||= OrderBuilder.new(order)
-    end
-
-    def subscription
-      installments.first.subscription
-    end
-
-    def ship_address
-      subscription.shipping_address_to_use
-    end
-
-    def bill_address
-      subscription.billing_address_to_use
-    end
-
-    def payment_source
-      subscription.payment_source_to_use
-    end
-
-    def payment_method
-      subscription.payment_method_to_use
-    end
-
-    def create_payment
-      order.payments.create(
-        source: payment_source,
-        amount: order.total,
-        payment_method: payment_method,
-      )
-    end
-
-    def apply_promotions
-      ::Spree::PromotionHandler::Cart.new(order).activate
-      order.updater.update # reload totals
-    end
-
-    def different_owners?
-      installments.map { |i| i.subscription.user }.uniq.length > 1
+      order.complete!
     end
   end
 end
diff --git a/app/services/solidus_subscriptions/order_builder.rb b/app/services/solidus_subscriptions/order_builder.rb
deleted file mode 100644
index a577e987..00000000
--- a/app/services/solidus_subscriptions/order_builder.rb
+++ /dev/null
@@ -1,42 +0,0 @@
-# frozen_string_literal: true
-
-# This class is responsible for adding line items to order without going
-# through order contents.
-module SolidusSubscriptions
-  class OrderBuilder
-    attr_reader :order
-
-    # Get a new instance of an OrderBuilder
-    #
-    # @param order [Spree::Order] The order to be built
-    #
-    # @return [SolidusSubscriptions::OrderBuilder]
-    def initialize(order)
-      @order = order
-    end
-
-    # Add line items to an order. If the order already
-    # has a line item for a given variant_id, update the quantity. Otherwise
-    # add the line item to the order.
-    #
-    # @param items [Array<Spree::LineItem>] The order to add the line item to
-    # @return [Array<Spree::LineItem] The collection that was passed in
-    def add_line_items(items)
-      items.map { |item| add_item_to_order(item) }
-    end
-
-    private
-
-    def add_item_to_order(new_item)
-      line_item = order.line_items.detect do |li|
-        li.variant_id == new_item.variant_id
-      end
-
-      if line_item
-        line_item.increment!(:quantity, new_item.quantity)
-      else
-        order.line_items << new_item
-      end
-    end
-  end
-end
diff --git a/app/services/solidus_subscriptions/user_mismatch_error.rb b/app/services/solidus_subscriptions/user_mismatch_error.rb
deleted file mode 100644
index 1d227ca8..00000000
--- a/app/services/solidus_subscriptions/user_mismatch_error.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-# frozen_string_literal: true
-
-module SolidusSubscriptions
-  class UserMismatchError < StandardError
-    def initialize(installments)
-      @installments = installments
-    end
-
-    def to_s
-      <<-MSG.squish
-        Installments must have the same user to be processed as a consolidated
-        installment. Could not process installments:
-        #{@installments.map(&:id).join(', ')}
-      MSG
-    end
-  end
-end
diff --git a/spec/services/solidus_subscriptions/checkout_spec.rb b/spec/services/solidus_subscriptions/checkout_spec.rb
index 4b408dc8..f35b8c0f 100644
--- a/spec/services/solidus_subscriptions/checkout_spec.rb
+++ b/spec/services/solidus_subscriptions/checkout_spec.rb
@@ -1,389 +1,122 @@
-require 'spec_helper'
+RSpec.describe SolidusSubscriptions::Checkout, :checkout do
+  context 'when the order can be created and paid' do
+    it 'creates and finalizes a new order for the installment' do
+      stub_spree_preferences(auto_capture: true)
+      installment = create(:installment, :actionable)
 
-RSpec.describe SolidusSubscriptions::Checkout do
-  let(:checkout) { described_class.new(installments) }
-  let(:root_order) { create :completed_order_with_pending_payment, user: subscription_user }
-  let(:subscription_user) { create(:user, :subscription_user) }
-  let!(:credit_card) {
-    card = create(:credit_card, user: subscription_user, gateway_customer_profile_id: 'BGS-123', payment_method: payment_method)
-    wallet_payment_source = subscription_user.wallet.add(card)
-    subscription_user.wallet.default_wallet_payment_source = wallet_payment_source
-    card
-  }
-  let(:payment_method) { create(:payment_method) }
-  let(:installments) { create_list(:installment, 2, installment_traits) }
+      order = described_class.new(installment).process
 
-  let(:installment_traits) do
-    {
-      subscription_traits: [{
-        user: subscription_user,
-        line_item_traits: [{
-          spree_line_item: root_order.line_items.first
-        }]
-      }]
-    }
-  end
-
-  before do
-    Spree::Variant.all.each { |v| v.update(subscribable: true) }
-  end
-
-  context 'initialized with installments belonging to multiple users' do
-    subject { checkout }
-
-    let(:installments) { build_stubbed_list :installment, 2 }
-
-    it 'raises an error' do
-      expect { subject }.
-        to raise_error SolidusSubscriptions::UserMismatchError, /must have the same user/
+      expect(order).to be_complete
+      expect(order).to be_paid
     end
-  end
-
-  describe '#process', :checkout do
-    subject(:order) { checkout.process }
-
-    let(:subscription_line_item) { installments.first.subscription.line_items.first }
-
-    shared_examples 'a completed checkout' do
-      it { is_expected.to be_a Spree::Order }
-
-      let(:total) { 49.98 }
-      let(:quantity) { installments.length }
-
-      it 'has the correct number of line items' do
-        count = order.line_items.length
-        expect(count).to eq quantity
-      end
 
-      it 'the line items have the correct values' do
-        line_item = order.line_items.first
-        expect(line_item).to have_attributes(
-          quantity: subscription_line_item.quantity,
-          variant_id: subscription_line_item.subscribable_id
-        )
-      end
-
-      it 'has a shipment' do
-        expect(order.shipments).to be_present
-      end
-
-      it 'has a payment' do
-        expect(order.payments.valid).to be_present
-      end
-
-      it 'has the correct totals' do
-        expect(order).to have_attributes(
-          total: total,
-          shipment_total: 10
-        )
-      end
+    # rubocop:disable RSpec/MultipleExpectations
+    it 'copies basic information from the subscription' do
+      stub_spree_preferences(auto_capture: true)
+      installment = create(:installment, :actionable)
+      subscription = installment.subscription
 
-      it { is_expected.to be_complete }
+      order = described_class.new(installment).process
 
-      it 'associates the order to the installment detail' do
-        order
-        installment_orders = installments.flat_map { |i| i.details.map(&:order) }.compact
-        expect(installment_orders).to all eq order
-      end
-
-      it 'creates an installment detail for each installment' do
-        expect { subject }.
-          to change { SolidusSubscriptions::InstallmentDetail.count }.
-          by(installments.count)
-      end
+      expect(order.ship_address.value_attributes).to eq(subscription.shipping_address_to_use.value_attributes)
+      expect(order.bill_address.value_attributes).to eq(subscription.billing_address_to_use.value_attributes)
+      expect(order.payments.first.payment_method).to eq(subscription.payment_method_to_use)
+      expect(order.payments.first.source).to eq(subscription.payment_source_to_use)
+      expect(order.user).to eq(subscription.user)
+      expect(order.email).to eq(subscription.user.email)
     end
+    # rubocop:enable RSpec/MultipleExpectations
 
-    context 'no line items get added to the cart' do
-      before do
-        installments
-        Spree::StockItem.update_all(count_on_hand: 0, backorderable: false)
-      end
-
-      it 'creates two failed installment details' do
-        expect { order }.
-          to change { SolidusSubscriptions::InstallmentDetail.count }.
-          by(installments.length)
+    it 'marks the order as a subscription order' do
+      stub_spree_preferences(auto_capture: true)
+      installment = create(:installment, :actionable)
+      subscription = installment.subscription
 
-        details = SolidusSubscriptions::InstallmentDetail.last(installments.length)
-        expect(details).to all be_failed
-      end
+      order = described_class.new(installment).process
 
-      it { is_expected.to be_nil }
-
-      it 'creates no order' do
-        expect { subject }.not_to change { Spree::Order.count }
-      end
+      expect(order.subscription).to eq(subscription)
+      expect(order.subscription_order).to eq(true)
     end
 
-    if Gem::Specification.find_by_name('solidus').version >= Gem::Version.new('1.4.0')
-      context 'Altered checkout flow' do
-        before do
-          @old_checkout_flow = Spree::Order.checkout_flow
-          Spree::Order.remove_checkout_step(:delivery)
-        end
+    it 'matches the total on the subscription' do
+      stub_spree_preferences(auto_capture: true)
+      installment = create(:installment, :actionable)
+      subscription = installment.subscription
 
-        after do
-          Spree::Order.checkout_flow(&@old_checkout_flow)
-        end
+      order = described_class.new(installment).process
 
-        it 'has a payment' do
-          expect(order.payments.valid).to be_present
-        end
-
-        it 'has the correct totals' do
-          expect(order).to have_attributes(
-            total: 39.98,
-            shipment_total: 0
-          )
-        end
-
-        it { is_expected.to be_complete }
-      end
+      expect(order.item_total).to eq(subscription.line_items.first.subscribable.price)
     end
 
-    context 'the variant is out of stock' do
-      let(:subscription_line_item) { installments.last.subscription.line_items.first }
-      let(:expected_date) { Time.zone.today + SolidusSubscriptions.configuration.reprocessing_interval }
-
-      # Remove stock for 1 variant in the consolidated installment
-      before do
-        subscribable_id = installments.first.subscription.line_items.first.subscribable_id
-        variant = Spree::Variant.find(subscribable_id)
-        variant.stock_items.update_all(count_on_hand: 0, backorderable: false)
-      end
-
-      it 'creates a failed installment detail' do
-        subject
-        detail = installments.first.details.last
-
-        expect(detail).not_to be_successful
-        expect(detail.message).
-          to eq I18n.t('solidus_subscriptions.installment_details.out_of_stock')
-      end
-
-      it 'removes the installment from the list of installments' do
-        expect { subject }.
-          to change { checkout.installments.length }.
-          by(-1)
-      end
-
-      it_behaves_like 'a completed checkout' do
-        let(:total) { 29.99 }
-        let(:quantity) { installments.length - 1 }
-      end
-    end
-
-    context 'the payment fails' do
-      let(:payment_method) { create(:payment_method) }
-      let!(:credit_card) {
-        card = create(:credit_card, user: checkout.user, payment_method: payment_method)
-        wallet_payment_source = checkout.user.wallet.add(card)
-        checkout.user.wallet.default_wallet_payment_source = wallet_payment_source
-        card
-      }
-      let(:expected_date) { Time.zone.today + SolidusSubscriptions.configuration.reprocessing_interval }
-
-      it { is_expected.to be_nil }
-
-      it 'marks all of the installments as failed' do
-        subject
-
-        details = installments.map do |installments|
-          installments.details.reload.last
-        end
-
-        expect(details).to all be_failed && have_attributes(
-          message: I18n.t('solidus_subscriptions.installment_details.payment_failed')
-        )
-      end
+    it 'calls the success dispatcher' do
+      stub_spree_preferences(auto_capture: true)
+      installment = create(:installment, :actionable)
+      success_dispatcher = stub_dispatcher(SolidusSubscriptions::Dispatcher::SuccessDispatcher, installment)
 
-      it 'marks the installment to be reprocessed' do
-        subject
-        actionable_dates = installments.map do |installment|
-          installment.reload.actionable_date
-        end
+      described_class.new(installment).process
 
-        expect(actionable_dates).to all eq expected_date
-      end
+      expect(success_dispatcher).to have_received(:dispatch)
     end
+  end
 
-    context 'when there are cart promotions' do
-      let!(:promo) do
-        create(
-          :promotion,
-          :with_item_total_rule,
-          :with_order_adjustment,
-          promo_params
-        )
+  context 'when payment of the order fails' do
+    it 'calls the payment failed dispatcher' do
+      stub_spree_preferences(auto_capture: true)
+      installment = create(:installment, :actionable).tap do |i|
+        i.subscription.update!(payment_source: create(:credit_card, number: '4111123412341234'))
       end
+      payment_failed_dispatcher = stub_dispatcher(SolidusSubscriptions::Dispatcher::PaymentFailedDispatcher, installment)
 
-      # Promotions require the :apply_automatically flag to be auto applied in
-      # solidus versions greater than 1.0
-      let(:promo_params) do
-        {}.tap do |params|
-          if Spree::Promotion.new.respond_to?(:apply_automatically)
-            params[:apply_automatically] = true
-          end
-        end
-      end
+      described_class.new(installment).process
 
-      it_behaves_like 'a completed checkout' do
-        let(:total) { 39.98 }
-      end
-
-      it 'applies the correct adjustments' do
-        expect(subject.adjustments).to be_present
-      end
+      expect(payment_failed_dispatcher).to have_received(:dispatch)
     end
+  end
 
-    context 'there is an aribitrary failure' do
-      let(:expected_date) { Time.zone.today + SolidusSubscriptions.configuration.reprocessing_interval }
-
-      before do
-        allow(checkout).to receive(:populate).and_raise('arbitrary runtime error')
-      end
-
-      it 'advances the installment actionable dates', :aggregate_failures do
-        expect { subject }.to raise_error('arbitrary runtime error')
-
-        actionable_dates = installments.map do |installment|
-          installment.reload.actionable_date
+  context 'when an item is out of stock' do
+    it 'calls the out of stock dispatcher' do
+      stub_spree_preferences(auto_capture: true)
+      installment = create(:installment, :actionable).tap do |i|
+        i.subscription.line_items.first.subscribable.stock_items.each do |stock_item|
+          stock_item.update!(backorderable: false)
         end
-
-        expect(actionable_dates).to all eq expected_date
       end
-    end
-
-    context 'the user has store credit' do
-      let!(:store_credit) { create :store_credit, user: subscription_user }
-      let!(:store_credit_payment_method) { create :store_credit_payment_method }
+      out_of_stock_dispatcher = stub_dispatcher(SolidusSubscriptions::Dispatcher::OutOfStockDispatcher, installment)
 
-      it_behaves_like 'a completed checkout'
+      described_class.new(installment).process
 
-      it 'has a valid store credit payment' do
-        expect(order.payments.valid.store_credits).to be_present
-      end
-    end
-
-    context 'the subscription has a shipping address' do
-      let(:installment_traits) do
-        {
-          subscription_traits: [{
-            shipping_address: shipping_address,
-            user: subscription_user,
-            line_item_traits: [{ spree_line_item: root_order.line_items.first }]
-          }]
-        }
-      end
-      let(:shipping_address) { create :address }
-
-      it_behaves_like 'a completed checkout'
-
-      it 'ships to the subscription address' do
-        expect(subject.ship_address).to eq shipping_address
-      end
-    end
-
-    context 'the subscription has a billing address' do
-      let(:installment_traits) do
-        {
-          subscription_traits: [{
-            billing_address: billing_address,
-            user: subscription_user,
-            line_item_traits: [{ spree_line_item: root_order.line_items.first }]
-          }]
-        }
-      end
-      let(:billing_address) { create :address }
-
-      it_behaves_like 'a completed checkout'
-
-      it 'bills to the subscription address' do
-        expect(subject.bill_address).to eq billing_address
-      end
-    end
-
-    context 'the subscription has a payment method' do
-      let(:installment_traits) do
-        {
-          subscription_traits: [{
-            payment_method: payment_method,
-            user: subscription_user,
-            line_item_traits: [{ spree_line_item: root_order.line_items.first }]
-          }]
-        }
-      end
-      let(:payment_method) { create :check_payment_method }
-
-      it_behaves_like 'a completed checkout'
-
-      it 'pays with the payment method' do
-        expect(subject.payments.valid.first.payment_method).to eq payment_method
-      end
-    end
-
-    context 'the subscription has a payment method and a source' do
-      let(:installment_traits) do
-        {
-          subscription_traits: [{
-            payment_method: payment_method,
-            payment_source: payment_source,
-            user: subscription_user,
-            line_item_traits: [{ spree_line_item: root_order.line_items.first }]
-          }]
-        }
-      end
-      let(:payment_source) { create :credit_card, payment_method: payment_method, user: subscription_user }
-      let(:payment_method) { create :credit_card_payment_method }
-
-      it_behaves_like 'a completed checkout'
-
-      it 'pays with the payment method' do
-        expect(subject.payments.valid.first.payment_method).to eq payment_method
-      end
-
-      it 'pays with the payment source' do
-        expect(subject.payments.valid.first.source).to eq payment_source
-      end
+      expect(out_of_stock_dispatcher).to have_received(:dispatch)
     end
+  end
 
-    context 'there are multiple associated subscritpion line items' do
-      it_behaves_like 'a completed checkout' do
-        let(:quantity) { subscription_line_items.length }
-      end
+  context 'when a generic transition error happens during checkout' do
+    it 'calls the failure dispatcher' do
+      stub_spree_preferences(auto_capture: true)
+      installment = create(:installment, :actionable)
+      failure_dispatcher = stub_dispatcher(SolidusSubscriptions::Dispatcher::FailureDispatcher, installment)
+      # rubocop:disable RSpec/AnyInstance
+      allow_any_instance_of(Spree::Order).to receive(:next!)
+        .and_raise(StateMachines::InvalidTransition.new(
+          Spree::Order.new,
+          Spree::Order.state_machines[:state],
+          :next,
+        ))
+      # rubocop:enable RSpec/AnyInstance
 
-      let(:installments) { create_list(:installment, 1, installment_traits) }
-      let(:subscription_line_items) { create_list(:subscription_line_item, 2, quantity: 1) }
+      described_class.new(installment).process
 
-      let(:installment_traits) do
-        {
-          subscription_traits: [{
-            user: subscription_user,
-            line_items: subscription_line_items
-          }]
-        }
-      end
+      expect(failure_dispatcher).to have_received(:dispatch)
     end
   end
 
-  describe '#order' do
-    subject { checkout.order }
-
-    let(:user) { installments.first.subscription.user }
-
-    it { is_expected.to be_a Spree::Order }
-
-    it 'has the correct attributes' do
-      expect(subject).to have_attributes(
-        user: user,
-        email: user.email,
-        store: installments.first.subscription.store
-      )
-    end
+  private
 
-    it 'is the same instance any time its called' do
-      order = checkout.order
-      expect(subject).to equal order
+  def stub_dispatcher(klass, installment)
+    instance_spy(klass).tap do |dispatcher|
+      allow(klass).to receive(:new).with(
+        [installment],
+        an_instance_of(Spree::Order)
+      ).and_return(dispatcher)
     end
   end
 end
diff --git a/spec/services/solidus_subscriptions/order_builder_spec.rb b/spec/services/solidus_subscriptions/order_builder_spec.rb
deleted file mode 100644
index 3463bd14..00000000
--- a/spec/services/solidus_subscriptions/order_builder_spec.rb
+++ /dev/null
@@ -1,53 +0,0 @@
-require 'spec_helper'
-
-RSpec.describe SolidusSubscriptions::OrderBuilder do
-  let(:builder) { described_class.new order }
-
-  describe '#add_line_items' do
-    subject { builder.add_line_items([line_item]) }
-
-    let(:variant) { create :variant, subscribable: true }
-    let(:order) do
-      create :order, line_items_attributes: line_items_attributes
-    end
-
-    let(:line_item) { create(:line_item, quantity: 4, variant: variant) }
-
-    context 'the line item doesnt already exist on the order' do
-      let(:line_items_attributes) { [] }
-
-      it 'adds a new line item to the order' do
-        expect { subject }.
-          to change { order.line_items.count }.
-          from(0).to(1)
-      end
-
-      it 'has a line item with the correct values' do
-        subject
-        line_item = order.line_items.last
-
-        expect(line_item).to have_attributes(
-          variant_id: variant.id,
-          quantity: line_item.quantity
-        )
-      end
-    end
-
-    context 'the line item already exists on the order' do
-      let(:line_items_attributes) do
-        [{
-          variant: variant,
-          quantity: 3
-        }]
-      end
-
-      it 'increases the correct line item by the correct amount' do
-        existing_line_item = order.line_items.first
-
-        expect { subject }.
-          to change { existing_line_item.reload.quantity }.
-          by(line_item.quantity)
-      end
-    end
-  end
-end

From 2836765c00abc1ba76793ad88c07a3d89a44d672 Mon Sep 17 00:00:00 2001
From: Alessandro Desantis <desa.alessandro@gmail.com>
Date: Fri, 27 Nov 2020 16:52:05 +0100
Subject: [PATCH 4/9] Reorganize hierarchy of dispatcher classes

---
 .../solidus_subscriptions/dispatcher.rb       | 23 -----------------
 .../solidus_subscriptions/dispatcher/base.rb  | 25 +++++++++++++++++++
 .../dispatcher/failure_dispatcher.rb          | 16 ++++++++++++
 .../out_of_stock_dispatcher.rb                |  8 +++---
 .../dispatcher/payment_failed_dispatcher.rb   | 22 ++++++++++++++++
 .../dispatcher/success_dispatcher.rb          | 20 +++++++++++++++
 .../failure_dispatcher.rb                     | 14 -----------
 .../payment_failed_dispatcher.rb              | 20 ---------------
 .../success_dispatcher.rb                     | 18 -------------
 .../install/templates/initializer.rb          |  8 +++---
 lib/solidus_subscriptions/configuration.rb    |  8 +++---
 .../{ => dispatcher}/dispatcher_spec.rb       |  2 +-
 .../failure_dispatcher_spec.rb                |  2 +-
 .../out_of_stock_dispatcher_spec.rb           |  2 +-
 .../payment_failed_dispatcher_spec.rb         |  2 +-
 .../success_dispatcher_spec.rb                |  2 +-
 16 files changed, 101 insertions(+), 91 deletions(-)
 delete mode 100644 app/services/solidus_subscriptions/dispatcher.rb
 create mode 100644 app/services/solidus_subscriptions/dispatcher/base.rb
 create mode 100644 app/services/solidus_subscriptions/dispatcher/failure_dispatcher.rb
 rename app/services/solidus_subscriptions/{ => dispatcher}/out_of_stock_dispatcher.rb (52%)
 create mode 100644 app/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb
 create mode 100644 app/services/solidus_subscriptions/dispatcher/success_dispatcher.rb
 delete mode 100644 app/services/solidus_subscriptions/failure_dispatcher.rb
 delete mode 100644 app/services/solidus_subscriptions/payment_failed_dispatcher.rb
 delete mode 100644 app/services/solidus_subscriptions/success_dispatcher.rb
 rename spec/services/solidus_subscriptions/{ => dispatcher}/dispatcher_spec.rb (79%)
 rename spec/services/solidus_subscriptions/{ => dispatcher}/failure_dispatcher_spec.rb (92%)
 rename spec/services/solidus_subscriptions/{ => dispatcher}/out_of_stock_dispatcher_spec.rb (82%)
 rename spec/services/solidus_subscriptions/{ => dispatcher}/payment_failed_dispatcher_spec.rb (94%)
 rename spec/services/solidus_subscriptions/{ => dispatcher}/success_dispatcher_spec.rb (92%)

diff --git a/app/services/solidus_subscriptions/dispatcher.rb b/app/services/solidus_subscriptions/dispatcher.rb
deleted file mode 100644
index 0472f792..00000000
--- a/app/services/solidus_subscriptions/dispatcher.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-# frozen_string_literal: true
-
-module SolidusSubscriptions
-  class Dispatcher
-    attr_reader :installments, :order
-
-    # Returns a new instance of this dispatcher.
-    #
-    # @param installments [Array<SolidusSubscriptions::Installment>] The installments to process
-    #   with this dispatcher
-    # @param order [Spree::Order] The order that was generated as a result of these installments
-    #
-    # @return [SolidusSubscriptions::Dispatcher]
-    def initialize(installments, order = nil)
-      @installments = installments
-      @order = order
-    end
-
-    def dispatch
-      raise NotImplementedError
-    end
-  end
-end
diff --git a/app/services/solidus_subscriptions/dispatcher/base.rb b/app/services/solidus_subscriptions/dispatcher/base.rb
new file mode 100644
index 00000000..84b87101
--- /dev/null
+++ b/app/services/solidus_subscriptions/dispatcher/base.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module SolidusSubscriptions
+  module Dispatcher
+    class Base
+      attr_reader :installments, :order
+
+      # Returns a new instance of this dispatcher.
+      #
+      # @param installments [Array<SolidusSubscriptions::Installment>] The installments to process
+      #   with this dispatcher
+      # @param order [Spree::Order] The order that was generated as a result of these installments
+      #
+      # @return [SolidusSubscriptions::Dispatcher]
+      def initialize(installments, order = nil)
+        @installments = installments
+        @order = order
+      end
+
+      def dispatch
+        raise NotImplementedError
+      end
+    end
+  end
+end
diff --git a/app/services/solidus_subscriptions/dispatcher/failure_dispatcher.rb b/app/services/solidus_subscriptions/dispatcher/failure_dispatcher.rb
new file mode 100644
index 00000000..286a4c94
--- /dev/null
+++ b/app/services/solidus_subscriptions/dispatcher/failure_dispatcher.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+# Handles failed installments.
+module SolidusSubscriptions
+  module Dispatcher
+    class FailureDispatcher < Base
+      def dispatch
+        order.touch(:completed_at)
+        order.cancel
+        installments.each do |installment|
+          installment.failed!(order)
+        end
+      end
+    end
+  end
+end
diff --git a/app/services/solidus_subscriptions/out_of_stock_dispatcher.rb b/app/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb
similarity index 52%
rename from app/services/solidus_subscriptions/out_of_stock_dispatcher.rb
rename to app/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb
index 05484f4b..ef79f843 100644
--- a/app/services/solidus_subscriptions/out_of_stock_dispatcher.rb
+++ b/app/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb
@@ -2,9 +2,11 @@
 
 # Handles installments that cannot be processed for lack of stock.
 module SolidusSubscriptions
-  class OutOfStockDispatcher < Dispatcher
-    def dispatch
-      installments.each(&:out_of_stock)
+  module Dispatcher
+    class OutOfStockDispatcher < Base
+      def dispatch
+        installments.each(&:out_of_stock)
+      end
     end
   end
 end
diff --git a/app/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb b/app/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb
new file mode 100644
index 00000000..74bee883
--- /dev/null
+++ b/app/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+# Handles payment failures for subscription installments.
+module SolidusSubscriptions
+  module Dispatcher
+    class PaymentFailedDispatcher < Base
+      def dispatch
+        order.touch(:completed_at)
+        order.cancel
+        installments.each do |installment|
+          installment.payment_failed!(order)
+        end
+
+        ::Spree::Event.fire(
+          'solidus_subscriptions.installments_failed_payment',
+          installments: installments,
+          order: order,
+        )
+      end
+    end
+  end
+end
diff --git a/app/services/solidus_subscriptions/dispatcher/success_dispatcher.rb b/app/services/solidus_subscriptions/dispatcher/success_dispatcher.rb
new file mode 100644
index 00000000..78563a45
--- /dev/null
+++ b/app/services/solidus_subscriptions/dispatcher/success_dispatcher.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+# Handles installments that are processed successfully.
+module SolidusSubscriptions
+  module Dispatcher
+    class SuccessDispatcher < Base
+      def dispatch
+        installments.each do |installment|
+          installment.success!(order)
+        end
+
+        ::Spree::Event.fire(
+          'solidus_subscriptions.installments_succeeded',
+          installments: installments,
+          order: order,
+        )
+      end
+    end
+  end
+end
diff --git a/app/services/solidus_subscriptions/failure_dispatcher.rb b/app/services/solidus_subscriptions/failure_dispatcher.rb
deleted file mode 100644
index c77d4b02..00000000
--- a/app/services/solidus_subscriptions/failure_dispatcher.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-# frozen_string_literal: true
-
-# Handles failed installments.
-module SolidusSubscriptions
-  class FailureDispatcher < Dispatcher
-    def dispatch
-      order.touch(:completed_at)
-      order.cancel
-      installments.each do |installment|
-        installment.failed!(order)
-      end
-    end
-  end
-end
diff --git a/app/services/solidus_subscriptions/payment_failed_dispatcher.rb b/app/services/solidus_subscriptions/payment_failed_dispatcher.rb
deleted file mode 100644
index 29eb2913..00000000
--- a/app/services/solidus_subscriptions/payment_failed_dispatcher.rb
+++ /dev/null
@@ -1,20 +0,0 @@
-# frozen_string_literal: true
-
-# Handles payment failures for subscription installments.
-module SolidusSubscriptions
-  class PaymentFailedDispatcher < Dispatcher
-    def dispatch
-      order.touch(:completed_at)
-      order.cancel
-      installments.each do |installment|
-        installment.payment_failed!(order)
-      end
-
-      ::Spree::Event.fire(
-        'solidus_subscriptions.installments_failed_payment',
-        installments: installments,
-        order: order,
-      )
-    end
-  end
-end
diff --git a/app/services/solidus_subscriptions/success_dispatcher.rb b/app/services/solidus_subscriptions/success_dispatcher.rb
deleted file mode 100644
index ce552662..00000000
--- a/app/services/solidus_subscriptions/success_dispatcher.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-# frozen_string_literal: true
-
-# Handles installments that are processed successfully.
-module SolidusSubscriptions
-  class SuccessDispatcher < Dispatcher
-    def dispatch
-      installments.each do |installment|
-        installment.success!(order)
-      end
-
-      ::Spree::Event.fire(
-        'solidus_subscriptions.installments_succeeded',
-        installments: installments,
-        order: order,
-      )
-    end
-  end
-end
diff --git a/lib/generators/solidus_subscriptions/install/templates/initializer.rb b/lib/generators/solidus_subscriptions/install/templates/initializer.rb
index faf03f4b..31e44b54 100644
--- a/lib/generators/solidus_subscriptions/install/templates/initializer.rb
+++ b/lib/generators/solidus_subscriptions/install/templates/initializer.rb
@@ -32,16 +32,16 @@
   # subclass from the the dispatcher you are replacing and call `super` from within `#dispatch`.
 
   # This handler is called when a subscription order is successfully placed.
-  # config.success_dispatcher_class = 'SolidusSubscriptions::SuccessDispatcher'
+  # config.success_dispatcher_class = 'SolidusSubscriptions::Dispatcher::SuccessDispatcher'
 
   # This handler is called when a group of installments fails to be processed.
-  # config.failure_dispatcher_class = 'SolidusSubscriptions::FailureDispatcher'
+  # config.failure_dispatcher_class = 'SolidusSubscriptions::Dispatcher::FailureDispatcher'
 
   # This handler is called when a payment fails on a subscription order.
-  # config.payment_failed_dispatcher_class = 'SolidusSubscriptions::PaymentFailedDispatcher'
+  # config.payment_failed_dispatcher_class = 'SolidusSubscriptions::Dispatcher::PaymentFailedDispatcher'
 
   # This handler is called when there isn't enough stock to fulfill an installment.
-  # config.out_of_stock_dispatcher = 'SolidusSubscriptions::OutOfStockDispatcher'
+  # config.out_of_stock_dispatcher = 'SolidusSubscriptions::Dispatcher::OutOfStockDispatcher'
 
   # ===================================== Permitted attributes =====================================
   #
diff --git a/lib/solidus_subscriptions/configuration.rb b/lib/solidus_subscriptions/configuration.rb
index 52c8fd49..536aad3a 100644
--- a/lib/solidus_subscriptions/configuration.rb
+++ b/lib/solidus_subscriptions/configuration.rb
@@ -15,22 +15,22 @@ class Configuration
     )
 
     def success_dispatcher_class
-      @success_dispatcher_class ||= 'SolidusSubscriptions::SuccessDispatcher'
+      @success_dispatcher_class ||= 'SolidusSubscriptions::Dispatcher::SuccessDispatcher'
       @success_dispatcher_class.constantize
     end
 
     def failure_dispatcher_class
-      @failure_dispatcher_class ||= 'SolidusSubscriptions::FailureDispatcher'
+      @failure_dispatcher_class ||= 'SolidusSubscriptions::Dispatcher::FailureDispatcher'
       @failure_dispatcher_class.constantize
     end
 
     def payment_failed_dispatcher_class
-      @payment_failed_dispatcher_class ||= 'SolidusSubscriptions::PaymentFailedDispatcher'
+      @payment_failed_dispatcher_class ||= 'SolidusSubscriptions::Dispatcher::PaymentFailedDispatcher'
       @payment_failed_dispatcher_class.constantize
     end
 
     def out_of_stock_dispatcher_class
-      @out_of_stock_dispatcher_class ||= 'SolidusSubscriptions::OutOfStockDispatcher'
+      @out_of_stock_dispatcher_class ||= 'SolidusSubscriptions::Dispatcher::OutOfStockDispatcher'
       @out_of_stock_dispatcher_class.constantize
     end
 
diff --git a/spec/services/solidus_subscriptions/dispatcher_spec.rb b/spec/services/solidus_subscriptions/dispatcher/dispatcher_spec.rb
similarity index 79%
rename from spec/services/solidus_subscriptions/dispatcher_spec.rb
rename to spec/services/solidus_subscriptions/dispatcher/dispatcher_spec.rb
index 49c3f4b6..7590422d 100644
--- a/spec/services/solidus_subscriptions/dispatcher_spec.rb
+++ b/spec/services/solidus_subscriptions/dispatcher/dispatcher_spec.rb
@@ -1,4 +1,4 @@
-RSpec.describe SolidusSubscriptions::Dispatcher do
+RSpec.describe SolidusSubscriptions::Dispatcher::Base do
   describe '#dispatch' do
     it 'raises a NotImplementedError' do
       dispatcher = described_class.new([])
diff --git a/spec/services/solidus_subscriptions/failure_dispatcher_spec.rb b/spec/services/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb
similarity index 92%
rename from spec/services/solidus_subscriptions/failure_dispatcher_spec.rb
rename to spec/services/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb
index a7c18ea5..55a883ac 100644
--- a/spec/services/solidus_subscriptions/failure_dispatcher_spec.rb
+++ b/spec/services/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb
@@ -1,4 +1,4 @@
-RSpec.describe SolidusSubscriptions::FailureDispatcher do
+RSpec.describe SolidusSubscriptions::Dispatcher::FailureDispatcher do
   describe '#dispatch' do
     it 'marks all the installments as failed' do
       installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
diff --git a/spec/services/solidus_subscriptions/out_of_stock_dispatcher_spec.rb b/spec/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb
similarity index 82%
rename from spec/services/solidus_subscriptions/out_of_stock_dispatcher_spec.rb
rename to spec/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb
index 6f3825ca..4ed0a53c 100644
--- a/spec/services/solidus_subscriptions/out_of_stock_dispatcher_spec.rb
+++ b/spec/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb
@@ -1,4 +1,4 @@
-RSpec.describe SolidusSubscriptions::OutOfStockDispatcher do
+RSpec.describe SolidusSubscriptions::Dispatcher::OutOfStockDispatcher do
   describe '#dispatch' do
     it 'marks all the installments as out of stock' do
       installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
diff --git a/spec/services/solidus_subscriptions/payment_failed_dispatcher_spec.rb b/spec/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb
similarity index 94%
rename from spec/services/solidus_subscriptions/payment_failed_dispatcher_spec.rb
rename to spec/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb
index a6a8a0ca..99794489 100644
--- a/spec/services/solidus_subscriptions/payment_failed_dispatcher_spec.rb
+++ b/spec/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb
@@ -1,4 +1,4 @@
-RSpec.describe SolidusSubscriptions::PaymentFailedDispatcher do
+RSpec.describe SolidusSubscriptions::Dispatcher::PaymentFailedDispatcher do
   describe '#dispatch' do
     it 'marks all the installments as payment_failed' do
       installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
diff --git a/spec/services/solidus_subscriptions/success_dispatcher_spec.rb b/spec/services/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb
similarity index 92%
rename from spec/services/solidus_subscriptions/success_dispatcher_spec.rb
rename to spec/services/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb
index ce41638a..aa1f72fe 100644
--- a/spec/services/solidus_subscriptions/success_dispatcher_spec.rb
+++ b/spec/services/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb
@@ -1,4 +1,4 @@
-RSpec.describe SolidusSubscriptions::SuccessDispatcher do
+RSpec.describe SolidusSubscriptions::Dispatcher::SuccessDispatcher do
   describe '#dispatch' do
     it 'marks all the installments as success' do
       installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }

From 784c1f07c61964c1a05549bffb0a709763bcf2d6 Mon Sep 17 00:00:00 2001
From: Alessandro Desantis <desa.alessandro@gmail.com>
Date: Fri, 27 Nov 2020 17:55:00 +0100
Subject: [PATCH 5/9] Pass one installment at a time to dispatcher classes

---
 .rubocop.yml                                  |  3 +++
 .../solidus_subscriptions/checkout.rb         |  8 ++++----
 .../solidus_subscriptions/dispatcher/base.rb  | 13 +++---------
 .../dispatcher/failure_dispatcher.rb          |  5 +----
 .../dispatcher/out_of_stock_dispatcher.rb     |  3 +--
 .../dispatcher/payment_failed_dispatcher.rb   |  9 +++------
 .../dispatcher/success_dispatcher.rb          |  9 +++------
 .../churn_buster_subscriber.rb                |  4 ++--
 .../solidus_subscriptions/checkout_spec.rb    |  2 +-
 .../dispatcher/dispatcher_spec.rb             | 11 ----------
 .../dispatcher/failure_dispatcher_spec.rb     | 12 +++++------
 .../out_of_stock_dispatcher_spec.rb           |  9 +++++----
 .../payment_failed_dispatcher_spec.rb         | 20 +++++++++----------
 .../dispatcher/success_dispatcher_spec.rb     | 16 +++++++--------
 .../churn_buster_subscriber_spec.rb           | 12 +++++------
 15 files changed, 56 insertions(+), 80 deletions(-)
 delete mode 100644 spec/services/solidus_subscriptions/dispatcher/dispatcher_spec.rb

diff --git a/.rubocop.yml b/.rubocop.yml
index fcd4bc4c..97c9838f 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -7,3 +7,6 @@ RSpec/DescribeClass:
   Exclude:
     - spec/requests/**/*
     - spec/features/**/*
+
+Rails/SkipsModelValidations:
+  Enabled: false
diff --git a/app/services/solidus_subscriptions/checkout.rb b/app/services/solidus_subscriptions/checkout.rb
index e348547b..83fc35c2 100644
--- a/app/services/solidus_subscriptions/checkout.rb
+++ b/app/services/solidus_subscriptions/checkout.rb
@@ -15,15 +15,15 @@ def process
         populate_order(order)
         finalize_order(order)
 
-        SolidusSubscriptions.configuration.success_dispatcher_class.new([installment], order).dispatch
+        SolidusSubscriptions.configuration.success_dispatcher_class.new(installment, order).dispatch
       rescue StateMachines::InvalidTransition
         if order.payments.any?(&:failed?)
-          SolidusSubscriptions.configuration.payment_failed_dispatcher_class.new([installment], order).dispatch
+          SolidusSubscriptions.configuration.payment_failed_dispatcher_class.new(installment, order).dispatch
         else
-          SolidusSubscriptions.configuration.failure_dispatcher_class.new([installment], order).dispatch
+          SolidusSubscriptions.configuration.failure_dispatcher_class.new(installment, order).dispatch
         end
       rescue ::Spree::Order::InsufficientStock
-        SolidusSubscriptions.configuration.out_of_stock_dispatcher_class.new([installment], order).dispatch
+        SolidusSubscriptions.configuration.out_of_stock_dispatcher_class.new(installment, order).dispatch
       end
 
       order
diff --git a/app/services/solidus_subscriptions/dispatcher/base.rb b/app/services/solidus_subscriptions/dispatcher/base.rb
index 84b87101..30c50346 100644
--- a/app/services/solidus_subscriptions/dispatcher/base.rb
+++ b/app/services/solidus_subscriptions/dispatcher/base.rb
@@ -3,17 +3,10 @@
 module SolidusSubscriptions
   module Dispatcher
     class Base
-      attr_reader :installments, :order
+      attr_reader :installment, :order
 
-      # Returns a new instance of this dispatcher.
-      #
-      # @param installments [Array<SolidusSubscriptions::Installment>] The installments to process
-      #   with this dispatcher
-      # @param order [Spree::Order] The order that was generated as a result of these installments
-      #
-      # @return [SolidusSubscriptions::Dispatcher]
-      def initialize(installments, order = nil)
-        @installments = installments
+      def initialize(installment, order)
+        @installment = installment
         @order = order
       end
 
diff --git a/app/services/solidus_subscriptions/dispatcher/failure_dispatcher.rb b/app/services/solidus_subscriptions/dispatcher/failure_dispatcher.rb
index 286a4c94..54f8f2b7 100644
--- a/app/services/solidus_subscriptions/dispatcher/failure_dispatcher.rb
+++ b/app/services/solidus_subscriptions/dispatcher/failure_dispatcher.rb
@@ -1,15 +1,12 @@
 # frozen_string_literal: true
 
-# Handles failed installments.
 module SolidusSubscriptions
   module Dispatcher
     class FailureDispatcher < Base
       def dispatch
         order.touch(:completed_at)
         order.cancel
-        installments.each do |installment|
-          installment.failed!(order)
-        end
+        installment.failed!(order)
       end
     end
   end
diff --git a/app/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb b/app/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb
index ef79f843..fe3e7014 100644
--- a/app/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb
+++ b/app/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb
@@ -1,11 +1,10 @@
 # frozen_string_literal: true
 
-# Handles installments that cannot be processed for lack of stock.
 module SolidusSubscriptions
   module Dispatcher
     class OutOfStockDispatcher < Base
       def dispatch
-        installments.each(&:out_of_stock)
+        installment.out_of_stock
       end
     end
   end
diff --git a/app/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb b/app/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb
index 74bee883..a56332ed 100644
--- a/app/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb
+++ b/app/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb
@@ -1,19 +1,16 @@
 # frozen_string_literal: true
 
-# Handles payment failures for subscription installments.
 module SolidusSubscriptions
   module Dispatcher
     class PaymentFailedDispatcher < Base
       def dispatch
         order.touch(:completed_at)
         order.cancel
-        installments.each do |installment|
-          installment.payment_failed!(order)
-        end
+        installment.payment_failed!(order)
 
         ::Spree::Event.fire(
-          'solidus_subscriptions.installments_failed_payment',
-          installments: installments,
+          'solidus_subscriptions.installment_failed_payment',
+          installment: installment,
           order: order,
         )
       end
diff --git a/app/services/solidus_subscriptions/dispatcher/success_dispatcher.rb b/app/services/solidus_subscriptions/dispatcher/success_dispatcher.rb
index 78563a45..0ae71dc7 100644
--- a/app/services/solidus_subscriptions/dispatcher/success_dispatcher.rb
+++ b/app/services/solidus_subscriptions/dispatcher/success_dispatcher.rb
@@ -1,17 +1,14 @@
 # frozen_string_literal: true
 
-# Handles installments that are processed successfully.
 module SolidusSubscriptions
   module Dispatcher
     class SuccessDispatcher < Base
       def dispatch
-        installments.each do |installment|
-          installment.success!(order)
-        end
+        installment.success!(order)
 
         ::Spree::Event.fire(
-          'solidus_subscriptions.installments_succeeded',
-          installments: installments,
+          'solidus_subscriptions.installment_succeeded',
+          installment: installment,
           order: order,
         )
       end
diff --git a/app/subscribers/solidus_subscriptions/churn_buster_subscriber.rb b/app/subscribers/solidus_subscriptions/churn_buster_subscriber.rb
index aa9dfa48..34540038 100644
--- a/app/subscribers/solidus_subscriptions/churn_buster_subscriber.rb
+++ b/app/subscribers/solidus_subscriptions/churn_buster_subscriber.rb
@@ -6,8 +6,8 @@ module ChurnBusterSubscriber
 
     event_action :report_subscription_cancellation, event_name: 'solidus_subscriptions.subscription_canceled'
     event_action :report_subscription_ending, event_name: 'solidus_subscriptions.subscription_ended'
-    event_action :report_payment_success, event_name: 'solidus_subscriptions.installments_succeeded'
-    event_action :report_payment_failure, event_name: 'solidus_subscriptions.installments_failed_payment'
+    event_action :report_payment_success, event_name: 'solidus_subscriptions.installment_succeeded'
+    event_action :report_payment_failure, event_name: 'solidus_subscriptions.installment_failed_payment'
     event_action :report_payment_method_change, event_name: 'solidus_subscriptions.subscription_payment_method_changed'
 
     def report_subscription_cancellation(event)
diff --git a/spec/services/solidus_subscriptions/checkout_spec.rb b/spec/services/solidus_subscriptions/checkout_spec.rb
index f35b8c0f..2a83d9bf 100644
--- a/spec/services/solidus_subscriptions/checkout_spec.rb
+++ b/spec/services/solidus_subscriptions/checkout_spec.rb
@@ -114,7 +114,7 @@
   def stub_dispatcher(klass, installment)
     instance_spy(klass).tap do |dispatcher|
       allow(klass).to receive(:new).with(
-        [installment],
+        installment,
         an_instance_of(Spree::Order)
       ).and_return(dispatcher)
     end
diff --git a/spec/services/solidus_subscriptions/dispatcher/dispatcher_spec.rb b/spec/services/solidus_subscriptions/dispatcher/dispatcher_spec.rb
deleted file mode 100644
index 7590422d..00000000
--- a/spec/services/solidus_subscriptions/dispatcher/dispatcher_spec.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-RSpec.describe SolidusSubscriptions::Dispatcher::Base do
-  describe '#dispatch' do
-    it 'raises a NotImplementedError' do
-      dispatcher = described_class.new([])
-
-      expect {
-        dispatcher.dispatch
-      }.to raise_error(NotImplementedError)
-    end
-  end
-end
diff --git a/spec/services/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb b/spec/services/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb
index 55a883ac..f2472374 100644
--- a/spec/services/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb
+++ b/spec/services/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb
@@ -1,13 +1,13 @@
 RSpec.describe SolidusSubscriptions::Dispatcher::FailureDispatcher do
   describe '#dispatch' do
-    it 'marks all the installments as failed' do
-      installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
+    it 'marks the installment as failed' do
+      installment = instance_spy(SolidusSubscriptions::Installment)
       order = create(:order_with_line_items)
 
-      dispatcher = described_class.new(installments, order)
+      dispatcher = described_class.new(installment, order)
       dispatcher.dispatch
 
-      expect(installments).to all(have_received(:failed!).with(order).once)
+      expect(installment).to have_received(:failed!).with(order)
     end
 
     it 'cancels the order' do
@@ -15,10 +15,10 @@
         skip 'Orders in `cart` state cannot be canceled starting from Solidus 2.11.'
       end
 
-      installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
+      installment = instance_spy(SolidusSubscriptions::Installment)
       order = create(:order_with_line_items)
 
-      dispatcher = described_class.new(installments, order)
+      dispatcher = described_class.new(installment, order)
       dispatcher.dispatch
 
       expect(order.state).to eq('canceled')
diff --git a/spec/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb b/spec/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb
index 4ed0a53c..abb8343f 100644
--- a/spec/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb
+++ b/spec/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb
@@ -1,12 +1,13 @@
 RSpec.describe SolidusSubscriptions::Dispatcher::OutOfStockDispatcher do
   describe '#dispatch' do
-    it 'marks all the installments as out of stock' do
-      installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
+    it 'marks the installment as out of stock' do
+      installment = instance_spy(SolidusSubscriptions::Installment)
+      order = build_stubbed(:order)
 
-      dispatcher = described_class.new(installments)
+      dispatcher = described_class.new(installment, order)
       dispatcher.dispatch
 
-      expect(installments).to all(have_received(:out_of_stock).once)
+      expect(installment).to have_received(:out_of_stock)
     end
   end
 end
diff --git a/spec/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb b/spec/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb
index 99794489..0a222d41 100644
--- a/spec/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb
+++ b/spec/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb
@@ -1,13 +1,13 @@
 RSpec.describe SolidusSubscriptions::Dispatcher::PaymentFailedDispatcher do
   describe '#dispatch' do
-    it 'marks all the installments as payment_failed' do
-      installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
+    it 'marks the installment as payment_failed' do
+      installment = instance_spy(SolidusSubscriptions::Installment)
       order = create(:order_with_line_items)
 
-      dispatcher = described_class.new(installments, order)
+      dispatcher = described_class.new(installment, order)
       dispatcher.dispatch
 
-      expect(installments).to all(have_received(:payment_failed!).with(order).once)
+      expect(installment).to have_received(:payment_failed!).with(order)
     end
 
     it 'cancels the order' do
@@ -15,10 +15,10 @@
         skip 'Orders in `cart` state cannot be canceled starting from Solidus 2.11.'
       end
 
-      installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
+      installment = instance_spy(SolidusSubscriptions::Installment)
       order = create(:order_with_line_items)
 
-      dispatcher = described_class.new(installments, order)
+      dispatcher = described_class.new(installment, order)
       dispatcher.dispatch
 
       expect(order.state).to eq('canceled')
@@ -26,15 +26,15 @@
 
     it 'fires an installments_failed_payment event' do
       stub_const('Spree::Event', class_spy(Spree::Event))
-      installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
+      installment = instance_spy(SolidusSubscriptions::Installment)
       order = create(:order_with_line_items)
 
-      dispatcher = described_class.new(installments, order)
+      dispatcher = described_class.new(installment, order)
       dispatcher.dispatch
 
       expect(Spree::Event).to have_received(:fire).with(
-        'solidus_subscriptions.installments_failed_payment',
-        installments: installments,
+        'solidus_subscriptions.installment_failed_payment',
+        installment: installment,
         order: order,
       )
     end
diff --git a/spec/services/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb b/spec/services/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb
index aa1f72fe..6a718003 100644
--- a/spec/services/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb
+++ b/spec/services/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb
@@ -1,26 +1,26 @@
 RSpec.describe SolidusSubscriptions::Dispatcher::SuccessDispatcher do
   describe '#dispatch' do
-    it 'marks all the installments as success' do
-      installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
+    it 'marks the installment as success' do
+      installment = instance_spy(SolidusSubscriptions::Installment)
       order = create(:order_with_line_items)
 
-      dispatcher = described_class.new(installments, order)
+      dispatcher = described_class.new(installment, order)
       dispatcher.dispatch
 
-      expect(installments).to all(have_received(:success!).with(order).once)
+      expect(installment).to have_received(:success!).with(order)
     end
 
     it 'fires an installments_succeeded event' do
       stub_const('Spree::Event', class_spy(Spree::Event))
-      installments = Array.new(2) { instance_spy(SolidusSubscriptions::Installment) }
+      installment = instance_spy(SolidusSubscriptions::Installment)
       order = create(:order_with_line_items)
 
-      dispatcher = described_class.new(installments, order)
+      dispatcher = described_class.new(installment, order)
       dispatcher.dispatch
 
       expect(Spree::Event).to have_received(:fire).with(
-        'solidus_subscriptions.installments_succeeded',
-        installments: installments,
+        'solidus_subscriptions.installment_succeeded',
+        installment: installment,
         order: order,
       )
     end
diff --git a/spec/subscribers/solidus_subscriptions/churn_buster_subscriber_spec.rb b/spec/subscribers/solidus_subscriptions/churn_buster_subscriber_spec.rb
index 572913bd..11ada2c4 100644
--- a/spec/subscribers/solidus_subscriptions/churn_buster_subscriber_spec.rb
+++ b/spec/subscribers/solidus_subscriptions/churn_buster_subscriber_spec.rb
@@ -29,10 +29,10 @@
       allow(SolidusSubscriptions).to receive(:churn_buster).and_return(churn_buster)
 
       order = build_stubbed(:order)
-      installments = build_list(:installment, 2)
+      installment = build_stubbed(:installment)
       Spree::Event.fire(
-        'solidus_subscriptions.installments_succeeded',
-        installments: installments,
+        'solidus_subscriptions.installment_succeeded',
+        installment: installment,
         order: order,
       )
 
@@ -46,10 +46,10 @@
       allow(SolidusSubscriptions).to receive(:churn_buster).and_return(churn_buster)
 
       order = build_stubbed(:order)
-      installments = build_list(:installment, 2)
+      installment = build_stubbed(:installment)
       Spree::Event.fire(
-        'solidus_subscriptions.installments_failed_payment',
-        installments: installments,
+        'solidus_subscriptions.installment_failed_payment',
+        installment: installment,
         order: order,
       )
 

From b4cd6b6e177df2603f370e050db291090668965e Mon Sep 17 00:00:00 2001
From: Alessandro Desantis <desa.alessandro@gmail.com>
Date: Wed, 20 Jan 2021 10:37:31 +0100
Subject: [PATCH 6/9] Set queue on `ProcessInstallmentJob` dynamically

By setting the queue through a block rather than a boot-time method
call, we ensure to play nicely with configuration stubs and in other
non-obvious scenarios.
---
 app/jobs/solidus_subscriptions/process_installment_job.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/jobs/solidus_subscriptions/process_installment_job.rb b/app/jobs/solidus_subscriptions/process_installment_job.rb
index 17eb446c..127c787c 100644
--- a/app/jobs/solidus_subscriptions/process_installment_job.rb
+++ b/app/jobs/solidus_subscriptions/process_installment_job.rb
@@ -2,7 +2,7 @@
 
 module SolidusSubscriptions
   class ProcessInstallmentJob < ApplicationJob
-    queue_as SolidusSubscriptions.configuration.processing_queue
+    queue_as { SolidusSubscriptions.configuration.processing_queue }
 
     def perform(installment)
       Checkout.new(installment).process

From 58972478854a4f8f137506591a186a4d528900b9 Mon Sep 17 00:00:00 2001
From: Alessandro Desantis <desa.alessandro@gmail.com>
Date: Wed, 20 Jan 2021 10:37:41 +0100
Subject: [PATCH 7/9] Resume processing installments for cancelled
 subscriptions

Because it's very business-specific, this kind of change should be done
at the application level in a decorator, rather than embedding it in the
extension.
---
 lib/solidus_subscriptions/processor.rb           |  2 +-
 spec/lib/solidus_subscriptions/processor_spec.rb | 12 ------------
 2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/lib/solidus_subscriptions/processor.rb b/lib/solidus_subscriptions/processor.rb
index b5aaa707..ffc274b4 100644
--- a/lib/solidus_subscriptions/processor.rb
+++ b/lib/solidus_subscriptions/processor.rb
@@ -5,7 +5,7 @@ class Processor
     class << self
       def run
         SolidusSubscriptions::Subscription.actionable.find_each(&method(:process_subscription))
-        SolidusSubscriptions::Installment.actionable.with_active_subscription.find_each(&method(:process_installment))
+        SolidusSubscriptions::Installment.actionable.find_each(&method(:process_installment))
       end
 
       private
diff --git a/spec/lib/solidus_subscriptions/processor_spec.rb b/spec/lib/solidus_subscriptions/processor_spec.rb
index 03be168c..e82c0e9e 100644
--- a/spec/lib/solidus_subscriptions/processor_spec.rb
+++ b/spec/lib/solidus_subscriptions/processor_spec.rb
@@ -121,16 +121,4 @@
       expect(subscription.reload.state).to eq('canceled')
     end
   end
-
-  context 'with a cancelled subscription with pending installments' do
-    it 'does not process the installment' do
-      subscription = create(:subscription)
-      create(:installment, subscription: subscription, actionable_date: Time.zone.today)
-      subscription.cancel!
-
-      described_class.run
-
-      expect(SolidusSubscriptions::ProcessInstallmentJob).not_to have_been_enqueued
-    end
-  end
 end

From ce4edc06e6079d8c098f1d0754e3c8e31b355e2d Mon Sep 17 00:00:00 2001
From: Alessandro Desantis <desa.alessandro@gmail.com>
Date: Wed, 20 Jan 2021 10:50:50 +0100
Subject: [PATCH 8/9] Move all business logic to `lib`

It wasn't clear why certain business logic should live in `app/services`
while other should live in `lib`. By unifying everything in one directory,
we make it easier for developers to inspect the code and reduce the
cognitive load when implementing new classes.
---
 lib/solidus_subscriptions.rb                             | 9 +++++++++
 {app/services => lib}/solidus_subscriptions/checkout.rb  | 0
 .../solidus_subscriptions/dispatcher/base.rb             | 0
 .../dispatcher/failure_dispatcher.rb                     | 0
 .../dispatcher/out_of_stock_dispatcher.rb                | 0
 .../dispatcher/payment_failed_dispatcher.rb              | 0
 .../dispatcher/success_dispatcher.rb                     | 0
 .../solidus_subscriptions/line_item_builder.rb           | 0
 .../solidus_subscriptions/subscription_generator.rb      | 0
 .../subscription_line_item_builder.rb                    | 0
 .../solidus_subscriptions/checkout_spec.rb               | 0
 .../dispatcher/failure_dispatcher_spec.rb                | 0
 .../dispatcher/out_of_stock_dispatcher_spec.rb           | 0
 .../dispatcher/payment_failed_dispatcher_spec.rb         | 0
 .../dispatcher/success_dispatcher_spec.rb                | 0
 .../solidus_subscriptions/line_item_builder_spec.rb      | 0
 .../solidus_subscriptions/subscription_generator_spec.rb | 0
 .../subscription_order_promotion_rule_spec.rb            | 0
 .../subscription_promotion_rule_spec.rb                  | 0
 19 files changed, 9 insertions(+)
 rename {app/services => lib}/solidus_subscriptions/checkout.rb (100%)
 rename {app/services => lib}/solidus_subscriptions/dispatcher/base.rb (100%)
 rename {app/services => lib}/solidus_subscriptions/dispatcher/failure_dispatcher.rb (100%)
 rename {app/services => lib}/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb (100%)
 rename {app/services => lib}/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb (100%)
 rename {app/services => lib}/solidus_subscriptions/dispatcher/success_dispatcher.rb (100%)
 rename {app/services => lib}/solidus_subscriptions/line_item_builder.rb (100%)
 rename {app/services => lib}/solidus_subscriptions/subscription_generator.rb (100%)
 rename {app/services => lib}/solidus_subscriptions/subscription_line_item_builder.rb (100%)
 rename spec/{services => lib}/solidus_subscriptions/checkout_spec.rb (100%)
 rename spec/{services => lib}/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb (100%)
 rename spec/{services => lib}/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb (100%)
 rename spec/{services => lib}/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb (100%)
 rename spec/{services => lib}/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb (100%)
 rename spec/{services => lib}/solidus_subscriptions/line_item_builder_spec.rb (100%)
 rename spec/{services => lib}/solidus_subscriptions/subscription_generator_spec.rb (100%)
 rename spec/{services => lib}/solidus_subscriptions/subscription_order_promotion_rule_spec.rb (100%)
 rename spec/{services => lib}/solidus_subscriptions/subscription_promotion_rule_spec.rb (100%)

diff --git a/lib/solidus_subscriptions.rb b/lib/solidus_subscriptions.rb
index e9dae93d..a0525040 100644
--- a/lib/solidus_subscriptions.rb
+++ b/lib/solidus_subscriptions.rb
@@ -18,6 +18,15 @@
 require 'solidus_subscriptions/churn_buster/subscription_payment_method_serializer'
 require 'solidus_subscriptions/churn_buster/subscription_serializer'
 require 'solidus_subscriptions/churn_buster/order_serializer'
+require 'solidus_subscriptions/checkout'
+require 'solidus_subscriptions/line_item_builder'
+require 'solidus_subscriptions/subscription_generator'
+require 'solidus_subscriptions/subscription_line_item_builder'
+require 'solidus_subscriptions/dispatcher/base'
+require 'solidus_subscriptions/dispatcher/failure_dispatcher'
+require 'solidus_subscriptions/dispatcher/out_of_stock_dispatcher'
+require 'solidus_subscriptions/dispatcher/payment_failed_dispatcher'
+require 'solidus_subscriptions/dispatcher/success_dispatcher'
 
 module SolidusSubscriptions
   class << self
diff --git a/app/services/solidus_subscriptions/checkout.rb b/lib/solidus_subscriptions/checkout.rb
similarity index 100%
rename from app/services/solidus_subscriptions/checkout.rb
rename to lib/solidus_subscriptions/checkout.rb
diff --git a/app/services/solidus_subscriptions/dispatcher/base.rb b/lib/solidus_subscriptions/dispatcher/base.rb
similarity index 100%
rename from app/services/solidus_subscriptions/dispatcher/base.rb
rename to lib/solidus_subscriptions/dispatcher/base.rb
diff --git a/app/services/solidus_subscriptions/dispatcher/failure_dispatcher.rb b/lib/solidus_subscriptions/dispatcher/failure_dispatcher.rb
similarity index 100%
rename from app/services/solidus_subscriptions/dispatcher/failure_dispatcher.rb
rename to lib/solidus_subscriptions/dispatcher/failure_dispatcher.rb
diff --git a/app/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb b/lib/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb
similarity index 100%
rename from app/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb
rename to lib/solidus_subscriptions/dispatcher/out_of_stock_dispatcher.rb
diff --git a/app/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb b/lib/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb
similarity index 100%
rename from app/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb
rename to lib/solidus_subscriptions/dispatcher/payment_failed_dispatcher.rb
diff --git a/app/services/solidus_subscriptions/dispatcher/success_dispatcher.rb b/lib/solidus_subscriptions/dispatcher/success_dispatcher.rb
similarity index 100%
rename from app/services/solidus_subscriptions/dispatcher/success_dispatcher.rb
rename to lib/solidus_subscriptions/dispatcher/success_dispatcher.rb
diff --git a/app/services/solidus_subscriptions/line_item_builder.rb b/lib/solidus_subscriptions/line_item_builder.rb
similarity index 100%
rename from app/services/solidus_subscriptions/line_item_builder.rb
rename to lib/solidus_subscriptions/line_item_builder.rb
diff --git a/app/services/solidus_subscriptions/subscription_generator.rb b/lib/solidus_subscriptions/subscription_generator.rb
similarity index 100%
rename from app/services/solidus_subscriptions/subscription_generator.rb
rename to lib/solidus_subscriptions/subscription_generator.rb
diff --git a/app/services/solidus_subscriptions/subscription_line_item_builder.rb b/lib/solidus_subscriptions/subscription_line_item_builder.rb
similarity index 100%
rename from app/services/solidus_subscriptions/subscription_line_item_builder.rb
rename to lib/solidus_subscriptions/subscription_line_item_builder.rb
diff --git a/spec/services/solidus_subscriptions/checkout_spec.rb b/spec/lib/solidus_subscriptions/checkout_spec.rb
similarity index 100%
rename from spec/services/solidus_subscriptions/checkout_spec.rb
rename to spec/lib/solidus_subscriptions/checkout_spec.rb
diff --git a/spec/services/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb b/spec/lib/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb
similarity index 100%
rename from spec/services/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb
rename to spec/lib/solidus_subscriptions/dispatcher/failure_dispatcher_spec.rb
diff --git a/spec/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb b/spec/lib/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb
similarity index 100%
rename from spec/services/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb
rename to spec/lib/solidus_subscriptions/dispatcher/out_of_stock_dispatcher_spec.rb
diff --git a/spec/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb b/spec/lib/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb
similarity index 100%
rename from spec/services/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb
rename to spec/lib/solidus_subscriptions/dispatcher/payment_failed_dispatcher_spec.rb
diff --git a/spec/services/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb b/spec/lib/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb
similarity index 100%
rename from spec/services/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb
rename to spec/lib/solidus_subscriptions/dispatcher/success_dispatcher_spec.rb
diff --git a/spec/services/solidus_subscriptions/line_item_builder_spec.rb b/spec/lib/solidus_subscriptions/line_item_builder_spec.rb
similarity index 100%
rename from spec/services/solidus_subscriptions/line_item_builder_spec.rb
rename to spec/lib/solidus_subscriptions/line_item_builder_spec.rb
diff --git a/spec/services/solidus_subscriptions/subscription_generator_spec.rb b/spec/lib/solidus_subscriptions/subscription_generator_spec.rb
similarity index 100%
rename from spec/services/solidus_subscriptions/subscription_generator_spec.rb
rename to spec/lib/solidus_subscriptions/subscription_generator_spec.rb
diff --git a/spec/services/solidus_subscriptions/subscription_order_promotion_rule_spec.rb b/spec/lib/solidus_subscriptions/subscription_order_promotion_rule_spec.rb
similarity index 100%
rename from spec/services/solidus_subscriptions/subscription_order_promotion_rule_spec.rb
rename to spec/lib/solidus_subscriptions/subscription_order_promotion_rule_spec.rb
diff --git a/spec/services/solidus_subscriptions/subscription_promotion_rule_spec.rb b/spec/lib/solidus_subscriptions/subscription_promotion_rule_spec.rb
similarity index 100%
rename from spec/services/solidus_subscriptions/subscription_promotion_rule_spec.rb
rename to spec/lib/solidus_subscriptions/subscription_promotion_rule_spec.rb

From 4717d24c0e7203210efe8c23b55467c3e728cdc5 Mon Sep 17 00:00:00 2001
From: Alessandro Desantis <desa.alessandro@gmail.com>
Date: Fri, 22 Jan 2021 13:59:59 +0100
Subject: [PATCH 9/9] Remove `dummy_order` and `dummy_line_item` helpers

These helpers are dangerous: they provide a false sense of assurance
by making you think that the order and the line item they return can
be used to infer the correct total value of future subscription orders.

In reality, order calculation in Solidus is an extremely complex process
that may take a ton of different parameters into account, and each store
is better off calculating the subscription total with their custom logic
rather than this extension trying to provide a solution that works for
everyone.

In the future, we may provide a way to compute a subscription's total,
but for the time being it's better to remove the helpers altogether.
---
 README.md                                     |  3 +-
 .../solidus_subscriptions/installment.rb      |  7 --
 app/models/solidus_subscriptions/line_item.rb | 29 -------
 .../solidus_subscriptions/subscription.rb     |  9 --
 lib/solidus_subscriptions.rb                  |  1 -
 .../line_item_builder.rb                      | 36 --------
 .../line_item_builder_spec.rb                 | 33 -------
 .../solidus_subscriptions/installment_spec.rb |  9 --
 .../solidus_subscriptions/line_item_spec.rb   | 86 -------------------
 .../subscription_spec.rb                      | 10 ---
 10 files changed, 1 insertion(+), 222 deletions(-)
 delete mode 100644 lib/solidus_subscriptions/line_item_builder.rb
 delete mode 100644 spec/lib/solidus_subscriptions/line_item_builder_spec.rb

diff --git a/README.md b/README.md
index 2705c2db..6c3ed603 100644
--- a/README.md
+++ b/README.md
@@ -62,8 +62,7 @@ This will associate a `SolidusSubscriptions::LineItem` to the line item being ad
 
 The customer will not be charged for the subscription until it is processed. The subscription line
 items should be shown to the user on the cart page by looping over
-`Spree::Order#subscription_line_items`. `SolidusSubscriptions::LineItem#dummy_line_item` may be
-useful to help you display the subscription line item with your existing cart infrastructure.
+`Spree::Order#subscription_line_items`.
 
 When the order is finalized, a `SolidusSubscriptions::Subscription` will be created for each group
 of subscription line items which can be fulfilled by a single subscription.
diff --git a/app/models/solidus_subscriptions/installment.rb b/app/models/solidus_subscriptions/installment.rb
index 0b5f6277..aed1f62c 100644
--- a/app/models/solidus_subscriptions/installment.rb
+++ b/app/models/solidus_subscriptions/installment.rb
@@ -31,13 +31,6 @@ class Installment < ApplicationRecord
       unfulfilled.where("#{table_name}.actionable_date <= ?", Time.zone.today)
     end)
 
-    # Get the builder for the subscription_line_item. This will be an
-    # object that can generate the appropriate line item for the subscribable
-    # object
-    #
-    # @return [SolidusSubscriptions::LineItemBuilder]
-    delegate :line_item_builder, to: :subscription
-
     # Mark this installment as out of stock.
     #
     # @return [SolidusSubscriptions::InstallmentDetail] The record of the failed
diff --git a/app/models/solidus_subscriptions/line_item.rb b/app/models/solidus_subscriptions/line_item.rb
index 64e6fb0b..2583ecc5 100644
--- a/app/models/solidus_subscriptions/line_item.rb
+++ b/app/models/solidus_subscriptions/line_item.rb
@@ -38,34 +38,5 @@ class LineItem < ApplicationRecord
     validates :subscribable_id, presence: true
     validates :quantity, numericality: { greater_than: 0 }
     validates :interval_length, numericality: { greater_than: 0 }, unless: -> { subscription }
-
-    def as_json(**options)
-      options[:methods] ||= [:dummy_line_item]
-      super(options)
-    end
-
-    # Get a placeholder line item for calculating the values of future
-    # subscription orders. It is frozen and cannot be saved
-    def dummy_line_item
-      li = LineItemBuilder.new([self]).spree_line_items.first
-      return unless li
-
-      li.order = dummy_order
-      li.validate
-      li.freeze
-    end
-
-    private
-
-    # Get a placeholder order for calculating the values of future
-    # subscription orders. It is a frozen duplicate of the current order and
-    # cannot be saved
-    def dummy_order
-      order = spree_line_item ? spree_line_item.order.dup : ::Spree::Order.create
-      order.ship_address = subscription.shipping_address || subscription.user.ship_address if subscription
-      order.bill_address = subscription.billing_address || subscription.user.bill_address if subscription
-
-      order.freeze
-    end
   end
 end
diff --git a/app/models/solidus_subscriptions/subscription.rb b/app/models/solidus_subscriptions/subscription.rb
index 41689732..15424d82 100644
--- a/app/models/solidus_subscriptions/subscription.rb
+++ b/app/models/solidus_subscriptions/subscription.rb
@@ -203,15 +203,6 @@ def advance_actionable_date
       actionable_date
     end
 
-    # Get the builder for the subscription_line_item. This will be an
-    # object that can generate the appropriate line item for the subscribable
-    # object
-    #
-    # @return [SolidusSubscriptions::LineItemBuilder]
-    def line_item_builder
-      LineItemBuilder.new(line_items)
-    end
-
     # The state of the last attempt to process an installment associated to
     # this subscription
     #
diff --git a/lib/solidus_subscriptions.rb b/lib/solidus_subscriptions.rb
index a0525040..d5e3d73c 100644
--- a/lib/solidus_subscriptions.rb
+++ b/lib/solidus_subscriptions.rb
@@ -19,7 +19,6 @@
 require 'solidus_subscriptions/churn_buster/subscription_serializer'
 require 'solidus_subscriptions/churn_buster/order_serializer'
 require 'solidus_subscriptions/checkout'
-require 'solidus_subscriptions/line_item_builder'
 require 'solidus_subscriptions/subscription_generator'
 require 'solidus_subscriptions/subscription_line_item_builder'
 require 'solidus_subscriptions/dispatcher/base'
diff --git a/lib/solidus_subscriptions/line_item_builder.rb b/lib/solidus_subscriptions/line_item_builder.rb
deleted file mode 100644
index c9aedb9f..00000000
--- a/lib/solidus_subscriptions/line_item_builder.rb
+++ /dev/null
@@ -1,36 +0,0 @@
-# frozen_string_literal: true
-
-# This class is responsible for taking SubscriptionLineItems and building
-# them into Spree::LineItems which can be added to an order
-module SolidusSubscriptions
-  class LineItemBuilder
-    attr_reader :subscription_line_items
-
-    # Get a new instance of a LineItemBuilder
-    #
-    # @param subscription_line_items[Array<SolidusSubscriptions::LineItem>] The
-    #   subscription line item to be converted into a Spree::LineItem
-    #
-    # @return [SolidusSubscriptions::LineItemBuilder]
-    def initialize(subscription_line_items)
-      @subscription_line_items = subscription_line_items
-    end
-
-    # Get a new (unpersisted) Spree::LineItem which matches the details of
-    # :subscription_line_item
-    #
-    # @return [Array<Spree::LineItem>]
-    def spree_line_items
-      line_items = subscription_line_items.map do |subscription_line_item|
-        variant = subscription_line_item.subscribable
-
-        next unless variant.can_supply?(subscription_line_item.quantity)
-
-        ::Spree::LineItem.new(variant: variant, quantity: subscription_line_item.quantity)
-      end
-
-      # Either all line items for an installment are fulfilled or none are
-      line_items.all? ? line_items : []
-    end
-  end
-end
diff --git a/spec/lib/solidus_subscriptions/line_item_builder_spec.rb b/spec/lib/solidus_subscriptions/line_item_builder_spec.rb
deleted file mode 100644
index d1e041c1..00000000
--- a/spec/lib/solidus_subscriptions/line_item_builder_spec.rb
+++ /dev/null
@@ -1,33 +0,0 @@
-require 'spec_helper'
-
-RSpec.describe SolidusSubscriptions::LineItemBuilder do
-  let(:builder) { described_class.new subscription_line_items }
-  let(:variant) { create(:variant, subscribable: true) }
-  let(:subscription_line_item) { subscription_line_items.first }
-  let(:subscription_line_items) do
-    build_stubbed_list(:subscription_line_item, 1, subscribable_id: variant.id)
-  end
-
-  describe '#spree_line_items' do
-    subject { builder.spree_line_items }
-
-    let(:expected_attributes) do
-      {
-        variant_id: subscription_line_item.subscribable_id,
-        quantity: subscription_line_item.quantity
-      }
-    end
-
-    it { is_expected.to be_a Array }
-
-    it 'contains a line item with the correct attributes' do
-      expect(subject.first).to have_attributes expected_attributes
-    end
-
-    context 'the variant is out of stock' do
-      before { create :stock_location, backorderable_default: false }
-
-      it { is_expected.to be_empty }
-    end
-  end
-end
diff --git a/spec/models/solidus_subscriptions/installment_spec.rb b/spec/models/solidus_subscriptions/installment_spec.rb
index 4e4831e8..399a7e6a 100644
--- a/spec/models/solidus_subscriptions/installment_spec.rb
+++ b/spec/models/solidus_subscriptions/installment_spec.rb
@@ -5,15 +5,6 @@
 
   it { is_expected.to validate_presence_of :subscription }
 
-  describe '#line_item_builder' do
-    subject { installment.line_item_builder }
-
-    let(:line_items) { installment.subscription.line_items }
-
-    it { is_expected.to be_a SolidusSubscriptions::LineItemBuilder }
-    it { is_expected.to have_attributes(subscription_line_items: line_items) }
-  end
-
   describe '#out_of_stock' do
     subject { installment.out_of_stock }
 
diff --git a/spec/models/solidus_subscriptions/line_item_spec.rb b/spec/models/solidus_subscriptions/line_item_spec.rb
index ad80384d..aaa24a52 100644
--- a/spec/models/solidus_subscriptions/line_item_spec.rb
+++ b/spec/models/solidus_subscriptions/line_item_spec.rb
@@ -24,90 +24,4 @@
       expect(subject.from_now).to eq Date.parse("2016-10-22")
     end
   end
-
-  describe '#as_json' do
-    subject { line_item.as_json }
-
-    around { |e| Timecop.freeze { e.run } }
-
-    let(:line_item) { create(:subscription_line_item, :with_subscription) }
-
-    let(:expected_hash) do
-      hash = {
-        "id" => line_item.id,
-        "spree_line_item_id" => line_item.spree_line_item.id,
-        "subscription_id" => line_item.subscription_id,
-        "quantity" => line_item.quantity,
-        "end_date" => line_item.end_date,
-        "subscribable_id" => line_item.subscribable_id,
-        "created_at" => line_item.created_at,
-        "updated_at" => line_item.updated_at,
-        "interval_units" => line_item.interval_units,
-        "interval_length" => line_item.interval_length
-      }
-      Rails.gem_version >= Gem::Version.new('6.0.0') ? hash.as_json : hash
-    end
-
-    it 'includes the attribute values' do
-      expect(subject).to match a_hash_including(expected_hash)
-    end
-
-    it 'includes the dummy lineitem' do
-      expect(subject).to have_key('dummy_line_item')
-    end
-  end
-
-  describe '#dummy_line_item' do
-    subject { line_item.dummy_line_item }
-
-    let(:line_item) { create(:subscription_line_item, :with_subscription) }
-
-    it { is_expected.to be_a Spree::LineItem }
-    it { is_expected.to be_frozen }
-
-    it 'has the correct variant' do
-      expect(subject.variant_id).to eq line_item.subscribable_id
-    end
-
-    context 'with no spree line item' do
-      let(:line_item) { create(:subscription_line_item, :with_subscription, spree_line_item: nil) }
-
-      it { is_expected.to be_a Spree::LineItem }
-      it { is_expected.to be_frozen }
-
-      it 'has the correct variant' do
-        expect(subject.variant_id).to eq line_item.subscribable_id
-      end
-    end
-
-    context 'with an associated subscription' do
-      context 'the associated subscription has a shipping address' do
-        let(:line_item) do
-          create(:subscription_line_item, :with_subscription, subscription_traits: [:with_shipping_address])
-        end
-
-        it 'uses the subscription shipping address' do
-          expect(subject.order.ship_address).to eq line_item.subscription.shipping_address
-        end
-
-        it 'uses the subscription users billing address' do
-          expect(subject.order.bill_address).to eq line_item.subscription.user.bill_address
-        end
-      end
-
-      context 'the associated subscription has a billing address' do
-        let(:line_item) do
-          create(:subscription_line_item, :with_subscription, subscription_traits: [:with_billing_address])
-        end
-
-        it 'uses the subscription users shipping address' do
-          expect(subject.order.ship_address).to eq line_item.subscription.user.ship_address
-        end
-
-        it 'uses the subscription billing address' do
-          expect(subject.order.bill_address).to eq line_item.subscription.billing_address
-        end
-      end
-    end
-  end
 end
diff --git a/spec/models/solidus_subscriptions/subscription_spec.rb b/spec/models/solidus_subscriptions/subscription_spec.rb
index 5d855db6..f305580a 100644
--- a/spec/models/solidus_subscriptions/subscription_spec.rb
+++ b/spec/models/solidus_subscriptions/subscription_spec.rb
@@ -348,16 +348,6 @@
     end
   end
 
-  describe '#line_item_builder' do
-    subject { subscription.line_item_builder }
-
-    let(:subscription) { create :subscription, :with_line_item }
-    let(:line_items) { subscription.line_items }
-
-    it { is_expected.to be_a SolidusSubscriptions::LineItemBuilder }
-    it { is_expected.to have_attributes(subscription_line_items: line_items) }
-  end
-
   describe '#processing_state' do
     subject { subscription.processing_state }