Włącz FETCH_PEERS (Django 6.1) w adminie

src/bpp/admin/autor.py

@@ -28,6 +28,7 @@
     OrcidObecnyFilter,
     PBN_UID_IDObecnyFilter,
     PBNIDObecnyFilter,
+    WydzialAutoraFilter,
 )
 from .helpers.fieldsets import ADNOTACJE_FIELDSET, ZapiszZAdnotacjaMixin
 from .helpers.site_filtered import SiteFilteredAdminMixin
@@ -335,7 +336,7 @@ def has_delete_permission(self, request, obj=None):
     ]
     list_filter = [
         JednostkaFilter,
-        "aktualna_jednostka__wydzial",
+        WydzialAutoraFilter,
         "tytul",
         PBNIDObecnyFilter,
         OrcidObecnyFilter,

src/bpp/admin/core.py

@@ -8,6 +8,7 @@
 from django.conf import settings
 from django.contrib import admin
 from django.core.cache import cache
+from django.db.models import FETCH_PEERS
 from django.db.models.fields import BLANK_CHOICE_DASH
 from django.forms import NullBooleanField
 from django.forms.widgets import HiddenInput
@@ -117,6 +118,41 @@ class BaseBppAdminMixin(DynamicAdminFilterMixin):
     # ograniczenie wielkosci listy
     list_per_page = 50
 
+    def get_queryset(self, request):
+        """Włącz ``FETCH_PEERS`` (Django 6.1) dla querysetów tego admina.
+
+        Changelisty admina to najgęstsze w BPP skupisko N+1: ``list_display``
+        i ``__str__`` modeli sięgają po FK, których nikt nie zadeklarował
+        w ``list_select_related``, a każde takie dotknięcie to osobny SELECT
+        per wiersz. ``FETCH_PEERS`` sprawia, że PIERWSZE leniwe dotknięcie
+        relacji (albo pola odroczonego) dociąga ją HURTEM dla całego
+        rodzeństwa z tego samego pobrania — N+1 zamienia się w 2 zapytania,
+        bez zgadywania z góry, które FK dotknie szablon.
+
+        Zmierzone na kopii bazy produkcyjnej (z produkcyjnymi regułami
+        ``CACHEOPS``) — zapytania i mediana czasu na request:
+
+        * changelist jednostek        66 → 17 zapytań, 185 → 120 ms
+        * changelist wyd. zwartych   220 → 40 zapytań, 227 → 184 ms
+        * changelist wyd. ciągłych   190 → 38 zapytań, 221 → 190 ms
+
+        Dlaczego TU, a nie globalnie (podstawienie ``DEFAULT_FETCH_MODE``):
+        ``track_peers`` trzyma ``weakref`` do każdej instancji z pobrania,
+        więc koszt ponosiłby KAŻDY queryset w aplikacji, a zysk jest
+        skoncentrowany w adminie. Samo ``get_queryset`` wystarcza, bo
+        ``QuerySet._clone()`` przenosi ``_fetch_mode`` — tryb przeżywa
+        filtry, sortowanie i slicing dokładane przez dalsze mixiny
+        i przez sam ``ChangeList``.
+
+        Semantyka się NIE zmienia: ``fetch_one`` i ``fetch_many`` idą tą samą
+        ścieżką managera (``_base_manager``), więc tryb nie zaczyna nagle
+        odfiltrowywać rekordów — istotne przy soft-delete.
+
+        WYMAGA Django >= 6.1 (``QuerySet.fetch_mode``) — dlatego ta zmiana
+        celuje w gałąź ``django-6.1``, a nie w ``dev``.
+        """
+        return super().get_queryset(request).fetch_mode(FETCH_PEERS)
+
     def save_related(self, request, form, formsets, change):
         """
         Przebuduj cache punktacji PO zapisaniu wszystkich inlines (autorzy/dyscypliny).

src/bpp/admin/filters.py

@@ -326,6 +326,40 @@ def queryset(self, request, queryset):
         return queryset
 
 
+class WydzialAutoraFilter(WydzialFilter):
+    """``WydzialFilter`` dla changelisty ``AutorAdmin`` (#438, domknięcie).
+
+    Ten sam problem co w ``JednostkaAdmin``, tylko o jeden hop dalej: goły
+    ``list_filter = ("aktualna_jednostka__wydzial", ...)`` kazał Django
+    zbudować ``RelatedFieldListFilter``, a ten woła ``field.get_choices()``.
+    Skoro po Fazie B denorm ``Jednostka.wydzial`` jest self-FK na
+    ``Jednostka``, ``get_choices()`` enumerowało CAŁĄ tabelę jednostek --
+    produkcyjnie 504 pozycje zamiast 7 wydziałów, plus 504 zapytania na
+    request (``Jednostka.__str__`` czyta ``self.uczelnia``).
+
+    Z klasy bazowej dziedziczymy BEZ zmian ``lookups`` (tylko korzenie,
+    zawężone do uczelni z requestu) i ``has_output`` (bramka
+    ``uzywaj_wydzialow``) -- lista „wydziałów" jest ta sama niezależnie od
+    tego, co filtrujemy. Nadpisujemy wyłącznie ``queryset``, bo tu
+    zawężamy ``Autor``, a nie ``Jednostka``: do korzenia trzeba dojść przez
+    ``aktualna_jednostka__``.
+    """
+
+    def queryset(self, request, queryset):
+        v = self.value()
+        if v:
+            # Odpowiednik `Q(wydzial_id=v) | Q(pk=v)` z klasy bazowej,
+            # przetraversowany o jeden FK dalej: autorzy z jednostek
+            # niosących denorm ``wydzial=korzeń`` PLUS autorzy przypisani
+            # wprost do samego korzenia (korzeń nie wskazuje na siebie).
+            # Oba warunki to forward-FK (bez multi-valued join), więc unia
+            # nie duplikuje wierszy i nie potrzebuje ``distinct()``.
+            return queryset.filter(
+                Q(aktualna_jednostka__wydzial_id=v) | Q(aktualna_jednostka_id=v)
+            )
+        return queryset
+
+
 class JednostkaFilter(SimpleListFilter):
     title = "Jednostka"
     parameter_name = "jednostka"
@@ -337,8 +371,18 @@ def queryset(self, request, queryset):
         return queryset
 
     def lookups(self, request, model_admin):
+        # ``uczelnia`` w select_related, NIE tylko ``wydzial``:
+        # ``Jednostka.__str__`` czyta OBA FK — ``self.uczelnia`` (bramka
+        # ``uzywaj_wydzialow``) i ``self.wydzial`` (skrót w nawiasie). Bez
+        # ``uczelnia`` każda opcja listy kosztowała osobny SELECT: na bazie
+        # produkcyjnej 504 jednostki = 504 zapytania na KAŻDE wejście na
+        # changelistę autorów. Cacheops zamieniał je na trafienia w Redis
+        # (``bpp.uczelnia`` jest w regułach), więc licznik zapytań SQL tego
+        # nie pokazywał — ale 504 round-tripy do Redisa nadal kosztowały
+        # ~200 ms na request.
         return (
-            (x.pk, str(x)) for x in Jednostka.objects.all().select_related("wydzial")
+            (x.pk, str(x))
+            for x in Jednostka.objects.all().select_related("wydzial", "uczelnia")
         )
 
 
@@ -358,11 +402,22 @@ def logentries(self):
         )
 
     def lookups(self, request, model_admin):
+        # ``only()`` MUSI wymieniać wszystkie pola, które czyta
+        # ``BppUser.__str__`` (``last_name``, ``first_name`` — patrz
+        # ``bpp/models/profile.py``), inaczej „optymalizacja" kosztuje
+        # zamiast oszczędzać: każde pole odroczone to osobny
+        # ``refresh_from_db()`` per użytkownik, czyli DWA dodatkowe SELECT-y
+        # na wiersz. Na bazie produkcyjnej to było 156 zapytań na wejście
+        # na changelistę wydawnictw ciągłych (78 użytkowników × 2 pola) —
+        # i w przeciwieństwie do słowników ``bpp.bppuser`` NIE jest
+        # cache'owany przez cacheops, więc szły wprost do PostgreSQL.
+        #
+        # Jeśli dokładasz pole do ``__str__``, dołóż je też tutaj.
         return (
             (x.pk, str(x))
             for x in BppUser.objects.filter(
                 pk__in=self.logentries().values_list("user_id")
-            ).only("pk", "username")
+            ).only("pk", "username", "last_name", "first_name")
         )
 
 

src/bpp/newsfragments/autor-filtr-wydzial.bugfix.rst

@@ -0,0 +1,4 @@
+Filtr „Wydział" na liście autorów w panelu admina listował wszystkie jednostki
+(na produkcji 504 pozycje) zamiast samych wydziałów — teraz pokazuje wyłącznie
+jednostki-korzenie i filtruje po całym poddrzewie wydziału, co dodatkowo
+usuwa setki zbędnych zapytań przy każdym wyświetleniu listy.

src/bpp/newsfragments/fetch-peers-admin.feature.rst

@@ -0,0 +1,5 @@
+Listy w panelu administracyjnym korzystają z nowego trybu pobierania relacji
+z Django 6.1 (``FETCH_PEERS``): powiązane obiekty potrzebne do wyświetlenia
+wiersza dociągane są hurtem dla całej strony, a nie osobno dla każdego
+wiersza. Na dużej bazie skraca to czas otwarcia list o kilkanaście do
+kilkudziesięciu procent, zależnie od listy.

src/bpp/newsfragments/wydajnosc-filtry-admina.bugfix.rst

@@ -0,0 +1,6 @@
+Przyspieszono listy rozwijane filtrów w panelu administracyjnym. Filtr
+„Jednostka" dociągał uczelnię osobnym zapytaniem dla każdej pozycji listy
+(na dużej bazie ponad 500 zapytań na każde wejście na listę autorów),
+a filtry „Utworzone przez" i „Ostatnio zmienione przez" pobierały imię
+i nazwisko użytkownika dwoma dodatkowymi zapytaniami na pozycję. Teraz
+każda z tych list powstaje jednym zapytaniem.

src/bpp/newsfragments/wydajnosc-indeks-jednostek.bugfix.rst

@@ -0,0 +1,4 @@
+Przyspieszono indeks jednostek w części publicznej. Liczba autorów przy
+każdej jednostce była liczona osobnym zapytaniem, i to kilkukrotnie na
+wiersz — na dużej bazie dawało to ponad 500 zapytań na jedno wyświetlenie
+strony. Teraz liczby przychodzą jednym zapytaniem razem z listą jednostek.

src/bpp/templates/browse/jednostki.html

@@ -125,10 +125,10 @@ <h2>
                                     </div>
                                 {% endif %}
 
-                                {% if item.aktualna_jednostka.count > 0 %}
+                                {% if item.liczba_autorow > 0 %}
                                     <div>
                                         <i class="fi-torsos-all"></i>
-                                        {{ item.aktualna_jednostka.count }} {% if item.aktualna_jednostka.count == 1 %}autor{% elif item.aktualna_jednostka.count < 5 %}autorów{% else %}autorów{% endif %}
+                                        {{ item.liczba_autorow }} {% if item.liczba_autorow == 1 %}autor{% elif item.liczba_autorow < 5 %}autorów{% else %}autorów{% endif %}
                                     </div>
                                 {% endif %}
                             </div>

src/bpp/tests/test_admin/test_autor_wydzial_filter.py

@@ -0,0 +1,146 @@
+"""Testy filtra „Wydział" na changeliście ``AutorAdmin`` (#438, domknięcie).
+
+Faza B (#438) zamieniła goły ``list_filter = ("wydzial", ...)`` na
+``WydzialFilter`` w ``JednostkaAdmin``, ale ``AutorAdmin`` został z gołym
+stringiem ``"aktualna_jednostka__wydzial"``. Ponieważ denorm
+``Jednostka.wydzial`` jest self-FK na ``Jednostka``, ``RelatedFieldListFilter``
+enumerował CAŁĄ tabelę jednostek (produkcyjnie: 504 pozycje zamiast
+7 wydziałów).
+"""
+
+import pytest
+from django.urls import reverse
+from model_bakery import baker
+
+from bpp.admin.filters import WydzialAutoraFilter
+from bpp.models import Autor, Jednostka, Uczelnia
+
+
+def _spec_wydzial(response):
+    """Zwraca FilterSpec o tytule „Wydział" z wyrenderowanej changelisty."""
+    for spec in response.context["cl"].filter_specs:
+        if str(spec.title) == "Wydział":
+            return spec
+    return None
+
+
+@pytest.mark.django_db
+def test_AutorAdmin_WydzialFilter_opcje_tylko_korzenie(
+    admin_client,
+    uczelnia: Uczelnia,
+    jednostka: Jednostka,
+    jednostka_podrzedna: Jednostka,
+    druga_jednostka: Jednostka,
+):
+    # ISTOTA BUGA: opcji filtra ma być tyle, ile jednostek-korzeni („wydziałów"),
+    # a nie tyle, ile jednostek w bazie. Tu drzewo to 1 korzeń (fixture
+    # `wydzial`) + 3 węzły niżej — więc dokładnie 1 opcja, nie 4.
+    uczelnia.uzywaj_wydzialow = True
+    uczelnia.save()
+
+    response = admin_client.get(reverse("admin:bpp_autor_changelist"))
+    assert response.status_code == 200
+
+    spec = _spec_wydzial(response)
+    assert spec is not None, "changelist nie renderuje filtra po wydziale"
+
+    korzenie = Jednostka.objects.filter(parent__isnull=True, widoczna=True)
+    lookup_pks = {pk for pk, _ in spec.lookup_choices}
+    assert lookup_pks == {j.pk for j in korzenie}
+    assert len(lookup_pks) == korzenie.count()
+    # Węzły spod korzenia NIE mogą trafiać na listę wyboru „wydziału".
+    assert jednostka.pk not in lookup_pks
+    assert jednostka_podrzedna.pk not in lookup_pks
+    assert druga_jednostka.pk not in lookup_pks
+
+
+@pytest.mark.django_db
+def test_AutorAdmin_WydzialFilter_filtruje_poddrzewo(
+    admin_client,
+    jednostka: Jednostka,
+    jednostka_podrzedna: Jednostka,
+    druga_jednostka: Jednostka,
+):
+    # Wybór korzenia zawęża do autorów z CAŁEGO poddrzewa: bezpośrednich
+    # dzieci, wnuków ORAZ przypisanych wprost do korzenia (ten ostatni
+    # przypadek obsługuje `| Q(aktualna_jednostka_id=v)`).
+    root = jednostka.parent
+    obcy_root = baker.make(Jednostka, parent=None, uczelnia=jednostka.uczelnia)
+    obca = baker.make(Jednostka, parent=obcy_root, uczelnia=jednostka.uczelnia)
+
+    a_dziecko = baker.make(Autor, aktualna_jednostka=jednostka)
+    a_wnuk = baker.make(Autor, aktualna_jednostka=jednostka_podrzedna)
+    a_drugie_dziecko = baker.make(Autor, aktualna_jednostka=druga_jednostka)
+    a_korzen = baker.make(Autor, aktualna_jednostka=root)
+    a_obcy = baker.make(Autor, aktualna_jednostka=obca)
+    a_bez_jednostki = baker.make(Autor, aktualna_jednostka=None)
+
+    response = admin_client.get(
+        reverse("admin:bpp_autor_changelist"), {"wydzial": root.pk}
+    )
+    assert response.status_code == 200
+
+    result_list = list(response.context["cl"].result_list)
+    assert a_dziecko in result_list
+    assert a_wnuk in result_list
+    assert a_drugie_dziecko in result_list
+    assert a_korzen in result_list
+    assert a_obcy not in result_list
+    assert a_bez_jednostki not in result_list
+
+
+@pytest.mark.django_db
+def test_AutorAdmin_WydzialFilter_ukryty_gdy_uczelnia_bez_wydzialow(
+    admin_client, uczelnia: Uczelnia, jednostka: Jednostka
+):
+    # Bramka `uzywaj_wydzialow` (dziedziczona z WydzialFilter): instytucja
+    # 1-progowa nie ma czego filtrować po wydziale.
+    uczelnia.uzywaj_wydzialow = False
+    uczelnia.save()
+
+    response = admin_client.get(reverse("admin:bpp_autor_changelist"))
+    assert response.status_code == 200
+    assert not any(
+        isinstance(spec, WydzialAutoraFilter)
+        for spec in response.context["cl"].filter_specs
+    )
+
+
+@pytest.mark.django_db
+def test_AutorAdmin_WydzialFilter_widoczny_gdy_uczelnia_z_wydzialami(
+    admin_client, uczelnia: Uczelnia, jednostka: Jednostka
+):
+    uczelnia.uzywaj_wydzialow = True
+    uczelnia.save()
+
+    response = admin_client.get(reverse("admin:bpp_autor_changelist"))
+    assert response.status_code == 200
+    assert any(
+        isinstance(spec, WydzialAutoraFilter)
+        for spec in response.context["cl"].filter_specs
+    )
+
+
+@pytest.mark.django_db
+def test_AutorAdmin_stary_querystring_wydzialu_daje_400(
+    admin_client, jednostka: Jednostka
+):
+    # Świadoma zmiana kontraktu URL: filtr zmienił parametr z
+    # `?aktualna_jednostka__wydzial__id__exact=<id>` na `?wydzial=<id>`.
+    # Filtry admina to ulotny stan UI (a nie trwałe linki), więc zerwanie
+    # starych URL-i akceptujemy — ale musi degradować się przewidywalnie,
+    # bez 500.
+    #
+    # ZMIERZONE zachowanie (nie założone): skoro pola nie ma już w
+    # `list_filter`, `ChangeList.get_filters` odrzuca lookup jako
+    # `DisallowedModelAdminLookup`. To podklasa `SuspiciousOperation`, więc
+    # handler wyjątków Django zamienia ją na **400 Bad Request** i loguje w
+    # kanale `django.security` — NIE jest to 500 ani redirect na `?e=1`
+    # (`?e=1` dostajemy tylko dla `IncorrectLookupParameters`, czyli dla
+    # DOZWOLONEGO pola z niepoprawną wartością).
+    root = jednostka.parent
+    response = admin_client.get(
+        reverse("admin:bpp_autor_changelist"),
+        {"aktualna_jednostka__wydzial__id__exact": root.pk},
+    )
+    assert response.status_code == 400

src/bpp/tests/test_admin/test_fetch_peers.py

@@ -0,0 +1,61 @@
+"""Kontrakt: adminy BPP pobierają dane w trybie ``FETCH_PEERS`` (Django 6.1).
+
+``BaseBppAdminMixin.get_queryset`` włącza ``FETCH_PEERS``, żeby pierwsze
+leniwe dotknięcie relacji (albo pola odroczonego) dociągało ją hurtem dla
+całego rodzeństwa z tego samego pobrania. To zamienia N+1 na changelistach
+admina na 2 zapytania — bez zgadywania z góry, które FK dotknie szablon.
+
+Testy tutaj pilnują dwóch rzeczy, które łatwo zepsuć niechcący:
+
+1. tryb jest w ogóle ustawiany (ktoś mógłby nadpisać ``get_queryset``
+   w podklasie i zapomnieć o ``super()``),
+2. tryb PRZEŻYWA łańcuch ``.filter()/.order_by()/[slice]`` — to jest cała
+   przesłanka, dla której wystarcza jedno wywołanie w ``get_queryset``,
+   a nie łatanie każdego miejsca osobno.
+"""
+
+import pytest
+from django.contrib import admin as dj_admin
+from django.db.models import FETCH_PEERS
+
+from bpp.admin.autor import AutorAdmin
+from bpp.admin.jednostka import JednostkaAdmin
+from bpp.admin.wydawnictwo_ciagle import Wydawnictwo_CiagleAdmin
+from bpp.models import Autor, Jednostka, Wydawnictwo_Ciagle
+
+
+def _queryset_admina(klasa_admina, model, rf, admin_user):
+    request = rf.get("/admin/")
+    request.user = admin_user
+    return klasa_admina(model, dj_admin.site).get_queryset(request)
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    "klasa_admina,model",
+    [
+        (AutorAdmin, Autor),
+        (JednostkaAdmin, Jednostka),
+        (Wydawnictwo_CiagleAdmin, Wydawnictwo_Ciagle),
+    ],
+)
+def test_admin_pobiera_w_trybie_fetch_peers(klasa_admina, model, rf, admin_user):
+    queryset = _queryset_admina(klasa_admina, model, rf, admin_user)
+    assert queryset._fetch_mode is FETCH_PEERS, klasa_admina.__name__
+
+
+@pytest.mark.django_db
+def test_fetch_peers_przezywa_lancuch_querysetu(rf, admin_user):
+    """Tryb przenosi się przez ``_clone()`` — filtr, sortowanie i slicing.
+
+    ``ChangeList`` i dalsze mixiny dokładają do querysetu własne ``filter``,
+    ``order_by`` i wycinek strony. Gdyby tryb ginął przy klonowaniu,
+    ustawienie go w ``get_queryset`` nic by nie dawało.
+    """
+    queryset = _queryset_admina(AutorAdmin, Autor, rf, admin_user)
+
+    assert queryset.filter(pokazuj=True)._fetch_mode is FETCH_PEERS
+    assert queryset.order_by("nazwisko")._fetch_mode is FETCH_PEERS
+    assert queryset.filter(pokazuj=True).order_by("nazwisko")[:10]._fetch_mode is (
+        FETCH_PEERS
+    )