Extract CapQuantityAndStoreChanges service

This is a responsiblity of its own that makes the job's tests really
complex.

app/jobs/subscription_placement_job.rb

@@ -30,36 +30,9 @@ class SubscriptionPlacementJob < ActiveJob::Base
       proxy_order,
       summarizer,
       JobLogger.logger,
-      lambda { cap_quantity_and_store_changes(proxy_order.order) }
+      CapQuantityAndStoreChanges.new(proxy_order.order)
     )
 
     place_order.call
   end
-
-  def cap_quantity_and_store_changes(order)
-    changes = {}
-    order.insufficient_stock_lines.each do |line_item|
-      changes[line_item.id] = line_item.quantity
-      line_item.cap_quantity_at_stock!
-    end
-    unavailable_stock_lines_for(order).each do |line_item|
-      changes[line_item.id] = changes[line_item.id] || line_item.quantity
-      line_item.update(quantity: 0)
-
-      Spree::OrderInventory.new(order).verify(line_item, order.shipment)
-    end
-    if changes.present?
-      order.line_items.reload
-      order.update_order_fees!
-    end
-    changes
-  end
-
-  def unavailable_stock_lines_for(order)
-    order.line_items.where('variant_id NOT IN (?)', available_variants_for(order).select(&:id))
-  end
-
-  def available_variants_for(order)
-    OrderCycleDistributedVariants.new(order.order_cycle, order.distributor).available_variants
-  end
 end

app/services/cap_quantity_and_store_changes.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+class CapQuantityAndStoreChanges
+  def initialize(order)
+    @order = order
+  end
+
+  def call
+    changes = {}
+
+    order.insufficient_stock_lines.each do |line_item|
+      changes[line_item.id] = line_item.quantity
+      line_item.cap_quantity_at_stock!
+    end
+
+    unavailable_stock_lines_for.each do |line_item|
+      changes[line_item.id] = changes[line_item.id] || line_item.quantity
+      line_item.update(quantity: 0)
+
+      Spree::OrderInventory.new(order).verify(line_item, order.shipment)
+    end
+
+    if changes.present?
+      order.line_items.reload
+      order.update_order_fees!
+    end
+
+    changes
+  end
+
+  private
+
+  attr_reader :order
+
+  def unavailable_stock_lines_for
+    order.line_items.where('variant_id NOT IN (?)', available_variants_for.select(&:id))
+  end
+
+  def available_variants_for
+    OrderCycleDistributedVariants.new(order.order_cycle, order.distributor).available_variants
+  end
+end

app/services/place_order.rb

@@ -1,12 +1,12 @@
 # frozen_string_literal: true
 
 class PlaceOrder
-  def initialize(proxy_order, summarizer, logger, changes_loader)
+  def initialize(proxy_order, summarizer, logger, stock_changes_loader)
     @proxy_order = proxy_order
     @subscription = proxy_order.subscription
     @summarizer = summarizer
     @logger = logger
-    @changes_loader = changes_loader
+    @stock_changes_loader = stock_changes_loader
   end
 
   def call
@@ -29,7 +29,7 @@ class PlaceOrder
 
   private
 
-  attr_reader :proxy_order, :subscription, :order, :summarizer, :logger, :changes_loader, :changes
+  attr_reader :proxy_order, :subscription, :order, :summarizer, :logger, :stock_changes_loader, :changes
 
   def initialise_order
     logger.info("Placing Order for Proxy Order #{proxy_order.id}")
@@ -51,7 +51,7 @@ class PlaceOrder
   end
 
   def load_changes
-    @changes = changes_loader.call
+    @changes = stock_changes_loader.call
   end
 
   def empty_order?

spec/jobs/subscription_placement_job_spec.rb

@@ -63,76 +63,19 @@ describe SubscriptionPlacementJob do
           proxy_order,
           summarizer,
           JobLogger.logger,
-          lambda { job.send(:cap_quantity_and_store_changes, order) }
+          CapQuantityAndStoreChanges.new(proxy_order.order)
         )
 
         allow(PlaceOrder).to receive(:new) { service }
         allow(service).to receive(:call)
+
         job.perform
+
         expect(service).to have_received(:call)
       end
     end
   end
 
-  describe "checking that line items are available to purchase" do
-    let(:order_cycle) { create(:simple_order_cycle) }
-    let(:shop) { order_cycle.coordinator }
-    let(:order) { create(:order, order_cycle: order_cycle, distributor: shop) }
-    let(:ex) {
-      create(:exchange, order_cycle: order_cycle, sender: shop, receiver: shop, incoming: false)
-    }
-    let(:variant1) { create(:variant, on_hand: 5) }
-    let(:variant2) { create(:variant, on_hand: 5) }
-    let(:variant3) { create(:variant, on_hand: 5) }
-    let!(:line_item1) { create(:line_item, order: order, variant: variant1, quantity: 3) }
-    let!(:line_item2) { create(:line_item, order: order, variant: variant2, quantity: 3) }
-    let!(:line_item3) { create(:line_item, order: order, variant: variant3, quantity: 3) }
-
-    context "when all items are available from the order cycle" do
-      before { [variant1, variant2, variant3].each { |v| ex.variants << v } }
-
-      context "and insufficient stock exists to fulfil the order for some items" do
-        before do
-          variant1.update_attribute(:on_hand, 5)
-          variant2.update_attribute(:on_hand, 2)
-          variant3.update_attribute(:on_hand, 0)
-        end
-
-        it "caps quantity at the stock level for stock-limited items, and reports the change" do
-          changes = job.send(:cap_quantity_and_store_changes, order.reload)
-          expect(line_item1.reload.quantity).to be 3 # not capped
-          expect(line_item2.reload.quantity).to be 2 # capped
-          expect(line_item3.reload.quantity).to be 0 # capped
-          expect(changes[line_item1.id]).to be nil
-          expect(changes[line_item2.id]).to be 3
-          expect(changes[line_item3.id]).to be 3
-        end
-      end
-    end
-
-    context "and some items are not available from the order cycle" do
-      before { [variant2, variant3].each { |v| ex.variants << v } }
-
-      context "and insufficient stock exists to fulfil the order for some items" do
-        before do
-          variant1.update_attribute(:on_hand, 5)
-          variant2.update_attribute(:on_hand, 2)
-          variant3.update_attribute(:on_hand, 0)
-        end
-
-        it "sets quantity to 0 for unavailable items, and reports the change" do
-          changes = job.send(:cap_quantity_and_store_changes, order.reload)
-          expect(line_item1.reload.quantity).to be 0 # unavailable
-          expect(line_item2.reload.quantity).to be 2 # capped
-          expect(line_item3.reload.quantity).to be 0 # capped
-          expect(changes[line_item1.id]).to be 3
-          expect(changes[line_item2.id]).to be 3
-          expect(changes[line_item3.id]).to be 3
-        end
-      end
-    end
-  end
-
   describe "processing a subscription order" do
     let!(:shipping_method_created_earlier) { create(:shipping_method, distributors: [shop]) }
     let!(:shipping_method) { create(:shipping_method, distributors: [shop]) }
@@ -162,7 +105,7 @@ describe SubscriptionPlacementJob do
           proxy_order,
           summarizer,
           JobLogger.logger,
-          lambda { job.send(:cap_quantity_and_store_changes, order) }
+          CapQuantityAndStoreChanges.new(order)
         )
 
         ActionMailer::Base.deliveries.clear
@@ -189,8 +132,10 @@ describe SubscriptionPlacementJob do
       end
 
       context "when no stock items are available after capping stock" do
+        let(:store_changes) { CapQuantityAndStoreChanges.new(order) }
+
         before do
-          allow(job).to receive(:unavailable_stock_lines_for) { order.line_items }
+          allow(store_changes).to receive(:unavailable_stock_lines_for) { order.line_items }
         end
 
         it "does not place the order, clears all adjustments, and sends an empty_order email" do
@@ -199,13 +144,13 @@ describe SubscriptionPlacementJob do
             proxy_order,
             summarizer,
             JobLogger.logger,
-            lambda { job.send(:cap_quantity_and_store_changes, order) }
+            store_changes
           )
 
           allow(service).to receive(:send_placement_email)
           allow(service).to receive(:send_empty_email)
 
-          expect{ service.call }.to_not change{ order.reload.completed_at }.from(nil)
+          expect { service.call }.to_not change { order.reload.completed_at }.from(nil)
           expect(order.all_adjustments).to be_empty
           expect(order.total).to eq 0
           expect(order.adjustment_total).to eq 0
@@ -223,7 +168,7 @@ describe SubscriptionPlacementJob do
             proxy_order,
             summarizer,
             JobLogger.logger,
-            lambda { job.send(:cap_quantity_and_store_changes, order) }
+            CapQuantityAndStoreChanges.new(order)
           )
         end
 

spec/services/cap_quantity_and_store_changes_spec.rb

@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe CapQuantityAndStoreChanges do
+  describe "checking that line items are available to purchase" do
+    let(:order_cycle) { create(:simple_order_cycle) }
+    let(:shop) { order_cycle.coordinator }
+    let(:order) { create(:order, order_cycle: order_cycle, distributor: shop) }
+    let(:ex) { create(:exchange, order_cycle: order_cycle, sender: shop, receiver: shop, incoming: false) }
+    let(:variant1) { create(:variant, on_hand: 5) }
+    let(:variant2) { create(:variant, on_hand: 5) }
+    let(:variant3) { create(:variant, on_hand: 5) }
+    let!(:line_item1) { create(:line_item, order: order, variant: variant1, quantity: 3) }
+    let!(:line_item2) { create(:line_item, order: order, variant: variant2, quantity: 3) }
+    let!(:line_item3) { create(:line_item, order: order, variant: variant3, quantity: 3) }
+
+    context "when all items are available from the order cycle" do
+      before { [variant1, variant2, variant3].each { |v| ex.variants << v } }
+
+      context "and insufficient stock exists to fulfil the order for some items" do
+        before do
+          variant1.update_attribute(:on_hand, 5)
+          variant2.update_attribute(:on_hand, 2)
+          variant3.update_attribute(:on_hand, 0)
+        end
+
+        it "caps quantity at the stock level for stock-limited items, and reports the change" do
+          changes = CapQuantityAndStoreChanges.new(order.reload).call
+
+          expect(line_item1.reload.quantity).to be 3 # not capped
+          expect(line_item2.reload.quantity).to be 2 # capped
+          expect(line_item3.reload.quantity).to be 0 # capped
+          expect(changes[line_item1.id]).to be nil
+          expect(changes[line_item2.id]).to be 3
+          expect(changes[line_item3.id]).to be 3
+        end
+      end
+    end
+
+    context "and some items are not available from the order cycle" do
+      before { [variant2, variant3].each { |v| ex.variants << v } }
+
+      context "and insufficient stock exists to fulfil the order for some items" do
+        before do
+          variant1.update_attribute(:on_hand, 5)
+          variant2.update_attribute(:on_hand, 2)
+          variant3.update_attribute(:on_hand, 0)
+        end
+
+        it "sets quantity to 0 for unavailable items, and reports the change" do
+          changes = CapQuantityAndStoreChanges.new(order.reload).call
+
+          expect(line_item1.reload.quantity).to be 0 # unavailable
+          expect(line_item2.reload.quantity).to be 2 # capped
+          expect(line_item3.reload.quantity).to be 0 # capped
+          expect(changes[line_item1.id]).to be 3
+          expect(changes[line_item2.id]).to be 3
+          expect(changes[line_item3.id]).to be 3
+        end
+
+        context "and the order has been placed" do
+          before do
+            allow(order).to receive(:ensure_available_shipping_rates) { true }
+            allow(order).to receive(:process_each_payment) { true }
+
+            order.create_proposed_shipments
+          end
+
+          it "removes the unavailable items from the shipment" do
+            expect {
+              CapQuantityAndStoreChanges.new(order.reload).call
+            }.to change {
+              order.reload.shipment.manifest.size
+            }.from(2).to(1)
+          end
+        end
+      end
+    end
+  end
+end

spec/services/place_order_spec.rb

@@ -5,9 +5,8 @@ require 'spec_helper'
 describe PlaceOrder do
   include ActiveSupport::Testing::TimeHelpers
 
-  subject { described_class.new(proxy_order, summarizer, logger, changes) }
+  subject { described_class.new(proxy_order, summarizer, logger, stock_changes_loader) }
 
-  let(:changes) { {} }
   let(:proxy_order) { create(:proxy_order, order: order) }
   let(:order) { build(:order) }
   let(:summarizer) { instance_double(OrderManagement::Subscriptions::Summarizer) }
@@ -19,7 +18,7 @@ describe PlaceOrder do
     let!(:subscription) { create(:subscription, with_items: true) }
     let!(:proxy_order) { create(:proxy_order, subscription: subscription, order: order) }
 
-    let(:changes) { lambda { {} } }
+    let(:stock_changes_loader) { lambda { {} } }
     let(:summarizer) { instance_double(OrderManagement::Subscriptions::Summarizer, record_order: true, record_issue: true) }
 
     before do
@@ -49,7 +48,7 @@ describe PlaceOrder do
     end
 
     context "when no changes are present" do
-      let(:changes) { lambda { {} } }
+      let(:stock_changes_loader) { lambda { {} } }
 
       it "logs a success and sends the email" do
         expect(summarizer).to receive(:record_success).with(order).once
@@ -63,7 +62,7 @@ describe PlaceOrder do
 
     context "when changes are present" do
       let(:changeset) { double(:changes) }
-      let(:changes) { lambda { changeset } }
+      let(:stock_changes_loader) { lambda { changeset } }
 
       it "logs an issue and sends the email" do
         expect(summarizer).to receive(:record_issue).with(:changes, order).once
@@ -80,7 +79,7 @@ describe PlaceOrder do
     let(:summarizer) { instance_double(OrderManagement::Subscriptions::Summarizer, record_order: true) }
 
     let(:changeset) { double(:changes) }
-    let(:changes) { lambda { changeset } }
+    let(:stock_changes_loader) { lambda { changeset } }
 
     before do
       allow(SubscriptionMailer).to receive(:empty_email) { mail_mock }
@@ -95,41 +94,4 @@ describe PlaceOrder do
       expect(mail_mock).to have_received(:deliver_now)
     end
   end
-
-  describe "#move_to_completion" do
-    let(:order_cycle) { create(:simple_order_cycle) }
-    let(:shop) { order_cycle.coordinator }
-    let(:order) { create(:order, order_cycle: order_cycle, distributor: shop) }
-    let(:ex) { create(:exchange, order_cycle: order_cycle, sender: shop, receiver: shop, incoming: false) }
-    let(:variant1) { create(:variant, on_hand: 5) }
-    let(:variant2) { create(:variant, on_hand: 5) }
-    let(:variant3) { create(:variant, on_hand: 5) }
-    let!(:line_item1) { create(:line_item, order: order, variant: variant1, quantity: 3) }
-    let!(:line_item2) { create(:line_item, order: order, variant: variant2, quantity: 3) }
-    let!(:line_item3) { create(:line_item, order: order, variant: variant3, quantity: 3) }
-
-    context "and some items are not available from the order cycle" do
-      before { [variant2, variant3].each { |v| ex.variants << v } }
-
-      context "and insufficient stock exists to fulfil the order for some items" do
-        before do
-          variant1.update_attribute(:on_hand, 5)
-          variant2.update_attribute(:on_hand, 2)
-          variant3.update_attribute(:on_hand, 0)
-        end
-
-        context "and the order has been placed" do
-          before do
-            allow(order).to receive(:ensure_available_shipping_rates) { true }
-            allow(order).to receive(:process_each_payment) { true }
-          end
-
-          it "removes the unavailable items from the shipment" do
-            subject.move_to_completion
-            expect(order.reload.shipment.manifest.size).to eq 1
-          end
-        end
-      end
-    end
-  end
 end