-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Adds ability to login with email when specifying it #4276
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4276 +/- ##
=========================================
+ Coverage 92.23% 92.43% +0.2%
=========================================
Files 116 118 +2
Lines 8124 8155 +31
=========================================
+ Hits 7493 7538 +45
+ Misses 631 617 -14
Continue to review full report at Codecov.
|
spec/ParseUser.spec.js
Outdated
}).then(done).catch(done.fail); | ||
}); | ||
|
||
it('can to login when both email and username are passed', (done) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can to
, probably should just can
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep!
spec/ParseUser.spec.js
Outdated
}).then(done).catch(done.fail); | ||
}); | ||
|
||
it('fails to login when username dont match email', (done) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not critical but dont
is bugging me
return rp.get(options); | ||
}).then(done).catch(done.fail); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could have an additional test here for trying cannot login with email and bad password
, just to cover if a possible bug were introduced regarding password validation with an email at any point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the test on the next line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good! Just some typos and an additional test and this should be ready to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, let's see what the CI says and we can move this along.
Thanks man! |
No description provided.