Skip to content

use mail.ParseAddress to cover missing email validations #1395

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

Merged
merged 3 commits into from
Mar 19, 2025

Conversation

eladb2011
Copy link
Contributor

@eladb2011 eladb2011 commented Mar 9, 2025

Fixes Or Enhances

Make sure that you've checked the boxes below before you submit PR:

  • Tests exist or have been written that cover this particular change.

@go-playground/validator-maintainers

Sorry, something went wrong.

@eladb2011 eladb2011 requested a review from a team as a code owner March 9, 2025 20:13
@coveralls
Copy link

coveralls commented Mar 9, 2025

Coverage Status

coverage: 74.241% (-0.2%) from 74.466%
when pulling d7f7722 on anchor-g:master
into bae7f6d on go-playground:master.

@eladb2011 eladb2011 changed the title user mail.ParseAddress to cover missing email validations use mail.ParseAddress to cover missing email validations Mar 9, 2025
errs = validate.Var(s, "email")
NotEqual(t, errs, nil)
AssertError(t, errs, "", "", "", "", "email")

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's include one more test case.

	s = "Foo Bar <foobar@example.com>"
	errs = validate.Var(s, "email")
	NotEqual(t, errs, nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, added this test as well

@nodivbyzero
Copy link
Contributor

nodivbyzero commented Mar 11, 2025

Connected to: #784, #1050

Copy link
Contributor

@nodivbyzero nodivbyzero left a comment

Choose a reason for hiding this comment

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

LGTM

@nodivbyzero
Copy link
Contributor

Connected to: #784, #1050

@deankarn @zemzale What are your thoughts on this fix?

@zemzale
Copy link
Member

zemzale commented Mar 19, 2025

This seems more robust, no problem with merging this

@eladb2011
Copy link
Contributor Author

Thanks mates, waiting for the merge, I want to clean the temp hack (override of the default email validate) from my code (:

@nodivbyzero nodivbyzero merged commit 8592022 into go-playground:master Mar 19, 2025
12 checks passed
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

4 participants