search

solidusio / solidus_auth_devise

🔑 Devise authentication for your Solidus store.
http://solidus.io
BSD 3-Clause "New" or "Revised" License
52 stars 124 forks source link

Change Devise's initializer email_regexp #208

Closed cesartalves closed 2 years ago

cesartalves commented 3 years ago

This solves a problem in the interaction between solidus_auth_devise and solidus Order association - specifically in situations where a User email might be valid according to Devise's default email_regexp but, upon being copied to the Order, invalidate the latter as the regex on Spree::EmailValidator is much less permissive. This change makes the email_regexp for Devise the same as the regexp for Spree::EmailValidator.

For deeper analysis see https://github.com/solidusio/solidus/issues/3427 and https://github.com/solidusio/solidus/pull/3927

spaghetticode commented 3 years ago

The regexp seems to be duplicated from Solidus. Do we have any plan for having a single source of truth? While I think this not-DRY solution is still better than the current situation, I think it's not optimal having to update this regexp on both projects.

cesartalves commented 3 years ago

The regexp seems to be duplicated from Solidus. Do we have any plan for having a single source of truth? While I think this not-DRY solution is still better than the current situation, I think it's not optimal having to update this regexp on both projects.

I dunno. It seems to me that using one of the repos as a source of truth necessarily means using some form of reflection or tying one of them to the other (like using defined? Devise).

I think is this case having a "duplication" is ok as we prefer keeping repositories independent rather than ensuring we only need to change things in one place. Lastly, I feel that we would rarely change this regex anyway (probably never).

What do you think? @spaghetticode

cesartalves commented 3 years ago

@spaghetticode and I discussed the matter and we came to the conclusion that introducing a Spree::Config.email_regexp that was easily customizable and both the EmailValidator and Devise.email_regexp would be a good idea. Then we could Devise.email_regexp = Spree::Config.email_regexp in the generated initializer.

However, I would be interested in knowing, do Spree Preferences allow regex type ones? I think this would not be the case, we would probably have to store the preference as a string and convert it.