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

[RFC] Rename order.update! to order.recalculate #2072

Merged
merged 1 commit into from
Jul 12, 2017
Merged
Show file tree
Hide file tree
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
Rename order.update! to order.recalculate
  • Loading branch information
jhawthorn committed Jul 7, 2017
commit a1dc52653032d003120a0df5d881ad5180359f38
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def find_adjustment
end

def update_totals
@order.reload.update!
@order.reload.recalculate
end

# Override method used to create a new instance to correctly
Expand Down
2 changes: 1 addition & 1 deletion backend/spec/features/admin/orders/adjustments_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
let!(:adjustment) { order.adjustments.create!(order: order, label: 'Rebate', amount: 10) }

before(:each) do
order.update!
order.recalculate

visit spree.admin_path
click_link "Orders"
Expand Down
21 changes: 15 additions & 6 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,19 @@ def updater
@updater ||= Spree::OrderUpdater.new(self)
end

def update!
def recalculate
updater.update
end

def update!(*args)
if args.empty?
Spree::Deprecation.warn "Calling order.update! with no arguments as a way to invoke the OrderUpdater is deprecated, since it conflicts with AR::Base#update! Please use order.recalculate instead"
recalculate
else
super
end
end

def assign_billing_to_shipping_address
self.ship_address = bill_address if bill_address
true
Expand Down Expand Up @@ -471,7 +480,7 @@ def empty!
shipments.destroy_all
order_promotions.destroy_all

update!
recalculate
end

alias_method :has_step?, :has_checkout_step?
Expand Down Expand Up @@ -529,7 +538,7 @@ def create_proposed_shipments

def apply_free_shipping_promotions
Spree::PromotionHandler::FreeShipping.new(self).activate
update!
recalculate
end

# Clean shipments and make order back to address state
Expand Down Expand Up @@ -565,7 +574,7 @@ def shipping_eq_billing_address?

def set_shipments_cost
shipments.each(&:update_amounts)
update!
recalculate
end
deprecate set_shipments_cost: :update!, deprecator: Spree::Deprecation

Expand Down Expand Up @@ -806,7 +815,7 @@ def ensure_promotions_eligible
end
if adjustment_changed
restart_checkout_flow
update!
recalculate
errors.add(:base, Spree.t(:promotion_total_changed_before_complete))
end
errors.empty?
Expand Down Expand Up @@ -838,7 +847,7 @@ def after_cancel
payments.store_credits.pending.each(&:void_transaction!)

send_cancel_email
update!
recalculate
end

def send_cancel_email
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/order/checkout.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def define_state_machine!
after_transition to: :canceled, do: :after_cancel

after_transition from: any - :cart, to: any - [:confirm, :complete] do |order|
order.update!
order.recalculate
end

after_transition do |order, transition|
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/order_cancellations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def short_ship(inventory_units, whodunnit:nil)
Spree::OrderMailer.inventory_cancellation_email(@order, inventory_units.to_a).deliver_later if Spree::OrderCancellations.send_cancellation_mailer
end

@order.update!
@order.recalculate

if short_ship_tax_notifier
short_ship_tax_notifier.call(unit_cancels)
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/order_shipping.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def ship(inventory_units:, stock_location:, address:, shipping_method:,

send_shipment_emails(carton) if stock_location.fulfillable? && !suppress_mailer # e.g. digital gift cards that aren't actually shipped
fulfill_order_stock_locations(stock_location)
@order.update!
@order.recalculate

carton
end
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/payment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ def invalidate_old_payments

def update_order
if order.completed? || completed? || void?
order.update!
order.recalculate
end
end

Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/promotion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def activate(order:, line_item: nil, user: nil, path: nil, promotion_code: nil)
action_taken
end

# called anytime order.update! happens
# called anytime order.recalculate happens
def eligible?(promotable, promotion_code: nil)
return false if inactive?
return false if usage_limit_exceeded?
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/promotion_handler/coupon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def determine_promotion_application_result
discount ||= order.adjustments.promotion.detect(&detector)

if discount && discount.eligible
order.update!
order.recalculate
set_success_code :coupon_code_applied
elsif order.promotions.with_coupon_code(order.coupon_code)
# if the promotion exists on an order, but wasn't found above,
Expand Down
4 changes: 2 additions & 2 deletions core/app/models/spree/shipment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,9 @@ def update_attributes_and_order(params = {})
if update_attributes params
if params.key? :selected_shipping_rate_id
# Changing the selected Shipping Rate won't update the cost (for now)
# so we persist the Shipment#cost before running `order.update!`
# so we persist the Shipment#cost before running `order.recalculate`
update_amounts
order.update!
order.recalculate
end

true
Expand Down
2 changes: 1 addition & 1 deletion core/lib/spree/testing_support/factories/order_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
create(:shipment, order: order, cost: evaluator.shipment_cost, shipping_method: evaluator.shipping_method, stock_location: evaluator.stock_location)
order.shipments.reload

order.update!
order.recalculate
end

factory :completed_order_with_promotion do
Expand Down
4 changes: 2 additions & 2 deletions core/spec/models/spree/order/checkout_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ def assert_state_changed(order, from, to)
context "with a shipment that has a price" do
before do
shipment.shipping_rates.first.update_column(:cost, 10)
order.update!
order.recalculate
end

it "transitions to payment" do
Expand Down Expand Up @@ -595,7 +595,7 @@ def assert_state_changed(order, from, to)

it 'can complete the order' do
create(:payment, state: 'completed', order: order, amount: order.total)
order.update!
order.recalculate
expect(order.complete).to eq(true)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

subject do
order.reload
order.update!
order.recalculate
order.outstanding_balance
end

Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/order/payment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ module Spree
payment: reimbursement.order.payments.first,
reimbursement: reimbursement
# Update the order totals so payment_total goes to 0 reflecting the refund..
order.update!
order.recalculate
end

context "for canceled orders" do
Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/order/updating_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
after { Spree::Order.update_hooks.clear }
it "should call each of the update hooks" do
expect(order).to receive :foo
order.update!
order.recalculate
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/order_cancellations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
source: promotion_action,
finalized: true,
)
order.update!
order.recalculate
end

it "generates the correct total amount" do
Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/order_capturing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@

before do
order.contents.add(variant, 3)
order.update!
order.recalculate
@secondary_bogus_payment = create(:payment, order: order, amount: secondary_total, payment_method: secondary_payment_method.create!(name: 'So bogus'))
@bogus_payment = create(:payment, order: order, amount: bogus_total)
order.contents.advance
Expand Down
10 changes: 5 additions & 5 deletions core/spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@
create(:shipment, order: order)
create(:adjustment, adjustable: order, order: order)
promotion.activate(order: order, promotion_code: code)
order.update!
order.recalculate

# Make sure we are asserting changes
expect(order.line_items).not_to be_empty
Expand Down Expand Up @@ -313,7 +313,7 @@ def merge!(other_order, user = nil)
it "calls hook during update" do
order = create(:order)
expect(order).to receive(:add_awesome_sauce)
order.update!
order.recalculate
end

it "calls hook during finalize" do
Expand Down Expand Up @@ -1182,7 +1182,7 @@ def generate

context "there is enough store credit to pay for the entire order" do
let(:store_credit) { create(:store_credit, amount: order_total) }
let(:order) { create(:order_with_totals, user: store_credit.user, line_items_price: order_total).tap(&:update!) }
let(:order) { create(:order_with_totals, user: store_credit.user, line_items_price: order_total).tap(&:recalculate) }

context "there are no other payments" do
before do
Expand Down Expand Up @@ -1210,7 +1210,7 @@ def generate
let(:order_total) { 500 }
let(:store_credit_total) { order_total - 100 }
let(:store_credit) { create(:store_credit, amount: store_credit_total) }
let(:order) { create(:order_with_totals, user: store_credit.user, line_items_price: order_total).tap(&:update!) }
let(:order) { create(:order_with_totals, user: store_credit.user, line_items_price: order_total).tap(&:recalculate) }

context "there are no other payments" do
it "adds an error to the model" do
Expand Down Expand Up @@ -1255,7 +1255,7 @@ def generate
let(:amount_difference) { 100 }
let!(:primary_store_credit) { create(:store_credit, amount: (order_total - amount_difference)) }
let!(:secondary_store_credit) { create(:store_credit, amount: order_total, user: primary_store_credit.user, credit_type: create(:secondary_credit_type)) }
let(:order) { create(:order_with_totals, user: primary_store_credit.user, line_items_price: order_total).tap(&:update!) }
let(:order) { create(:order_with_totals, user: primary_store_credit.user, line_items_price: order_total).tap(&:recalculate) }

before do
subject
Expand Down
26 changes: 13 additions & 13 deletions core/spec/models/spree/order_updater_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ module Spree

before do
promotion.activate(order: order)
order.update!
order.recalculate
line_item.update!(quantity: 2)
end

it 'updates the promotion amount' do
expect {
order.update!
order.recalculate
}.to change {
line_item.promo_total
}.from(-1).to(-2)
Expand All @@ -117,7 +117,7 @@ def initialize(_adjustments)
end

it 'uses the defined promotion chooser' do
expect { order.update! }.to raise_error('Custom promotion chooser')
expect { order.recalculate }.to raise_error('Custom promotion chooser')
end
end

Expand Down Expand Up @@ -156,7 +156,7 @@ def create_adjustment(label, amount)
label: 'Some other credit')
line_item.adjustments.each { |a| a.update_column(:eligible, true) }

order.update!
order.recalculate

expect(line_item.adjustments.promotion.eligible.count).to eq(1)
expect(line_item.adjustments.promotion.eligible.first.label).to eq('Promotion C')
Expand All @@ -170,7 +170,7 @@ def create_adjustment(label, amount)
end
line_item.adjustments.each { |a| a.update_column(:eligible, true) }

order.update!
order.recalculate

expect(line_item.adjustments.promotion.eligible.count).to eq(1)
expect(line_item.adjustments.promotion.eligible.first.label).to eq('Promotion B')
Expand All @@ -184,7 +184,7 @@ def create_adjustment(label, amount)
end
line_item.adjustments.each { |a| a.update_column(:eligible, true) }

order.update!
order.recalculate

expect(line_item.adjustments.promotion.eligible.count).to eq(1)
expect(line_item.adjustments.promotion.eligible.first.label).to eq('Promotion B')
Expand Down Expand Up @@ -212,7 +212,7 @@ def create_adjustment(label, amount)
order_promos[promo_sequence[0]].activate order: order
order_promos[promo_sequence[1]].activate order: order

order.update!
order.recalculate
order.reload
expect(order.all_adjustments.count).to eq(2), 'Expected two adjustments'
expect(order.all_adjustments.eligible.count).to eq(1), 'Expected one elegible adjustment'
Expand Down Expand Up @@ -263,7 +263,7 @@ def create_adjustment(label, amount)

# regression for https://github.com/spree/spree/issues/3274
it 'still makes the previous best eligible adjustment valid' do
order.update!
order.recalculate
expect(line_item.adjustments.promotion.eligible.first.label).to eq('Promotion A')
end
end
Expand All @@ -273,7 +273,7 @@ def create_adjustment(label, amount)
create_adjustment('Promotion B', -200)
create_adjustment('Promotion C', -200)

order.update!
order.recalculate

expect(line_item.adjustments.promotion.eligible.count).to eq(1)
expect(line_item.adjustments.promotion.eligible.first.amount.to_i).to eq(-200)
Expand Down Expand Up @@ -305,7 +305,7 @@ def create_adjustment(label, amount)

it 'updates the promotion amount' do
expect {
order.update!
order.recalculate
}.to change {
line_item.additional_tax_total
}.from(1).to(2)
Expand All @@ -327,7 +327,7 @@ def create_adjustment(label, amount)
Spree::Tax::OrderTax.new(order_id: order.id, line_item_taxes: [], shipment_taxes: [])
)

order.update!
order.recalculate
end
end
end
Expand Down Expand Up @@ -535,7 +535,7 @@ def create_adjustment(label, amount)
expect(line_item.promo_total).to eq(0)
expect(order.promo_total).to eq(0)

order.update!
order.recalculate

expect(line_item.promo_total).to eq(-1)
expect(order.promo_total).to eq(-1)
Expand All @@ -548,7 +548,7 @@ def create_adjustment(label, amount)
it "updates the totals" do
line_item.update!(adjustment_total: 100)
expect {
order.update!
order.recalculate
}.to change { line_item.reload.adjustment_total }.from(100).to(0)
end
end
Expand Down
Loading