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

Make it possible for a user to reset its password #198

Merged
merged 11 commits into from Sep 12, 2017

Conversation

Projects
None yet
2 participants
@libre-man
Copy link
Collaborator

libre-man commented Sep 11, 2017

Description

Make it possible for a user to reset its password

Checklist:

  • Linter & type checker OK
  • Tests
  • Added type information
  • Docs

Screenshot or -recording (if applicable)



movie

@libre-man libre-man force-pushed the add-reset-password-page branch from 815c609 to f576b76 Sep 11, 2017

@libre-man libre-man force-pushed the add-reset-password-page branch from d4bf8ea to 0a62cb5 Sep 11, 2017

@codecov

This comment has been minimized.

Copy link

codecov bot commented Sep 11, 2017

Codecov Report

Merging #198 into master will increase coverage by 0.14%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #198      +/-   ##
=========================================
+ Coverage   92.25%   92.4%   +0.14%     
=========================================
  Files          22      22              
  Lines        2634    2685      +51     
=========================================
+ Hits         2430    2481      +51     
  Misses        204     204
Impacted Files Coverage Δ
psef/errors.py 100% <100%> (ø) ⬆️
psef/models.py 98.47% <100%> (+0.04%) ⬆️
psef/v1/login.py 100% <100%> (ø) ⬆️
psef/__init__.py 98.27% <100%> (+0.09%) ⬆️

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 cf6ca6b...a77ecc5. Read the comment docs.

@olmokramer

This comment has been minimized.

Copy link
Collaborator

olmokramer commented Sep 11, 2017

I found a few issues:

  • It says site_url in the email template
    codegrade-mail
  • After I've changed my password I'd like to be forwarded to the login page instead of having to click on the link in the navbar manually
@olmokramer

This comment has been minimized.

Copy link
Collaborator

olmokramer commented Sep 11, 2017

Oh and another thing, when I follow the link from the email a second time, I get to the same page as before where I can fill in the new password, only this time I get an error that the token is expired after clicking the submit button. I think it would be better if you'd get an error right away that the token is invalid and not even get to see the password inputs.

@olmokramer

This comment has been minimized.

Copy link
Collaborator

olmokramer commented Sep 11, 2017

And yet another thing: when I click "Forgot password" I would expect to be able to use the back button in the browser/on my mouse to go back to the login form but this doesn't work.

@olmokramer

This comment has been minimized.

Copy link
Collaborator

olmokramer commented Sep 11, 2017

codegrade-nonempty

@libre-man

This comment has been minimized.

Copy link
Collaborator Author

libre-man commented Sep 11, 2017

The last thing could be easily solved by adding a anchor tag. The thing be fore that is not possible and would lead to security issues if it were so I prefer if we not add it.

@olmokramer

This comment has been minimized.

Copy link
Collaborator

olmokramer commented Sep 11, 2017

Oh, and it would also be nice if the username field on either of the pages would be focused when switching between login and forgot password.

@libre-man

This comment has been minimized.

Copy link
Collaborator Author

libre-man commented Sep 11, 2017

Agree, will fix


<script>
import Icon from 'vue-awesome/components/Icon';
import 'vue-awesome/icons/check';

This comment has been minimized.

Copy link
@olmokramer

olmokramer Sep 11, 2017

Collaborator

Please remove unused import

import 'vue-awesome/icons/check';
import 'vue-awesome/icons/eye';
import 'vue-awesome/icons/eye-slash';
import 'vue-awesome/icons/times';

This comment has been minimized.

Copy link
@olmokramer

olmokramer Sep 11, 2017

Collaborator

Please remove unused import

libre-man added some commits Sep 11, 2017

All issues were fixed.

@libre-man libre-man merged commit 8455e2b into master Sep 12, 2017

5 checks passed

codecov/patch 100% of diff hit (target 92.25%)
Details
codecov/project 92.4% (+0.14%) compared to cf6ca6b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 92.402%
Details
pyup.io/safety-ci No dependencies with known security vulnerabilities.
Details

@olmokramer olmokramer deleted the add-reset-password-page branch Sep 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.