Skip to content

Commit d4463ad

Browse files
authored
Merge pull request #6493 from ikraamg/perf/order-number-uniqueness-on-change
Validate order number uniqueness only when it changes
2 parents 7b576fe + a84c051 commit d4463ad

2 files changed

Lines changed: 20 additions & 1 deletion

File tree

core/app/models/spree/order.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ def states
151151
validates :email, presence: true, if: :email_required?
152152
validates :email, "spree/email" => true, :allow_blank => true
153153
validates :guest_token, presence: {allow_nil: true}
154-
validates :number, presence: true, uniqueness: {allow_blank: true, case_sensitive: true}
154+
validates :number, presence: true
155+
validates :number, uniqueness: {allow_blank: true, case_sensitive: true}, if: :number_changed?
155156
validates :store_id, presence: true
156157

157158
def self.find_by_param(value)

core/spec/models/spree/order_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,24 @@ def call
920920
end
921921
end
922922

923+
describe "number uniqueness" do
924+
let(:existing_order) { create(:order) }
925+
926+
it "rejects a new order that reuses an existing number" do
927+
expect(build(:order, number: existing_order.number)).not_to be_valid
928+
end
929+
930+
it "rejects changing a persisted order's number to one already taken" do
931+
order.number = existing_order.number
932+
expect(order).not_to be_valid
933+
end
934+
935+
it "skips the uniqueness query when the number is unchanged" do
936+
order
937+
expect { order.valid? }.not_to make_database_queries(matching: /SELECT 1 .*spree_orders.*number/i)
938+
end
939+
end
940+
923941
context "#associate_user!" do
924942
let!(:user) { FactoryBot.create(:user) }
925943

0 commit comments

Comments
 (0)