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

Make emails unique #439

Merged
merged 2 commits into from
Jul 20, 2015
Merged

Make emails unique #439

merged 2 commits into from
Jul 20, 2015

Conversation

igorauad
Copy link
Contributor

Emails are made unique. When user attempts to sign in through a provider in which his email is one that is already registered, user is redirected to the signin page with an error passed as a query string parameter.

Is this a reasonable solution to #118?

Your feedback is welcome. Thanks in advance.

@ilanbiala
Copy link
Member

@igorauad I think if we merge in #337 then this may have some issues working. It would be better to merge that in first IMO and then make the necessary changes. It would be great if you could provide your 2¢ on that.

@igorauad
Copy link
Contributor Author

igorauad commented Mar 1, 2015

Ok, @ilanbiala . Thank you!

@ilanbiala ilanbiala mentioned this pull request Mar 3, 2015
@lirantal
Copy link
Member

this PR needs some conflicts fixing before we can merge but overall looks like we can go for it.
@igorauad ?

Emails are made unique. When user attempts to sign in through a provider in which his email is one that is already registered, user is redirected to the signin page with an error passed as a query string parameter.
@igorauad
Copy link
Contributor Author

@lirantal I've just fixed the conflicts

@ilanbiala ilanbiala added this to the 0.4.0 milestone Jul 17, 2015
@lirantal
Copy link
Member

thanks @igorauad
@rhutchison @codydaig @mleanos LGTY?

@lirantal lirantal mentioned this pull request Jul 20, 2015
14 tasks
@codydaig
Copy link
Member

@igorauad I pulled your branch down to test and it works! But, can we make it so it says the email is already in use versus "Unique field already exists". The user would have no way to know if it's the username or email field that is already taken. (Or even which fields must be unique.)

@igorauad
Copy link
Contributor Author

@codydaig see if the commit that I've just added solves the problem for you

@codydaig
Copy link
Member

@igorauad Tested and works great! Thanks :)

LGTM

@lirantal
Copy link
Member

Thanks @igorauad , @codydaig.

lirantal added a commit that referenced this pull request Jul 20, 2015
@lirantal lirantal merged commit 82d2377 into meanjs:0.4.0 Jul 20, 2015
@igorauad
Copy link
Contributor Author

Thanks @codydaig , @lirantal

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