From 7f219282b557d44e98c09f0f8b43ca5e3296801b Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 27 Nov 2017 18:14:23 -0500 Subject: [PATCH] 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. --- .../spree/admin/taxjar_settings_controller.rb | 19 --- app/helpers/taxjar_helper.rb | 67 ----------- app/models/concerns/taxable.rb | 7 +- .../spree/app_configuration_decorator.rb | 5 - .../spree/calculator/taxjar_calculator.rb | 20 ++-- app/models/spree/order_decorator.rb | 12 +- app/models/spree/reimbursment_decorator.rb | 6 +- app/models/spree/taxjar.rb | 35 +++--- .../admin/shared/sub_menu/_configuration.rb | 6 - .../spree/admin/taxjar_settings/edit.html.erb | 28 ----- config/initializers/logger.rb | 2 - config/locales/en.yml | 2 - .../admin/taxjar_settings_controller_spec.rb | 69 ----------- .../spree/app_configuration_decorator_spec.rb | 7 -- .../calculator/taxjar_calculator_spec.rb | 108 ++++++------------ spec/models/spree/order_decorator_spec.rb | 81 ++++--------- .../spree/reimbursment_decorator_spec.rb | 36 ++---- spec/models/spree/taxjar_spec.rb | 17 ++- 18 files changed, 113 insertions(+), 414 deletions(-) delete mode 100644 app/controllers/spree/admin/taxjar_settings_controller.rb delete mode 100644 app/helpers/taxjar_helper.rb delete mode 100644 app/models/spree/app_configuration_decorator.rb delete mode 100644 app/overrides/spree/admin/shared/sub_menu/_configuration.rb delete mode 100644 app/views/spree/admin/taxjar_settings/edit.html.erb delete mode 100644 config/initializers/logger.rb delete mode 100644 spec/controllers/spree/admin/taxjar_settings_controller_spec.rb delete mode 100644 spec/models/spree/app_configuration_decorator_spec.rb diff --git a/app/controllers/spree/admin/taxjar_settings_controller.rb b/app/controllers/spree/admin/taxjar_settings_controller.rb deleted file mode 100644 index 6963bee..0000000 --- a/app/controllers/spree/admin/taxjar_settings_controller.rb +++ /dev/null @@ -1,19 +0,0 @@ -module Spree - module Admin - class TaxjarSettingsController < Spree::Admin::BaseController - def edit - @preferences_api = [:taxjar_api_key, :taxjar_enabled, :taxjar_debug_enabled] - end - - def update - Spree::Config[:taxjar_api_key] = params[:taxjar_api_key] - Spree::Config[:taxjar_enabled] = params[:taxjar_enabled] - Spree::Config[:taxjar_debug_enabled] = params[:taxjar_debug_enabled] - - flash[:success] = Spree.t(:taxjar_settings_updated) - redirect_to edit_admin_taxjar_settings_path - end - - end - end -end diff --git a/app/helpers/taxjar_helper.rb b/app/helpers/taxjar_helper.rb deleted file mode 100644 index 0c679ea..0000000 --- a/app/helpers/taxjar_helper.rb +++ /dev/null @@ -1,67 +0,0 @@ -module TaxjarHelper - class Pretty < Logger::Formatter - # Provide a call() method that returns the formatted message. - def call(severity, time, program_name, message) - "#{time.utc.iso8601} #{Process.pid} TID-#{Thread.current.object_id.to_s(36)}#{context} #{severity}: #{message}\n" - end - - def context - Thread.current[:spree_taxjar_context] ? " #{c}" : '' - end - end - - class TaxjarLog - attr_reader :logger - - def initialize(path_name, file_name, log_info = nil, schedule = "weekly") - @logger ||= Logger.new("#{Rails.root}/log/#{path_name}.log", schedule) - @logger.formatter = Pretty.new - progname(file_name.split("/").last.chomp(".rb")) - info(log_info) unless log_info.nil? - end - - def logger_enabled? - Spree::Config[:taxjar_debug_enabled] - end - - def progname(progname = nil) - progname.nil? ? logger.progname : logger.progname = progname - end - - def info(log_info = nil) - if logger_enabled? - logger.info log_info unless log_info.nil? - end - end - - def info_and_debug(log_info, response) - if logger_enabled? - logger.info log_info - if response.is_a?(Hash) - logger.debug JSON.generate(response) - else - logger.debug response - end - end - end - - def debug(error, text = nil) - if logger_enabled? - logger.debug error - if text.nil? - error - else - logger.debug text - text - 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 diff --git a/app/models/concerns/taxable.rb b/app/models/concerns/taxable.rb index cd14b2b..d40869d 100644 --- a/app/models/concerns/taxable.rb +++ b/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 diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb deleted file mode 100644 index 4601ba2..0000000 --- a/app/models/spree/app_configuration_decorator.rb +++ /dev/null @@ -1,5 +0,0 @@ -Spree::AppConfiguration.class_eval do - preference :taxjar_api_key, :string - preference :taxjar_enabled, :boolean, default: false - preference :taxjar_debug_enabled, :boolean, default: false -end diff --git a/app/models/spree/calculator/taxjar_calculator.rb b/app/models/spree/calculator/taxjar_calculator.rb index 0f30961..1641bab 100644 --- a/app/models/spree/calculator/taxjar_calculator.rb +++ b/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,12 +60,12 @@ 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 @@ -75,8 +73,8 @@ def tax_for_item(item) 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 diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 086107c..3a21dc0 100644 --- a/app/models/spree/order_decorator.rb +++ b/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 diff --git a/app/models/spree/reimbursment_decorator.rb b/app/models/spree/reimbursment_decorator.rb index a8d2d26..d1715b3 100644 --- a/app/models/spree/reimbursment_decorator.rb +++ b/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 diff --git a/app/models/spree/taxjar.rb b/app/models/spree/taxjar.rb index 1645533..00ca6e8 100644 --- a/app/models/spree/taxjar.rb +++ b/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 diff --git a/app/overrides/spree/admin/shared/sub_menu/_configuration.rb b/app/overrides/spree/admin/shared/sub_menu/_configuration.rb deleted file mode 100644 index 401bcd0..0000000 --- a/app/overrides/spree/admin/shared/sub_menu/_configuration.rb +++ /dev/null @@ -1,6 +0,0 @@ -Deface::Override.new( - virtual_path: "spree/admin/shared/sub_menu/_configuration", - name: "add_taxjar_admin_menu_link", - insert_bottom: "[data-hook='admin_configurations_sidebar_menu']", - text: "<%= configurations_sidebar_menu_item 'Taxjar Settings', edit_admin_taxjar_settings_path %>" -) diff --git a/app/views/spree/admin/taxjar_settings/edit.html.erb b/app/views/spree/admin/taxjar_settings/edit.html.erb deleted file mode 100644 index 0d03295..0000000 --- a/app/views/spree/admin/taxjar_settings/edit.html.erb +++ /dev/null @@ -1,28 +0,0 @@ -<%= render 'spree/admin/shared/taxes_tabs' %> - -<% content_for :page_title do %> - <%= Spree.t(:taxjar_settings) %> -<% end %> - -<%= form_tag admin_taxjar_settings_path, method: :put do %> -
-
-
-
- <% @preferences_api.each do |key| %> -
- <%= label_tag key %> - <%= render "spree/admin/shared/preference_fields/#{Spree::Config.preference_type(key)}", name: key, value: Spree::Config[key] %> -
- <% end %> -
-
-
- -
- <%= button Spree.t('actions.update') %> - <%= Spree.t(:or) %> - <%= button_link_to Spree.t('actions.cancel'), admin_orders_url %> -
-
-<% end %> diff --git a/config/initializers/logger.rb b/config/initializers/logger.rb deleted file mode 100644 index 56fb741..0000000 --- a/config/initializers/logger.rb +++ /dev/null @@ -1,2 +0,0 @@ -SpreeTaxjar::Logger = TaxjarHelper::TaxjarLog.new("spree_taxjar", "taxjar_calculator") -SpreeTaxjar::Logger.logger.extend(ActiveSupport::Logger.broadcast(Rails.logger)) diff --git a/config/locales/en.yml b/config/locales/en.yml index 0b5d0fa..3c1c416 100644 --- a/config/locales/en.yml +++ b/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 diff --git a/spec/controllers/spree/admin/taxjar_settings_controller_spec.rb b/spec/controllers/spree/admin/taxjar_settings_controller_spec.rb deleted file mode 100644 index 7992a16..0000000 --- a/spec/controllers/spree/admin/taxjar_settings_controller_spec.rb +++ /dev/null @@ -1,69 +0,0 @@ -require 'spec_helper' - -describe Spree::Admin::TaxjarSettingsController, type: :controller do - render_views - - let(:user) { mock_model(Spree.user_class).as_null_object } - - before(:each) do - allow(controller).to receive(:spree_current_user).and_return(user) - allow(controller).to receive(:authorize!).and_return(true) - allow(controller).to receive(:authorize_admin).and_return(true) - allow(user).to receive(:spree_roles).and_return [] - end - - describe "GET 'edit'" do - - def send_request - get :edit - end - - before do - send_request - end - - it "assigns @preferences_api" do - expect( response.body ).to match 'Taxjar api key' - expect( response.body ).to match 'Taxjar enabled' - expect( response.body ).to match 'Taxjar debug enabled' - end - - it "renders edit template" do - expect( response.body ).to match Spree.t(:taxjar_settings) - end - - end - - describe "PUT 'update'" do - - def send_request - put :update, params: {taxjar_api_key: 'SAMPLE_API_KEY', taxjar_enabled: '1', taxjar_debug_enabled: '1' } - end - - before do - send_request - end - - it "saves taxjar_api_key with passed parameter" do - expect(Spree::Config[:taxjar_api_key]).to eq 'SAMPLE_API_KEY' - end - - it "saves taxjar_enabled with passed parameter" do - expect(Spree::Config[:taxjar_enabled]).to be(true) - end - - it "saves taxjar_debug_enabled with passed parameter" do - expect(Spree::Config[:taxjar_debug_enabled]).to be(true) - end - - it "sets flash message to success" do - expect(flash[:success]).to eq Spree.t(:taxjar_settings_updated) - end - - it "renders edit template" do - expect(response).to redirect_to(edit_admin_taxjar_settings_path) - end - - end - -end diff --git a/spec/models/spree/app_configuration_decorator_spec.rb b/spec/models/spree/app_configuration_decorator_spec.rb deleted file mode 100644 index dbe9b99..0000000 --- a/spec/models/spree/app_configuration_decorator_spec.rb +++ /dev/null @@ -1,7 +0,0 @@ -require 'spec_helper' - -describe Spree::AppConfiguration do - it 'expects spree config to have taxjar_api_key' do - expect(Spree::Config).to have_preference(:taxjar_api_key) - end -end diff --git a/spec/models/spree/calculator/taxjar_calculator_spec.rb b/spec/models/spree/calculator/taxjar_calculator_spec.rb index 931df65..91c28a1 100644 --- a/spec/models/spree/calculator/taxjar_calculator_spec.rb +++ b/spec/models/spree/calculator/taxjar_calculator_spec.rb @@ -13,7 +13,7 @@ let!(:tax_category_exempt) { create(:tax_category, tax_rates: []) } let!(:rate) { create(:tax_rate, tax_categories: [tax_category], amount: 0.05, included_in_price: included_in_price) } let(:included_in_price) { false } - let!(:calculator) { Spree::Calculator::TaxjarCalculator.new(calculable: rate) } + let!(:calculator) { Spree::Calculator::TaxjarCalculator.new(calculable: rate, preferred_api_key: '04d828b7374896d7867b03289ea20957') } let!(:order) { create(:order,ship_address_id: ship_address.id) } let!(:line_item) { create(:line_item, price: 10, quantity: 3, order_id: order.id) } let!(:line_item_exempt) { create(:line_item, price: 10, quantity: 3, order_id: order.id) } @@ -25,7 +25,6 @@ let(:taxjar_response) { double(Taxjar::Tax) } before do - Spree::Config[:taxjar_api_key] = '04d828b7374896d7867b03289ea20957' ## Forcing tests with shipping_address as tax_address Spree::Config[:tax_using_ship_address] = true end @@ -43,78 +42,49 @@ end describe '#compute_line_item' do - context 'when taxjar calculation disabled' do - before :each do - Spree::Config[:taxjar_enabled] = false - end - - it 'tax should be zero' do - expect(calculator.compute_line_item(line_item)).to eq(0) - end + before :each do + tax_category_exempt.update_column(:tax_code, taxjar_exempt_tax_code) + line_item_exempt.update_column(:tax_category_id, tax_category_exempt.id) end - context 'when taxjar calculation enabled' do - before :each do - Spree::Config[:taxjar_enabled] = true - tax_category_exempt.update_column(:tax_code, taxjar_exempt_tax_code) - line_item_exempt.update_column(:tax_category_id, tax_category_exempt.id) - end - - context 'when rate not included in price' do - it 'returns tax for the line_item upto two decimal places' do - VCR.use_cassette "fully_taxable_line_item" do - expect(calculator.compute_line_item(line_item)).to eq(2.33) - end - end - - it 'should return ZERO tax for line_item having tax exempt code' do - VCR.use_cassette "fully_exempt_line_item" do - expect(calculator.compute_line_item(line_item_exempt)).to eq(0.0) - end + context 'when rate not included in price' do + it 'returns tax for the line_item upto two decimal places' do + VCR.use_cassette "fully_taxable_line_item" do + expect(calculator.compute_line_item(line_item)).to eq(2.33) end end - context 'when rate included in price' do - before do - rate.included_in_price = true - rate.save! - end - it 'returns tax for the line_item upto two decimal places' do - expect(calculator.compute_line_item(line_item)).to eq(0) + it 'should return ZERO tax for line_item having tax exempt code' do + VCR.use_cassette "fully_exempt_line_item" do + expect(calculator.compute_line_item(line_item_exempt)).to eq(0.0) end end end - end - describe '#compute_shipment' do - context 'when taxjar calculation disabled' do - before :each do - Spree::Config[:taxjar_enabled] = false + context 'when rate included in price' do + before do + rate.included_in_price = true + rate.save! end - - it 'tax should be zero' do - expect(calculator.compute_shipment(shipment)).to eq(0) + it 'returns tax for the line_item upto two decimal places' do + expect(calculator.compute_line_item(line_item)).to eq(0) end end + end - context 'when taxjar calculation enabled' do - before :each do - Spree::Config[:taxjar_enabled] = true - end - - context 'Nexus charges tax on shipping' do - it 'should return tax on shipping' do - VCR.use_cassette "compute_shipment_with_texas_address" do - expect(calculator.compute_shipment(shipment)).to eq(0.78) - end + describe '#compute_shipment' do + context 'Nexus charges tax on shipping' do + it 'should return tax on shipping' do + VCR.use_cassette "compute_shipment_with_texas_address" do + expect(calculator.compute_shipment(shipment)).to eq(0.78) end end + end - context 'Nexus charges NO tax on shipping' do - it 'should return tax on shipping as ZERO' do - VCR.use_cassette "compute_shipment_with_california_address" do - expect(calculator.compute_shipment(shipment_ca)).to eq(0) - end + context 'Nexus charges NO tax on shipping' do + it 'should return tax on shipping as ZERO' do + VCR.use_cassette "compute_shipment_with_california_address" do + expect(calculator.compute_shipment(shipment_ca)).to eq(0) end end end @@ -128,24 +98,12 @@ describe '#compute_shipping_rate' do context 'when rate included in price' do - context 'when taxjar calculation disabled' do - before :each do - Spree::Config[:taxjar_enabled] = false - end - - it 'tax should be zero' do - expect(calculator.compute_shipment(shipment)).to eq(0) - end + before do + rate.included_in_price = true + rate.save! end - context 'when taxjar calculation enabled' do - before do - rate.included_in_price = true - rate.save! - Spree::Config[:taxjar_enabled] = true - end - it 'will raise RuntimeError' do - expect{ calculator.compute_shipping_rate(line_item)}.to raise_error(RuntimeError) - end + it 'will raise RuntimeError' do + expect{ calculator.compute_shipping_rate(line_item)}.to raise_error(RuntimeError) end end context 'when rate not included in price' do diff --git a/spec/models/spree/order_decorator_spec.rb b/spec/models/spree/order_decorator_spec.rb index 267b1f4..529078f 100644 --- a/spec/models/spree/order_decorator_spec.rb +++ b/spec/models/spree/order_decorator_spec.rb @@ -4,6 +4,7 @@ let(:order) { create(:order) } let(:client) { double(Spree::Taxjar) } + let(:rate) { create :tax_rate, calculator: Spree::Calculator::TaxjarCalculator.new(preferred_api_key: 'api-key') } describe 'Constants' do it 'should include Taxable' do @@ -13,82 +14,46 @@ describe 'Instance Methods' do describe '#delete_taxjar_transaction' do - context 'when taxjar calculation disabled' do - before :each do - Spree::Config[:taxjar_enabled] = false + context 'when taxjar_rate is nil' do + it 'should return nil' do + expect(order.send(:delete_taxjar_transaction)).to eq nil end - - it 'tax should be zero' do - expect(order).to_not receive(:taxjar_applicable?) - end - - after { order.send(:delete_taxjar_transaction) } end - context 'when taxjar calculation enabled' do - before :each do - Spree::Config[:taxjar_enabled] = true + context 'when taxjar_rate is available' do + before do + allow(order).to receive(:taxjar_rate).with(order).and_return(rate) + allow(Spree::Taxjar).to receive(:new).with('api-key', order).and_return(client) + allow(client).to receive(:delete_transaction_for_order) end - context 'when taxjar_applicable? returns false' do - it 'should return nil' do - expect(order.send(:delete_taxjar_transaction)).to eq nil - end + it 'should delete transaction for order' do + expect(client).to receive(:delete_transaction_for_order) end - context 'when taxjar_applicable? return true' do - before do - allow(order).to receive(:taxjar_applicable?).with(order).and_return(true) - allow(Spree::Taxjar).to receive(:new).with(order).and_return(client) - allow(client).to receive(:delete_transaction_for_order) - end - - it 'should delete transaction for order' do - expect(client).to receive(:delete_transaction_for_order) - end - - after { order.send(:delete_taxjar_transaction) } - end + after { order.send(:delete_taxjar_transaction) } end end describe '#capture_taxjar' do - context 'when taxjar calculation disabled' do - before :each do - Spree::Config[:taxjar_enabled] = false + context 'when taxjar_rate is nil' do + it 'should return nil' do + expect(order.send(:capture_taxjar)).to eq nil end - - it 'tax should be zero' do - expect(order).to_not receive(:taxjar_applicable?) - end - - after { order.send(:capture_taxjar) } end - context 'when taxjar calculation enabled' do - before :each do - Spree::Config[:taxjar_enabled] = true + context 'when taxjar_rate is available' do + before do + allow(order).to receive(:taxjar_rate).with(order).and_return(rate) + allow(Spree::Taxjar).to receive(:new).with('api-key', order).and_return(client) + allow(client).to receive(:create_transaction_for_order) end - context 'when taxjar_applicable? returns false' do - it 'should return nil' do - expect(order.send(:capture_taxjar)).to eq nil - end + it 'should create transaction for order' do + expect(client).to receive(:create_transaction_for_order) end - context 'when taxjar_applicable? return true' do - before do - allow(order).to receive(:taxjar_applicable?).with(order).and_return(true) - allow(Spree::Taxjar).to receive(:new).with(order).and_return(client) - allow(client).to receive(:create_transaction_for_order) - end - - it 'should create transaction for order' do - expect(client).to receive(:create_transaction_for_order) - end - - after { order.send(:capture_taxjar) } - end + after { order.send(:capture_taxjar) } end end end diff --git a/spec/models/spree/reimbursment_decorator_spec.rb b/spec/models/spree/reimbursment_decorator_spec.rb index cc2cd02..538e747 100644 --- a/spec/models/spree/reimbursment_decorator_spec.rb +++ b/spec/models/spree/reimbursment_decorator_spec.rb @@ -4,6 +4,7 @@ let(:reimbursement) { create(:reimbursement) } let(:client) { double(Spree::Taxjar) } + let(:rate) { create :tax_rate, calculator: Spree::Calculator::TaxjarCalculator.new(preferred_api_key: 'api-key') } describe 'Constants' do it 'should include Taxable' do @@ -13,39 +14,24 @@ describe 'Instance Methods' do describe '#remove_tax_for_returned_items' do - context 'when taxjar_applicable? returns false' do + context 'when taxjar_rate is nil' do it 'should return nil' do expect(reimbursement.send(:remove_tax_for_returned_items)).to eq nil end end context 'when taxjar_applicable? return true' do - context 'when taxjar calculation disabled' do - before :each do - Spree::Config[:taxjar_enabled] = false - end - - it 'tax should be zero' do - expect(reimbursement).to_not receive(:taxjar_applicable?) - end - - after { reimbursement.remove_tax_for_returned_items } + before do + @order = reimbursement.order + allow(reimbursement).to receive(:taxjar_rate).with(@order).and_return(rate) + allow(Spree::Taxjar).to receive(:new).with('api-key', @order, reimbursement).and_return(client) + allow(client).to receive(:create_refund_transaction_for_order) end - context 'when taxjar calculation enabled' do - before do - Spree::Config[:taxjar_enabled] = true - @order = reimbursement.order - allow(reimbursement).to receive(:taxjar_applicable?).with(@order).and_return(true) - allow(Spree::Taxjar).to receive(:new).with(@order, reimbursement).and_return(client) - allow(client).to receive(:create_refund_transaction_for_order) - end - - it 'should remive tax for reimbursed items' do - expect(client).to receive(:create_refund_transaction_for_order) - end - - after { reimbursement.remove_tax_for_returned_items } + it 'should remive tax for reimbursed items' do + expect(client).to receive(:create_refund_transaction_for_order) end + + after { reimbursement.remove_tax_for_returned_items } end end diff --git a/spec/models/spree/taxjar_spec.rb b/spec/models/spree/taxjar_spec.rb index 670cd7e..e3990e0 100644 --- a/spec/models/spree/taxjar_spec.rb +++ b/spec/models/spree/taxjar_spec.rb @@ -15,10 +15,10 @@ let!(:order_al) { create(:order,ship_address_id: ship_address_al.id) } let!(:line_item_al) { create(:line_item, price: 10, quantity: 3, order_id: order_al.id) } let!(:shipment_al) { create(:shipment, cost: 10, order: order_al) } - let!(:taxjar_api_key) { Spree::Config[:taxjar_api_key] = '04d828b7374896d7867b03289ea20957' } + let!(:taxjar_api_key) { '04d828b7374896d7867b03289ea20957' } let(:client) { double(Taxjar::Client) } - let(:spree_taxjar) { Spree::Taxjar.new(order) } + let(:spree_taxjar) { Spree::Taxjar.new(taxjar_api_key, order) } describe '#has_nexus?' do context 'nexus_regions is not present' do @@ -40,7 +40,7 @@ context 'tax_address is not present in nexus regions' do before :each do - @spree_taxjar_new = Spree::Taxjar.new(order_al) + @spree_taxjar_new = Spree::Taxjar.new(taxjar_api_key, order_al) end it 'should return false' do @@ -54,11 +54,10 @@ context 'When reimbursement is not present' do before :each do - Spree::Config[:taxjar_api_key] = '04d828b7374896d7867b03289ea20957' - allow(::Taxjar::Client).to receive(:new).with(api_key: Spree::Config[:taxjar_api_key]).and_return(client) + allow(::Taxjar::Client).to receive(:new).with(api_key: taxjar_api_key).and_return(client) end - let(:spree_taxjar) { Spree::Taxjar.new(order) } + let(:spree_taxjar) { Spree::Taxjar.new(taxjar_api_key, order) } describe '#initialize' do @@ -88,7 +87,7 @@ context 'when has_nexus? returns true' do before do - allow(::Taxjar::Client).to receive(:new).with(api_key: Spree::Config[:taxjar_api_key]).and_return(client) + allow(::Taxjar::Client).to receive(:new).with(api_key: taxjar_api_key).and_return(client) allow(spree_taxjar).to receive(:has_nexus?).and_return(true) allow(client).to receive(:create_order).and_return(true) end @@ -158,9 +157,9 @@ end context 'When reimbursement is present' do - let(:spree_taxjar) { Spree::Taxjar.new(order, reimbursement) } + let(:spree_taxjar) { Spree::Taxjar.new(taxjar_api_key, order, reimbursement) } before do - allow(::Taxjar::Client).to receive(:new).with(api_key: Spree::Config[:taxjar_api_key]).and_return(client) + allow(::Taxjar::Client).to receive(:new).with(api_key: taxjar_api_key).and_return(client) end describe '#create_refund_transaction_for_order' do