212 password change #254
212 password change #254
Conversation
|
@dethos How do you think the new password form should behave for users that registered using GithubOauth? (there is no current password for these cases). |
| {{ form.current_password.errors }} {{ form.current_password.label_tag }} {{ form.current_password }} | ||
| </div> | ||
| <div class="form__block"> | ||
| {{ form.new_password2.errors }} {{ form.new_password1.label_tag }} {{ form.new_password1 }} |
There was a problem hiding this comment.
I think this is a typo, should be {{ form.new_password1.errors }}
|
I did it on purpose to always display the same message to both of them!
…On Oct 3, 2017 16:00, "Nuno Silva" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In humans/templates/humans/update_user_form.html
<#254 (comment)>:
> @@ -65,6 +66,19 @@
</div>
</li>
<li class="section" id="section3">
+ <div class="col-xxs-12 col-xs-12 col-md-12 start-xs">
+ <div class="form__block">
+ {{ form.current_password.errors }} {{ form.current_password.label_tag }} {{ form.current_password }}
+ </div>
+ <div class="form__block">
+ {{ form.new_password2.errors }} {{ form.new_password1.label_tag }} {{ form.new_password1 }}
I think this is a typo, should be {{ form.new_password1.errors }}
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#254 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA7IXlcWpOnpyF9sYQfv-dilnDERQgXkks5sooRXgaJpZM4PsRfq>
.
|
|
@Lobosque for users who signed up using Github, we should not display the change password tab or to make it clear, instead of the form a message could be displayed saying that since they used Oauth there is not need to update passwords. By the way, thanks for your contribution. As soon as you hide the tab or display that message, I will make a full review and merge the PR (on a first look everything appears to be fine 👍 ) |
There was a problem hiding this comment.
After testing a bit I found a small issue.
Also #tab4 is missing here: https://github.com/whitesmith/hawkpost/pull/254/files#diff-634d4c9620895e0cdf312c07c3ad90b6R49
| $("#tab3").addClass("active"); | ||
| $("#tab1").removeClass("active"); | ||
| $("#tab2").removeClass("active"); | ||
| $(".sett__nav__item").click(function(evt) { |
There was a problem hiding this comment.
For some reason this event is not being added to #tab4 (at least on Firefox)
There was a problem hiding this comment.
I tested here on Firefox and it triggers the event just fine.
Maybe a version issue? Can you try again?
There was a problem hiding this comment.
Just tested again, with different browsers and it is working fine. Not sure what was the problem the other time.
Lobosque
force-pushed
the
212-password-change
branch
from
53c3e51
to
88df73f
Compare
| $("#tab3").addClass("active"); | ||
| $("#tab1").removeClass("active"); | ||
| $("#tab2").removeClass("active"); | ||
| $(".sett__nav__item").click(function(evt) { |
There was a problem hiding this comment.
I tested here on Firefox and it triggers the event just fine.
Maybe a version issue? Can you try again?
humans/forms.py
Outdated
| self.pub_key = None | ||
| return super().__init__(*args, **kwargs) | ||
| return super(UpdateUserInfoForm, self).__init__(*args, **kwargs) |
There was a problem hiding this comment.
Why this change? This is a Python 3 project, there is no need to call super using the old "style".
humans/forms.py
Outdated
| new_password = self.cleaned_data.get('new_password2') | ||
| if self.change_password: | ||
| self.instance.set_password(new_password) | ||
| return super(UpdateUserInfoForm, self).save(commit=commit) |
There was a problem hiding this comment.
here super can also be changed to the new style
Fixes #212
UpdateUserInfoFormto accept password change;