Skip to content
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

Add email validation scoping #5136

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

christianrolle
Copy link

For multi tenancy the email validation scoping is
crucial. This enhancement is actually based on the
currently open PR:
#5094
As soon as the feature is merged, one should use the
Devise trunk again.

For multi tenancy the email validation scoping is
crucial. This enhancement is actually based on the
currently open PR:
heartcombo#5094
As soon as the feature is merged, one should use the
Devise trunk again.
@sourcelevel-bot
Copy link

Hello, @christianrolle! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

@@ -115,6 +115,10 @@ module Test
mattr_accessor :email_regexp
@@email_regexp = /\A[^@\s]+@[^@\s]+\z/

# Option to scope the email uniqueness validator
mattr_accessor :email_scope
@@email_scope = nil

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace class var @@email_scope with a class instance var.

@@ -115,6 +115,10 @@ module Test
mattr_accessor :email_regexp
@@email_regexp = /\A[^@\s]+@[^@\s]+\z/

# Option to scope the email uniqueness validator
mattr_accessor :email_scope
@@email_scope = nil

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devise declares the class variable '@@email_scope'

Read more about it here.

@sourcelevel-bot
Copy link

SourceLevel has finished reviewing this Pull Request and has found:

  • 2 possible new issues (including those that may have been commented here).

You can see more details about this review at https://app.sourcelevel.io/github/plataformatec/devise/pulls/5136.

@danzanzini
Copy link

danzanzini commented Sep 29, 2020

Isn't this a duplication of #5094?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants