From 65c26296bd236be41cc0a7e4d5bc7d46f6713fcc Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 1 Jul 2015 12:56:51 +0800 Subject: [PATCH] Billable period updater cleans up untouched billable periods for the current billing period --- app/jobs/update_billable_periods.rb | 40 ++++++++++---------- spec/jobs/update_billable_periods_spec.rb | 46 ++++++++++++++++++++++- 2 files changed, 63 insertions(+), 23 deletions(-) diff --git a/app/jobs/update_billable_periods.rb b/app/jobs/update_billable_periods.rb index 563b86e9b6..c3229f52f0 100644 --- a/app/jobs/update_billable_periods.rb +++ b/app/jobs/update_billable_periods.rb @@ -4,6 +4,7 @@ UpdateBillablePeriods = Struct.new("UpdateBillablePeriods") do # Otherwise, calculate turnover for the current month start_date = (Time.now - 1.day).beginning_of_month end_date = Time.now.beginning_of_day + job_start_time = Time.now enterprises = Enterprise.select([:id, :name, :owner_id, :sells, :shop_trial_start_date, :created_at]) @@ -30,6 +31,8 @@ UpdateBillablePeriods = Struct.new("UpdateBillablePeriods") do ends_at = end_date split_for_trial(enterprise, begins_at, ends_at, trial_start, trial_expiry) + + clean_up_untouched_billable_periods_for(enterprise, start_date, job_start_time) end end @@ -60,27 +63,6 @@ UpdateBillablePeriods = Struct.new("UpdateBillablePeriods") do sells = enterprise.sells orders = Spree::Order.where('distributor_id = (?) AND completed_at >= (?) AND completed_at < (?)', enterprise.id, begins_at, ends_at) - # Snagging any BillablePeriods which overlap - overlapping_billable_periods = BillablePeriod.where('begins_at <= (?) AND ends_at >= (?) AND enterprise_id = (?)',ends_at, begins_at, enterprise.id) - overlapping_billable_periods.each do |billable_period| - if billable_period.sells != sells || billable_period.trial != trial || billable_period.owner_id != owner_id - Bugsnag.notify(RuntimeError.new("Duplicate BillablePeriod"), { - billable_periods: { - new: { - begins_at: begins_at, - ends_at: ends_at, - sells: sells, - trial: trial, - turnover: orders.sum(&:total), - owner_id: owner_id, - enterprise_id: enterprise.id - }.as_json, - existing: billable_period.as_json - } - }) - end - end - billable_period = BillablePeriod.where(begins_at: begins_at, enterprise_id: enterprise.id).first billable_period ||= BillablePeriod.new(begins_at: begins_at, enterprise_id: enterprise.id) billable_period.update_attributes({ @@ -93,4 +75,20 @@ UpdateBillablePeriods = Struct.new("UpdateBillablePeriods") do billable_period end + + def clean_up_untouched_billable_periods_for(enterprise, start_of_month, job_start_time) + # Snag and then delete any BillablePeriods which overlap + obsolete_billable_periods = enterprise.billable_periods.where('ends_at >= (?) AND updated_at < (?)', start_of_month, job_start_time) + + if obsolete_billable_periods.any? + current_billable_periods = enterprise.billable_periods.where('ends_at >= (?) AND updated_at >= (?)', start_of_month, job_start_time) + + Bugsnag.notify(RuntimeError.new("Duplicate BillablePeriod"), { + current: current_billable_periods.map(&:as_json), + obsolete: obsolete_billable_periods.map(&:as_json) + }) + end + + obsolete_billable_periods.each(&:delete) + end end diff --git a/spec/jobs/update_billable_periods_spec.rb b/spec/jobs/update_billable_periods_spec.rb index 8e5f808709..b89e724c5f 100644 --- a/spec/jobs/update_billable_periods_spec.rb +++ b/spec/jobs/update_billable_periods_spec.rb @@ -14,6 +14,7 @@ describe UpdateBillablePeriods do let!(:enterprise) { create(:supplier_enterprise, created_at: start_of_july - 1.month, sells: 'any') } before do + expect(updater).to receive(:clean_up_untouched_billable_periods_for).once allow(Enterprise).to receive(:select) { [enterprise] } end @@ -371,6 +372,29 @@ describe UpdateBillablePeriods do end end end + + context "cleaning up untouched billable periods" do + let(:now) { Time.now } + let(:enterprise) { create(:enterprise) } + let!(:bp1) { create(:billable_period, enterprise: enterprise, updated_at: now + 2.seconds, begins_at: start_of_july, ends_at: start_of_july + 5.days ) } + let!(:bp2) { create(:billable_period, enterprise: enterprise, updated_at: now + 2.seconds, begins_at: start_of_july + 5.days, ends_at: start_of_july + 10.days ) } + let!(:bp3) { create(:billable_period, enterprise: enterprise, updated_at: now - 5.seconds, begins_at: start_of_july, ends_at: start_of_july + 10.days ) } + + before do + allow(Bugsnag).to receive(:notify) + updater.clean_up_untouched_billable_periods_for(enterprise, start_of_july, now) + end + + it "soft deletes untouched billable_periods" do + expect(bp1.reload.deleted_at).to be_nil + expect(bp2.reload.deleted_at).to be_nil + expect(bp3.reload.deleted_at).to_not be_nil + end + + it "notifies bugsnag" do + expect(Bugsnag).to have_received(:notify).once + end + end end describe "validation spec" do @@ -383,6 +407,14 @@ describe UpdateBillablePeriods do let!(:new_owner) { create(:user) } + # This BP was updated before the current run and so should be marked for deletion at the end of the run + let!(:obsolete_bp) { create(:billable_period, enterprise: enterprise, updated_at: start_of_july + 10.days, begins_at: start_of_july + 6.5.days, ends_at: start_of_july + 10.days ) } + + # This one has an updated_at in the future (so that it doesn't get deleted) + # It also has a begins_at date which matches a period that would otherwise be created, + # and so it should be picked up and overwritten + let!(:bp_to_overwrite) { create(:billable_period, enterprise: enterprise, updated_at: start_of_july + 21.days, begins_at: start_of_july + 10.days, ends_at: start_of_july + 15.days ) } + let!(:order1) { create(:order, completed_at: start_of_july + 1.days, distributor: enterprise) } let!(:order2) { create(:order, completed_at: start_of_july + 3.days, distributor: enterprise) } let!(:order3) { create(:order, completed_at: start_of_july + 5.days, distributor: enterprise) } @@ -437,9 +469,19 @@ describe UpdateBillablePeriods do UpdateBillablePeriods.new.perform end - let(:billable_periods) { BillablePeriod.order(:id) } + let(:billable_periods) { BillablePeriod.order(:updated_at) } + + it "creates the correct billable periods and deleted obsolete ones" do + expect(obsolete_bp.reload.deleted_at).to_not be_nil + + bp_to_overwrite.reload + expect(bp_to_overwrite.sells).to eq 'own' + expect(bp_to_overwrite.trial).to be true + expect(bp_to_overwrite.owner).to eq original_owner + expect(bp_to_overwrite.begins_at).to eq start_of_july + 10.days + expect(bp_to_overwrite.ends_at).to eq start_of_july + 12.days + expect(bp_to_overwrite.turnover).to eq order6.total - it "creates the correct billable periods" do expect(billable_periods.count).to eq 9 expect(billable_periods.map(&:begins_at)).to eq [