Preventing double creation of invoices by recognising completed invoices within the specifed period

spec/jobs/finalize_user_invoices_spec.rb

@@ -14,9 +14,9 @@ describe FinalizeUserInvoices 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!(:invoice3) { create(:order, distributor: accounts_distributor, created_at: start_of_july, completed_at: nil) }
       let!(:invoice4) { create(:order, distributor: accounts_distributor, created_at: start_of_july + 10.days, completed_at: nil) }
-      let!(:invoice5) { create(:order, distributor: accounts_distributor, created_at: start_of_july - 30.days +  3.hours, completed_at: nil) }
+      let!(:invoice5) { create(:order, distributor: accounts_distributor, created_at: start_of_july - 30.days, completed_at: nil) }
 
       before do
         allow(Enterprise).to receive(:find_by_id) { accounts_distributor }
@@ -70,13 +70,13 @@ describe FinalizeUserInvoices do
         context "and no date arguments are passed to the job" 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
+          it "finalizes the uncompleted orders for accounts_distributor created in the previous calendar 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(invoice3)
             expect(finalizer).to_not have_received(:finalize).with(invoice2)
             expect(finalizer).to_not have_received(:finalize).with(invoice4)
-            expect(finalizer).to_not have_received(:finalize).with(invoice5)
+            expect(finalizer).to have_received(:finalize).with(invoice5)
           end
         end
 
@@ -90,13 +90,13 @@ describe FinalizeUserInvoices do
           context "that ends in the past" do
             travel_to(3.hours)
 
-            it "finalizes the uncompleted orders for accounts_distributor created in the specified calendar month (or on the first of the following month)" do
+            it "finalizes the uncompleted orders for accounts_distributor created in the specified calendar 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(invoice3)
               expect(finalizer).to_not have_received(:finalize).with(invoice2)
               expect(finalizer).to_not have_received(:finalize).with(invoice4)
-              expect(finalizer).to_not have_received(:finalize).with(invoice5)
+              expect(finalizer).to have_received(:finalize).with(invoice5)
             end
           end
 

spec/jobs/update_user_invoices_spec.rb

@@ -94,9 +94,10 @@ describe UpdateUserInvoices do
       let(:invoice) { create(:order, user: user) }
 
       before do
-        allow(user).to receive(:current_invoice) { invoice }
+        allow(user).to receive(:invoice_for) { invoice }
         allow(updater).to receive(:clean_up_and_save)
         allow(updater).to receive(:finalize)
+        allow(Bugsnag).to receive(:notify)
       end
 
       context "on the first of the month" do
@@ -105,18 +106,52 @@ describe UpdateUserInvoices do
         before do
           allow(updater).to receive(:adjustment_label_from).exactly(1).times.and_return("Old Item")
           allow(old_billable_period).to receive(:bill) { 666.66 }
-          updater.update_invoice_for(user, [old_billable_period])
         end
 
-        it "creates adjustments for each billing item" do
-          adjustments = invoice.adjustments
-          expect(adjustments.map(&:source_id)).to eq [old_billable_period.id]
-          expect(adjustments.map(&:amount)).to eq [666.66]
-          expect(adjustments.map(&:label)).to eq ["Old Item"]
+        context "where the invoice was not created at start_date" do
+          before do
+            invoice.update_attribute(:created_at, start_of_july - 1.month + 1.day)
+            updater.update_invoice_for(user, [old_billable_period])
+          end
+
+          it "snags a bug" do
+            expect(Bugsnag).to have_received(:notify)
+          end
         end
 
-        it "cleans up and saves the invoice" do
-          expect(updater).to have_received(:clean_up_and_save).with(invoice, anything).once
+        context "where the invoice was created at start_date" do
+          before do
+            invoice.update_attribute(:created_at, start_of_july - 1.month)
+          end
+
+          context "where the invoice is already complete" do
+            before do
+              allow(invoice).to receive(:complete?) { true }
+              updater.update_invoice_for(user, [old_billable_period])
+            end
+
+            it "snags a bug" do
+              expect(Bugsnag).to have_received(:notify)
+            end
+          end
+
+          context "where the invoice is not complete" do
+            before do
+              allow(invoice).to receive(:complete?) { false }
+              updater.update_invoice_for(user, [old_billable_period])
+            end
+
+            it "creates adjustments for each billing item" do
+              adjustments = invoice.adjustments
+              expect(adjustments.map(&:source_id)).to eq [old_billable_period.id]
+              expect(adjustments.map(&:amount)).to eq [666.66]
+              expect(adjustments.map(&:label)).to eq ["Old Item"]
+            end
+
+            it "cleans up and saves the invoice" do
+              expect(updater).to have_received(:clean_up_and_save).with(invoice, anything).once
+            end
+          end
         end
       end
 
@@ -127,18 +162,52 @@ describe UpdateUserInvoices do
           allow(updater).to receive(:adjustment_label_from).exactly(2).times.and_return("BP1 Item", "BP2 Item")
           allow(billable_period1).to receive(:bill) { 123.45 }
           allow(billable_period2).to receive(:bill) { 543.21 }
-          updater.update_invoice_for(user, [billable_period1, billable_period2])
         end
 
-        it "creates adjustments for each billing item" do
-          adjustments = invoice.adjustments
-          expect(adjustments.map(&:source_id)).to eq [billable_period1.id, billable_period2.id]
-          expect(adjustments.map(&:amount)).to eq [123.45, 543.21]
-          expect(adjustments.map(&:label)).to eq ["BP1 Item", "BP2 Item"]
+        context "where the invoice was not created at start_date" do
+          before do
+            invoice.update_attribute(:created_at, start_of_july + 1.day)
+            updater.update_invoice_for(user, [billable_period1, billable_period2])
+          end
+
+          it "snags a bug" do
+            expect(Bugsnag).to have_received(:notify)
+          end
         end
 
-        it "cleans up and saves the invoice" do
-          expect(updater).to have_received(:clean_up_and_save).with(invoice, anything).once
+        context "where the invoice was created at start_date" do
+          before do
+            invoice.update_attribute(:created_at, start_of_july)
+          end
+
+          context "where the invoice is already complete" do
+            before do
+              allow(invoice).to receive(:complete?) { true }
+              updater.update_invoice_for(user, [billable_period1, billable_period2])
+            end
+
+            it "snags a bug" do
+              expect(Bugsnag).to have_received(:notify)
+            end
+          end
+
+          context "where the invoice is not complete" do
+            before do
+              allow(invoice).to receive(:complete?) { false }
+              updater.update_invoice_for(user, [billable_period1, billable_period2])
+            end
+
+            it "creates adjustments for each billing item" do
+              adjustments = invoice.adjustments
+              expect(adjustments.map(&:source_id)).to eq [billable_period1.id, billable_period2.id]
+              expect(adjustments.map(&:amount)).to eq [123.45, 543.21]
+              expect(adjustments.map(&:label)).to eq ["BP1 Item", "BP2 Item"]
+            end
+
+            it "cleans up and saves the invoice" do
+              expect(updater).to have_received(:clean_up_and_save).with(invoice, anything).once
+            end
+          end
         end
       end
     end
@@ -282,7 +351,7 @@ describe UpdateUserInvoices do
     end
 
     context "when an invoice currently exists" do
-      let!(:invoice) { create(:order, user: user, distributor: accounts_distributor, created_at: start_of_july + 3.days) }
+      let!(:invoice) { create(:order, user: user, distributor: accounts_distributor, created_at: start_of_july) }
       let!(:billable_adjustment) { create(:adjustment, adjustable: invoice, source_type: 'BillablePeriod') }
 
       before do

spec/models/spree/user_spec.rb

@@ -80,7 +80,7 @@ describe Spree.user_class do
     end
   end
 
-  describe "current_invoice" do
+  describe "invoice_for" do
     let!(:user) { create(:user) }
     let!(:accounts_distributor) { create(:distributor_enterprise) }
     let!(:start_of_month) { Time.now.beginning_of_month }
@@ -89,20 +89,16 @@ describe Spree.user_class do
       Spree::Config.accounts_distributor_id = accounts_distributor.id
     end
 
-    context "where no relevant invoice exists for the current month" do
+    context "where no relevant invoice exists for the given period" do
       # Created during previous month
-      let!(:order1) { create(:order, user: user, created_at: start_of_month - 20.days, completed_at: nil, distributor: accounts_distributor) }
-      # Already Completed
-      let!(:order2) { create(:order, user: user, created_at: start_of_month + 3.hours, completed_at: start_of_month + 3.days, distributor: accounts_distributor) }
+      let!(:order1) { create(:order, user: user, created_at: start_of_month - 3.hours, completed_at: nil, distributor: accounts_distributor) }
       # Incorrect distributor
       let!(:order3) { create(:order, user: user, created_at: start_of_month + 3.hours, completed_at: nil, distributor: create(:distributor_enterprise)) }
       # Incorrect user
       let!(:order4) { create(:order, user: create(:user), created_at: start_of_month + 3.hours, completed_at: nil, distributor: accounts_distributor) }
 
-      around { |example| Timecop.travel(start_of_month + 20.days) { example.run } }
-
       it "creates a new invoice" do
-        current_invoice = user.current_invoice
+        current_invoice = user.invoice_for(start_of_month, start_of_month + 20.days)
         expect(current_invoice).to be_a_new Spree::Order
         expect(current_invoice.completed_at).to be nil
         expect(current_invoice.distributor).to eq accounts_distributor
@@ -113,10 +109,8 @@ describe Spree.user_class do
     context "where an invoice exists for the current month" do
       let!(:order) { create(:order, user: user, created_at: start_of_month + 3.hours, completed_at: nil, distributor: accounts_distributor) }
 
-      around { |example| Timecop.travel(start_of_month + 20.days) { example.run } }
-
       it "returns the existing invoice" do
-        current_invoice = user.current_invoice
+        current_invoice = user.invoice_for(start_of_month, start_of_month + 20.days)
         expect(current_invoice).to eq order
       end
     end