From fa8f3f1d10725be47f45830926f8d633b58ad33d Mon Sep 17 00:00:00 2001
From: John Hawthorn <john.hawthorn@gmail.com>
Date: Thu, 15 Feb 2018 12:24:13 -0800
Subject: [PATCH 1/2] Don't use association in batch_builder
Previously this was building codes on the association and then relying
on the autosave behaviour to save the new records when save was called
on the Promotion.
It's simpler to just explicitly create them.
---
.../models/spree/promotion_code/batch_builder.rb | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/core/app/models/spree/promotion_code/batch_builder.rb b/core/app/models/spree/promotion_code/batch_builder.rb
index 0578b825fe5..42de084d8e2 100644
--- a/core/app/models/spree/promotion_code/batch_builder.rb
+++ b/core/app/models/spree/promotion_code/batch_builder.rb
@@ -37,16 +37,13 @@ def generate_random_codes
codes_for_current_batch += get_unique_codes(new_codes)
end
- codes_for_current_batch.map do |value|
- promotion.codes.build(value: value, promotion_code_batch: promotion_code_batch)
+ codes_for_current_batch.each do |value|
+ Spree::PromotionCode.create!(
+ value: value,
+ promotion: promotion,
+ promotion_code_batch: promotion_code_batch
+ )
end
-
- promotion.save!
-
- # We have to reload the promotion because otherwise all promotion codes
- # we are creating will remain in memory. Doing a reload will remove all
- # codes from memory.
- promotion.reload
end
end
From c9051584d5821a9072009c36b34c5f3aaddf2f3c Mon Sep 17 00:00:00 2001
From: John Hawthorn <john.hawthorn@gmail.com>
Date: Thu, 15 Feb 2018 12:28:53 -0800
Subject: [PATCH 2/2] Generate correct number of codes in BatchBuilder
Previously, due to a bug in the inner loop, BatchBuilder could generate
more codes than were requested.
This removes the inner loop in favour of a single, smarter outer loop.
Instead of always creating exactly batch_size new codes, we now create
up to batch_size new codes. On each loop we try to generate batch_size
new potential codes and only save the ones which are unique.
This should result in fewer selects in scenarios where there is
contention over codes. Though this is entire system is not well suited
for there to be many conflicts.
---
.../models/spree/promotion_code/batch_builder.rb | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/core/app/models/spree/promotion_code/batch_builder.rb b/core/app/models/spree/promotion_code/batch_builder.rb
index 42de084d8e2..a13c3f22792 100644
--- a/core/app/models/spree/promotion_code/batch_builder.rb
+++ b/core/app/models/spree/promotion_code/batch_builder.rb
@@ -26,16 +26,13 @@ def build_promotion_codes
private
def generate_random_codes
- total_batches = (number_of_codes.to_f / self.class.batch_size).ceil
+ created_codes = 0
- total_batches.times do |i|
- codes_for_current_batch = Set.new
- codes_to_generate = [self.class.batch_size, number_of_codes - i * batch_size].min
+ while created_codes < number_of_codes
+ max_codes_to_generate = [self.class.batch_size, number_of_codes - created_codes].min
- while codes_for_current_batch.size < codes_to_generate
- new_codes = Array.new(codes_to_generate) { generate_random_code }.to_set
- codes_for_current_batch += get_unique_codes(new_codes)
- end
+ new_codes = Array.new(max_codes_to_generate) { generate_random_code }.uniq
+ codes_for_current_batch = get_unique_codes(new_codes)
codes_for_current_batch.each do |value|
Spree::PromotionCode.create!(
@@ -44,6 +41,7 @@ def generate_random_codes
promotion_code_batch: promotion_code_batch
)
end
+ created_codes += codes_for_current_batch.size
end
end