From 6d6422d16c6175e668b0331e24f6fc9332184913 Mon Sep 17 00:00:00 2001
From: Andy Dymond <andy.dymond@housecallpro.com>
Date: Mon, 19 Jun 2023 20:22:00 -0600
Subject: [PATCH] Fintechqb 1967 enable service level logging (#600)

* Add total_tax_specified xml_accessor to TransactionTaxDetail

In a recent update to the QBO API, if a company that uses automated sales tax has an invoice that has no tax amount, we need to specify TotalTaxSpecified = false. Not passing this results in an error from QBO indicating that sales tax was miscalculated.

* Accidentally commited a binding.pry

* Add condense_logs to quickbooks config & update logs to only log once per request and once per response if enabled

* Update variable names & move condense_logs? checks to logging util

* Update Quickbooks.log to log without condition & move condition to logging utility to allow per-service-instance logging capability

* Update README to include instance configurability demo

* Update log settings check to check for ivar definition instead of just memoizing to handle @log explicitly being set to false for an instance

---------

Co-authored-by: Daniel Ho <daniel.ho@housecallpro.com>
---
 README.md                                |  8 ++++
 lib/quickbooks-ruby.rb                   |  6 +--
 lib/quickbooks/util/logging.rb           |  6 ++-
 spec/lib/quickbooks/util/logging_spec.rb | 58 ++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 6 deletions(-)
 create mode 100644 spec/lib/quickbooks/util/logging_spec.rb

diff --git a/README.md b/README.md
index cbc12e09..e4c5387b 100644
--- a/README.md
+++ b/README.md
@@ -758,9 +758,17 @@ JSON]( https://github.com/ruckus/quickbooks-ruby/issues/257#issuecomment-1268344
 
 ## Logging
 
+Set the default log enablement:
 ```ruby
 Quickbooks.log = true
 ```
+
+Configure a service instance to log (or not) independently:
+```ruby
+customer_service = Quickbooks::Service::Customer.new
+customer_service.log = true
+```
+
 By default, logging is directed at STDOUT, but another target may be defined, e.g. in Rails
 ```ruby
 Quickbooks.logger = Rails.logger
diff --git a/lib/quickbooks-ruby.rb b/lib/quickbooks-ruby.rb
index 19956d9b..416e7300 100644
--- a/lib/quickbooks-ruby.rb
+++ b/lib/quickbooks-ruby.rb
@@ -248,10 +248,8 @@ def log_xml_pretty_print?
     end
 
     def log(msg)
-      if log?
-        logger.info(msg)
-        logger.flush if logger.respond_to?(:flush)
-      end
+      logger.info(msg)
+      logger.flush if logger.respond_to?(:flush)
     end
   end # << self
 
diff --git a/lib/quickbooks/util/logging.rb b/lib/quickbooks/util/logging.rb
index 1d33653f..273d9237 100644
--- a/lib/quickbooks/util/logging.rb
+++ b/lib/quickbooks/util/logging.rb
@@ -1,8 +1,10 @@
 module Quickbooks
   module Util
     module Logging
+      attr_writer :log
+
       def log(msg)
-        ::Quickbooks.log(msg)
+        ::Quickbooks.log(msg) if log?
       end
 
       def log_multiple(messages)
@@ -14,7 +16,7 @@ def log_multiple(messages)
       end
 
       def log?
-        ::Quickbooks.log?
+        defined?(@log) ? @log : ::Quickbooks.log?
       end
 
       def condense_logs?
diff --git a/spec/lib/quickbooks/util/logging_spec.rb b/spec/lib/quickbooks/util/logging_spec.rb
new file mode 100644
index 00000000..13a70f8d
--- /dev/null
+++ b/spec/lib/quickbooks/util/logging_spec.rb
@@ -0,0 +1,58 @@
+describe Quickbooks::Util::Logging do
+  before { Quickbooks.log = general_log_setting }
+  let(:general_log_setting) { true }
+  let(:dummy_class) { Class.new { include Quickbooks::Util::Logging } }
+
+  describe 'log' do
+    context 'when one service needs to log but general logging is disabled' do
+      let(:general_log_setting) { false }
+      let(:control_instance) { instance = dummy_class.new }
+      let(:loggable_instance) do
+        instance = dummy_class.new
+        instance.log = true
+        instance
+      end
+
+      it 'allows one service instance to log without affecting other service instances' do
+        expect(Quickbooks.log?).to be(false)
+        expect(control_instance.log?).to be(false)
+        expect(loggable_instance.log?).to be(true)
+      end
+
+      it 'does not log if disabled' do
+        expect(Quickbooks).not_to receive(:log)
+        control_instance.log('test message')
+      end
+
+      it 'does log if enabled' do
+        expect(Quickbooks).to receive(:log)
+        loggable_instance.log('test message')
+      end
+    end
+
+    context 'when one service needs to not log but general logging is enabled' do
+      let(:control_instance) { instance = dummy_class.new }
+      let(:unloggable_instance) do
+        instance = dummy_class.new
+        instance.log = false
+        instance
+      end
+
+      it 'allows one service instance to log without affecting other service instances' do
+        expect(Quickbooks.log?).to be(true)
+        expect(control_instance.log?).to be(true)
+        expect(unloggable_instance.log?).to be(false)
+      end
+
+      it 'does not log if disabled' do
+        expect(Quickbooks).not_to receive(:log)
+        unloggable_instance.log('test message')
+      end
+
+      it 'does log if enabled' do
+        expect(Quickbooks).to receive(:log)
+        control_instance.log('test message')
+      end
+    end
+  end
+end