From 11c3cf5f71fa616ff7c5d178154190a2dfbe9681 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 9 Jul 2015 12:05:10 +0800 Subject: [PATCH] Pushing setting validation for accounts jobs into separate methods capable of logging errors to bugsnag --- app/jobs/finalize_user_invoices.rb | 58 ++++++++++++++++++++++-- app/jobs/update_billable_periods.rb | 20 +++++++- app/jobs/update_user_invoices.rb | 32 ++++++++++++- spec/jobs/finalize_user_invoices_spec.rb | 11 +++-- spec/jobs/update_user_invoices_spec.rb | 7 ++- 5 files changed, 115 insertions(+), 13 deletions(-) diff --git a/app/jobs/finalize_user_invoices.rb b/app/jobs/finalize_user_invoices.rb index b99c0328a5..18d5d3b8b9 100644 --- a/app/jobs/finalize_user_invoices.rb +++ b/app/jobs/finalize_user_invoices.rb @@ -12,13 +12,10 @@ class FinalizeUserInvoices end def perform - return unless end_date <= Time.now - 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) + return unless settings_are_valid? invoices = Spree::Order.where('distributor_id = (?) AND created_at >= (?) AND created_at <= (?) AND completed_at IS NULL', - accounts_distributor, start_date + 1.day, end_date + 1.day) + @accounts_distributor, start_date + 1.day, end_date + 1.day) invoices.each do |invoice| finalize(invoice) @@ -35,4 +32,55 @@ class FinalizeUserInvoices invoice.next end end + + private + + def settings_are_valid? + unless end_date <= Time.now + Bugsnag.notify(RuntimeError.new("InvalidJobSettings"), { + job: "FinalizeUserInvoices", + error: "end_date is in the future", + data: { + end_date: end_date.localtime.strftime("%F %T"), + now: Time.now.strftime("%F %T") + } + }) + return false + end + + unless @accounts_distributor = Enterprise.find_by_id(Spree::Config.accounts_distributor_id) + Bugsnag.notify(RuntimeError.new("InvalidJobSettings"), { + job: "FinalizeUserInvoices", + error: "accounts_distributor_id is invalid", + data: { + accounts_distributor_id: Spree::Config.accounts_distributor_id + } + }) + return false + end + + unless @accounts_distributor.payment_methods.find_by_id(Spree::Config.default_accounts_payment_method_id) + Bugsnag.notify(RuntimeError.new("InvalidJobSettings"), { + job: "FinalizeUserInvoices", + error: "default_accounts_payment_method_id is invalid", + data: { + default_accounts_payment_method_id: Spree::Config.default_accounts_payment_method_id + } + }) + return false + end + + unless @accounts_distributor.shipping_methods.find_by_id(Spree::Config.default_accounts_shipping_method_id) + Bugsnag.notify(RuntimeError.new("InvalidJobSettings"), { + job: "FinalizeUserInvoices", + error: "default_accounts_shipping_method_id is invalid", + data: { + default_accounts_shipping_method_id: Spree::Config.default_accounts_shipping_method_id + } + }) + return false + end + + true + end end diff --git a/app/jobs/update_billable_periods.rb b/app/jobs/update_billable_periods.rb index f594c8f166..4e8f4375c8 100644 --- a/app/jobs/update_billable_periods.rb +++ b/app/jobs/update_billable_periods.rb @@ -7,7 +7,7 @@ class UpdateBillablePeriods end def perform - return unless end_date <= Time.now + return unless settings_are_valid? job_start_time = Time.now @@ -99,4 +99,22 @@ class UpdateBillablePeriods obsolete_billable_periods.each(&:delete) end + + private + + def settings_are_valid? + unless end_date <= Time.now + Bugsnag.notify(RuntimeError.new("InvalidJobSettings"), { + job: "UpdateBillablePeriods", + error: "end_date is in the future", + data: { + end_date: end_date.localtime.strftime("%F %T"), + now: Time.now.strftime("%F %T") + } + }) + return false + end + + true + end end diff --git a/app/jobs/update_user_invoices.rb b/app/jobs/update_user_invoices.rb index 95078d8310..b062d12fdf 100644 --- a/app/jobs/update_user_invoices.rb +++ b/app/jobs/update_user_invoices.rb @@ -11,8 +11,7 @@ class UpdateUserInvoices end def perform - return unless end_date <= Time.now - return unless accounts_distributor = Enterprise.find_by_id(Spree::Config.accounts_distributor_id) + return unless settings_are_valid? # Find all users that have owned an enterprise at some point in the relevant period enterprise_users = Spree::User.joins(:billable_periods) @@ -84,4 +83,33 @@ class UpdateUserInvoices invoice.destroy end end + + private + + def settings_are_valid? + unless end_date <= Time.now + Bugsnag.notify(RuntimeError.new("InvalidJobSettings"), { + job: "UpdateUserInvoices", + error: "end_date is in the future", + data: { + end_date: end_date.localtime.strftime("%F %T"), + now: Time.now.strftime("%F %T") + } + }) + return false + end + + unless Enterprise.find_by_id(Spree::Config.accounts_distributor_id) + Bugsnag.notify(RuntimeError.new("InvalidJobSettings"), { + job: "UpdateUserInvoices", + error: "accounts_distributor_id is invalid", + data: { + accounts_distributor_id: Spree::Config.accounts_distributor_id + } + }) + return false + end + + true + end end diff --git a/spec/jobs/finalize_user_invoices_spec.rb b/spec/jobs/finalize_user_invoices_spec.rb index dc0cc8d237..00a9387fdc 100644 --- a/spec/jobs/finalize_user_invoices_spec.rb +++ b/spec/jobs/finalize_user_invoices_spec.rb @@ -23,6 +23,7 @@ describe FinalizeUserInvoices do 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) + allow(Bugsnag).to receive(:notify) end context "when necessary global config setting have not been set" do @@ -34,7 +35,8 @@ describe FinalizeUserInvoices do finalizer.perform end - it "doesn't run" do + it "snags errors and doesn't run" do + expect(Bugsnag).to have_received(:notify).with(RuntimeError.new("InvalidJobSettings"), anything) expect(finalizer).to_not have_received(:finalize) end end @@ -45,7 +47,8 @@ describe FinalizeUserInvoices do finalizer.perform end - it "doesn't run" do + it "snags errors and doesn't run" do + expect(Bugsnag).to have_received(:notify).with(RuntimeError.new("InvalidJobSettings"), anything) expect(finalizer).to_not have_received(:finalize) end end @@ -56,7 +59,8 @@ describe FinalizeUserInvoices do finalizer.perform end - it "doesn't run" do + it "snags errors and doesn't run" do + expect(Bugsnag).to have_received(:notify).with(RuntimeError.new("InvalidJobSettings"), anything) expect(finalizer).to_not have_received(:finalize) end end @@ -101,6 +105,7 @@ describe FinalizeUserInvoices do it "does not finalize any orders" do finalizer.perform + expect(Bugsnag).to have_received(:notify).with(RuntimeError.new("InvalidJobSettings"), anything) expect(finalizer).to_not have_received(:finalize) end end diff --git a/spec/jobs/update_user_invoices_spec.rb b/spec/jobs/update_user_invoices_spec.rb index dfa78e0f32..dd131e623f 100644 --- a/spec/jobs/update_user_invoices_spec.rb +++ b/spec/jobs/update_user_invoices_spec.rb @@ -20,6 +20,7 @@ describe UpdateUserInvoices do before do allow(Enterprise).to receive(:find_by_id) { accounts_distributor } allow(updater).to receive(:update_invoice_for) + allow(Bugsnag).to receive(:notify) end context "when necessary global config setting have not been set" do @@ -31,7 +32,8 @@ describe UpdateUserInvoices do updater.perform end - it "doesn't run" do + it "snags errors and doesn't run" do + expect(Bugsnag).to have_received(:notify).with(RuntimeError.new("InvalidJobSettings"), anything) expect(updater).to_not have_received(:update_invoice_for) end end @@ -78,8 +80,9 @@ describe UpdateUserInvoices do context "that ends in the future" do travel_to 30.days - it "does not update the user's invoice" do + it "snags an error and does not update the user's invoice" do updater.perform + expect(Bugsnag).to have_received(:notify).with(RuntimeError.new("InvalidJobSettings"), anything) expect(updater).to_not have_received(:update_invoice_for) end end