Possibility to delete a user #2371

As mentioned in the comments I didn't see the first pull request (https://github.com/torchbox/wagtail/pull/2509)
However, I think my changes were a tiny bit more complete in terms of UI/UX. I allow to delete a user directly from the user list + you can delete any user if you are superuser, except yourself. This way we are sure to keep at least one superuser but we can still delete superusers.
I added some tests from this PR to my code and also added the permission denied on the delete page.

CHANGELOG.txt

@@ -21,6 +21,7 @@ Changelog
  * Added formal APIs for customising the display of StructBlock forms within the page editor (Matt Westcott)
  * `wagtailforms.models.AbstractEmailForm` now supports multiple email recipients (Serafeim Papastefanos)
  * Added the ``include_block`` template tag for improved StreamField template inclusion (Matt Westcott)
+ * Added ability to delete users through Settings -> Users (Vincent Audebert; thanks also to Ludolf Takens and Tobias Schmidt for alternative implementations)
  * Fix: Email templates and document uploader now support custom `STATICFILES_STORAGE` (Jonny Scholes)
  * Fix: Removed alignment options (deprecated in HTML and not rendered by Wagtail) from `TableBlock` context menu (Moritz Pfeiffer)
  * Fix: Fixed incorrect CSS path on ModelAdmin's "choose a parent page" view

CONTRIBUTORS.rst

@@ -152,6 +152,7 @@ Contributors
 * Raphael Stolt
 * Tim Graham
 * Thibaud Colas
+* Tobias Schmidt
 
 Translators
 ===========

docs/releases/1.6.rst

@@ -36,6 +36,7 @@ Minor features
  * Added formal APIs for customising the display of StructBlock forms within the page editor - see :ref:`custom_editing_interfaces_for_structblock` (Matt Westcott)
  * ``wagtailforms.models.AbstractEmailForm`` now supports multiple email recipients (Serafeim Papastefanos)
  * Added the ``include_block`` template tag for improved StreamField template inclusion. See :doc:`/topics/streamfield` for documentation (Matt Westcott)
+ * Added ability to delete users through Settings -> Users (Vincent Audebert; thanks also to Ludolf Takens and Tobias Schmidt for alternative implementations)
 
 
 Bug fixes

wagtail/wagtailusers/templates/wagtailusers/users/confirm_delete.html

@@ -0,0 +1,16 @@
+{% extends "wagtailadmin/base.html" %}
+{% load i18n %}
+{% block titletag %}{% trans "Delete user" %}{% endblock %}
+
+{% block content %}
+    {% trans "Delete user" as del_str %}
+    {% include "wagtailadmin/shared/header.html" with title=del_str subtitle=user icon="user" %}
+
+    <div class="nice-padding">
+        <p>{% trans "Are you sure you want to delete this user?" %}</p>
+        <form action="{% url 'wagtailusers_users:delete' user.pk %}" method="POST">
+            {% csrf_token %}
+            <input type="submit" value="{% trans 'Yes, delete' %}" class="button serious" />
+        </form>
+    </div>
+{% endblock %}

wagtail/wagtailusers/templates/wagtailusers/users/edit.html

@@ -30,14 +30,24 @@
                         {% include "wagtailadmin/shared/field_as_li.html" with field=form.password2 %}
                         {% include "wagtailadmin/shared/field_as_li.html" with field=form.is_active %}
                     {% endblock fields %}
-                    <li><input type="submit" value="{% trans 'Save' %}" class="button" /></li>
+                    <li>
+                        <input type="submit" value="{% trans 'Save' %}" class="button" />
+                        {% if can_delete %}
+                            <a href="{% url 'wagtailusers_users:delete' user.pk %}" class="button button-secondary no">{% trans "Delete user" %}</a>
+                        {% endif %}
+                    </li>
                 </ul>
             </section>
             <section id="roles" class="nice-padding">
                 <ul class="fields">
                     {% include "wagtailadmin/shared/field_as_li.html" with field=form.is_superuser %}
                     {% include "wagtailadmin/shared/field_as_li.html" with field=form.groups %}
-                    <li><input type="submit" value="{% trans 'Save' %}" class="button" /></li>
+                    <li>
+                        <input type="submit" value="{% trans 'Save' %}" class="button" />
+                        {% if can_delete %}
+                            <a href="{% url 'wagtailusers_users:delete' user.pk %}" class="button button-secondary no">{% trans "Delete user" %}</a>
+                        {% endif %}
+                    </li>
                 </ul>
             </section>
         </div>

wagtail/wagtailusers/templates/wagtailusers/users/list.html

@@ -1,4 +1,4 @@
-{% load i18n %}
+{% load i18n wagtailusers_tags %}
 {% load gravatar %}
 <table class="listing">
     <thead>
@@ -26,15 +26,18 @@
     <tbody>
         {% for user in users %}
             <tr>
-                <td class="title">
+                <td class="title" valign="top">
                     <h2>
                         <span class="avatar small icon icon-user"><img src="{% gravatar_url user.email 25 %}" /></span>
                         <a href="{% url 'wagtailusers_users:edit' user.pk %}">{{ user.get_full_name|default:user.get_username }}</a>
                     </h2>
+                    <ul class="actions">
+                        {% user_listing_buttons user %}
+                    </ul>
                 </td>
-                <td class="username">{{ user.get_username }}</td>
-                <td class="level">{% if user.is_superuser %}{% trans "Admin" %}{% endif %}</td>
-                <td class="status"><div class="status-tag {% if user.is_active %}primary{% endif %}">{% if user.is_active %}{% trans "Active" %}{% else %}{% trans "Inactive" %}{% endif %}</div></td>
+                <td class="username" valign="top">{{ user.get_username }}</td>
+                <td class="level" valign="top">{% if user.is_superuser %}{% trans "Admin" %}{% endif %}</td>
+                <td class="status" valign="top"><div class="status-tag {% if user.is_active %}primary{% endif %}">{% if user.is_active %}{% trans "Active" %}{% else %}{% trans "Inactive" %}{% endif %}</div></td>
             </tr>
         {% endfor %}
     </tbody>

wagtail/wagtailusers/templatetags/wagtailusers_tags.py

@@ -1,6 +1,9 @@
 from __future__ import absolute_import, unicode_literals
 
+import itertools
+
 from django import template
+from wagtail.wagtailcore import hooks
 
 register = template.Library()
 
@@ -55,3 +58,13 @@ def format_permissions(permission_bound_field):
         'object_perms': object_perms,
         'other_perms': other_perms,
     }
+
+
+@register.inclusion_tag("wagtailadmin/pages/listing/_buttons.html",
+                        takes_context=True)
+def user_listing_buttons(context, user):
+    button_hooks = hooks.get_hooks('register_user_listing_buttons')
+    buttons = sorted(itertools.chain.from_iterable(
+        hook(context, user)
+        for hook in button_hooks))
+    return {'user': user, 'buttons': buttons}

wagtail/wagtailusers/tests.py

@@ -10,6 +10,7 @@ from django.utils import six
 
 from wagtail.tests.utils import WagtailTestUtils
 from wagtail.wagtailcore import hooks
+from wagtail.wagtailcore.compat import AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME
 from wagtail.wagtailcore.models import (
     Collection, GroupCollectionPermission, GroupPagePermission, Page)
 from wagtail.wagtailusers.forms import UserCreationForm, UserEditForm
@@ -17,6 +18,9 @@ from wagtail.wagtailusers.models import UserProfile
 from wagtail.wagtailusers.views.users import get_user_creation_form, get_user_edit_form
 
 
+delete_user_perm_codename = "delete_{0}".format(AUTH_USER_MODEL_NAME.lower())
+
+
 class CustomUserCreationForm(UserCreationForm):
     country = forms.CharField(required=True, label="Country")
 
@@ -179,6 +183,122 @@ class TestUserCreateView(TestCase, WagtailTestUtils):
         self.assertEqual(users.count(), 0)
 
 
+class TestUserDeleteView(TestCase, WagtailTestUtils):
+    def setUp(self):
+        # create a user that should be visible in the listing
+        self.test_user = get_user_model().objects.create_user(
+            username='testuser',
+            email='testuser@email.com',
+            password='password'
+        )
+        # also create a superuser to delete
+        self.superuser = get_user_model().objects.create_superuser(
+            username='testsuperuser',
+            email='testsuperuser@email.com',
+            password='password'
+        )
+        self.current_user = self.login()
+
+    def get(self, params={}):
+        return self.client.get(reverse('wagtailusers_users:delete', args=(self.test_user.pk,)), params)
+
+    def post(self, post_data={}):
+        return self.client.post(reverse('wagtailusers_users:delete', args=(self.test_user.pk,)), post_data)
+
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailusers/users/confirm_delete.html')
+
+    def test_delete(self):
+        response = self.post()
+
+        # Should redirect back to index
+        self.assertRedirects(response, reverse('wagtailusers_users:index'))
+
+        # Check that the user was deleted
+        users = get_user_model().objects.filter(username='testuser')
+        self.assertEqual(users.count(), 0)
+
+    def test_user_cannot_delete_self(self):
+        response = self.client.get(reverse('wagtailusers_users:delete', args=(self.current_user.pk,)))
+
+        # Should redirect to admin index (permission denied)
+        self.assertRedirects(response, reverse('wagtailadmin_home'))
+        # Check user was not deleted
+        self.assertTrue(get_user_model().objects.filter(pk=self.current_user.pk).exists())
+
+    def test_user_can_delete_other_superuser(self):
+        response = self.client.get(reverse('wagtailusers_users:delete', args=(self.superuser.pk,)))
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailusers/users/confirm_delete.html')
+
+        response = self.client.post(reverse('wagtailusers_users:delete', args=(self.superuser.pk,)))
+        # Should redirect back to index
+        self.assertRedirects(response, reverse('wagtailusers_users:index'))
+
+        # Check that the user was deleted
+        users = get_user_model().objects.filter(username='testsuperuser')
+        self.assertEqual(users.count(), 0)
+
+
+class TestUserDeleteViewForNonSuperuser(TestCase, WagtailTestUtils):
+    def setUp(self):
+        # create a user that should be visible in the listing
+        self.test_user = get_user_model().objects.create_user(
+            username='testuser',
+            email='testuser@email.com',
+            password='password'
+        )
+        # create a user with delete permission
+        self.deleter_user = get_user_model().objects.create_user(
+            username='deleter',
+            email='deleter@email.com',
+            password='password'
+        )
+        deleters_group = Group.objects.create(name='User deleters')
+        deleters_group.permissions.add(Permission.objects.get(codename='access_admin'))
+        deleters_group.permissions.add(Permission.objects.get(
+            content_type__app_label=AUTH_USER_APP_LABEL, codename=delete_user_perm_codename
+        ))
+        self.deleter_user.groups.add(deleters_group)
+
+        self.superuser = self.create_test_user()
+
+        self.client.login(username='deleter', password='password')
+
+    def test_simple(self):
+        response = self.client.get(reverse('wagtailusers_users:delete', args=(self.test_user.pk,)))
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailusers/users/confirm_delete.html')
+
+    def test_delete(self):
+        response = self.client.post(reverse('wagtailusers_users:delete', args=(self.test_user.pk,)))
+
+        # Should redirect back to index
+        self.assertRedirects(response, reverse('wagtailusers_users:index'))
+
+        # Check that the user was deleted
+        users = get_user_model().objects.filter(username='testuser')
+        self.assertEqual(users.count(), 0)
+
+    def test_user_cannot_delete_self(self):
+        response = self.client.post(reverse('wagtailusers_users:delete', args=(self.deleter_user.pk,)))
+
+        # Should redirect to admin index (permission denied)
+        self.assertRedirects(response, reverse('wagtailadmin_home'))
+        # Check user was not deleted
+        self.assertTrue(get_user_model().objects.filter(pk=self.deleter_user.pk).exists())
+
+    def test_user_cannot_delete_superuser(self):
+        response = self.client.post(reverse('wagtailusers_users:delete', args=(self.superuser.pk,)))
+
+        # Should redirect to admin index (permission denied)
+        self.assertRedirects(response, reverse('wagtailadmin_home'))
+        # Check user was not deleted
+        self.assertTrue(get_user_model().objects.filter(pk=self.superuser.pk).exists())
+
+
 class TestUserEditView(TestCase, WagtailTestUtils):
     def setUp(self):
         # Create a user to edit

wagtail/wagtailusers/urls/users.py

@@ -8,4 +8,5 @@ urlpatterns = [
     url(r'^$', users.index, name='index'),
     url(r'^add/$', users.create, name='add'),
     url(r'^([^\/]+)/$', users.edit, name='edit'),
+    url(r'^([^\/]+)/delete/$', users.delete, name='delete'),
 ]

wagtail/wagtailusers/utils.py

@@ -0,0 +1,21 @@
+from __future__ import absolute_import, unicode_literals
+
+from wagtail.wagtailcore.compat import AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME
+
+
+delete_user_perm = "{0}.delete_{1}".format(AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME.lower())
+
+
+def user_can_delete_user(current_user, user_to_delete):
+    if not current_user.has_perm(delete_user_perm):
+        return False
+
+    if current_user == user_to_delete:
+        # users may not delete themselves
+        return False
+
+    if user_to_delete.is_superuser and not current_user.is_superuser:
+        # ordinary users may not delete superusers
+        return False
+
+    return True

wagtail/wagtailusers/views/users.py

@@ -13,9 +13,11 @@ from django.views.decorators.vary import vary_on_headers
 from wagtail.utils.pagination import paginate
 from wagtail.wagtailadmin import messages
 from wagtail.wagtailadmin.forms import SearchForm
-from wagtail.wagtailadmin.utils import any_permission_required, permission_required
+from wagtail.wagtailadmin.utils import (
+    any_permission_required, permission_denied, permission_required)
 from wagtail.wagtailcore.compat import AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME
 from wagtail.wagtailusers.forms import UserCreationForm, UserEditForm
+from wagtail.wagtailusers.utils import user_can_delete_user
 
 User = get_user_model()
 
@@ -140,6 +142,8 @@ def create(request):
 @permission_required(change_user_perm)
 def edit(request, user_id):
     user = get_object_or_404(User, pk=user_id)
+    can_delete = user_can_delete_user(request.user, user)
+
     if request.method == 'POST':
         form = get_user_edit_form()(request.POST, instance=user)
         if form.is_valid():
@@ -156,4 +160,22 @@ def edit(request, user_id):
     return render(request, 'wagtailusers/users/edit.html', {
         'user': user,
         'form': form,
+        'can_delete': can_delete,
+    })
+
+
+@permission_required(delete_user_perm)
+def delete(request, user_id):
+    user = get_object_or_404(User, pk=user_id)
+
+    if not user_can_delete_user(request.user, user):
+        return permission_denied(request)
+
+    if request.method == 'POST':
+        user.delete()
+        messages.success(request, _("User '{0}' deleted.").format(user))
+        return redirect('wagtailusers_users:index')
+
+    return render(request, "wagtailusers/users/confirm_delete.html", {
+        'user': user,
     })

wagtail/wagtailusers/wagtail_hooks.py

@@ -3,6 +3,7 @@ from __future__ import absolute_import, unicode_literals
 from django.conf.urls import include, url
 from django.contrib.auth.models import Permission
 from django.core import urlresolvers
+from django.core.urlresolvers import reverse
 from django.db.models import Q
 from django.utils.translation import ugettext_lazy as _
 
@@ -11,6 +12,8 @@ from wagtail.wagtailadmin.search import SearchArea
 from wagtail.wagtailcore import hooks
 from wagtail.wagtailcore.compat import AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME
 from wagtail.wagtailusers.urls import groups, users
+from wagtail.wagtailusers.utils import user_can_delete_user
+from wagtail.wagtailusers.widgets import UserListingButton
 
 
 @hooks.register('register_admin_urls')
@@ -95,3 +98,10 @@ def register_users_search_area():
         name='users',
         classnames='icon icon-user',
         order=600)
+
+
+@hooks.register('register_user_listing_buttons')
+def user_listing_buttons(context, user):
+    yield UserListingButton(_('Edit'), reverse('wagtailusers_users:edit', args=[user.pk]), attrs={'title': _('Edit this user')}, priority=10)
+    if user_can_delete_user(context.request.user, user):
+        yield UserListingButton(_('Delete'), reverse('wagtailusers_users:delete', args=[user.pk]), classes={'no'}, attrs={'title': _('Delete this user')}, priority=20)

wagtail/wagtailusers/widgets.py

@@ -0,0 +1,9 @@
+from __future__ import absolute_import, unicode_literals
+
+from wagtail.wagtailadmin.widgets import Button
+
+
+class UserListingButton(Button):
+    def __init__(self, label, url, classes=set(), **kwargs):
+        classes = {'button', 'button-small', 'button-secondary'} | set(classes)
+        super(UserListingButton, self).__init__(label, url, classes=classes, **kwargs)