Remove state and mandatory columns from adjustments

api/app/helpers/spree/api/api_helpers.rb

@@ -129,7 +129,7 @@ def required_fields_for(model)
 
       @@adjustment_attributes = [
         :id, :source_type, :source_id, :adjustable_type, :adjustable_id,
-        :originator_type, :originator_id, :amount, :label, :mandatory, :promotion_code,
+        :originator_type, :originator_id, :amount, :label, :promotion_code,
         :locked, :eligible,  :created_at, :updated_at
       ]
 

core/app/models/spree/adjustment.rb

@@ -7,16 +7,7 @@ module Spree
   #
   # == Boolean attributes
   #
-  # 1. *mandatory*
-  #
-  #    If this flag is set to true then it means the the charge is required and
-  #    will not be removed from the order, even if the amount is zero. In other
-  #    words a record will be created even if the amount is zero. This is
-  #    useful for representing things such as shipping and tax charges where
-  #    you may want to make it explicitly clear that no charge was made for
-  #    such things.
-  #
-  # 2. *eligible?*
+  # 1. *eligible?*
   #
   #    This boolean attributes stores whether this adjustment is currently
   #    eligible for its order. Only eligible adjustments count towards the
@@ -35,21 +26,11 @@ class Adjustment < Spree::Base
     validates :amount, numericality: true
     validates :promotion_code, presence: true, if: :require_promotion_code?
 
-    state_machine :state, initial: :open do
-      event :close do
-        transition from: :open, to: :closed
-      end
-
-      event :open do
-        transition from: :closed, to: :open
-      end
-    end
-
     after_create :update_adjustable_adjustment_total
     after_destroy :update_adjustable_adjustment_total
 
-    scope :open, -> { where(state: 'open') }
-    scope :closed, -> { where(state: 'closed') }
+    scope :open, -> { where(finalized: false) }
+    scope :closed, -> { where(finalized: true) }
     scope :cancellation, -> { where(source_type: 'Spree::UnitCancel') }
     scope :tax, -> { where(source_type: 'Spree::TaxRate') }
     scope :non_tax, -> do
@@ -58,7 +39,6 @@ class Adjustment < Spree::Base
     end
     scope :price, -> { where(adjustable_type: 'Spree::LineItem') }
     scope :shipping, -> { where(adjustable_type: 'Spree::Shipment') }
-    scope :optional, -> { where(mandatory: false) }
     scope :eligible, -> { where(eligible: true) }
     scope :charge, -> { where("#{quoted_table_name}.amount >= 0") }
     scope :credit, -> { where("#{quoted_table_name}.amount < 0") }
@@ -72,9 +52,55 @@ class Adjustment < Spree::Base
     extend DisplayMoney
     money_methods :amount
 
+    def finalize!
+      update_attributes!(finalized: true)
+    end
+
+    def unfinalize!
+      update_attributes!(finalized: false)
+    end
+
+
+    # Deprecated methods
+    def state
+      ActiveSupport::Deprecation.warn "Adjustment#state is deprecated. Instead use Adjustment#finalized?", caller
+      finalized?? "closed" : "open"
+    end
+
+    def state=(new_state)
+      ActiveSupport::Deprecation.warn "Adjustment#state= is deprecated. Instead use Adjustment#finalized=", caller
+      case new_state
+      when "open"
+        self.finalized = false
+      when "closed"
+        self.finalized = true
+      else
+        raise "invaliid adjustment state #{new_state}"
+      end
+    end
+
+    def open?
+      ActiveSupport::Deprecation.warn "Adjustment#open? is deprecated. Instead use Adjustment#finalized?", caller
+      !closed?
+    end
+
     def closed?
-      state == "closed"
+      ActiveSupport::Deprecation.warn "Adjustment#closed? is deprecated. Instead use Adjustment#finalized?", caller
+      finalized?
+    end
+
+    def open
+      ActiveSupport::Deprecation.warn "Adjustment#open is deprecated. Instead use Adjustment#unfinalize!", caller
+      unfinalize!
+    end
+    alias_method :open!, :open
+
+    def close
+      ActiveSupport::Deprecation.warn "Adjustment#close is deprecated. Instead use Adjustment#finalize!", caller
+      finalize!
     end
+    alias_method :close!, :close
+    # End deprecated methods
 
     def currency
       adjustable ? adjustable.currency : Spree::Config[:currency]
@@ -107,7 +133,7 @@ def update!(target = nil)
       if target
         ActiveSupport::Deprecation.warn("Passing a target to Adjustment#update! is deprecated. The adjustment will use the correct target from it's adjustable association.", caller)
       end
-      return amount if closed?
+      return amount if finalized?
 
       # If the adjustment has no source, do not attempt to re-calculate the amount.
       # Chances are likely that this was a manually created adjustment in the admin backend.

core/app/models/spree/order.rb

@@ -370,7 +370,7 @@ def valid_credit_cards
     # Called after transition to complete state when payments will have been processed
     def finalize!
       # lock all adjustments (coupon promotions, etc.)
-      all_adjustments.each{|a| a.close}
+      all_adjustments.each(&:finalize!)
 
       # update payment and shipment(s) states, and save
       updater.update_payment_state

core/app/models/spree/unit_cancel.rb

@@ -20,7 +20,7 @@ def adjust!
       order: inventory_unit.order,
       label: "#{Spree.t(:cancellation)} - #{reason}",
       eligible: true,
-      state: 'closed',
+      finalized: true
     )
   end
 

core/db/migrate/20150811210350_remove_mandatory_from_adjustments.rb

@@ -0,0 +1,5 @@
+class RemoveMandatoryFromAdjustments < ActiveRecord::Migration
+  def change
+    remove_column :spree_adjustments, :mandatory, :boolean
+  end
+end

core/db/migrate/20150811211025_add_finalized_to_spree_adjustments.rb

@@ -0,0 +1,7 @@
+class AddFinalizedToSpreeAdjustments < ActiveRecord::Migration
+  def change
+    add_column :spree_adjustments, :finalized, :boolean
+    execute %q(UPDATE spree_adjustments SET finalized=('open' = state))
+    remove_column :spree_adjustments, :state, :string
+  end
+end

core/lib/spree/core/importer/order.rb

@@ -122,7 +122,7 @@ def self.create_adjustments_from_params(adjustments, order)
               label:  a[:label]
             )
             adjustment.save!
-            adjustment.close!
+            adjustment.finalize!
           end
         end
 

core/spec/lib/spree/core/importer/order_spec.rb

@@ -341,7 +341,7 @@ module Core
             { label: 'Promotion Discount', amount: -3.00 }] }
 
         order = Importer::Order.import(user,params)
-        expect(order.adjustments.all?(&:closed?)).to be true
+        expect(order.adjustments.all?(&:finalized?)).to be true
         expect(order.adjustments.first.label).to eq 'Shipping Discount'
         expect(order.adjustments.first.amount).to eq -4.99
       end

core/spec/models/spree/adjustment_spec.rb

@@ -44,22 +44,6 @@
     end
   end
 
-  context "adjustment state" do
-    subject { build(:adjustment, order: order, state: state) }
-
-    context "#closed?" do
-      context 'is closed' do
-        let(:state) { 'closed' }
-        it { is_expected.to be_closed }
-      end
-
-      context 'is open' do
-        let(:state) { 'open' }
-        it { is_expected.to_not be_closed }
-      end
-    end
-  end
-
   context '#currency' do
     let(:order) { Spree::Order.new currency: 'JPY' }
 
@@ -96,22 +80,22 @@
   end
 
   context '#update!' do
-    let(:adjustment) { Spree::Adjustment.create!(label: 'Adjustment', order: order, adjustable: order, amount: 5, state: state, source: source) }
+    let(:adjustment) { Spree::Adjustment.create!(label: 'Adjustment', order: order, adjustable: order, amount: 5, finalized: finalized, source: source) }
     let(:source) { mock_model(Spree::TaxRate, compute_amount: 10) }
 
     subject { adjustment.update! }
 
     context "when adjustment is closed" do
-      let(:state) { 'closed' }
+      let(:finalized) { true }
 
       it "does not update the adjustment" do
         expect(adjustment).to_not receive(:update_column)
         subject
       end
     end
 
-    context "when adjustment is open" do
-      let(:state) { 'open' }
+    context "when adjustment isn't finalized" do
+      let(:finalized) { false }
 
       it "updates the amount" do
         expect { subject }.to change { adjustment.amount }.to(10)

core/spec/models/spree/item_adjustments_spec.rb

@@ -113,9 +113,8 @@ def create_adjustment(label, amount)
                             :adjustable => line_item,
                             :source     => source,
                             :amount     => amount,
-                            :state      => "closed",
-                            :label      => label,
-                            :mandatory  => false)
+                            :finalized  => true,
+                            :label      => label)
       end
 
       it "should make all but the most valuable promotion adjustment ineligible, leaving non promotion adjustments alone" do
@@ -126,7 +125,7 @@ def create_adjustment(label, amount)
                             :adjustable => line_item,
                             :source => nil,
                             :amount => -500,
-                            :state => "closed",
+                            :finalized => true,
                             :label => "Some other credit")
         line_item.adjustments.each {|a| a.update_column(:eligible, true)}
 

core/spec/models/spree/order/finalizing_spec.rb

@@ -75,7 +75,7 @@
       adjustments = [double]
       expect(order).to receive(:all_adjustments).and_return(adjustments)
       adjustments.each do |adj|
-        expect(adj).to receive(:close)
+        expect(adj).to receive(:finalize!)
       end
       order.finalize!
     end

core/spec/models/spree/order_cancellations_spec.rb

@@ -82,7 +82,7 @@
           adjustable: line_item,
           amount: 0.01,
           label: 'some fake tax',
-          state: 'closed',
+          finalized: true
         )
         order.update!
       end

core/spec/models/spree/shipment_spec.rb

@@ -508,7 +508,7 @@
         label:    "Additional",
         amount:   5,
         included: false,
-        state:    "closed"
+        finalized: true
       )
       shipment.update_amounts
       expect(shipment.reload.adjustment_total).to eq(5)
@@ -520,7 +520,7 @@
         label:    "Included",
         amount:   5,
         included: true,
-        state:    "closed"
+        finalized: true
       )
       shipment.update_amounts
       expect(shipment.reload.adjustment_total).to eq(0)

core/spec/models/spree/unit_cancel_spec.rb

@@ -15,8 +15,8 @@
       expect(adjustment.amount).to eq -10.0
       expect(adjustment.order).to eq inventory_unit.order
       expect(adjustment.label).to eq "Cancellation - Short Ship"
-      expect(adjustment.eligible).to eq true
-      expect(adjustment.state).to eq 'closed'
+      expect(adjustment).to be_eligible
+      expect(adjustment).to be_finalized
     end
 
     context "when an adjustment has already been created" do