Sanitize email when name has special characters #2026
Conversation
lib/faker/default/internet.rb
Outdated
| char_range.include?(char) ? char : '#' | ||
| end.join | ||
|
|
||
| [sanitized_local_part, domain_name].join('@') |
There was a problem hiding this comment.
to me sanitize doesn't mean that it will construct the email as well. Can we instead pass a full email string?
There was a problem hiding this comment.
Agree, the naming isn't clear. What do you think about changing the name to construct_email?
That way we sanitize it and construct it in the method, so we avoid the repeated [..., ...].join('@') logic
There was a problem hiding this comment.
Still feels like sperate concepts. The generators can probably continue to do the construction of the email addresses, and then pass it to sanitize_email before being returned
There was a problem hiding this comment.
I think construct_email works better because it describes the functionality of the method better. Sanitization is the process of cleaning or filtering input data. Hence, single-responsibility principle comes into my mind.
martinjaimem
force-pushed
the
fix/sanitize-email-for-spec-char-username
branch
from
54543c5
to
384e602
Compare
martinjaimem
force-pushed
the
fix/sanitize-email-for-spec-char-username
branch
from
384e602
to
c8b307a
Compare
|
|
|
Thanks! |
Resolves #1974
Description:
This pull request fixes a small edge case when faking an email with a
nameparam with special characters. Lets say,Faker::Internet.email(name: 'martín'), should not resolve tomartín@example.comas it is not a valid email.With that in mind, the fix replaces the forbidden character with a
#. That decision was made basically trying to keep the number of characters in thenameparams.I'm open to any suggestions not only regarding the replacement character (
#) but also the way this pr is written. Also, willing to add more tests if needed