fix code based on pull request review

password_reset/forms.py

@@ -2,6 +2,7 @@ from django import forms
 from django.core.validators import validate_email
 from django.db.models import Q
 from django.utils.translation import ugettext_lazy as _
+from django.conf import settings
 
 from .utils import get_user_model
 
@@ -45,15 +46,14 @@ class PasswordRecoveryForm(forms.Form):
         cleaner = getattr(self, 'get_user_by_%s' % self.label_key)
         self.cleaned_data['user'] = user = cleaner(username)
 
-        from django.conf import settings
+        user_is_active = getattr(user, 'is_active', True)
         recovery_only_active_users = getattr(settings,
                                              'RECOVER_ONLY_ACTIVE_USERS',
                                              False)
 
-        if recovery_only_active_users and \
-           hasattr(user, 'is_active') and not user.is_active:
-            raise forms.ValidationError(_("Sorry, this user is inactive and "
-                                          "his password can't be recovered."))
+        if recovery_only_active_users and not user_is_active:
+            raise forms.ValidationError(_("Sorry, inactive users can't "
+                                        "recover their password."))
 
         return username
 

password_reset/tests/tests.py

@@ -6,7 +6,6 @@ from django.test import TestCase
 from django.test.utils import override_settings
 from django.utils import timezone
 from django.utils.unittest import SkipTest
-from django.conf import settings
 
 from ..forms import PasswordRecoveryForm, PasswordResetForm
 from ..utils import get_user_model
@@ -105,19 +104,16 @@ class FormTests(TestCase):
             self.assertTrue(form.is_valid(),
                             'Password from inactive should be recovered')
 
-            settings.RECOVER_ONLY_ACTIVE_USERS = True
+            with self.settings(RECOVER_ONLY_ACTIVE_USERS=True):
+                form = PasswordRecoveryForm(data={
+                    'username_or_email': user.email})
+                self.assertFalse(form.is_valid(),
+                                 'Password from inactive user should '
+                                 'not be recovered')
 
-            form = PasswordRecoveryForm(data={'username_or_email': user.email})
-            self.assertFalse(form.is_valid(),
-                             'Password from inactive user should '
-                             'not be recovered')
-
-            self.assertEquals(form.errors['username_or_email'],
-                              [u"Sorry, this user is inactive and his "
-                              "password can't be recovered."])
-
-            # tear down settings
-            del settings.RECOVER_ONLY_ACTIVE_USERS
+                self.assertEqual(form.errors['username_or_email'],
+                                 [u"Sorry, inactive users can't recover "
+                                 "their password."])
 
     def test_form_custom_search(self):
         # Searching only for email does some extra validation