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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix validating email in strict mode #976

Merged
merged 2 commits into from May 17, 2024

Conversation

manuelmeurer
Copy link
Contributor

@manuelmeurer manuelmeurer commented Jul 13, 2022

To enable strict mode in email validation, mode: :strict should be used, not strict_mode: true. Maybe it used to be strict_mode: true with an earlier version of the email_validator gem?
Be that as it is, it seems that the strict_mode parameter is ignored and loose validation is used.

Now, with strict mode turned on, emails like foo;@example.com, foo@.example.com or foo@example..com are no longer valid, so I removed the specs. Why were they expected to be valid until now? I'm not an expert in that matter, but they look pretty invalid to me. 馃樃

@@ -150,7 +150,7 @@ module Validations

included do
validates :email,
email: { strict_mode: true },
email: { mode: :strict },
Copy link

Choose a reason for hiding this comment

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

Layout/AlignArguments: Align the arguments of a method call if they span more than one line.

@manuelmeurer manuelmeurer requested a review from sej3506 as a code owner May 8, 2024 19:12
Copy link
Contributor

@sej3506 sej3506 left a comment

Choose a reason for hiding this comment

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

Thank you!

@sej3506 sej3506 merged commit 82e6f73 into thoughtbot:main May 17, 2024
14 checks passed
@manuelmeurer manuelmeurer deleted the strict-mode branch May 17, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants