Skip to content

Commit

Permalink
Deprecate public visibility of order#finalize!
Browse files Browse the repository at this point in the history
The only invocation of `order#finalize!` comes from the state machine
transaction to `complete`:

https://github.com/solidusio/solidus/blob/e18f34541bcb9b396cc026f9579d01bfae12182e/core/lib/spree/core/state_machines/order.rb#L122

Calling it standalone should not be encouraged, as all the safety checks
implemented as before hooks would be skipped.

Consequently, we're updating the test suite only to reference the public
API. I.e, `Spree::Order#complete!`. That has forced us to refactor some
tests. From `core/spec/models/spree/order/finalizing_spec.rb`:

- `should set completed_at`: Before being called with `:completed_at`
as an argument, `touch` is called by the state machine transition. It's
easier if we just assert on the field value.

- `should sell inventory units`: During the transaction, the shipment's
`#object_id` gets changed, so we can't longer assert on those instances.
Instead, we test the actual behavior. The assertion about
`#update_state` is already covered in the following test.

- `should change the shipment state to ready if order is paid`: We take
the ocassion to assert on the actual behavior.

- `should freeze all adjustments`: There're other methods called on the
collection of adjustments that are not covered by the double. It's
easier to test the actual behavior.

Besides, we're moving some tests about the order completion from
`order_spec.rb` to `finalizing_spec.rb`, also adapting them to test
`#complete!` instead of `finalize!`. These added tests explain the
removal of others which were simple duplication.
  • Loading branch information
waiting-for-dev committed Feb 9, 2022
1 parent e9ca0fb commit 5f22628
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 96 deletions.
2 changes: 2 additions & 0 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,8 @@ def valid_credit_cards

# Finalizes an in progress order after checkout is complete.
# Called after transition to complete state when payments will have been processed
# TODO: Make private on Solidus 4.0.
# @api private
def finalize!
# lock all adjustments (coupon promotions, etc.)
all_adjustments.each(&:finalize!)
Expand Down
121 changes: 77 additions & 44 deletions core/spec/models/spree/order/finalizing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -84,9 +54,72 @@
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
end
52 changes: 0 additions & 52 deletions core/spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

Expand Down

0 comments on commit 5f22628

Please sign in to comment.