Deprecate public visibility of order#finalize!

core/app/models/spree/order.rb

@@ -379,25 +379,16 @@ def valid_credit_cards
  Spree::CreditCard.where(id: credit_card_ids)
  end
 
- # Finalizes an in progress order after checkout is complete.
- # Called after transition to complete state when payments will have been processed
+ # TODO: Remove on Solidus 4.0
+ # @api private
  def finalize!
- # lock all adjustments (coupon promotions, etc.)
- all_adjustments.each(&:finalize!)
-
- # update payment and shipment(s) states, and save
- updater.update_payment_state
- shipments.each do |shipment|
- shipment.update_state
- shipment.finalize!
- end
-
- updater.update_shipment_state
- save!
-
- touch :completed_at
-
- Spree::Event.fire 'order_finalized', order: self
+ Spree::Deprecation.warn <<~MSG
+ Calling `Spree::Order#finalize!` is discouraged. Instead, use
+ `Spree::Order#complete!`, which goes through all the needed safety
+ checks before finalizing an order. This method will be removed
+ altogether on Solidus 4.0.
+ MSG
+ finalize
  end
 
  def fulfill!
@@ -741,6 +732,27 @@ def process_payments_before_complete
  end
  end
 
+ # Finalizes an in progress order after checkout is complete.
+ # Called after transition to complete state when payments will have been processed
+ def finalize
+ # lock all adjustments (coupon promotions, etc.)
+ all_adjustments.each(&:finalize!)
+
+ # update payment and shipment(s) states, and save
+ updater.update_payment_state
+ shipments.each do |shipment|
+ shipment.update_state
+ shipment.finalize!
+ end
+
+ updater.update_shipment_state
+ save!
+
+ touch :completed_at
+
+ Spree::Event.fire 'order_finalized', order: self
+ end
+
  def associate_store
  self.store ||= Spree::Store.default
  end

core/lib/spree/core/state_machines/order.rb

@@ -119,7 +119,7 @@ def define_state_machine!
  before_transition to: :complete, do: :process_payments_before_complete
  end
 
- after_transition to: :complete, do: :finalize!
+ after_transition to: :complete, do: :finalize
  after_transition to: :resumed, do: :after_resume
  after_transition to: :canceled, do: :after_cancel
 

core/lib/spree/event.rb

@@ -97,7 +97,7 @@ def registry(adapter: default_adapter)
  #
  # @example Trigger an event named 'order_finalized'
  # Spree::Event.fire 'order_finalized', order: @order do
- # @order.finalize!
+ # @order.complete!
  # end
  #
  # TODO: Change signature so that `opts` are keyword arguments, and include

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

@@ -3,62 +3,31 @@
 require 'rails_helper'
 
 RSpec.describe Spree::Order, type: :model do
- context "#finalize!" do
+ context "#complete!" do
  let(:order) { create(:order_ready_to_complete) }
 
- before do
- order.update_column :state, 'complete'
- end
-
  it "should set completed_at" do
- expect(order).to receive(:touch).with(:completed_at)
- order.finalize!
+ expect { order.complete! }.to change { order.completed_at }
  end
 
  it "should sell inventory units" do
- order.shipments.each do |shipment|
- expect(shipment).to receive(:update_state)
- expect(shipment).to receive(:finalize!)
- end
- order.finalize!
- end
+ inventory_unit = order.shipments.first.inventory_units.first
 
- it "should change the shipment state to ready if order is paid" do
- allow(order).to receive_messages(paid?: true, complete?: true)
- order.finalize!
- order.reload # reload so we're sure the changes are persisted
- expect(order.shipment_state).to eq('ready')
- end
+ order.payments.map(&:complete!)
 
- it "should send an order confirmation email" do
- mail_message = double "Mail::Message"
- expect(Spree::OrderMailer).to receive(:confirm_email).with(order).and_return mail_message
- expect(mail_message).to receive :deliver_later
- order.finalize!
+ expect { order.complete! }.to change { inventory_unit.reload.pending }.from(true).to(false)
  end
 
- it "sets confirmation delivered when finalizing" do
- expect(order.confirmation_delivered?).to be false
- order.finalize!
- expect(order.confirmation_delivered?).to be true
- end
+ it "should change the shipment state to ready if order is paid" do
+ order.payments.map(&:complete!)
 
- it "should not send duplicate confirmation emails" do
- order.update(confirmation_delivered: true)
- expect(Spree::OrderMailer).not_to receive(:confirm_email)
- order.finalize!
+ expect { order.complete! }.to change { order.shipments.first.state }.from('pending').to('ready')
  end
 
  it "should freeze all adjustments" do
- # Stub this method as it's called due to a callback
- # and it's irrelevant to this test
- allow(Spree::OrderMailer).to receive_message_chain :confirm_email, :deliver_later
- adjustments = [double]
- expect(order).to receive(:all_adjustments).and_return(adjustments)
- adjustments.each do |adj|
- expect(adj).to receive(:finalize!)
- end
- order.finalize!
+ adjustment = create(:adjustment, order: order)
+
+ expect { order.complete! }.to change { adjustment.reload.finalized }.from(false).to(true)
  end
 
  context "order is considered risky" do
@@ -72,7 +41,8 @@
  end
 
  it "should leave order in complete state" do
- order.finalize!
+ order.complete!
+
  expect(order.state).to eq 'complete'
  end
  end
@@ -84,9 +54,84 @@
  end
 
  it "should set completed_at" do
- order.finalize!
+ order.complete!
+
  expect(order.completed_at).to be_present
  end
  end
+
+ context 'with event notifications' do
+ it 'sends an email' do
+ expect(Spree::Config.order_mailer_class).to receive(:confirm_email).and_call_original
+
+ order.complete!
+ end
+
+ it 'marks the order as confirmation_delivered' do
+ expect do
+ order.complete!
+ end.to change(order, :confirmation_delivered).to true
+ end
+
+ it 'sends the email' do
+ expect(Spree::Config.order_mailer_class).to receive(:confirm_email).and_call_original
+
+ order.complete!
+ end
+
+ it "doesn't send duplicate confirmation emails" do
+ order.update(confirmation_delivered: true)
+
+ expect(Spree::OrderMailer).not_to receive(:confirm_email)
+
+ order.complete!
+ end
+
+ # These specs show how notifications can be removed, one at a time or
+ # all the ones set by MailerSubscriber module
+ context 'when removing the default email notification subscription' do
+ before do
+ Spree::MailerSubscriber.deactivate(:order_finalized)
+ end
+
+ after do
+ Spree::MailerSubscriber.activate
+ end
+
+ it 'does not send the email' do
+ expect(Spree::Config.order_mailer_class).not_to receive(:confirm_email)
+
+ order.complete!
+ end
+ end
+
+ context 'when removing all the email notification subscriptions' do
+ before do
+ Spree::MailerSubscriber.deactivate
+ end
+
+ after do
+ Spree::MailerSubscriber.activate
+ end
+
+ it 'does not send the email' do
+ expect(Spree::Config.order_mailer_class).not_to receive(:confirm_email)
+
+ order.complete!
+ end
+ end
+ end
+ end
+
+ context '#finalize!' do
+ it 'deprecates method and goes ahead' do
+ order = create(:order_ready_to_complete)
+
+ expect(Spree::Deprecation).to receive(:warn).with(/discouraged/)
+
+ order.finalize!
+
+ expect(order.state).to eq("confirm")
+ end
  end
 end

core/spec/models/spree/order_spec.rb

@@ -15,58 +15,6 @@
  end
  let(:code) { promotion.codes.first }
 
- describe '#finalize!' do
- context 'with event notifications' do
- it 'sends an email' do
- expect(Spree::Config.order_mailer_class).to receive(:confirm_email).and_call_original
- order.finalize!
- end
-
- it 'marks the order as confirmation_delivered' do
- expect do
- order.finalize!
- end.to change(order, :confirmation_delivered).to true
- end
-
- it 'sends the email' do
- expect(Spree::Config.order_mailer_class).to receive(:confirm_email).and_call_original
- order.finalize!
- end
-
- # These specs show how notifications can be removed, one at a time or
- # all the ones set by MailerSubscriber module
- context 'when removing the default email notification subscription' do
- before do
- Spree::MailerSubscriber.deactivate(:order_finalized)
- end
-
- after do
- Spree::MailerSubscriber.activate
- end
-
- it 'does not send the email' do
- expect(Spree::Config.order_mailer_class).not_to receive(:confirm_email)
- order.finalize!
- end
- end
-
- context 'when removing all the email notification subscriptions' do
- before do
- Spree::MailerSubscriber.deactivate
- end
-
- after do
- Spree::MailerSubscriber.activate
- end
-
- it 'does not send the email' do
- expect(Spree::Config.order_mailer_class).not_to receive(:confirm_email)
- order.finalize!
- end
- end
- end
- end
-
  context '#store' do
  it { is_expected.to respond_to(:store) }
 

guides/source/developers/events/overview.html.md

@@ -123,15 +123,15 @@ and an optional code block can be passed:
 
 ```ruby
 Spree::Event.fire 'order_finalized', order: @order do
- @order.finalize!
+ @order.complete!
 end
 ```
 
 This is an alternative way to basically have the same functionality but
 without the block:
 
 ```ruby
-@order.finalize!
+@order.complete!
 Spree::Event.fire 'order_finalized', order: @order
 ```