Skip to content

Set login form to use WTForms - #321

Open
NogaOs wants to merge 6 commits into
masterfrom
set-login-form-to-use-WTForms
Open

Set login form to use WTForms#321
NogaOs wants to merge 6 commits into
masterfrom
set-login-form-to-use-WTForms

Conversation

@NogaOs

Copy link
Copy Markdown
Contributor

No description provided.

@NogaOs
NogaOs requested a review from yammesickaOctober 1, 2021 21:16
@NogaOsNogaOs self-assigned this Oct 1, 2021
@NogaOsNogaOs added the good first issue Good for newcomers label Oct 1, 2021
@sourcery-ai

Copy link
Copy Markdown

Sourcery Code Quality Report

✅ Merging this PR will increase code quality in the affected files by 0.02%.

Quality metricsBeforeAfterChange
Complexity3.72 ⭐3.66 ⭐-0.06 👍
Method Length52.73 ⭐52.73 ⭐0.00
Working memory6.99 🙂7.00 🙂0.01 👎
Quality74.57% 🙂74.59% 🙂0.02% 👍
Other metricsBeforeAfterChange
Lines6126164
Changed filesQuality BeforeQuality AfterQuality Change
lms/lmsweb/views.py74.55% 🙂74.56% 🙂0.01% 👍
lms/lmsweb/forms/reset_password.py75.87% ⭐75.87% ⭐0.00%

Here are some functions in these files that still need a tune-up:

FileFunctionComplexityLengthWorking MemoryQualityRecommendation
lms/lmsweb/views.pycomment13 🙂202 😞8 🙂50.15% 🙂Try splitting into smaller methods
lms/lmsweb/views.pyview8 ⭐140 😞10 😞56.16% 🙂Try splitting into smaller methods. Extract out complex expressions
lms/lmsweb/views.pylogin4 ⭐127 😞12 😞57.91% 🙂Try splitting into smaller methods. Extract out complex expressions
lms/lmsweb/views.pynote6 ⭐144 😞8 🙂61.90% 🙂Try splitting into smaller methods
lms/lmsweb/views.pyshared_solution2 ⭐84 🙂10 😞69.27% 🙂Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

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!

@yammesickayammesicka left a comment

Copy link
Copy Markdown
Member

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 :)

Comment threadlms/lmsweb/forms/reset_password.py
Comment threadlms/lmsweb/views.py
Comment threadlms/lmsweb/views.py
login_message = request.args.get('login_message')
if not form.validate_on_submit():
return render_template(
'login.html', form=form, login_message=login_message,

Copy link
Copy Markdown
Member

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)

Copy link
Copy Markdown
ContributorAuthor

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?

Copy link
Copy Markdown
Collaborator

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

Comment on lines +5 to +7
<div class="container">
<div id="login-container">
<div id="login" class="text-center">

Copy link
Copy Markdown
Member

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">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please reindent it

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

good first issueGood for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@NogaOs@yammesicka@orronai@noga-acumen