Openfoodnetwork: [Spree Upgrade] Adjustments with amount zero are being displayed in the orders edit page

[luisramos0]

Description

Order edit page is showing adjustments that have amount zero. These adjustments should be removed by using the scope adjustment.eligible. See this review comment.

Additionally, the id order-charges is being used several times making the html invalid. See this comment

Expected Behavior

Adjustments with amount zero should not be shown.
html div Id order-charges is NOT present multiple times in the html if order has more than one adjustment.

Actual Behaviour

Adjustments with amount zero are listed.
html div Id order-charges is present multiple times in the html if order has more than one adjustment.

Steps to Reproduce

Create an order with a adjustment with a zero amount (a specific example needs to be given here) and verify it is not showing on the orders edit page.

Possible Fix

Use the eligible scope to fetch adjustments.

[kristinalim]

Noting that the bug here is not just with $0 adjustments. See further investigation of a bug of the same nature here: #3478, PR #3512

It seems that adjustments can exist even when the adjustment amount is 0, for promotions associated with an order even when they are not the final one applied, and for computed payment transaction fees even if the payment failed or was invalid.

[luisramos0]

I see you have investigated the adjustments topic, nice! It's very complicated with callbacks after callbacks...
I am not sure what do you mean in terms of this issue, this issue is simply a visual bug, entries with value zero should not be listed in the order page. All problems with adjustments are out of scope here (at least in my mind when I created this issue).
Problems with adjustments calculations are represented in 1830

[kristinalim]

Sorry I wasn't clear. My point was that some invalid non-zero adjustments could be showing up in the order page too, not just $0 adjustments, until this is scoped to eligibile: true. And I agree that this is still a display bug.

[luisramos0]

this would be the change here:

--- 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, :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.adjustments.eligible, :order => order, :title => Spree.t(:order_adjustments)}

we would need to put this in and write some tests for it.

I am not doing it right now because it's really a minor problem. I think it could be tagged bug-s5...

[mkllnk]

I looked at the id order-charges.

# Spree code
backend/app/views/spree/admin/orders/_form.html.erb:        <tbody id="order-charges" data-hook="order_details_adjustments"  class="with-border">
backend/app/views/spree/admin/shared/_order_details.html.erb:  <tbody id="order-charges" data-hook="order_details_adjustments"  class="with-border">
frontend/app/views/spree/checkout/_summary.html.erb:    <tbody id="summary-order-charges" data-hook>
frontend/app/views/spree/shared/_order_details.html.erb:  <tfoot id="order-charges" data-hook="order_details_adjustments">

# OFN code
app/views/spree/admin/orders/_form/_adjustments.html.haml:      %tbody#order-charges.with-border
app/views/spree/orders/_summary.html.haml:    #order-charges{"data-hook" => "order_details_adjustments"}
spec/features/admin/orders_spec.rb:        within('tbody#order-charges') do

None of these is in a loop and I think that none of these appear on the same page. Did I overlook something? Otherwise we don't need to change it.