4 Commits

Author SHA1 Message Date
imperosol
25cd877160 fix: Counter.edit_groups 2025-09-13 11:39:53 +02:00
thomas girod
b767079c5a Merge pull request #1167 from ae-utbm/page-n+1
Page n+1
2025-09-08 11:28:55 +02:00
imperosol
37961e437b fix: N+1 queries on PageListView 2025-09-04 17:39:17 +02:00
imperosol
b97a1a2e56 improve User.can_view and User.can_edit 2025-09-04 17:38:58 +02:00
14 changed files with 61 additions and 186 deletions

View File

@@ -34,12 +34,10 @@ def migrate_meta_groups(apps: StateApps, schema_editor):
clubs = list(Club.objects.all()) clubs = list(Club.objects.all())
for club in clubs: for club in clubs:
club.board_group = meta_groups.get_or_create( club.board_group = meta_groups.get_or_create(
name=club.unix_name + settings.SITH_BOARD_SUFFIX, name=f"{club.unix_name}-bureau", defaults={"is_meta": True}
defaults={"is_meta": True},
)[0] )[0]
club.members_group = meta_groups.get_or_create( club.members_group = meta_groups.get_or_create(
name=club.unix_name + settings.SITH_MEMBER_SUFFIX, name=f"{club.unix_name}-membres", defaults={"is_meta": True}
defaults={"is_meta": True},
)[0] )[0]
club.save() club.save()
club.refresh_from_db() club.refresh_from_db()

View File

@@ -25,7 +25,6 @@ from core.schemas import (
UserFamilySchema, UserFamilySchema,
UserFilterSchema, UserFilterSchema,
UserProfileSchema, UserProfileSchema,
UserSchema,
) )
from core.templatetags.renderer import markdown from core.templatetags.renderer import markdown
@@ -70,22 +69,16 @@ class MailingListController(ControllerBase):
return data return data
@api_controller("/user") @api_controller("/user", permissions=[CanAccessLookup])
class UserController(ControllerBase): class UserController(ControllerBase):
@route.get("", response=list[UserProfileSchema], permissions=[CanAccessLookup]) @route.get("", response=list[UserProfileSchema])
def fetch_profiles(self, pks: Query[set[int]]): def fetch_profiles(self, pks: Query[set[int]]):
return User.objects.filter(pk__in=pks) return User.objects.filter(pk__in=pks)
@route.get("/{int:user_id}", response=UserSchema, permissions=[CanView])
def fetch_user(self, user_id: int):
"""Fetch a single user"""
return self.get_object_or_exception(User, id=user_id)
@route.get( @route.get(
"/search", "/search",
response=PaginatedResponseSchema[UserProfileSchema], response=PaginatedResponseSchema[UserProfileSchema],
url_name="search_users", url_name="search_users",
permissions=[CanAccessLookup],
) )
@paginate(PageNumberPaginationExtra, page_size=20) @paginate(PageNumberPaginationExtra, page_size=20)
def search_users(self, filters: Query[UserFilterSchema]): def search_users(self, filters: Query[UserFilterSchema]):

View File

@@ -94,11 +94,7 @@ class Command(BaseCommand):
username=self.faker.user_name(), username=self.faker.user_name(),
first_name=self.faker.first_name(), first_name=self.faker.first_name(),
last_name=self.faker.last_name(), last_name=self.faker.last_name(),
date_of_birth=( date_of_birth=self.faker.date_of_birth(minimum_age=15, maximum_age=25),
None
if random.random() < 0.2
else self.faker.date_of_birth(minimum_age=15, maximum_age=25)
),
email=self.faker.email(), email=self.faker.email(),
phone=self.faker.phone_number(), phone=self.faker.phone_number(),
address=self.faker.address(), address=self.faker.address(),

View File

@@ -560,7 +560,7 @@ class User(AbstractUser):
"""Determine if the object is owned by the user.""" """Determine if the object is owned by the user."""
if hasattr(obj, "is_owned_by") and obj.is_owned_by(self): if hasattr(obj, "is_owned_by") and obj.is_owned_by(self):
return True return True
if hasattr(obj, "owner_group") and self.is_in_group(pk=obj.owner_group.id): if hasattr(obj, "owner_group") and self.is_in_group(pk=obj.owner_group_id):
return True return True
return self.is_root return self.is_root
@@ -569,9 +569,15 @@ class User(AbstractUser):
if hasattr(obj, "can_be_edited_by") and obj.can_be_edited_by(self): if hasattr(obj, "can_be_edited_by") and obj.can_be_edited_by(self):
return True return True
if hasattr(obj, "edit_groups"): if hasattr(obj, "edit_groups"):
for pk in obj.edit_groups.values_list("pk", flat=True): if (
if self.is_in_group(pk=pk): hasattr(obj, "_prefetched_objects_cache")
return True and "edit_groups" in obj._prefetched_objects_cache
):
pks = [g.id for g in obj.edit_groups.all()]
else:
pks = list(obj.edit_groups.values_list("id", flat=True))
if any(self.is_in_group(pk=pk) for pk in pks):
return True
if isinstance(obj, User) and obj == self: if isinstance(obj, User) and obj == self:
return True return True
return self.is_owner(obj) return self.is_owner(obj)
@@ -581,9 +587,18 @@ class User(AbstractUser):
if hasattr(obj, "can_be_viewed_by") and obj.can_be_viewed_by(self): if hasattr(obj, "can_be_viewed_by") and obj.can_be_viewed_by(self):
return True return True
if hasattr(obj, "view_groups"): if hasattr(obj, "view_groups"):
for pk in obj.view_groups.values_list("pk", flat=True): # if "view_groups" has already been prefetched, use
if self.is_in_group(pk=pk): # the prefetch cache, else fetch only the ids, to make
return True # the query lighter.
if (
hasattr(obj, "_prefetched_objects_cache")
and "view_groups" in obj._prefetched_objects_cache
):
pks = [g.id for g in obj.view_groups.all()]
else:
pks = list(obj.view_groups.values_list("id", flat=True))
if any(self.is_in_group(pk=pk) for pk in pks):
return True
return self.can_edit(obj) return self.can_edit(obj)
def can_be_edited_by(self, user): def can_be_edited_by(self, user):
@@ -1384,9 +1399,9 @@ class Page(models.Model):
@cached_property @cached_property
def is_club_page(self): def is_club_page(self):
club_root_page = Page.objects.filter(name=settings.SITH_CLUB_ROOT_PAGE).first() return (
return club_root_page is not None and ( self.name == settings.SITH_CLUB_ROOT_PAGE
self == club_root_page or club_root_page in self.get_parent_list() or settings.SITH_CLUB_ROOT_PAGE in [p.name for p in self.get_parent_list()]
) )
@cached_property @cached_property

View File

@@ -34,22 +34,6 @@ class SimpleUserSchema(ModelSchema):
fields = ["id", "nick_name", "first_name", "last_name"] fields = ["id", "nick_name", "first_name", "last_name"]
class UserSchema(ModelSchema):
class Meta:
model = User
fields = [
"id",
"nick_name",
"first_name",
"last_name",
"date_of_birth",
"email",
"role",
"quote",
"promo",
]
class UserProfileSchema(ModelSchema): class UserProfileSchema(ModelSchema):
"""The necessary information to show a user profile""" """The necessary information to show a user profile"""

View File

@@ -5,16 +5,12 @@
{% endblock %} {% endblock %}
{% block content %} {% block content %}
{% if page_list %} <h3>{% trans %}Page list{% endtrans %}</h3>
<h3>{% trans %}Page list{% endtrans %}</h3> <ul>
<ul> {% for p in page_list %}
{% for p in page_list %} <li><a href="{{ p.get_absolute_url() }}">{{ p.display_name }}</a></li>
<li><a href="{{ p.get_absolute_url() }}">{{ p.get_display_name() }}</a></li> {% endfor %}
{% endfor %} </ul>
</ul>
{% else %}
{% trans %}There is no page in this website.{% endtrans %}
{% endif %}
{% endblock %} {% endblock %}

View File

@@ -12,7 +12,10 @@
# OR WITHIN THE LOCAL FILE "LICENSE" # OR WITHIN THE LOCAL FILE "LICENSE"
# #
# #
from django.contrib.auth.mixins import PermissionRequiredMixin from django.contrib.auth.mixins import PermissionRequiredMixin
from django.db.models import F, OuterRef, Subquery
from django.db.models.functions import Coalesce
# This file contains all the views that concern the page model # This file contains all the views that concern the page model
from django.forms.models import modelform_factory from django.forms.models import modelform_factory
@@ -43,6 +46,20 @@ class CanEditPagePropMixin(CanEditPropMixin):
class PageListView(CanViewMixin, ListView): class PageListView(CanViewMixin, ListView):
model = Page model = Page
template_name = "core/page_list.jinja" template_name = "core/page_list.jinja"
queryset = (
Page.objects.annotate(
display_name=Coalesce(
Subquery(
PageRev.objects.filter(page=OuterRef("id"))
.order_by("-date")
.values("title")[:1]
),
F("name"),
)
)
.prefetch_related("view_groups")
.select_related("parent")
)
class PageView(CanViewMixin, DetailView): class PageView(CanViewMixin, DetailView):

View File

@@ -535,13 +535,6 @@ class Counter(models.Model):
def __str__(self): def __str__(self):
return self.name return self.name
def __getattribute__(self, name: str):
if name == "edit_groups":
return Group.objects.filter(
name=self.club.unix_name + settings.SITH_BOARD_SUFFIX
).all()
return object.__getattribute__(self, name)
def get_absolute_url(self) -> str: def get_absolute_url(self) -> str:
if self.type == "EBOUTIC": if self.type == "EBOUTIC":
return reverse("eboutic:main") return reverse("eboutic:main")

View File

@@ -6,7 +6,7 @@
msgid "" msgid ""
msgstr "" msgstr ""
"Report-Msgid-Bugs-To: \n" "Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2025-09-02 15:56+0200\n" "POT-Creation-Date: 2025-09-01 18:18+0200\n"
"PO-Revision-Date: 2016-07-18\n" "PO-Revision-Date: 2016-07-18\n"
"Last-Translator: Maréchal <thomas.girod@utbm.fr\n" "Last-Translator: Maréchal <thomas.girod@utbm.fr\n"
"Language-Team: AE info <ae.info@utbm.fr>\n" "Language-Team: AE info <ae.info@utbm.fr>\n"
@@ -5135,10 +5135,6 @@ msgstr "Tee-shirt AE"
msgid "A user with that email address already exists" msgid "A user with that email address already exists"
msgstr "Un utilisateur avec cette adresse email existe déjà" msgstr "Un utilisateur avec cette adresse email existe déjà"
#: subscription/forms.py
msgid "This user didn't fill its birthdate yet."
msgstr "Cet utilisateur n'a pas encore renseigné sa date de naissance"
#: subscription/models.py #: subscription/models.py
msgid "Bad subscription type" msgid "Bad subscription type"
msgstr "Mauvais type de cotisation" msgstr "Mauvais type de cotisation"
@@ -5178,7 +5174,7 @@ msgid ""
"%(user)s received its new %(type)s subscription. It will be active until " "%(user)s received its new %(type)s subscription. It will be active until "
"%(end)s included." "%(end)s included."
msgstr "" msgstr ""
"%(user)s a reçu sa nouvelle cotisaton %(type)s. Elle sera active jusqu'au " "%(user)s a reçu sa nouvelle cotisaton %(type)s. Elle sert active jusqu'au "
"%(end)s inclu." "%(end)s inclu."
#: subscription/templates/subscription/fragments/creation_success.jinja #: subscription/templates/subscription/fragments/creation_success.jinja

View File

@@ -405,9 +405,6 @@ SITH_FORUM_PAGE_LENGTH = 30
SITH_SAS_ROOT_DIR_ID = env.int("SITH_SAS_ROOT_DIR_ID", default=4) SITH_SAS_ROOT_DIR_ID = env.int("SITH_SAS_ROOT_DIR_ID", default=4)
SITH_SAS_IMAGES_PER_PAGE = 60 SITH_SAS_IMAGES_PER_PAGE = 60
SITH_BOARD_SUFFIX = "-bureau"
SITH_MEMBER_SUFFIX = "-membres"
SITH_PROFILE_DEPARTMENTS = [ SITH_PROFILE_DEPARTMENTS = [
("TC", _("TC")), ("TC", _("TC")),
("IMSI", _("IMSI")), ("IMSI", _("IMSI")),

View File

@@ -23,8 +23,8 @@ class SelectionDateForm(forms.Form):
class SubscriptionForm(forms.ModelForm): class SubscriptionForm(forms.ModelForm):
def __init__(self, *args, initial=None, **kwargs): def __init__(self, *args, **kwargs):
initial = initial or {} initial = kwargs.pop("initial", {})
if "subscription_type" not in initial: if "subscription_type" not in initial:
initial["subscription_type"] = "deux-semestres" initial["subscription_type"] = "deux-semestres"
if "payment_method" not in initial: if "payment_method" not in initial:
@@ -131,57 +131,8 @@ class SubscriptionExistingUserForm(SubscriptionForm):
"""Form to add a subscription to an existing user.""" """Form to add a subscription to an existing user."""
template_name = "subscription/forms/create_existing_user.html" template_name = "subscription/forms/create_existing_user.html"
required_css_class = "required"
birthdate = forms.fields_for_model(
User,
["date_of_birth"],
widgets={"date_of_birth": SelectDate(attrs={"hidden": True})},
help_texts={"date_of_birth": _("This user didn't fill its birthdate yet.")},
)["date_of_birth"]
class Meta: class Meta:
model = Subscription model = Subscription
fields = ["member", "subscription_type", "payment_method", "location"] fields = ["member", "subscription_type", "payment_method", "location"]
widgets = {"member": AutoCompleteSelectUser} widgets = {"member": AutoCompleteSelectUser}
field_order = [
"member",
"birthdate",
"subscription_type",
"payment_method",
"location",
]
def __init__(self, *args, initial=None, **kwargs):
super().__init__(*args, initial=initial, **kwargs)
self.fields["birthdate"].required = True
if not initial:
return
member: str | None = initial.get("member")
if member and member.isdigit():
member: User | None = User.objects.filter(id=int(member)).first()
else:
member = None
if member and member.date_of_birth:
# if there is an initial member with a birthdate,
# there is no need to ask this to the user
self.fields["birthdate"].initial = member.date_of_birth
elif member:
# if there is an initial member without a birthdate,
# then the field must be displayed
self.fields["birthdate"].widget.attrs.update({"hidden": False})
# if there is no initial member, it means that it will be
# dynamically selected using the AutoCompleteSelectUser widget.
# JS will take care of un-hiding the field if necessary
def save(self, *args, **kwargs):
if self.errors:
return super().save(*args, **kwargs)
if (
self.cleaned_data["birthdate"] is not None
and self.instance.member.date_of_birth is None
):
self.instance.member.date_of_birth = self.cleaned_data["birthdate"]
self.instance.member.save()
return super().save(*args, **kwargs)

View File

@@ -1,5 +1,3 @@
import { userFetchUser } from "#openapi";
document.addEventListener("alpine:init", () => { document.addEventListener("alpine:init", () => {
Alpine.data("existing_user_subscription_form", () => ({ Alpine.data("existing_user_subscription_form", () => ({
loading: false, loading: false,
@@ -14,24 +12,13 @@ document.addEventListener("alpine:init", () => {
}, },
async loadProfile(userId: number) { async loadProfile(userId: number) {
const birthdayInput = document.getElementById("id_birthdate") as HTMLInputElement;
if (!Number.isInteger(userId)) { if (!Number.isInteger(userId)) {
this.profileFragment = ""; this.profileFragment = "";
birthdayInput.hidden = true;
return; return;
} }
this.loading = true; this.loading = true;
const [miniProfile, userInfos] = await Promise.all([ const response = await fetch(`/user/${userId}/mini/`);
fetch(`/user/${userId}/mini/`), this.profileFragment = await response.text();
// biome-ignore lint/style/useNamingConvention: api is snake_case
userFetchUser({ path: { user_id: userId } }),
]);
this.profileFragment = await miniProfile.text();
// If the user has no birthdate yet, show the form input
// to fill this info.
// Else keep the input hidden and change its value to the user birthdate
birthdayInput.value = userInfos.data.date_of_birth;
birthdayInput.hidden = userInfos.data.date_of_birth !== null;
this.loading = false; this.loading = false;
}, },
})); }));

View File

@@ -1,14 +1,4 @@
#subscription-form form { #subscription-form form {
margin-top: 0;
.form-content {
margin-top: 0;
}
fieldset p:first-of-type, & > p:first-of-type {
margin-top: 0;
}
.form-content.existing-user { .form-content.existing-user {
max-height: 100%; max-height: 100%;
display: flex; display: flex;
@@ -23,11 +13,6 @@
* then display the user profile right in the middle of the remaining space. */ * then display the user profile right in the middle of the remaining space. */
fieldset { fieldset {
flex: 0 1 auto; flex: 0 1 auto;
p:has(input[hidden]) {
// when the input is hidden, hide the whole label+input+help text group
display: none;
}
} }
#subscription-form-user-mini-profile { #subscription-form-user-mini-profile {

View File

@@ -1,6 +1,6 @@
"""Tests focused on testing subscription creation""" """Tests focused on testing subscription creation"""
from datetime import date, timedelta from datetime import timedelta
from typing import Callable from typing import Callable
import pytest import pytest
@@ -31,26 +31,6 @@ def test_form_existing_user_valid(
): ):
"""Test `SubscriptionExistingUserForm`""" """Test `SubscriptionExistingUserForm`"""
user = user_factory() user = user_factory()
user.date_of_birth = date(year=1967, month=3, day=14)
user.save()
data = {
"member": user,
"birthdate": user.date_of_birth,
"subscription_type": "deux-semestres",
"location": settings.SITH_SUBSCRIPTION_LOCATIONS[0][0],
"payment_method": settings.SITH_SUBSCRIPTION_PAYMENT_METHOD[0][0],
}
form = SubscriptionExistingUserForm(data)
assert form.is_valid()
form.save()
user.refresh_from_db()
assert user.is_subscribed
@pytest.mark.django_db
def test_form_existing_user_with_birthdate(settings: SettingsWrapper):
"""Test `SubscriptionExistingUserForm`"""
user = baker.make(User, date_of_birth=None)
data = { data = {
"member": user, "member": user,
"subscription_type": "deux-semestres", "subscription_type": "deux-semestres",
@@ -58,15 +38,11 @@ def test_form_existing_user_with_birthdate(settings: SettingsWrapper):
"payment_method": settings.SITH_SUBSCRIPTION_PAYMENT_METHOD[0][0], "payment_method": settings.SITH_SUBSCRIPTION_PAYMENT_METHOD[0][0],
} }
form = SubscriptionExistingUserForm(data) form = SubscriptionExistingUserForm(data)
assert not form.is_valid()
data |= {"birthdate": date(year=1967, month=3, day=14)}
form = SubscriptionExistingUserForm(data)
assert form.is_valid() assert form.is_valid()
form.save() form.save()
user.refresh_from_db() user.refresh_from_db()
assert user.is_subscribed assert user.is_subscribed
assert user.date_of_birth == date(year=1967, month=3, day=14)
@pytest.mark.django_db @pytest.mark.django_db
@@ -156,14 +132,6 @@ def test_page_access(
assert res.status_code == status_code assert res.status_code == status_code
@pytest.mark.django_db
def test_page_access_with_get_data(client: Client):
user = old_subscriber_user.make()
client.force_login(baker.make(User, is_superuser=True))
res = client.get(reverse("subscription:subscription", query={"member": user.id}))
assert res.status_code == 200
@pytest.mark.django_db @pytest.mark.django_db
def test_submit_form_existing_user(client: Client, settings: SettingsWrapper): def test_submit_form_existing_user(client: Client, settings: SettingsWrapper):
client.force_login( client.force_login(
@@ -172,12 +140,11 @@ def test_submit_form_existing_user(client: Client, settings: SettingsWrapper):
user_permissions=Permission.objects.filter(codename="add_subscription"), user_permissions=Permission.objects.filter(codename="add_subscription"),
) )
) )
user = old_subscriber_user.make(date_of_birth=date(year=1967, month=3, day=14)) user = old_subscriber_user.make()
response = client.post( response = client.post(
reverse("subscription:fragment-existing-user"), reverse("subscription:fragment-existing-user"),
{ {
"member": user.id, "member": user.id,
"birthdate": user.date_of_birth,
"subscription_type": "deux-semestres", "subscription_type": "deux-semestres",
"location": settings.SITH_SUBSCRIPTION_LOCATIONS[0][0], "location": settings.SITH_SUBSCRIPTION_LOCATIONS[0][0],
"payment_method": settings.SITH_SUBSCRIPTION_PAYMENT_METHOD[0][0], "payment_method": settings.SITH_SUBSCRIPTION_PAYMENT_METHOD[0][0],