Fix PromotionMigrator duplicate codes when promotions share base_code#6422
Open
NewAlexandria wants to merge 2 commits intosolidusio:mainfrom
Open
Fix PromotionMigrator duplicate codes when promotions share base_code#6422NewAlexandria wants to merge 2 commits intosolidusio:mainfrom
NewAlexandria wants to merge 2 commits intosolidusio:mainfrom
Conversation
github-actions
bot
added
the
changelog:solidus_promotions
mamhoff
reviewed
Mar 3, 2026
| @@ -0,0 +1,60 @@ | |||
| # PromotionMigrator duplicate code fix | |||
Contributor
There was a problem hiding this comment.
I think this documentation should be the commit message, rather than be in the docs folder.
Author
There was a problem hiding this comment.
I prompted the change. Please let me know if this is what you're thinking or if you wanted the exact text/the whole file. Thanks.
Contributor
There was a problem hiding this comment.
Please rebase the change into a single commit. Thank you!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6422 +/- ##
==========================================
+ Coverage 89.50% 89.51% +0.01%
==========================================
Files 981 981
Lines 20477 20504 +27
==========================================
+ Hits 18328 18355 +27
Misses 2149 2149 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mamhoff
reviewed
Mar 11, 2026
Constrain the promotion code batch join in copy_promotion_codes by promotion_id and created_at to prevent cross-promotion row multiplication and duplicate value inserts. Adds a regression spec and an audit doc summarizing root cause, release verification, and related issue/PR research.
mamhoff
force-pushed
the
fix/promotion-migrator-duplicate-codes-join
branch
from
6280b82 to
59d2748
Compare
…ase_code
During migration from solidus_legacy_promotions to solidus_promotions,
PromotionMigrator#copy_promotion_codes joins legacy code batches to new
code batches. Previously the join matched only on base_code, which is not
globally unique across promotions. Multiple promotions with batches sharing
the same base_code could multiply rows in the INSERT ... SELECT and attempt
to insert duplicate values into solidus_promotions_promotion_codes, causing
PG::UniqueViolation on index_solidus_promotions_promotion_codes_on_value.
Constrain the LEFT OUTER JOIN on solidus_promotions_promotion_code_batches
with promotion_id = #{new_promotion.id} and created_at equal to the source
batch so each migrated code row resolves to the correct batch for the
promotion being migrated.
Validation: regression spec covers two legacy promotions with PromotionCodeBatch
records sharing the same base_code; migration inserts each code once.
Related research: searched solidusio/solidus issues/PRs for PromotionMigrator,
copy_promotion_codes, base_code, and duplicate-key migration failures; no
prior fix for this join. Issue solidusio#1248 concerns an older spree_promotions.code
migration path and is not the same bug.
Reviewer follow-up: move narrative out of docs/ into this message; drop the
unnecessary Integer() cast on new_promotion.id in the SQL interpolation.
Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi — we ran into this during our promotion migration and wanted to see if this would contribute the fix back.
We may be missing something about the intended behavior, so would appreciate a sanity check. Thanks for your time, of course.
I also added
promotions/docs/PROMOTION_MIGRATOR_DUPLICATE_CODES_FIX.mdwith some RCA context, where we saw it, and version info.@mrshahsuvie FYI
(an llm summary follows)
Summary
Fixes
PG::UniqueViolationduring promotion migration when multiple promotions have batches with the samebase_code.Problem
copy_promotion_codesjoins tosolidus_promotions_promotion_code_batchesonbase_codeonly. If two promotions have batches with the same base code, that join can produce multiple rows per legacy code and attempt duplicatevalueinserts intosolidus_promotions_promotion_codes.Solution
Constrain the batch join by:
promotion_id = new_promotion.idcreated_at = spree_promotion_code_batches.created_atThis prevents cross-promotion row multiplication while preserving expected batch matching.
Testing
promotions/spec/lib/solidus_promotions/promotion_migrator_spec.rbfor two promotions sharing the same batchbase_code.bundle exec rspec spec/lib/solidus_promotions/promotion_migrator_spec.rbbundle exec rspec(promotions engine suite)