From 6e800341c359356801d529ff9cfd5a2cf5fe6c0f Mon Sep 17 00:00:00 2001 From: Victor Nava Date: Wed, 4 Mar 2015 16:00:46 +1100 Subject: [PATCH 01/35] Fixes issue #362 Change admin account link to point to account page instead of edit user. --- app/views/spree/layouts/admin/_login_nav.html.haml | 2 +- spec/features/admin/authentication_spec.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/views/spree/layouts/admin/_login_nav.html.haml b/app/views/spree/layouts/admin/_login_nav.html.haml index 4ecb72d148..088ac02377 100644 --- a/app/views/spree/layouts/admin/_login_nav.html.haml +++ b/app/views/spree/layouts/admin/_login_nav.html.haml @@ -5,7 +5,7 @@ \: #{spree_current_user.email} %li{"data-hook" => "user-account-link"} %i.icon-user - = link_to t(:account), spree.edit_user_path(spree_current_user) + = link_to t(:account), account_path %li{"data-hook" => "user-logout-link"} %i.icon-signout = link_to t(:logout), spree.logout_path diff --git a/spec/features/admin/authentication_spec.rb b/spec/features/admin/authentication_spec.rb index beb8c31c2b..12e9795ad2 100644 --- a/spec/features/admin/authentication_spec.rb +++ b/spec/features/admin/authentication_spec.rb @@ -2,6 +2,9 @@ require 'spec_helper' feature "Authentication", js: true do include UIComponentHelper + include AuthenticationWorkflow + include WebHelper + let(:user) { create(:user, password: "password", password_confirmation: "password") } scenario "logging into admin redirects home, then back to admin" do @@ -16,4 +19,10 @@ feature "Authentication", js: true do page.should have_content "Dashboard" current_path.should == spree.admin_path end + + scenario "viewing my account" do + login_to_admin_section + click_link "Account" + current_path.should == spree.account_path + end end From be55f461d086017c54359c5a89518eec7aab6d32 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Sun, 23 Nov 2014 19:51:52 +0000 Subject: [PATCH 02/35] Report: sales tax on orders --- .../admin/reports_controller_decorator.rb | 34 ++++++++++++- app/models/spree/ability_decorator.rb | 2 +- config/routes.rb | 3 +- lib/open_food_network/sales_tax_report.rb | 49 +++++++++++++++++++ spec/features/admin/reports_spec.rb | 9 ++++ 5 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 lib/open_food_network/sales_tax_report.rb diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index 655d1778dc..cb07e7245e 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -6,6 +6,7 @@ require 'open_food_network/order_grouper' require 'open_food_network/customers_report' require 'open_food_network/users_and_enterprises_report' require 'open_food_network/order_cycle_management_report' +require 'open_food_network/sales_tax_report' Spree::Admin::ReportsController.class_eval do @@ -102,6 +103,36 @@ Spree::Admin::ReportsController.class_eval do send_data csv_string, :filename => "orders_and_distributors_#{timestamp}.csv" end end + + def sales_tax + params[:q] = {} unless params[:q] + + if params[:q][:completed_at_gt].blank? + params[:q][:completed_at_gt] = Time.zone.now.beginning_of_month + else + params[:q][:completed_at_gt] = Time.zone.parse(params[:q][:completed_at_gt]).beginning_of_day rescue Time.zone.now.beginning_of_month + end + + if params[:q] && !params[:q][:completed_at_lt].blank? + params[:q][:completed_at_lt] = Time.zone.parse(params[:q][:completed_at_lt]).end_of_day rescue "" + end + params[:q][:meta_sort] ||= "completed_at.desc" + + @search = Spree::Order.complete.not_state(:canceled).managed_by(spree_current_user).search(params[:q]) + orders = @search.result + @distributors = Enterprise.is_distributor.managed_by(spree_current_user) + + @report = OpenFoodNetwork::SalesTaxReport.new orders + unless params[:csv] + render :html => @report + else + csv_string = CSV.generate do |csv| + csv << @report.header + @report.table.each { |row| csv << row } + end + send_data csv_string, :filename => "sales_tax.csv" + end + end def bulk_coop params[:q] = {} unless params[:q] @@ -641,7 +672,8 @@ Spree::Admin::ReportsController.class_eval do :products_and_inventory => {:name => "Products & Inventory", :description => ''}, :sales_total => { :name => "Sales Total", :description => "Sales Total For All Orders" }, :users_and_enterprises => { :name => "Users & Enterprises", :description => "Enterprise Ownership & Status" }, - :order_cycle_management => {:name => "Order Cycle Management", :description => ''} + :order_cycle_management => {:name => "Order Cycle Management", :description => ''}, + :sales_tax => { :name => "Sales Tax", :description => "Sales Tax For Orders" } } # Return only reports the user is authorized to view. reports.select { |action| can? action, :report } diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 9bff908346..145d8310de 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -153,7 +153,7 @@ class AbilityDecorator end # Reports page - can [:admin, :index, :customers, :orders_and_distributors, :group_buys, :bulk_coop, :payments, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management], :report + can [:admin, :index, :customers, :group_buys, :bulk_coop, :sales_tax, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management], :report end diff --git a/config/routes.rb b/config/routes.rb index e7f9f2bba0..401ecbeaf3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -123,7 +123,8 @@ Spree::Core::Engine.routes.prepend do match '/admin/reports/bulk_coop' => 'admin/reports#bulk_coop', :as => "bulk_coop_admin_reports", :via => [:get, :post] match '/admin/reports/payments' => 'admin/reports#payments', :as => "payments_admin_reports", :via => [:get, :post] match '/admin/reports/orders_and_fulfillment' => 'admin/reports#orders_and_fulfillment', :as => "orders_and_fulfillment_admin_reports", :via => [:get, :post] - match '/admin/reports/users_and_enterprises' => 'admin/reports#users_and_enterprises', :as => "users_and_enterprises_admin_reports", :via => [:get, :post] + match '/admin/reports/users_and_enterprises' => 'admin/reports#users_and_enterprises', :as => "users_and_enterprises_admin_reports", :via => [:get, :post] + match '/admin/reports/sales_tax' => 'admin/reports#sales_tax', :as => "sales_tax_admin_reports", :via => [:get, :post] match '/admin/products/bulk_edit' => 'admin/products#bulk_edit', :as => "bulk_edit_admin_products" match '/admin/orders/bulk_management' => 'admin/orders#bulk_management', :as => "admin_bulk_order_management" match '/admin/reports/products_and_inventory' => 'admin/reports#products_and_inventory', :as => "products_and_inventory_admin_reports", :via => [:get, :post] diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb new file mode 100644 index 0000000000..b3aa8b5419 --- /dev/null +++ b/lib/open_food_network/sales_tax_report.rb @@ -0,0 +1,49 @@ + +module OpenFoodNetwork + class SalesTaxReport + + def initialize orders + @orders = orders + end + + def header + ["Order number", "Date", "Items", "Items total", "Taxable Items Total", "Sales Tax", + "Delivery Charge", "Tax on Delivery", "Total Tax", "Customer", "Distributor"] + end + + def table + sales_tax_details = [] + @orders.each do |order| + totals = {"items" => 0, "items_total" => 0.0, "taxable_total" => 0.0, "sales_tax" => 0.0} + + order.line_items.each do |line_item| + totals["items"] += line_item.quantity + totals["items_total"] += line_item.price * line_item.quantity + + tax_rate = Spree::TaxRate.find_by_tax_category_id(line_item.variant.product.tax_category_id).andand.amount + + if tax_rate != nil && tax_rate != 0 + totals["taxable_total"] += (line_item.price * line_item.quantity) + totals["sales_tax"] += (line_item.price * line_item.quantity) * tax_rate + end + end + + shipping_cost = order.adjustments.find_by_label("Shipping").andand.amount + shipping_cost = (shipping_cost == nil) ? 0.0 : shipping_cost + shipping_tax = (Spree::Config[:shipment_inc_vat] && shipping_cost != nil) ? shipping_cost * 0.2 : 0.0 + + #config option for charging tax on shipping fee or not? exists, need to set rate... + #calculate additional tax for shipping... + #ignore non-shipping adjustments? any potential issues? + #show payment status? other necessary/useful info? + #check which orders are pulled, and which are filtered out... maybe have a dropdown to make it explicit...? + + sales_tax_details << [order.number, order.created_at, totals["items"], totals["items_total"], + totals["taxable_total"], totals["sales_tax"], shipping_cost, shipping_tax, totals["sales_tax"] + shipping_tax, + order.bill_address.full_name, order.distributor.andand.name] + + end + sales_tax_details + end + end +end diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 0524a6769e..056b9b44b8 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -98,6 +98,15 @@ feature %q{ page.should have_content 'Payment State' end + + scenario "Sales Tax report" do + login_to_admin_section + click_link 'Reports' + click_link 'Sales Tax' + + page.should have_content 'Total Tax' + page.should have_select 'distributor_id' + end describe "orders & fulfilment reports" do it "loads the report page" do From 371f966f63dc1259932d98d64ec5db2063fdd4fa Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Sun, 23 Nov 2014 19:55:03 +0000 Subject: [PATCH 03/35] sales tax view --- .../spree/admin/reports/sales_tax.html.haml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 app/views/spree/admin/reports/sales_tax.html.haml diff --git a/app/views/spree/admin/reports/sales_tax.html.haml b/app/views/spree/admin/reports/sales_tax.html.haml new file mode 100644 index 0000000000..805f0ae627 --- /dev/null +++ b/app/views/spree/admin/reports/sales_tax.html.haml @@ -0,0 +1,40 @@ += form_for @search, :url => spree.sales_tax_admin_reports_path do |s| + = label_tag nil, t(:date_range) + %br + %br + .date-range-filter + = label_tag nil, t(:start), :class => 'inline' + = s.text_field :completed_at_gt, :class => 'datepicker datepicker-from' + %span.range-divider + %i.icon-arrow-right + = s.text_field :completed_at_lt, :class => 'datepicker datepicker-to' + = label_tag nil, t(:stop), :class => 'inline' + %br + %div{"class" => "row"} + %div{"class" => "four columns alpha"} + = label_tag nil, "Distributor: " + = s.collection_select(:distributor_id_eq, @distributors, :id, :name, {:include_blank => 'All'}, {:class => "select2 fullwidth"}) + = check_box_tag :csv + = label_tag :csv, "Download as csv" + %br + = button t(:search) +%br +%br +%table#listing_orders.index + %thead + %tr{'data-hook' => "orders_header"} + - @report.header.each do |heading| + %th=heading + %tbody + - @report.table.each do |row| + %tr + - row.each_with_index do |column, i| + -if i==0 + %td + %a{:class => "edit-order", 'href' => "/admin/orders/#{column}"} #{column} + -else + %td= column + - if @report.table.empty? + %tr + %td{:colspan => "2"}= t(:none) + From 623882a2a1aeeb0a32a3a4addfb45ec8e324732e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Mon, 24 Nov 2014 03:01:00 +0000 Subject: [PATCH 04/35] Config option for tax rate on shipping --- app/models/spree/app_configuration_decorator.rb | 15 ++++++++------- .../edit/shipping_tax_rate.html.haml.deface | 5 +++++ lib/open_food_network/sales_tax_report.rb | 12 +++++------- 3 files changed, 18 insertions(+), 14 deletions(-) create mode 100644 app/overrides/spree/admin/tax_settings/edit/shipping_tax_rate.html.haml.deface diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb index f4f4acc5ec..5ad4a9a1c5 100644 --- a/app/models/spree/app_configuration_decorator.rb +++ b/app/models/spree/app_configuration_decorator.rb @@ -1,9 +1,10 @@ Spree::AppConfiguration.class_eval do - # This file decorates the existing preferences file defined by Spree. - # It allows us to add our own global configuration variables, which - # we can allow to be modified in the UI by adding appropriate form - # elements to existing or new configuration pages. + # This file decorates the existing preferences file defined by Spree. + # It allows us to add our own global configuration variables, which + # we can allow to be modified in the UI by adding appropriate form + # elements to existing or new configuration pages. - # Tax Preferences - preference :products_require_tax_category, :boolean, default: false -end \ No newline at end of file + # Tax Preferences + preference :products_require_tax_category, :boolean, default: false + preference :shipping_tax_rate, :decimal, default: 0 +end diff --git a/app/overrides/spree/admin/tax_settings/edit/shipping_tax_rate.html.haml.deface b/app/overrides/spree/admin/tax_settings/edit/shipping_tax_rate.html.haml.deface new file mode 100644 index 0000000000..222e189d87 --- /dev/null +++ b/app/overrides/spree/admin/tax_settings/edit/shipping_tax_rate.html.haml.deface @@ -0,0 +1,5 @@ +/ insert_after "[data-hook='shipment_vat']" + +%div.field.align-center{ "data-hook" => "shipping_tax_rate" } + = number_field_tag "preferences[shipping_tax_rate]", Spree::Config[:shipping_tax_rate].to_f, in: 0.0..1.0, step: 0.01 + = label_tag nil, t(:shipping_tax_rate) \ No newline at end of file diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index b3aa8b5419..6d1945e1f5 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -30,13 +30,11 @@ module OpenFoodNetwork shipping_cost = order.adjustments.find_by_label("Shipping").andand.amount shipping_cost = (shipping_cost == nil) ? 0.0 : shipping_cost - shipping_tax = (Spree::Config[:shipment_inc_vat] && shipping_cost != nil) ? shipping_cost * 0.2 : 0.0 - - #config option for charging tax on shipping fee or not? exists, need to set rate... - #calculate additional tax for shipping... - #ignore non-shipping adjustments? any potential issues? - #show payment status? other necessary/useful info? - #check which orders are pulled, and which are filtered out... maybe have a dropdown to make it explicit...? + if Spree::Config[:shipment_inc_vat] && shipping_cost != nil + shipping_tax = shipping_cost * Spree::Config[:shipping_tax_rate] + else + shipping_tax = 0.0 + end sales_tax_details << [order.number, order.created_at, totals["items"], totals["items_total"], totals["taxable_total"], totals["sales_tax"], shipping_cost, shipping_tax, totals["sales_tax"] + shipping_tax, From bbca674937b2d332049e4e250890971576da004e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Mon, 24 Nov 2014 12:23:48 +0000 Subject: [PATCH 05/35] Update reports_spec.rb --- spec/features/admin/reports_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 056b9b44b8..7899858d2f 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -105,7 +105,6 @@ feature %q{ click_link 'Sales Tax' page.should have_content 'Total Tax' - page.should have_select 'distributor_id' end describe "orders & fulfilment reports" do From 0f3723a923a951ff30390254026c06209271dadf Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Mon, 24 Nov 2014 23:34:02 +0000 Subject: [PATCH 06/35] Added currency symbols to sales tax report --- app/models/spree/money_decorator.rb | 7 +++++++ lib/open_food_network/sales_tax_report.rb | 6 ++++-- 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 app/models/spree/money_decorator.rb diff --git a/app/models/spree/money_decorator.rb b/app/models/spree/money_decorator.rb new file mode 100644 index 0000000000..e179343bc5 --- /dev/null +++ b/app/models/spree/money_decorator.rb @@ -0,0 +1,7 @@ +Spree::Money.class_eval do + + # return the currency symbol (on it's own) for the current default currency + def self.currency_symbol + Money.new(0, Spree::Config[:currency]).symbol + end +end diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index 6d1945e1f5..76b2534a66 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -7,8 +7,10 @@ module OpenFoodNetwork end def header - ["Order number", "Date", "Items", "Items total", "Taxable Items Total", "Sales Tax", - "Delivery Charge", "Tax on Delivery", "Total Tax", "Customer", "Distributor"] + currency_symbol = Spree::Money.currency_symbol + ["Order number", "Date", "Items", "Items total (#{currency_symbol})", "Taxable Items Total (#{currency_symbol})", + "Sales Tax", "Delivery Charge (#{currency_symbol})", "Tax on Delivery (#{currency_symbol})", + "Total Tax (#{currency_symbol})", "Customer", "Distributor"] end def table From 3f61a5412c74211732aa2be033b48b9eccbe6090 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Thu, 27 Nov 2014 12:45:53 +0000 Subject: [PATCH 07/35] Spec file attempt... --- spec/features/admin/reports_spec.rb | 75 +++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 5 deletions(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 7899858d2f..2d76405e87 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -99,12 +99,77 @@ feature %q{ page.should have_content 'Payment State' end - scenario "Sales Tax report" do - login_to_admin_section - click_link 'Reports' - click_link 'Sales Tax' + describe "Sales Tax report" do + include AuthenticationWorkflow + include WebHelper + let(:user1) do + create_enterprise_user(enterprises: [ + create(:distributor_enterprise) + ]) + end + let(:user2) do + create_enterprise_user(enterprises: [ + create(:distributor_enterprise) + ]) + end + let(:tax_category1) { create(:tax_category) } + let(:tax_category2) { create(:tax_category) } + let(:country) { create(:country, :name => "Spec Republic") } + let(:state) { create(:state, :country => country, :name => "Specville") } + #let(:zone) { create(:zone, :name => "Test Tax Zone") } + #let(:zoneable) { create(:zoneable, :zoneable => country) } + let(:tax_rate1) { create(:tax_rate, :amount => 0.0, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category1, :zone => zone) } + let(:tax_rate2) { create(:tax_rate, :amount => 0.2, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category2, :zone => zone) } + + let(:product1) { create(:product, :tax_category => tax_category1) } + let(:product2) { create(:product, :tax_category => tax_category2) } + let(:variant1) { create(:variant, price: 12.54, :product => product1) } + let(:variant2) { create(:variant, price: 500.15, :product => product2) } + + let(:product_distribution1) { create(:product_distribution, :product => variant1.product, :distributor => user1.enterprises.first) } + let(:product_distribution2) { create(:product_distribution, :product => variant2.product, :distributor => user2.enterprises.first) } + let(:address) { create(:address) } # + let(:shipping_method) { create(:shipping_method, name: "Shipping", description: "Expensive", calculator: Spree::Calculator::FlatRate.new(preferred_amount: 100.55)) } + let(:order1) { create(:order, :distributor => user1.enterprises.first, :shipping_method => shipping_method, bill_address: address, ship_address: address) } + let(:line_item1) { create(:line_item, :variant => variant1, :price => 12.54, :quantity => 1, :order => order1) } + let(:line_item2) { create(:line_item, :variant => variant2, :price => 500.15, :quantity => 3, :order => order1) } + let(:adjustment) { create(:adjustment, :adjustable => order1, :label => "Shipping", :amount => 100.55) } + + before do + Spree::Config.shipment_inc_vat = true + Spree::Config.shipping_tax_rate = 0.2 + order1.finalize! + login_to_admin_as user1 + click_link "Reports" + click_link "Sales Tax" + end + + it "displays the report" do + page.should have_content "#{user1.enterprises.first.name}" #listed in distributors dropdown + page.should_not have_content "#{user2.enterprises.first.name}" #not listed + + select user1.enterprises.first.name, from: 'q_distributor_id_eq' + click_button 'Search' + + page.should have_content "#{order1.number}" + end - page.should have_content 'Total Tax' + it "calculates sales tax on orders" do + select user1.enterprises.first.name, from: 'q_distributor_id_eq' + click_button 'Search' + + page.should have_content "1512.99" #items total + page.should have_content "1500.45" #taxable items total + page.should have_content "300.09" #sales tax + end + + it "calculates shipping tax on orders" do + select user1.enterprises.first.name, from: 'q_distributor_id_eq' + click_button 'Search' + + page.should have_content "100.55" #shipping cost + page.should have_content "20.11" #shipping tax + end end describe "orders & fulfilment reports" do From d194e74edaafe47b882888e50c93bb7404dd98cf Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Thu, 27 Nov 2014 13:38:52 +0000 Subject: [PATCH 08/35] Rohan's suggested changes --- .../admin/reports_controller_decorator.rb | 2 +- .../spree/admin/reports/sales_tax.html.haml | 2 +- lib/open_food_network/sales_tax_report.rb | 18 +++++++++--------- spec/models/spree/ability_spec.rb | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index cb07e7245e..931e8bf9ff 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -76,7 +76,7 @@ Spree::Admin::ReportsController.class_eval do end def orders_and_distributors - params[:q] = {} unless params[:q] + params[:q] ||= {} if params[:q][:completed_at_gt].blank? params[:q][:completed_at_gt] = Time.zone.now.beginning_of_month diff --git a/app/views/spree/admin/reports/sales_tax.html.haml b/app/views/spree/admin/reports/sales_tax.html.haml index 805f0ae627..9231438588 100644 --- a/app/views/spree/admin/reports/sales_tax.html.haml +++ b/app/views/spree/admin/reports/sales_tax.html.haml @@ -36,5 +36,5 @@ %td= column - if @report.table.empty? %tr - %td{:colspan => "2"}= t(:none) + %td{:colspan => "12"}= t(:none) diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index 76b2534a66..cb604ce702 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -9,37 +9,37 @@ module OpenFoodNetwork def header currency_symbol = Spree::Money.currency_symbol ["Order number", "Date", "Items", "Items total (#{currency_symbol})", "Taxable Items Total (#{currency_symbol})", - "Sales Tax", "Delivery Charge (#{currency_symbol})", "Tax on Delivery (#{currency_symbol})", + "Sales Tax (#{currency_symbol})", "Delivery Charge (#{currency_symbol})", "Tax on Delivery (#{currency_symbol})", "Total Tax (#{currency_symbol})", "Customer", "Distributor"] end def table sales_tax_details = [] @orders.each do |order| - totals = {"items" => 0, "items_total" => 0.0, "taxable_total" => 0.0, "sales_tax" => 0.0} + totals = {items: 0, items_total: 0.0, taxable_total: 0.0, sales_tax: 0.0} order.line_items.each do |line_item| - totals["items"] += line_item.quantity - totals["items_total"] += line_item.price * line_item.quantity + totals[:items] += line_item.quantity + totals[:items_total] += line_item.amount tax_rate = Spree::TaxRate.find_by_tax_category_id(line_item.variant.product.tax_category_id).andand.amount if tax_rate != nil && tax_rate != 0 - totals["taxable_total"] += (line_item.price * line_item.quantity) - totals["sales_tax"] += (line_item.price * line_item.quantity) * tax_rate + totals[:taxable_total] += line_item.amount + totals[:sales_tax] += line_item.amount * tax_rate end end shipping_cost = order.adjustments.find_by_label("Shipping").andand.amount - shipping_cost = (shipping_cost == nil) ? 0.0 : shipping_cost + shipping_cost = shipping_cost.nil? ? 0.0 : shipping_cost if Spree::Config[:shipment_inc_vat] && shipping_cost != nil shipping_tax = shipping_cost * Spree::Config[:shipping_tax_rate] else shipping_tax = 0.0 end - sales_tax_details << [order.number, order.created_at, totals["items"], totals["items_total"], - totals["taxable_total"], totals["sales_tax"], shipping_cost, shipping_tax, totals["sales_tax"] + shipping_tax, + sales_tax_details << [order.number, order.created_at, totals[:items], totals[:items_total], + totals[:taxable_total], totals[:sales_tax], shipping_cost, shipping_tax, totals[:sales_tax] + shipping_tax, order.bill_address.full_name, order.distributor.andand.name] end diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 30fbbd8ab6..19006cbcd1 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -350,7 +350,7 @@ module Spree end it "should be able to read some reports" do - should have_ability([:admin, :index, :customers, :group_buys, :bulk_coop, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory], for: :report) + should have_ability([:admin, :index, :customers, :sales_tax, :group_buys, :bulk_coop, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory], for: :report) end it "should not be able to read other reports" do From 2c80be7e9e84b4b4c678adc395e97b8d649215b6 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Thu, 27 Nov 2014 14:11:21 +0000 Subject: [PATCH 09/35] Update --- spec/features/admin/reports_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 2d76405e87..ec9f756c93 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -116,7 +116,7 @@ feature %q{ let(:tax_category2) { create(:tax_category) } let(:country) { create(:country, :name => "Spec Republic") } let(:state) { create(:state, :country => country, :name => "Specville") } - #let(:zone) { create(:zone, :name => "Test Tax Zone") } + let(:zone) { create(:zone, :name => "Test Tax Zone") } #let(:zoneable) { create(:zoneable, :zoneable => country) } let(:tax_rate1) { create(:tax_rate, :amount => 0.0, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category1, :zone => zone) } let(:tax_rate2) { create(:tax_rate, :amount => 0.2, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category2, :zone => zone) } @@ -128,9 +128,9 @@ feature %q{ let(:product_distribution1) { create(:product_distribution, :product => variant1.product, :distributor => user1.enterprises.first) } let(:product_distribution2) { create(:product_distribution, :product => variant2.product, :distributor => user2.enterprises.first) } - let(:address) { create(:address) } # - let(:shipping_method) { create(:shipping_method, name: "Shipping", description: "Expensive", calculator: Spree::Calculator::FlatRate.new(preferred_amount: 100.55)) } - let(:order1) { create(:order, :distributor => user1.enterprises.first, :shipping_method => shipping_method, bill_address: address, ship_address: address) } + let(:address) { create(:address, :state => state, :country => country) } # + let(:shipping_method) { create(:shipping_method, :name => "Shipping", :description => "Expensive", :calculator => Spree::Calculator::FlatRate.new(preferred_amount: 100.55)) } + let(:order1) { create(:order, :distributor => user1.enterprises.first, :shipping_method => shipping_method, :bill_address => address, :ship_address => address) } let(:line_item1) { create(:line_item, :variant => variant1, :price => 12.54, :quantity => 1, :order => order1) } let(:line_item2) { create(:line_item, :variant => variant2, :price => 500.15, :quantity => 3, :order => order1) } let(:adjustment) { create(:adjustment, :adjustable => order1, :label => "Shipping", :amount => 100.55) } From e6368af75799aad9a8e040ff4af187462b8a8312 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Thu, 27 Nov 2014 14:15:13 +0000 Subject: [PATCH 10/35] Update reports_spec.rb --- spec/features/admin/reports_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index ec9f756c93..9f52b5fcc9 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -100,8 +100,6 @@ feature %q{ end describe "Sales Tax report" do - include AuthenticationWorkflow - include WebHelper let(:user1) do create_enterprise_user(enterprises: [ create(:distributor_enterprise) From 29653a5595a5c638f53a1978cd8c9578606737ab Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 14:43:24 +1100 Subject: [PATCH 11/35] Fix spec infinite recursion issue --- spec/features/admin/reports_spec.rb | 34 ++++++++++++----------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 9f52b5fcc9..92bffefebd 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -99,44 +99,38 @@ feature %q{ page.should have_content 'Payment State' end - describe "Sales Tax report" do + describe "Sales Tax report" do let(:user1) do - create_enterprise_user(enterprises: [ - create(:distributor_enterprise) - ]) + create_enterprise_user(enterprises: [create(:distributor_enterprise)]) end let(:user2) do - create_enterprise_user(enterprises: [ - create(:distributor_enterprise) - ]) + create_enterprise_user(enterprises: [create(:distributor_enterprise)]) end let(:tax_category1) { create(:tax_category) } let(:tax_category2) { create(:tax_category) } let(:country) { create(:country, :name => "Spec Republic") } let(:state) { create(:state, :country => country, :name => "Specville") } let(:zone) { create(:zone, :name => "Test Tax Zone") } - #let(:zoneable) { create(:zoneable, :zoneable => country) } - let(:tax_rate1) { create(:tax_rate, :amount => 0.0, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category1, :zone => zone) } - let(:tax_rate2) { create(:tax_rate, :amount => 0.2, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category2, :zone => zone) } - + let!(:tax_rate1) { create(:tax_rate, :amount => 0.0, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category1, :zone => zone) } + let!(:tax_rate2) { create(:tax_rate, :amount => 0.2, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category2, :zone => zone) } + let(:product1) { create(:product, :tax_category => tax_category1) } let(:product2) { create(:product, :tax_category => tax_category2) } let(:variant1) { create(:variant, price: 12.54, :product => product1) } let(:variant2) { create(:variant, price: 500.15, :product => product2) } - - let(:product_distribution1) { create(:product_distribution, :product => variant1.product, :distributor => user1.enterprises.first) } - let(:product_distribution2) { create(:product_distribution, :product => variant2.product, :distributor => user2.enterprises.first) } - let(:address) { create(:address, :state => state, :country => country) } # + + let(:address) { create(:address, :state => state, :country => country) } let(:shipping_method) { create(:shipping_method, :name => "Shipping", :description => "Expensive", :calculator => Spree::Calculator::FlatRate.new(preferred_amount: 100.55)) } - let(:order1) { create(:order, :distributor => user1.enterprises.first, :shipping_method => shipping_method, :bill_address => address, :ship_address => address) } - let(:line_item1) { create(:line_item, :variant => variant1, :price => 12.54, :quantity => 1, :order => order1) } - let(:line_item2) { create(:line_item, :variant => variant2, :price => 500.15, :quantity => 3, :order => order1) } - let(:adjustment) { create(:adjustment, :adjustable => order1, :label => "Shipping", :amount => 100.55) } - + let(:order1) { create(:order, :distributor => user1.enterprises.first, :shipping_method => shipping_method, :bill_address => address) } + let!(:line_item1) { create(:line_item, :variant => variant1, :price => 12.54, :quantity => 1, :order => order1) } + let!(:line_item2) { create(:line_item, :variant => variant2, :price => 500.15, :quantity => 3, :order => order1) } + let!(:adjustment) { create(:adjustment, :adjustable => order1, :label => "Shipping", :amount => 100.55) } + before do Spree::Config.shipment_inc_vat = true Spree::Config.shipping_tax_rate = 0.2 order1.finalize! + login_to_admin_as user1 click_link "Reports" click_link "Sales Tax" From 0212351d32e26b0c8bd86210a993c140cb1cb599 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 14:48:06 +1100 Subject: [PATCH 12/35] Tighten test --- spec/features/admin/reports_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 92bffefebd..78f3b238b8 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -137,9 +137,9 @@ feature %q{ end it "displays the report" do - page.should have_content "#{user1.enterprises.first.name}" #listed in distributors dropdown - page.should_not have_content "#{user2.enterprises.first.name}" #not listed - + page.should have_select 'q_distributor_id_eq', with_options: [user1.enterprises.first.name] + page.should_not have_select 'q_distributor_id_eq', with_options: [user2.enterprises.first.name] + select user1.enterprises.first.name, from: 'q_distributor_id_eq' click_button 'Search' From 3aa30af199279b0c86015bad9991c849f3da6d20 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 14:55:19 +1100 Subject: [PATCH 13/35] Remove unneeded objects from spec setup --- spec/features/admin/reports_spec.rb | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 78f3b238b8..cc87a98c9f 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -108,22 +108,16 @@ feature %q{ end let(:tax_category1) { create(:tax_category) } let(:tax_category2) { create(:tax_category) } - let(:country) { create(:country, :name => "Spec Republic") } - let(:state) { create(:state, :country => country, :name => "Specville") } - let(:zone) { create(:zone, :name => "Test Tax Zone") } - let!(:tax_rate1) { create(:tax_rate, :amount => 0.0, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category1, :zone => zone) } - let!(:tax_rate2) { create(:tax_rate, :amount => 0.2, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category2, :zone => zone) } + let!(:tax_rate1) { create(:tax_rate, :amount => 0.0, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category1) } + let!(:tax_rate2) { create(:tax_rate, :amount => 0.2, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category2) } - let(:product1) { create(:product, :tax_category => tax_category1) } - let(:product2) { create(:product, :tax_category => tax_category2) } - let(:variant1) { create(:variant, price: 12.54, :product => product1) } - let(:variant2) { create(:variant, price: 500.15, :product => product2) } + let(:product1) { create(:product, price: 12.54, :tax_category => tax_category1) } + let(:product2) { create(:product, price: 500.15, :tax_category => tax_category2) } - let(:address) { create(:address, :state => state, :country => country) } let(:shipping_method) { create(:shipping_method, :name => "Shipping", :description => "Expensive", :calculator => Spree::Calculator::FlatRate.new(preferred_amount: 100.55)) } - let(:order1) { create(:order, :distributor => user1.enterprises.first, :shipping_method => shipping_method, :bill_address => address) } - let!(:line_item1) { create(:line_item, :variant => variant1, :price => 12.54, :quantity => 1, :order => order1) } - let!(:line_item2) { create(:line_item, :variant => variant2, :price => 500.15, :quantity => 3, :order => order1) } + let(:order1) { create(:order, :distributor => user1.enterprises.first, :shipping_method => shipping_method, :bill_address => create(:address)) } + let!(:line_item1) { create(:line_item, :variant => product1.master, :price => 12.54, :quantity => 1, :order => order1) } + let!(:line_item2) { create(:line_item, :variant => product2.master, :price => 500.15, :quantity => 3, :order => order1) } let!(:adjustment) { create(:adjustment, :adjustable => order1, :label => "Shipping", :amount => 100.55) } before do From 2b9fef6aec20d5378f15099d9364160027c6fbdb Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 14:57:39 +1100 Subject: [PATCH 14/35] Convert to Ruby 1.9 hash syntax --- spec/features/admin/reports_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index cc87a98c9f..4e2e372225 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -108,17 +108,17 @@ feature %q{ end let(:tax_category1) { create(:tax_category) } let(:tax_category2) { create(:tax_category) } - let!(:tax_rate1) { create(:tax_rate, :amount => 0.0, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category1) } - let!(:tax_rate2) { create(:tax_rate, :amount => 0.2, :calculator => Spree::Calculator::DefaultTax.new, :tax_category => tax_category2) } + let!(:tax_rate1) { create(:tax_rate, amount: 0.0, calculator: Spree::Calculator::DefaultTax.new, tax_category: tax_category1) } + let!(:tax_rate2) { create(:tax_rate, amount: 0.2, calculator: Spree::Calculator::DefaultTax.new, tax_category: tax_category2) } - let(:product1) { create(:product, price: 12.54, :tax_category => tax_category1) } - let(:product2) { create(:product, price: 500.15, :tax_category => tax_category2) } + let(:product1) { create(:product, price: 12.54, tax_category: tax_category1) } + let(:product2) { create(:product, price: 500.15, tax_category: tax_category2) } - let(:shipping_method) { create(:shipping_method, :name => "Shipping", :description => "Expensive", :calculator => Spree::Calculator::FlatRate.new(preferred_amount: 100.55)) } - let(:order1) { create(:order, :distributor => user1.enterprises.first, :shipping_method => shipping_method, :bill_address => create(:address)) } - let!(:line_item1) { create(:line_item, :variant => product1.master, :price => 12.54, :quantity => 1, :order => order1) } - let!(:line_item2) { create(:line_item, :variant => product2.master, :price => 500.15, :quantity => 3, :order => order1) } - let!(:adjustment) { create(:adjustment, :adjustable => order1, :label => "Shipping", :amount => 100.55) } + let(:shipping_method) { create(:shipping_method, name: "Shipping", description: "Expensive", calculator: Spree::Calculator::FlatRate.new(preferred_amount: 100.55)) } + let(:order1) { create(:order, distributor: user1.enterprises.first, shipping_method: shipping_method, bill_address: create(:address)) } + let!(:line_item1) { create(:line_item, variant: product1.master, price: 12.54, quantity: 1, order: order1) } + let!(:line_item2) { create(:line_item, variant: product2.master, price: 500.15, quantity: 3, order: order1) } + let!(:adjustment) { create(:adjustment, adjustable: order1, label: "Shipping", amount: 100.55) } before do Spree::Config.shipment_inc_vat = true From 0b636c1d89fa0db25ca32ad508901477b76cd29e Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 15:04:48 +1100 Subject: [PATCH 15/35] Combine spec cases for faster runtime --- spec/features/admin/reports_spec.rb | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 4e2e372225..93b0495ef7 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -130,29 +130,24 @@ feature %q{ click_link "Sales Tax" end - it "displays the report" do + it "reports" do + # Then it should give me access only to managed enterprises page.should have_select 'q_distributor_id_eq', with_options: [user1.enterprises.first.name] page.should_not have_select 'q_distributor_id_eq', with_options: [user2.enterprises.first.name] + # When I filter to just one distributor select user1.enterprises.first.name, from: 'q_distributor_id_eq' click_button 'Search' - + + # Then I should see the relevant order page.should have_content "#{order1.number}" - end - it "calculates sales tax on orders" do - select user1.enterprises.first.name, from: 'q_distributor_id_eq' - click_button 'Search' - - page.should have_content "1512.99" #items total - page.should have_content "1500.45" #taxable items total - page.should have_content "300.09" #sales tax - end + # And the totals and sales tax should be correct + page.should have_content "1512.99" # items total + page.should have_content "1500.45" # taxable items total + page.should have_content "300.09" # sales tax - it "calculates shipping tax on orders" do - select user1.enterprises.first.name, from: 'q_distributor_id_eq' - click_button 'Search' - + # And the shipping cost and tax should be correct page.should have_content "100.55" #shipping cost page.should have_content "20.11" #shipping tax end From cc7d6cde1d989f4c6109f3908519b7330209fe14 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 12:35:37 +1100 Subject: [PATCH 16/35] Shorter syntax --- app/controllers/spree/admin/reports_controller_decorator.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index 931e8bf9ff..68048ee870 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -105,7 +105,7 @@ Spree::Admin::ReportsController.class_eval do end def sales_tax - params[:q] = {} unless params[:q] + params[:q] ||= {} if params[:q][:completed_at_gt].blank? params[:q][:completed_at_gt] = Time.zone.now.beginning_of_month @@ -135,7 +135,7 @@ Spree::Admin::ReportsController.class_eval do end def bulk_coop - params[:q] = {} unless params[:q] + params[:q] ||= {} if params[:q][:completed_at_gt].blank? params[:q][:completed_at_gt] = Time.zone.now.beginning_of_month @@ -288,7 +288,7 @@ Spree::Admin::ReportsController.class_eval do end def payments - params[:q] = {} unless params[:q] + params[:q] ||= {} if params[:q][:completed_at_gt].blank? params[:q][:completed_at_gt] = Time.zone.now.beginning_of_month From 67c77cea81e2771f15da17663a880a7396236f89 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 12:41:42 +1100 Subject: [PATCH 17/35] Tidy up haml --- .../edit/shipping_tax_rate.html.haml.deface | 2 +- .../spree/admin/reports/sales_tax.html.haml | 34 +++++++------------ 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/app/overrides/spree/admin/tax_settings/edit/shipping_tax_rate.html.haml.deface b/app/overrides/spree/admin/tax_settings/edit/shipping_tax_rate.html.haml.deface index 222e189d87..b378ba84a6 100644 --- a/app/overrides/spree/admin/tax_settings/edit/shipping_tax_rate.html.haml.deface +++ b/app/overrides/spree/admin/tax_settings/edit/shipping_tax_rate.html.haml.deface @@ -1,5 +1,5 @@ / insert_after "[data-hook='shipment_vat']" -%div.field.align-center{ "data-hook" => "shipping_tax_rate" } +.field.align-center{ "data-hook" => "shipping_tax_rate" } = number_field_tag "preferences[shipping_tax_rate]", Spree::Config[:shipping_tax_rate].to_f, in: 0.0..1.0, step: 0.01 = label_tag nil, t(:shipping_tax_rate) \ No newline at end of file diff --git a/app/views/spree/admin/reports/sales_tax.html.haml b/app/views/spree/admin/reports/sales_tax.html.haml index 9231438588..a7b3d9275d 100644 --- a/app/views/spree/admin/reports/sales_tax.html.haml +++ b/app/views/spree/admin/reports/sales_tax.html.haml @@ -1,40 +1,32 @@ -= form_for @search, :url => spree.sales_tax_admin_reports_path do |s| - = label_tag nil, t(:date_range) - %br - %br - .date-range-filter - = label_tag nil, t(:start), :class => 'inline' - = s.text_field :completed_at_gt, :class => 'datepicker datepicker-from' - %span.range-divider - %i.icon-arrow-right - = s.text_field :completed_at_lt, :class => 'datepicker datepicker-to' - = label_tag nil, t(:stop), :class => 'inline' - %br - %div{"class" => "row"} - %div{"class" => "four columns alpha"} - = label_tag nil, "Distributor: " - = s.collection_select(:distributor_id_eq, @distributors, :id, :name, {:include_blank => 'All'}, {:class => "select2 fullwidth"}) += form_for @search, :url => spree.sales_tax_admin_reports_path do |f| + = render 'date_range_form', f: f + + .row + .four.columns.alpha + = label_tag nil, "Distributor:" + = f.collection_select(:distributor_id_eq, @distributors, :id, :name, {:include_blank => 'All'}, {:class => "select2 fullwidth"}) = check_box_tag :csv = label_tag :csv, "Download as csv" %br = button t(:search) + %br %br %table#listing_orders.index %thead %tr{'data-hook' => "orders_header"} - @report.header.each do |heading| - %th=heading + %th= heading %tbody - @report.table.each do |row| %tr - row.each_with_index do |column, i| - -if i==0 + - if i == 0 %td - %a{:class => "edit-order", 'href' => "/admin/orders/#{column}"} #{column} - -else + %a.edit-order{'href' => "/admin/orders/#{column}"}= column + - else %td= column - if @report.table.empty? %tr - %td{:colspan => "12"}= t(:none) + %td{:colspan => @report.header.count}= t(:none) From 89199ef30af849098afd2a28c5dc5a552d645ac8 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 12:46:41 +1100 Subject: [PATCH 18/35] Use map instead of each and var --- lib/open_food_network/sales_tax_report.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index cb604ce702..75a1764551 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -14,8 +14,7 @@ module OpenFoodNetwork end def table - sales_tax_details = [] - @orders.each do |order| + @orders.map do |order| totals = {items: 0, items_total: 0.0, taxable_total: 0.0, sales_tax: 0.0} order.line_items.each do |line_item| @@ -38,12 +37,10 @@ module OpenFoodNetwork shipping_tax = 0.0 end - sales_tax_details << [order.number, order.created_at, totals[:items], totals[:items_total], - totals[:taxable_total], totals[:sales_tax], shipping_cost, shipping_tax, totals[:sales_tax] + shipping_tax, - order.bill_address.full_name, order.distributor.andand.name] - + [order.number, order.created_at, totals[:items], totals[:items_total], + totals[:taxable_total], totals[:sales_tax], shipping_cost, shipping_tax, totals[:sales_tax] + shipping_tax, + order.bill_address.full_name, order.distributor.andand.name] end - sales_tax_details end end end From 3b4d73760b5cc49c82b774dc8bf4c3336bae9ed5 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 14:12:18 +1100 Subject: [PATCH 19/35] Break up sales tax report into methods --- lib/open_food_network/sales_tax_report.rb | 62 +++++++++++++++-------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index 75a1764551..86a46187be 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -15,32 +15,52 @@ module OpenFoodNetwork def table @orders.map do |order| - totals = {items: 0, items_total: 0.0, taxable_total: 0.0, sales_tax: 0.0} - - order.line_items.each do |line_item| - totals[:items] += line_item.quantity - totals[:items_total] += line_item.amount - - tax_rate = Spree::TaxRate.find_by_tax_category_id(line_item.variant.product.tax_category_id).andand.amount - - if tax_rate != nil && tax_rate != 0 - totals[:taxable_total] += line_item.amount - totals[:sales_tax] += line_item.amount * tax_rate - end - end - - shipping_cost = order.adjustments.find_by_label("Shipping").andand.amount - shipping_cost = shipping_cost.nil? ? 0.0 : shipping_cost - if Spree::Config[:shipment_inc_vat] && shipping_cost != nil - shipping_tax = shipping_cost * Spree::Config[:shipping_tax_rate] - else - shipping_tax = 0.0 - end + totals = totals_of order.line_items + shipping_cost = shipping_cost_for order + shipping_tax = shipping_tax_on shipping_cost [order.number, order.created_at, totals[:items], totals[:items_total], totals[:taxable_total], totals[:sales_tax], shipping_cost, shipping_tax, totals[:sales_tax] + shipping_tax, order.bill_address.full_name, order.distributor.andand.name] end end + + + private + + def totals_of(line_items) + totals = {items: 0, items_total: 0.0, taxable_total: 0.0, sales_tax: 0.0} + + line_items.each do |line_item| + totals[:items] += line_item.quantity + totals[:items_total] += line_item.amount + + tax_rate = tax_rate_on line_item + + if tax_rate != nil && tax_rate != 0 + totals[:taxable_total] += line_item.amount + totals[:sales_tax] += line_item.amount * tax_rate + end + end + + totals + end + + def shipping_cost_for(order) + shipping_cost = order.adjustments.find_by_label("Shipping").andand.amount + shipping_cost = shipping_cost.nil? ? 0.0 : shipping_cost + end + + def shipping_tax_on(shipping_cost) + if Spree::Config[:shipment_inc_vat] && shipping_cost != nil + shipping_cost * Spree::Config[:shipping_tax_rate] + else + 0.0 + end + end + + def tax_rate_on(line_item) + Spree::TaxRate.find_by_tax_category_id(line_item.variant.product.tax_category_id).andand.amount + end end end From 10e5d09416fd1f0625b58146334cc54128dfb339 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 8 Jan 2015 14:15:57 +1100 Subject: [PATCH 20/35] Use ReportsHelper for currency_symbol --- lib/open_food_network/sales_tax_report.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index 86a46187be..2579f91e59 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -1,13 +1,12 @@ - module OpenFoodNetwork class SalesTaxReport + include Spree::ReportsHelper def initialize orders @orders = orders end def header - currency_symbol = Spree::Money.currency_symbol ["Order number", "Date", "Items", "Items total (#{currency_symbol})", "Taxable Items Total (#{currency_symbol})", "Sales Tax (#{currency_symbol})", "Delivery Charge (#{currency_symbol})", "Tax on Delivery (#{currency_symbol})", "Total Tax (#{currency_symbol})", "Customer", "Distributor"] From 27a730ef6c35d8973b6dcc8d9435931c5209cc39 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 9 Jan 2015 15:05:25 +1100 Subject: [PATCH 21/35] Add spec for sales tax report totals calcs --- lib/open_food_network/sales_tax_report.rb | 2 +- .../sales_tax_report_spec.rb | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 spec/lib/open_food_network/sales_tax_report_spec.rb diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index 2579f91e59..f5755210c1 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -38,7 +38,7 @@ module OpenFoodNetwork if tax_rate != nil && tax_rate != 0 totals[:taxable_total] += line_item.amount - totals[:sales_tax] += line_item.amount * tax_rate + totals[:sales_tax] += (line_item.amount * tax_rate / (1 + tax_rate)).round(2) end end diff --git a/spec/lib/open_food_network/sales_tax_report_spec.rb b/spec/lib/open_food_network/sales_tax_report_spec.rb new file mode 100644 index 0000000000..32354dfa38 --- /dev/null +++ b/spec/lib/open_food_network/sales_tax_report_spec.rb @@ -0,0 +1,50 @@ +require 'open_food_network/sales_tax_report' + +module OpenFoodNetwork + describe SalesTaxReport do + describe "calculating totals for line items" do + let(:li1) { double(:line_item, quantity: 1, amount: 12) } + let(:li2) { double(:line_item, quantity: 2, amount: 24) } + let(:report) { SalesTaxReport.new(nil) } + let(:totals) { report.send(:totals_of, [li1, li2]) } + + before do + report.stub(:tax_rate_on) { 0.2 } + end + + it "calculates total quantity" do + totals[:items].should == 3 + end + + it "calculates total price" do + totals[:items_total].should == 36 + end + + it "calculates the taxable total price" do + totals[:taxable_total].should == 36 + end + + it "calculates sales tax" do + totals[:sales_tax].should == 6 + end + + context "when there is no tax on a line item" do + before do + report.stub(:tax_rate_on) { nil } + end + + it "does not appear in taxable total" do + totals[:taxable_total].should == 0 + end + + it "still appears on items total" do + totals[:items_total].should == 36 + end + + it "does not register sales tax" do + totals[:sales_tax].should == 0 + end + end + end + end +end From 11f59c9f59189ed04adf47689df17cdb5f8a3d9b Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 9 Jan 2015 15:21:54 +1100 Subject: [PATCH 22/35] Add spec for calculating shipping tax. Fix incorrect formula. --- lib/open_food_network/sales_tax_report.rb | 14 +++++++++--- .../sales_tax_report_spec.rb | 22 ++++++++++++++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index f5755210c1..7687b22ef7 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -51,15 +51,23 @@ module OpenFoodNetwork end def shipping_tax_on(shipping_cost) - if Spree::Config[:shipment_inc_vat] && shipping_cost != nil - shipping_cost * Spree::Config[:shipping_tax_rate] + if shipment_inc_vat && shipping_cost.present? + (shipping_cost * shipping_tax_rate / (1 + shipping_tax_rate)).round(2) else - 0.0 + 0 end end def tax_rate_on(line_item) Spree::TaxRate.find_by_tax_category_id(line_item.variant.product.tax_category_id).andand.amount end + + def shipment_inc_vat + Spree::Config.shipment_inc_vat + end + + def shipping_tax_rate + Spree::Config.shipping_tax_rate + end end end diff --git a/spec/lib/open_food_network/sales_tax_report_spec.rb b/spec/lib/open_food_network/sales_tax_report_spec.rb index 32354dfa38..234318e9df 100644 --- a/spec/lib/open_food_network/sales_tax_report_spec.rb +++ b/spec/lib/open_food_network/sales_tax_report_spec.rb @@ -2,10 +2,11 @@ require 'open_food_network/sales_tax_report' module OpenFoodNetwork describe SalesTaxReport do + let(:report) { SalesTaxReport.new(nil) } + describe "calculating totals for line items" do let(:li1) { double(:line_item, quantity: 1, amount: 12) } let(:li2) { double(:line_item, quantity: 2, amount: 24) } - let(:report) { SalesTaxReport.new(nil) } let(:totals) { report.send(:totals_of, [li1, li2]) } before do @@ -46,5 +47,24 @@ module OpenFoodNetwork end end end + + describe "calculating the shipping tax on a shipping cost" do + it "returns zero when shipping does not include VAT" do + report.stub(:shipment_inc_vat) { false } + report.send(:shipping_tax_on, 12).should == 0 + end + + it "returns zero when no shipping cost is passed" do + report.stub(:shipment_inc_vat) { true } + report.send(:shipping_tax_on, nil).should == 0 + end + + + it "returns the tax included in the price otherwise" do + report.stub(:shipment_inc_vat) { true } + report.stub(:shipping_tax_rate) { 0.2 } + report.send(:shipping_tax_on, 12).should == 2 + end + end end end From 54894fb2222e925e3adafbe972323383741f6a74 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 9 Jan 2015 15:24:55 +1100 Subject: [PATCH 23/35] Update spec with correct tax amounts --- spec/features/admin/reports_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index 93b0495ef7..c74b079ae5 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -145,11 +145,11 @@ feature %q{ # And the totals and sales tax should be correct page.should have_content "1512.99" # items total page.should have_content "1500.45" # taxable items total - page.should have_content "300.09" # sales tax + page.should have_content "250.08" # sales tax # And the shipping cost and tax should be correct page.should have_content "100.55" #shipping cost - page.should have_content "20.11" #shipping tax + page.should have_content "16.76" #shipping tax end end From ec22f4c09f1c3802a7f2b865e128946fd4f83733 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 13 Jan 2015 13:46:09 +1100 Subject: [PATCH 24/35] Sales tax report pulls sales tax from adjustments instead of recalculating it at report-time --- app/models/spree/adjustment_decorator.rb | 1 + lib/open_food_network/sales_tax_report.rb | 10 +++++----- spec/features/admin/reports_spec.rb | 13 ++++++++----- spec/lib/open_food_network/sales_tax_report_spec.rb | 4 ++-- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/app/models/spree/adjustment_decorator.rb b/app/models/spree/adjustment_decorator.rb index 2c5bb0be0a..0c6ce0b63d 100644 --- a/app/models/spree/adjustment_decorator.rb +++ b/app/models/spree/adjustment_decorator.rb @@ -3,5 +3,6 @@ module Spree has_one :metadata, class_name: 'AdjustmentMetadata', dependent: :destroy scope :enterprise_fee, where(originator_type: 'EnterpriseFee') + scope :included_tax, where(originator_type: 'Spree::TaxRate', adjustable_type: 'Spree::LineItem') end end diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index 7687b22ef7..3920773d06 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -34,11 +34,11 @@ module OpenFoodNetwork totals[:items] += line_item.quantity totals[:items_total] += line_item.amount - tax_rate = tax_rate_on line_item + sales_tax = tax_included_in line_item - if tax_rate != nil && tax_rate != 0 + if sales_tax > 0 totals[:taxable_total] += line_item.amount - totals[:sales_tax] += (line_item.amount * tax_rate / (1 + tax_rate)).round(2) + totals[:sales_tax] += sales_tax end end @@ -58,8 +58,8 @@ module OpenFoodNetwork end end - def tax_rate_on(line_item) - Spree::TaxRate.find_by_tax_category_id(line_item.variant.product.tax_category_id).andand.amount + def tax_included_in(line_item) + line_item.adjustments.included_tax.sum &:amount end def shipment_inc_vat diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index c74b079ae5..2ad4e34f84 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -99,7 +99,7 @@ feature %q{ page.should have_content 'Payment State' end - describe "Sales Tax report" do + describe "Sales tax report" do let(:user1) do create_enterprise_user(enterprises: [create(:distributor_enterprise)]) end @@ -118,7 +118,9 @@ feature %q{ let(:order1) { create(:order, distributor: user1.enterprises.first, shipping_method: shipping_method, bill_address: create(:address)) } let!(:line_item1) { create(:line_item, variant: product1.master, price: 12.54, quantity: 1, order: order1) } let!(:line_item2) { create(:line_item, variant: product2.master, price: 500.15, quantity: 3, order: order1) } - let!(:adjustment) { create(:adjustment, adjustable: order1, label: "Shipping", amount: 100.55) } + + let!(:adj_shipping) { create(:adjustment, adjustable: order1, label: "Shipping", amount: 100.55) } + let!(:adj_li2_tax) { create(:adjustment, adjustable: line_item2, source: line_item2, originator: tax_rate2, label: "RandomTax", amount: 123.00) } before do Spree::Config.shipment_inc_vat = true @@ -145,11 +147,12 @@ feature %q{ # And the totals and sales tax should be correct page.should have_content "1512.99" # items total page.should have_content "1500.45" # taxable items total - page.should have_content "250.08" # sales tax + page.should have_content "123.0" # sales tax (from adj_li2_tax, not calculated on the fly) + page.should_not have_content "250.08" # the number that would have been calculated on the fly # And the shipping cost and tax should be correct - page.should have_content "100.55" #shipping cost - page.should have_content "16.76" #shipping tax + page.should have_content "100.55" # shipping cost + page.should have_content "16.76" # shipping tax # TODO: do not calculate on the fly end end diff --git a/spec/lib/open_food_network/sales_tax_report_spec.rb b/spec/lib/open_food_network/sales_tax_report_spec.rb index 234318e9df..cf00f650ff 100644 --- a/spec/lib/open_food_network/sales_tax_report_spec.rb +++ b/spec/lib/open_food_network/sales_tax_report_spec.rb @@ -10,7 +10,7 @@ module OpenFoodNetwork let(:totals) { report.send(:totals_of, [li1, li2]) } before do - report.stub(:tax_rate_on) { 0.2 } + report.stub(:tax_included_in).and_return(2, 4) end it "calculates total quantity" do @@ -31,7 +31,7 @@ module OpenFoodNetwork context "when there is no tax on a line item" do before do - report.stub(:tax_rate_on) { nil } + report.stub(:tax_included_in) { 0 } end it "does not appear in taxable total" do From dd61034908c7ea50f426af4af6376a32cb7573a1 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 13 Jan 2015 14:47:13 +1100 Subject: [PATCH 25/35] Fix fractional cents appearing on sales tax report totals --- lib/open_food_network/sales_tax_report.rb | 4 ++++ spec/lib/open_food_network/sales_tax_report_spec.rb | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index 3920773d06..d5b69011d2 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -42,6 +42,10 @@ module OpenFoodNetwork end end + totals.each_pair do |k, v| + totals[k] = totals[k].round(2) + end + totals end diff --git a/spec/lib/open_food_network/sales_tax_report_spec.rb b/spec/lib/open_food_network/sales_tax_report_spec.rb index cf00f650ff..043eac6ae7 100644 --- a/spec/lib/open_food_network/sales_tax_report_spec.rb +++ b/spec/lib/open_food_network/sales_tax_report_spec.rb @@ -21,6 +21,15 @@ module OpenFoodNetwork totals[:items_total].should == 36 end + context "when floating point math would result in fractional cents" do + let(:li1) { double(:line_item, quantity: 1, amount: 0.11) } + let(:li2) { double(:line_item, quantity: 2, amount: 0.12) } + + it "rounds to the nearest cent" do + totals[:items_total].should == 0.23 + end + end + it "calculates the taxable total price" do totals[:taxable_total].should == 36 end From 61c08997a1383732acb3b7eb4fc12b00d41761bf Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 25 Feb 2015 22:25:53 +1100 Subject: [PATCH 26/35] EnterpriseFee has a TaxCategory --- app/models/enterprise_fee.rb | 1 + ...50225111538_add_tax_category_to_enterprise_fee.rb | 7 +++++++ db/schema.rb | 12 +++++++++--- 3 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20150225111538_add_tax_category_to_enterprise_fee.rb diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index 6270f77b7a..23ea158181 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -1,5 +1,6 @@ class EnterpriseFee < ActiveRecord::Base belongs_to :enterprise + belongs_to :tax_category, class_name: 'Spree::TaxCategory', foreign_key: 'tax_category_id' has_and_belongs_to_many :order_cycles, join_table: 'coordinator_fees' has_many :exchange_fees, dependent: :destroy has_many :exchanges, through: :exchange_fees diff --git a/db/migrate/20150225111538_add_tax_category_to_enterprise_fee.rb b/db/migrate/20150225111538_add_tax_category_to_enterprise_fee.rb new file mode 100644 index 0000000000..05b5410587 --- /dev/null +++ b/db/migrate/20150225111538_add_tax_category_to_enterprise_fee.rb @@ -0,0 +1,7 @@ +class AddTaxCategoryToEnterpriseFee < ActiveRecord::Migration + def change + add_column :enterprise_fees, :tax_category_id, :integer + add_foreign_key :enterprise_fees, :spree_tax_categories, column: :tax_category_id + add_index :enterprise_fees, :tax_category_id + end +end diff --git a/db/schema.rb b/db/schema.rb index a8ac91e6d8..9301baf229 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20150220035501) do +ActiveRecord::Schema.define(:version => 20150225111538) do create_table "adjustment_metadata", :force => true do |t| t.integer "adjustment_id" @@ -177,11 +177,13 @@ ActiveRecord::Schema.define(:version => 20150220035501) do t.integer "enterprise_id" t.string "fee_type" t.string "name" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false + t.integer "tax_category_id" end add_index "enterprise_fees", ["enterprise_id"], :name => "index_enterprise_fees_on_enterprise_id" + add_index "enterprise_fees", ["tax_category_id"], :name => "index_enterprise_fees_on_tax_category_id" create_table "enterprise_groups", :force => true do |t| t.string "name" @@ -1088,6 +1090,10 @@ ActiveRecord::Schema.define(:version => 20150220035501) do add_foreign_key "distributors_shipping_methods", "spree_shipping_methods", name: "distributors_shipping_methods_shipping_method_id_fk", column: "shipping_method_id" add_foreign_key "enterprise_fees", "enterprises", name: "enterprise_fees_enterprise_id_fk" + add_foreign_key "enterprise_fees", "spree_tax_categories", name: "enterprise_fees_tax_category_id_fk", column: "tax_category_id" + + add_foreign_key "enterprise_groups", "spree_addresses", name: "enterprise_groups_address_id_fk", column: "address_id" + add_foreign_key "enterprise_groups", "spree_users", name: "enterprise_groups_owner_id_fk", column: "owner_id" add_foreign_key "enterprise_groups", "spree_addresses", name: "enterprise_groups_address_id_fk", column: "address_id" add_foreign_key "enterprise_groups", "spree_users", name: "enterprise_groups_owner_id_fk", column: "owner_id" From 53fa71d1f334c268dbd78a2c5607fa3891faad26 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 26 Feb 2015 09:20:35 +1100 Subject: [PATCH 27/35] Admin can set enterprise fee's tax category --- app/controllers/admin/enterprise_fees_controller.rb | 1 + app/models/enterprise_fee.rb | 2 +- app/presenters/enterprise_fee_presenter.rb | 2 +- app/views/admin/enterprise_fees/index.html.haml | 2 ++ app/views/admin/enterprise_fees/index.rep | 1 + spec/features/admin/enterprise_fees_spec.rb | 12 ++++++++++-- 6 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/enterprise_fees_controller.rb b/app/controllers/admin/enterprise_fees_controller.rb index 085828d7b3..c4d4a621b1 100644 --- a/app/controllers/admin/enterprise_fees_controller.rb +++ b/app/controllers/admin/enterprise_fees_controller.rb @@ -59,6 +59,7 @@ module Admin def load_data @calculators = EnterpriseFee.calculators.sort_by(&:name) + @tax_categories = Spree::TaxCategory.order('is_default DESC, name ASC') end def collection diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index 23ea158181..730bac0787 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -9,7 +9,7 @@ class EnterpriseFee < ActiveRecord::Base calculated_adjustments - attr_accessible :enterprise_id, :fee_type, :name, :calculator_type + attr_accessible :enterprise_id, :fee_type, :name, :tax_category_id, :calculator_type FEE_TYPES = %w(packing transport admin sales fundraising) PER_ORDER_CALCULATORS = ['Spree::Calculator::FlatRate', 'Spree::Calculator::FlexiRate'] diff --git a/app/presenters/enterprise_fee_presenter.rb b/app/presenters/enterprise_fee_presenter.rb index a0a6d8460a..b8f9ae4655 100644 --- a/app/presenters/enterprise_fee_presenter.rb +++ b/app/presenters/enterprise_fee_presenter.rb @@ -3,7 +3,7 @@ class EnterpriseFeePresenter @controller, @enterprise_fee, @index = controller, enterprise_fee, index end - delegate :id, :enterprise_id, :fee_type, :name, :calculator_type, :to => :enterprise_fee + delegate :id, :enterprise_id, :fee_type, :name, :tax_category_id, :calculator_type, :to => :enterprise_fee def enterprise_fee @enterprise_fee diff --git a/app/views/admin/enterprise_fees/index.html.haml b/app/views/admin/enterprise_fees/index.html.haml index ba3c3e6f4c..2237ca8a43 100644 --- a/app/views/admin/enterprise_fees/index.html.haml +++ b/app/views/admin/enterprise_fees/index.html.haml @@ -13,6 +13,7 @@ %th Enterprise %th Fee Type %th Name + %th Tax Category %th Calculator %th Calculator values %th.actions @@ -24,6 +25,7 @@ = f.ng_collection_select :enterprise_id, @enterprises, :id, :name, 'enterprise_fee.enterprise_id', :include_blank => true %td= f.ng_select :fee_type, enterprise_fee_type_options, 'enterprise_fee.fee_type' %td= f.ng_text_field :name, { placeholder: 'e.g. packing fee' } + %td= f.ng_collection_select :tax_category_id, @tax_categories, :id, :name, 'enterprise_fee.tax_category_id' %td= f.ng_collection_select :calculator_type, @calculators, :name, :description, 'enterprise_fee.calculator_type', {'class' => 'calculator_type', 'ng-model' => 'calculatorType', 'spree-ensure-calculator-preferences-match-type' => "1"} %td{'ng-bind-html-unsafe-compiled' => 'enterprise_fee.calculator_settings'} %td.actions{'spree-delete-resource' => "1"} diff --git a/app/views/admin/enterprise_fees/index.rep b/app/views/admin/enterprise_fees/index.rep index 61192ca786..8dc24b5d74 100644 --- a/app/views/admin/enterprise_fees/index.rep +++ b/app/views/admin/enterprise_fees/index.rep @@ -4,6 +4,7 @@ r.list_of :enterprise_fees, @presented_collection do r.element :enterprise_name r.element :fee_type r.element :name + r.element :tax_category_id r.element :calculator_type r.element :calculator_description r.element :calculator_settings if @include_calculators diff --git a/spec/features/admin/enterprise_fees_spec.rb b/spec/features/admin/enterprise_fees_spec.rb index 44f26deddd..36fe1b76ac 100644 --- a/spec/features/admin/enterprise_fees_spec.rb +++ b/spec/features/admin/enterprise_fees_spec.rb @@ -6,9 +6,12 @@ feature %q{ }, js: true do include AuthenticationWorkflow include WebHelper - + + let!(:tax_category_gst) { create(:tax_category, name: 'GST') } + let!(:tax_category_gst_exempt) { create(:tax_category, name: 'GST exempt') } + scenario "listing enterprise fees" do - fee = create(:enterprise_fee, name: '$0.50 / kg', fee_type: 'packing') + fee = create(:enterprise_fee, name: '$0.50 / kg', fee_type: 'packing', tax_category: tax_category_gst) amount = fee.calculator.preferred_amount login_to_admin_section @@ -18,6 +21,7 @@ feature %q{ page.should have_selector "#enterprise_fee_set_collection_attributes_0_enterprise_id" page.should have_selector "option[selected]", text: 'Packing' page.should have_selector "input[value='$0.50 / kg']" + page.should have_selector "option[selected]", text: 'GST' page.should have_selector "option[selected]", text: 'Flat Rate (per item)' page.should have_selector "input[value='#{amount}']" end @@ -35,6 +39,7 @@ feature %q{ select 'Feedme', from: 'enterprise_fee_set_collection_attributes_0_enterprise_id' select 'Admin', from: 'enterprise_fee_set_collection_attributes_0_fee_type' fill_in 'enterprise_fee_set_collection_attributes_0_name', with: 'Hello!' + select 'GST', from: 'enterprise_fee_set_collection_attributes_0_tax_category_id' select 'Flat Percent', from: 'enterprise_fee_set_collection_attributes_0_calculator_type' click_button 'Update' @@ -64,6 +69,7 @@ feature %q{ select 'Foo', from: 'enterprise_fee_set_collection_attributes_0_enterprise_id' select 'Admin', from: 'enterprise_fee_set_collection_attributes_0_fee_type' fill_in 'enterprise_fee_set_collection_attributes_0_name', with: 'Greetings!' + select 'GST exempt', from: 'enterprise_fee_set_collection_attributes_0_tax_category_id' select 'Flat Percent', from: 'enterprise_fee_set_collection_attributes_0_calculator_type' click_button 'Update' @@ -71,6 +77,7 @@ feature %q{ page.should have_selector "option[selected]", text: 'Foo' page.should have_selector "option[selected]", text: 'Admin' page.should have_selector "input[value='Greetings!']" + page.should have_selector "option[selected]", text: 'GST exempt' page.should have_selector "option[selected]", text: 'Flat Percent' end @@ -137,6 +144,7 @@ feature %q{ select distributor1.name, :from => 'enterprise_fee_set_collection_attributes_0_enterprise_id' fill_in 'enterprise_fee_set_collection_attributes_0_name', :with => 'foo' + select 'GST', from: 'enterprise_fee_set_collection_attributes_0_tax_category_id' select 'Flat Percent', :from => 'enterprise_fee_set_collection_attributes_0_calculator_type' click_button 'Update' From 9395f6c80803861141e397f3a99e7e7e0f1dd41a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 2 Mar 2015 11:01:37 +1100 Subject: [PATCH 28/35] Record the tax included in adjustments. TaxRate adjustments consist of 100% tax. --- app/models/spree/adjustment_decorator.rb | 6 ++++ app/models/spree/tax_rate_decorator.rb | 11 +++++++ ...5232938_add_included_tax_to_adjustments.rb | 5 ++++ db/schema.rb | 3 +- spec/models/spree/adjustment_spec.rb | 30 +++++++++++++++++++ 5 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 app/models/spree/tax_rate_decorator.rb create mode 100644 db/migrate/20150225232938_add_included_tax_to_adjustments.rb diff --git a/app/models/spree/adjustment_decorator.rb b/app/models/spree/adjustment_decorator.rb index 0c6ce0b63d..b9b89cde6a 100644 --- a/app/models/spree/adjustment_decorator.rb +++ b/app/models/spree/adjustment_decorator.rb @@ -4,5 +4,11 @@ module Spree scope :enterprise_fee, where(originator_type: 'EnterpriseFee') scope :included_tax, where(originator_type: 'Spree::TaxRate', adjustable_type: 'Spree::LineItem') + + attr_accessible :included_tax + + def set_included_tax!(fraction) + update_attributes! included_tax: (amount * fraction).round(2) + end end end diff --git a/app/models/spree/tax_rate_decorator.rb b/app/models/spree/tax_rate_decorator.rb new file mode 100644 index 0000000000..f8a0333ce2 --- /dev/null +++ b/app/models/spree/tax_rate_decorator.rb @@ -0,0 +1,11 @@ +Spree::TaxRate.class_eval do + def adjust_with_included_tax(order) + adjust_without_included_tax(order) + + (order.adjustments.tax + order.price_adjustments).each do |a| + a.set_included_tax! 1.0 + end + end + + alias_method_chain :adjust, :included_tax +end diff --git a/db/migrate/20150225232938_add_included_tax_to_adjustments.rb b/db/migrate/20150225232938_add_included_tax_to_adjustments.rb new file mode 100644 index 0000000000..fa28ab20c7 --- /dev/null +++ b/db/migrate/20150225232938_add_included_tax_to_adjustments.rb @@ -0,0 +1,5 @@ +class AddIncludedTaxToAdjustments < ActiveRecord::Migration + def change + add_column :spree_adjustments, :included_tax, :decimal, precision: 10, scale: 2 + end +end diff --git a/db/schema.rb b/db/schema.rb index 9301baf229..cc55ad2392 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20150225111538) do +ActiveRecord::Schema.define(:version => 20150225232938) do create_table "adjustment_metadata", :force => true do |t| t.integer "adjustment_id" @@ -416,6 +416,7 @@ ActiveRecord::Schema.define(:version => 20150225111538) do t.string "originator_type" t.boolean "eligible", :default => true t.string "adjustable_type" + t.decimal "included_tax", :precision => 10, :scale => 2 end add_index "spree_adjustments", ["adjustable_id"], :name => "index_adjustments_on_order_id" diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 60dfd631ce..dc17ac5eb2 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -4,5 +4,35 @@ module Spree adjustment = create(:adjustment, metadata: create(:adjustment_metadata)) adjustment.metadata.should be end + + describe "recording included tax" do + describe "TaxRate adjustments" do + let!(:zone) { create(:zone, default_tax: true) } + let!(:zone_member) { ZoneMember.create!(zone: zone, zoneable: Country.find_by_name('Australia')) } + let!(:order) { create(:order) } + let!(:line_item) { create(:line_item, order: order) } + let(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::FlatRate.new(preferred_amount: 0.1)) } + let(:adjustment) { line_item.adjustments(:reload).first } + + before do + order.reload + tax_rate.adjust(order) + end + + it "has 100% tax included" do + adjustment.amount.should be > 0 + adjustment.included_tax.should == adjustment.amount + end + end + + describe "setting the included tax by fraction" do + let(:adjustment) { Adjustment.new label: 'foo', amount: 123.45 } + + it "sets it, rounding to two decimal places" do + adjustment.set_included_tax! 0.1 + adjustment.included_tax.should == 12.35 + end + end + end end end From dfb855bd14a40e0df8bb5cdb7c8f94445d0ccfc6 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 2 Mar 2015 15:17:14 +1100 Subject: [PATCH 29/35] Record the tax included in shipping. --- app/models/spree/shipment_decorator.rb | 11 ++++ ...5232938_add_included_tax_to_adjustments.rb | 2 +- db/schema.rb | 2 +- spec/models/spree/adjustment_spec.rb | 60 +++++++++++++++++-- 4 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 app/models/spree/shipment_decorator.rb diff --git a/app/models/spree/shipment_decorator.rb b/app/models/spree/shipment_decorator.rb new file mode 100644 index 0000000000..ee9189efdd --- /dev/null +++ b/app/models/spree/shipment_decorator.rb @@ -0,0 +1,11 @@ +module Spree + Shipment.class_eval do + def ensure_correct_adjustment_with_included_tax + ensure_correct_adjustment_without_included_tax + + adjustment.set_included_tax! Config.shipping_tax_rate if Config.shipment_inc_vat + end + + alias_method_chain :ensure_correct_adjustment, :included_tax + end +end diff --git a/db/migrate/20150225232938_add_included_tax_to_adjustments.rb b/db/migrate/20150225232938_add_included_tax_to_adjustments.rb index fa28ab20c7..f3815dec2a 100644 --- a/db/migrate/20150225232938_add_included_tax_to_adjustments.rb +++ b/db/migrate/20150225232938_add_included_tax_to_adjustments.rb @@ -1,5 +1,5 @@ class AddIncludedTaxToAdjustments < ActiveRecord::Migration def change - add_column :spree_adjustments, :included_tax, :decimal, precision: 10, scale: 2 + add_column :spree_adjustments, :included_tax, :decimal, precision: 10, scale: 2, null: false, default: 0 end end diff --git a/db/schema.rb b/db/schema.rb index cc55ad2392..a325b69f80 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -416,7 +416,7 @@ ActiveRecord::Schema.define(:version => 20150225232938) do t.string "originator_type" t.boolean "eligible", :default => true t.string "adjustable_type" - t.decimal "included_tax", :precision => 10, :scale => 2 + t.decimal "included_tax", :precision => 10, :scale => 2, :default => 0.0, :null => false end add_index "spree_adjustments", ["adjustable_id"], :name => "index_adjustments_on_order_id" diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index dc17ac5eb2..44d3cab868 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -7,12 +7,12 @@ module Spree describe "recording included tax" do describe "TaxRate adjustments" do - let!(:zone) { create(:zone, default_tax: true) } + let!(:zone) { create(:zone, default_tax: true) } let!(:zone_member) { ZoneMember.create!(zone: zone, zoneable: Country.find_by_name('Australia')) } - let!(:order) { create(:order) } - let!(:line_item) { create(:line_item, order: order) } - let(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::FlatRate.new(preferred_amount: 0.1)) } - let(:adjustment) { line_item.adjustments(:reload).first } + let!(:order) { create(:order) } + let!(:line_item) { create(:line_item, order: order) } + let(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::FlatRate.new(preferred_amount: 0.1)) } + let(:adjustment) { line_item.adjustments(:reload).first } before do order.reload @@ -25,6 +25,56 @@ module Spree end end + describe "Shipment adjustments" do + let!(:order) { create(:order, shipping_method: shipping_method) } + let!(:line_item) { create(:line_item, order: order) } + let(:shipping_method) { create(:shipping_method, calculator: Calculator::FlatRate.new(preferred_amount: 50.0)) } + let(:adjustment) { order.adjustments(:reload).shipping.first } + + it "has a shipping charge of $50" do + order.create_shipment! + adjustment.amount.should == 50 + end + + context "when tax on shipping is disabled" do + it "records 0% tax on shipment adjustments" do + Config.shipment_inc_vat = false + Config.shipping_tax_rate = 0 + order.create_shipment! + + adjustment.included_tax.should == 0 + end + + it "records 0% tax on shipments when a rate is set but shipment_inc_vat is false" do + Config.shipment_inc_vat = false + Config.shipping_tax_rate = 0.25 + order.create_shipment! + + adjustment.included_tax.should == 0 + end + end + + context "when tax on shipping is enabled" do + before do + Config.shipment_inc_vat = true + Config.shipping_tax_rate = 0.25 + order.create_shipment! + end + + it "takes the shipment adjustment tax included from the system setting" do + adjustment.included_tax.should == 12.50 + end + + it "records 0% tax on shipments when shipping_tax_rate is not set" do + Config.shipment_inc_vat = true + Config.shipping_tax_rate = nil + order.create_shipment! + + adjustment.included_tax.should == 0 + end + end + end + describe "setting the included tax by fraction" do let(:adjustment) { Adjustment.new label: 'foo', amount: 123.45 } From b5ce056d0679d0d4cd83cc651c028f66c7ea706a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 5 Mar 2015 13:03:53 +1100 Subject: [PATCH 30/35] Fix tax calculations for determining tax included in an amount --- app/models/spree/adjustment_decorator.rb | 9 +++++++-- app/models/spree/tax_rate_decorator.rb | 2 +- spec/models/spree/adjustment_spec.rb | 14 +++++++++----- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/app/models/spree/adjustment_decorator.rb b/app/models/spree/adjustment_decorator.rb index b9b89cde6a..836080183c 100644 --- a/app/models/spree/adjustment_decorator.rb +++ b/app/models/spree/adjustment_decorator.rb @@ -7,8 +7,13 @@ module Spree attr_accessible :included_tax - def set_included_tax!(fraction) - update_attributes! included_tax: (amount * fraction).round(2) + def set_included_tax!(rate) + tax = amount - (amount / (1 + rate)) + set_absolute_included_tax! tax + end + + def set_absolute_included_tax!(tax) + update_attributes! included_tax: tax.round(2) end end end diff --git a/app/models/spree/tax_rate_decorator.rb b/app/models/spree/tax_rate_decorator.rb index f8a0333ce2..e41c20db70 100644 --- a/app/models/spree/tax_rate_decorator.rb +++ b/app/models/spree/tax_rate_decorator.rb @@ -3,7 +3,7 @@ Spree::TaxRate.class_eval do adjust_without_included_tax(order) (order.adjustments.tax + order.price_adjustments).each do |a| - a.set_included_tax! 1.0 + a.set_absolute_included_tax! a.amount end end diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 44d3cab868..acd0fa64ac 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -62,7 +62,11 @@ module Spree end it "takes the shipment adjustment tax included from the system setting" do - adjustment.included_tax.should == 12.50 + # Finding the tax included in an amount that's already inclusive of tax: + # total - ( total / (1 + rate) ) + # 50 - ( 50 / (1 + 0.25) ) + # = 10 + adjustment.included_tax.should == 10.00 end it "records 0% tax on shipments when shipping_tax_rate is not set" do @@ -75,12 +79,12 @@ module Spree end end - describe "setting the included tax by fraction" do - let(:adjustment) { Adjustment.new label: 'foo', amount: 123.45 } + describe "setting the included tax by tax rate" do + let(:adjustment) { Adjustment.new label: 'foo', amount: 50 } it "sets it, rounding to two decimal places" do - adjustment.set_included_tax! 0.1 - adjustment.included_tax.should == 12.35 + adjustment.set_included_tax! 0.25 + adjustment.included_tax.should == 10.00 end end end From 98ff895f5fcb7bfbd28463a70846698da6c1a07a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 6 Mar 2015 10:35:40 +1100 Subject: [PATCH 31/35] Record the tax included in per-order EnterpriseFees --- .../enterprise_fee_applicator.rb | 44 ++++++++++++++++ .../enterprise_fee_applicator_spec.rb | 29 +++++++++++ spec/models/spree/adjustment_spec.rb | 51 +++++++++++++++++++ 3 files changed, 124 insertions(+) diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index 45905bfaca..9a2bd657b5 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -2,12 +2,16 @@ module OpenFoodNetwork class EnterpriseFeeApplicator < Struct.new(:enterprise_fee, :variant, :role) def create_line_item_adjustment(line_item) a = enterprise_fee.create_locked_adjustment(line_item_adjustment_label, line_item.order, line_item, true) + AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role end def create_order_adjustment(order) a = enterprise_fee.create_locked_adjustment(order_adjustment_label, order, order, true) + AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role + + a.set_absolute_included_tax! adjustment_tax(order, a) end @@ -24,6 +28,46 @@ module OpenFoodNetwork def base_adjustment_label "#{enterprise_fee.fee_type} fee by #{role} #{enterprise_fee.enterprise.name}" end + + def adjustment_tax(order, adjustment) + enterprise_fee.tax_category.tax_rates.match(order).sum do |rate| + compute_tax rate, adjustment.amount + end + end + + # Apply a TaxRate to a particular amount. TaxRates normally compute against + # LineItems or Orders, so we mock out a line item here to fit the interface + # that our calculator (usually DefaultTax) expects. + def compute_tax(tax_rate, amount) + product = OpenStruct.new tax_category: tax_rate.tax_category + line_item = Spree::LineItem.new quantity: 1 + line_item.define_singleton_method(:product) { product } + line_item.define_singleton_method(:price) { amount } + + # The enterprise fee adjustments for which we're calculating tax are always inclusive of + # tax. However, there's nothing to stop an admin from setting one up with a tax rate + # that's marked as not inclusive of tax, and that would result in the DefaultTax + # calculator generating a slightly incorrect value. Therefore, we treat the tax + # rate as inclusive of tax for the calculations below, regardless of its original + # setting. + with_tax_included_in_price(tax_rate) do + tax_rate.calculator.compute line_item + end + end + + def with_tax_included_in_price(tax_rate) + old_included_in_price = tax_rate.included_in_price + + tax_rate.included_in_price = true + tax_rate.calculator.calculable.included_in_price = true + + result = yield + + tax_rate.included_in_price = old_included_in_price + tax_rate.calculator.calculable.included_in_price = old_included_in_price + + result + end end end diff --git a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb index 9ad1d222b3..6703f844ab 100644 --- a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb +++ b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb @@ -65,4 +65,33 @@ module OpenFoodNetwork efa.send(:order_adjustment_label).should == "Whole order - packing fee by distributor Ballantyne" end end + + describe "ensuring that tax rate is marked as tax included_in_price" do + let(:efa) { EnterpriseFeeApplicator.new nil, nil, nil } + let(:tax_rate) { create(:tax_rate, included_in_price: false, calculator: Spree::Calculator::DefaultTax.new) } + + it "sets included_in_price to true" do + efa.send(:with_tax_included_in_price, tax_rate) do + tax_rate.included_in_price.should be_true + end + end + + it "sets the included_in_price value accessible to the calculator to true" do + efa.send(:with_tax_included_in_price, tax_rate) do + tax_rate.calculator.calculable.included_in_price.should be_true + end + end + + it "passes through the return value of the block" do + efa.send(:with_tax_included_in_price, tax_rate) do + 'asdf' + end.should == 'asdf' + end + + it "restores both values to their original afterwards" do + efa.send(:with_tax_included_in_price, tax_rate) {} + tax_rate.included_in_price.should be_false + tax_rate.calculator.calculable.included_in_price.should be_false + end + end end diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index acd0fa64ac..d329db9546 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -79,6 +79,57 @@ module Spree end end + describe "EnterpriseFee adjustments" do + let!(:zone) { create(:zone, default_tax: true) } + let!(:zone_member) { ZoneMember.create!(zone: zone, zoneable: Country.find_by_name('Australia')) } + let(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.1) } + let(:tax_category) { create(:tax_category, tax_rates: [tax_rate]) } + let(:tax_category_untaxed) { create(:tax_category) } + + let(:coordinator) { create(:distributor_enterprise) } + let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: tax_category, calculator: Calculator::FlatRate.new(preferred_amount: 50.0)) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator, coordinator_fees: [enterprise_fee], distributors: [coordinator]) } + let!(:order) { create(:order, order_cycle: order_cycle, distributor: coordinator) } + let!(:line_item) { create(:line_item, order: order) } + let(:adjustment) { order.adjustments(:reload).enterprise_fee.first } + + before do + order.update_distribution_charge! + end + + context "when enterprise fees are taxed" do + it "records the tax on the enterprise fee adjustments" do + # The fee is $50, tax is 10%, and the fee is inclusive of tax + # Therefore, the included tax should be 0.1/1.1 * 50 = $4.55 + + adjustment.included_tax.should == 4.55 + end + + context "when the tax rate does not include the tax in the price" do + before do + tax_rate.update_attribute :included_in_price, false + order.update_distribution_charge! + end + + it "treats it as inclusive anyway" do + adjustment.included_tax.should == 4.55 + end + end + end + + context "when enterprise fees have no tax" do + before do + enterprise_fee.tax_category = tax_category_untaxed + enterprise_fee.save! + order.update_distribution_charge! + end + + it "records no tax as charged" do + adjustment.included_tax.should == 0 + end + end + end + describe "setting the included tax by tax rate" do let(:adjustment) { Adjustment.new label: 'foo', amount: 50 } From 41792395aaa96a6fa7d566229b384c47c71f3467 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 6 Mar 2015 11:13:47 +1100 Subject: [PATCH 32/35] Record the tax included in per-item EnterpriseFees --- .../enterprise_fee_applicator.rb | 2 + spec/models/spree/adjustment_spec.rb | 51 +++++++++++-------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index 9a2bd657b5..f59c94f119 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -4,6 +4,8 @@ module OpenFoodNetwork a = enterprise_fee.create_locked_adjustment(line_item_adjustment_label, line_item.order, line_item, true) AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role + + a.set_absolute_included_tax! adjustment_tax(line_item.order, a) end def create_order_adjustment(order) diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index d329db9546..289b70ccab 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -80,24 +80,26 @@ module Spree end describe "EnterpriseFee adjustments" do - let!(:zone) { create(:zone, default_tax: true) } - let!(:zone_member) { ZoneMember.create!(zone: zone, zoneable: Country.find_by_name('Australia')) } - let(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.1) } - let(:tax_category) { create(:tax_category, tax_rates: [tax_rate]) } + let!(:zone) { create(:zone, default_tax: true) } + let!(:zone_member) { ZoneMember.create!(zone: zone, zoneable: Country.find_by_name('Australia')) } + let(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.1) } + let(:tax_category) { create(:tax_category, tax_rates: [tax_rate]) } let(:tax_category_untaxed) { create(:tax_category) } - let(:coordinator) { create(:distributor_enterprise) } - let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: tax_category, calculator: Calculator::FlatRate.new(preferred_amount: 50.0)) } - let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator, coordinator_fees: [enterprise_fee], distributors: [coordinator]) } - let!(:order) { create(:order, order_cycle: order_cycle, distributor: coordinator) } - let!(:line_item) { create(:line_item, order: order) } - let(:adjustment) { order.adjustments(:reload).enterprise_fee.first } + let(:coordinator) { create(:distributor_enterprise) } + let(:variant) { create(:variant) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator, coordinator_fees: [enterprise_fee], distributors: [coordinator], variants: [variant]) } + let!(:order) { create(:order, order_cycle: order_cycle, distributor: coordinator) } + let!(:line_item) { create(:line_item, order: order, variant: variant) } + let(:adjustment) { order.adjustments(:reload).enterprise_fee.first } before do - order.update_distribution_charge! + order.reload.update_distribution_charge! end - context "when enterprise fees are taxed" do + context "when enterprise fees are taxed per-order" do + let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: tax_category, calculator: Calculator::FlatRate.new(preferred_amount: 50.0)) } + it "records the tax on the enterprise fee adjustments" do # The fee is $50, tax is 10%, and the fee is inclusive of tax # Therefore, the included tax should be 0.1/1.1 * 50 = $4.55 @@ -115,17 +117,26 @@ module Spree adjustment.included_tax.should == 4.55 end end + + context "when enterprise fees have no tax" do + before do + enterprise_fee.tax_category = tax_category_untaxed + enterprise_fee.save! + order.update_distribution_charge! + end + + it "records no tax as charged" do + adjustment.included_tax.should == 0 + end + end end - context "when enterprise fees have no tax" do - before do - enterprise_fee.tax_category = tax_category_untaxed - enterprise_fee.save! - order.update_distribution_charge! - end - it "records no tax as charged" do - adjustment.included_tax.should == 0 + context "when enterprise fees are taxed per-item" do + let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: tax_category, calculator: Calculator::PerItem.new(preferred_amount: 50.0)) } + + it "records the tax on the enterprise fee adjustments" do + adjustment.included_tax.should == 4.55 end end end From 1e18f773f57b2d70eaf05ada746c11a0a6337df9 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 6 Mar 2015 11:15:07 +1100 Subject: [PATCH 33/35] Switch context -> describe, clarify test grammar --- spec/models/spree/adjustment_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 289b70ccab..0922597c2a 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -36,7 +36,7 @@ module Spree adjustment.amount.should == 50 end - context "when tax on shipping is disabled" do + describe "when tax on shipping is disabled" do it "records 0% tax on shipment adjustments" do Config.shipment_inc_vat = false Config.shipping_tax_rate = 0 @@ -54,7 +54,7 @@ module Spree end end - context "when tax on shipping is enabled" do + describe "when tax on shipping is enabled" do before do Config.shipment_inc_vat = true Config.shipping_tax_rate = 0.25 @@ -107,7 +107,7 @@ module Spree adjustment.included_tax.should == 4.55 end - context "when the tax rate does not include the tax in the price" do + describe "when the tax rate does not include the tax in the price" do before do tax_rate.update_attribute :included_in_price, false order.update_distribution_charge! @@ -118,7 +118,7 @@ module Spree end end - context "when enterprise fees have no tax" do + describe "when enterprise fees have no tax" do before do enterprise_fee.tax_category = tax_category_untaxed enterprise_fee.save! From 381bfd383bc29f5c1eddc04d501923dc10cfd868 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 6 Mar 2015 12:12:33 +1100 Subject: [PATCH 34/35] Allow enterprise fee with no tax category --- lib/open_food_network/enterprise_fee_applicator.rb | 4 +++- spec/models/spree/adjustment_spec.rb | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index f59c94f119..070cb4a91c 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -32,7 +32,9 @@ module OpenFoodNetwork end def adjustment_tax(order, adjustment) - enterprise_fee.tax_category.tax_rates.match(order).sum do |rate| + tax_rates = enterprise_fee.tax_category ? enterprise_fee.tax_category.tax_rates.match(order) : [] + + tax_rates.sum do |rate| compute_tax rate, adjustment.amount end end diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 0922597c2a..35746b28fd 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -84,7 +84,6 @@ module Spree let!(:zone_member) { ZoneMember.create!(zone: zone, zoneable: Country.find_by_name('Australia')) } let(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.1) } let(:tax_category) { create(:tax_category, tax_rates: [tax_rate]) } - let(:tax_category_untaxed) { create(:tax_category) } let(:coordinator) { create(:distributor_enterprise) } let(:variant) { create(:variant) } @@ -120,7 +119,7 @@ module Spree describe "when enterprise fees have no tax" do before do - enterprise_fee.tax_category = tax_category_untaxed + enterprise_fee.tax_category = nil enterprise_fee.save! order.update_distribution_charge! end From 1e5e009735c67caf6807321ff57b8b75a6deeb86 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 12 Mar 2015 11:29:01 +1100 Subject: [PATCH 35/35] Spree init works when database has not been created, remove duplicate FK from db/schema.rb --- config/initializers/spree.rb | 8 ++++++-- db/schema.rb | 3 --- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb index 5912e0b9ec..9c81da88ef 100644 --- a/config/initializers/spree.rb +++ b/config/initializers/spree.rb @@ -14,8 +14,12 @@ Spree.config do |config| config.checkout_zone = ENV["CHECKOUT_ZONE"] config.address_requires_state = true - country = Spree::Country.find_by_name(ENV["DEFAULT_COUNTRY"]) - config.default_country_id = country.id if country.present? + if Spree::Country.table_exists? + country = Spree::Country.find_by_name(ENV["DEFAULT_COUNTRY"]) + config.default_country_id = country.id if country.present? + else + config.default_country_id = 12 # Australia + end # -- spree_paypal_express # Auto-capture payments. Without this option, payments must be manually captured in the paypal interface. diff --git a/db/schema.rb b/db/schema.rb index a325b69f80..d885130600 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1096,9 +1096,6 @@ ActiveRecord::Schema.define(:version => 20150225232938) do add_foreign_key "enterprise_groups", "spree_addresses", name: "enterprise_groups_address_id_fk", column: "address_id" add_foreign_key "enterprise_groups", "spree_users", name: "enterprise_groups_owner_id_fk", column: "owner_id" - add_foreign_key "enterprise_groups", "spree_addresses", name: "enterprise_groups_address_id_fk", column: "address_id" - add_foreign_key "enterprise_groups", "spree_users", name: "enterprise_groups_owner_id_fk", column: "owner_id" - add_foreign_key "enterprise_groups_enterprises", "enterprise_groups", name: "enterprise_groups_enterprises_enterprise_group_id_fk" add_foreign_key "enterprise_groups_enterprises", "enterprises", name: "enterprise_groups_enterprises_enterprise_id_fk"