Merge pull request #3416 from nebulab/kennyadsl/promo-fixes

Several small refactors to promotions code

core/app/models/spree/shipping_calculator.rb

@@ -2,10 +2,6 @@
 
 module Spree
   class ShippingCalculator < Calculator
-    def compute_shipment(_shipment)
-      raise NotImplementedError, "Please implement 'compute_shipment(shipment)' in your calculator: #{self.class.name}"
-    end
-
     def compute_package(_package)
       raise NotImplementedError, "Please implement 'compute_package(package)' in your calculator: #{self.class.name}"
     end

core/spec/models/spree/calculator/shipping/flat_percent_item_total_spec.rb

@@ -25,7 +25,7 @@ module Calculator::Shipping
         )
       end
 
-      subject { FlatPercentItemTotal.new(preferred_flat_percent: 10) }
+      subject { described_class.new(preferred_flat_percent: 10) }
 
       it "should round result correctly" do
         expect(subject.compute(package)).to eq(4.04)

core/spec/models/spree/calculator/shipping/flat_rate_spec.rb

@@ -6,7 +6,7 @@
 module Spree
   module Calculator::Shipping
     RSpec.describe FlatRate, type: :model do
-      subject { Calculator::Shipping::FlatRate.new(preferred_amount: 4.00) }
+      subject { described_class.new(preferred_amount: 4.00) }
 
       it_behaves_like 'a calculator with a description'
 

core/spec/models/spree/calculator/shipping/flexi_rate_spec.rb

@@ -15,7 +15,7 @@ module Calculator::Shipping
         build(:stock_package, variants_contents: { variant1 => 4, variant2 => 6 })
       end
 
-      let(:subject) { FlexiRate.new }
+      let(:subject) { described_class.new }
 
       context "compute" do
         it "should compute amount correctly when all fees are 0" do
@@ -46,7 +46,7 @@ module Calculator::Shipping
         end
 
         it "should allow creation of new object with all the attributes" do
-          FlexiRate.new(preferred_first_item: 1,
+          described_class.new(preferred_first_item: 1,
                         preferred_additional_item: 1,
                         preferred_max_items: 1)
         end

core/spec/models/spree/calculator/shipping/per_item_spec.rb

@@ -15,7 +15,7 @@ module Calculator::Shipping
         build(:stock_package, variants_contents: { variant1 => 5, variant2 => 3 })
       end
 
-      subject { PerItem.new(preferred_amount: 10) }
+      subject { described_class.new(preferred_amount: 10) }
 
       it "correctly calculates per item shipping" do
         expect(subject.compute(package).to_f).to eq(80) # 5 x 10 + 3 x 10

core/spec/models/spree/calculator/shipping/price_sack_spec.rb

@@ -5,7 +5,7 @@
 
 RSpec.describe Spree::Calculator::Shipping::PriceSack, type: :model do
   let(:calculator) do
-    calculator = Spree::Calculator::PriceSack.new
+    calculator = described_class.new
     calculator.preferred_minimal_amount = 5
     calculator.preferred_normal_amount = 10
     calculator.preferred_discount_amount = 1
@@ -14,22 +14,31 @@
 
   it_behaves_like 'a calculator with a description'
 
-  let(:order) { stub_model(Spree::Order) }
-  let(:shipment) { stub_model(Spree::Shipment, amount: 10) }
+  describe '#compute' do
+    subject { calculator.compute(package) }
+    let(:package) { build(:stock_package, variants_contents: { build(:variant) => 1 }) }
 
-  # Regression test for https://github.com/spree/spree/issues/714 and https://github.com/spree/spree/issues/739
-  it "computes with an order object" do
-    calculator.compute(order)
-  end
+    before do
+      # This hack is due to our factories not being so smart to understand
+      # that they should create line items with the price of the associated
+      # variant by default.
+      allow_any_instance_of(Spree::Stock::ContentItem).to receive(:price) { amount }
+    end
 
-  # Regression test for https://github.com/spree/spree/issues/1156
-  it "computes with a shipment object" do
-    calculator.compute(shipment)
-  end
+    context 'when price < minimal amount' do
+      let(:amount) { 2 }
+
+      it "returns the discounted amount" do
+        expect(subject).to eq(10)
+      end
+    end
+
+    context 'when price > minimal amount' do
+      let(:amount) { 6 }
 
-  # Regression test for https://github.com/spree/spree/issues/2055
-  it "computes the correct amount" do
-    expect(calculator.compute(2)).to eq(calculator.preferred_normal_amount)
-    expect(calculator.compute(6)).to eq(calculator.preferred_discount_amount)
+      it "returns the discounted amount" do
+        expect(subject).to eq(1)
+      end
+    end
   end
 end

core/spec/models/spree/returns_calculator_spec.rb

@@ -5,9 +5,9 @@
 module Spree
   RSpec.describe ReturnsCalculator, type: :model do
     let(:return_item) { build(:return_item) }
-    subject { ReturnsCalculator.new }
+    subject { described_class.new }
 
-    it 'compute_shipment must be overridden' do
+    it 'compute must be overridden' do
       expect {
         subject.compute(return_item)
       }.to raise_error NotImplementedError

core/spec/models/spree/shipping_calculator_spec.rb

@@ -21,25 +21,13 @@ module Spree
       )
     end
 
-    subject { ShippingCalculator.new }
-
-    it 'computes with a shipment' do
-      shipment = mock_model(Spree::Shipment)
-      expect(subject).to receive(:compute_shipment).with(shipment)
-      subject.compute(shipment)
-    end
+    subject { described_class.new }
 
     it 'computes with a package' do
       expect(subject).to receive(:compute_package).with(package)
       subject.compute(package)
     end
 
-    it 'compute_shipment must be overridden' do
-      expect {
-        subject.compute_shipment(shipment)
-      }.to raise_error NameError
-    end
-
     it 'compute_package must be overridden' do
       expect {
         subject.compute_package(package)