Added test_cases for taxjar calculator

Added vcr and web mock gem to avoid mocking
 and leaving a trace for api requests params and responses
Added logging to see and trace the action in logs

.gitignore

@@ -16,3 +16,4 @@ spec/dummy
 .rvmrc
 .ruby-version
 .ruby-gemset
+.byebug_history

Gemfile

@@ -5,5 +5,6 @@ gem 'spree', github: 'spree/spree', branch: '3-1-stable'
 gem 'spree_auth_devise', github: 'spree/spree_auth_devise', branch: '3-1-stable'
 group :test do
   gem 'rails-controller-testing'
+  gem 'byebug'
 end
 gemspec

app/helpers/taxjar_helper.rb

@@ -59,5 +59,12 @@ def debug(error, text = nil)
         end
       end
     end
+
+    def log(method, request_hash = nil)
+      logger.info method.to_s + ' call'
+      return if request_hash.nil?
+      logger.debug request_hash
+      logger.debug JSON.generate(request_hash)
+    end
   end
 end

app/models/spree/calculator/taxjar_calculator.rb

@@ -15,6 +15,7 @@ def compute_order(order)
     end
 
     def compute_line_item(item)
+      SpreeTaxjar::Logger.log(__method__, {line_item: {order: {id: item.order.id, number: item.order.number}}}) if SpreeTaxjar::Logger.logger_enabled?
       return 0 unless Spree::Config[:taxjar_enabled]
       if rate.included_in_price
         0
@@ -23,9 +24,10 @@ def compute_line_item(item)
       end
     end
 
-    def compute_shipment(item)
+    def compute_shipment(shipment)
+      SpreeTaxjar::Logger.log(__method__, {shipment: {order: {id: shipment.order.id, number: shipment.order.number}}}) if SpreeTaxjar::Logger.logger_enabled?
       return 0 unless Spree::Config[:taxjar_enabled]
-      tax_for_shipment(item)
+      tax_for_shipment(shipment)
     end
 
     def compute_shipping_rate(shipping_rate)
@@ -46,7 +48,11 @@ def tax_for_shipment(shipment)
         order = shipment.order
         return 0 unless tax_address = order.tax_address
 
-        Rails.cache.fetch(cache_key(order, shipment, tax_address), expires_in: CACHE_EXPIRATION_DURATION) do
+        rails_cache_key = cache_key(order, shipment, tax_address)
+
+        SpreeTaxjar::Logger.log(__method__, {shipment: {order: {id: shipment.order.id, number: shipment.order.number}}, cache_key: rails_cache_key}) if SpreeTaxjar::Logger.logger_enabled?
+
+        Rails.cache.fetch(rails_cache_key, expires_in: CACHE_EXPIRATION_DURATION) do
           Spree::Taxjar.new(order, nil, shipment).calculate_tax_for_shipment
         end
       end
@@ -55,20 +61,35 @@ def tax_for_item(item)
         order = item.order
         return 0 unless tax_address = order.tax_address
 
-        unless Rails.cache.read(cache_key(order, item, tax_address))
+        rails_cache_key = cache_key(order, item, tax_address)
+
+        SpreeTaxjar::Logger.log(__method__, {line_item: {order: {id: item.order.id, number: item.order.number}}, cache_key: rails_cache_key}) if SpreeTaxjar::Logger.logger_enabled?
+
+        ## Test when caching enabled that only 1 API call is sent for an order
+        ## should avoid N calls for N line_items
+        Rails.cache.fetch(rails_cache_key, expires_in: CACHE_EXPIRATION_DURATION) do
           taxjar_response = Spree::Taxjar.new(order).calculate_tax_for_order
           return 0 unless taxjar_response
-          cache_response(taxjar_response, order, tax_address)
+          tax_for_current_item = cache_response(taxjar_response, order, tax_address, item)
+          tax_for_current_item
         end
-
-        Rails.cache.read(cache_key(order, item, tax_address))
       end
 
-      def cache_response(taxjar_response, order, address)
+      def cache_response(taxjar_response, order, address, item = nil)
+        SpreeTaxjar::Logger.log(__method__, {order: {id: order.id, number: order.number}, taxjar_api_advanced_res: taxjar_response}) if SpreeTaxjar::Logger.logger_enabled?
+        SpreeTaxjar::Logger.log(__method__, {order: {id: order.id, number: order.number}, taxjar_api_advanced_res: taxjar_response.breakdown.line_items}) if SpreeTaxjar::Logger.logger_enabled?
+        ## res is set to faciliate testing as to return computed result from API
+        ## for given line_item
+        ## better to use Rails.cache.fetch for order and wrapping lookup based on line_item id
+        res = nil
         taxjar_response.breakdown.line_items.each do |line_item|
-          item =  Spree::LineItem.find_by(id: line_item.id)
-          Rails.cache.write(cache_key(order, item, address), line_item.tax_collectable, expires_in: CACHE_EXPIRATION_DURATION)
+          item_from_db = Spree::LineItem.find_by(id: line_item.id)
+          if item && item_from_db.id == item.id
+            res = line_item.tax_collectable
+          end
+          Rails.cache.write(cache_key(order, item_from_db, address), line_item.tax_collectable, expires_in: CACHE_EXPIRATION_DURATION)
         end
+        res
       end
 
       def cache_key(order, item, address)

app/models/spree/taxjar.rb

@@ -23,8 +23,13 @@ def delete_transaction_for_order
     end
 
     def calculate_tax_for_shipment
+      SpreeTaxjar::Logger.log(__method__, {shipment: {order: {id: @shipment.order.id, number: @shipment.order.number}}}) if SpreeTaxjar::Logger.logger_enabled?
       if has_nexus?
-        @client.tax_for_order(shipment_tax_params).amount_to_collect
+        api_params = shipment_tax_params
+        SpreeTaxjar::Logger.log(__method__, {shipment: {order: {id: @shipment.order.id, number: @shipment.order.number}, api_params: api_params}}) if SpreeTaxjar::Logger.logger_enabled?
+        api_response = @client.tax_for_order(api_params)
+        SpreeTaxjar::Logger.log(__method__, {shipment: {order: {id: @shipment.order.id, number: @shipment.order.number}, api_response: api_response}}) if SpreeTaxjar::Logger.logger_enabled?
+        api_response.amount_to_collect
       else
         0
       end
@@ -44,8 +49,13 @@ def nexus_states(nexus_regions)
     end
 
     def calculate_tax_for_order
+      SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}}) if SpreeTaxjar::Logger.logger_enabled?
       if has_nexus?
-        @client.tax_for_order(tax_params)
+        api_params = tax_params
+        SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}, api_params: api_params}) if SpreeTaxjar::Logger.logger_enabled?
+        api_response = @client.tax_for_order(api_params)
+        SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}, api_response: api_response}) if SpreeTaxjar::Logger.logger_enabled?
+        api_response
       else
         0
       end