solidus
solidus copied to clipboard
Unify `Spree::User` and `Spree::Order` email validation
(not sure if this is more a bug or a new feature... I'm going for the latter as I think the solution will end up being a new feature 😄 )
At the moment we have a custom validator for the email in Spree::Order
. This validator has different logic than the one used in solidus_auth_devise
, so there may be instances when the user is valid, but the order is not. This creates confusion and can affect shop sales negatively when the issue goes unnoticed.
One possible course of action may be to use the same validator from solidus_auth_devise
, which happens to be the one used by devise
, as it's more used (if not respectful of standards) than ours... but only when the application is using solidus_auth_devise
as well. More in general, if a user validator for email exists, the order should use that one instead of the one provided by Solidus.
email = '[email protected]'
u = Spree::User.new(email: email)
u.valid?
u.errors[:email]
# => []
o = Spree::Order.new(email: email)
o.valid?
o.errors[:email]
# => ["is invalid"]
A bit more context on why this can be insidious, again from Solidus sandbox:
u = Spree::User.create email: '[email protected]', password: 'password', password_confirmation: 'password'
o = u.orders.create
o.persisted?
# => true
o.valid?
# => false
o.errors[:email]
=> ["is invalid"]
The order is valid before being persisted, as the email field is blank. After the order is persisted, it becomes invalid as the email field, copied from the user, is not valid according to the validator.
Hi there, I took a look at devise
email validation and it turns out to be a regexp
similar to Spree::EmailValidator::EMAIL_REGEXP
. See
https://github.com/plataformatec/devise/blob/b52e642c0131f7b0d9f2dd24d8607a186f18223e/lib/devise.rb#L116
and
https://github.com/plataformatec/devise/blob/07f2712a22aa05b8da61c85307b80a3bd2ed6c4c/lib/devise/models/validatable.rb#L34
Then, replacing Spree::EmailValidator::EMAIL_REGEXP
by Devise::email_regexp
when Devise
is defined would fix the issue and all commands you presented would be okay.
core/lib/spree/core/validators/email.rb
- unless EMAIL_REGEXP.match? value
+ email_regexp = defined?(Devise) && defined?(Devise::email_regexp) ? Devise::email_regexp : EMAIL_REGEXP
+ unless email_regexp.match? value
WDYT?
For what it's worth, the email regexp used in orders can now be configured: https://github.com/solidusio/solidus/blob/38ef1644f8ae03df8c83f2f28b0258669453722f/core/lib/spree/app_configuration.rb#L156 We should probably have solidus_auth_devise use the same one.