Merge pull request #5802 from mamhoff/move-eligible-column-to-legacy-…

…promotions

Move eligible column to legacy promotions

admin/app/controllers/solidus_admin/adjustments_controller.rb

@@ -4,13 +4,7 @@ class SolidusAdmin::AdjustmentsController < SolidusAdmin::BaseController
   before_action :load_order
 
   def index
-    @adjustments = @order
-      .all_adjustments
-      .eligible
-      .order("adjustable_type ASC, created_at ASC")
-      .ransack(params[:q])
-      .result
-
+    load_adjustments
     set_page_and_extract_portion_from(@adjustments)
 
     respond_to do |format|
@@ -49,6 +43,14 @@ def destroy
 
   private
 
+  def load_adjustments
+    @adjustments = @order
+      .all_adjustments
+      .order("adjustable_type ASC, created_at ASC")
+      .ransack(params[:q])
+      .result
+  end
+
   def load_order
     @order = Spree::Order.find_by!(number: params[:order_id])
   end

admin/spec/features/orders/adjustments_spec.rb

@@ -17,7 +17,6 @@
       adjustable: order,
       amount: 10,
       label: "Test Adjustment",
-      eligible: true,
       finalized: false,
       created_at: Time.current,
       updated_at: Time.current,
@@ -54,7 +53,6 @@
       adjustable: order,
       amount: 10,
       label: "No Source Adjustment",
-      eligible: true,
       finalized: false,
       created_at: Time.current,
       updated_at: Time.current,

api/lib/spree/api_configuration.rb

@@ -73,7 +73,7 @@ class ApiConfiguration < Preferences::Configuration
     preference :adjustment_attributes, :array, default: [
       :id, :source_type, :source_id, :adjustable_type, :adjustable_id,
       :amount, :label,
-      :finalized, :eligible, :created_at, :updated_at
+      :finalized, :created_at, :updated_at
     ]
 
     preference :inventory_unit_attributes, :array, default: [

api/openapi/solidus-api.oas.yml

@@ -6118,8 +6118,6 @@ components:
           type: string
         display_amount:
           type: string
-        eligible:
-          type: boolean
         finalized:
           type: boolean
         id:

backend/app/assets/javascripts/spree/backend/views/order/details_adjustments.js

@@ -9,8 +9,7 @@ Spree.Views.Order.DetailsAdjustments = Backbone.View.extend({
     var collection = this.collection ? this.collection.chain() : _.chain([this.model]);
     collection
       .map(function(item) {
-        return (item.get("adjustments") || [])
-          .filter(function(adjustment) { return (adjustment.eligible === true); });
+        return (item.get("adjustments") || []);
       })
       .flatten(true)
       .each(function(adjustment){

backend/app/assets/stylesheets/spree/backend/sections/_adjustments.scss

@@ -1,3 +0,0 @@
-.adjustment-ineligible {
-  text-decoration: line-through;
-}

backend/app/views/spree/admin/adjustments/_adjustment.html.erb

@@ -4,7 +4,6 @@
 %>
 <tr id="<%= spree_dom_id(adjustment) %>"
     data-hook="adjustment_row"
-    class="<%= "adjustment-ineligible" if !adjustment.eligible? %>"
     >
   <td><%= display_adjustable(adjustment.adjustable) %></td>
   <td><%= adjustment.label %></td>

backend/app/views/spree/admin/orders/_adjustments.html.erb

@@ -8,7 +8,7 @@
       </tr>
     </thead>
     <tbody data-hook="order_details_adjustments">
-      <% adjustments.eligible.group_by(&:label).each do |label, adjustments| %>
+      <% adjustments.group_by(&:label).each do |label, adjustments| %>
         <tr class="total">
           <td><%= label %>:</td>
           <td class="total"><span><%= Spree::Money.new(adjustments.sum(&:amount), currency: adjustments.first.order.try(:currency)) %></span></td>

backend/app/views/spree/admin/orders/_order_details.html.erb

@@ -1,4 +1,4 @@
-<div class="js-order-line-item-adjustments <%= "hidden" if order.line_item_adjustments.eligible.none? %>">
+<div class="js-order-line-item-adjustments <%= "hidden" if order.line_item_adjustments.none? %>">
   <%=
     render "spree/admin/orders/adjustments", {
       adjustments: order.line_item_adjustments,
@@ -7,7 +7,7 @@
     }
   %>
 </div>
-<div class="js-order-shipment-adjustments <%= "hidden" if order.shipment_adjustments.eligible.none? %>">
+<div class="js-order-shipment-adjustments <%= "hidden" if order.shipment_adjustments.none? %>">
   <%=
     render "spree/admin/orders/adjustments", {
       adjustments: order.shipment_adjustments,
@@ -16,7 +16,7 @@
     }
   %>
 </div>
-<div class="js-order-adjustments <%= "hidden" if order.adjustments.eligible.none? %>">
+<div class="js-order-adjustments <%= "hidden" if order.adjustments.none? %>">
   <%=
     render "spree/admin/orders/adjustments", {
       adjustments: order.adjustments,

backend/app/views/spree/admin/orders/confirm.html.erb

@@ -18,7 +18,7 @@
       order: @order,
       title: t('spree.line_item_adjustments'),
     }
-  ) if @order.line_item_adjustments.eligible.any?
+  ) if @order.line_item_adjustments.any?
 %>
 
 <%=
@@ -29,7 +29,7 @@
       order: @order,
       title: t('spree.shipment_adjustments'),
     }
-  ) if @order.shipment_adjustments.eligible.any?
+  ) if @order.shipment_adjustments.any?
 %>
 
 <%=
@@ -40,7 +40,7 @@
       order: @order,
       title: t('spree.order_adjustments'),
     }
-  ) if @order.adjustments.eligible.any?
+  ) if @order.adjustments.any?
 %>
 
 <% if @order.line_items.exists? %>

backend/spec/features/admin/orders/adjustments_spec.rb

@@ -22,7 +22,6 @@
     let(:tax_category) { create(:tax_category) }
     let(:variant) { create(:variant, tax_category: tax_category) }
 
-    let!(:non_eligible_adjustment) { order.adjustments.create!(order: order, label: 'Non-Eligible', amount: 10, eligible: false) }
     let!(:adjustment) { order.adjustments.create!(order: order, label: 'Rebate', amount: 10) }
 
     before(:each) do
@@ -41,13 +40,6 @@
           expect(column_text(3)).to eq("$2.00")
         end
       end
-
-      it "shows both eligible and non-eligible adjustments" do
-        expect(page).to have_content("Rebate")
-        expect(page).to have_content("Non-Eligible")
-        expect(find('tr', text: 'Rebate')[:class]).not_to eq('adjustment-ineligible')
-        expect(find('tr', text: 'Non-Eligible')[:class]).to eq('adjustment-ineligible')
-      end
     end
 
     context "admin creating a new adjustment" do

backend/spec/features/admin/orders/order_details_spec.rb

@@ -247,21 +247,6 @@
         end
       end
 
-      context "with adjustments" do
-        let(:order) do
-          super().tap do |o|
-            o.adjustments.create!(order: order, label: 'Non-Eligible', amount: 10, eligible: false)
-            o.adjustments.create!(order: order, label: 'Rebate', amount: 10)
-          end
-        end
-
-        it "shows only eligible adjustments" do
-          visit spree.cart_admin_order_path(order)
-          expect(page).to have_content("Rebate")
-          expect(page).not_to have_content("Non-Eligible")
-        end
-      end
-
       context "variant doesn't track inventory" do
         let(:track_inventory) { false }
         let(:backorderable) { false }

core/app/models/spree/adjustment.rb

@@ -4,17 +4,8 @@ module Spree
   # Adjustments represent a change to the +item_total+ of an Order. Each
   # adjustment has an +amount+ that can be either positive or negative.
   #
-  # Adjustments can be "opened" or "closed". Once an adjustment is closed, it
+  # Adjustments can be "unfinalized" or "finalized". Once an adjustment is finalized, it
   # will not be automatically updated.
-  #
-  # == Boolean attributes
-  #
-  # 1. *eligible?*
-  #
-  #    This boolean attributes stores whether this adjustment is currently
-  #    eligible for its order. Only eligible adjustments count towards the
-  #    order's adjustment total. This allows an adjustment to be preserved if
-  #    it becomes ineligible so it might be reinstated.
   class Adjustment < Spree::Base
     belongs_to :adjustable, polymorphic: true, touch: true, optional: true
     belongs_to :source, polymorphic: true, optional: true
@@ -36,7 +27,10 @@ class Adjustment < Spree::Base
     end
     scope :price, -> { where(adjustable_type: 'Spree::LineItem') }
     scope :shipping, -> { where(adjustable_type: 'Spree::Shipment') }
-    scope :eligible, -> { where(eligible: true) }
+    scope :eligible, -> { all }
+    class << self
+      deprecate :eligible, deprecator: Spree.deprecator
+    end
     scope :charge, -> { where("#{quoted_table_name}.amount >= 0") }
     scope :credit, -> { where("#{quoted_table_name}.amount < 0") }
     scope :nonzero, -> { where("#{quoted_table_name}.amount != 0") }
@@ -87,5 +81,10 @@ def tax?
     def cancellation?
       source_type == 'Spree::UnitCancel'
     end
+
+    def eligible?
+      true
+    end
+    alias_method :eligible, :eligible?
   end
 end

core/app/models/spree/line_item.rb

@@ -62,7 +62,7 @@ def total
     # @return [BigDecimal] the amount of this item, taking into consideration
     #   all non-tax adjustments.
     def total_before_tax
-      amount + adjustments.select { |value| !value.tax? && value.eligible? }.sum(&:amount)
+      amount + adjustments.reject(&:tax?).sum(&:amount)
     end
 
     # @return [BigDecimal] the amount of this line item before VAT tax

core/app/models/spree/order.rb

@@ -227,7 +227,7 @@ def currency
     end
 
     def shipping_discount
-      shipment_adjustments.credit.eligible.sum(:amount) * - 1
+      shipment_adjustments.credit.sum(:amount) * - 1
     end
 
     def to_param

core/app/models/spree/order_updater.rb

@@ -157,9 +157,9 @@ def update_adjustment_total
       recalculate_adjustments
 
       all_items = line_items + shipments
-      order_tax_adjustments = adjustments.select(&:eligible?).select(&:tax?)
+      order_tax_adjustments = adjustments.select(&:tax?)
 
-      order.adjustment_total = all_items.sum(&:adjustment_total) + adjustments.select(&:eligible?).sum(&:amount)
+      order.adjustment_total = all_items.sum(&:adjustment_total) + adjustments.sum(&:amount)
       order.included_tax_total = all_items.sum(&:included_tax_total) + order_tax_adjustments.select(&:included?).sum(&:amount)
       order.additional_tax_total = all_items.sum(&:additional_tax_total) + order_tax_adjustments.reject(&:included?).sum(&:amount)
 
@@ -224,7 +224,6 @@ def update_item_totals
         # The cancellation_total isn't persisted anywhere but is included in
         # the adjustment_total
         item.adjustment_total = item.adjustments.
-          select(&:eligible?).
           reject(&:included?).
           sum(&:amount)
 

core/app/models/spree/shipment.rb

@@ -92,7 +92,7 @@ def total
     # @return [BigDecimal] the amount of this item, taking into consideration
     #   all non-tax adjustments.
     def total_before_tax
-      amount + adjustments.select { |adjustment| !adjustment.tax? && adjustment.eligible? }.sum(&:amount)
+      amount + adjustments.reject(&:tax?).sum(&:amount)
     end
 
     # @return [BigDecimal] the amount of this shipment before VAT tax

core/app/models/spree/unit_cancel.rb

@@ -23,7 +23,6 @@ def adjust!
       amount: amount,
       order: inventory_unit.order,
       label: "#{I18n.t('spree.cancellation')} - #{reason}",
-      eligible: true,
       finalized: true
     )
 

core/app/views/spree/order_mailer/cancel_email.html.erb

@@ -26,7 +26,7 @@
           <td><%= t('.subtotal') %></td>
           <td><%= @order.display_item_total %></td>
         </tr>
-        <% @order.adjustments.eligible.each do |adjustment| %>
+        <% @order.adjustments.each do |adjustment| %>
           <tr>
             <td></td>
             <td><%= sanitize(adjustment.label) %></td>

core/app/views/spree/order_mailer/cancel_email.text.erb

@@ -10,7 +10,7 @@
 <% end %>
 ============================================================
 <%= t('.subtotal') %> <%= @order.display_item_total %>
-<% @order.adjustments.eligible.each do |adjustment| %>
+<% @order.adjustments.each do |adjustment| %>
   <%= raw(adjustment.label) %> <%= adjustment.display_amount %>
 <% end %>
 <%= t('.total') %> <%= @order.display_total %>

core/app/views/spree/order_mailer/confirm_email.html.erb

@@ -31,8 +31,8 @@
           </td>
         </tr>
         <% if @order.line_item_adjustments.exists? %>
-          <% if @order.all_adjustments.promotion.eligible.exists? %>
-            <% @order.all_adjustments.promotion.eligible.group_by(&:label).each do |label, adjustments| %>
+          <% if @order.all_adjustments.promotion.exists? %>
+            <% @order.all_adjustments.promotion.group_by(&:label).each do |label, adjustments| %>
               <tr>
                 <td></td>
                 <td><%= t('spree.promotion') %> <%= label %>:</td>
@@ -48,16 +48,16 @@
             <td><%= Spree::Money.new(shipments.sum(&:total_before_tax), currency: @order.currency) %></td>
           </tr>
         <% end %>
-        <% if @order.all_adjustments.eligible.tax.exists? %>
-          <% @order.all_adjustments.eligible.tax.group_by(&:label).each do |label, adjustments| %>
+        <% if @order.all_adjustments.tax.exists? %>
+          <% @order.all_adjustments.tax.group_by(&:label).each do |label, adjustments| %>
             <tr>
               <td></td>
               <td><%= t('spree.tax') %> <%= label %>:</td>
               <td><%= Spree::Money.new(adjustments.sum(&:amount), currency: @order.currency) %></td>
             </tr>
           <% end %>
         <% end %>
-        <% @order.adjustments.eligible.each do |adjustment| %>
+        <% @order.adjustments.each do |adjustment| %>
           <% next if (adjustment.source_type == 'Spree::TaxRate') and (adjustment.amount == 0) %>
           <tr>
             <td></td>

core/app/views/spree/order_mailer/confirm_email.text.erb

@@ -11,8 +11,8 @@
 ============================================================
 <%= t('.subtotal') %> <%= @order.display_item_total %>
 <% if @order.line_item_adjustments.exists? %>
-  <% if @order.all_adjustments.promotion.eligible.exists? %>
-    <% @order.all_adjustments.promotion.eligible.group_by(&:label).each do |label, adjustments| %>
+  <% if @order.all_adjustments.promotion.exists? %>
+    <% @order.all_adjustments.promotion.group_by(&:label).each do |label, adjustments| %>
 <%= t('spree.promotion') %>: <%= label %> <%= Spree::Money.new(adjustments.sum(&:amount), currency: @order.currency) %>
     <% end %>
   <% end %>
@@ -22,13 +22,13 @@
 <%= t('spree.shipping') %>: <%= name %> <%= Spree::Money.new(shipments.sum(&:total_before_tax), currency: @order.currency) %>
 <% end %>
 
-<% if @order.all_adjustments.eligible.tax.exists? %>
-  <% @order.all_adjustments.eligible.tax.group_by(&:label).each do |label, adjustments| %>
+<% if @order.all_adjustments.tax.exists? %>
+  <% @order.all_adjustments.tax.group_by(&:label).each do |label, adjustments| %>
 <%= t('spree.tax') %>: <%= label %> <%= Spree::Money.new(adjustments.sum(&:amount), currency: @order.currency) %>
   <% end %>
 <% end %>
 
-<% @order.adjustments.eligible.each do |adjustment| %>
+<% @order.adjustments.each do |adjustment| %>
   <% next if (adjustment.source_type == 'Spree::TaxRate') and (adjustment.amount == 0) %>
 <%= adjustment.label %> <%= adjustment.display_amount %>
 <% end %>

core/db/migrate/20160101010000_solidus_one_four.rb

@@ -85,7 +85,6 @@ def up
       t.integer "adjustable_id", null: false
       t.decimal "amount", precision: 10, scale: 2
       t.string "label"
-      t.boolean "eligible", default: true
       t.datetime "created_at", precision: 6
       t.datetime "updated_at", precision: 6
       t.integer "order_id", null: false
@@ -94,7 +93,6 @@ def up
       t.boolean "finalized", default: false, null: false
       t.index ["adjustable_id", "adjustable_type"], name: "index_spree_adjustments_on_adjustable_id_and_adjustable_type"
       t.index ["adjustable_id"], name: "index_adjustments_on_order_id"
-      t.index ["eligible"], name: "index_spree_adjustments_on_eligible"
       t.index ["order_id"], name: "index_spree_adjustments_on_order_id"
       t.index ["source_id", "source_type"], name: "index_spree_adjustments_on_source_id_and_source_type"
     end

core/lib/spree/testing_support/factories/adjustment_factory.rb

@@ -7,7 +7,6 @@
     amount { 100.0 }
     label { 'Shipping' }
     association(:source, factory: :tax_rate)
-    eligible { true }
 
     after(:build) do |adjustment|
       adjustments = adjustment.adjustable.adjustments