Pass in the order when it's already initialized

At `CapQuantity`'s instantiation time the proxy's order is not yet
initialized, and so `CapQuantity` was checking against nil all the time.

This went unnoticed because the job's specs were not integration-level
tests and were stubbing way too many things.

app/jobs/subscription_placement_job.rb

@@ -26,6 +26,6 @@ class SubscriptionPlacementJob < ActiveJob::Base
   end
 
   def place_order_for(proxy_order)
-    PlaceProxyOrder.new(proxy_order, summarizer, JobLogger.logger, CapQuantity.new(proxy_order.order)).call
+    PlaceProxyOrder.new(proxy_order, summarizer, JobLogger.logger, CapQuantity.new).call
   end
 end

app/services/cap_quantity.rb

@@ -1,12 +1,13 @@
 # frozen_string_literal: true
 
 class CapQuantity
-  def initialize(order)
-    @order = order
+  def initialize
     @changes = {}
   end
 
-  def call
+  def call(order)
+    @order = order
+
     cap_insufficient_stock!
     verify_line_items
 

app/services/place_proxy_order.rb

@@ -7,6 +7,7 @@ class PlaceProxyOrder
     @summarizer = summarizer
     @logger = logger
     @stock_changes_loader = stock_changes_loader
+    @order = nil
   end
 
   def call
@@ -29,12 +30,13 @@ class PlaceProxyOrder
 
   private
 
-  attr_reader :proxy_order, :subscription, :order, :summarizer, :logger, :stock_changes_loader, :changes
+  attr_reader :proxy_order, :subscription, :summarizer, :logger, :stock_changes_loader, :changes
+  attr_accessor :order
 
   def initialise_order
     logger.info("Placing Order for Proxy Order #{proxy_order.id}")
 
-    @order = proxy_order.initialise_order!
+    self.order = proxy_order.initialise_order!
 
     if order.nil?
       summarizer.record_subscription_issue(subscription)
@@ -51,7 +53,7 @@ class PlaceProxyOrder
   end
 
   def load_changes
-    @changes = stock_changes_loader.call
+    @changes = stock_changes_loader.call(order)
   end
 
   def empty_order?

spec/jobs/subscription_placement_job_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe SubscriptionPlacementJob do
   let(:job) { SubscriptionPlacementJob.new }
+  let(:summarizer) { OrderManagement::Subscriptions::Summarizer.new }
 
   describe "finding proxy_orders that are ready to be placed" do
     let(:shop) { create(:distributor_enterprise) }
@@ -59,13 +60,7 @@ describe SubscriptionPlacementJob do
       end
 
       it "processes placeable proxy_orders" do
-        summarizer = instance_double(OrderManagement::Subscriptions::Summarizer)
-        service = PlaceProxyOrder.new(
-          proxy_order,
-          summarizer,
-          JobLogger.logger,
-          CapQuantity.new(proxy_order.order)
-        )
+        service = PlaceProxyOrder.new(proxy_order, summarizer, JobLogger.logger, CapQuantity.new)
 
         allow(PlaceProxyOrder).to receive(:new) { service }
         allow(service).to receive(:call)
@@ -110,44 +105,35 @@ describe SubscriptionPlacementJob do
 
     context "when the order is not already complete" do
       context "when no stock items are available after capping stock" do
-        let(:store_changes) { CapQuantity.new(order) }
+        let(:service) do
+          PlaceProxyOrder.new(proxy_order, summarizer, JobLogger.logger, store_updater)
+        end
+        let(:store_updater) { CapQuantity.new }
 
         before do
-          allow(store_changes).to receive(:unavailable_stock_lines_for) { order.line_items }
+          fake_relation = instance_double(ActiveRecord::Relation, select: -123)
+          allow(store_updater).to receive(:available_variants_for).and_return(fake_relation)
         end
 
         it "does not place the order, clears all adjustments, and sends an empty_order email" do
-          summarizer = instance_double(OrderManagement::Subscriptions::Summarizer, record_order: true, record_issue: true)
-          service = PlaceProxyOrder.new(
-            proxy_order,
-            summarizer,
-            JobLogger.logger,
-            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(order.all_adjustments).to be_empty
-          expect(order.total).to eq 0
-          expect(order.adjustment_total).to eq 0
+          service.call
+
+          expect(proxy_order.order.reload.completed_at).to be_nil
+          expect(proxy_order.order.all_adjustments).to be_empty
+          expect(proxy_order.order.total).to eq 0
+          expect(proxy_order.order.adjustment_total).to eq 0
+
           expect(service).to_not have_received(:send_placement_email)
           expect(service).to have_received(:send_empty_email)
         end
       end
 
       context "when at least one stock item is available after capping stock" do
-        let(:summarizer) do
-          instance_double(OrderManagement::Subscriptions::Summarizer, record_order: true, record_success: true)
-        end
         let(:service) do
-          PlaceProxyOrder.new(
-            proxy_order,
-            summarizer,
-            JobLogger.logger,
-            CapQuantity.new(order)
-          )
+          PlaceProxyOrder.new(proxy_order, summarizer, JobLogger.logger, CapQuantity.new)
         end
 
         before do
@@ -180,17 +166,5 @@ describe SubscriptionPlacementJob do
         end
       end
     end
-
-    context "when the proxy order fails to generate an order" do
-      before do
-        allow(proxy_order).to receive(:order) { nil }
-      end
-
-      it "records an error " do
-        expect_any_instance_of(OrderManagement::Subscriptions::Summarizer).to receive(:record_subscription_issue)
-        expect(job).to_not receive(:place_order)
-        job.send(:place_order_for, proxy_order)
-      end
-    end
   end
 end

spec/services/cap_quantity_spec.rb

@@ -35,7 +35,7 @@ describe CapQuantity do
         end
 
         it "caps quantity at the stock level for stock-limited items, and reports the change" do
-          changes = CapQuantity.new(order).call
+          changes = CapQuantity.new.call(order)
 
           expect(line_item1.reload.quantity).to be 3 # not capped
           expect(line_item2.reload.quantity).to be 2 # capped
@@ -58,7 +58,7 @@ describe CapQuantity do
         end
 
         it "sets quantity to 0 for unavailable items, and reports the change" do
-          changes = CapQuantity.new(order).call
+          changes = CapQuantity.new.call(order)
 
           expect(line_item1.reload.quantity).to be 0 # unavailable
           expect(line_item2.reload.quantity).to be 2 # capped
@@ -77,7 +77,7 @@ describe CapQuantity do
           end
 
           it "removes the unavailable items from the shipment" do
-            expect { CapQuantity.new(order).call }
+            expect { CapQuantity.new.call(order) }
               .to change { order.reload.shipment.manifest.size }.from(2).to(1)
           end
         end

spec/services/place_proxy_order_spec.rb

@@ -9,7 +9,7 @@ describe PlaceProxyOrder do
 
   let(:proxy_order) { create(:proxy_order, order: order) }
   let(:order) { build(:order, distributor: build(:enterprise)) }
-  let(:summarizer) { instance_double(OrderManagement::Subscriptions::Summarizer) }
+  let(:summarizer) { OrderManagement::Subscriptions::Summarizer.new }
   let(:logger) { instance_double(JobLogger.logger.class, info: true) }
 
   let(:mail_mock) { double(:mailer_mock, deliver_now: true) }
@@ -18,10 +18,11 @@ describe PlaceProxyOrder do
     let!(:subscription) { create(:subscription, with_items: true) }
     let!(:proxy_order) { create(:proxy_order, subscription: subscription, order: order) }
 
-    let(:stock_changes_loader) { lambda { {} } }
-    let(:summarizer) { OrderManagement::Subscriptions::Summarizer.new }
+    let(:changes) { {} }
+    let(:stock_changes_loader) { instance_double(CapQuantity) }
 
     before do
+      allow(stock_changes_loader).to receive(:call).and_return(changes)
       allow(SubscriptionMailer).to receive(:empty_email) { mail_mock }
     end
 
@@ -85,6 +86,22 @@ describe PlaceProxyOrder do
         end
       end
     end
+
+    context "when the proxy order fails to generate an order" do
+      before do
+        allow(proxy_order).to receive(:initialise_order!) { nil }
+      end
+
+      it "records an error" do
+        expect(summarizer).to receive(:record_subscription_issue)
+        subject.call
+      end
+
+      it 'does not process the proxy order' do
+        subject.call
+        expect(proxy_order.reload.placed_at).to be_nil
+      end
+    end
   end
 
   describe "#send_placement_email" do
@@ -94,9 +111,7 @@ describe PlaceProxyOrder do
 
     before do
       allow(SubscriptionMailer).to receive(:placement_email) { mail_mock }
-    end
 
-    before do
       order.line_items << build(:line_item)
 
       order_workflow = instance_double(OrderWorkflow, complete!: true)
@@ -104,7 +119,12 @@ describe PlaceProxyOrder do
     end
 
     context "when no changes are present" do
-      let(:stock_changes_loader) { lambda { {} } }
+      let(:changes) { {} }
+      let(:stock_changes_loader) { instance_double(CapQuantity) }
+
+      before do
+        allow(stock_changes_loader).to receive(:call).with(order).and_return(changes)
+      end
 
       it "logs a success and sends the email" do
         expect(summarizer).to receive(:record_success).with(order).once
@@ -117,15 +137,19 @@ describe PlaceProxyOrder do
     end
 
     context "when changes are present" do
-      let(:changeset) { double(:changes) }
-      let(:stock_changes_loader) { lambda { changeset } }
+      let(:changes) { double(:changes) }
+      let(:stock_changes_loader) { instance_double(CapQuantity) }
+
+      before do
+        allow(stock_changes_loader).to receive(:call).with(order).and_return(changes)
+      end
 
       it "logs an issue and sends the email" do
         expect(summarizer).to receive(:record_issue).with(:changes, order).once
 
         subject.call
 
-        expect(SubscriptionMailer).to have_received(:placement_email).with(order, changeset)
+        expect(SubscriptionMailer).to have_received(:placement_email).with(order, changes)
         expect(mail_mock).to have_received(:deliver_now)
       end
     end
@@ -136,10 +160,11 @@ describe PlaceProxyOrder do
       instance_double(OrderManagement::Subscriptions::Summarizer, record_order: true)
     }
 
-    let(:changeset) { double(:changes) }
-    let(:stock_changes_loader) { lambda { changeset } }
+    let(:changes) { double(:changes) }
+    let(:stock_changes_loader) { instance_double(CapQuantity) }
 
     before do
+      allow(stock_changes_loader).to receive(:call).with(order).and_return(changes)
       allow(SubscriptionMailer).to receive(:empty_email) { mail_mock }
     end
 
@@ -148,7 +173,7 @@ describe PlaceProxyOrder do
 
       subject.call
 
-      expect(SubscriptionMailer).to have_received(:empty_email).with(order, changeset)
+      expect(SubscriptionMailer).to have_received(:empty_email).with(order, changes)
       expect(mail_mock).to have_received(:deliver_now)
     end
   end