Skip to content

Commit 26f8ed9

Browse files
senemsoyforkata
authored andcommitted
Set required line item attributes earlier
This makes working with unpersisted line items easier because required attributes are set immediately. This change is in service of creating an in memory order updater.
1 parent 2bf924b commit 26f8ed9

File tree

3 files changed

+16
-21
lines changed

3 files changed

+16
-21
lines changed

core/app/models/spree/line_item.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class LineItem < Spree::Base
2222
has_many :inventory_units, inverse_of: :line_item
2323

2424
before_validation :normalize_quantity
25-
before_validation :set_required_attributes
25+
after_initialize :set_required_attributes
2626

2727
validates :variant, presence: true
2828
validates :quantity, numericality: {
@@ -159,6 +159,7 @@ def normalize_quantity
159159
# Sets tax category, price-related attributes from
160160
# its variant if they are nil and a variant is present.
161161
def set_required_attributes
162+
return if persisted?
162163
return unless variant
163164
self.tax_category ||= variant.tax_category
164165
set_pricing_attributes

core/spec/models/spree/line_item_spec.rb

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,31 +39,25 @@
3939
end
4040
end
4141

42-
describe 'line item creation' do
42+
describe '.new' do
4343
let(:variant) { create :variant }
4444

4545
subject(:line_item) { Spree::LineItem.new(variant:, order:) }
4646

47-
# Tests for https://github.com/spree/spree/issues/3391
48-
context 'before validation' do
49-
before { line_item.valid? }
50-
51-
it 'copies the variants price' do
52-
expect(line_item.price).to eq(variant.price)
53-
end
47+
it 'copies the variants price' do
48+
expect(line_item.price).to eq(variant.price)
49+
end
5450

55-
it 'copies the variants cost_price' do
56-
expect(line_item.cost_price).to eq(variant.cost_price)
57-
end
51+
it 'copies the variants cost_price' do
52+
expect(line_item.cost_price).to eq(variant.cost_price)
53+
end
5854

59-
it "copies the order's currency" do
60-
expect(line_item.currency).to eq(order.currency)
61-
end
55+
it "copies the order's currency" do
56+
expect(line_item.currency).to eq(order.currency)
57+
end
6258

63-
# Test for https://github.com/spree/spree/issues/3481
64-
it 'copies the variants tax category' do
65-
expect(line_item.tax_category).to eq(line_item.variant.tax_category)
66-
end
59+
it 'copies the variants tax category' do
60+
expect(line_item.tax_category).to eq(line_item.variant.tax_category)
6761
end
6862
end
6963

legacy_promotions/spec/models/spree/promotion/rules/product_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@
134134
let(:other_line_item) { Spree::LineItem.new(product: other_product) }
135135

136136
let(:rule_options) { super().merge(products: [rule_product]) }
137-
let(:rule_product) { mock_model(Spree::Product) }
138-
let(:other_product) { mock_model(Spree::Product) }
137+
let(:rule_product) { mock_model(Spree::Product, tax_category: nil) }
138+
let(:other_product) { mock_model(Spree::Product, tax_category: nil) }
139139

140140
context "with 'any' match policy" do
141141
let(:rule_options) { super().merge(preferred_match_policy: 'any') }

0 commit comments

Comments
 (0)