-
Notifications
You must be signed in to change notification settings - Fork 33
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
Set login form to use WTForms #321
base: master
Are you sure you want to change the base?
Conversation
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.02%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
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.
Great start! I've added few comments that can help improve this PR further :)
login_message = request.args.get('login_message') | ||
if not form.validate_on_submit(): | ||
return render_template( | ||
'login.html', form=form, login_message=login_message, |
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.
Should we keep the login message from the last page request or generate new one? (I might have missed something in the logic here)
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.
I don't think I changed anything meaningful here. Me missing something in the logic here would be more probable :P
From just messing around with the system, it seems to work - what should I fix here?
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.
Try to create a form validator instead of the auth
method and because of that you would remove 113-122
lines.
You can take a look in the lmsweb/tools/validators.py
and the auth
method in order to create the validator
<div class="container"> | ||
<div id="login-container"> | ||
<div id="login" class="text-center"> |
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.
Please configure your IDE to convert TAB to 2 spaces in HTML
<hr class="mt-3 mb-3"> | ||
<a href="/" class="btn btn-success btn-sm" role="button">{{ _('Back to login page') }}</a> | ||
</div> | ||
<div class="container"> |
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.
Please reindent it
No description provided.