Feature user change password by MartynaAnnaGottschling · Pull Request #38 · Code-Poets/sheetstorm

MartynaAnnaGottschling · 2019-02-19T11:58:32Z

Resolves #79
Resolves #104

kbeker

Some minor changes, but I would like to see it again. How can I test it locally?

sheetstorm/settings/base.py

users/common/strings.py

users/templates/registration/password_reset_email.html

users/tests/test_unit_users_function_view.py

users/views.py

users/templates/registration/password_reset_email.html

kbeker · 2019-02-26T09:13:06Z

users/tests/test_unit_users_function_view.py

+        self.assertTrue(CustomValidationErrorText.VALIDATION_ERROR_SIGNUP_PASSWORD_MESSAGE in
+            response.context['form'].errors.get('new_password2'))
+        self.assertFalse(self.user.check_password(data['new_password1']))
+        self.assertTrue(self.user.check_password(data['old_password']))


Besides, I think that test like this where you send request should be named test_integration... because it sets you django client to send request.

@kbeker so in this case should I change the name of tests?

sheetstorm/settings/base.py

users/common/strings.py

users/views.py

rwrzesien · 2019-02-28T01:49:46Z

users/templates/registration/password_reset_email.html

+To initiate the password reset process for your {{ user.get_username }} Sheet Storm Account,
+click the link below:
+
+{{ protocol }}://127.0.0.1:8000{% url 'password_reset_confirm' uidb64=uid token=token %}


I know that in the all-auth templates it is done in this way, but it is much better to generate whole url in the backend and pass it to email template. The case is that page might work on a different host that 127.0.0.1:8000 and the email will still contain it. I can help you with that but I need to know which view is supposed to use this and other password_reset_* templates?

@rwrzesien I think it's: password_reset, password_reset_done, password_reset_confirm, password_reset_complete from django.contrib.auth.views

rwrzesien · 2019-02-28T01:53:07Z

@MartynaAnnaGottschling Looks good, but it would be much easier to review this pull request if you could create or link an issue for this. It is easier to read it in single place than from several commit messages. I am not sure but I think you are trying to use templates without telling views to use them. But as I have said, it would be easier to tell with issue description.

kbeker

I can't see anything wrong in existing code, but there is no test for it. You should at least write one test to be sure that this mechanism flow works.

The 'users' app should be befor 'django.contrib.admin' in base.py INSTALLED_APPS

MartynaAnnaGottschling self-assigned this Feb 19, 2019

MartynaAnnaGottschling requested review from Karrp, Szymiks, TheCM, kbeker, maciejSamerdak and wyspiansky87 February 19, 2019 11:58

MartynaAnnaGottschling added the feature New feature label Feb 19, 2019

MartynaAnnaGottschling force-pushed the feature-user-change-password branch 3 times, most recently from 6b7735a to 5f6da54 Compare February 25, 2019 10:22

kbeker suggested changes Feb 26, 2019

View reviewed changes

MartynaAnnaGottschling force-pushed the feature-user-change-password branch from 5f6da54 to d8f44d8 Compare February 26, 2019 15:17