Refactor Spree::ShippingRate#display_price

This method used to be a monster. With the new ShippingRateTax Model,
we can now add support for multiple taxes on shipping rates.

The calculation of shipping rate tax amounts now goes through
Spree::TaxRate#compute_amount. This way, we don't have to duplicate
the multiplying by (-1) in the tax calculator and further align how
shipping rate taxes are calculated.

This required a serious spec-rewrite.

core/app/models/spree/shipping_rate.rb

@@ -24,29 +24,26 @@ def calculate_tax_amount
 
     def display_price
       price = display_amount.to_s
-      if tax_rate
-        tax_amount = calculate_tax_amount
-        if tax_amount != 0
-          if tax_rate.included_in_price?
-            if tax_amount > 0
-              amount = "#{display_tax_amount(tax_amount)} #{tax_rate.name}"
-              price += " (#{Spree.t(:incl)} #{amount})"
-            else
-              amount = "#{display_tax_amount(tax_amount * -1)} #{tax_rate.name}"
-              price += " (#{Spree.t(:excl)} #{amount})"
-            end
-          else
-            amount = "#{display_tax_amount(tax_amount)} #{tax_rate.name}"
-            price += " (+ #{amount})"
-          end
-        end
-      end
-      price
+
+      return price if taxes.empty? || amount == 0
+
+      tax_explanations = taxes.map(&:label).join(tax_label_separator)
+
+      Spree.t :display_price_with_explanations,
+               scope: 'shipping_rate.display_price',
+               price: price,
+               explanations: tax_explanations
     end
     alias_method :display_cost, :display_price
 
     def display_tax_amount(tax_amount)
       Spree::Money.new(tax_amount, currency: currency)
     end
+
+    private
+
+    def tax_label_separator
+      Spree.t :tax_label_separator, scope: 'shipping_rate.display_price'
+    end
   end
 end

core/config/locales/en.yml

@@ -1646,6 +1646,10 @@ en:
     shipping_method: Shipping Method
     shipping_methods: Shipping Methods
     shipping_price_sack: Price sack
+    shipping_rate:
+      display_price:
+        display_price_with_explanations: "%{price} (%{explanations})"
+        tax_label_separator: ", "
     shipping_rate_tax:
       label:
         sales_tax: "+ %{amount} %{tax_rate_name}"

core/spec/models/spree/shipping_rate_spec.rb

@@ -4,10 +4,12 @@
 
 describe Spree::ShippingRate, type: :model do
   let(:address) { create(:address) }
-  let(:order) { create(:order, ship_address: address) }
-  let(:tax_category) { create(:tax_category) }
-  let(:shipment) { create(:shipment) }
+  let(:foreign_address) { create :address, country_iso_code: "DE" }
+  let(:order) { create :order, ship_address: address }
+  let(:shipment) { create(:shipment, order: order) }
   let(:shipping_method) { create(:shipping_method, tax_category: tax_category) }
+  let(:tax_category) { create :tax_category }
+
   subject(:shipping_rate) do
     Spree::ShippingRate.new(
       shipment: shipment,
@@ -16,71 +18,88 @@
     )
   end
 
-  it { is_expected.to respond_to(:taxes) }
-
   context "#display_price" do
-    context "when tax included in price" do
-      context "when the tax rate is from the default zone" do
-        before { shipment.order.update_attributes!(ship_address_id: nil) }
-        let!(:zone) { create(:zone, default_tax: true, countries: [address.country]) }
-        let(:tax_rate) do
-          create(:tax_rate,
-            name: "VAT",
-            amount: 0.1,
-            included_in_price: true,
-            zone: zone,
-            tax_category: tax_category)
-        end
+    let!(:default_zone) { create :zone, countries: [address.country], default_tax: true }
+    let!(:other_zone) { create :zone, countries: [foreign_address.country] }
 
-        before { shipping_rate.tax_rate = tax_rate }
+    before do
+      allow(order).to receive(:tax_address).and_return(order_address)
+    end
 
-        it "shows correct tax amount" do
-          expect(shipping_rate.display_price.to_s).to eq("$10.00 (incl. $0.91 #{tax_rate.name})")
-        end
+    context 'with one included tax rate' do
+      let!(:tax_rate) do
+        create :tax_rate,
+        included_in_price: true,
+        name: "VAT",
+        zone: default_zone,
+        tax_category: tax_category
+      end
 
-        context "when cost is zero" do
-          before do
-            shipping_rate.cost = 0
-          end
+      let(:order_address) { address }
 
-          it "shows no tax amount" do
-            expect(shipping_rate.display_price.to_s).to eq("$0.00")
-          end
-        end
+      before do
+        Spree::Tax::ShippingRateTaxer.new.tax(shipping_rate)
+      end
+
+      it "shows correct tax amount" do
+        expect(shipping_rate.display_price.to_s).to eq("$10.00 (incl. $0.91 #{tax_rate.name})")
       end
 
-      context "when shipping to a non-default zone" do
-        let!(:zone) { create(:zone, default_tax: true, countries: [address.country]) }
-        let!(:address) { create(:address, country_iso_code: "DE") }
-        let(:tax_rate) do
-          create(:tax_rate,
-            name: "VAT",
-            amount: 0.1,
-            included_in_price: true,
-            zone: zone,
-            tax_category: tax_category)
+      context "when cost is zero" do
+        before do
+          shipping_rate.cost = 0
         end
-        before { shipping_rate.tax_rate = tax_rate }
 
-        it "shows correct tax amount" do
-          expect(shipping_rate.display_price.to_s).to eq("$10.00 (excl. $0.91 #{tax_rate.name})")
+        it "shows no tax amount" do
+          expect(shipping_rate.display_price.to_s).to eq("$0.00")
         end
+      end
+    end
 
-        context "when cost is zero" do
-          before do
-            shipping_rate.cost = 0
-          end
+    context 'with one tax rate that will be refunded' do
+      let!(:tax_rate) do
+        create :tax_rate,
+        included_in_price: true,
+        name: "VAT",
+        zone: default_zone,
+        tax_category: tax_category
+      end
+
+      let(:order_address) { foreign_address }
 
-          it "shows no tax amount" do
-            expect(shipping_rate.display_price.to_s).to eq("$0.00")
-          end
+      before do
+        Spree::Tax::ShippingRateTaxer.new.tax(shipping_rate)
+      end
+
+      it "shows correct tax amount" do
+        expect(shipping_rate.display_price.to_s).to eq("$10.00 (excl. $0.91 #{tax_rate.name})")
+      end
+
+      context "when cost is zero" do
+        before do
+          shipping_rate.cost = 0
+        end
+
+        it "shows no tax amount" do
+          expect(shipping_rate.display_price.to_s).to eq("$0.00")
         end
       end
     end
 
-    context "when tax is additional to price" do
-      let(:tax_rate) { create(:tax_rate, name: "Sales Tax", amount: 0.1) }
-      before { shipping_rate.tax_rate = tax_rate }
+    context 'with one additional tax rate' do
+      let!(:tax_rate) do
+        create :tax_rate,
+        included_in_price: false,
+        name: "Sales Tax",
+        zone: default_zone,
+        tax_category: tax_category
+      end
+
+      let(:order_address) { address }
+
+      before do
+        Spree::Tax::ShippingRateTaxer.new.tax(shipping_rate)
+      end
 
       it "shows correct tax amount" do
         expect(shipping_rate.display_price.to_s).to eq("$10.00 (+ $1.00 #{tax_rate.name})")
@@ -97,15 +116,42 @@
       end
     end
 
-    context "when the currency is JPY" do
-      let(:shipping_rate) {
-        shipping_rate = Spree::ShippingRate.new(cost: 205)
-        allow(shipping_rate).to receive_messages(currency: "JPY")
-        shipping_rate
-      }
+    context 'with two additional tax rates' do
+      let!(:tax_rate) do
+        create :tax_rate,
+        included_in_price: false,
+        name: "Sales Tax",
+        zone: default_zone,
+        tax_category: tax_category
+      end
+
+      let!(:other_tax_rate) do
+        create :tax_rate,
+        included_in_price: false,
+        name: "Other Sales Tax",
+        zone: default_zone,
+        tax_category: tax_category,
+        amount: 0.05
+      end
+
+      let(:order_address) { address }
 
-      it "displays the price in yen" do
-        expect(shipping_rate.display_price.to_s).to eq("¥205")
+      before do
+        Spree::Tax::ShippingRateTaxer.new.tax(shipping_rate)
+      end
+
+      it "shows correct tax amount" do
+        expect(shipping_rate.display_price.to_s).to eq("$10.00 (+ $1.00 Sales Tax, + $0.50 Other Sales Tax)")
+      end
+
+      context "when cost is zero" do
+        before do
+          shipping_rate.cost = 0
+        end
+
+        it "shows no tax amount" do
+          expect(shipping_rate.display_price.to_s).to eq("$0.00")
+        end
       end
     end
   end

core/spec/models/spree/tax/taxation_integration_spec.rb

@@ -150,8 +150,7 @@
         end
 
         it 'has a shipping rate that correctly reflects the shipment' do
-          pending 'since no tax created, no correct display price'
-          expect(shipping_rate.display_price).to eq("$8.00 (incl. $0.52 German VAT)")
+          expect(shipping_rate.display_price).to eq("$8.00 (incl. $0.52 German reduced VAT)")
         end
       end
 
@@ -185,7 +184,6 @@
         end
 
         it 'has a shipping rate that correctly reflects the shipment' do
-          pending 'since no tax created, no correct display price'
           expect(shipping_rate.display_price).to eq("$16.00 (incl. $2.55 German VAT)")
         end
       end
@@ -266,8 +264,7 @@
         end
 
         it 'has a shipping rate that correctly reflects the shipment' do
-          pending 'since no tax created, no correct display price'
-          expect(shipping_rate.display_price).to eq("$8.00 (incl. $0.52 German VAT)")
+          expect(shipping_rate.display_price).to eq("$8.00 (incl. $0.52 German reduced VAT)")
         end
       end
 
@@ -305,7 +302,6 @@
         end
 
         it 'has a shipping rate that correctly reflects the shipment' do
-          pending 'since no tax created, no correct display price'
           expect(shipping_rate.display_price).to eq("$16.00 (incl. $2.55 German VAT)")
         end
       end
@@ -349,7 +345,7 @@
         end
 
         it 'has a shipping rate that correctly reflects the shipment' do
-          pending 'since no tax created, no correct display price'
+          pending 'But the shipping rate is not adjusted'
           expect(shipping_rate.display_price).to eq("$2.08 (incl. $0.40 Romanian VAT)")
         end
       end
@@ -675,8 +671,9 @@
         end
 
         it 'has a shipping rate that correctly reflects the shipment' do
-          pending 'since no tax created, no correct display price'
-          expect(shipping_rate.display_price).to eq("$8.00 (+ $0.80 Federal Sales Tax, + $0.40 New York Sales Tax)")
+          expect(shipping_rate.display_price).to include("$8.00")
+          expect(shipping_rate.display_price).to include("+ $0.80 Federal Sales Tax")
+          expect(shipping_rate.display_price).to include("+ $0.40 New York Sales Tax")
         end
       end
 
@@ -770,7 +767,6 @@
         end
 
         it 'has a shipping rate that correctly reflects the shipment' do
-          pending 'since no tax created, no correct display price'
           expect(shipping_rate.display_price).to eq("$2.00 (+ $0.40 Federal Sales Tax)")
         end
       end