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.

3 files changed, 115 insertions, 211 deletions

diff --git a/lib/solidus_subscriptions/processor.rb b/lib/solidus_subscriptions/processor.rb
index d56816a..4547cec 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 2eb5069..20dcc04 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 @@ FactoryBot.define do
         [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 d3bb268..a7301f0 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