mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-09 03:36:48 +00:00
perf(admin): the Users page tells who can clone in one query
Each row of the Users page asked has_perm whether its user may clone: two queries a user (the user's permissions and the groups'). The list's query now annotates it with one Exists over the clone permission, given to the user or to a group of the user; a blocked user is shown without it, as has_perm did. The page's query count no longer grows with the users.
This commit is contained in:
parent
40087143a1
commit
2484d4efb3
3 changed files with 38 additions and 8 deletions
|
|
@ -1,7 +1,6 @@
|
|||
{% extends "base.html" %}
|
||||
{% load i18n %}
|
||||
{% load static %}
|
||||
{% load common_tags %}
|
||||
{% load bootstrap_icons %}
|
||||
|
||||
{% block title %}{{ title }}{% endblock %}
|
||||
|
|
@ -22,7 +21,6 @@
|
|||
</div>
|
||||
<div class="list-group list-group-flush searchable">
|
||||
{% for user in users %}
|
||||
{% has_perm user 'instances.clone_instances' as can_clone %}
|
||||
<div class="list-group-item py-3" data-search="{{ user.username }} {{ user.email }}{% for group in user.groups.all %} {{ group.name }}{% endfor %} {% if user.is_superuser %}{% trans "Superuser" %}{% elif user.is_staff %}{% trans "Template editor" %}{% else %}{% trans "User" %}{% endif %} {% if user.is_active %}{% trans "Active" %}{% else %}{% trans "Blocked" %}{% endif %}">
|
||||
<div class="row g-2 align-items-center">
|
||||
<div class="col-12 col-md-4">
|
||||
|
|
@ -46,11 +44,11 @@
|
|||
<div class="d-flex flex-wrap align-items-center gap-2">
|
||||
{% for group in user.groups.all %}<span class="badge text-bg-light border mw-100 text-wrap text-break">{{ group.name }}</span>{% endfor %}
|
||||
{# a superuser has every permission: only a granted clone permission is news #}
|
||||
{% if can_clone and not user.is_superuser %}<span>{% trans "Can clone" %}</span>{% endif %}
|
||||
{% if user.is_active and user.can_clone and not user.is_superuser %}<span>{% trans "Can clone" %}</span>{% endif %}
|
||||
{% if user.api_tokens %}
|
||||
<a href="{% url 'accounts:account' user.id %}#apitokens">{% blocktrans count count=user.api_tokens %}{{ count }} API token{% plural %}{{ count }} API tokens{% endblocktrans %}</a>
|
||||
{% endif %}
|
||||
{% if not user.groups.all and not user.api_tokens %}{% if user.is_superuser or not can_clone %}
|
||||
{% if not user.groups.all and not user.api_tokens %}{% if user.is_superuser or not user.is_active or not user.can_clone %}
|
||||
<span class="text-body-secondary">{% trans "No groups or API tokens" %}</span>
|
||||
{% endif %}{% endif %}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -1,7 +1,10 @@
|
|||
from django.contrib.auth.models import Group, User
|
||||
from django.contrib.auth.models import Group, Permission, User
|
||||
from django.core.exceptions import ObjectDoesNotExist
|
||||
from django.db import connection
|
||||
from django.shortcuts import reverse
|
||||
from django.test import RequestFactory, TestCase
|
||||
from django.test.utils import CaptureQueriesContext
|
||||
from lxml import html
|
||||
|
||||
from accounts.models import UserAttributes
|
||||
from logs.models import Logs
|
||||
|
|
@ -54,6 +57,29 @@ class AdminTestCase(TestCase):
|
|||
response = self.client.get(reverse("admin:user_list"))
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
def test_user_list_shows_who_can_clone_in_queries_independent_of_the_users(self):
|
||||
clone = Permission.objects.get(content_type__app_label="instances", codename="clone_instances")
|
||||
User.objects.create_user("direct").user_permissions.add(clone)
|
||||
group = Group.objects.create(name="Cloners")
|
||||
group.permissions.add(clone)
|
||||
User.objects.create_user("by-group").groups.add(group)
|
||||
blocked = User.objects.create_user("blocked", is_active=False)
|
||||
blocked.user_permissions.add(clone)
|
||||
User.objects.create_user("plain")
|
||||
|
||||
def rows():
|
||||
with CaptureQueriesContext(connection) as queries:
|
||||
page = html.fromstring(self.client.get(reverse("admin:user_list")).content)
|
||||
return {item.xpath(".//a[contains(@class, 'fw-semibold')]")[0].text: "Can clone" in item.text_content()
|
||||
for item in page.xpath("//div[contains(@class, 'list-group-item')]")}, len(queries)
|
||||
|
||||
shown, count = rows()
|
||||
self.assertEqual({name: shown[name] for name in ("direct", "by-group", "blocked", "plain", "admin")},
|
||||
{"direct": True, "by-group": True, "blocked": False, "plain": False, "admin": False})
|
||||
for i in range(5):
|
||||
User.objects.create_user(f"more-{i}").user_permissions.add(clone)
|
||||
self.assertEqual(rows()[1], count)
|
||||
|
||||
def test_users(self):
|
||||
response = self.client.get(reverse("admin:user_create"))
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
|
|
|||
|
|
@ -5,11 +5,11 @@ from appsettings.settings import app_settings
|
|||
from django.conf import settings
|
||||
from django.contrib import messages
|
||||
from django.contrib.auth import update_session_auth_hash
|
||||
from django.contrib.auth.models import Group, User
|
||||
from django.contrib.auth.models import Group, Permission, User
|
||||
from django.core.paginator import Paginator
|
||||
from django.http import HttpResponse
|
||||
from django.db import transaction
|
||||
from django.db.models import Count, Q
|
||||
from django.db.models import Count, Exists, OuterRef, Q
|
||||
from django.shortcuts import get_object_or_404, redirect, render
|
||||
from django.utils import timezone
|
||||
from django.utils.http import url_has_allowed_host_and_scheme, urlencode
|
||||
|
|
@ -107,7 +107,13 @@ OWN_ACCOUNT = _("You cannot block, delete or demote your own account")
|
|||
def user_list(request):
|
||||
# API tokens that still work: an expired one is deleted only at the user's next create
|
||||
live = Q(auth_token_set__expiry__isnull=True) | Q(auth_token_set__expiry__gt=timezone.now())
|
||||
users = User.objects.annotate(api_tokens=Count("auth_token_set", filter=live)).prefetch_related("groups")
|
||||
# the clone permission, given to the user or to a group of the user: one
|
||||
# query for the list, not a has_perm (two queries) a user
|
||||
clone = Permission.objects.filter(content_type__app_label="instances", codename="clone_instances")
|
||||
users = User.objects.annotate(
|
||||
api_tokens=Count("auth_token_set", filter=live),
|
||||
can_clone=Exists(clone.filter(Q(user=OuterRef("pk")) | Q(group__user=OuterRef("pk")))),
|
||||
).prefetch_related("groups")
|
||||
return render(
|
||||
request,
|
||||
"admin/user_list.html",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue