Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

feat(users): change username to usernameOrEmail in signin #1545

Merged
merged 2 commits into from
Oct 6, 2016

Conversation

itelo
Copy link
Contributor

@itelo itelo commented Oct 6, 2016

It's quite boring to force your users to remember the username they've created in your app to signin. So I've made an option to put an email or username. It's far more interesting to user experience, and a lot of applications are doing that, eg. Snapchat.

@coveralls
Copy link

coveralls commented Oct 6, 2016

Coverage Status

Coverage remained the same at 73.036% when pulling 4d1a92f on itelo:signin-email into 73a7c2c on meanjs:master.

Copy link
Member

@simison simison left a comment

Choose a reason for hiding this comment

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

Awesome! Found just one thing to nitpick.

$or: [{
username: usernameOrEmail.toLowerCase()
}, {
email: usernameOrEmail
Copy link
Member

@simison simison Oct 6, 2016

Choose a reason for hiding this comment

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

Emails are always saved lowercase in the db, so it's safe to set this usernameOrEmail.toLowerCase() as well. Otherwise I won't be able to login when writing e.g. "Fred@example.com".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed (:

@coveralls
Copy link

coveralls commented Oct 6, 2016

Coverage Status

Coverage remained the same at 73.036% when pulling b116fe6 on itelo:signin-email into 73a7c2c on meanjs:master.

Copy link
Member

@simison simison left a comment

Choose a reason for hiding this comment

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

👍

@simison
Copy link
Member

simison commented Oct 6, 2016

We've been running pretty much identical signup controller for over a year now. Works great — no more "I forgot my username" support mails.

@lirantal
Copy link
Member

lirantal commented Oct 6, 2016

Looks good, thanks @itelo !

@lirantal lirantal merged commit 6a6b630 into meanjs:master Oct 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants