Refactor preferences dropping unneeded functionality

The core goal was to move the preference off the global config. This
is primarily because Solidus has made the global config store by
default not database-driven.

This means everytime the system is reset any config set is reverted
to whatever the intial state is (blank for these new config).

I did consider just adding the API key to to code but I think it is
more flexible and better to store it with the actual calculator rather
than have it be global.

In addition to the API key there are two other config options:

* Enabled flag
* Special logging

The enabled flag is not really needed. If you don't want it enabled
just don't have the rate! This allows us to reduce the code paths.
It is slightly less efficient in the case where there is no configured
tax rate. But since that is not really the normal state I didn't really
worry about htat.

The special logging was actually quite a bit of code and non-normal
for a Rails app. The better thing is just to log to the normal Rails
logger so tools can consume it.

I can understand some argument for a seperate file so you can just
get the taxjar info. But if you really need that lets do it the right
way. Add tagging to the log messages and then the log system can
easily filter on that tag.

End result is that we remove a lot of unnecessary code by removing
some of the options. We also give ourselve more flexibility by storing
the API key with the calculator.

app/models/concerns/taxable.rb

@@ -1,8 +1,7 @@
 module Taxable
   extend ActiveSupport::Concern
 
-  private
-    def taxjar_applicable?(order)
-      Spree::TaxRate.for_address(order.tax_address).any? { |rate| rate.calculator_type == "Spree::Calculator::TaxjarCalculator" }
-    end
+  def taxjar_rate order
+    Spree::TaxRate.for_address(order.tax_address).to_a.find { |rate| rate.calculator_type == "Spree::Calculator::TaxjarCalculator" }
+  end
 end

app/models/spree/calculator/taxjar_calculator.rb

@@ -2,6 +2,7 @@
 
 module Spree
   class Calculator::TaxjarCalculator < Calculator
+    preference :api_key, :string
 
     CACHE_EXPIRATION_DURATION = 10.minutes
 
@@ -14,8 +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]
+      logger.debug line_item: {order: {id: item.order.id, number: item.order.number}}
       if rate.included_in_price
         0
       else
@@ -24,13 +24,11 @@ def compute_line_item(item)
     end
 
     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]
+      logger.debug shipment: {order: {id: shipment.order.id, number: shipment.order.number}}
       tax_for_shipment(shipment)
     end
 
     def compute_shipping_rate(shipping_rate)
-      return 0 unless Spree::Config[:taxjar_enabled]
       if rate.included_in_price
         raise Spree.t(:shipping_rate_exception_message)
       else
@@ -49,10 +47,10 @@ def tax_for_shipment(shipment)
 
         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?
+        logger.debug shipment: {order: {id: shipment.order.id, number: shipment.order.number}}, cache_key: rails_cache_key
 
         Rails.cache.fetch(rails_cache_key, expires_in: CACHE_EXPIRATION_DURATION) do
-          Spree::Taxjar.new(order, nil, shipment).calculate_tax_for_shipment
+          Spree::Taxjar.new(preferred_api_key, order, nil, shipment).calculate_tax_for_shipment
         end
       end
 
@@ -62,21 +60,21 @@ def tax_for_item(item)
 
         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?
+        logger.debug line_item: {order: {id: item.order.id, number: item.order.number}}, cache_key: rails_cache_key
 
         ## 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
+          taxjar_response = Spree::Taxjar.new(preferred_api_key, order).calculate_tax_for_order
           return 0 unless taxjar_response
           tax_for_current_item = cache_response(taxjar_response, order, tax_address, item)
           tax_for_current_item
         end
       end
 
       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?
+        logger.debug order: {id: order.id, number: order.number}, taxjar_api_advanced_res: taxjar_response
+        logger.debug order: {id: order.id, number: order.number}, taxjar_api_advanced_res: taxjar_response.breakdown.line_items
         ## 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

app/models/spree/order_decorator.rb

@@ -8,16 +8,16 @@
   private
 
     def delete_taxjar_transaction
-      return unless Spree::Config[:taxjar_enabled]
-      return unless taxjar_applicable?(self)
-      client = Spree::Taxjar.new(self)
+      rate = taxjar_rate self
+      return unless rate
+      client = Spree::Taxjar.new(rate.calculator.preferred_api_key, self)
       client.delete_transaction_for_order
     end
 
     def capture_taxjar
-      return unless Spree::Config[:taxjar_enabled]
-      return unless taxjar_applicable?(self)
-      client = Spree::Taxjar.new(self)
+      rate = taxjar_rate self
+      return unless rate
+      client = Spree::Taxjar.new(rate.calculator.preferred_api_key, self)
       client.create_transaction_for_order
     end
 end

app/models/spree/reimbursment_decorator.rb

@@ -5,9 +5,9 @@
   state_machine.after_transition to: [:reimbursed], do: :remove_tax_for_returned_items
 
   def remove_tax_for_returned_items
-    return unless Spree::Config[:taxjar_enabled]
-    return unless taxjar_applicable?(order)
-    client = Spree::Taxjar.new(order, self)
+    rate = taxjar_rate order
+    return unless rate
+    client = Spree::Taxjar.new(rate.calculator.preferred_api_key, order, self)
     client.create_refund_transaction_for_order
   end
 end

app/models/spree/taxjar.rb

@@ -2,52 +2,52 @@ module Spree
   class Taxjar
     attr_reader :client, :order, :reimbursement, :shipment
 
-    def initialize(order = nil, reimbursement = nil, shipment = nil)
+    def initialize(api_key, order = nil, reimbursement = nil, shipment = nil)
       @order = order
       @shipment = shipment
       @reimbursement = reimbursement
-      @client = ::Taxjar::Client.new(api_key: Spree::Config[:taxjar_api_key])
+      @client = ::Taxjar::Client.new api_key: api_key
     end
 
     def create_refund_transaction_for_order
       if has_nexus? && !reimbursement_present?
         api_params = refund_params
-        SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}, reimbursement: {id: @reimbursement.id, number: @reimbursement.number}, api_params: api_params}) if SpreeTaxjar::Logger.logger_enabled?
+        Rails.logger.debug order: {id: @order.id, number: @order.number}, reimbursement: {id: @reimbursement.id, number: @reimbursement.number}, api_params: api_params
         api_response = @client.create_refund(api_params)
-        SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}, reimbursement: {id: @reimbursement.id, number: @reimbursement.number}, api_response: api_response}) if SpreeTaxjar::Logger.logger_enabled?
+        Rails.logger.debug order: {id: @order.id, number: @order.number}, reimbursement: {id: @reimbursement.id, number: @reimbursement.number}, api_response: api_response
         api_response
       end
     end
 
     def create_transaction_for_order
-      SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}}) if SpreeTaxjar::Logger.logger_enabled?
+      Rails.logger.debug order: {id: @order.id, number: @order.number}
       if has_nexus?
         api_params = transaction_parameters
-        SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}, api_params: api_params}) if SpreeTaxjar::Logger.logger_enabled?
+        Rails.logger.debug order: {id: @order.id, number: @order.number}, api_params: api_params
         api_response = @client.create_order(api_params)
-        SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}, api_response: api_response}) if SpreeTaxjar::Logger.logger_enabled?
+        Rails.logger.debug order: {id: @order.id, number: @order.number}, api_response: api_response
         api_response
       end
     end
 
     def delete_transaction_for_order
-      SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}}) if SpreeTaxjar::Logger.logger_enabled?
+      Rails.logger.debug order: {id: @order.id, number: @order.number}
       if has_nexus?
         api_response = @client.delete_order(@order.number)
-        SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}, api_response: api_response}) if SpreeTaxjar::Logger.logger_enabled?
+        Rails.logger.debug order: {id: @order.id, number: @order.number}, api_response: api_response
         api_response
       end
     rescue ::Taxjar::Error::NotFound => e
-      SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}, error_msg: e.message}) if SpreeTaxjar::Logger.logger_enabled?
+      Rails.logger.warn order: {id: @order.id, number: @order.number}, error_msg: e.message
     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?
+      Rails.logger.debug shipment: {order: {id: @shipment.order.id, number: @shipment.order.number}}
       if has_nexus?
         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?
+        Rails.logger.debug shipment: {order: {id: @shipment.order.id, number: @shipment.order.number}, api_params: api_params}
         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?
+        Rails.logger.debug shipment: {order: {id: @shipment.order.id, number: @shipment.order.number}, api_response: api_response}
         api_response.amount_to_collect
       else
         0
@@ -56,11 +56,10 @@ def calculate_tax_for_shipment
 
     def has_nexus?
       nexus_regions = @client.nexus_regions
-      SpreeTaxjar::Logger.log(__method__, {
+      Rails.logger.debug \
         order: {id: @order.id, number: @order.number},
         nexus_regions: nexus_regions,
         address: {state: tax_address_state_abbr, city: tax_address_city, zip: tax_address_zip}
-      }) if SpreeTaxjar::Logger.logger_enabled?
       if nexus_regions.present?
         nexus_states(nexus_regions).include?(tax_address_state_abbr)
       else
@@ -69,12 +68,12 @@ def has_nexus?
     end
 
     def calculate_tax_for_order
-      SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}}) if SpreeTaxjar::Logger.logger_enabled?
+      Rails.logger.debug order: {id: @order.id, number: @order.number}
       if has_nexus?
         api_params = tax_params
-        SpreeTaxjar::Logger.log(__method__, {order: {id: @order.id, number: @order.number}, api_params: api_params}) if SpreeTaxjar::Logger.logger_enabled?
+        Rails.logger.debug order: {id: @order.id, number: @order.number}, api_params: api_params
         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?
+        Rails.logger.debug order: {id: @order.id, number: @order.number}, api_response: api_response
         api_response
       end
     end

config/locales/en.yml

@@ -3,7 +3,5 @@ en:
     taxjar_settings_updated: TaxJar Settings successfully updated
     shipping_rate_exception_message: TaxJar cannot calculate inclusive sales taxes.
     taxjar_api_key: TaxJar API Token
-    taxjar_enabled: Enable TaxJar Calculation
-    taxjar_debug_enabled: Enable TaxJar Debug Logging
     taxjar_calculator_description: TaxJar Calculator
     taxjar_settings: TaxJar Settings