Merge pull request #3667 from mkllnk/3477-display-eligible-adjustments

3477 Render only eligible adjustments on the order page

app/helpers/admin/orders_helper.rb

@@ -0,0 +1,13 @@
+module Admin
+  module OrdersHelper
+    # 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.
+    def order_adjustments_for_display(order)
+      order.adjustments.eligible.select do |adjustment|
+        adjustment.originator_type != "Spree::ShippingMethod"
+      end
+    end
+  end
+end

app/views/spree/admin/orders/_form.html.haml

@@ -4,12 +4,12 @@
 
   = render :partial => "spree/admin/orders/shipment", :collection => @order.shipments, :locals => { :order => order }
 
-  = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => @order.price_adjustments, :order => order, :title => Spree.t(:line_item_adjustments)}
-  = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => @order.adjustments, :order => order, :title => Spree.t(:order_adjustments)}
+  = 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_for_display(@order), :title => t(".order_adjustments")}
 
   - if order.line_items.exists?
     %fieldset#order-total.no-border-bottom{"data-hook" => "order_details_total"}
-      %legend= Spree.t(:order_total)
+      %legend= t(".order_total")
       %span.order-total= order.display_total
 
   = form_for @order, url: admin_order_url(@order), method: :put do |f|

app/views/spree/admin/orders/_form/_adjustments.html.haml

@@ -3,12 +3,12 @@
     %legend= title
     %table>
       %thead
-        %th= Spree.t('name')
-        %th= Spree.t('amount')
-      %tbody#order-charges.with-border
+        %tr
+          %th= Spree.t('name')
+          %th= Spree.t('amount')
+      %tbody.with-border
         - adjustments.each do |adjustment|
-          - if (adjustment.originator_type != 'Spree::ShippingMethod') && !(adjustment.originator_type == 'Spree::TaxRate' && adjustment.amount == 0)
-            %tr.total
-              %td.strong= adjustment.label + ":"
-              %td.total.align-center
-                %span= Spree::Money.new(adjustment.amount)
+          %tr.total
+            %td.strong= adjustment.label + ":"
+            %td.total.align-center
+              %span= Spree::Money.new(adjustment.amount)

spec/controllers/spree/admin/orders_controller_spec.rb

@@ -14,6 +14,23 @@ describe Spree::Admin::OrdersController, type: :controller do
         spree_get :edit, id: order
       }.to change { order.reload.state }.from("cart").to("complete")
     end
+
+    describe "view" do
+      render_views
+
+      it "shows only eligible adjustments" do
+        adjustment = create(
+          :adjustment,
+          adjustable: order,
+          label: "invalid adjustment",
+          amount: 0
+        )
+
+        spree_get :edit, id: order
+
+        expect(response.body).to_not match adjustment.label
+      end
+    end
   end
 
   context "#update" do

spec/features/admin/orders_spec.rb

@@ -255,7 +255,6 @@ feature %q{
       scenario "shows the order non-tax adjustments" do
         within('table.index tbody') do
           @order.adjustments.eligible.each do |adjustment|
-            next if (adjustment.originator_type == 'Spree::TaxRate') && (adjustment.amount == 0)
             expect(page).to have_selector "td", match: :first, text: adjustment.label
             expect(page).to have_selector "td.total", text: adjustment.display_amount
           end
@@ -267,7 +266,7 @@ feature %q{
       end
 
       scenario "shows the order tax adjustments" do
-        within('tbody#order-charges') do
+        within('fieldset', text: I18n.t('spree.admin.orders.form.line_item_adjustments').upcase) do
           expect(page).to have_selector "td", match: :first, text: "Tax 1"
           expect(page).to have_selector "td.total", text: Spree::Money.new(10)
         end

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