Change order state to merged instead of destroying it when merging orders

backend/app/controllers/spree/admin/orders_controller.rb

@@ -49,7 +49,7 @@ def index
           params[:q][:completed_at_lt] = params[:q].delete(:created_at_lt)
         end
 
-        @search = Spree::Order.accessible_by(current_ability, :index).ransack(params[:q])
+        @search = Spree::Order.not_merged.accessible_by(current_ability, :index).ransack(params[:q])
         @orders = @search.result.includes([:user]).
           page(params[:page]).
           per(params[:per_page] || Spree::Config[:orders_per_page])

core/app/models/spree/order.rb

@@ -54,6 +54,9 @@ class CannotRebuildShipments < StandardError; end
     deprecate temporary_credit_card: :temporary_payment_source, deprecator: Spree::Deprecation
     deprecate :temporary_credit_card= => :temporary_payment_source=, deprecator: Spree::Deprecation
 
+    has_many :from_merged_orders, foreign_key: :merged_to_order_id, class_name: 'Spree::Order', inverse_of: :merged_to_order
+    belongs_to :merged_to_order, foreign_key: :merged_to_order_id, class_name: 'Spree::Order', inverse_of: :from_merged_orders, optional: true
+
     # Customer info
     belongs_to :user, class_name: Spree::UserClassHandle.new, optional: true
     belongs_to :bill_address, foreign_key: :bill_address_id, class_name: 'Spree::Address', optional: true
@@ -179,6 +182,14 @@ def self.not_canceled
       where.not(state: 'canceled')
     end
 
+    def self.merged
+      where(state: 'merged')
+    end
+
+    def self.not_merged
+      where.not(state: 'merged')
+    end
+
     # Use this method in other gems that wish to register their own custom logic
     # that should be called after Order#update
     def self.register_update_hook(hook)
@@ -846,7 +857,7 @@ def link_by_email
 
     # Determine if email is required (we don't want validation errors before we hit the checkout)
     def require_email
-      true unless new_record? || ['cart', 'address'].include?(state)
+      true unless new_record? || ['cart', 'address', 'merged'].include?(state)
     end
 
     def ensure_inventory_units
@@ -875,6 +886,12 @@ def validate_line_item_availability
       raise InsufficientStock unless line_items.all? { |line_item| availability_validator.validate(line_item) }
     end
 
+    def ensure_merged_to_order_present
+      if merged_to_order.blank?
+        errors.add(:base, :merged_to_order_must_be_present) && (return false)
+      end
+    end
+
     def ensure_line_items_present
       unless line_items.present?
         errors.add(:base, I18n.t('spree.there_are_no_items_for_this_order')) && (return false)

core/app/models/spree/order/checkout.rb

@@ -59,6 +59,10 @@ def define_state_machine!
               transition to: :canceled, if: :allow_cancel?
             end
 
+            event :merge do
+              transition to: :merged
+            end
+
             event :return do
               transition to: :returned, from: [:returned, :complete, :awaiting_return, :canceled], if: :all_inventory_units_returned?
             end
@@ -89,7 +93,9 @@ def define_state_machine!
               # be added in the correct sequence.
             end
 
-            before_transition from: :cart, do: :ensure_line_items_present
+            before_transition from: :cart, except_to: :merged, do: :ensure_line_items_present
+
+            before_transition to: :merged, do: :ensure_merged_to_order_present
 
             if states[:address]
               before_transition to: :address, do: :assign_default_user_addresses
@@ -106,6 +112,11 @@ def define_state_machine!
             before_transition to: :resumed, do: :ensure_line_item_variants_are_not_deleted
             before_transition to: :resumed, do: :validate_line_item_availability
 
+            before_transition from: :merged do |order|
+              order.errors.add(:base, :cannot_transition_from_merged)
+              false
+            end
+
             # Sequence of before_transition to: :complete
             # calls matter so that we do not process payments
             # until validations have passed

core/app/models/spree/order_merger.rb

@@ -31,15 +31,16 @@ def initialize(order)
     # will add the quantity of the incoming line item to the existing line item.
     # Otherwise, it will assign the line item to the new order.
     #
-    # After the orders have been merged the `other_order` will be destroyed.
+    # After the orders have been merged the `other_order` will be marked as merged.
     #
     # @example
     #   initial_order = Spree::Order.find(1)
     #   order_to_merge = Spree::Order.find(2)
     #   merger = Spree::OrderMerger.new(initial_order)
     #   merger.merge!(order_to_merge)
-    #   # order_to_merge is destroyed, initial order now contains the line items
-    #   # of order_to_merge
+
+    # order_to_merge is marked as merged, initial order now contains the line
+    # items of order_to_merge
     #
     # @api public
     # @param [Spree::Order] other_order An order which will be merged in to the
@@ -58,9 +59,8 @@ def merge!(other_order, user = nil)
       set_user(user)
       persist_merge
 
-      # So that the destroy doesn't take out line items which may have been re-assigned
-      other_order.line_items.reload
-      other_order.destroy
+      other_order.merged_to_order = order
+      other_order.merge && other_order.save!
     end
 
     private

core/config/locales/en.yml

@@ -469,6 +469,12 @@ en:
             price:
               not_a_number: is not valid
               does_not_match_order_currency: Line item price currency must match order currency!
+        spree/order:
+          attributes:
+            base:
+              cannot_transition_from_merged: Cannot transition from merged
+              merged_to_order_must_be_present: The order must be associated with
+                the order that is merged into via the `merged_to_order` association
         spree/price:
           attributes:
             currency:
@@ -1716,6 +1722,7 @@ en:
       complete: Complete
       confirm: Confirm
       delivery: Delivery
+      merged: Merged
       payment: Payment
       resumed: Resumed
       returned: Returned

core/db/migrate/20200117125833_add_merged_to_order_id.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class AddMergedToOrderId < ActiveRecord::Migration[5.2]
+  def change
+    add_column :spree_orders, :merged_to_order_id, :integer, limit: 4
+    add_foreign_key :spree_orders, :spree_orders, column: :merged_to_order_id
+  end
+end

core/spec/models/spree/order_merger_spec.rb

@@ -12,9 +12,15 @@ module Spree
     let(:user) { stub_model(Spree::LegacyUser, email: "spree@example.com") }
     let(:subject) { Spree::OrderMerger.new(order_1) }
 
-    it "destroys the other order" do
+    it "marks the other order as merged" do
       subject.merge!(order_2)
-      expect { order_2.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect(order_2).to be_merged
+    end
+
+    it "associates the merged orders" do
+      subject.merge!(order_2)
+      expect(order_2.merged_to_order).to eq order_1
+      expect(order_1.from_merged_orders).to include order_2
     end
 
     it "persist the merge" do

core/spec/models/spree/order_spec.rb

@@ -334,14 +334,40 @@
     end
   end
 
+  context "when changing order state to 'merged'" do
+    it "requires the merged_to_order association to be present" do
+      expect(subject.merge).to be false
+      expect(subject.errors[:base]).to be_present
+    end
+  end
+
+  context "when the order is in 'merged' state" do
+    before { subject.update(state: :merged) }
+
+    it "cannot transition to other states" do
+      %w[cancel return resume complete authorize_return].each do |transition|
+        expect do
+          subject.send("#{transition}!")
+        end.to raise_error StateMachines::InvalidTransition
+      end
+    end
+  end
+
   describe '#merge!' do
     let(:order1) { create(:order_with_line_items) }
     let(:order2) { create(:order_with_line_items) }
 
+    subject { order1.merge!(order2) }
+
     it 'merges the orders' do
-      order1.merge!(order2)
+      subject
       expect(order1.line_items.count).to eq(2)
-      expect(order2.destroyed?).to be_truthy
+      expect(order2).to be_merged
+    end
+
+    it 'sets both orders "merge" associations' do
+      expect { subject }.to change { order2.merged_to_order }.to(order1)
+        .and change { order1.reload.from_merged_orders }.to([order2])
     end
 
     describe 'order_merger_class customization' do