From 843bca9ea352c9c7432793c150353d108f169a56 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Mon, 26 Mar 2018 14:01:30 +0200 Subject: [PATCH 1/6] Updated calculation for flexible rate --- .../spree/calculator/flexi_rate_decorator.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models/spree/calculator/flexi_rate_decorator.rb b/app/models/spree/calculator/flexi_rate_decorator.rb index 04ad709cbe..ff8d28f87c 100644 --- a/app/models/spree/calculator/flexi_rate_decorator.rb +++ b/app/models/spree/calculator/flexi_rate_decorator.rb @@ -5,7 +5,7 @@ module Spree extend Spree::LocalizedNumber localize_number :preferred_first_item, - :preferred_additional_item + :preferred_additional_item def self.description I18n.t(:flexible_rate) @@ -14,13 +14,13 @@ module Spree def compute(object) sum = 0 max = self.preferred_max_items.to_i - items_count = line_items_for(object).map(&:quantity).sum - items_count.times do |i| - # check max value to avoid divide by 0 errors - if (max == 0 && i == 0) || (max > 0) && (i % max == 0) - sum += self.preferred_first_item.to_f - else - sum += self.preferred_additional_item.to_f + items_count = line_items_for(object).map(&:quantity).sum + # check max value to avoid divide by 0 errors + unless max == 0 + if items_count > max + sum += (max - 1) * self.preferred_additional_item.to_f + self.preferred_first_item.to_f + elsif items_count <= max + sum += (items_count - 1) * self.preferred_additional_item.to_f + self.preferred_first_item.to_f end end From f40bd9cfcb7918366f71e9215f3b4c8b68445c8d Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Mon, 26 Mar 2018 14:03:09 +0200 Subject: [PATCH 2/6] Fixing wrong incrementation --- app/models/spree/calculator/flexi_rate_decorator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/spree/calculator/flexi_rate_decorator.rb b/app/models/spree/calculator/flexi_rate_decorator.rb index ff8d28f87c..d2b0457b44 100644 --- a/app/models/spree/calculator/flexi_rate_decorator.rb +++ b/app/models/spree/calculator/flexi_rate_decorator.rb @@ -5,7 +5,7 @@ module Spree extend Spree::LocalizedNumber localize_number :preferred_first_item, - :preferred_additional_item + :preferred_additional_item def self.description I18n.t(:flexible_rate) @@ -14,7 +14,7 @@ module Spree def compute(object) sum = 0 max = self.preferred_max_items.to_i - items_count = line_items_for(object).map(&:quantity).sum + items_count = line_items_for(object).map(&:quantity).sum # check max value to avoid divide by 0 errors unless max == 0 if items_count > max From a71b650f892c9c39cdd4c70051b16f197f30d41e Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Mon, 26 Mar 2018 15:58:31 +0200 Subject: [PATCH 3/6] Added tests --- spec/models/spree/calculator/flexi_rate_spec.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/spec/models/spree/calculator/flexi_rate_spec.rb b/spec/models/spree/calculator/flexi_rate_spec.rb index 49c2371e13..8b00d60b2d 100644 --- a/spec/models/spree/calculator/flexi_rate_spec.rb +++ b/spec/models/spree/calculator/flexi_rate_spec.rb @@ -1,13 +1,18 @@ require 'spec_helper' describe Spree::Calculator::FlexiRate do - let(:calculator) { Spree::Calculator::FlexiRate.new } + let(:calculator) { Spree::Calculator::FlexiRate.new(preferred_first_item: 2, preferred_additional_item: 1) } let(:line_item) { instance_double(Spree::LineItem, amount: 10, quantity: 4) } describe "computing for a single line item" do - it "returns the first item rate" do - calculator.stub preferred_first_item: 1.0 - calculator.compute(line_item).round(2).should == 1.0 + it "returns the first item rate when above max" do + calculator.stub preferred_max_items: 3.0 + calculator.compute(line_item).round(2).should == 4.0 + end + + it "returns the first item rate when below max" do + calculator.stub preferred_max_items: 5.0 + calculator.compute(line_item).round(2).should == 5.0 end end From 68ca352510f611d5ae35b4dff8b248ab88e44c54 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Mon, 26 Mar 2018 16:27:27 +0200 Subject: [PATCH 4/6] Use rspec expect syntax --- spec/models/spree/calculator/flexi_rate_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/spree/calculator/flexi_rate_spec.rb b/spec/models/spree/calculator/flexi_rate_spec.rb index 8b00d60b2d..d85e778df6 100644 --- a/spec/models/spree/calculator/flexi_rate_spec.rb +++ b/spec/models/spree/calculator/flexi_rate_spec.rb @@ -7,12 +7,12 @@ describe Spree::Calculator::FlexiRate do describe "computing for a single line item" do it "returns the first item rate when above max" do calculator.stub preferred_max_items: 3.0 - calculator.compute(line_item).round(2).should == 4.0 + expect(calculator.compute(line_item).round(2)).to eq(4.0) end it "returns the first item rate when below max" do calculator.stub preferred_max_items: 5.0 - calculator.compute(line_item).round(2).should == 5.0 + expect(calculator.compute(line_item).round(2)).to eq(5.0) end end From bd97d08653f81cde24b5f679086601324e117ec3 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Mon, 26 Mar 2018 17:34:32 +0200 Subject: [PATCH 5/6] Remove stub after test and create two distinct calculators --- spec/models/spree/calculator/flexi_rate_spec.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/models/spree/calculator/flexi_rate_spec.rb b/spec/models/spree/calculator/flexi_rate_spec.rb index d85e778df6..a34dd15b6b 100644 --- a/spec/models/spree/calculator/flexi_rate_spec.rb +++ b/spec/models/spree/calculator/flexi_rate_spec.rb @@ -1,18 +1,18 @@ require 'spec_helper' describe Spree::Calculator::FlexiRate do - let(:calculator) { Spree::Calculator::FlexiRate.new(preferred_first_item: 2, preferred_additional_item: 1) } + let(:calculator_first) { Spree::Calculator::FlexiRate.new(preferred_first_item: 2, preferred_additional_item: 1, preferred_max_items: 3.0) } + let(:calculator_second) { Spree::Calculator::FlexiRate.new(preferred_first_item: 2, preferred_additional_item: 1, preferred_max_items: 5.0) } + let(:line_item) { instance_double(Spree::LineItem, amount: 10, quantity: 4) } describe "computing for a single line item" do - it "returns the first item rate when above max" do - calculator.stub preferred_max_items: 3.0 - expect(calculator.compute(line_item).round(2)).to eq(4.0) + it "returns the first item rate when quantity is above max" do + expect(calculator_first.compute(line_item).round(2)).to eq(4.0) end - it "returns the first item rate when below max" do - calculator.stub preferred_max_items: 5.0 - expect(calculator.compute(line_item).round(2)).to eq(5.0) + it "returns the first item rate when quantity is below max" do + expect(calculator_second.compute(line_item).round(2)).to eq(5.0) end end From 2a942da1604c9b0fd8a6120f1b15e918b3710589 Mon Sep 17 00:00:00 2001 From: Hugo Daniel Date: Wed, 28 Mar 2018 15:43:10 +0200 Subject: [PATCH 6/6] Use rspec context --- .../spree/calculator/flexi_rate_spec.rb | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/spec/models/spree/calculator/flexi_rate_spec.rb b/spec/models/spree/calculator/flexi_rate_spec.rb index a34dd15b6b..6659da86fd 100644 --- a/spec/models/spree/calculator/flexi_rate_spec.rb +++ b/spec/models/spree/calculator/flexi_rate_spec.rb @@ -1,19 +1,29 @@ require 'spec_helper' describe Spree::Calculator::FlexiRate do - let(:calculator_first) { Spree::Calculator::FlexiRate.new(preferred_first_item: 2, preferred_additional_item: 1, preferred_max_items: 3.0) } - let(:calculator_second) { Spree::Calculator::FlexiRate.new(preferred_first_item: 2, preferred_additional_item: 1, preferred_max_items: 5.0) } + let(:line_item) { instance_double(Spree::LineItem, amount: 10, quantity: quantity) } + let(:calculator) do + Spree::Calculator::FlexiRate.new( + preferred_first_item: 2, + preferred_additional_item: 1, + preferred_max_items: 3 + ) + end - let(:line_item) { instance_double(Spree::LineItem, amount: 10, quantity: 4) } + context 'when nb of items ordered is above preferred max' do + let(:quantity) { 4.0 } - describe "computing for a single line item" do - it "returns the first item rate when quantity is above max" do - expect(calculator_first.compute(line_item).round(2)).to eq(4.0) - end + it "returns the first item rate" do + expect(calculator.compute(line_item).round(2)).to eq(4.0) + end + end - it "returns the first item rate when quantity is below max" do - expect(calculator_second.compute(line_item).round(2)).to eq(5.0) - end + context 'when nb of items ordered is below preferred max' do + let(:quantity) { 2.0 } + + it "returns the first item rate" do + expect(calculator.compute(line_item).round(2)).to eq(3.0) + end end it "allows creation of new object with all the attributes" do