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

Fixes #687 : Basic Login and Register #688

Merged
merged 4 commits into from
May 4, 2018
Merged

Conversation

gabru-md
Copy link
Member

@gabru-md gabru-md commented May 4, 2018

Fixes #687

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.
  • I have added necessary documentation (if appropriate)

Preview Link

Changes proposed in this pull request:

  • Adds basic login and Register

Adds User Login and Register functionality to Badgeyay API.

Cheers!

Adds Git Ignore
def verifyPassword(user, password):
return check_password_hash(
user.password,
password)

Choose a reason for hiding this comment

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

no newline at end of file

@@ -0,0 +1,6 @@
from werkzeug.security import check_password_hash

def verifyPassword(user, password):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1


app.run()
db.init_app(app)

Choose a reason for hiding this comment

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

expected 2 blank lines after class or function definition, found 1

app.register_blueprint(homePage.router)
app.register_blueprint(errorHandlers.router)

@app.before_first_request

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

name = db.Column(db.String(80))


def __init__(self, username, password, name):

Choose a reason for hiding this comment

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

too many blank lines (2)

data['username'],
data['password'],
data['name']
)

Choose a reason for hiding this comment

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

closing bracket does not match indentation of opening bracket's line


return jsonify(
Response(403).generateMessage(
'No data received'))

Choose a reason for hiding this comment

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

no newline at end of file


token = jwt.encode(
{'user' : user.username,
'exp' : datetime.datetime.utcnow() + datetime.timedelta(seconds=900)},

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent
whitespace before ':'

'Wrong username & password combination'))

token = jwt.encode(
{'user' : user.username,

Choose a reason for hiding this comment

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

whitespace before ':'

Response(403).generateMessage(
'Could not find the Username Specified'))

if not verifyPassword(user,data['password']):

Choose a reason for hiding this comment

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

missing whitespace after ','

@gabru-md
Copy link
Member Author

gabru-md commented May 4, 2018

CC : @djmgit @ParthS007

Video Preview
Please ignore the song in background 😆

@codecov
Copy link

codecov bot commented May 4, 2018

Codecov Report

Merging #688 into development will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           development   #688   +/-   ##
==========================================
  Coverage          100%   100%           
==========================================
  Files                1      1           
  Lines               43     43           
==========================================
  Hits                43     43

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c14622...33dfdcc. Read the comment docs.

data['name'])

try:
newUser.save_to_db()

Choose a reason for hiding this comment

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

trailing whitespace

return check_password_hash(
user.password,
password)

Choose a reason for hiding this comment

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

blank line at end of file

return jsonify(
Response(403).generateMessage(
'No data received'))

Choose a reason for hiding this comment

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

blank line at end of file

Copy link
Member

@djmgit djmgit left a comment

Choose a reason for hiding this comment

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

Nicely done 👍
Please solve the hound errors

from flask_sqlalchemy import SQLAlchemy

db = SQLAlchemy()

Choose a reason for hiding this comment

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

blank line at end of file

@gabru-md gabru-md force-pushed the users branch 2 times, most recently from 30e11eb to 4e7bdf4 Compare May 4, 2018 12:20
Fixes Hound2

Fixes Hound3

Fixes Hound4

Fixes Hound
@gabru-md gabru-md force-pushed the users branch 2 times, most recently from db11386 to 83e0b8b Compare May 4, 2018 12:27
Copy link
Member

@djmgit djmgit left a comment

Choose a reason for hiding this comment

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

This is nice to start with. In further iterations we can decide what fields need to present while registration. We can collect email and other stuff, need to discuss on that. Also other stuff like restrictions on password, email check, etc.

@djmgit djmgit requested a review from ParthS007 May 4, 2018 12:31
@gabru-md
Copy link
Member Author

gabru-md commented May 4, 2018

Sure @djmgit
There are a lot of other things to be added. I'll be adding them along with @ParthS007 as we claimed the issue together( for backend API ). Meanwhile @yashLadha can work on refractoring the current codebase so that when we shift it to the new API, then everything is working?

your views : @djmgit @ParthS007 ?

@djmgit
Copy link
Member

djmgit commented May 4, 2018

@yashLadha (unable to tag you from reviewers list, don't know why :P) @ParthS007 please review

Copy link
Member

@ParthS007 ParthS007 left a comment

Choose a reason for hiding this comment

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

LGTM!

@djmgit djmgit merged commit 1a21189 into fossasia:development May 4, 2018
@gabru-md gabru-md deleted the users branch May 4, 2018 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add basic authentication to API
4 participants