Skip to content

Commit e2da3b7

Browse files
Kendra Rigasenemsoyadammathys
authored andcommitted
Only log state changes when persisting new state
On the classic order updater, we were persisting updates to shipment and payment states in the respective recalculate methods. We are no longer persisting changes in those methods; now we only save changes in `#persist_totals`, so this is where we should be logging. Co-authored-by: Senem Soy <senem@super.gd> Co-authored-by: Adam Mueller <adam@super.gd> Co-authored-by: Kendra Riga <kendra@super.gd>
1 parent 3d14efb commit e2da3b7

File tree

2 files changed

+36
-17
lines changed

2 files changed

+36
-17
lines changed

core/app/models/spree/in_memory_order_updater.rb

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,7 @@ def recalculate(persist: true)
7474
def recalculate_shipment_state
7575
shipments.each(&:recalculate_state)
7676

77-
log_state_change('shipment') do
78-
order.shipment_state = determine_shipment_state
79-
end
80-
77+
order.shipment_state = determine_shipment_state
8178
order.shipment_state
8279
end
8380
alias_method :update_shipment_state, :recalculate_shipment_state
@@ -93,10 +90,7 @@ def recalculate_shipment_state
9390
#
9491
# The +payment_state+ value helps with reporting, etc. since it provides a quick and easy way to locate Orders needing attention.
9592
def recalculate_payment_state
96-
log_state_change('payment') do
97-
order.payment_state = determine_payment_state
98-
end
99-
93+
order.payment_state = determine_payment_state
10094
order.payment_state
10195
end
10296
alias_method :update_payment_state, :recalculate_payment_state
@@ -237,20 +231,23 @@ def recalculate_item_total
237231
def persist_totals
238232
shipments.each(&:persist_amounts)
239233
assign_item_totals
234+
log_state_change("payment")
235+
log_state_change("shipment")
240236
order.save!
241237
end
242238

243239
def log_state_change(name)
244240
state = "#{name}_state"
245-
old_state = order.public_send(state)
246-
yield
247-
new_state = order.public_send(state)
248-
if old_state != new_state
249-
order.state_changes.new(
250-
previous_state: old_state,
251-
next_state: new_state,
252-
user_id: order.user_id,
253-
name:
241+
previous_state, current_state = order.changes[state]
242+
243+
if previous_state != current_state
244+
# Enqueue the job to track this state change
245+
StateChangeTrackingJob.perform_later(
246+
order,
247+
previous_state,
248+
current_state,
249+
Time.current,
250+
name
254251
)
255252
end
256253
end

core/spec/models/spree/in_memory_order_updater_spec.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,28 @@ module Spree
111111
expect(Rails.logger).not_to have_received(:warn)
112112
end
113113
end
114+
115+
context "when the payment state changes" do
116+
let(:order) { create(:completed_order_with_pending_payment) }
117+
118+
it "logs a state change for the payment" do
119+
expect { order.payments.first.update(state: "completed") }
120+
.to enqueue_job(Spree::StateChangeTrackingJob)
121+
.with(order, "balance_due", "paid", a_kind_of(Time), "payment")
122+
.once
123+
end
124+
end
125+
126+
context "when the shipment state changes" do
127+
let(:order) { create(:order_ready_to_ship) }
128+
129+
it "logs a state change for the shipment" do
130+
expect { order.shipments.first.ship! }
131+
.to enqueue_job(Spree::StateChangeTrackingJob)
132+
.with(order, "ready", "shipped", a_kind_of(Time), "shipment")
133+
.once
134+
end
135+
end
114136
end
115137
end
116138

0 commit comments

Comments
 (0)