Modify email method in Internet class #1259
Conversation
| [user_name(name, separators), domain_name].join('@') | ||
| else | ||
| [user_name(name), domain_name].join('@') | ||
| end |
There was a problem hiding this comment.
I think you should create two tests because you have two scenarios.
I added a new rake task. I don't know if I told you: rake coverage_report. By running this command, you can see the code coverage report.
There was a problem hiding this comment.
There was a test already in place to test the else case. I only added the first scenario, which is why only one test was needed. Also, running rake coverage_report opens up a page in my browser and shoes me coverage report. I don't get a report on the terminal. Is this how it's supposed to work?
There was a problem hiding this comment.
I didn't see the test for the else statement. Let me check it out.
There was a problem hiding this comment.
doh it's correct 👍 sorry about that
| Faker::Internet.email #=> "[email protected]" | ||
|
|
||
| Faker::Internet.email('Nancy') #=> "[email protected]" | ||
|
|
||
| Faker::Internet.email('Janelle Santiago', '+') #=> [email protected]" | ||
|
|
There was a problem hiding this comment.
What do you think about adding a whitelist for these separators? We know that there are special characters that aren't accepted in emails.
I think it could be a bit dangerous if people seed their staging database and add wrong separators accidentally 😆
There was a problem hiding this comment.
I wonder if these are Google's specific restrictions. It looks like this is a larger topic than I expected. And from stack overflow.
There was a problem hiding this comment.
I say skip the whitelist... if people are using the optional argument to add separators, then it's up to them to wield the sharp knife safely. :)
There was a problem hiding this comment.
yeah if we don't do that, we could focus on other features and bugs
* Modify email method in Internet class * Update CHANGELOG.md
This PR is for the feature request in issue #1178.
The PR does: