From 7035d572678ccd2efe6ae74e2db2cfcf56f6ee1c Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 3 Apr 2019 11:07:22 +1100
Subject: [PATCH] Better helper naming and spec cover

---
 app/helpers/admin/orders_helper.rb           | 14 +++++++----
 app/views/spree/admin/orders/_form.html.haml |  2 +-
 spec/helpers/admin/orders_helper_spec.rb     | 25 ++++++++++++++++++++
 3 files changed, 36 insertions(+), 5 deletions(-)
 create mode 100644 spec/helpers/admin/orders_helper_spec.rb

diff --git a/app/helpers/admin/orders_helper.rb b/app/helpers/admin/orders_helper.rb
index bd64bbe1c4..9549f538c2 100644
--- a/app/helpers/admin/orders_helper.rb
+++ b/app/helpers/admin/orders_helper.rb
@@ -1,13 +1,19 @@
 module Admin
   module OrdersHelper
-    def order_adjustments(order)
+    # Adjustments to display under "Order adjustments".
+    #
+    # We exclude shipping method adjustments because they are displayed in a
+    # separate table together with the order line items.
+    #
+    # We also exclude tax rate adjustment with zero value.
+    def order_adjustments_for_display(order)
       order.adjustments.eligible.select do |adjustment|
         type = adjustment.originator_type
 
-        is_shipping_method_fee = (type == 'Spree::ShippingMethod')
-        is_zero_tax_rate = (type == 'Spree::TaxRate' && adjustment.amount.zero?)
+        is_shipping_method_adjustment = (type == 'Spree::ShippingMethod')
+        is_zero_tax_rate_adjustment = (type == 'Spree::TaxRate' && adjustment.amount.zero?)
 
-        !is_shipping_method_fee && !is_zero_tax_rate
+        !is_shipping_method_adjustment && !is_zero_tax_rate_adjustment
       end
     end
   end
diff --git a/app/views/spree/admin/orders/_form.html.haml b/app/views/spree/admin/orders/_form.html.haml
index 96ba6ea89b..a14b308ba5 100644
--- a/app/views/spree/admin/orders/_form.html.haml
+++ b/app/views/spree/admin/orders/_form.html.haml
@@ -5,7 +5,7 @@
   = render :partial => "spree/admin/orders/shipment", :collection => @order.shipments, :locals => { :order => order }
 
   = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => @order.price_adjustments, :title => t(".line_item_adjustments")}
-  = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => order_adjustments(@order), :title => t(".order_adjustments")}
+  = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => order_adjustments_for_display(@order), :title => t(".order_adjustments")}
 
   - if order.line_items.exists?
     %fieldset#order-total.no-border-bottom{"data-hook" => "order_details_total"}
diff --git a/spec/helpers/admin/orders_helper_spec.rb b/spec/helpers/admin/orders_helper_spec.rb
new file mode 100644
index 0000000000..6245e7ec62
--- /dev/null
+++ b/spec/helpers/admin/orders_helper_spec.rb
@@ -0,0 +1,25 @@
+require "spec_helper"
+
+describe Admin::OrdersHelper, type: :helper do
+  describe "#order_adjustments_for_display" do
+    let(:order) { create(:order) }
+
+    it "selects eligible adjustments" do
+      adjustment = create(:adjustment, adjustable: order, amount: 1)
+
+      expect(helper.order_adjustments_for_display(order)).to eq [adjustment]
+    end
+
+    it "filters shipping method adjustments" do
+      create(:adjustment, adjustable: order, amount: 1, originator_type: "Spree::ShippingMethod")
+
+      expect(helper.order_adjustments_for_display(order)).to eq []
+    end
+
+    it "filters zero tax rate adjustments" do
+      create(:adjustment, adjustable: order, amount: 0, originator_type: "Spree::TaxRate")
+
+      expect(helper.order_adjustments_for_display(order)).to eq []
+    end
+  end
+end