From d46712de84b659eff3d75e2dcc154d52dcdc52ab Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 19 Nov 2015 11:27:20 +1100 Subject: [PATCH] Obsolete BillablePeriods only deleted if their associated order is not already complete --- app/jobs/update_billable_periods.rb | 8 +++++--- spec/jobs/update_billable_periods_spec.rb | 3 +++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/jobs/update_billable_periods.rb b/app/jobs/update_billable_periods.rb index 598d5f8e7b..d775b093f3 100644 --- a/app/jobs/update_billable_periods.rb +++ b/app/jobs/update_billable_periods.rb @@ -91,10 +91,10 @@ class UpdateBillablePeriods def clean_up_untouched_billable_periods_for(enterprise, job_start_time) # Snag and then delete any BillablePeriods which overlap - obsolete_billable_periods = enterprise.billable_periods.where('ends_at > (?) AND begins_at < (?) AND updated_at < (?)', start_date, end_date, job_start_time) + obsolete_billable_periods = enterprise.billable_periods.where('ends_at > (?) AND begins_at < (?) AND billable_periods.updated_at < (?)', start_date, end_date, job_start_time) if obsolete_billable_periods.any? - current_billable_periods = enterprise.billable_periods.where('ends_at >= (?) AND begins_at <= (?) AND updated_at > (?)', start_date, end_date, job_start_time) + current_billable_periods = enterprise.billable_periods.where('ends_at >= (?) AND begins_at <= (?) AND billable_periods.updated_at > (?)', start_date, end_date, job_start_time) Delayed::Worker.logger.info "#{enterprise.name} #{start_date.strftime("%F %T")} #{job_start_time.strftime("%F %T")}" Delayed::Worker.logger.info "#{obsolete_billable_periods.first.updated_at.strftime("%F %T")}" @@ -105,7 +105,9 @@ class UpdateBillablePeriods }) end - obsolete_billable_periods.each(&:delete) + obsolete_billable_periods.includes({ account_invoice: :order}). + where('spree_orders.state <> \'complete\' OR account_invoices.order_id IS NULL'). + each(&:delete) end private diff --git a/spec/jobs/update_billable_periods_spec.rb b/spec/jobs/update_billable_periods_spec.rb index bfdbfa8434..d868550e7e 100644 --- a/spec/jobs/update_billable_periods_spec.rb +++ b/spec/jobs/update_billable_periods_spec.rb @@ -524,6 +524,8 @@ describe UpdateBillablePeriods do let!(:bp6) { create(:billable_period, enterprise: enterprise, updated_at: job_start_time - 5.seconds, begins_at: start_of_july - 10.days, ends_at: start_of_july - 5.days ) } # Updated before start but ends at start_date (ie. not after start_date, so should be ignored) EDGE CASE let!(:bp7) { create(:billable_period, enterprise: enterprise, updated_at: job_start_time - 5.seconds, begins_at: start_of_july - 5.days, ends_at: start_of_july ) } + # Updated before start, but order is already complete, so should not be deleted + let!(:bp8) { create(:billable_period, enterprise: enterprise, updated_at: job_start_time - 5.seconds, begins_at: start_of_july, ends_at: start_of_july + 10.days, account_invoice: create(:account_invoice, order: create(:order, state: 'complete', completed_at: 5.minutes.ago))) } before do allow(Bugsnag).to receive(:notify) @@ -540,6 +542,7 @@ describe UpdateBillablePeriods do expect(bp5.reload.deleted_at).to be_nil expect(bp6.reload.deleted_at).to be_nil expect(bp7.reload.deleted_at).to be_nil + expect(bp8.reload.deleted_at).to be_nil end it "notifies bugsnag" do