Skip to content

Commit 84c1ce4

Browse files
committed
Conditionally persist Shipment#update_amounts changes
This is in service of supporting the InMemoryOrderUpdater's goal to not do database writes.
1 parent 5ecd078 commit 84c1ce4

File tree

2 files changed

+28
-9
lines changed

2 files changed

+28
-9
lines changed

core/app/models/spree/shipment.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,10 @@ def tracking_url
258258
@tracking_url ||= shipping_method.build_tracking_url(tracking)
259259
end
260260

261-
def update_amounts
261+
def update_amounts(persist: true)
262262
if selected_shipping_rate
263263
self.cost = selected_shipping_rate.cost
264-
if changed?
264+
if changed? && persist
265265
update_columns(
266266
cost: cost,
267267
updated_at: Time.current

core/spec/models/spree/shipment_spec.rb

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -508,22 +508,41 @@
508508
end
509509

510510
describe "#update_amounts" do
511-
let(:shipment) { create(:shipment, cost: 3) }
511+
subject { shipment.update_amounts(persist: persist) }
512+
513+
let(:persist) { true }
514+
let(:shipment) { create(:shipment, cost: 1) }
512515

513516
context 'when the selected shipping rate cost is different than the current shipment cost' do
514-
before { shipment.selected_shipping_rate.update!(cost: 5) }
517+
before { shipment.selected_shipping_rate.update!(cost: 999) }
515518

516-
it "updates the shipments cost" do
519+
it "changes and persists the shipments cost" do
517520
expect {
518-
shipment.update_amounts
519-
}.to change { shipment.reload.cost }.to(5)
521+
subject
522+
}.to change { shipment.reload.cost }.to(999)
520523
end
521524

522-
it 'changes the updated_at column' do
525+
it 'changes and persists the updated_at column' do
523526
expect {
524-
shipment.update_amounts
527+
subject
525528
}.to change { shipment.reload.updated_at }
526529
end
530+
531+
context 'when `persist: false` is passed' do
532+
let(:persist) { false }
533+
534+
it 'does not perform any database writes' do
535+
expect {
536+
subject
537+
}.not_to make_database_queries(manipulative: true)
538+
end
539+
540+
it "changes but does not persist the shipments cost" do
541+
subject
542+
expect(shipment.cost).to eq 999
543+
expect(shipment.reload.cost).to eq 1
544+
end
545+
end
527546
end
528547
end
529548

0 commit comments

Comments
 (0)