Skip to content

Commit 75effa5

Browse files
committed
Autosave shipment adjustments to persist taxes on recalculate
A commit was made in #6315 (5979f25) in service of the in memory order updater #5872 that changed the behavior in the OrderTaxation class to set the adjustment value but not persist it. Instead, the persistence happens when the order is saved. If a shipment is changed, and an associated adjustment is changed, the shipment adjustment will be autosaved when the order is. However, if a shipment is *not* changed, but the adjustment is, the adjustment will *not* be autosaved when the order is. In #6315, we ensured that these changed associations cannot get left behind by setting `autosave: true`, which forces the model to check if it has any associations that need saving instead of telling it's relation "nope I haven't changed" We can do the same thing here for the shipment to ensure changing adjustments will make the shipment tell it's order "I have changed", even if none of it's direct properties have changed. You can test this change by removing the `autosave: true` option the adjustments and running 'when the address has changed to a different state' specs. They fail with the error "expected `order.shipments.first.adjustments.first.amount` to have changed from 1 to 2, but did not change", demonstrating a scenario where the shipment adjustment changes do not get persisted.
1 parent d070f94 commit 75effa5

File tree

2 files changed

+60
-14
lines changed

2 files changed

+60
-14
lines changed

core/app/models/spree/shipment.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class Shipment < Spree::Base
99
belongs_to :order, class_name: 'Spree::Order', touch: true, inverse_of: :shipments, optional: true
1010
belongs_to :stock_location, class_name: 'Spree::StockLocation', optional: true
1111

12-
has_many :adjustments, as: :adjustable, inverse_of: :adjustable, dependent: :delete_all
12+
has_many :adjustments, as: :adjustable, inverse_of: :adjustable, dependent: :delete_all, autosave: true
1313
has_many :inventory_units, dependent: :destroy, inverse_of: :shipment
1414
has_many :shipping_rates, -> { order(:cost) }, dependent: :destroy, inverse_of: :shipment
1515
has_many :shipping_methods, through: :shipping_rates

core/spec/models/spree/order_updater_spec.rb

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,14 @@ module Spree
7272
let(:tax_category) { create(:tax_category) }
7373
let(:ship_address) { create(:address, state: new_york) }
7474
let(:new_york) { create(:state, state_code: "NY") }
75-
let(:tax_zone) { create(:zone, states: [new_york]) }
75+
let(:new_york_tax_zone) { create(:zone, states: [new_york]) }
7676

77-
let!(:tax_rate) do
77+
let!(:new_york_tax_rate) do
7878
create(
7979
:tax_rate,
8080
name: "New York Sales Tax",
8181
tax_categories: [tax_category],
82-
zone: tax_zone,
82+
zone: new_york_tax_zone,
8383
included_in_price: false,
8484
amount: 0.1
8585
)
@@ -111,21 +111,67 @@ module Spree
111111
end
112112

113113
context 'when the address has changed to a different state' do
114-
let(:new_shipping_address) { create(:address) }
114+
let(:oregon) { create(:state, state_code: "OR") }
115+
let(:oregon_tax_zone) { create(:zone, states: [oregon]) }
116+
let!(:oregon_tax_rate) do
117+
create(
118+
:tax_rate,
119+
name: "Oregon Sales Tax",
120+
tax_categories: [tax_category],
121+
zone: oregon_tax_zone,
122+
included_in_price: false,
123+
amount: 0.2
124+
)
125+
end
126+
let(:new_address) { create(:address, state: oregon) }
127+
let(:shipping_method) { create(:shipping_method, tax_category:, zones: [oregon_tax_zone, new_york_tax_zone], cost: 10) }
128+
let(:shipping_rate) do
129+
create(:shipping_rate, cost: 10, shipping_method: shipping_method)
130+
end
131+
let(:shipment) { order.shipments[0] }
132+
133+
subject do
134+
order.ship_address = new_address
135+
order.bill_address = new_address
136+
137+
order.recalculate
138+
end
115139

116140
before do
117-
order.ship_address = new_shipping_address
141+
shipment.shipping_rates = [shipping_rate]
142+
shipment.selected_shipping_rate_id = shipping_rate.id
143+
order.recalculate
118144
end
119145

120-
it 'removes the old taxes' do
146+
it 'updates the taxes to reflect the new state' do
121147
expect {
122-
order.recalculate
148+
subject
123149
}.to change {
124-
order.all_adjustments.tax.count
125-
}.from(1).to(0)
150+
order.additional_tax_total
151+
}.from(2).to(4)
152+
end
126153

127-
expect(order.additional_tax_total).to eq 0
128-
expect(order.adjustment_total).to eq 0
154+
it 'updates the shipment taxes to reflect the new state' do
155+
$started_spec = true
156+
expect {
157+
subject
158+
}.to change {
159+
order.shipments.first.additional_tax_total
160+
}.from(1).to(2)
161+
.and change {
162+
order.shipments.first.adjustments.first.amount
163+
}.from(1).to(2)
164+
end
165+
166+
it 'updates the line item taxes to reflect the new state' do
167+
expect {
168+
subject
169+
}.to change {
170+
order.line_items.first.additional_tax_total
171+
}.from(1).to(2)
172+
.and change {
173+
order.line_items.first.adjustments.first.amount
174+
}.from(1).to(2)
129175
end
130176
end
131177

@@ -179,7 +225,7 @@ module Spree
179225
order_taxes: [
180226
Spree::Tax::ItemTax.new(
181227
label: "Delivery Fee",
182-
tax_rate:,
228+
tax_rate: new_york_tax_rate,
183229
amount: 2.60,
184230
included_in_price: false
185231
)
@@ -188,7 +234,7 @@ module Spree
188234
Spree::Tax::ItemTax.new(
189235
item_id: line_item.id,
190236
label: "Item Tax",
191-
tax_rate:,
237+
tax_rate: new_york_tax_rate,
192238
amount: 1.40,
193239
included_in_price: false
194240
)

0 commit comments

Comments
 (0)