Splitting out invoice finalization as a separate step

app/jobs/finalize_user_invoices.rb

@@ -0,0 +1,29 @@
+FinalizeUserInvoices = Struct.new("FinalizeUserInvoices") do
+
+  def perform
+    return unless accounts_distributor = Enterprise.find_by_id(Spree::Config.accounts_distributor_id)
+    return unless accounts_distributor.payment_methods.find_by_id(Spree::Config.default_accounts_payment_method_id)
+    return unless accounts_distributor.shipping_methods.find_by_id(Spree::Config.default_accounts_shipping_method_id)
+
+    start_date = (Time.now.beginning_of_month - 1.month) + 1.day
+    end_date = Time.now.beginning_of_month + 1.day
+
+    invoices = Spree::Order.where('distributor_id = (?) AND created_at >= (?) AND created_at <= (?) AND completed_at IS NULL',
+      accounts_distributor, start_date, end_date)
+
+    invoices.each do |invoice|
+      finalize(invoice)
+    end
+  end
+
+  def finalize(invoice)
+    # TODO: When we implement per-customer and/or per-user preferences around shipping and payment methods
+    # we can update these to read from those preferences
+    invoice.payments.create(payment_method_id: Spree::Config.default_accounts_payment_method_id, amount: invoice.total)
+    invoice.update_attribute(:shipping_method_id, Spree::Config.default_accounts_shipping_method_id)
+
+    while invoice.state != "complete"
+      invoice.next
+    end
+  end
+end

app/jobs/update_user_invoices.rb

@@ -1,8 +1,6 @@
 UpdateUserInvoices = Struct.new("UpdateUserInvoices") do
   def perform
     return unless accounts_distributor = Enterprise.find_by_id(Spree::Config.accounts_distributor_id)
-    return unless accounts_distributor.payment_methods.find_by_id(Spree::Config.default_accounts_payment_method_id)
-    return unless accounts_distributor.shipping_methods.find_by_id(Spree::Config.default_accounts_shipping_method_id)
 
     # If it is the first of the month, update invoices for the previous month up until midnight last night
     # Otherwise, update invoices for the current month
@@ -29,8 +27,6 @@ UpdateUserInvoices = Struct.new("UpdateUserInvoices") do
     end
 
     invoice.save
-
-    finalize(invoice) if Date.today.day == 1
   end
 
   def adjustment_attrs_from(billable_period)
@@ -54,15 +50,4 @@ UpdateUserInvoices = Struct.new("UpdateUserInvoices") do
 
     "#{enterprise.name} (#{category}) [#{begins} - #{ends}]"
   end
-
-  def finalize(invoice)
-    # TODO: When we implement per-customer and/or per-user preferences around shipping and payment methods
-    # we can update these to read from those preferences
-    invoice.payments.create(payment_method_id: Spree::Config.default_accounts_payment_method_id, amount: invoice.total)
-    invoice.update_attribute(:shipping_method_id, Spree::Config.default_accounts_shipping_method_id)
-
-    while invoice.state != "complete"
-      invoice.next
-    end
-  end
 end

spec/jobs/finalize_user_invoices_spec.rb

@@ -0,0 +1,150 @@
+require 'spec_helper'
+
+def travel_to(time)
+  around { |example| Timecop.travel(start_of_july + time) { example.run } }
+end
+
+
+describe FinalizeUserInvoices do
+  describe "unit specs" do
+    let!(:finalizer) { FinalizeUserInvoices.new }
+    let!(:start_of_july) { Time.now.beginning_of_year + 6.months }
+
+    describe "perform" do
+      let!(:accounts_distributor) { create(:distributor_enterprise) }
+      let!(:invoice1) { create(:order, distributor: accounts_distributor, created_at: start_of_july - 10.days, completed_at: nil) }
+      let!(:invoice2) { create(:order, distributor: accounts_distributor, created_at: start_of_july - 10.days, completed_at: start_of_july - 10.days) }
+      let!(:invoice3) { create(:order, distributor: accounts_distributor, created_at: start_of_july + 3.hours, completed_at: nil) }
+      let!(:invoice4) { create(:order, distributor: accounts_distributor, created_at: start_of_july + 10.days, completed_at: nil) }
+
+      before do
+        allow(Enterprise).to receive(:find_by_id) { accounts_distributor }
+        allow(accounts_distributor).to receive(:payment_methods) { double(:payment_methods, find_by_id: true) }
+        allow(accounts_distributor).to receive(:shipping_methods) { double(:shipping_methods, find_by_id: true) }
+        allow(finalizer).to receive(:finalize)
+      end
+
+      context "when necessary global config setting have not been set" do
+        travel_to(20.days)
+
+        context "when accounts_distributor has been set" do
+          before do
+            allow(Enterprise).to receive(:find_by_id) { false }
+            finalizer.perform
+          end
+
+          it "doesn't run" do
+            expect(finalizer).to_not have_received(:finalize)
+          end
+        end
+
+        context "when default payment method has been set" do
+          before do
+            allow(accounts_distributor).to receive(:payment_methods) { double(:payment_methods, find_by_id: false) }
+            finalizer.perform
+          end
+
+          it "doesn't run" do
+            expect(finalizer).to_not have_received(:finalize)
+          end
+        end
+
+        context "when default shipping method has been set" do
+          before do
+            allow(accounts_distributor).to receive(:shipping_methods) { double(:shipping_methods, find_by_id: false) }
+            finalizer.perform
+          end
+
+          it "doesn't run" do
+            expect(finalizer).to_not have_received(:finalize)
+          end
+        end
+      end
+
+      context "when necessary global config setting have been set" do
+        travel_to(3.days)
+
+        it "finalizes the uncompleted orders for accounts_distributor created in the previous calendar month (or on the 1st of this month)" do
+          finalizer.perform
+          expect(finalizer).to have_received(:finalize).with(invoice1)
+          expect(finalizer).to have_received(:finalize).with(invoice3)
+          expect(finalizer).to_not have_received(:finalize).with(invoice2)
+          expect(finalizer).to_not have_received(:finalize).with(invoice4)
+        end
+      end
+    end
+
+    describe "finalize" do
+      let!(:pm) { create(:payment_method, name: "PM1") }
+      let!(:sm) { create(:shipping_method, name: "ship1") }
+      let!(:accounts_distributor) { create(:distributor_enterprise, payment_methods: [pm], shipping_methods: [sm]) }
+      let!(:invoice) { create(:order, distributor: accounts_distributor) }
+
+      before do
+        Spree::Config.set({ accounts_distributor_id: accounts_distributor.id })
+        Spree::Config.set({ default_accounts_payment_method_id: pm.id })
+        Spree::Config.set({ default_accounts_shipping_method_id: sm.id })
+        invoice.line_items.clear
+      end
+
+      it "creates payment, assigns shipping method and finalizes the order" do
+        expect(invoice.completed_at).to be nil
+        finalizer.finalize(invoice)
+        expect(invoice.completed_at).to_not be nil
+        expect(invoice.payments.count).to eq 1
+        expect(invoice.payments.first.payment_method).to eq pm
+        expect(invoice.shipping_method).to eq sm
+      end
+
+      it "does not send a confirmation email" do
+        expect(invoice).to receive(:deliver_order_confirmation_email).and_call_original
+        expect{finalizer.finalize(invoice)}.to_not enqueue_job ConfirmOrderJob
+      end
+    end
+  end
+
+  describe "validation spec" do
+    let!(:start_of_july) { Time.now.beginning_of_year + 6.months }
+
+    let!(:updater) { UpdateUserInvoices.new }
+    let!(:finalizer) { FinalizeUserInvoices.new }
+
+    let!(:pm) { create(:payment_method, name: "Default Payment Method") }
+    let!(:sm) { create(:shipping_method, name: "Default Shipping Method") }
+    let!(:accounts_distributor) { create(:distributor_enterprise, payment_methods: [pm], shipping_methods: [sm]) }
+
+    let!(:user) { create(:user) }
+    let!(:billable_period1) { create(:billable_period, sells: 'any', owner: user, begins_at: start_of_july - 1.month, ends_at: start_of_july) }
+    let!(:billable_period2) { create(:billable_period, owner: user, begins_at: start_of_july, ends_at: start_of_july + 10.days) }
+    let!(:billable_period3) { create(:billable_period, owner: user, begins_at: start_of_july + 12.days, ends_at: start_of_july + 20.days) }
+
+    before do
+      sm.calculator.set_preference(:amount, 0); sm.calculator.save!
+
+      Spree::Config.set({ accounts_distributor_id: accounts_distributor.id })
+      Spree::Config.set({ default_accounts_payment_method_id: pm.id })
+      Spree::Config.set({ default_accounts_shipping_method_id: sm.id })
+    end
+
+    context "finalizing an invoice" do
+      travel_to(3.hours)
+
+      it "finalizes it" do
+        # Create an invoice using the updater, to make sure we are using
+        # an order as it would be when generated this way
+        expect{updater.perform}.to change{Spree::Order.count}.from(0).to(1)
+        invoice = user.orders.first
+
+        # Finalize invoices
+        finalizer.perform
+        invoice.reload
+
+        expect(invoice.completed_at).to_not be_nil
+        expect(invoice.total).to eq billable_period1.bill
+        expect(invoice.payments.count).to eq 1
+        expect(invoice.payments.first.amount).to eq billable_period1.bill
+        expect(invoice.state).to eq 'complete'
+      end
+    end
+  end
+end

spec/jobs/update_user_invoices_spec.rb

@@ -19,8 +19,6 @@ describe UpdateUserInvoices do
       let(:accounts_distributor) { double(:accounts_distributor) }
       before do
         allow(Enterprise).to receive(:find_by_id) { accounts_distributor }
-        allow(accounts_distributor).to receive(:payment_methods) { double(:payment_methods, find_by_id: true) }
-        allow(accounts_distributor).to receive(:shipping_methods) { double(:shipping_methods, find_by_id: true) }
         allow(updater).to receive(:update_invoice_for)
       end
 
@@ -37,28 +35,6 @@ describe UpdateUserInvoices do
             expect(updater).to_not have_received(:update_invoice_for)
           end
         end
-
-        context "when default payment method has been set" do
-          before do
-            allow(accounts_distributor).to receive(:payment_methods) { double(:payment_methods, find_by_id: false) }
-            updater.perform
-          end
-
-          it "doesn't run" do
-            expect(updater).to_not have_received(:update_invoice_for)
-          end
-        end
-
-        context "when default shipping method has been set" do
-          before do
-            allow(accounts_distributor).to receive(:shipping_methods) { double(:shipping_methods, find_by_id: false) }
-            updater.perform
-          end
-
-          it "doesn't run" do
-            expect(updater).to_not have_received(:update_invoice_for)
-          end
-        end
       end
 
       context "when necessary global config setting have been set" do
@@ -109,13 +85,9 @@ describe UpdateUserInvoices do
           expect(adjustments.map(&:label)).to eq ["Old Item"]
         end
 
-        it "saves to invoice" do
+        it "saves the invoice" do
           expect(invoice).to have_received(:save).once
         end
-
-        it "finalizes the invoice" do
-          expect(updater).to have_received(:finalize).with(invoice)
-        end
       end
 
       context "on other days" do
@@ -138,38 +110,6 @@ describe UpdateUserInvoices do
         it "saves to invoice" do
           expect(invoice).to have_received(:save).once
         end
-
-        it "does not finalize the invoice" do
-          expect(updater).to_not have_received(:finalize)
-        end
-      end
-    end
-
-    describe "finalize" do
-      let!(:pm) { create(:payment_method, name: "PM1") }
-      let!(:sm) { create(:shipping_method, name: "ship1") }
-      let!(:accounts_distributor) { create(:distributor_enterprise, payment_methods: [pm], shipping_methods: [sm]) }
-      let!(:invoice) { create(:order, distributor: accounts_distributor) }
-
-      before do
-        Spree::Config.set({ accounts_distributor_id: accounts_distributor.id })
-        Spree::Config.set({ default_accounts_payment_method_id: pm.id })
-        Spree::Config.set({ default_accounts_shipping_method_id: sm.id })
-        invoice.line_items.clear
-      end
-
-      it "creates payment, assigns shipping method and finalizes the order" do
-        expect(invoice.completed_at).to be nil
-        updater.finalize(invoice)
-        expect(invoice.completed_at).to_not be nil
-        expect(invoice.payments.count).to eq 1
-        expect(invoice.payments.first.payment_method).to eq pm
-        expect(invoice.shipping_method).to eq sm
-      end
-
-      it "does not send a confirmation email" do
-        expect(invoice).to receive(:deliver_order_confirmation_email).and_call_original
-        expect{updater.finalize(invoice)}.to_not enqueue_job ConfirmOrderJob
       end
     end
   end
@@ -179,9 +119,7 @@ describe UpdateUserInvoices do
 
     let!(:updater) { UpdateUserInvoices.new }
 
-    let!(:pm) { create(:payment_method, name: "Default Payment Method") }
-    let!(:sm) { create(:shipping_method, name: "Default Shipping Method") }
-    let!(:accounts_distributor) { create(:distributor_enterprise, payment_methods: [pm], shipping_methods: [sm]) }
+    let!(:accounts_distributor) { create(:distributor_enterprise) }
 
     let!(:user) { create(:user) }
     let!(:billable_period1) { create(:billable_period, sells: 'any', owner: user, begins_at: start_of_july - 1.month, ends_at: start_of_july) }
@@ -189,17 +127,13 @@ describe UpdateUserInvoices do
     let!(:billable_period3) { create(:billable_period, owner: user, begins_at: start_of_july + 12.days, ends_at: start_of_july + 20.days) }
 
     before do
-      sm.calculator.set_preference(:amount, 0); sm.calculator.save!
-
       Spree::Config.set({ accounts_distributor_id: accounts_distributor.id })
-      Spree::Config.set({ default_accounts_payment_method_id: pm.id })
-      Spree::Config.set({ default_accounts_shipping_method_id: sm.id })
     end
 
     context "updating an invoice" do
       travel_to(20.days)
 
-      it "does not creates an invoice when one does not already exist, but does not finalize it" do
+      it "creates an invoice when one does not already exist" do
         expect{updater.perform}.to change{Spree::Order.count}.from(0).to(1)
         invoice = user.orders.first
         expect(invoice.completed_at).to be_nil
@@ -208,19 +142,5 @@ describe UpdateUserInvoices do
         expect(invoice.state).to eq 'cart'
       end
     end
-
-    context "finalizing an invoice" do
-      travel_to(3.hours)
-
-      it "creates an invoice and finalizes it" do
-        expect{updater.perform}.to change{Spree::Order.count}.from(0).to(1)
-        invoice = user.orders.first
-        expect(invoice.completed_at).to_not be_nil
-        expect(invoice.total).to eq billable_period1.bill
-        expect(invoice.payments.count).to eq 1
-        expect(invoice.payments.first.amount).to eq billable_period1.bill
-        expect(invoice.state).to eq 'complete'
-      end
-    end
   end
 end