Skip to content

Commit 0e6c01d

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 32f86ba commit 0e6c01d

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
@@ -244,20 +238,23 @@ def recalculate_item_total
244238
def persist_totals
245239
shipments.each(&:persist_amounts)
246240
persist_item_totals
241+
log_state_change("payment")
242+
log_state_change("shipment")
247243
order.save!
248244
end
249245

250246
def log_state_change(name)
251247
state = "#{name}_state"
252-
old_state = order.public_send(state)
253-
yield
254-
new_state = order.public_send(state)
255-
if old_state != new_state
256-
order.state_changes.new(
257-
previous_state: old_state,
258-
next_state: new_state,
259-
user_id: order.user_id,
260-
name:
248+
previous_state, current_state = order.changes[state]
249+
250+
if previous_state != current_state
251+
# Enqueue the job to track this state change
252+
StateChangeTrackingJob.perform_later(
253+
order,
254+
previous_state,
255+
current_state,
256+
Time.current,
257+
name
261258
)
262259
end
263260
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)