From 625566ec86006e7ab6a11429f9f20d583c698b93 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sat, 12 May 2018 12:21:22 +0200 Subject: [PATCH 01/20] Prepared admin integration This is basically the start of the development of admin-related functions of django-auth_enhanced. Development will be split between modifications of the list-view inside Django's admin backend and the actual detail view of a single User object. However, this will be the branch that represents the pull request that is going to close #4. --- auth_enhanced/admin.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index dec5915..6b80985 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -3,7 +3,9 @@ # Django imports from django.conf import settings -from django.contrib.admin import ModelAdmin, register +from django.contrib.admin import ModelAdmin, register, site +from django.contrib.auth import get_user_model +from django.contrib.auth.admin import UserAdmin # app imports from auth_enhanced.models import UserEnhancement @@ -30,6 +32,15 @@ def _wrapper_noop(admin_class): return _wrapper_noop +class EnhancedUserAdmin(UserAdmin): + """This class substitutes the default admin interface for user objects. + + It is designed to enhance and substitute the default admin interface + provided by 'django.contrib.auth'. But furthermore, it should be as + pluggable as possible, to be able to deal with custom user models.""" + pass + + @register_only_debug(UserEnhancement) class UserEnhancementAdmin(ModelAdmin): """Integrates UserEnhancement into Django's admin menu. @@ -39,3 +50,8 @@ class UserEnhancementAdmin(ModelAdmin): UserEnhancements will be integrated into the respective admin class for the User-objects.""" pass + + +# +site.unregister(get_user_model()) +site.register(get_user_model(), EnhancedUserAdmin) From fcc325634693a6b3b9a8e0d11bd7002cf54adf40 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sat, 12 May 2018 13:15:52 +0200 Subject: [PATCH 02/20] Made 'list_display' configurable --- auth_enhanced/admin.py | 17 +++++++++++++++-- tests/utils/settings_dev.py | 5 ----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 6b80985..c6dd34a 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -38,7 +38,19 @@ class EnhancedUserAdmin(UserAdmin): It is designed to enhance and substitute the default admin interface provided by 'django.contrib.auth'. But furthermore, it should be as pluggable as possible, to be able to deal with custom user models.""" - pass + + # 'list_display' controls, which fields will be displayed in the list view. + # This is configurable with an app-specific setting or will be left at + # Django's default value: + # ('username', 'email', 'first_name', 'last_name', 'is_staff') + # TODO: Django already checks the value of 'list_display', so it might be + # unnecessary to actually check DAE_ADMIN_LIST_DISPLAY within the app. + # However, it should be ensured, that the app's setting only includes + # valid values, which depends on the AUTH_USER_MODEL in use. + try: + list_display = settings.DAE_ADMIN_LIST_DISPLAY + except AttributeError: + pass @register_only_debug(UserEnhancement) @@ -52,6 +64,7 @@ class UserEnhancementAdmin(ModelAdmin): pass -# +# substitute the default implementation of the user admin +# TODO: should this be configurable? site.unregister(get_user_model()) site.register(get_user_model(), EnhancedUserAdmin) diff --git a/tests/utils/settings_dev.py b/tests/utils/settings_dev.py index b83140a..51fedf0 100644 --- a/tests/utils/settings_dev.py +++ b/tests/utils/settings_dev.py @@ -87,8 +87,3 @@ EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' ### CURRENT DEVELOPMENT -DAE_ADMIN_SIGNUP_NOTIFICATION = ( - ('django', 'django@localhost', ('mail', )), -) - -DAE_OPERATION_MODE = 'email-verification' From 62110c2e554518629225550643994791655992e8 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sat, 12 May 2018 18:59:55 +0200 Subject: [PATCH 03/20] Created custom list filter --- auth_enhanced/admin.py | 52 ++++++++++++++++-- tests/test_admin.py | 46 +++++++++++++++- .../utils/fixtures/test_different_users.json | 54 ++++++++++++++++++- 3 files changed, 144 insertions(+), 8 deletions(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index c6dd34a..5c4de75 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -3,9 +3,10 @@ # Django imports from django.conf import settings -from django.contrib.admin import ModelAdmin, register, site +from django.contrib import admin from django.contrib.auth import get_user_model from django.contrib.auth.admin import UserAdmin +from django.utils.translation import ugettext_lazy as _ # app imports from auth_enhanced.models import UserEnhancement @@ -26,12 +27,48 @@ def _wrapper_noop(admin_class): if settings.DEBUG: # re-use Django's register-decorator - return register(*models, **kwargs) + return admin.register(*models, **kwargs) # return a noop return _wrapper_noop +class EnhancedUserStatusFilter(admin.SimpleListFilter): + """Custom SimpleListFilter to filter on user's status""" + + # the title of this filter + title = _('status') + + # the parameter in the URL + parameter_name = 'status' + + def lookups(self, request, model_admin): + """Controls the options in the filter list. + + First value in the tuple: parameter in the query string + Second value: The caption for the filter list""" + + return ( + ('users', _('Users')), + ('staff', _('Staff')), + ('superusers', _('Superusers')) + ) # pragma nocover + + def queryset(self, request, queryset): + """This actually modifies the queryset, if the filter is applied.""" + + if self.value() == 'users': + return queryset.filter(is_staff=False).filter(is_superuser=False) + + if self.value() == 'staff': + return queryset.filter(is_staff=True) + + if self.value() == 'superusers': + return queryset.filter(is_superuser=True) + + return queryset + + class EnhancedUserAdmin(UserAdmin): """This class substitutes the default admin interface for user objects. @@ -52,9 +89,14 @@ class EnhancedUserAdmin(UserAdmin): except AttributeError: pass + # 'list_filter' controls, which filters will be usable in the list view. + # Django's default admin class provides the following list: + # ('is_staff', 'is_superuser', 'is_active', 'groups') + list_filter = (EnhancedUserStatusFilter, 'is_active', 'groups') + @register_only_debug(UserEnhancement) -class UserEnhancementAdmin(ModelAdmin): +class UserEnhancementAdmin(admin.ModelAdmin): """Integrates UserEnhancement into Django's admin menu. This ModelAdmin is just used for development and should not be registered @@ -66,5 +108,5 @@ class UserEnhancementAdmin(ModelAdmin): # substitute the default implementation of the user admin # TODO: should this be configurable? -site.unregister(get_user_model()) -site.register(get_user_model(), EnhancedUserAdmin) +admin.site.unregister(get_user_model()) +admin.site.register(get_user_model(), EnhancedUserAdmin) diff --git a/tests/test_admin.py b/tests/test_admin.py index 59eda19..057ddfb 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -8,12 +8,54 @@ from unittest import skip # noqa # Django imports +from django.contrib.auth import get_user_model from django.test import override_settings, tag # noqa +# app imports +from auth_enhanced.admin import EnhancedUserAdmin, EnhancedUserStatusFilter + # app imports from .utils.testcases import AuthEnhancedTestCase @tag('admin') -class UserEnhancementAdminTests(AuthEnhancedTestCase): - pass +class EnhancedUserStatusFilterTest(AuthEnhancedTestCase): + """These tests target the custom status filter.""" + + fixtures = ['tests/utils/fixtures/test_different_users.json'] + + def test_filter_superusers(self): + """Does the filter return the superusers?""" + + f = EnhancedUserStatusFilter(None, {'status': 'superusers'}, get_user_model(), EnhancedUserAdmin) + f_result = f.queryset(None, get_user_model().objects.all()) + + # number of superusers in fixture is subject to changes! + self.assertEqual(len(f_result), 3) + + def test_filter_staffusers(self): + """Does the filter return the staff users?""" + + f = EnhancedUserStatusFilter(None, {'status': 'staff'}, get_user_model(), EnhancedUserAdmin) + f_result = f.queryset(None, get_user_model().objects.all()) + + # number of superusers in fixture is subject to changes! + self.assertEqual(len(f_result), 5) + + def test_filter_users(self): + """Does the filter return the default users?""" + + f = EnhancedUserStatusFilter(None, {'status': 'users'}, get_user_model(), EnhancedUserAdmin) + f_result = f.queryset(None, get_user_model().objects.all()) + + # number of superusers in fixture is subject to changes! + self.assertEqual(len(f_result), 1) + + def test_filter_no_filter(self): + """If the filter is not applied, return the full user list.""" + + f = EnhancedUserStatusFilter(None, {}, get_user_model(), EnhancedUserAdmin) + f_result = f.queryset(None, get_user_model().objects.all()) + + # number of superusers in fixture is subject to changes! + self.assertEqual(len(f_result), 6) diff --git a/tests/utils/fixtures/test_different_users.json b/tests/utils/fixtures/test_different_users.json index 6017572..046a6b3 100644 --- a/tests/utils/fixtures/test_different_users.json +++ b/tests/utils/fixtures/test_different_users.json @@ -4,7 +4,7 @@ "pk": 1, "fields": { "password": "pbkdf2_sha256$100000$cmT7Sn6F8MDR$roOaI7UWMsBft3jCnE03mA/WONqVVNjR8HqJCPK+Ts8=", - "last_login": "2018-05-11T15:11:42.378", + "last_login": null, "is_superuser": true, "username": "django", "first_name": "", @@ -71,6 +71,42 @@ "user_permissions": [] } }, +{ + "model": "auth.user", + "pk": 5, + "fields": { + "password": "pbkdf2_sha256$100000$lV52crpDsEzY$ayhyunUhctRHY2JcTiOnGZqT72VzfVk9uqLNdRmKKaA=", + "last_login": null, + "is_superuser": false, + "username": "staff_user1", + "first_name": "", + "last_name": "", + "email": "staff_user1@localhost", + "is_staff": true, + "is_active": true, + "date_joined": "2018-05-11T15:12:20", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 6, + "fields": { + "password": "pbkdf2_sha256$100000$lV52crpDsEzY$ayhyunUhctRHY2JcTiOnGZqT72VzfVk9uqLNdRmKKaA=", + "last_login": null, + "is_superuser": false, + "username": "staff_user2", + "first_name": "", + "last_name": "", + "email": "staff_user2@localhost", + "is_staff": true, + "is_active": true, + "date_joined": "2018-05-11T15:12:20", + "groups": [], + "user_permissions": [] + } +}, { "model": "auth_enhanced.userenhancement", "pk": 1, @@ -102,5 +138,21 @@ "email_verification_status": "EMAIL_VERIFICATION_COMPLETED", "user": 4 } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 5, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_COMPLETED", + "user": 5 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 6, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_FAILED", + "user": 6 + } } ] From 5750b12fd51f215dfbfa8694ce9e241005686ac7 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sat, 12 May 2018 19:27:44 +0200 Subject: [PATCH 04/20] Added 'ordering' and searchbox control --- auth_enhanced/admin.py | 16 ++++++++++++++++ auth_enhanced/checks.py | 14 ++++++++++++++ auth_enhanced/settings.py | 9 +++++++++ tests/test_checks.py | 16 +++++++++++++++- 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 5c4de75..4d6143e 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -94,6 +94,22 @@ class EnhancedUserAdmin(UserAdmin): # ('is_staff', 'is_superuser', 'is_active', 'groups') list_filter = (EnhancedUserStatusFilter, 'is_active', 'groups') + # 'search_fields' determines the target fields for the search box + # The search box can be disabled with an app-specific setting, accordingly + # the 'search_field' will be set to an empty tuple. + try: + if not settings.DAE_ADMIN_SHOW_SEARCHBOX: + search_fields = () + setattr(settings, 'DAE_ADMIN_SHOW_SEARCHBOX', False) + except AttributeError: + # following line is exactly Django's default. Doesn't need to be set! + # search_fields = ('username', 'email', 'first_name', 'last_name') + setattr(settings, 'DAE_ADMIN_SHOW_SEARCHBOX', True) + + # 'ordering' controls the default ordering of the list view + # Django's default value is just 'username' + ordering = ('-is_superuser', '-is_staff', 'is_active', 'username') + @register_only_debug(UserEnhancement) class UserEnhancementAdmin(admin.ModelAdmin): diff --git a/auth_enhanced/checks.py b/auth_enhanced/checks.py index 350c3dd..ab4a184 100644 --- a/auth_enhanced/checks.py +++ b/auth_enhanced/checks.py @@ -136,6 +136,16 @@ id='dae.e010' ) +# DAE_ADMIN_SHOW_SEARCHBOX +E011 = Error( + _("'DAE_ADMIN_SHOW_SEARCHBOX' has to be a boolean value!"), + hint=_( + "Please check your settings and ensure that " + "'DAE_ADMIN_SHOW_SEARCHBOX' is either True or False." + ), + id='dae.e011' +) + def check_settings_values(app_configs, **kwargs): """Checks, if the app-specific settings have valid values.""" @@ -222,5 +232,9 @@ def check_settings_values(app_configs, **kwargs): if not isinstance(settings.DAE_VERIFICATION_TOKEN_MAX_AGE, six.integer_types): errors.append(E010) + # DAE_ADMIN_SHOW_SEARCHBOX + if not isinstance(settings.DAE_ADMIN_SHOW_SEARCHBOX, bool): + errors.append(E011) + # and now hope, this is still empty! ;) return errors diff --git a/auth_enhanced/settings.py b/auth_enhanced/settings.py index 3a63caa..cf91cf1 100644 --- a/auth_enhanced/settings.py +++ b/auth_enhanced/settings.py @@ -92,6 +92,15 @@ def inject_setting(name, default_value): def set_app_default_settings(): """Sets all app-specific default settings.""" + # ### DAE_ADMIN_SHOW_SEARCHBOX + # This setting controls, if the searchbox in admin's list view will be + # activated. + # Possible values: + # False or True (default value, like Django default) + # In case of True, Django's default value for the fields to search in + # is used. + inject_setting('DAE_ADMIN_SHOW_SEARCHBOX', True) + # ### DAE_ADMIN_SIGNUP_NOTIFICATION # This setting controls, if emails will be sent to specified superusers/ # admins whenever a new user signs up. diff --git a/tests/test_checks.py b/tests/test_checks.py index 3ac03a5..ea437a9 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -14,7 +14,7 @@ # app imports from auth_enhanced.checks import ( - E001, E002, E003, E004, E008, E009, E010, W005, W006, W007, + E001, E002, E003, E004, E008, E009, E010, E011, W005, W006, W007, check_settings_values, ) from auth_enhanced.settings import ( @@ -173,3 +173,17 @@ def test_e010_invalid(self): Actually, 'None' is the only way to raise this error.""" errors = check_settings_values(None) self.assertEqual(errors, [E010]) + + @override_settings(DAE_ADMIN_SHOW_SEARCHBOX=True) + def test_e011_valid(self): + """Check should accept valid values.""" + errors = check_settings_values(None) + self.assertEqual(errors, []) + + @override_settings(DAE_ADMIN_SHOW_SEARCHBOX='foo') + def test_e011_invalid(self): + """Invalid values show an error message. + + Actually, 'None' is the only way to raise this error.""" + errors = check_settings_values(None) + self.assertEqual(errors, [E011]) From f096e952a917531488cd0a446ae9a17879de164f Mon Sep 17 00:00:00 2001 From: Mischback Date: Sat, 12 May 2018 19:52:33 +0200 Subject: [PATCH 05/20] Added documentation In fact, only the headlines for documentation were added. Real documentation will be done later. --- auth_enhanced/admin.py | 13 +++++++++++++ docs/source/settings.rst | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 4d6143e..fda2822 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -110,6 +110,19 @@ class EnhancedUserAdmin(UserAdmin): # Django's default value is just 'username' ordering = ('-is_superuser', '-is_staff', 'is_active', 'username') + def get_actions(self, request): + """Extends the default 'get_actions()'-method to exclude the bulk + action 'delete objects' from the dropdown.""" + + # get the original list of actions + actions = super(EnhancedUserAdmin, self).get_actions(request) + + # remove the action from the dropdown + if 'delete_selected' in actions: + del actions['delete_selected'] + + return actions + @register_only_debug(UserEnhancement) class UserEnhancementAdmin(admin.ModelAdmin): diff --git a/docs/source/settings.rst b/docs/source/settings.rst index dd9e259..c93a13d 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -17,6 +17,12 @@ Available Settings .. glossary:: + DAE_ADMIN_LIST_DISPLAY + TODO: document this setting! + + DAE_ADMIN_SHOW_SEARCHBOX + TODO: document this setting! + DAE_ADMIN_SIGNUP_NOTIFICATION Controls, if admins / superusers will be notified of new signups. From 27e2de2941880d769e180842002f484200d37650 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sat, 12 May 2018 20:02:35 +0200 Subject: [PATCH 06/20] Implemented 'status_aggregated' --- auth_enhanced/admin.py | 15 +++++++++++++++ tests/test_admin.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index fda2822..7377e3d 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -123,6 +123,21 @@ def get_actions(self, request): return actions + def status_aggregated(self, obj): + """Returns the status of an user as string. + + Possible values: 'user', 'staff', 'superuser'; these strings will be + localized.""" + + status = _('user') + if obj.is_superuser: + status = _('superuser') + elif obj.is_staff: + status = _('staff') + + return status + status_aggregated.short_description = _('Status') + @register_only_debug(UserEnhancement) class UserEnhancementAdmin(admin.ModelAdmin): diff --git a/tests/test_admin.py b/tests/test_admin.py index 057ddfb..beeb2c0 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -8,6 +8,7 @@ from unittest import skip # noqa # Django imports +from django.contrib.admin.sites import AdminSite from django.contrib.auth import get_user_model from django.test import override_settings, tag # noqa @@ -59,3 +60,37 @@ def test_filter_no_filter(self): # number of superusers in fixture is subject to changes! self.assertEqual(len(f_result), 6) + + +@tag('admin', 'current') +class EnhancedUserAdminUserRelatedTests(AuthEnhancedTestCase): + """These tests target the admin class and require some user objects.""" + + fixtures = ['tests/utils/fixtures/test_different_users.json'] + + @classmethod + def setUpClass(cls): + """Basic setup""" + + # call the parent constructor + super(EnhancedUserAdminUserRelatedTests, cls).setUpClass() + + cls.admin_obj = EnhancedUserAdmin(get_user_model(), AdminSite()) + + def test_status_aggregated_user(self): + """Does the method returns the correct status for simple users?""" + + u = get_user_model().objects.get(username='baz') + self.assertEqual(self.admin_obj.status_aggregated(u), 'user') + + def test_status_aggregated_staff(self): + """Does the method returns the correct status for staff users?""" + + u = get_user_model().objects.get(username='staff_user1') + self.assertEqual(self.admin_obj.status_aggregated(u), 'staff') + + def test_status_aggregated_superuser(self): + """Does the method returns the correct status for superusers?""" + + u = get_user_model().objects.get(username='django') + self.assertEqual(self.admin_obj.status_aggregated(u), 'superuser') From 125bc2a3ebd1a05f42529ef5cd68d9d133659cb3 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sat, 12 May 2018 22:11:08 +0200 Subject: [PATCH 07/20] Implemented 'username_status_color' --- auth_enhanced/admin.py | 30 ++++++++++++++++++++--- auth_enhanced/checks.py | 29 ++++++++++++++++++++++ docs/source/settings.rst | 3 +++ tests/test_admin.py | 53 +++++++++++++++++++++++++++++++++++++++- tests/test_checks.py | 30 ++++++++++++++++++++--- 5 files changed, 137 insertions(+), 8 deletions(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 7377e3d..b6eab49 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -6,6 +6,7 @@ from django.contrib import admin from django.contrib.auth import get_user_model from django.contrib.auth.admin import UserAdmin +from django.utils.html import format_html from django.utils.translation import ugettext_lazy as _ # app imports @@ -123,21 +124,44 @@ def get_actions(self, request): return actions - def status_aggregated(self, obj): + def status_aggregated(self, user_obj): """Returns the status of an user as string. Possible values: 'user', 'staff', 'superuser'; these strings will be localized.""" status = _('user') - if obj.is_superuser: + if user_obj.is_superuser: status = _('superuser') - elif obj.is_staff: + elif user_obj.is_staff: status = _('staff') return status status_aggregated.short_description = _('Status') + def username_status_color(self, user_obj): + """Returns a colored username, according to its status.""" + + try: + color = settings.DAE_ADMIN_USERNAME_STATUS_COLOR + except AttributeError: + return getattr(user_obj, user_obj.USERNAME_FIELD) + + if user_obj.is_superuser: + obj_color = color[0] + elif user_obj.is_staff: + obj_color = color[1] + else: + return getattr(user_obj, user_obj.USERNAME_FIELD) + + return format_html( + '{}', + obj_color, + getattr(user_obj, user_obj.USERNAME_FIELD) + ) + username_status_color.short_description = _('Username (status)') + username_status_color.admin_order_field = '-username' + @register_only_debug(UserEnhancement) class UserEnhancementAdmin(admin.ModelAdmin): diff --git a/auth_enhanced/checks.py b/auth_enhanced/checks.py index ab4a184..1d66354 100644 --- a/auth_enhanced/checks.py +++ b/auth_enhanced/checks.py @@ -9,6 +9,9 @@ 2) checks, that the logical connection between different settings is valid""" +# Python imports +import re + # Django imports from django.conf import settings from django.core.checks import Error, Warning @@ -146,6 +149,17 @@ id='dae.e011' ) +# DAE_ADMIN_USERNAME_STATUS_COLOR +E012 = Error( + _("'DAE_ADMIN_USERNAME_STATUS_COLOR' is set to an invalid value!"), + hint=_( + "Please check your settings and ensure, that " + "'DAE_ADMIN_USERNAME_STATUS_COLOR' is set to a tuple containing two " + "valid hexadecimal color codes ('#rrggbb')." + ), + id='dae.e012' +) + def check_settings_values(app_configs, **kwargs): """Checks, if the app-specific settings have valid values.""" @@ -236,5 +250,20 @@ def check_settings_values(app_configs, **kwargs): if not isinstance(settings.DAE_ADMIN_SHOW_SEARCHBOX, bool): errors.append(E011) + # DAE_ADMIN_USERNAME_STATUS_COLOR + try: + if not isinstance(settings.DAE_ADMIN_USERNAME_STATUS_COLOR, tuple): + errors.append(E012) + elif len(settings.DAE_ADMIN_USERNAME_STATUS_COLOR) != 2: + errors.append(E012) + else: + if ( + not re.match('^#[0-9A-Fa-f]{6}$', settings.DAE_ADMIN_USERNAME_STATUS_COLOR[0]) or + not re.match('^#[0-9A-Fa-f]{6}$', settings.DAE_ADMIN_USERNAME_STATUS_COLOR[1]) + ): + errors.append(E012) + except AttributeError: + pass + # and now hope, this is still empty! ;) return errors diff --git a/docs/source/settings.rst b/docs/source/settings.rst index c93a13d..6b4daec 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -31,6 +31,9 @@ Available Settings * ``False`` (default value): No notification will be sent. * a tuple of the following structure: ``('django', 'django@localhost', ('mail', )),``, where ``'django'`` is a username, ``'django@localhost'`` a valid email address and ``('mail', )`` a tuple of notification methods. As of now, only ``'mail'`` is supported. + DAE_ADMIN_USERNAME_STATUS_COLOR + TODO: document this setting! + DAE_EMAIL_ADMIN_NOTIFICATION_PREFIX All emails to admins / superusers will have a subject, that is prefixed with this string, put in ``[]``, i.e. if this setting is set to ``'foo'``, diff --git a/tests/test_admin.py b/tests/test_admin.py index beeb2c0..199a3d7 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -8,6 +8,7 @@ from unittest import skip # noqa # Django imports +from django.conf import settings from django.contrib.admin.sites import AdminSite from django.contrib.auth import get_user_model from django.test import override_settings, tag # noqa @@ -62,7 +63,7 @@ def test_filter_no_filter(self): self.assertEqual(len(f_result), 6) -@tag('admin', 'current') +@tag('admin') class EnhancedUserAdminUserRelatedTests(AuthEnhancedTestCase): """These tests target the admin class and require some user objects.""" @@ -94,3 +95,53 @@ def test_status_aggregated_superuser(self): u = get_user_model().objects.get(username='django') self.assertEqual(self.admin_obj.status_aggregated(u), 'superuser') + + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=None) + def test_username_status_color_missing_setting(self): + """Just returns the username, if no colors are specified.""" + + # actually delete the setting + # Please note, how the setting is first overridden and then deleted. + # This is done to ensure, that this works independently from the + # test settings. + del settings.DAE_ADMIN_USERNAME_STATUS_COLOR + + u = get_user_model().objects.get(username='django') + self.assertEqual( + self.admin_obj.username_status_color(u), getattr(u, u.USERNAME_FIELD) + ) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=('#0f0f0f', '#f0f0f0')) + def test_username_status_color_user(self): + """Just returns the username, if it is a normal user.""" + + u = get_user_model().objects.get(username='baz') + self.assertEqual( + self.admin_obj.username_status_color(u), getattr(u, u.USERNAME_FIELD) + ) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=('#0f0f0f', '#f0f0f0')) + def test_username_status_color_staff(self): + """Applies a color to the username, if staff status.""" + + u = get_user_model().objects.get(username='staff_user1') + self.assertEqual( + self.admin_obj.username_status_color(u), + '{}'.format( + '#f0f0f0', + getattr(u, u.USERNAME_FIELD) + ) + ) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=('#0f0f0f', '#f0f0f0')) + def test_username_status_color_superuser(self): + """Applies a color to the username, if superuser status.""" + + u = get_user_model().objects.get(username='django') + self.assertEqual( + self.admin_obj.username_status_color(u), + '{}'.format( + '#0f0f0f', + getattr(u, u.USERNAME_FIELD) + ) + ) diff --git a/tests/test_checks.py b/tests/test_checks.py index ea437a9..53f3eb1 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -14,7 +14,7 @@ # app imports from auth_enhanced.checks import ( - E001, E002, E003, E004, E008, E009, E010, E011, W005, W006, W007, + E001, E002, E003, E004, E008, E009, E010, E011, E012, W005, W006, W007, check_settings_values, ) from auth_enhanced.settings import ( @@ -182,8 +182,30 @@ def test_e011_valid(self): @override_settings(DAE_ADMIN_SHOW_SEARCHBOX='foo') def test_e011_invalid(self): - """Invalid values show an error message. - - Actually, 'None' is the only way to raise this error.""" + """Invalid values show an error message.""" errors = check_settings_values(None) self.assertEqual(errors, [E011]) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=('#0f0f0f', '#f0f0f0')) + def test_e012_valid(self): + """Check should accept valid values.""" + errors = check_settings_values(None) + self.assertEqual(errors, []) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR='foo') + def test_e012_invalid_no_tuple(self): + """Invalid values show an error message.""" + errors = check_settings_values(None) + self.assertEqual(errors, [E012]) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=('foo', 'bar', 'baz')) + def test_e012_invalid_invalid_number_of_parameters(self): + """Invalid values show an error message.""" + errors = check_settings_values(None) + self.assertEqual(errors, [E012]) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=('foo', 'bar')) + def test_e012_invalid_no_color_codes(self): + """Invalid values show an error message.""" + errors = check_settings_values(None) + self.assertEqual(errors, [E012]) From cb9a205587b1a9c5ef176ef8b7d488c30f8e53f9 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sat, 12 May 2018 22:53:14 +0200 Subject: [PATCH 08/20] Implemented 'username_status_char' --- auth_enhanced/admin.py | 19 +++++++++++++++++ auth_enhanced/checks.py | 26 ++++++++++++++++++++++++ docs/source/settings.rst | 3 +++ tests/test_admin.py | 44 ++++++++++++++++++++++++++++++++++++++++ tests/test_checks.py | 28 +++++++++++++++++++++++-- 5 files changed, 118 insertions(+), 2 deletions(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index b6eab49..5108b0c 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -139,6 +139,25 @@ def status_aggregated(self, user_obj): return status status_aggregated.short_description = _('Status') + def username_status_char(self, user_obj): + """Returns the username with a prefix, according to its status.""" + + try: + char = settings.DAE_ADMIN_USERNAME_STATUS_CHAR + except AttributeError: + return getattr(user_obj, user_obj.USERNAME_FIELD) + + if user_obj.is_superuser: + obj_char = char[0] + elif user_obj.is_staff: + obj_char = char[1] + else: + return getattr(user_obj, user_obj.USERNAME_FIELD) + + return format_html('[{}] {}', obj_char, getattr(user_obj, user_obj.USERNAME_FIELD)) + username_status_char.short_description = _('Username (status)') + username_status_char.admin_order_field = '-username' + def username_status_color(self, user_obj): """Returns a colored username, according to its status.""" diff --git a/auth_enhanced/checks.py b/auth_enhanced/checks.py index 1d66354..2a531fe 100644 --- a/auth_enhanced/checks.py +++ b/auth_enhanced/checks.py @@ -160,6 +160,17 @@ id='dae.e012' ) +# DAE_ADMIN_USERNAME_STATUS_CHAR +E013 = Error( + _("'DAE_ADMIN_USERNAME_STATUS_CHAR' is set to an invalid value!"), + hint=_( + "Please check your settings and ensure, that " + "'DAE_ADMIN_USERNAME_STATUS_CHAR' is set to a tuple containing two " + "single characters." + ), + id='dae.e013' +) + def check_settings_values(app_configs, **kwargs): """Checks, if the app-specific settings have valid values.""" @@ -265,5 +276,20 @@ def check_settings_values(app_configs, **kwargs): except AttributeError: pass + # DAE_ADMIN_USERNAME_STATUS_CHAR + try: + if not isinstance(settings.DAE_ADMIN_USERNAME_STATUS_CHAR, tuple): + errors.append(E013) + elif len(settings.DAE_ADMIN_USERNAME_STATUS_CHAR) != 2: + errors.append(E013) + else: + if ( + not re.match('^\S{1}$', settings.DAE_ADMIN_USERNAME_STATUS_CHAR[0]) or + not re.match('^\S{1}$', settings.DAE_ADMIN_USERNAME_STATUS_CHAR[1]) + ): + errors.append(E013) + except AttributeError: + pass + # and now hope, this is still empty! ;) return errors diff --git a/docs/source/settings.rst b/docs/source/settings.rst index 6b4daec..70b6b69 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -31,6 +31,9 @@ Available Settings * ``False`` (default value): No notification will be sent. * a tuple of the following structure: ``('django', 'django@localhost', ('mail', )),``, where ``'django'`` is a username, ``'django@localhost'`` a valid email address and ``('mail', )`` a tuple of notification methods. As of now, only ``'mail'`` is supported. + DAE_ADMIN_USERNAME_STATUS_CHAR + TODO: document this setting! + DAE_ADMIN_USERNAME_STATUS_COLOR TODO: document this setting! diff --git a/tests/test_admin.py b/tests/test_admin.py index 199a3d7..9902e4e 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -145,3 +145,47 @@ def test_username_status_color_superuser(self): getattr(u, u.USERNAME_FIELD) ) ) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=None) + def test_username_status_char_missing_setting(self): + """Just returns the username, if no colors are specified.""" + + # actually delete the setting + # Please note, how the setting is first overridden and then deleted. + # This is done to ensure, that this works independently from the + # test settings. + del settings.DAE_ADMIN_USERNAME_STATUS_CHAR + + u = get_user_model().objects.get(username='django') + self.assertEqual( + self.admin_obj.username_status_char(u), getattr(u, u.USERNAME_FIELD) + ) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=('a', 'b')) + def test_username_status_char_user(self): + """Just returns the username, if it is a normal user.""" + + u = get_user_model().objects.get(username='baz') + self.assertEqual( + self.admin_obj.username_status_char(u), getattr(u, u.USERNAME_FIELD) + ) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=('a', 'b')) + def test_username_status_char_staff(self): + """Applies a color to the username, if staff status.""" + + u = get_user_model().objects.get(username='staff_user1') + self.assertEqual( + self.admin_obj.username_status_char(u), + '[{}] {}'.format('b', getattr(u, u.USERNAME_FIELD)) + ) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=('a', 'b')) + def test_username_status_char_superuser(self): + """Applies a color to the username, if superuser status.""" + + u = get_user_model().objects.get(username='django') + self.assertEqual( + self.admin_obj.username_status_char(u), + '[{}] {}'.format('a', getattr(u, u.USERNAME_FIELD)) + ) diff --git a/tests/test_checks.py b/tests/test_checks.py index 53f3eb1..c1fb767 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -14,8 +14,8 @@ # app imports from auth_enhanced.checks import ( - E001, E002, E003, E004, E008, E009, E010, E011, E012, W005, W006, W007, - check_settings_values, + E001, E002, E003, E004, E008, E009, E010, E011, E012, E013, W005, W006, + W007, check_settings_values, ) from auth_enhanced.settings import ( DAE_CONST_MODE_AUTO_ACTIVATION, DAE_CONST_RECOMMENDED_LOGIN_URL, @@ -209,3 +209,27 @@ def test_e012_invalid_no_color_codes(self): """Invalid values show an error message.""" errors = check_settings_values(None) self.assertEqual(errors, [E012]) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=('#', '$')) + def test_e013_valid(self): + """Check should accept valid values.""" + errors = check_settings_values(None) + self.assertEqual(errors, []) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR='foo') + def test_e013_invalid_no_tuple(self): + """Invalid values show an error message.""" + errors = check_settings_values(None) + self.assertEqual(errors, [E013]) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=('#', '$', '!')) + def test_e013_invalid_invalid_number_of_parameters(self): + """Invalid values show an error message.""" + errors = check_settings_values(None) + self.assertEqual(errors, [E013]) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=('foo', 'bar')) + def test_e013_invalid_no_color_codes(self): + """Invalid values show an error message.""" + errors = check_settings_values(None) + self.assertEqual(errors, [E013]) From d3038d7dc42c75ea399fd42a038bc5a03786c56f Mon Sep 17 00:00:00 2001 From: Mischback Date: Sun, 13 May 2018 12:24:00 +0200 Subject: [PATCH 09/20] Added a legend to 'change_list'-view --- auth_enhanced/admin.py | 40 +++++++++- .../admin/auth/user/change_list.html | 16 ++++ tests/test_admin.py | 73 ++++++++++++++++++- tests/test_checks.py | 27 +++++++ tests/utils/settings_dev.py | 2 + 5 files changed, 155 insertions(+), 3 deletions(-) create mode 100644 auth_enhanced/templates/admin/auth/user/change_list.html diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 5108b0c..e404eda 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -111,6 +111,21 @@ class EnhancedUserAdmin(UserAdmin): # Django's default value is just 'username' ordering = ('-is_superuser', '-is_staff', 'is_active', 'username') + def changelist_view(self, request, extra_context=None): + """Pass some more context into the view. + + This is used to: + - provide the legend for color-coded or prefixed usernames. + + The legend will be provided at the bottom of the list. Please note, + that a modified/extended template is used to provide the actual + HTML-code for this.""" + + extra_context = extra_context or {} + extra_context['enhanceduseradmin_legend'] = self.get_additional_legend() + + return super(EnhancedUserAdmin, self).changelist_view(request, extra_context=extra_context) + def get_actions(self, request): """Extends the default 'get_actions()'-method to exclude the bulk action 'delete objects' from the dropdown.""" @@ -124,6 +139,29 @@ def get_actions(self, request): return actions + def get_additional_legend(self): + """Returns a legend depending on the applied settings. + + Especially relevant is the 'list_display' attribute of this class and + the settings of 'DAE_ADMIN_USERNAME_STATUS_CHAR' and + 'DAE_ADMIN_USERNAME_STATUS_COLOR'.""" + + result = {} + + try: + if 'username_status_char' in self.list_display: + result['char'] = settings.DAE_ADMIN_USERNAME_STATUS_CHAR + except AttributeError: + pass + + try: + if 'username_status_color' in self.list_display: + result['color'] = settings.DAE_ADMIN_USERNAME_STATUS_COLOR + except AttributeError: + pass + + return result + def status_aggregated(self, user_obj): """Returns the status of an user as string. @@ -154,7 +192,7 @@ def username_status_char(self, user_obj): else: return getattr(user_obj, user_obj.USERNAME_FIELD) - return format_html('[{}] {}', obj_char, getattr(user_obj, user_obj.USERNAME_FIELD)) + return format_html('[{}]{}', obj_char, getattr(user_obj, user_obj.USERNAME_FIELD)) username_status_char.short_description = _('Username (status)') username_status_char.admin_order_field = '-username' diff --git a/auth_enhanced/templates/admin/auth/user/change_list.html b/auth_enhanced/templates/admin/auth/user/change_list.html new file mode 100644 index 0000000..e4fb959 --- /dev/null +++ b/auth_enhanced/templates/admin/auth/user/change_list.html @@ -0,0 +1,16 @@ +{% extends "admin/change_list.html" %} +{% load i18n admin_urls static admin_list %} +{% comment %} +This template will provide a legend for the list view of users. +If a custom user model is used, this template must be provided under the following path 'templates/[app_name]/[user_model_name]/change_list.html'. +The legend will take into account, which presentation of usernames is used and will include a legend only, if a custom presentation is specified. +{% endcomment %} +{% block result_list %} + {{ block.super }} + {% if enhanceduseradmin_legend %} +

+ {% blocktrans %}Legend{% endblocktrans %}: + {% if enhanceduseradmin_legend.color %}Superuser, Staff{% endif %}{% if enhanceduseradmin_legend.color and enhanceduseradmin_legend.char %}, {% endif %}{% if enhanceduseradmin_legend.char %}[{{ enhanceduseradmin_legend.char.0 }}]Superuser, [{{ enhanceduseradmin_legend.char.1 }}]Staff{% endif %} +

+ {% endif %} +{% endblock %} diff --git a/tests/test_admin.py b/tests/test_admin.py index 9902e4e..26fa5e5 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -177,7 +177,7 @@ def test_username_status_char_staff(self): u = get_user_model().objects.get(username='staff_user1') self.assertEqual( self.admin_obj.username_status_char(u), - '[{}] {}'.format('b', getattr(u, u.USERNAME_FIELD)) + '[{}]{}'.format('b', getattr(u, u.USERNAME_FIELD)) ) @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=('a', 'b')) @@ -187,5 +187,74 @@ def test_username_status_char_superuser(self): u = get_user_model().objects.get(username='django') self.assertEqual( self.admin_obj.username_status_char(u), - '[{}] {}'.format('a', getattr(u, u.USERNAME_FIELD)) + '[{}]{}'.format('a', getattr(u, u.USERNAME_FIELD)) + ) + + +class EnhancedUserAdminTests(AuthEnhancedTestCase): + + def setUp(self): + """Per test setup""" + + # create an instance of the admin class + self.admin_obj = EnhancedUserAdmin(get_user_model(), AdminSite()) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=('a', 'b')) + def test_get_legend_char(self): + """Is the legend properly populated?""" + + # override 'list_display' per test + self.admin_obj.list_display = ('username_status_char', ) + + self.assertEqual( + self.admin_obj.get_additional_legend(), + {'char': ('a', 'b')} + ) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=None) + def test_get_legend_char_missing_setting(self): + """Legend remains empty, if the corresponding setting is missing.""" + + # override 'list_display' per test + self.admin_obj.list_display = ('username_status_char', ) + + # actually delete the setting + # Please note, how the setting is first overridden and then deleted. + # This is done to ensure, that this works independently from the + # test settings. + del settings.DAE_ADMIN_USERNAME_STATUS_CHAR + + self.assertEqual( + self.admin_obj.get_additional_legend(), + {} + ) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=('#f0f0f0', '#0f0f0f')) + def test_get_legend_color(self): + """Is the legend properly populated?""" + + # override 'list_display' per test + self.admin_obj.list_display = ('username_status_color', ) + + self.assertEqual( + self.admin_obj.get_additional_legend(), + {'color': ('#f0f0f0', '#0f0f0f')} + ) + + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=None) + def test_get_legend_color_missing_setting(self): + """Legend remains empty, if the corresponding setting is missing.""" + + # override 'list_display' per test + self.admin_obj.list_display = ('username_status_color', ) + + # actually delete the setting + # Please note, how the setting is first overridden and then deleted. + # This is done to ensure, that this works independently from the + # test settings. + del settings.DAE_ADMIN_USERNAME_STATUS_COLOR + + self.assertEqual( + self.admin_obj.get_additional_legend(), + {} ) diff --git a/tests/test_checks.py b/tests/test_checks.py index c1fb767..24f9fff 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -10,6 +10,7 @@ from unittest import skip # noqa # Django imports +from django.conf import settings from django.test import override_settings, tag # noqa # app imports @@ -192,6 +193,19 @@ def test_e012_valid(self): errors = check_settings_values(None) self.assertEqual(errors, []) + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=None) + def test_e012_missing_setting(self): + """Missing setting will be ignored.""" + + # actually delete the setting + # Please note, how the setting is first overridden and then deleted. + # This is done to ensure, that this works independently from the + # test settings. + del settings.DAE_ADMIN_USERNAME_STATUS_COLOR + + errors = check_settings_values(None) + self.assertEqual(errors, []) + @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR='foo') def test_e012_invalid_no_tuple(self): """Invalid values show an error message.""" @@ -216,6 +230,19 @@ def test_e013_valid(self): errors = check_settings_values(None) self.assertEqual(errors, []) + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR=None) + def test_e013_missing_setting(self): + """Missing setting will be ignored.""" + + # actually delete the setting + # Please note, how the setting is first overridden and then deleted. + # This is done to ensure, that this works independently from the + # test settings. + del settings.DAE_ADMIN_USERNAME_STATUS_CHAR + + errors = check_settings_values(None) + self.assertEqual(errors, []) + @override_settings(DAE_ADMIN_USERNAME_STATUS_CHAR='foo') def test_e013_invalid_no_tuple(self): """Invalid values show an error message.""" diff --git a/tests/utils/settings_dev.py b/tests/utils/settings_dev.py index 51fedf0..348626c 100644 --- a/tests/utils/settings_dev.py +++ b/tests/utils/settings_dev.py @@ -87,3 +87,5 @@ EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' ### CURRENT DEVELOPMENT +#DAE_ADMIN_LIST_DISPLAY = ('username_status_color', 'email') +#DAE_ADMIN_USERNAME_STATUS_COLOR = ('#cc0000', '#00cc00') From 2e039b78e62302b0eb36f83acfa5ee1197a52521 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sun, 13 May 2018 15:10:55 +0200 Subject: [PATCH 10/20] Implemented 'email_with_verification_status' --- auth_enhanced/admin.py | 15 +++++++++++++++ tests/utils/settings_dev.py | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index e404eda..b8b2aff 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -4,6 +4,7 @@ # Django imports from django.conf import settings from django.contrib import admin +from django.contrib.admin.templatetags.admin_list import _boolean_icon from django.contrib.auth import get_user_model from django.contrib.auth.admin import UserAdmin from django.utils.html import format_html @@ -111,6 +112,10 @@ class EnhancedUserAdmin(UserAdmin): # Django's default value is just 'username' ordering = ('-is_superuser', '-is_staff', 'is_active', 'username') + # 'list_select_related' will automatically add an 'select_related' + # statement to the queryset and thus reduce the number of SQL queries. + list_select_related = ('enhancement',) + def changelist_view(self, request, extra_context=None): """Pass some more context into the view. @@ -126,6 +131,16 @@ def changelist_view(self, request, extra_context=None): return super(EnhancedUserAdmin, self).changelist_view(request, extra_context=extra_context) + def email_with_verification_status(self, user_obj): + """Combines the email-address and the email's verification status.""" + + # get the icon + icon = _boolean_icon(user_obj.enhancement.email_is_verified) + + return format_html('{} {}', icon, getattr(user_obj, user_obj.EMAIL_FIELD)) + email_with_verification_status.short_description = _('EMail Address') + email_with_verification_status.admin_order_field = '-email' + def get_actions(self, request): """Extends the default 'get_actions()'-method to exclude the bulk action 'delete objects' from the dropdown.""" diff --git a/tests/utils/settings_dev.py b/tests/utils/settings_dev.py index 348626c..960a4a3 100644 --- a/tests/utils/settings_dev.py +++ b/tests/utils/settings_dev.py @@ -87,5 +87,5 @@ EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' ### CURRENT DEVELOPMENT -#DAE_ADMIN_LIST_DISPLAY = ('username_status_color', 'email') -#DAE_ADMIN_USERNAME_STATUS_COLOR = ('#cc0000', '#00cc00') +DAE_ADMIN_LIST_DISPLAY = ('username_status_color', 'email_with_verification_status') +DAE_ADMIN_USERNAME_STATUS_COLOR = ('#cc0000', '#00cc00') From 1a441a31b354b03f159cfe30568524b9b784a9e7 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sun, 13 May 2018 18:32:54 +0200 Subject: [PATCH 11/20] Implemented 'action_bulk_activate_user' --- auth_enhanced/admin.py | 86 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index b8b2aff..06d6b4a 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -7,11 +7,13 @@ from django.contrib.admin.templatetags.admin_list import _boolean_icon from django.contrib.auth import get_user_model from django.contrib.auth.admin import UserAdmin +from django.contrib.messages import ERROR, SUCCESS, WARNING from django.utils.html import format_html -from django.utils.translation import ugettext_lazy as _ +from django.utils.translation import ugettext_lazy as _, ungettext_lazy # app imports from auth_enhanced.models import UserEnhancement +from auth_enhanced.settings import DAE_CONST_MODE_EMAIL_ACTIVATION def register_only_debug(*models, **kwargs): @@ -116,6 +118,88 @@ class EnhancedUserAdmin(UserAdmin): # statement to the queryset and thus reduce the number of SQL queries. list_select_related = ('enhancement',) + actions = ['action_bulk_activate_user', ] + + def action_bulk_activate_user(self, request, queryset, user_id=None): + """Performs bulk activation of users in Django admin. + + This action is accessible from the drop-down menu and works together + with selecting user objects by checking their respective checkbox. + Furthermore, it handles the actual activation of single user aswell, + because ultimatively, this method is used to perform the activation + when 'action_activate_user()' is called.""" + + activated = [] + not_activated = [] + user_model = get_user_model() + + # the method is called from 'action_activate_user', so a queryset has + # to be constructed... + if user_id and not queryset: + try: + queryset = [user_model.objects.get(pk=user_id)] + except user_model.DoesNotExist: + # provide an empty queryset. This mimics the behaviour of + # Django, if invalid user IDs are provided in the POST-request + queryset = [] + + # at this point, 'queryset' is filled and can be iterated + for user in queryset: + if ( + settings.DAE_OPERATION_MODE == DAE_CONST_MODE_EMAIL_ACTIVATION and + not user.enhancement.email_is_verified + ): + not_activated.append(getattr(user, user_model.USERNAME_FIELD)) + else: + user.is_active = True + user.save() + activated.append(getattr(user, user_model.USERNAME_FIELD)) + + # return messages for successfully activated accounts + if activated: + count = len(activated) + self.message_user( + request, + ungettext_lazy( + '%(count)d user was activated successfully (%(activated_list)s).', + '%(count)d users were activated successfully (%(activated_list)s).', + count + ) % { + 'count': count, + 'activated_list': ', '.join(activated), + }, + SUCCESS, + ) + + # return messages for accounts, that could not be activated, because + # their mail address is not verified + if not_activated: + count = len(not_activated) + self.message_user( + request, + ungettext_lazy( + "%(count)d user could not be activated, because his email " + "address is not verified (%(activated_list)s)!", + "%(count)d users could not be activated, because their " + "email addresses are not verified (%(activated_list)s)!", + count + ) % { + 'count': count, + 'activated_list': ', '.join(not_activated), + }, + ERROR, + ) + + # the method did nothing; this means something unexpected happened, + # i.e. invalid user IDs were provided + if not (activated or not_activated): + self.message_user( + request, + _('Nothing was done. Probably this means, that no or invalid user IDs were provided.'), + ERROR, + ) + action_bulk_activate_user.short_description = _('Activate selected users') + def changelist_view(self, request, extra_context=None): """Pass some more context into the view. From b59410a3eb215a476549962fd7e10002ad9fb5c0 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sun, 13 May 2018 20:25:57 +0200 Subject: [PATCH 12/20] Updated the test-fixture --- tests/test_admin.py | 48 +- tests/test_commands.py | 28 +- .../utils/fixtures/test_different_users.json | 458 +++++++++++++++--- tests/utils/settings_dev.py | 2 +- 4 files changed, 446 insertions(+), 90 deletions(-) diff --git a/tests/test_admin.py b/tests/test_admin.py index 26fa5e5..5b5b449 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -33,7 +33,7 @@ def test_filter_superusers(self): f_result = f.queryset(None, get_user_model().objects.all()) # number of superusers in fixture is subject to changes! - self.assertEqual(len(f_result), 3) + self.assertEqual(len(f_result), 7) def test_filter_staffusers(self): """Does the filter return the staff users?""" @@ -42,7 +42,7 @@ def test_filter_staffusers(self): f_result = f.queryset(None, get_user_model().objects.all()) # number of superusers in fixture is subject to changes! - self.assertEqual(len(f_result), 5) + self.assertEqual(len(f_result), 13) def test_filter_users(self): """Does the filter return the default users?""" @@ -51,7 +51,7 @@ def test_filter_users(self): f_result = f.queryset(None, get_user_model().objects.all()) # number of superusers in fixture is subject to changes! - self.assertEqual(len(f_result), 1) + self.assertEqual(len(f_result), 6) def test_filter_no_filter(self): """If the filter is not applied, return the full user list.""" @@ -60,7 +60,7 @@ def test_filter_no_filter(self): f_result = f.queryset(None, get_user_model().objects.all()) # number of superusers in fixture is subject to changes! - self.assertEqual(len(f_result), 6) + self.assertEqual(len(f_result), 19) @tag('admin') @@ -81,19 +81,19 @@ def setUpClass(cls): def test_status_aggregated_user(self): """Does the method returns the correct status for simple users?""" - u = get_user_model().objects.get(username='baz') + u = get_user_model().objects.get(username='user') self.assertEqual(self.admin_obj.status_aggregated(u), 'user') def test_status_aggregated_staff(self): """Does the method returns the correct status for staff users?""" - u = get_user_model().objects.get(username='staff_user1') + u = get_user_model().objects.get(username='staff') self.assertEqual(self.admin_obj.status_aggregated(u), 'staff') def test_status_aggregated_superuser(self): """Does the method returns the correct status for superusers?""" - u = get_user_model().objects.get(username='django') + u = get_user_model().objects.get(username='superuser') self.assertEqual(self.admin_obj.status_aggregated(u), 'superuser') @override_settings(DAE_ADMIN_USERNAME_STATUS_COLOR=None) @@ -106,7 +106,7 @@ def test_username_status_color_missing_setting(self): # test settings. del settings.DAE_ADMIN_USERNAME_STATUS_COLOR - u = get_user_model().objects.get(username='django') + u = get_user_model().objects.get(username='superuser') self.assertEqual( self.admin_obj.username_status_color(u), getattr(u, u.USERNAME_FIELD) ) @@ -115,7 +115,7 @@ def test_username_status_color_missing_setting(self): def test_username_status_color_user(self): """Just returns the username, if it is a normal user.""" - u = get_user_model().objects.get(username='baz') + u = get_user_model().objects.get(username='user') self.assertEqual( self.admin_obj.username_status_color(u), getattr(u, u.USERNAME_FIELD) ) @@ -124,7 +124,7 @@ def test_username_status_color_user(self): def test_username_status_color_staff(self): """Applies a color to the username, if staff status.""" - u = get_user_model().objects.get(username='staff_user1') + u = get_user_model().objects.get(username='staff') self.assertEqual( self.admin_obj.username_status_color(u), '{}'.format( @@ -137,7 +137,7 @@ def test_username_status_color_staff(self): def test_username_status_color_superuser(self): """Applies a color to the username, if superuser status.""" - u = get_user_model().objects.get(username='django') + u = get_user_model().objects.get(username='superuser') self.assertEqual( self.admin_obj.username_status_color(u), '{}'.format( @@ -156,7 +156,7 @@ def test_username_status_char_missing_setting(self): # test settings. del settings.DAE_ADMIN_USERNAME_STATUS_CHAR - u = get_user_model().objects.get(username='django') + u = get_user_model().objects.get(username='superuser') self.assertEqual( self.admin_obj.username_status_char(u), getattr(u, u.USERNAME_FIELD) ) @@ -165,7 +165,7 @@ def test_username_status_char_missing_setting(self): def test_username_status_char_user(self): """Just returns the username, if it is a normal user.""" - u = get_user_model().objects.get(username='baz') + u = get_user_model().objects.get(username='user') self.assertEqual( self.admin_obj.username_status_char(u), getattr(u, u.USERNAME_FIELD) ) @@ -174,7 +174,7 @@ def test_username_status_char_user(self): def test_username_status_char_staff(self): """Applies a color to the username, if staff status.""" - u = get_user_model().objects.get(username='staff_user1') + u = get_user_model().objects.get(username='staff') self.assertEqual( self.admin_obj.username_status_char(u), '[{}]{}'.format('b', getattr(u, u.USERNAME_FIELD)) @@ -184,14 +184,16 @@ def test_username_status_char_staff(self): def test_username_status_char_superuser(self): """Applies a color to the username, if superuser status.""" - u = get_user_model().objects.get(username='django') + u = get_user_model().objects.get(username='superuser') self.assertEqual( self.admin_obj.username_status_char(u), '[{}]{}'.format('a', getattr(u, u.USERNAME_FIELD)) ) +@tag('admin') class EnhancedUserAdminTests(AuthEnhancedTestCase): + """These tests target the admin class, but do not require any user objects.""" def setUp(self): """Per test setup""" @@ -258,3 +260,19 @@ def test_get_legend_color_missing_setting(self): self.admin_obj.get_additional_legend(), {} ) + + +@tag('admin') +class EnhancedUserAdminRequestsTests(AuthEnhancedTestCase): + """These tests target the admin class, but rely on real requests.""" + + fixtures = ['tests/utils/fixtures/test_different_users.json'] + + def setUp(self): + self.client.force_login( + get_user_model().objects.get(username='superuser') + ) + + def test_action_bulk_activate_single_user(self): + """Activate a single user by dropdown.""" + pass diff --git a/tests/test_commands.py b/tests/test_commands.py index 77b262a..01dc167 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -113,7 +113,7 @@ class CheckAdminNotificationTests(AuthEnhancedTestCase): @override_settings(DAE_ADMIN_SIGNUP_NOTIFICATION=( ('django', 'django@localhost', ('mail', )), - ('foo', 'foo@localhost', ('mail', )), + ('superuser', 'superuser@localhost', ('mail', )), )) def test_all_valid(self): """Returns True, if the setting is completely valid.""" @@ -122,8 +122,8 @@ def test_all_valid(self): @override_settings(DAE_ADMIN_SIGNUP_NOTIFICATION=( ('django', 'django@localhost', ('mail', )), - ('foo', 'foo@localhost', ('mail', )), - ('bar', 'bar@localhost', ('mail', )), + ('superuser', 'superuser@localhost', ('mail', )), + ('su_inactive_2', 'su_inactive_2@localhost', ('mail', )), )) def test_address_unverified(self): """Raises an exception, if one of the accounts has an unverified email @@ -131,15 +131,15 @@ def test_address_unverified(self): with self.assertRaisesMessage( CommandError, - "The following accounts do not have a verified email address: bar. " - "Administrative notifications will only be sent to verified email " - "addresses." + "The following accounts do not have a verified email address: " + "su_inactive_2. Administrative notifications will only be sent to " + "verified email addresses." ): check_admin_notification() @override_settings(DAE_ADMIN_SIGNUP_NOTIFICATION=( ('django', 'django@localhost', ('mail', )), - ('foo', 'bar@localhost', ('mail', )), + ('superuser', 'foo@localhost', ('mail', )), )) def test_address_not_matching(self): """Raises an exception, if one of the accounts has an unverified email @@ -147,17 +147,17 @@ def test_address_not_matching(self): with self.assertRaisesMessage( CommandError, - "The following accounts do not match the project's settings: foo. " - "The specified email addresses are not the ones associated with " - "the account. Administrative notifications will only be sent to " - "registered email addresses." + "The following accounts do not match the project's settings: " + "superuser. The specified email addresses are not the ones " + "associated with the account. Administrative notifications will " + "only be sent to registered email addresses." ): check_admin_notification() @override_settings(DAE_ADMIN_SIGNUP_NOTIFICATION=( ('django', 'django@localhost', ('mail', )), - ('foo', 'foo@localhost', ('mail', )), - ('baz', 'baz@localhost', ('mail', )), + ('superuser', 'superuser@localhost', ('mail', )), + ('user', 'user@localhost', ('mail', )), )) def test_insufficient_permissions(self): """Raises an exception, if one of the accounts does not have sufficient @@ -166,7 +166,7 @@ def test_insufficient_permissions(self): with self.assertRaisesMessage( CommandError, "The following accounts do not have the sufficient permissions to " - "actually modify accounts: baz." + "actually modify accounts: user." ): check_admin_notification() diff --git a/tests/utils/fixtures/test_different_users.json b/tests/utils/fixtures/test_different_users.json index 046a6b3..b72daa3 100644 --- a/tests/utils/fixtures/test_different_users.json +++ b/tests/utils/fixtures/test_different_users.json @@ -3,8 +3,8 @@ "model": "auth.user", "pk": 1, "fields": { - "password": "pbkdf2_sha256$100000$cmT7Sn6F8MDR$roOaI7UWMsBft3jCnE03mA/WONqVVNjR8HqJCPK+Ts8=", - "last_login": null, + "password": "pbkdf2_sha256$100000$R64FbUqOu4Jy$FZYm0VPf30ARjs2NoUTJBy/dcM6BpG+4decpHVF3Afc=", + "last_login": "2018-05-13T12:46:22.899", "is_superuser": true, "username": "django", "first_name": "", @@ -12,7 +12,7 @@ "email": "django@localhost", "is_staff": true, "is_active": true, - "date_joined": "2018-05-11T15:09:47.750", + "date_joined": "2018-05-13T12:32:23.331", "groups": [], "user_permissions": [] } @@ -21,16 +21,16 @@ "model": "auth.user", "pk": 2, "fields": { - "password": "pbkdf2_sha256$100000$O281wr64DfOP$D9kqIbQbkf7t0e/QFgtoFVDB4eovVodIFBjsflCvdgA=", + "password": "pbkdf2_sha256$100000$bDdAGd1rPAyp$nkwaHyZo/GKBxhFGKsjnbOveN4nqJhI8jIg8cuivo8U=", "last_login": null, "is_superuser": true, - "username": "foo", + "username": "superuser", "first_name": "", "last_name": "", - "email": "foo@localhost", + "email": "superuser@localhost", "is_staff": true, "is_active": true, - "date_joined": "2018-05-11T15:10:18.134", + "date_joined": "2018-05-13T12:35:18.733", "groups": [], "user_permissions": [] } @@ -39,16 +39,16 @@ "model": "auth.user", "pk": 3, "fields": { - "password": "pbkdf2_sha256$100000$Y9EhOQADMfqI$XyehmEjtq2CRmZlzDCTZmT4M9g9nrvXC4vAEXqqmEjQ=", + "password": "pbkdf2_sha256$100000$sPEZ8lmqWCo2$DxmWuX3SqUZoGV9dtliLFlTVZ9Sw2wD032vUz0Vkdgs=", "last_login": null, "is_superuser": true, - "username": "bar", + "username": "superuser_not_verified", "first_name": "", "last_name": "", - "email": "bar@localhost", + "email": "superuser_not_verified@localhost", "is_staff": true, "is_active": true, - "date_joined": "2018-05-11T15:10:58.126", + "date_joined": "2018-05-13T12:36:18.757", "groups": [], "user_permissions": [] } @@ -57,55 +57,289 @@ "model": "auth.user", "pk": 4, "fields": { - "password": "pbkdf2_sha256$100000$lV52crpDsEzY$ayhyunUhctRHY2JcTiOnGZqT72VzfVk9uqLNdRmKKaA=", + "password": "pbkdf2_sha256$100000$7SpuNw8nr93m$doMHZHdIFoQYwdAjWwCDcxBNKYKC8rQ3Umge8x+xDlw=", + "last_login": null, + "is_superuser": true, + "username": "superuser_in_progress", + "first_name": "", + "last_name": "", + "email": "superuser_in_progress@localhost", + "is_staff": true, + "is_active": true, + "date_joined": "2018-05-13T12:44:30.257", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 5, + "fields": { + "password": "pbkdf2_sha256$100000$1bAlt74zoYqx$yQBJPYZnBiHGBUqm919+JLwoIqc3AnraCTZ1hboCo0Q=", + "last_login": null, + "is_superuser": true, + "username": "su_inactive_1", + "first_name": "", + "last_name": "", + "email": "su_inactive_1@localhost", + "is_staff": true, + "is_active": false, + "date_joined": "2018-05-13T12:48:58", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 6, + "fields": { + "password": "pbkdf2_sha256$100000$I0i3DoEGSorr$mxR2tayLH2YX5Fcx7yyjSyXzI+Z5sGTQhgjkuhZLmQA=", + "last_login": null, + "is_superuser": true, + "username": "su_inactive_2", + "first_name": "", + "last_name": "", + "email": "su_inactive_2@localhost", + "is_staff": true, + "is_active": false, + "date_joined": "2018-05-13T12:49:36", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 7, + "fields": { + "password": "pbkdf2_sha256$100000$dgYLUsF3QbAu$/ueKbtgiC023/5SjwQrE87YifyokzjjW2VnStKBf2Bc=", + "last_login": null, + "is_superuser": true, + "username": "su_inactive_3", + "first_name": "", + "last_name": "", + "email": "su_inactive_3@localhost", + "is_staff": true, + "is_active": false, + "date_joined": "2018-05-13T12:49:49", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 8, + "fields": { + "password": "pbkdf2_sha256$100000$x1bE6a03txio$tsZWJqeYbTQ1YfvQaRxTuVRlOW9XBkK3xhuzDe2ABqU=", "last_login": null, "is_superuser": false, - "username": "baz", + "username": "staff", "first_name": "", "last_name": "", - "email": "baz@localhost", + "email": "staff@localhost", + "is_staff": true, + "is_active": true, + "date_joined": "2018-05-13T12:51:27", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 9, + "fields": { + "password": "pbkdf2_sha256$100000$wo6lHNqw03mt$z+/C2U7zu0n+KeyO6HgsErFVD0zgbWfLW3NjkxgGFxU=", + "last_login": null, + "is_superuser": false, + "username": "staff_not_verified", + "first_name": "", + "last_name": "", + "email": "staff_not_verified@localhost", + "is_staff": true, + "is_active": true, + "date_joined": "2018-05-13T12:52:05", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 10, + "fields": { + "password": "pbkdf2_sha256$100000$PYQ5FbghZTDv$dgP2bUfpCY0dlOA0+pQsdvb1cYLGWvc8OWJHYOhbPHs=", + "last_login": null, + "is_superuser": false, + "username": "staff_in_progress", + "first_name": "", + "last_name": "", + "email": "staff_in_progress@localhost", + "is_staff": true, + "is_active": true, + "date_joined": "2018-05-13T12:52:34", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 11, + "fields": { + "password": "pbkdf2_sha256$100000$Zor7zrjSntIl$Gb0fBC5UWbDY7pK+3ic8rYU4F//WeQaAwJRdG0emSBg=", + "last_login": null, + "is_superuser": false, + "username": "staff1", + "first_name": "", + "last_name": "", + "email": "staff1@localhost", + "is_staff": true, + "is_active": false, + "date_joined": "2018-05-13T12:53:07", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 12, + "fields": { + "password": "pbkdf2_sha256$100000$33cjRQNOELRT$RNV6LPMe09Z1t2qwpwwOKQOENQeA5UwgzI9Vj6LXDvM=", + "last_login": null, + "is_superuser": false, + "username": "staff2", + "first_name": "", + "last_name": "", + "email": "staff2@localhost", + "is_staff": true, + "is_active": false, + "date_joined": "2018-05-13T12:53:46", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 13, + "fields": { + "password": "pbkdf2_sha256$100000$jK9yS8cbG7rK$Lp9wqp0AeKHkj8v85eDH7+Mo0OLpuD3VAb2UfPYfuk0=", + "last_login": null, + "is_superuser": false, + "username": "staff3", + "first_name": "", + "last_name": "", + "email": "staff3@localhost", + "is_staff": true, + "is_active": false, + "date_joined": "2018-05-13T12:54:05", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 14, + "fields": { + "password": "pbkdf2_sha256$100000$hHgF2wt0yS1c$1rh1n1zPr6Ijo64Px3BlyLNS5+Nd5C7Vv+BIXNs/zkg=", + "last_login": null, + "is_superuser": false, + "username": "user", + "first_name": "", + "last_name": "", + "email": "user@localhost", "is_staff": false, "is_active": true, - "date_joined": "2018-05-11T15:12:20", + "date_joined": "2018-05-13T12:55:48", "groups": [], "user_permissions": [] } }, { - "model": "auth.user", - "pk": 5, - "fields": { - "password": "pbkdf2_sha256$100000$lV52crpDsEzY$ayhyunUhctRHY2JcTiOnGZqT72VzfVk9uqLNdRmKKaA=", - "last_login": null, - "is_superuser": false, - "username": "staff_user1", - "first_name": "", - "last_name": "", - "email": "staff_user1@localhost", - "is_staff": true, - "is_active": true, - "date_joined": "2018-05-11T15:12:20", - "groups": [], - "user_permissions": [] - } + "model": "auth.user", + "pk": 15, + "fields": { + "password": "pbkdf2_sha256$100000$gqb1sHxIU9IK$rl4Tw7V69z93VfTp3uJN+HQ+s1/5tOLMvkhh5GQH7M8=", + "last_login": null, + "is_superuser": false, + "username": "user_not_verified", + "first_name": "", + "last_name": "", + "email": "user_not_verified@localhost", + "is_staff": false, + "is_active": true, + "date_joined": "2018-05-13T12:56:09", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 16, + "fields": { + "password": "pbkdf2_sha256$100000$UK5hQwf9WxEC$SrfKt8G3oW6fOJxcuWGX7j9PmQmbk50VFR8aSUt+nw0=", + "last_login": null, + "is_superuser": false, + "username": "user_in_progress", + "first_name": "", + "last_name": "", + "email": "user_in_progress@localhost", + "is_staff": false, + "is_active": true, + "date_joined": "2018-05-13T12:56:35", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 17, + "fields": { + "password": "pbkdf2_sha256$100000$FSB24sBm2N28$TETou+koQzKxSYVi00PB6eP/C5rbWSc9Nx8DjKahkoM=", + "last_login": null, + "is_superuser": false, + "username": "user1", + "first_name": "", + "last_name": "", + "email": "user1@localhost", + "is_staff": false, + "is_active": false, + "date_joined": "2018-05-13T12:56:54", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 18, + "fields": { + "password": "pbkdf2_sha256$100000$JtoTGhCr6Nza$mBUfLAbJLcF9tAarjrWO0DXDNcP+fB/cb26d34563HA=", + "last_login": null, + "is_superuser": false, + "username": "user2", + "first_name": "", + "last_name": "", + "email": "user2@localhost", + "is_staff": false, + "is_active": false, + "date_joined": "2018-05-13T12:57:11", + "groups": [], + "user_permissions": [] + } }, { - "model": "auth.user", - "pk": 6, - "fields": { - "password": "pbkdf2_sha256$100000$lV52crpDsEzY$ayhyunUhctRHY2JcTiOnGZqT72VzfVk9uqLNdRmKKaA=", - "last_login": null, - "is_superuser": false, - "username": "staff_user2", - "first_name": "", - "last_name": "", - "email": "staff_user2@localhost", - "is_staff": true, - "is_active": true, - "date_joined": "2018-05-11T15:12:20", - "groups": [], - "user_permissions": [] - } + "model": "auth.user", + "pk": 19, + "fields": { + "password": "pbkdf2_sha256$100000$5dkFQELRCIbT$zBuvpIC86ez5AZYRa3Xn3jRShLfTFeyUh0U6HuRvFoI=", + "last_login": null, + "is_superuser": false, + "username": "user3", + "first_name": "", + "last_name": "", + "email": "user3@localhost", + "is_staff": false, + "is_active": false, + "date_joined": "2018-05-13T12:57:29", + "groups": [], + "user_permissions": [] + } }, { "model": "auth_enhanced.userenhancement", @@ -135,24 +369,128 @@ "model": "auth_enhanced.userenhancement", "pk": 4, "fields": { - "email_verification_status": "EMAIL_VERIFICATION_COMPLETED", + "email_verification_status": "EMAIL_VERIFICATION_IN_PROGRESS", "user": 4 } }, { - "model": "auth_enhanced.userenhancement", - "pk": 5, - "fields": { - "email_verification_status": "EMAIL_VERIFICATION_COMPLETED", - "user": 5 - } + "model": "auth_enhanced.userenhancement", + "pk": 5, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_COMPLETED", + "user": 5 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 6, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_FAILED", + "user": 6 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 7, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_IN_PROGRESS", + "user": 7 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 8, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_COMPLETED", + "user": 8 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 9, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_FAILED", + "user": 9 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 10, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_IN_PROGRESS", + "user": 10 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 11, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_COMPLETED", + "user": 11 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 12, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_FAILED", + "user": 12 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 13, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_IN_PROGRESS", + "user": 13 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 14, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_COMPLETED", + "user": 14 + } }, { - "model": "auth_enhanced.userenhancement", - "pk": 6, - "fields": { - "email_verification_status": "EMAIL_VERIFICATION_FAILED", - "user": 6 - } + "model": "auth_enhanced.userenhancement", + "pk": 15, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_FAILED", + "user": 15 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 16, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_IN_PROGRESS", + "user": 16 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 17, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_COMPLETED", + "user": 17 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 18, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_FAILED", + "user": 18 + } +}, +{ + "model": "auth_enhanced.userenhancement", + "pk": 19, + "fields": { + "email_verification_status": "EMAIL_VERIFICATION_IN_PROGRESS", + "user": 19 + } } ] diff --git a/tests/utils/settings_dev.py b/tests/utils/settings_dev.py index 960a4a3..57de3c3 100644 --- a/tests/utils/settings_dev.py +++ b/tests/utils/settings_dev.py @@ -87,5 +87,5 @@ EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' ### CURRENT DEVELOPMENT -DAE_ADMIN_LIST_DISPLAY = ('username_status_color', 'email_with_verification_status') +DAE_ADMIN_LIST_DISPLAY = ('username_status_color', 'email_with_verification_status', 'is_active') DAE_ADMIN_USERNAME_STATUS_COLOR = ('#cc0000', '#00cc00') From 31747975004f756d5b7e2a850fa2cb9041b3b312 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sun, 13 May 2018 21:20:59 +0200 Subject: [PATCH 13/20] Completed tests for 'action_bulk_activate_user' --- auth_enhanced/admin.py | 4 +- tests/test_admin.py | 162 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 160 insertions(+), 6 deletions(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 06d6b4a..83d5fd1 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -7,7 +7,7 @@ from django.contrib.admin.templatetags.admin_list import _boolean_icon from django.contrib.auth import get_user_model from django.contrib.auth.admin import UserAdmin -from django.contrib.messages import ERROR, SUCCESS, WARNING +from django.contrib.messages import ERROR, SUCCESS, WARNING # noqa from django.utils.html import format_html from django.utils.translation import ugettext_lazy as _, ungettext_lazy @@ -152,7 +152,7 @@ def action_bulk_activate_user(self, request, queryset, user_id=None): not_activated.append(getattr(user, user_model.USERNAME_FIELD)) else: user.is_active = True - user.save() + user.save(update_fields=['is_active']) activated.append(getattr(user, user_model.USERNAME_FIELD)) # return messages for successfully activated accounts diff --git a/tests/test_admin.py b/tests/test_admin.py index 5b5b449..6134f83 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -9,12 +9,18 @@ # Django imports from django.conf import settings +from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME from django.contrib.admin.sites import AdminSite from django.contrib.auth import get_user_model +from django.contrib.contenttypes.models import ContentType from django.test import override_settings, tag # noqa +from django.urls import reverse # app imports from auth_enhanced.admin import EnhancedUserAdmin, EnhancedUserStatusFilter +from auth_enhanced.settings import ( + DAE_CONST_MODE_EMAIL_ACTIVATION, DAE_CONST_MODE_MANUAL_ACTIVATION, +) # app imports from .utils.testcases import AuthEnhancedTestCase @@ -198,6 +204,8 @@ class EnhancedUserAdminTests(AuthEnhancedTestCase): def setUp(self): """Per test setup""" + super(EnhancedUserAdminTests, self).setUp() + # create an instance of the admin class self.admin_obj = EnhancedUserAdmin(get_user_model(), AdminSite()) @@ -269,10 +277,156 @@ class EnhancedUserAdminRequestsTests(AuthEnhancedTestCase): fixtures = ['tests/utils/fixtures/test_different_users.json'] def setUp(self): - self.client.force_login( - get_user_model().objects.get(username='superuser') - ) + super(EnhancedUserAdminRequestsTests, self).setUp() + + self.user_model = get_user_model() + self.content_type = ContentType.objects.get_for_model(self.user_model) + self.client.force_login(self.user_model.objects.get(username='superuser')) + + @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_MANUAL_ACTIVATION) def test_action_bulk_activate_single_user(self): """Activate a single user by dropdown.""" - pass + + u = self.user_model.objects.get(username='user1') + + # check, if the user is inactive + self.assertFalse(u.is_active) + + # try to activate a single user + action_data = { + ACTION_CHECKBOX_NAME: [u.pk], + 'action': 'action_bulk_activate_user' + } + response = self.client.post( + reverse('admin:{}_{}_changelist'.format(self.content_type.app_label, self.content_type.model)), + action_data, + follow=True + ) + + # user should now be active + self.assertTrue(self.user_model.objects.get(username='user1').is_active) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + self.assertEqual(str(messages[0]), '1 user was activated successfully (user1).') + + @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_MANUAL_ACTIVATION) + def test_action_bulk_activate_multiple_users(self): + """Activate a multiple users by dropdown.""" + + u = self.user_model.objects.get(username='user1') + v = self.user_model.objects.get(username='user2') + + # check, if the user is inactive + self.assertFalse(u.is_active) + self.assertFalse(v.is_active) + + # try to activate a single user + action_data = { + ACTION_CHECKBOX_NAME: [u.pk, v.pk], + 'action': 'action_bulk_activate_user' + } + response = self.client.post( + reverse('admin:{}_{}_changelist'.format(self.content_type.app_label, self.content_type.model)), + action_data, + follow=True + ) + + # user should now be active + self.assertTrue(self.user_model.objects.get(username='user1').is_active) + self.assertTrue(self.user_model.objects.get(username='user2').is_active) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + # TODO: What if the users are in a different order? + self.assertEqual(str(messages[0]), '2 users were activated successfully (user1, user2).') + + @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_EMAIL_ACTIVATION) + def test_action_bulk_activate_single_user_fail(self): + """Activate a single user by dropdown.""" + + u = self.user_model.objects.get(username='user2') + + # check, if the user is inactive + self.assertFalse(u.is_active) + + # try to activate a single user + action_data = { + ACTION_CHECKBOX_NAME: [u.pk], + 'action': 'action_bulk_activate_user' + } + response = self.client.post( + reverse('admin:{}_{}_changelist'.format(self.content_type.app_label, self.content_type.model)), + action_data, + follow=True + ) + + # user should now be active + self.assertFalse(self.user_model.objects.get(username='user2').is_active) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + self.assertEqual( + str(messages[0]), + "1 user could not be activated, because his email address is not " + "verified (user2)!" + ) + + @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_EMAIL_ACTIVATION) + def test_action_bulk_activate_multiple_users_fail(self): + """Activate a multiple users by dropdown.""" + + u = self.user_model.objects.get(username='user2') + v = self.user_model.objects.get(username='user3') + + # check, if the user is inactive + self.assertFalse(u.is_active) + self.assertFalse(v.is_active) + + # try to activate a single user + action_data = { + ACTION_CHECKBOX_NAME: [u.pk, v.pk], + 'action': 'action_bulk_activate_user' + } + response = self.client.post( + reverse('admin:{}_{}_changelist'.format(self.content_type.app_label, self.content_type.model)), + action_data, + follow=True + ) + + # user should now be active + self.assertFalse(self.user_model.objects.get(username='user2').is_active) + self.assertFalse(self.user_model.objects.get(username='user3').is_active) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + # TODO: What if the users are in a different order? + self.assertEqual( + str(messages[0]), + "2 users could not be activated, because their email addresses are " + "not verified (user2, user3)!" + ) + + @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_EMAIL_ACTIVATION) + def test_action_bulk_activate_invalid(self): + """Activate a multiple users by dropdown.""" + + # try to activate a single user + action_data = { + ACTION_CHECKBOX_NAME: [1338], + 'action': 'action_bulk_activate_user' + } + response = self.client.post( + reverse('admin:{}_{}_changelist'.format(self.content_type.app_label, self.content_type.model)), + action_data, + follow=True + ) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + # TODO: What if the users are in a different order? + self.assertEqual( + str(messages[0]), + 'Nothing was done. Probably this means, that no or invalid user IDs were provided.' + ) From ddeb2b38c7690d4fe8393e32e30abae6fea87596 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sun, 13 May 2018 22:24:20 +0200 Subject: [PATCH 14/20] Implemented 'action_bulk_deactivate_user' --- auth_enhanced/admin.py | 79 +++++++++++++++++++++++++++++- tests/test_admin.py | 107 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 1 deletion(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 83d5fd1..3be5b43 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -118,7 +118,7 @@ class EnhancedUserAdmin(UserAdmin): # statement to the queryset and thus reduce the number of SQL queries. list_select_related = ('enhancement',) - actions = ['action_bulk_activate_user', ] + actions = ['action_bulk_activate_user', 'action_bulk_deactivate_user'] def action_bulk_activate_user(self, request, queryset, user_id=None): """Performs bulk activation of users in Django admin. @@ -200,6 +200,83 @@ def action_bulk_activate_user(self, request, queryset, user_id=None): ) action_bulk_activate_user.short_description = _('Activate selected users') + def action_bulk_deactivate_user(self, request, queryset, user_id=None): + """Performs bulk deactivation of users in Django admin. + + This action is accessible from the drop-down menu and works together + with selecting user objects by checking their respective checkbox. + Furthermore, it handles the actual deactivation of single user aswell, + because ultimatively, this method is used to perform the deactivation + when 'action_deactivate_user()' is called.""" + + deactivated = [] + not_deactivated = [] + user_model = get_user_model() + + # the method is called from 'action_deactivate_user', so a queryset has + # to be constructed... + if user_id and not queryset: + try: + queryset = [user_model.objects.get(pk=user_id)] + except user_model.DoesNotExist: + # provide an empty queryset. This mimics the behaviour of + # Django, if invalid user IDs are provided in the POST-request + queryset = [] + + # at this point, 'queryset' is filled and can be iterated + for user in queryset: + if user == request.user: + not_deactivated.append(getattr(user, user_model.USERNAME_FIELD)) + else: + user.is_active = False + user.save(update_fields=['is_active']) + deactivated.append(getattr(user, user_model.USERNAME_FIELD)) + + # return messages for successfully deactivated accounts + if deactivated: + count = len(deactivated) + self.message_user( + request, + ungettext_lazy( + '%(count)d user was deactivated successfully (%(deactivated_list)s).', + '%(count)d users were deactivated successfully (%(deactivated_list)s).', + count + ) % { + 'count': count, + 'deactivated_list': ', '.join(deactivated), + }, + SUCCESS, + ) + + # return messages for accounts, that could not be deactivated + # This should only return a message, if the superuser tries to + # deactivate his own account + if not_deactivated: + count = len(not_deactivated) + self.message_user( + request, + ungettext_lazy( + "%(count)d user could not be deactivated, because this is " + "your own account (%(deactivated_list)s)!", + "%(count)d users could not be deactivated (%(deactivated_list)s)!", + count + ) % { + 'count': count, + 'deactivated_list': ', '.join(not_deactivated), + }, + ERROR, + ) + + # the method did nothing; this means something unexpected happened, + # i.e. invalid user IDs were provided + if not (deactivated or not_deactivated): + self.message_user( + request, + _('Nothing was done. Probably this means, that no or invalid user IDs were provided.'), + ERROR, + ) + action_bulk_deactivate_user.short_description = _('Deactivate selected users') + def changelist_view(self, request, extra_context=None): """Pass some more context into the view. diff --git a/tests/test_admin.py b/tests/test_admin.py index 6134f83..2c5c2dc 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -430,3 +430,110 @@ def test_action_bulk_activate_invalid(self): str(messages[0]), 'Nothing was done. Probably this means, that no or invalid user IDs were provided.' ) + + def test_action_bulk_deactivate_single_user(self): + """Deactivate a single user by dropdown.""" + + u = self.user_model.objects.get(username='user') + + # check, if the user is inactive + self.assertTrue(u.is_active) + + # try to activate a single user + action_data = { + ACTION_CHECKBOX_NAME: [u.pk], + 'action': 'action_bulk_deactivate_user' + } + response = self.client.post( + reverse('admin:{}_{}_changelist'.format(self.content_type.app_label, self.content_type.model)), + action_data, + follow=True + ) + + # user should now be active + self.assertFalse(self.user_model.objects.get(username='user').is_active) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + self.assertEqual(str(messages[0]), '1 user was deactivated successfully (user).') + + def test_action_bulk_deactivate_multiple_users(self): + """Deactivate a multiple users by dropdown.""" + + u = self.user_model.objects.get(username='user') + v = self.user_model.objects.get(username='user_in_progress') + + # check, if the user is inactive + self.assertTrue(u.is_active) + self.assertTrue(v.is_active) + + # try to activate a single user + action_data = { + ACTION_CHECKBOX_NAME: [u.pk, v.pk], + 'action': 'action_bulk_deactivate_user' + } + response = self.client.post( + reverse('admin:{}_{}_changelist'.format(self.content_type.app_label, self.content_type.model)), + action_data, + follow=True + ) + + # user should now be active + self.assertFalse(self.user_model.objects.get(username='user').is_active) + self.assertFalse(self.user_model.objects.get(username='user_in_progress').is_active) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + # TODO: What if the users are in a different order? + self.assertEqual(str(messages[0]), '2 users were deactivated successfully (user, user_in_progress).') + + def test_action_bulk_deactivate_single_user_fail(self): + """Deactivate a single user by dropdown.""" + + u = self.user_model.objects.get(username='superuser') + + # check, if the user is inactive + self.assertTrue(u.is_active) + + # try to activate a single user + action_data = { + ACTION_CHECKBOX_NAME: [u.pk], + 'action': 'action_bulk_deactivate_user' + } + response = self.client.post( + reverse('admin:{}_{}_changelist'.format(self.content_type.app_label, self.content_type.model)), + action_data, + follow=True + ) + + # user should now be active + self.assertTrue(self.user_model.objects.get(username='superuser').is_active) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + self.assertEqual( + str(messages[0]), + "1 user could not be deactivated, because this is your own account (superuser)!" + ) + + def test_action_bulk_deactivate_invalid(self): + """Activate a multiple users by dropdown.""" + + # try to activate a single user + action_data = { + ACTION_CHECKBOX_NAME: [1338], + 'action': 'action_bulk_deactivate_user' + } + response = self.client.post( + reverse('admin:{}_{}_changelist'.format(self.content_type.app_label, self.content_type.model)), + action_data, + follow=True + ) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + # TODO: What if the users are in a different order? + self.assertEqual( + str(messages[0]), + 'Nothing was done. Probably this means, that no or invalid user IDs were provided.' + ) From b81d4a7e6e9d238029cefcb1a394eae1826478f2 Mon Sep 17 00:00:00 2001 From: Mischback Date: Sun, 13 May 2018 23:56:39 +0200 Subject: [PATCH 15/20] Implemented some utility methods to improve the list-view --- auth_enhanced/admin.py | 96 ++++++++++++++++++++++++++++++- tests/test_admin.py | 111 +++++++++++++++++++++++++++++++++++- tests/utils/settings_dev.py | 4 +- 3 files changed, 206 insertions(+), 5 deletions(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 3be5b43..0759999 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -3,11 +3,15 @@ # Django imports from django.conf import settings +from django.conf.urls import url from django.contrib import admin from django.contrib.admin.templatetags.admin_list import _boolean_icon from django.contrib.auth import get_user_model from django.contrib.auth.admin import UserAdmin -from django.contrib.messages import ERROR, SUCCESS, WARNING # noqa +from django.contrib.contenttypes.models import ContentType +from django.contrib.messages import ERROR, SUCCESS +from django.shortcuts import redirect +from django.urls import reverse from django.utils.html import format_html from django.utils.translation import ugettext_lazy as _, ungettext_lazy @@ -118,8 +122,13 @@ class EnhancedUserAdmin(UserAdmin): # statement to the queryset and thus reduce the number of SQL queries. list_select_related = ('enhancement',) + # 'actions' determines the actions, that are accessible from the dropdown actions = ['action_bulk_activate_user', 'action_bulk_deactivate_user'] + # 'url_helper' is used to construct URLs by decoupling them from the used + # AUTH_USER_MODEL + url_helper = ContentType.objects.get_for_model(get_user_model()) + def action_bulk_activate_user(self, request, queryset, user_id=None): """Performs bulk activation of users in Django admin. @@ -277,6 +286,38 @@ def action_bulk_deactivate_user(self, request, queryset, user_id=None): ) action_bulk_deactivate_user.short_description = _('Deactivate selected users') + def action_activate_user(self, request, user_id, *args, **kwargs): + """This action activates an user-object in Django admin + + This action is accessible as a button per object row and will activate + only that single user. + + TODO: Here, server state is modified by a GET-request. *fubar*""" + + self.action_bulk_activate_user(request, None, user_id=user_id) + + return redirect( + reverse( + 'admin:{}_{}_changelist'.format(self.url_helper.app_label, self.url_helper.model) + ) + ) + + def action_deactivate_user(self, request, user_id, *args, **kwargs): + """This action deactivates an user-object in Django admin + + This action is accessible as a button per object row and will deactivate + only that single user. + + TODO: Here, server state is modified by a GET-request. *fubar*""" + + self.action_bulk_deactivate_user(request, None, user_id=user_id) + + return redirect( + reverse( + 'admin:{}_{}_changelist'.format(self.url_helper.app_label, self.url_helper.model) + ) + ) + def changelist_view(self, request, extra_context=None): """Pass some more context into the view. @@ -338,6 +379,35 @@ def get_additional_legend(self): return result + def get_urls(self): + """Override get_urls()-method to include our custom admin actions and + make them accessible with a single button.""" + + # TODO: Write tests for this override! + custom_urls = [ + url( + r'^(?P.+)/activate/$', + self.admin_site.admin_view(self.action_activate_user), + name='enhanced-activate-user' + ), + url( + r'^(?P.+)/deactivate/$', + self.admin_site.admin_view(self.action_deactivate_user), + name='enhanced-deactivate-user' + ), + ] + + return custom_urls + super(EnhancedUserAdmin, self).get_urls() + + def is_active_with_action(self, user_obj): + """Combines the activation status with the buttons to modify it.""" + + # get the icon (with Django's template tag) + icon = _boolean_icon(user_obj.is_active) + + return format_html('{} {}', icon, self.toggle_is_active(user_obj)) + is_active_with_action.short_description = _('Activation Status') + def status_aggregated(self, user_obj): """Returns the status of an user as string. @@ -395,6 +465,30 @@ def username_status_color(self, user_obj): username_status_color.short_description = _('Username (status)') username_status_color.admin_order_field = '-username' + def toggle_is_active(self, user_obj): + """Shows a button to activate or deactivate an account, depending on + 'is_active' + + TODO: Write tests for this method!""" + + if user_obj.is_active: + # show deactivate button + button = format_html( + 'deactivate'.format( + reverse('admin:enhanced-deactivate-user', args=[user_obj.pk]) + ) + ) + else: + # show activate button + button = format_html( + 'activate'.format( + reverse('admin:enhanced-activate-user', args=[user_obj.pk]) + ) + ) + + return button + toggle_is_active.short_description = _('Modify activation status') + @register_only_debug(UserEnhancement) class UserEnhancementAdmin(admin.ModelAdmin): diff --git a/tests/test_admin.py b/tests/test_admin.py index 2c5c2dc..1abe943 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -11,6 +11,7 @@ from django.conf import settings from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME from django.contrib.admin.sites import AdminSite +from django.contrib.admin.templatetags.admin_list import _boolean_icon from django.contrib.auth import get_user_model from django.contrib.contenttypes.models import ContentType from django.test import override_settings, tag # noqa @@ -196,6 +197,48 @@ def test_username_status_char_superuser(self): '[{}]{}'.format('a', getattr(u, u.USERNAME_FIELD)) ) + def test_toggle_is_active(self): + """Should display an activation button for inactive users and a + deactivation button for active users.""" + + u = get_user_model().objects.get(username='user') + self.assertIn( + 'deactivate', + self.admin_obj.toggle_is_active(u) + ) + + u = get_user_model().objects.get(username='user1') + self.assertIn( + 'activate', + self.admin_obj.toggle_is_active(u) + ) + + def test_is_active_with_action(self): + """Should display a Django icon with the button from 'toggle_is_active'.""" + + u = get_user_model().objects.get(username='user') + retval = self.admin_obj.is_active_with_action(u) + self.assertIn('deactivate', retval) + self.assertIn(_boolean_icon(u.is_active), retval) + + u = get_user_model().objects.get(username='user1') + retval = self.admin_obj.is_active_with_action(u) + self.assertIn('activate', retval) + self.assertIn(_boolean_icon(u.is_active), retval) + + def test_email_with_verification_status(self): + """Should display the user's email address and its verification status.""" + + u = get_user_model().objects.get(username='user') + retval = self.admin_obj.email_with_verification_status(u) + self.assertIn(getattr(u, u.EMAIL_FIELD), retval) + self.assertIn(_boolean_icon(u.enhancement.email_is_verified), retval) + + u = get_user_model().objects.get(username='user_not_verified') + retval = self.admin_obj.email_with_verification_status(u) + self.assertIn(getattr(u, u.EMAIL_FIELD), retval) + self.assertIn(_boolean_icon(u.enhancement.email_is_verified), retval) + @tag('admin') class EnhancedUserAdminTests(AuthEnhancedTestCase): @@ -506,7 +549,7 @@ def test_action_bulk_deactivate_single_user_fail(self): follow=True ) - # user should now be active + # user should still be active self.assertTrue(self.user_model.objects.get(username='superuser').is_active) # message indicates the activation of 1 user @@ -517,7 +560,7 @@ def test_action_bulk_deactivate_single_user_fail(self): ) def test_action_bulk_deactivate_invalid(self): - """Activate a multiple users by dropdown.""" + """Deactivate a multiple users by dropdown.""" # try to activate a single user action_data = { @@ -537,3 +580,67 @@ def test_action_bulk_deactivate_invalid(self): str(messages[0]), 'Nothing was done. Probably this means, that no or invalid user IDs were provided.' ) + + @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_MANUAL_ACTIVATION) + def test_action_activate_user_valid(self): + """Activate a single user by button (pass to bulk method).""" + + u = self.user_model.objects.get(username='user1') + + # check, if the user is inactive + self.assertFalse(u.is_active) + + # activate the user (by 'clicking' the button) + response = self.client.get(reverse('admin:enhanced-activate-user', args=[u.pk])) + + # the user should be active + self.assertTrue(self.user_model.objects.get(username='user1').is_active) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + self.assertEqual(str(messages[0]), '1 user was activated successfully (user1).') + + @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_MANUAL_ACTIVATION) + def test_action_activate_user_invalid(self): + """Invalid user IDs are handled with a simple message.""" + + # spoof an invalid user ID + response = self.client.get(reverse('admin:enhanced-activate-user', args=[1338])) + + # message indicates, that no user got activated + messages = list(response.wsgi_request._messages) + self.assertEqual( + str(messages[0]), + 'Nothing was done. Probably this means, that no or invalid user IDs were provided.' + ) + + def test_action_deactivate_user_valid(self): + """Deactivate a single user by button (pass to bulk method).""" + + u = self.user_model.objects.get(username='user') + + # check, if the user is inactive + self.assertTrue(u.is_active) + + # activate the user (by 'clicking' the button) + response = self.client.get(reverse('admin:enhanced-deactivate-user', args=[u.pk])) + + # the user should be active + self.assertFalse(self.user_model.objects.get(username='user').is_active) + + # message indicates the activation of 1 user + messages = list(response.wsgi_request._messages) + self.assertEqual(str(messages[0]), '1 user was deactivated successfully (user).') + + def test_action_deactivate_user_invalid(self): + """Invalid user IDs are handled with a simple message.""" + + # spoof an invalid user ID + response = self.client.get(reverse('admin:enhanced-deactivate-user', args=[1338])) + + # message indicates, that no user got activated + messages = list(response.wsgi_request._messages) + self.assertEqual( + str(messages[0]), + 'Nothing was done. Probably this means, that no or invalid user IDs were provided.' + ) diff --git a/tests/utils/settings_dev.py b/tests/utils/settings_dev.py index 57de3c3..eed9a25 100644 --- a/tests/utils/settings_dev.py +++ b/tests/utils/settings_dev.py @@ -87,5 +87,5 @@ EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' ### CURRENT DEVELOPMENT -DAE_ADMIN_LIST_DISPLAY = ('username_status_color', 'email_with_verification_status', 'is_active') -DAE_ADMIN_USERNAME_STATUS_COLOR = ('#cc0000', '#00cc00') +# DAE_ADMIN_LIST_DISPLAY = ('username_status_color', 'email_with_verification_status', 'is_active', 'toggle_is_active') +# DAE_ADMIN_USERNAME_STATUS_COLOR = ('#cc0000', '#00cc00') From bcb0e69e1451533aea7f7b9640a9b6ae310742ea Mon Sep 17 00:00:00 2001 From: Mischback Date: Mon, 14 May 2018 00:11:32 +0200 Subject: [PATCH 16/20] Fixing a failing Travis build The fail only happens on Travis, not while running 'tox'. --- auth_enhanced/admin.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 0759999..92c6a3e 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -125,10 +125,6 @@ class EnhancedUserAdmin(UserAdmin): # 'actions' determines the actions, that are accessible from the dropdown actions = ['action_bulk_activate_user', 'action_bulk_deactivate_user'] - # 'url_helper' is used to construct URLs by decoupling them from the used - # AUTH_USER_MODEL - url_helper = ContentType.objects.get_for_model(get_user_model()) - def action_bulk_activate_user(self, request, queryset, user_id=None): """Performs bulk activation of users in Django admin. @@ -294,11 +290,15 @@ def action_activate_user(self, request, user_id, *args, **kwargs): TODO: Here, server state is modified by a GET-request. *fubar*""" + # 'url_helper' is used to construct URLs by decoupling them from the used + # AUTH_USER_MODEL + url_helper = ContentType.objects.get_for_model(get_user_model()) + self.action_bulk_activate_user(request, None, user_id=user_id) return redirect( reverse( - 'admin:{}_{}_changelist'.format(self.url_helper.app_label, self.url_helper.model) + 'admin:{}_{}_changelist'.format(url_helper.app_label, url_helper.model) ) ) @@ -310,11 +310,15 @@ def action_deactivate_user(self, request, user_id, *args, **kwargs): TODO: Here, server state is modified by a GET-request. *fubar*""" + # 'url_helper' is used to construct URLs by decoupling them from the used + # AUTH_USER_MODEL + url_helper = ContentType.objects.get_for_model(get_user_model()) + self.action_bulk_deactivate_user(request, None, user_id=user_id) return redirect( reverse( - 'admin:{}_{}_changelist'.format(self.url_helper.app_label, self.url_helper.model) + 'admin:{}_{}_changelist'.format(url_helper.app_label, url_helper.model) ) ) From f546b64082e5f1458016591a0ce4657f93d2bba1 Mon Sep 17 00:00:00 2001 From: Mischback Date: Wed, 16 May 2018 20:18:07 +0200 Subject: [PATCH 17/20] Solved some TODOs --- Makefile | 5 ++++ auth_enhanced/admin.py | 18 ++++++-------- auth_enhanced/email.py | 6 +---- auth_enhanced/forms.py | 2 -- tests/test_admin.py | 21 ++++++++-------- tests/test_forms.py | 53 ++++++++++++++++++++++------------------- tests/utils/urls_dev.py | 4 +--- 7 files changed, 53 insertions(+), 56 deletions(-) diff --git a/Makefile b/Makefile index df93e71..47763a6 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,7 @@ .SILENT: .PHONY: benchmark clean coverage doc doc-srv flake8 isort isort-full test \ + test-tag test-time todo check createsuperuser django migrate runserver shell @@ -57,6 +58,10 @@ test-tag: test-time: $(MAKE) test test_cmd="--time" +# fetch a list of TODOs +todo: + grep -nr --exclude-dir=".tox" --exclude="Makefile" --color "TODO" * + ##### wrapper for django-admin commands ##### # runs the check framework diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 92c6a3e..11eb4b1 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -88,10 +88,6 @@ class EnhancedUserAdmin(UserAdmin): # This is configurable with an app-specific setting or will be left at # Django's default value: # ('username', 'email', 'first_name', 'last_name', 'is_staff') - # TODO: Django already checks the value of 'list_display', so it might be - # unnecessary to actually check DAE_ADMIN_LIST_DISPLAY within the app. - # However, it should be ensured, that the app's setting only includes - # valid values, which depends on the AUTH_USER_MODEL in use. try: list_display = settings.DAE_ADMIN_LIST_DISPLAY except AttributeError: @@ -288,7 +284,7 @@ def action_activate_user(self, request, user_id, *args, **kwargs): This action is accessible as a button per object row and will activate only that single user. - TODO: Here, server state is modified by a GET-request. *fubar*""" + FIXME: Here, server state is modified by a GET-request. *fubar*""" # 'url_helper' is used to construct URLs by decoupling them from the used # AUTH_USER_MODEL @@ -308,7 +304,7 @@ def action_deactivate_user(self, request, user_id, *args, **kwargs): This action is accessible as a button per object row and will deactivate only that single user. - TODO: Here, server state is modified by a GET-request. *fubar*""" + FIXME: Here, server state is modified by a GET-request. *fubar*""" # 'url_helper' is used to construct URLs by decoupling them from the used # AUTH_USER_MODEL @@ -385,9 +381,11 @@ def get_additional_legend(self): def get_urls(self): """Override get_urls()-method to include our custom admin actions and - make them accessible with a single button.""" + make them accessible with a single button. + + This method is not unittested! But the URLs are used in several other + unittests, so any error in this method would be revealed.""" - # TODO: Write tests for this override! custom_urls = [ url( r'^(?P.+)/activate/$', @@ -471,9 +469,7 @@ def username_status_color(self, user_obj): def toggle_is_active(self, user_obj): """Shows a button to activate or deactivate an account, depending on - 'is_active' - - TODO: Write tests for this method!""" + 'is_active'.""" if user_obj.is_active: # show deactivate button diff --git a/auth_enhanced/email.py b/auth_enhanced/email.py index 41f12e4..6e11410 100644 --- a/auth_enhanced/email.py +++ b/auth_enhanced/email.py @@ -35,7 +35,6 @@ def __init__(self, template_name=None, context=None, **kwargs): super(AuthEnhancedEmail, self).__init__(**kwargs) # check for 'template_name' - # TODO: Is this really a minimum requirement? if not template_name: raise self.AuthEnhancedEmailException(_("A 'template_name' must be provided!")) @@ -82,7 +81,6 @@ def callback_admin_information_new_signup(sender, instance, created, **kwargs): # prepare the recipient list mail_to = [(x[0], x[1]) for x in settings.DAE_ADMIN_SIGNUP_NOTIFICATION if 'mail' in x[2]] - # TODO: prepare the context mail_context = { # TODO: sufficient? 'new_user.username' in templates rely on 'username' # being get_username()... KEEP IT AS ABSTRACT AS POSSIBLE @@ -99,8 +97,6 @@ def callback_admin_information_new_signup(sender, instance, created, **kwargs): } # addes the current operation mode to the context - # TODO: This is not the best solution, but even better than to compare - # to some string in the template if settings.DAE_OPERATION_MODE == DAE_CONST_MODE_AUTO_ACTIVATION: mail_context['mode_auto'] = True elif settings.DAE_OPERATION_MODE == DAE_CONST_MODE_EMAIL_ACTIVATION: @@ -159,7 +155,7 @@ def callback_user_signup_email_verification(sender, instance, created, **kwargs) from_email=settings.DAE_EMAIL_FROM_ADDRESS, subject=mail_subject, template_name='user_email_verification', - to=(instance.email, ) # TODO: don't rely on email! Use EMAIL_FIELD + to=(getattr(instance, instance.EMAIL_FIELD), ) ) # actually send the mail diff --git a/auth_enhanced/forms.py b/auth_enhanced/forms.py index 55722ad..21c1c6c 100644 --- a/auth_enhanced/forms.py +++ b/auth_enhanced/forms.py @@ -50,7 +50,6 @@ def clean_token(self): code='dae_token_expired' ) except EnhancedCrypto.EnhancedCryptoException: - # TODO: provide some meaningful error message here raise ValidationError( _("Your submitted token could not be verified!"), code='dae_token_could_not_be_verified' @@ -77,7 +76,6 @@ def activate_user(self): enhancement = UserEnhancement.objects.get(user=user_to_be_activated) except UserEnhancement.DoesNotExist: # logical database integrity FAILED - # TODO: Should this be raised? Or handle it gracefully by creating an enhancement-object? enhancement = UserEnhancement.objects.create(user=user_to_be_activated) # update the verification status diff --git a/tests/test_admin.py b/tests/test_admin.py index 1abe943..f8a45ce 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -382,8 +382,9 @@ def test_action_bulk_activate_multiple_users(self): # message indicates the activation of 1 user messages = list(response.wsgi_request._messages) - # TODO: What if the users are in a different order? - self.assertEqual(str(messages[0]), '2 users were activated successfully (user1, user2).') + self.assertIn('2 users were activated successfully', str(messages[0])) + self.assertIn('user1', str(messages[0])) + self.assertIn('user2', str(messages[0])) @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_EMAIL_ACTIVATION) def test_action_bulk_activate_single_user_fail(self): @@ -444,12 +445,13 @@ def test_action_bulk_activate_multiple_users_fail(self): # message indicates the activation of 1 user messages = list(response.wsgi_request._messages) - # TODO: What if the users are in a different order? - self.assertEqual( - str(messages[0]), + self.assertIn( "2 users could not be activated, because their email addresses are " - "not verified (user2, user3)!" + "not verified", + str(messages[0]), ) + self.assertIn('user2', str(messages[0])) + self.assertIn('user3', str(messages[0])) @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_EMAIL_ACTIVATION) def test_action_bulk_activate_invalid(self): @@ -468,7 +470,6 @@ def test_action_bulk_activate_invalid(self): # message indicates the activation of 1 user messages = list(response.wsgi_request._messages) - # TODO: What if the users are in a different order? self.assertEqual( str(messages[0]), 'Nothing was done. Probably this means, that no or invalid user IDs were provided.' @@ -527,8 +528,9 @@ def test_action_bulk_deactivate_multiple_users(self): # message indicates the activation of 1 user messages = list(response.wsgi_request._messages) - # TODO: What if the users are in a different order? - self.assertEqual(str(messages[0]), '2 users were deactivated successfully (user, user_in_progress).') + self.assertIn('2 users were deactivated successfully', str(messages[0])) + self.assertIn('user', str(messages[0])) + self.assertIn('user_in_progress', str(messages[0])) def test_action_bulk_deactivate_single_user_fail(self): """Deactivate a single user by dropdown.""" @@ -575,7 +577,6 @@ def test_action_bulk_deactivate_invalid(self): # message indicates the activation of 1 user messages = list(response.wsgi_request._messages) - # TODO: What if the users are in a different order? self.assertEqual( str(messages[0]), 'Nothing was done. Probably this means, that no or invalid user IDs were provided.' diff --git a/tests/test_forms.py b/tests/test_forms.py index bfcaae2..a7a5171 100644 --- a/tests/test_forms.py +++ b/tests/test_forms.py @@ -147,9 +147,12 @@ class SignupFormTests(AuthEnhancedTestCase): """These tests target the SignupForm. SignupForm is derived from Django's 'UserCreationForm' and adds some small - additional functions. + additional functions.""" - TODO: Cache 'get_user_model()' in class variable or something...""" + @classmethod + def setUpClass(cls): + super(SignupFormTests, cls).setUpClass() + cls.user_model = get_user_model() @tag('settings', 'setting_operation_mode') @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_AUTO_ACTIVATION) @@ -159,7 +162,7 @@ def test_exclude_email_field_1(self): See '__init__()'-method.""" form = SignupForm() - self.assertNotIn(get_user_model().EMAIL_FIELD, form.fields) + self.assertNotIn(self.user_model.EMAIL_FIELD, form.fields) @tag('settings', 'setting_operation_mode') @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_MANUAL_ACTIVATION) @@ -169,7 +172,7 @@ def test_exclude_email_field_2(self): See '__init__()'-method.""" form = SignupForm() - self.assertNotIn(get_user_model().EMAIL_FIELD, form.fields) + self.assertNotIn(self.user_model.EMAIL_FIELD, form.fields) @tag('settings', 'setting_operation_mode') @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_EMAIL_ACTIVATION) @@ -179,7 +182,7 @@ def test_include_email_field(self): See '__init__()'-method.""" form = SignupForm() - self.assertIn(get_user_model().EMAIL_FIELD, form.fields) + self.assertIn(self.user_model.EMAIL_FIELD, form.fields) @tag('settings', 'setting_operation_mode') @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_EMAIL_ACTIVATION) @@ -190,7 +193,7 @@ def test_required_email_field_missing(self): form = SignupForm( data={ - get_user_model().USERNAME_FIELD: 'foo', + self.user_model.USERNAME_FIELD: 'foo', 'password1': 'foo', 'password2': 'foo' } @@ -207,10 +210,10 @@ def test_required_email_field_empty(self): form = SignupForm( data={ - get_user_model().USERNAME_FIELD: 'foo', + self.user_model.USERNAME_FIELD: 'foo', 'password1': 'foo', 'password2': 'foo', - get_user_model().EMAIL_FIELD: '' + self.user_model.EMAIL_FIELD: '' } ) self.assertFalse(form.is_valid()) @@ -225,10 +228,10 @@ def test_required_email_field_invalid(self): form = SignupForm( data={ - get_user_model().USERNAME_FIELD: 'foo', + self.user_model.USERNAME_FIELD: 'foo', 'password1': 'foo', 'password2': 'foo', - get_user_model().EMAIL_FIELD: 'foo.localhost' + self.user_model.EMAIL_FIELD: 'foo.localhost' } ) self.assertFalse(form.is_valid()) @@ -246,10 +249,10 @@ def test_required_email_field_valid(self): form = SignupForm( data={ - get_user_model().USERNAME_FIELD: 'foo', + self.user_model.USERNAME_FIELD: 'foo', 'password1': 'foo', 'password2': 'foo', - get_user_model().EMAIL_FIELD: 'foo@localhost' + self.user_model.EMAIL_FIELD: 'foo@localhost' } ) self.assertTrue(form.is_valid()) @@ -261,17 +264,17 @@ def test_unique_email(self): See 'clean()'-method.""" - user = get_user_model().objects.create(**{ # noqa - get_user_model().USERNAME_FIELD: 'django', # noqa - get_user_model().EMAIL_FIELD: 'foo@localhost' # noqa + user = self.user_model.objects.create(**{ # noqa + self.user_model.USERNAME_FIELD: 'django', # noqa + self.user_model.EMAIL_FIELD: 'foo@localhost' # noqa }) # noqa form = SignupForm( data={ - get_user_model().USERNAME_FIELD: 'foo', + self.user_model.USERNAME_FIELD: 'foo', 'password1': 'foo', 'password2': 'foo', - get_user_model().EMAIL_FIELD: 'foo@localhost' + self.user_model.EMAIL_FIELD: 'foo@localhost' } ) self.assertFalse(form.is_valid()) @@ -292,7 +295,7 @@ def test_save_no_commit(self): form = SignupForm( data={ - get_user_model().USERNAME_FIELD: 'foo', + self.user_model.USERNAME_FIELD: 'foo', 'password1': 'foo', 'password2': 'foo' } @@ -302,9 +305,9 @@ def test_save_no_commit(self): user = form.save(commit=False) # noqa # it should not be possible to fetch the user from the DB - with self.assertRaises(get_user_model().DoesNotExist): - db_user = get_user_model().objects.get(**{ # noqa - get_user_model().USERNAME_FIELD: 'foo', # noqa + with self.assertRaises(self.user_model.DoesNotExist): + db_user = self.user_model.objects.get(**{ # noqa + self.user_model.USERNAME_FIELD: 'foo', # noqa }) # noqa @tag('settings', 'setting_operation_mode') @@ -317,7 +320,7 @@ def test_save_is_active_auto_true(self): form = SignupForm( data={ - get_user_model().USERNAME_FIELD: 'foo', + self.user_model.USERNAME_FIELD: 'foo', 'password1': 'foo', 'password2': 'foo' } @@ -337,7 +340,7 @@ def test_save_is_active_auto_false_manual(self): form = SignupForm( data={ - get_user_model().USERNAME_FIELD: 'foo', + self.user_model.USERNAME_FIELD: 'foo', 'password1': 'foo', 'password2': 'foo' } @@ -357,8 +360,8 @@ def test_save_is_active_auto_false_email(self): form = SignupForm( data={ - get_user_model().USERNAME_FIELD: 'foo', - get_user_model().EMAIL_FIELD: 'foo@localhost', + self.user_model.USERNAME_FIELD: 'foo', + self.user_model.EMAIL_FIELD: 'foo@localhost', 'password1': 'foo', 'password2': 'foo' } diff --git a/tests/utils/urls_dev.py b/tests/utils/urls_dev.py index 8d6c681..ea52eb0 100644 --- a/tests/utils/urls_dev.py +++ b/tests/utils/urls_dev.py @@ -2,9 +2,7 @@ """Provides a minimal URL configuration for tests and development. This file mimics a project's URL configuration, in order to run the app during -development and testing. - -TODO: create a 'root-view' under the URL '/', which renders some basic template.""" +development and testing.""" # Django imports from django.conf.urls import include, url From 241117c71dcdb7eb056cc5f48c84107eac697704 Mon Sep 17 00:00:00 2001 From: Mischback Date: Wed, 16 May 2018 23:25:33 +0200 Subject: [PATCH 18/20] Added documentation --- auth_enhanced/admin.py | 3 +++ docs/source/settings.rst | 39 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index 11eb4b1..ca1505c 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -101,6 +101,9 @@ class EnhancedUserAdmin(UserAdmin): # 'search_fields' determines the target fields for the search box # The search box can be disabled with an app-specific setting, accordingly # the 'search_field' will be set to an empty tuple. + # TODO: This is *wrong*. DAE_ADMIN_SHOW_SEARCHBOX should not be a boolean + # value, but a tuple, just like DAE_ADMIN_LIST_DISPLAY + # This is required to actually support custom user models. try: if not settings.DAE_ADMIN_SHOW_SEARCHBOX: search_fields = () diff --git a/docs/source/settings.rst b/docs/source/settings.rst index 70b6b69..38f52cc 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -18,7 +18,28 @@ Available Settings .. glossary:: DAE_ADMIN_LIST_DISPLAY - TODO: document this setting! + Controls the values, that are included in Django's admin backend object + list view. Django's default ``UserAdmin`` implementation uses the + following list, which is considered the *default value*: ``('username', 'email', 'first_name', 'last_name', 'is_staff')``. + + If this setting value is set, it will simply substitute the ``list_display``-attribute + of the admin class. + + If your project uses a custom user model, make sure to include actual + attributes of that model in this list. + + Additionally, the following enhancements are provided by the app's + implementation of the admin class: + + * ``'is_active_with_action'``: Combines the value of ``is_active`` with a button to activate / deactive this user. + * ``'status_aggregated'``: Returns localized strings, describing the status of the user. Possible values: ``'user'``, ``'staff'`` and ``'superuser'``. + * ``'username_status_char'``: Returns the username attribute, prefixed with a configurable character. See :term:`DAE_ADMIN_USERNAME_STATUS_CHAR`. + * ``'username_status_color'``: Returns the username, colored with configurable colors. See :term:`DAE_ADMIN_USERNAME_STATUS_COLOR`. + * ``'toggle_is_active'``: Shows a button to activate / deactivate the user, depending on his activation status (``is_active``). It is recommended to use ``'is_active_with_action'`` instead. + + **Accepted Values:** + + * a tuple containing attributes of the user model as strings, e.g. ``('username', 'email')``. DAE_ADMIN_SHOW_SEARCHBOX TODO: document this setting! @@ -32,10 +53,22 @@ Available Settings * a tuple of the following structure: ``('django', 'django@localhost', ('mail', )),``, where ``'django'`` is a username, ``'django@localhost'`` a valid email address and ``('mail', )`` a tuple of notification methods. As of now, only ``'mail'`` is supported. DAE_ADMIN_USERNAME_STATUS_CHAR - TODO: document this setting! + This setting controls the characters, that are used to prefix usernames + depending on their status, if ``'username_status_char'`` is in + :term:`DAE_ADMIN_LIST_DISPLAY`. + + **Accepted Value:** + + * a tuple containing two single characters, e.g. ``('#', '$')``. ``'#'`` will be used to prefix superusers and ``'$'`` for staff users. DAE_ADMIN_USERNAME_STATUS_COLOR - TODO: document this setting! + This setting controls the color, that is used to color usernames, + depending on their status, if ``'username_status_color'`` is in + :term:`DAE_ADMIN_LIST_DISPLAY`. + + **Accepted Value:** + + * a tuple containing two hexadecimal color codes (with leading ``#``), e.g. ``('#cc0000', '#00cc00')``. Names of superusers will be colored red (``'#cc0000'``), staff users green (``'#00cc00'``). DAE_EMAIL_ADMIN_NOTIFICATION_PREFIX All emails to admins / superusers will have a subject, that is prefixed From 113d87def8d16f64212ef83b15b7109ea45cf537 Mon Sep 17 00:00:00 2001 From: Mischback Date: Thu, 17 May 2018 19:15:55 +0200 Subject: [PATCH 19/20] Changed behaviour of 'search_fields' --- Makefile | 2 +- auth_enhanced/admin.py | 19 +++++++------------ auth_enhanced/checks.py | 4 ++-- docs/source/settings.rst | 21 +++++++++++++++++++-- tests/test_checks.py | 2 ++ 5 files changed, 31 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index 47763a6..99a3719 100644 --- a/Makefile +++ b/Makefile @@ -60,7 +60,7 @@ test-time: # fetch a list of TODOs todo: - grep -nr --exclude-dir=".tox" --exclude="Makefile" --color "TODO" * + git grep -n "TODO" ##### wrapper for django-admin commands ##### diff --git a/auth_enhanced/admin.py b/auth_enhanced/admin.py index ca1505c..0bf12a8 100644 --- a/auth_enhanced/admin.py +++ b/auth_enhanced/admin.py @@ -98,20 +98,15 @@ class EnhancedUserAdmin(UserAdmin): # ('is_staff', 'is_superuser', 'is_active', 'groups') list_filter = (EnhancedUserStatusFilter, 'is_active', 'groups') - # 'search_fields' determines the target fields for the search box - # The search box can be disabled with an app-specific setting, accordingly - # the 'search_field' will be set to an empty tuple. - # TODO: This is *wrong*. DAE_ADMIN_SHOW_SEARCHBOX should not be a boolean - # value, but a tuple, just like DAE_ADMIN_LIST_DISPLAY - # This is required to actually support custom user models. + # 'search_fields' determines the target fields for the search box. + # Django's default admin class provides the following list: + # ('username', 'email', 'first_name', 'last_name') + # Adjust this list, if you are using a custom user model or set it to () + # to deactivate the search box. try: - if not settings.DAE_ADMIN_SHOW_SEARCHBOX: - search_fields = () - setattr(settings, 'DAE_ADMIN_SHOW_SEARCHBOX', False) + search_fields = settings.DAE_ADMIN_SEARCH_FIELDS except AttributeError: - # following line is exactly Django's default. Doesn't need to be set! - # search_fields = ('username', 'email', 'first_name', 'last_name') - setattr(settings, 'DAE_ADMIN_SHOW_SEARCHBOX', True) + pass # 'ordering' controls the default ordering of the list view # Django's default value is just 'username' diff --git a/auth_enhanced/checks.py b/auth_enhanced/checks.py index 2a531fe..208d21e 100644 --- a/auth_enhanced/checks.py +++ b/auth_enhanced/checks.py @@ -258,8 +258,8 @@ def check_settings_values(app_configs, **kwargs): errors.append(E010) # DAE_ADMIN_SHOW_SEARCHBOX - if not isinstance(settings.DAE_ADMIN_SHOW_SEARCHBOX, bool): - errors.append(E011) + # if not isinstance(settings.DAE_ADMIN_SHOW_SEARCHBOX, bool): + # errors.append(E011) # DAE_ADMIN_USERNAME_STATUS_COLOR try: diff --git a/docs/source/settings.rst b/docs/source/settings.rst index 38f52cc..1754887 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -41,8 +41,25 @@ Available Settings * a tuple containing attributes of the user model as strings, e.g. ``('username', 'email')``. - DAE_ADMIN_SHOW_SEARCHBOX - TODO: document this setting! + DAE_ADMIN_SEARCH_FIELDS + Controls the fields, that are considered when using the search box in + Django's admin backend object list view. Django's default ``UserAdmin`` + implementation uses the following list, which is considered the + *default value*: ``('username', 'email', 'first_name', 'last_name')``. + + If this setting value is set, it will simply substitute the ``search_fields``-attribute + of the admin class. + + If your project uses a custom user model, make sure to include actual + attributes of that model in this list. + + Furthermore, if set to an empty tuple, ``()``, the search box will be + disabled. + + **Accepted Values:** + + * a tuple containing attributes of the user model as strings, e.g. ``('username', 'email')``. + * an empty tuple ``()``. This deactivates the search box. DAE_ADMIN_SIGNUP_NOTIFICATION Controls, if admins / superusers will be notified of new signups. diff --git a/tests/test_checks.py b/tests/test_checks.py index 24f9fff..c5abffb 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -175,12 +175,14 @@ def test_e010_invalid(self): errors = check_settings_values(None) self.assertEqual(errors, [E010]) + @skip('currently inactive') @override_settings(DAE_ADMIN_SHOW_SEARCHBOX=True) def test_e011_valid(self): """Check should accept valid values.""" errors = check_settings_values(None) self.assertEqual(errors, []) + @skip('currently inactive') @override_settings(DAE_ADMIN_SHOW_SEARCHBOX='foo') def test_e011_invalid(self): """Invalid values show an error message.""" From d5bc28910644c3e9f35a058b2716c4af60279f4d Mon Sep 17 00:00:00 2001 From: Mischback Date: Thu, 17 May 2018 20:05:28 +0200 Subject: [PATCH 20/20] Fixing a failing Travis build Actually there was a bug in an email sending callback, that would have tried to send emails as admin notifications, even if the setting said, that admins should not be notified. I don't really get, why tox didn't reveal this and why this code only fails in Travis 2.7 environment... But better late than never... --- auth_enhanced/email.py | 2 +- tests/test_email.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/auth_enhanced/email.py b/auth_enhanced/email.py index 6e11410..5b8b5b8 100644 --- a/auth_enhanced/email.py +++ b/auth_enhanced/email.py @@ -71,7 +71,7 @@ def callback_admin_information_new_signup(sender, instance, created, **kwargs): This function acts like a callback to a 'post_save'-signal.""" # only send email on new registration - if created: + if created and settings.DAE_ADMIN_SIGNUP_NOTIFICATION: # set the email subject mail_subject = _('New Signup Notification') diff --git a/tests/test_email.py b/tests/test_email.py index 673f903..74b975e 100644 --- a/tests/test_email.py +++ b/tests/test_email.py @@ -122,6 +122,22 @@ def test_callback_subject_prefix(self): settings.DAE_EMAIL_ADMIN_NOTIFICATION_PREFIX )) + @override_settings(DAE_ADMIN_SIGNUP_NOTIFICATION=False) + def test_callback_notification_false(self): + """Recipient list can not be prepared if DAE_ADMIN_SIGNUP_NOTIFICATION is False. + + See 'callback_admin_information_new_signup()'-function.""" + + # create a User object to pass along + u = get_user_model().objects.create(username='foo') + + retval = callback_admin_information_new_signup( + get_user_model(), + u, + True + ) + self.assertFalse(retval) + @override_settings(DAE_OPERATION_MODE=DAE_CONST_MODE_AUTO_ACTIVATION) def test_callback_apply_mode_auto(self): """Is 'context['mode_auto']' set?