Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to login using email or username alike #27

Merged
merged 1 commit into from
Oct 14, 2018

Conversation

adri
Copy link
Contributor

@adri adri commented Oct 13, 2018

To avoid complexity in the frontend, the email field is used for
logging in with email or username.

Fixes #9.

@coveralls
Copy link

coveralls commented Oct 13, 2018

Coverage Status

Coverage increased (+0.05%) to 57.368% when pulling addbe03 on adri:feature/login-with-user-name into 65cda46 on CaptainFact:staging.

Copy link
Member

@Betree Betree left a comment

Choose a reason for hiding this comment

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

Hey @adri thanks again! You made a good choice keeping the API as is, it will reduce the complexity of implementing the frontend.

Beside the confusion between name and username fields, I have nothing more to add to this PR. Good job 😉

def get_user_for_email_or_name_password(email_or_name, password) do
user =
User
|> where([u], u.email == ^email_or_name or u.name == ^email_or_name)
Copy link
Member

Choose a reason for hiding this comment

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

User schema fields may be confusing. name is meant to store User's real name and is optional. The field you're looking for is u.username which is not nullable and unique among users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooops good one. Thanks! Fixed :)

To avoid complexity in the frontend, the email field is used for
logging in with email or username.
@adri adri force-pushed the feature/login-with-user-name branch from 398fd4f to addbe03 Compare October 14, 2018 10:59
@Betree Betree merged commit 4c042ae into CaptainFact:staging Oct 14, 2018
@Betree
Copy link
Member

Betree commented Oct 18, 2018

Hi @adri! We have created a contributors gratification program to thank the people like you who help this project move forward. Your two first contributions to the API were excellent, so check this page if you want to qualify 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants