-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Backport #4228 to V2.11 #4230
Backport #4228 to V2.11 #4230
Conversation
Promotion codes are always stored in the database in lowercase. Up to this commit, the transformation of the in-memory value of a code was performed in a `before_save` callback. This implementation led to a weird behavior that can easily be reproduced with a fresh Solidus install: - create a first promotion, with a single promotion code (e.g. "test") - attempt to create another promotion, with the exact same value for the single promotion code => triggers a flash error messages which states "Codes is invalid" - attempt to re-save the same form, this time with a single promotion code with the value "TEST" => the page crashes due to an `ActiveRecord::RecordNotUnique` error The expected error handling would be that any input that would end up matching an existing code (after having been stripped of spaces & downcased) should trigger the same flash error rather than crash the page. This behavior can be achieved by normalizing values earlier in a `before_validation` callback: that way, the validation callback will query the database using a stripped/downcased value and, should a match be found, set an error on the model instance as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, but if we want to backport I think we also need to do it on 3.0.
My bad, I forgot there already was a new minor version for solidus 3. I'll open another PR right away! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
We're forgetting the most recent minor version: v3.1. We need to backport there 🙂 |
Hello @waiting-for-dev, happy New Year! Here is the PR for 3.1 => #4237 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! And Happy New Year @jcsanti! 🥳
Many thanks to @waiting-for-dev & @kennyadsl for the quick review of #4228 🙂🚀
Description
This PR backports #4228 to Solidus 2.11.
Checklist:
[ ] I have updated Guides and README accordingly to this change (if needed)