From 10d5260c477fb74de6a854bdae840c497a7ad209 Mon Sep 17 00:00:00 2001 From: Matteo Latini Date: Tue, 23 May 2017 14:20:09 +0100 Subject: [PATCH] Add validity period for Spree::TaxRate This code adds a starts_at and expires_at datetime field to Spree::TaxRate so that tax rates can be scoped in time. Null values will be treated as valid so that a tax rate is valid by default and you can just ignore this feature if you don't need it. The Spree::Calculator::DefaultTax will always return 0 in case the tax rate is not valid (either not started or expired). Ref. #1857 --- .../spree/admin/tax_rates/_form.html.erb | 14 ++++ .../spree/admin/tax_rates/index.html.erb | 5 +- .../models/spree/calculator/default_tax.rb | 2 + core/app/models/spree/tax_rate.rb | 5 ++ core/config/locales/en.yml | 5 ++ ...170522143442_add_time_range_to_tax_rate.rb | 6 ++ .../spree/calculator/default_tax_spec.rb | 73 ++++++++++++++++++- core/spec/models/spree/tax_rate_spec.rb | 70 ++++++++++++++++++ 8 files changed, 178 insertions(+), 2 deletions(-) create mode 100644 core/db/migrate/20170522143442_add_time_range_to_tax_rate.rb diff --git a/backend/app/views/spree/admin/tax_rates/_form.html.erb b/backend/app/views/spree/admin/tax_rates/_form.html.erb index b4afb5d125a..379af3ca9ea 100644 --- a/backend/app/views/spree/admin/tax_rates/_form.html.erb +++ b/backend/app/views/spree/admin/tax_rates/_form.html.erb @@ -32,6 +32,20 @@ <%= f.check_box :show_rate_in_label %> <%= f.label :show_rate_in_label, Spree.t(:show_rate_in_label) %> + +
+ <%= label_tag :validity_period, Spree.t(:validity_period) %> + <%= admin_hint Spree.t(:validity_period), Spree.t(:validity_period, scope: [:hints, "spree/tax_rate"]) %> +
+ <%= f.text_field :starts_at, class: 'datepicker form-control datepicker-from', value: datepicker_field_value(f.object.starts_at), placeholder: Spree::TaxRate.human_attribute_name(:starts_at) %> + + + + + + <%= f.text_field :expires_at, class: 'datepicker form-control datepicker-to', value: datepicker_field_value(f.object.expires_at), placeholder: Spree::TaxRate.human_attribute_name(:expires_at) %> +
+
diff --git a/backend/app/views/spree/admin/tax_rates/index.html.erb b/backend/app/views/spree/admin/tax_rates/index.html.erb index 17a455c0b31..7481ce8dead 100644 --- a/backend/app/views/spree/admin/tax_rates/index.html.erb +++ b/backend/app/views/spree/admin/tax_rates/index.html.erb @@ -20,7 +20,8 @@ - + + @@ -33,6 +34,7 @@ <%= Spree::TaxRate.human_attribute_name(:amount) %> <%= Spree::TaxRate.human_attribute_name(:included_in_price) %> <%= Spree::TaxRate.human_attribute_name(:show_rate_in_label) %> + <%= Spree::TaxRate.human_attribute_name(:expires_at) %> <%= Spree::Calculator.model_name.human %> @@ -52,6 +54,7 @@ <%=tax_rate.amount %> <%=tax_rate.included_in_price? ? Spree.t(:say_yes) : Spree.t(:say_no) %> <%=tax_rate.show_rate_in_label? ? Spree.t(:say_yes) : Spree.t(:say_no) %> + <%=tax_rate.expires_at.to_date.to_s(:short_date) if tax_rate.expires_at %> <%=tax_rate.calculator.to_s %> <% if can?(:update, tax_rate) %> diff --git a/core/app/models/spree/calculator/default_tax.rb b/core/app/models/spree/calculator/default_tax.rb index d232ef18f4b..89f28e5b248 100644 --- a/core/app/models/spree/calculator/default_tax.rb +++ b/core/app/models/spree/calculator/default_tax.rb @@ -8,6 +8,7 @@ class Calculator::DefaultTax < Calculator # Orders created before Spree 2.1 had tax adjustments applied to the order, as a whole. # Orders created with Spree 2.2 and after, have them applied to the line items individually. def compute_order(order) + return 0 unless rate.active? matched_line_items = order.line_items.select do |line_item| rate.tax_categories.include?(line_item.tax_category) end @@ -23,6 +24,7 @@ def compute_order(order) # When it comes to computing shipments or line items: same same. def compute_item(item) + return 0 unless rate.active? if rate.included_in_price deduced_total_by_rate(item, rate) else diff --git a/core/app/models/spree/tax_rate.rb b/core/app/models/spree/tax_rate.rb index 924cb7b02a8..3069444cbd9 100644 --- a/core/app/models/spree/tax_rate.rb +++ b/core/app/models/spree/tax_rate.rb @@ -98,6 +98,11 @@ def compute_amount(item) calculator.compute(item) end + def active? + (starts_at.nil? || starts_at < Time.current) && + (expires_at.nil? || expires_at > Time.current) + end + def adjustment_label(amount) Spree.t( translation_key(amount), diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index ccdc31968b8..e59a5cff53f 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -349,6 +349,8 @@ en: included_in_price: Included In Price name: Name show_rate_in_label: Show Rate In Label + starts_at: Start date + expires_at: Expiration date spree/taxon: description: Description icon: Icon @@ -1271,6 +1273,8 @@ en: deleted: "Deleted Variant" deleted_explanation: "This variant was deleted on %{date}." deleted_explanation_with_replacement: "This variant was deleted on %{date}. It has since been replaced by another with the same SKU." + spree/tax_rate: + validity_period: "This determines the validity period within which the tax rate is valid and will be applied to eligible items.
If no start date value is specified, the tax rate will be immediately available.
If no expiration date value is specified, the tax rate will never expire" failed_payment_attempts: Failed Payment Attempts failure: Failure filename: Filename @@ -2031,6 +2035,7 @@ en: is_too_large: is too large -- stock on hand cannot cover requested quantity! must_be_int: must be an integer must_be_non_negative: must be a non-negative value + validity_period: Validity Period value: Value variant: Variant variant_placeholder: Choose a variant diff --git a/core/db/migrate/20170522143442_add_time_range_to_tax_rate.rb b/core/db/migrate/20170522143442_add_time_range_to_tax_rate.rb new file mode 100644 index 00000000000..1b2ba172e20 --- /dev/null +++ b/core/db/migrate/20170522143442_add_time_range_to_tax_rate.rb @@ -0,0 +1,6 @@ +class AddTimeRangeToTaxRate < ActiveRecord::Migration[5.0] + def change + add_column :spree_tax_rates, :starts_at, :datetime + add_column :spree_tax_rates, :expires_at, :datetime + end +end diff --git a/core/spec/models/spree/calculator/default_tax_spec.rb b/core/spec/models/spree/calculator/default_tax_spec.rb index 2aa8f46f2a1..ebae15e889c 100644 --- a/core/spec/models/spree/calculator/default_tax_spec.rb +++ b/core/spec/models/spree/calculator/default_tax_spec.rb @@ -6,7 +6,13 @@ let!(:zone) { create(:zone, name: "Country Zone", default_tax: default_tax, countries: [tax_rate_country]) } let(:tax_rate_country) { address.country } let(:tax_category) { create(:tax_category) } - let!(:rate) { create(:tax_rate, tax_categories: [tax_category], amount: 0.05, included_in_price: included_in_price, zone: zone) } + let(:starts_at) { nil } + let(:expires_at) { nil } + let!(:rate) do + create(:tax_rate, tax_categories: [tax_category], amount: 0.05, + included_in_price: included_in_price, zone: zone, + starts_at: starts_at, expires_at: expires_at) + end let(:included_in_price) { false } let(:default_tax) { false } subject(:calculator) { Spree::Calculator::DefaultTax.new(calculable: rate ) } @@ -32,6 +38,15 @@ it "should be equal to the sum of the item totals * rate" do expect(calculator.compute(order)).to eq(3) end + + context "when rate is not in its validity period" do + let(:starts_at) { 1.day.from_now } + let(:expires_at) { 2.days.from_now } + + it "should be 0" do + expect(calculator.compute(order)).to eq(0) + end + end end context "when no line items match the tax category" do @@ -52,6 +67,15 @@ expect(calculator.compute(order)).to eq(1.5) end + context "when rate is not in its validity period" do + let(:starts_at) { 1.day.from_now } + let(:expires_at) { 2.days.from_now } + + it "should be 0" do + expect(calculator.compute(order)).to eq(0) + end + end + context "correctly rounds to within two decimal places" do let(:line_item_one_options) { { price: 10.333, quantity: 1 } } @@ -73,6 +97,15 @@ expect(calculator.compute(order).to_f).to eql 2.86 end + context "when rate is not in its validity period" do + let(:starts_at) { 1.day.from_now } + let(:expires_at) { 2.days.from_now } + + it "should be 0" do + expect(calculator.compute(order)).to eq(0) + end + end + context "when the order's tax address is outside the default VAT zone" do let(:default_tax) { true } let(:default_vat_country) { create(:country, iso: "DE") } @@ -86,6 +119,17 @@ expect(subject.compute(order)).to eq(-2.86) end end + + context "when rate is not in its validity period" do + let(:starts_at) { 1.day.from_now } + let(:expires_at) { 2.days.from_now } + + it "should be 0" do + Spree::Deprecation.silence do + expect(calculator.compute(order)).to eq(0) + end + end + end end end end @@ -103,6 +147,15 @@ expect(calculator.compute(item)).to eql 1.43 end + context "when rate is not in its validity period" do + let(:starts_at) { 1.day.from_now } + let(:expires_at) { 2.days.from_now } + + it "should be 0" do + expect(calculator.compute(item)).to eq(0) + end + end + context "when line item is discounted" do let(:promo_total) { -1 } @@ -135,12 +188,30 @@ it "should be equal to the item's pre-tax total * rate" do expect(calculator.compute(item)).to eq(1.45) end + + context "when rate is not in its validity period" do + let(:starts_at) { 1.day.from_now } + let(:expires_at) { 2.days.from_now } + + it "should be 0" do + expect(calculator.compute(item)).to eq(0) + end + end end context "when the variant matches the tax category" do it "should be equal to the item pre-tax total * rate" do expect(calculator.compute(item)).to eq(1.50) end + + context "when rate is not in its validity period" do + let(:starts_at) { 1.day.from_now } + let(:expires_at) { 2.days.from_now } + + it "should be 0" do + expect(calculator.compute(item)).to eq(0) + end + end end end end diff --git a/core/spec/models/spree/tax_rate_spec.rb b/core/spec/models/spree/tax_rate_spec.rb index 9bdb781687a..73d543a2b26 100644 --- a/core/spec/models/spree/tax_rate_spec.rb +++ b/core/spec/models/spree/tax_rate_spec.rb @@ -264,4 +264,74 @@ end end end + + describe "#active?" do + subject(:rate) { create(:tax_rate, validity).active? } + + context "when validity is not set" do + let(:validity) { {} } + + it { is_expected.to eq(true) } + end + + context "when starts_at is set" do + context "now" do + let(:validity) { { starts_at: DateTime.now } } + + it { is_expected.to eq(true) } + end + + context "in the past" do + let(:validity) { { starts_at: 1.day.ago } } + + it { is_expected.to eq(true) } + end + + context "in the future" do + let(:validity) { { starts_at: 1.day.from_now } } + + it { is_expected.to eq(false) } + end + end + + context "when expires_at is set" do + context "now" do + let(:validity) { { expires_at: DateTime.now } } + + it { is_expected.to eq(false) } + end + + context "in the past" do + let(:validity) { { expires_at: 1.day.ago } } + + it { is_expected.to eq(false) } + end + + context "in the future" do + let(:validity) { { expires_at: 1.day.from_now } } + + it { is_expected.to eq(true) } + end + end + + context "when starts_at and expires_at are set" do + context "so that today is in range" do + let(:validity) { { starts_at: 1.day.ago, expires_at: 1.day.from_now } } + + it { is_expected.to eq(true) } + end + + context "both in the past" do + let(:validity) { { starts_at: 2.days.ago, expires_at: 1.day.ago } } + + it { is_expected.to eq(false) } + end + + context "both in the future" do + let(:validity) { { starts_at: 1.day.from_now, expires_at: 2.days.from_now } } + + it { is_expected.to eq(false) } + end + end + end end