Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate public visibility of order#finalize! #4260

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 30 additions & 18 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion core/lib/spree/core/state_machines/order.rb
Original file line number Diff line number Diff line change
@@ -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

2 changes: 1 addition & 1 deletion core/lib/spree/event.rb
Original file line number Diff line number Diff line change
@@ -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
133 changes: 89 additions & 44 deletions core/spec/models/spree/order/finalizing_spec.rb
Original file line number Diff line number Diff line change
@@ -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
52 changes: 0 additions & 52 deletions core/spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
@@ -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) }

4 changes: 2 additions & 2 deletions guides/source/developers/events/overview.html.md
Original file line number Diff line number Diff line change
@@ -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
```