Email validation

[jloveland]

I am writing some e2e test and found that 123@123 is a valid email according to HTML5 and RFC 822, but we don't support it on the back end.

If we wanted to support this we could set validator.isEmail('test@test', {require_tld: false}) in user.server.model

If we don't want 123@123 to be accepted on the front end, we could check for this on the front end with a pattern like so:
pattern="[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,63}$"

reference: http://stackoverflow.com/questions/20573488/why-does-html5-form-validation-allow-emails-without-a-dot

[lirantal]

@ilanbiala why 0.4.x milestone? I thought we are avoiding that and always settings all PRs to the next version, unless we know it "needs more time" and so we can set it to to the one after it (i.e: 0.5)

@jloveland I don't specifically mind whether we force the tld option or not.

[mleanos]

@jloveland I agree with this statement from the 2nd comment on that SO's accepted answer..

If somebody is entering something unusual as their email, just accept it - chances are they know better than you.

And since it is possible to have an email without a "." in it, shouldn't we accept it as well?

[lirantal]

That's why I said I'm ok with it - it is possible for example to utilize meanjs for internal enterprise projects where you may have alias addresses like root@localhost which are valid.

[ilanbiala]

@lirantal I figured we should put this up for discussion and it actually is a breaking change, so it should go in 0.5.0.

[lirantal]

@ilanbiala what do you refer to by a "breaking change"?

[ilanbiala]

We are changing the functionality of email validation, so if users updated to this code as 0.4.2, then their code wouldn't function as before and it might break things, hence a breaking change.

[jloveland]

@ilanbiala, how is allowing html5 valid emails (i.e. root@localhost) a breaking change? The application will not stop working, it will allow valid emails. If they don't want to allow root@localhost or test@test, they can remove the {require_tld: false}.

[ilanbiala]

It's different than before, which breaks the expected functionality. Before we didn't allow emails without a . or whatever, and now we do, which changes how things work. That would be a breaking change.

[mleanos]

I can see the "breaking change" issue from both sides. However, I think what @ilanbiala is trying to say is that even though this won't stop anything from working, it actually will allow more emails through; this could be breaking for some apps that rely on this restriction.

[lirantal]

OK, so Ilan, if we will want to go with this change, you prefer we do it in
a 0.5 release?
9

Regards,
Liran Tal.
On Sep 26, 2015 9:49 PM, "Ilan Biala" notifications@github.com wrote:

It's different than before, which breaks the expected functionality.
Before we didn't allow emails without a . or whatever, and now we do,
which changes how things work. That would be a breaking change.

—
Reply to this email directly or view it on GitHub
#934 (comment).

[lirantal]

BTW I could also argue that not allowing users anymore to create, edit,
delete articles is also a breaking change. Which is also what I said on
that PR.

Regards,
Liran Tal.
On Sep 27, 2015 1:02 AM, "Liran Tal" liran.tal@gmail.com wrote:

OK, so Ilan, if we will want to go with this change, you prefer we do it
in a 0.5 release?
9

Regards,
Liran Tal.
On Sep 26, 2015 9:49 PM, "Ilan Biala" notifications@github.com wrote:

It's different than before, which breaks the expected functionality.
Before we didn't allow emails without a . or whatever, and now we do,
which changes how things work. That would be a breaking change.

—
Reply to this email directly or view it on GitHub
#934 (comment).

[mleanos]

@lirantal I agree.. That's a good reason for that PR to be added to the 0.5.0 milestone as well.

Also, I wasn't taking a particularly strong stance on either side of this issue. I really don't know how likely it is for a user of this project to be relying on this restriction.

[ilanbiala]

@lirantal in both cases you are correct. These changes should only be published in 0.5.0 because they alter the functionality.