From 1c924e965a2089840c2a256a622227bc7d8a5084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sun, 16 Aug 2026 22:38:01 +0200 Subject: [PATCH 01/18] feat(soft-delete): context manager atrybucji usera (thread-local) Sygnaly pakietu django-soft-delete niosa wylacznie sender+instance, wiec user i powod musza dojechac do receiverow innym kanalem. Thread-local ustawiany przez delete(user=, reason=) jest najwezszym rozwiazaniem, ktore obsluguje wszystkie modele soft-delete naraz i nie wymaga forka pakietu. Sprzatanie w finally, nie po yield: guard fazy 04 przerywa delete() autora z pracami przez ProtectedError, a przeciekly kontekst przypisalby cudzego usera nastepnej operacji w tym samym watku (kolejny request na tym samym workerze). Blad bylby cichy i nie do wykrycia po fakcie -- stad osobny test. Reentrancja jest wymagana, nie kosmetyczna: waska kaskada fazy 02 wchodzi w kontekst ponownie dla kazdego wiersza *_Autor, wiec bez odtworzenia poprzednich wartosci kaskada wyzerowalaby usera w polowie operacji. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/models/soft_delete_context.py | 71 +++++++++++++++++++ .../test_soft_delete_context.py | 59 +++++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 src/bpp/models/soft_delete_context.py create mode 100644 src/bpp/tests/test_soft_delete/test_soft_delete_context.py diff --git a/src/bpp/models/soft_delete_context.py b/src/bpp/models/soft_delete_context.py new file mode 100644 index 000000000..0e2d3312e --- /dev/null +++ b/src/bpp/models/soft_delete_context.py @@ -0,0 +1,71 @@ +"""Kanał atrybucji „kto i dlaczego" dla operacji soft-delete. + +DLACZEGO THREAD-LOCAL, A NIE ARGUMENT: sygnały pakietu +``django-soft-delete`` (``post_soft_delete``, ``post_restore``, +``post_hard_delete``) niosą wyłącznie ``sender`` i ``instance`` — usera ani +powodu nie przekazują i nie da się ich dołożyć bez forka pakietu. Receivery +z ``bpp/receivers/soft_delete.py`` są jednak JEDYNYM miejscem, w którym +powstaje ``SoftDeleteLog``, więc user musi tam jakoś dojechać. Kontekst +ustawiany przez ``delete(user=, reason=)`` tuż przed wysłaniem sygnału jest +najwęższym kanałem, jaki załatwia sprawę dla wszystkich modeli naraz. + +KONTRAKT Z REVERSION: to ten SAM punkt wstrzyknięcia, w który wepnie się +przyszłe ``reversion.set_user`` — jeden hook, nie dwa konkurencyjne. + +Operacje bez zalogowanego użytkownika (scalanie duplikatów, celery, gołe +``.delete()`` w skryptach) po prostu nie wchodzą w kontekst: akcesory +zwracają wtedy ``None``/``""`` i log powstaje bez atrybucji. To jest +poprawny wynik, nie błąd — „nie wiadomo kto" jest uczciwszą odpowiedzią niż +podstawienie pierwszego lepszego konta. +""" + +import threading +from contextlib import contextmanager + +__all__ = [ + "soft_delete_context", + "current_soft_delete_user", + "current_soft_delete_reason", +] + +_ctx = threading.local() + + +@contextmanager +def soft_delete_context(user=None, reason=""): + """Ustawia thread-local user/reason na czas soft-delete / restore. + + Receivery czytają to przez ``current_soft_delete_user()`` / + ``current_soft_delete_reason()``. + + REENTRANCJA JEST WYMAGANA, nie „miła": soft-delete publikacji kasuje + najpierw swoje wiersze ``*_Autor`` (wąska kaskada fazy 02), a każdy + z nich wchodzi w ten kontekst ponownie. Zapamiętanie i odtworzenie + poprzednich wartości sprawia, że kaskada dziedziczy usera rodzica, + zamiast go wyzerować w połowie operacji. + + Sprzątanie siedzi w ``finally``, bo ``delete()`` bywa przerywany — + guard fazy 04 rzuca ``ProtectedError`` dla autora z pracami. Kontekst, + który przeciekłby po wyjątku, przypisałby cudzego usera NASTĘPNEJ + operacji w tym wątku (w produkcji: kolejnemu requestowi na tym samym + workerze). Byłby to błąd cichy i nie do wykrycia po fakcie. + """ + prev_user = getattr(_ctx, "user", None) + prev_reason = getattr(_ctx, "reason", "") + _ctx.user = user + _ctx.reason = reason + try: + yield + finally: + _ctx.user = prev_user + _ctx.reason = prev_reason + + +def current_soft_delete_user(): + """User bieżącej operacji soft-delete albo ``None`` poza kontekstem.""" + return getattr(_ctx, "user", None) + + +def current_soft_delete_reason(): + """Powód bieżącej operacji soft-delete albo ``""`` poza kontekstem.""" + return getattr(_ctx, "reason", "") diff --git a/src/bpp/tests/test_soft_delete/test_soft_delete_context.py b/src/bpp/tests/test_soft_delete/test_soft_delete_context.py new file mode 100644 index 000000000..67363bffb --- /dev/null +++ b/src/bpp/tests/test_soft_delete/test_soft_delete_context.py @@ -0,0 +1,59 @@ +"""Kontekst atrybucji usera dla soft-delete (faza 06, Task 1). + +Sygnały pakietu ``django-soft-delete`` niosą wyłącznie ``sender`` +i ``instance`` — usera ani powodu nie przekazują. Ten thread-local jest +kanałem, którym ``delete(user=, reason=)`` dostarcza je receiverowi. +""" + +import pytest + +from bpp.models.soft_delete_context import ( + current_soft_delete_reason, + current_soft_delete_user, + soft_delete_context, +) + + +def test_context_brak_usera_domyslnie(): + assert current_soft_delete_user() is None + assert current_soft_delete_reason() == "" + + +def test_context_ustawia_i_czysci(django_user_model, db): + u = django_user_model.objects.create(username="ktos") + with soft_delete_context(user=u, reason="literówka"): + assert current_soft_delete_user() == u + assert current_soft_delete_reason() == "literówka" + assert current_soft_delete_user() is None + assert current_soft_delete_reason() == "" + + +def test_context_zagniezdzony_przywraca_zewnetrzny(django_user_model, db): + """Reentrancja: kaskada ``*_Autor`` odpala się WEWNĄTRZ kontekstu + rodzica, więc zagnieżdżone wejście nie może zgubić usera zewnętrznego. + """ + a = django_user_model.objects.create(username="a") + b = django_user_model.objects.create(username="b") + with soft_delete_context(user=a, reason="zewn"): + with soft_delete_context(user=b, reason="wewn"): + assert current_soft_delete_user() == b + assert current_soft_delete_reason() == "wewn" + assert current_soft_delete_user() == a + assert current_soft_delete_reason() == "zewn" + assert current_soft_delete_user() is None + + +def test_context_czysci_takze_przy_wyjatku(django_user_model, db): + """``delete()`` może paść (np. ``ProtectedError`` guardu fazy 04). + + Gdyby kontekst przeciekł, NASTĘPNA operacja w tym samym wątku — + w produkcji: kolejny request na tym samym workerze gunicorna — + zalogowałaby cudzego usera. To cichy błąd atrybucji audytu, więc + sprzątanie musi być w ``finally``, nie po ``yield``. + """ + u = django_user_model.objects.create(username="pechowiec") + with pytest.raises(ValueError): + with soft_delete_context(user=u, reason="bum"): + raise ValueError("bum") + assert current_soft_delete_user() is None + assert current_soft_delete_reason() == "" From dca90117b96de10a96317039e2cede66fc65f030 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sun, 16 Aug 2026 22:43:57 +0200 Subject: [PATCH 02/18] feat(soft-delete): model SoftDeleteLog + migracja 0503 Osobny model, a nie django-easy-audit, bo easy-audit zapisuje ZMIANY POL: soft-delete widzi jako "deleted_at: null -> 2026-08-16", bez powodu, bez zwiazku ze zleceniem wycofania z PBN i bez odpowiedzi na "co jeszcze poszlo do kosza ta sama decyzja". SoftDeleteLog odpowiada na pytania operacyjne. GFK, nie FK, bo log musi przezyc rekord: przy HARD_DELETE wiersz publikacji znika fizycznie, a wpis zostaje jedynym sladem, ze istniala. CASCADE skasowalby dowod razem z rekordem, PROTECT zablokowalby samo kasowanie. Oba warunki maja test. Dwa odejscia od litery planu, obie w miejscu, gdzie plan sam sobie przeczyl albo lamal lokalna konwencje: - content_type ma db_index=False -- osobny indeks jest redundantny wobec Meta.indexes Index(content_type, object_id), gdzie content_type jest kolumna wiodaca. Wzorzec z sasiedniego OplatyPublikacjiLog. - Meta.indexes NIE dubluje indeksu na timestamp, ktory pole ma juz przez db_index=True (plan mial oba). Drugi identyczny indeks to czysty koszt zapisu, ponoszony przy KAZDYM wpisie -- takze dla kazdego wiersza *_Autor kasowanego kaskada. Sygnatury pol PINNED zachowane co do joty. AUTH_USER_MODEL przez settings.AUTH_USER_MODEL (konwencja repo), nie przez import modulu settingsow, jak sugerowal plan. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/migrations/0503_softdeletelog.py | 37 +++++++++ src/bpp/models/__init__.py | 1 + src/bpp/models/soft_delete_log.py | 83 +++++++++++++++++++ .../test_soft_delete_log_model.py | 70 ++++++++++++++++ 4 files changed, 191 insertions(+) create mode 100644 src/bpp/migrations/0503_softdeletelog.py create mode 100644 src/bpp/models/soft_delete_log.py create mode 100644 src/bpp/tests/test_soft_delete/test_soft_delete_log_model.py diff --git a/src/bpp/migrations/0503_softdeletelog.py b/src/bpp/migrations/0503_softdeletelog.py new file mode 100644 index 000000000..e34e9cf2f --- /dev/null +++ b/src/bpp/migrations/0503_softdeletelog.py @@ -0,0 +1,37 @@ +# Generated by Django 5.2.16 on 2026-08-16 20:41 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('bpp', '0502_autor_soft_delete'), + ('contenttypes', '0002_remove_content_type_name'), + ('pbn_export_queue', '0011_pbn_export_queue_operacja'), + ] + + operations = [ + migrations.CreateModel( + name='SoftDeleteLog', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('object_id', models.PositiveIntegerField(db_index=True, verbose_name='ID obiektu')), + ('akcja', models.CharField(choices=[('delete', 'Usunięcie (kosz)'), ('restore', 'Przywrócenie'), ('hard_delete', 'Usunięcie trwałe')], db_index=True, max_length=20, verbose_name='Akcja')), + ('timestamp', models.DateTimeField(auto_now_add=True, db_index=True, verbose_name='Data operacji')), + ('powod', models.TextField(blank=True, default='', verbose_name='Powód')), + ('pbn_status', models.CharField(blank=True, default='', max_length=50, verbose_name='Status PBN')), + ('content_type', models.ForeignKey(db_index=False, on_delete=django.db.models.deletion.CASCADE, to='contenttypes.contenttype', verbose_name='Typ rekordu')), + ('pbn_queue_entry', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='pbn_export_queue.pbn_export_queue', verbose_name='Wpis kolejki PBN')), + ('user', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL, verbose_name='Użytkownik')), + ], + options={ + 'verbose_name': 'Log operacji soft-delete', + 'verbose_name_plural': 'Logi operacji soft-delete', + 'ordering': ['-timestamp'], + 'indexes': [models.Index(fields=['content_type', 'object_id'], name='bpp_softdel_content_e5d237_idx')], + }, + ), + ] diff --git a/src/bpp/models/__init__.py b/src/bpp/models/__init__.py index 2116c2bb0..4cad1fa40 100644 --- a/src/bpp/models/__init__.py +++ b/src/bpp/models/__init__.py @@ -52,3 +52,4 @@ from .opi_2012 import * # noqa from .oplaty_log import * # noqa +from .soft_delete_log import * # noqa diff --git a/src/bpp/models/soft_delete_log.py b/src/bpp/models/soft_delete_log.py new file mode 100644 index 000000000..9abe1a643 --- /dev/null +++ b/src/bpp/models/soft_delete_log.py @@ -0,0 +1,83 @@ +"""Dedykowany audyt operacji soft-delete (kto / kiedy / dlaczego / PBN). + +DLACZEGO OSOBNY MODEL, SKORO JEST ``django-easy-audit``: easy-audit +zapisuje ZMIANY PÓL, więc soft-delete widzi jako „``deleted_at``: null → +2026-08-16" — bez powodu, bez związku ze zleceniem wycofania z PBN i bez +odpowiedzi na pytanie „co jeszcze poszło do kosza tą samą decyzją". +Ten model odpowiada na pytania operacyjne: kto usunął, dlaczego, czy +oświadczenia wyjechały z PBN i pod jakim wpisem kolejki. + +DLACZEGO GFK, A NIE FK: log musi przeżyć rekord. Przy ``HARD_DELETE`` +wiersz publikacji znika fizycznie, a wpis zostaje jako jedyny ślad, że +istniała. FK z ``CASCADE`` skasowałby dowód razem z rekordem, a FK +z ``PROTECT`` uniemożliwiłby samo kasowanie. +""" + +from django.conf import settings +from django.contrib.contenttypes.fields import GenericForeignKey +from django.contrib.contenttypes.models import ContentType +from django.db import models + +__all__ = ["SoftDeleteLog"] + + +class SoftDeleteLog(models.Model): + """Wpis audytu pojedynczej operacji soft-delete / restore / hard-delete.""" + + class Akcja(models.TextChoices): + DELETE = "delete", "Usunięcie (kosz)" + RESTORE = "restore", "Przywrócenie" + HARD_DELETE = "hard_delete", "Usunięcie trwałe" + + # db_index=False: redundantny wobec Meta.indexes Index(content_type, + # object_id) — content_type jest tam kolumną wiodącą. Wzorzec jak + # w OplatyPublikacjiLog. + content_type = models.ForeignKey( + ContentType, + on_delete=models.CASCADE, + verbose_name="Typ rekordu", + db_index=False, + ) + object_id = models.PositiveIntegerField(db_index=True, verbose_name="ID obiektu") + content_object = GenericForeignKey("content_type", "object_id") + + akcja = models.CharField( + max_length=20, choices=Akcja.choices, db_index=True, verbose_name="Akcja" + ) + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + null=True, + blank=True, + on_delete=models.SET_NULL, + verbose_name="Użytkownik", + ) + timestamp = models.DateTimeField( + auto_now_add=True, db_index=True, verbose_name="Data operacji" + ) + powod = models.TextField(blank=True, default="", verbose_name="Powód") + + pbn_queue_entry = models.ForeignKey( + "pbn_export_queue.PBN_Export_Queue", + null=True, + blank=True, + on_delete=models.SET_NULL, + verbose_name="Wpis kolejki PBN", + ) + pbn_status = models.CharField( + max_length=50, blank=True, default="", verbose_name="Status PBN" + ) + + class Meta: + verbose_name = "Log operacji soft-delete" + verbose_name_plural = "Logi operacji soft-delete" + ordering = ["-timestamp"] + # Bez osobnego Index(["timestamp"]) — pole ma już db_index=True, + # a drugi identyczny indeks to wyłącznie koszt zapisu przy każdym + # wpisie logu (a wpis powstaje przy KAŻDYM soft-delete, także dla + # każdego wiersza *_Autor kasowanego kaskadą). + indexes = [ + models.Index(fields=["content_type", "object_id"]), + ] + + def __str__(self): + return f"{self.get_akcja_display()}: {self.content_object} ({self.timestamp})" diff --git a/src/bpp/tests/test_soft_delete/test_soft_delete_log_model.py b/src/bpp/tests/test_soft_delete/test_soft_delete_log_model.py new file mode 100644 index 000000000..004f9ebac --- /dev/null +++ b/src/bpp/tests/test_soft_delete/test_soft_delete_log_model.py @@ -0,0 +1,70 @@ +"""Model ``SoftDeleteLog`` — audyt operacji soft-delete (faza 06, Task 2).""" + +import pytest +from django.contrib.contenttypes.models import ContentType +from model_bakery import baker + +from bpp.models.soft_delete_log import SoftDeleteLog + + +@pytest.mark.django_db +def test_softdeletelog_gfk_wskazuje_na_rekord(wydawnictwo_ciagle): + log = SoftDeleteLog.objects.create( + content_type=ContentType.objects.get_for_model(wydawnictwo_ciagle), + object_id=wydawnictwo_ciagle.pk, + akcja=SoftDeleteLog.Akcja.DELETE, + powod="test", + ) + assert log.content_object == wydawnictwo_ciagle + assert log.timestamp is not None + assert log.user is None + assert log.pbn_queue_entry is None + assert log.pbn_status == "" + + +@pytest.mark.django_db +def test_softdeletelog_akcja_choices(): + assert SoftDeleteLog.Akcja.DELETE == "delete" + assert SoftDeleteLog.Akcja.RESTORE == "restore" + assert SoftDeleteLog.Akcja.HARD_DELETE == "hard_delete" + + +@pytest.mark.django_db +def test_softdeletelog_user_set_null(wydawnictwo_ciagle, django_user_model): + """Skasowanie konta NIE MOŻE zabrać ze sobą wpisów audytu. + + ``SET_NULL``, nie ``CASCADE``: log ma przeżyć odejście pracownika — + inaczej usunięcie konta wymazywałoby ślad po jego decyzjach, czyli + dokładnie to, przed czym ten model ma chronić. + """ + u = baker.make(django_user_model) + log = SoftDeleteLog.objects.create( + content_type=ContentType.objects.get_for_model(wydawnictwo_ciagle), + object_id=wydawnictwo_ciagle.pk, + akcja=SoftDeleteLog.Akcja.DELETE, + user=u, + ) + u.delete() + log.refresh_from_db() + assert log.user is None + + +@pytest.mark.django_db +def test_softdeletelog_przezywa_twarde_skasowanie_rekordu(wydawnictwo_ciagle): + """Wpis logu MUSI przetrwać zniknięcie rekordu, którego dotyczy. + + To jest cały powód istnienia GFK zamiast FK: przy ``HARD_DELETE`` + wiersz publikacji znika fizycznie, a log ma zostać jako jedyny ślad, + że kiedykolwiek istniała. FK z ``CASCADE`` skasowałby dowód razem + z dowodem winy; FK z ``PROTECT`` zablokowałby samo kasowanie. + """ + ct = ContentType.objects.get_for_model(wydawnictwo_ciagle) + pk = wydawnictwo_ciagle.pk + log = SoftDeleteLog.objects.create( + content_type=ct, object_id=pk, akcja=SoftDeleteLog.Akcja.HARD_DELETE + ) + wydawnictwo_ciagle.hard_delete() + + log.refresh_from_db() + assert log.object_id == pk + assert log.content_object is None From c0886f68cb5ffad69aa7560e90c0fcd9fc4630ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sun, 16 Aug 2026 22:58:55 +0200 Subject: [PATCH 03/18] feat(soft-delete): receiver post_hard_delete -> log HARD_DELETE Receivery podpiete BEZ sender=, wiec obejmuja kazdy model soft-delete; rejestracja w BppConfig.ready() z dispatch_uid (ready() bywa wolane wielokrotnie, m.in. przy TransactionTestCase -- bez uid-a kazdy soft-delete produkowalby dwa wpisy logu). PK PRZY HARD-DELETE. Plan kazal ustalic to empirycznie, nie zgadywac -- ustalone testem-sonda: SoftDeleteModel.hard_delete() wola Model.delete(), kolektor Django zeruje pk na koniec, a post_hard_delete leci PO tym. Receiver dostaje wiec instancje z pk=None. Naiwny zapis dalby object_id=None, czyli IntegrityError W RECEIVERZE -- audyt przewrocilby operacje, ktora ma tylko obserwowac. Stad BppPkPrzedHardDeleteMixin zapamietujacy pk przed skasowaniem; pk_dla_audytu() czyta go z fallbackiem i rzuca opisowy RuntimeError, gdy modelowi brakuje mixinu. Mixin jest zwykla klasa (nie models.Model), wiec dopisanie go do baz istniejacych modeli NIE generuje migracji -- zweryfikowane makemigrations --check. Test-sonda na kolejnosc zerowania pk zostaje jako straznik: gdy pakiet kiedys ja zmieni, zapali sie tam, a nie w postaci logow z zapasowego pola. STRAZNIK KOMPLETNOSCI ZNALAZL SZOSTY MODEL. Testowa asercja "kazdy SoftDeleteModel ma mixin" wykryla zglos_publikacje.Zgloszenie_Publikacji -- model soft-delete NIEZALEZNY od faz 01-04, uzywajacy pakietu od dawna na wlasne potrzeby. Nie bylo go na zadnej liscie w planie ani w handoffie. Bez mixinu jego hard_delete() wywalilby sie dopiero na produkcji. To dokladnie powtorka lekcji z handoffu 4.2 (faza 04 pominela czwartego dziedzica abstraktu): wyliczanka modelow z glowy jest niepelna, asercja nad apps.get_models() nie jest. Skutek uboczny do odnotowania: operacje na Zgloszenie_Publikacji trafiaja teraz do SoftDeleteLog. Bez skutkow PBN -- gate kolejkowania (Task 6) obejmie wylacznie 5 modeli publikacji. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/apps.py | 7 ++ src/bpp/models/autor.py | 9 +- src/bpp/models/repozytorium.py | 3 +- src/bpp/models/soft_delete.py | 61 ++++++++++++- src/bpp/receivers/__init__.py | 0 src/bpp/receivers/soft_delete.py | 57 +++++++++++++ .../tests/test_soft_delete/test_receivers.py | 85 +++++++++++++++++++ src/zglos_publikacje/models.py | 12 ++- 8 files changed, 229 insertions(+), 5 deletions(-) create mode 100644 src/bpp/receivers/__init__.py create mode 100644 src/bpp/receivers/soft_delete.py create mode 100644 src/bpp/tests/test_soft_delete/test_receivers.py diff --git a/src/bpp/apps.py b/src/bpp/apps.py index 370ab210b..d1f5f9a3b 100644 --- a/src/bpp/apps.py +++ b/src/bpp/apps.py @@ -56,6 +56,13 @@ def ready(self): zainstaluj() + # Receivery soft-delete -> SoftDeleteLog + kolejka PBN (faza 06). + # Podpięte bez `sender=`, czyli obejmują KAŻDY model soft-delete — + # dodanie kolejnego nie wymaga dopisywania nic tutaj. + from bpp.receivers import soft_delete as soft_delete_receivers + + soft_delete_receivers.register() + # Initialize Rollbar with global hostname handler from bpp.rollbar_config import configure_rollbar diff --git a/src/bpp/models/autor.py b/src/bpp/models/autor.py index ed7447b5b..839f2a941 100644 --- a/src/bpp/models/autor.py +++ b/src/bpp/models/autor.py @@ -32,6 +32,7 @@ from bpp.models import LinkDoPBNMixin, ModelZAdnotacjami, ModelZNazwa, NazwaISkrot from bpp.models.abstract import ModelZPBN_ID from bpp.models.soft_delete import ( + BppPkPrzedHardDeleteMixin, BppSoftDeleteQuerySet, dopisz_znacznik_zmiany, raise_if_has_protected_children, @@ -256,7 +257,13 @@ def _relacje_chronione_autora(): ] -class Autor(LinkDoPBNMixin, ModelZAdnotacjami, ModelZPBN_ID, SoftDeleteModel): +class Autor( + BppPkPrzedHardDeleteMixin, + LinkDoPBNMixin, + ModelZAdnotacjami, + ModelZPBN_ID, + SoftDeleteModel, +): url_do_pbn = const.LINK_PBN_DO_AUTORA imiona = models.CharField(max_length=512, db_index=True) diff --git a/src/bpp/models/repozytorium.py b/src/bpp/models/repozytorium.py index 85f3bc110..129ff79b3 100644 --- a/src/bpp/models/repozytorium.py +++ b/src/bpp/models/repozytorium.py @@ -8,6 +8,7 @@ from model_utils import Choices from bpp.const import TRYB_DOSTEPU +from bpp.models.soft_delete import BppPkPrzedHardDeleteMixin def element_repozytorium_upload_to(instance, filename): @@ -15,7 +16,7 @@ def element_repozytorium_upload_to(instance, filename): return f"protected/repozytorium/{uuid.uuid4()}{ext}" -class Element_Repozytorium(SoftDeleteModel): +class Element_Repozytorium(BppPkPrzedHardDeleteMixin, SoftDeleteModel): ER_TRYB_DOSTEPU = Choices( (TRYB_DOSTEPU.NIEJAWNY, "niejawny"), (TRYB_DOSTEPU.TYLKO_W_SIECI, "tylko w sieci"), diff --git a/src/bpp/models/soft_delete.py b/src/bpp/models/soft_delete.py index 1fda922e2..7e10a734b 100644 --- a/src/bpp/models/soft_delete.py +++ b/src/bpp/models/soft_delete.py @@ -228,7 +228,64 @@ def get_queryset(self): ) -class BppAutorstwoSoftDeleteMixin(SoftDeleteModel): +#: Atrybut, pod którym ``hard_delete()`` zostawia ``pk`` na czas sygnału. +ATRYBUT_PK_PRZED_HARD_DELETE = "_bpp_pk_przed_hard_delete" + + +class BppPkPrzedHardDeleteMixin: + """Zapamiętuje ``pk`` przed twardym skasowaniem, dla receivera fazy 06. + + ``SoftDeleteModel.hard_delete()`` woła ``Model.delete()``, a kolektor + Django na samym końcu zeruje ``pk`` skasowanych instancji — + ``post_hard_delete`` leci PO tym (pakiet, ``models.py:84-85``). Receiver + dostaje więc instancję z ``pk is None`` i nie ma z czego zapisać + ``SoftDeleteLog.object_id``. + + Naiwne „zapisz ``instance.pk``" dałoby ``object_id=None``, czyli + ``IntegrityError`` w receiverze — a wyjątek z receivera przewróciłby + ``hard_delete()``. Audyt zepsułby operację, którą ma tylko obserwować. + + NIE jest modelem (zwykła klasa, nie ``models.Model``) — dlatego + dopisanie go do baz istniejącego modelu NIE generuje migracji. Musi + stać PRZED ``SoftDeleteModel`` w liście baz, żeby jego ``hard_delete`` + wygrał w MRO. + + Strażnikiem kompletności jest + ``test_receivers.py::test_kazdy_model_soft_delete_zachowuje_pk`` — + nowy model soft-delete bez tego mixinu zapala się w testach, nie + dopiero przy pierwszym twardym skasowaniu na produkcji. + """ + + def hard_delete(self, *args, **kwargs): + setattr(self, ATRYBUT_PK_PRZED_HARD_DELETE, self.pk) + return super().hard_delete(*args, **kwargs) + + hard_delete.alters_data = True + + +def pk_dla_audytu(instance): + """``pk`` instancji, także po twardym skasowaniu (zerującym ``pk``). + + Rzuca ``RuntimeError``, gdy ``pk`` nie jest znany — to znaczy, że model + soft-delete nie ma ``BppPkPrzedHardDeleteMixin``. Głośno, bo cichy + ``return`` zamieniłby audyt w atrapę dokładnie w tym przypadku, przed + którym ma chronić: rekord znikający fizycznie i bez śladu. + """ + if instance.pk is not None: + return instance.pk + + pk = getattr(instance, ATRYBUT_PK_PRZED_HARD_DELETE, None) + if pk is None: + raise RuntimeError( + f"Nie znam pk dla {instance._meta.label} — model soft-delete bez " + f"BppPkPrzedHardDeleteMixin, więc SoftDeleteLog nie ma czego " + f"zapisać w object_id. Dopisz ten mixin do baz modelu (przed " + f"SoftDeleteModel)." + ) + return pk + + +class BppAutorstwoSoftDeleteMixin(BppPkPrzedHardDeleteMixin, SoftDeleteModel): """SoftDeleteModel + nasze managery dla through-modeli *_Autor. Wpinany w 3 KONKRETNE modele (Wydawnictwo_Ciagle_Autor, @@ -287,7 +344,7 @@ def restore( return super().restore(strict, transaction_id, *args, **kwargs) -class BppPublikacjaSoftDeleteMixin(SoftDeleteModel): +class BppPublikacjaSoftDeleteMixin(BppPkPrzedHardDeleteMixin, SoftDeleteModel): """SoftDeleteModel dla 5 modeli PUBLIKACJI (faza 02) z **wąską, kontrolowaną** kaskadą na własne wiersze ``*_Autor`` pod wspólnym ``transaction_id``. diff --git a/src/bpp/receivers/__init__.py b/src/bpp/receivers/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/bpp/receivers/soft_delete.py b/src/bpp/receivers/soft_delete.py new file mode 100644 index 000000000..2b2d6db5e --- /dev/null +++ b/src/bpp/receivers/soft_delete.py @@ -0,0 +1,57 @@ +"""Receivery sygnałów ``django-soft-delete`` → ``SoftDeleteLog``. + +JEDEN punkt podpięcia dla WSZYSTKICH modeli soft-delete (publikacje, +``*_Autor``, ``Autor``, ``Element_Repozytorium``) — receivery są rejestrowane +bez ``sender=``, więc nowy model soft-delete jest logowany bez dopisywania +czegokolwiek tutaj. Rejestracja: ``BppConfig.ready()``. + +Usera i powód wnosi thread-local ``soft_delete_context`` — sygnały pakietu +ich nie niosą (patrz ``bpp/models/soft_delete_context.py``). +""" + +from django.contrib.contenttypes.models import ContentType + +from bpp.models.soft_delete import pk_dla_audytu +from bpp.models.soft_delete_context import ( + current_soft_delete_reason, + current_soft_delete_user, +) +from bpp.models.soft_delete_log import SoftDeleteLog + + +def _utworz_log(instance, akcja, pbn_queue_entry=None, pbn_status=""): + return SoftDeleteLog.objects.create( + content_type=ContentType.objects.get_for_model(instance), + object_id=pk_dla_audytu(instance), + akcja=akcja, + user=current_soft_delete_user(), + powod=current_soft_delete_reason(), + pbn_queue_entry=pbn_queue_entry, + pbn_status=pbn_status, + ) + + +def on_post_hard_delete(sender, instance, **kwargs): + """Hard-delete: rekord fizycznie znika, więc bez operacji PBN. + + Wycofanie oświadczeń wymaga ``pbn_uid`` odczytanego z rekordu, a tego + już nie ma — dług opisany w handoffie fazy 05a §6. Log powstaje mimo + to i jest wtedy JEDYNYM śladem, że rekord istniał. + """ + _utworz_log(instance, SoftDeleteLog.Akcja.HARD_DELETE) + + +def register(): + """Podłącza receivery. Woła to ``BppConfig.ready()``. + + ``dispatch_uid`` przy każdym połączeniu: ``ready()`` bywa wołane więcej + niż raz (m.in. przy ``TransactionTestCase``), a bez uid-a receiver + zostałby podpięty dwa razy i każdy soft-delete produkowałby dwa wpisy + logu. Import sygnałów jest lokalny, żeby moduł dał się zaimportować + poza kontekstem gotowej aplikacji. + """ + from django_softdelete.signals import post_hard_delete + + post_hard_delete.connect( + on_post_hard_delete, dispatch_uid="bpp.soft_delete.post_hard_delete" + ) diff --git a/src/bpp/tests/test_soft_delete/test_receivers.py b/src/bpp/tests/test_soft_delete/test_receivers.py new file mode 100644 index 000000000..c4c823b20 --- /dev/null +++ b/src/bpp/tests/test_soft_delete/test_receivers.py @@ -0,0 +1,85 @@ +"""Receivery sygnałów soft-delete → ``SoftDeleteLog`` (faza 06).""" + +import pytest +from django.contrib.contenttypes.models import ContentType +from django_softdelete.signals import post_hard_delete + +from bpp.models.soft_delete_context import soft_delete_context +from bpp.models.soft_delete_log import SoftDeleteLog + + +def _logi(instance, akcja): + return SoftDeleteLog.objects.filter( + content_type=ContentType.objects.get_for_model(instance), + object_id=instance.pk, + akcja=akcja, + ) + + +@pytest.mark.django_db +def test_pakiet_zeruje_pk_przed_wyslaniem_post_hard_delete(wydawnictwo_ciagle): + """Fakt o pakiecie, na którym opiera się cała obsługa HARD_DELETE. + + ``SoftDeleteModel.hard_delete()`` woła ``Model.delete()``, a kolektor + Django na koniec zeruje ``pk`` skasowanych instancji — ``post_hard_delete`` + leci PO tym. Receiver nie ma więc z czego wziąć ``object_id`` i musi + dostać ``pk`` zapamiętany wcześniej. + + Ten test jest strażnikiem założenia: gdyby aktualizacja pakietu + przestawiła kolejność (sygnał przed zerowaniem), zapali się tutaj, + a nie w postaci logów z ``object_id`` wziętym z zapasowego pola. + """ + zebrane = [] + + def _sonda(sender, instance, **kwargs): + zebrane.append(instance.pk) + + post_hard_delete.connect(_sonda, dispatch_uid="test.sonda.pk", weak=False) + try: + wydawnictwo_ciagle.hard_delete() + finally: + post_hard_delete.disconnect(dispatch_uid="test.sonda.pk") + + assert zebrane == [None], ( + "Pakiet zaczął wysyłać post_hard_delete z niewyzerowanym pk — " + "stash pk w BppPkPrzedHardDeleteMixin można wtedy uprościć." + ) + + +def test_kazdy_model_soft_delete_zachowuje_pk(): + """KAŻDY model soft-delete musi mieć ``BppPkPrzedHardDeleteMixin``. + + Bez niego ``post_hard_delete`` dostaje instancję z ``pk is None``, + a ``pk_dla_audytu()`` rzuca ``RuntimeError`` — czyli twarde skasowanie + takiego modelu przewróciłoby się w produkcji. Ten test przenosi wykrycie + z produkcji do testów: nowy model soft-delete zapala się TUTAJ. + """ + from django.apps import apps + from django_softdelete.models import SoftDeleteModel + + from bpp.models.soft_delete import BppPkPrzedHardDeleteMixin + + bez_mixinu = [ + model._meta.label + for model in apps.get_models() + if issubclass(model, SoftDeleteModel) + and not issubclass(model, BppPkPrzedHardDeleteMixin) + ] + assert bez_mixinu == [], ( + "Modele soft-delete bez BppPkPrzedHardDeleteMixin — ich hard_delete() " + f"wywali sie na pk_dla_audytu(): {bez_mixinu}" + ) + + +@pytest.mark.django_db +def test_hard_delete_tworzy_log(wydawnictwo_ciagle, superuser): + pk = wydawnictwo_ciagle.pk + ct = ContentType.objects.get_for_model(wydawnictwo_ciagle) + with soft_delete_context(user=superuser, reason="trwałe"): + wydawnictwo_ciagle.hard_delete() + log = SoftDeleteLog.objects.get( + content_type=ct, object_id=pk, akcja=SoftDeleteLog.Akcja.HARD_DELETE + ) + assert log.user == superuser + assert log.powod == "trwałe" + assert log.pbn_queue_entry is None diff --git a/src/zglos_publikacje/models.py b/src/zglos_publikacje/models.py index ab1fe39dd..5e61fc3ad 100644 --- a/src/zglos_publikacje/models.py +++ b/src/zglos_publikacje/models.py @@ -17,6 +17,7 @@ ModelZOplataZaPublikacje, ModelZRokiem, ) +from bpp.models.soft_delete import BppPkPrzedHardDeleteMixin from bpp.models.wydawca import Wydawca from bpp.models.wydawnictwo_zwarte import Wydawnictwo_Zwarte from pbn_api.models.publication import Publication as PBN_Publication @@ -58,7 +59,16 @@ def zgloszenie_publikacji_upload_to(instance, filename): class Zgloszenie_Publikacji( - ModelZRokiem, DwaTytuly, ModelZDOI, ModelZOplataZaPublikacje, SoftDeleteModel + # BppPkPrzedHardDeleteMixin: ten model jest soft-delete NIEZALEŻNIE od + # faz 01-04 (używa pakietu od dawna, na własne potrzeby), więc receivery + # fazy 06 obejmują go razem z resztą — a wtedy jego hard_delete() + # potrzebuje zachowanego pk. Patrz docstring mixinu. + BppPkPrzedHardDeleteMixin, + ModelZRokiem, + DwaTytuly, + ModelZDOI, + ModelZOplataZaPublikacje, + SoftDeleteModel, ): email = models.EmailField("E-mail zgłaszającego") From 252ea4dc057a071e12f572ccec1a46f12b63a084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sun, 16 Aug 2026 23:11:14 +0200 Subject: [PATCH 04/18] feat(soft-delete): receivery DELETE/RESTORE + realne wpiecie atrybucji Fazy 02/04 przyjmowaly user=/reason= i je PORZUCALY ("konsumuje je SoftDeleteLog z fazy 06"). Plan zakladal, ze owijaja super().delete() w soft_delete_context -- w rzeczywistosci nie wolaja super().delete() wcale (swiadomie, zeby uniknac refleksyjnej kaskady pakietu), tylko same ustawiaja deleted_at i same wysylaja sygnal. Faza 06 domyka wiec obietnice: kontekst zakladany jest w 4 metodach (publikacje delete/restore, Autor delete/restore). Bez tego mechanizm atrybucji dzialalby wylacznie dla wolajacych, ktorzy sami weszliby w context manager -- czyli dla nikogo. Kontekst obejmuje CALE cialo delete(), nie samo post_soft_delete.send(): kaskada na *_Autor wysyla wlasne sygnaly, ktore maja byc zalogowane z tym samym userem i powodem. POMINIETY ARGUMENT DZIEDZICZY, NIE ZERUJE -- regula wymuszona przez test. Plan przewidywal dwa sposoby atrybucji, ktore po wpieciu okazaly sie wzajemnie sprzeczne: delete() zaklada kontekst ZAWSZE, wiec wywolanie bez user= wewnatrz jawnego soft_delete_context(user=X) zerowalo X i operacja trafiala do logu jako niczyja (test z planu padal na assert None == user). Regula siedzi w samym context managerze, nie w czterech miejscach wywolan -- inaczej nastepny model soft-delete zgubilby usera po cichu. Uzasadnienie: opakowanie, ktore nie wnosi informacji, nie ma prawa jej niszczyc; None znaczy "nie wiem", a nie "wiem, ze nikt". Log jest wierny sygnalom: soft-delete publikacji z N autorami daje 1+N wpisow (decyzja wlasciciela). Test pilnuje, ze wiersze *_Autor dziedzicza usera i powod przez reentrancje. 156 passed w src/bpp/tests/test_soft_delete/ po wpieciu. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/models/autor.py | 15 ++-- src/bpp/models/soft_delete.py | 16 ++-- src/bpp/models/soft_delete_context.py | 13 ++- src/bpp/receivers/soft_delete.py | 18 +++- .../tests/test_soft_delete/test_receivers.py | 82 +++++++++++++++++++ .../test_soft_delete_context.py | 21 +++++ 6 files changed, 152 insertions(+), 13 deletions(-) diff --git a/src/bpp/models/autor.py b/src/bpp/models/autor.py index 839f2a941..dca48520c 100644 --- a/src/bpp/models/autor.py +++ b/src/bpp/models/autor.py @@ -37,6 +37,7 @@ dopisz_znacznik_zmiany, raise_if_has_protected_children, ) +from bpp.models.soft_delete_context import soft_delete_context from bpp.util import FulltextSearchMixin, zaloguj_polkniety_wyjatek logger = logging.getLogger(__name__) @@ -603,9 +604,13 @@ def delete(self, *args, user=None, reason="", **kwargs): autorstw (spec §1, §10.1). Autor z autorstwami i tak tu nie dojdzie — zatrzyma go guard. - ``user``/``reason`` są tylko przepuszczane; konsumuje je - ``SoftDeleteLog`` z fazy 06. W sygnaturze muszą być już teraz - (kontrakt PINNED). + ``user``/``reason`` trafiają do ``SoftDeleteLog`` (faza 06) przez + thread-local ``soft_delete_context``. + + Kontekst zakładamy PO guardzie, nie przed: gdy guard rzuci + ``ProtectedError``, żaden sygnał nie poleci, więc nie ma czego + atrybuować. (Poprawność nie zależy od tej kolejności — context + manager sprząta w ``finally`` — ale węższy zakres jest uczciwszy.) """ raise_if_has_protected_children( self, @@ -614,7 +619,7 @@ def delete(self, *args, user=None, reason="", **kwargs): ) txid = kwargs.pop("transaction_id", None) or uuid.uuid4() - with transaction.atomic(): + with soft_delete_context(user=user, reason=reason), transaction.atomic(): self.deleted_at = timezone.now() self.restored_at = None self.transaction_id = txid @@ -639,7 +644,7 @@ def restore(self, *args, strict: bool = False, user=None, **kwargs): nie zabrało, więc nie ma czego wskrzeszać. """ txid = self.transaction_id - with transaction.atomic(): + with soft_delete_context(user=user), transaction.atomic(): self.deleted_at = None self.restored_at = timezone.now() self.transaction_id = None diff --git a/src/bpp/models/soft_delete.py b/src/bpp/models/soft_delete.py index 7e10a734b..e82c41c69 100644 --- a/src/bpp/models/soft_delete.py +++ b/src/bpp/models/soft_delete.py @@ -53,6 +53,8 @@ from django_softdelete.models import SoftDeleteModel from django_softdelete.signals import post_restore, post_soft_delete +from bpp.models.soft_delete_context import soft_delete_context + #: Akcesor relacji odwrotnej publikacja -> wiersze ``*_Autor``. Istnieje #: jako PRAWDZIWA relacja tylko dla trzech typów z through-modelem; #: ``Praca_Doktorska``/``Praca_Habilitacyjna`` mają pod tą nazwą property @@ -441,12 +443,16 @@ def save(self, *args, **kwargs): def delete(self, *args, user=None, reason="", **kwargs): """Soft-delete publikacji + wąska kaskada na ``*_Autor``. - ``user``/``reason`` są na razie wyłącznie przepuszczane — konsumuje - je ``SoftDeleteLog`` z fazy 06. W sygnaturze MUSZĄ być już teraz - (kontrakt PINNED), żeby wołający kod nie wymagał później zmiany. + ``user``/``reason`` trafiają do ``SoftDeleteLog`` (faza 06) przez + thread-local ``soft_delete_context``. Kontekst obejmuje CAŁE ciało, + nie samo ``post_soft_delete.send()``: kaskada na ``*_Autor`` wysyła + własne sygnały (przez ``delete()`` pakietu), a te mają zostać + zalogowane z tym samym userem i powodem. Zawężenie kontekstu do + ostatniej linii dałoby wpisy autorstw z ``user=None`` mimo + świadomej decyzji operatora. """ txid = kwargs.pop("transaction_id", None) or uuid.uuid4() - with transaction.atomic(): + with soft_delete_context(user=user, reason=reason), transaction.atomic(): # 1. kaskada per-instancja — NIGDY bulk update(deleted_at=...), # bo omijałby post_save, sygnały i reversion (gate w # BppSoftDeleteQuerySet.update() egzekwuje to fail-fast). @@ -477,7 +483,7 @@ def restore(self, *args, strict: bool = False, user=None, **kwargs): """ txid = self.transaction_id rel = self._relacja_autorstw() - with transaction.atomic(): + with soft_delete_context(user=user), transaction.atomic(): if txid is not None and rel is not None: # Nazwa pola FK z metadanych relacji — bez zaszywania # "rekord" na sztywno. diff --git a/src/bpp/models/soft_delete_context.py b/src/bpp/models/soft_delete_context.py index 0e2d3312e..a12016409 100644 --- a/src/bpp/models/soft_delete_context.py +++ b/src/bpp/models/soft_delete_context.py @@ -49,11 +49,20 @@ def soft_delete_context(user=None, reason=""): który przeciekłby po wyjątku, przypisałby cudzego usera NASTĘPNEJ operacji w tym wątku (w produkcji: kolejnemu requestowi na tym samym workerze). Byłby to błąd cichy i nie do wykrycia po fakcie. + + POMINIĘTY ARGUMENT DZIEDZICZY, NIE ZERUJE. Wejście bez ``user`` + zachowuje usera z kontekstu zewnętrznego. Bez tej reguły oba + przewidziane sposoby atrybucji wykluczałyby się nawzajem: ``delete()`` + (fazy 02/04) zakłada ten kontekst ZAWSZE, więc wywołanie bez ``user=`` + wewnątrz jawnego ``soft_delete_context(user=X)`` wyzerowałoby X i + operacja trafiłaby do logu jako niczyja. Opakowanie, które nie wnosi + informacji, nie ma prawa jej niszczyć — ``None`` znaczy tu „nie wiem", + a nie „wiem, że nikt". """ prev_user = getattr(_ctx, "user", None) prev_reason = getattr(_ctx, "reason", "") - _ctx.user = user - _ctx.reason = reason + _ctx.user = user if user is not None else prev_user + _ctx.reason = reason if reason else prev_reason try: yield finally: diff --git a/src/bpp/receivers/soft_delete.py b/src/bpp/receivers/soft_delete.py index 2b2d6db5e..d8fc054e1 100644 --- a/src/bpp/receivers/soft_delete.py +++ b/src/bpp/receivers/soft_delete.py @@ -31,6 +31,14 @@ def _utworz_log(instance, akcja, pbn_queue_entry=None, pbn_status=""): ) +def on_post_soft_delete(sender, instance, **kwargs): + _utworz_log(instance, SoftDeleteLog.Akcja.DELETE) + + +def on_post_restore(sender, instance, **kwargs): + _utworz_log(instance, SoftDeleteLog.Akcja.RESTORE) + + def on_post_hard_delete(sender, instance, **kwargs): """Hard-delete: rekord fizycznie znika, więc bez operacji PBN. @@ -50,8 +58,16 @@ def register(): logu. Import sygnałów jest lokalny, żeby moduł dał się zaimportować poza kontekstem gotowej aplikacji. """ - from django_softdelete.signals import post_hard_delete + from django_softdelete.signals import ( + post_hard_delete, + post_restore, + post_soft_delete, + ) + post_soft_delete.connect( + on_post_soft_delete, dispatch_uid="bpp.soft_delete.post_soft_delete" + ) + post_restore.connect(on_post_restore, dispatch_uid="bpp.soft_delete.post_restore") post_hard_delete.connect( on_post_hard_delete, dispatch_uid="bpp.soft_delete.post_hard_delete" ) diff --git a/src/bpp/tests/test_soft_delete/test_receivers.py b/src/bpp/tests/test_soft_delete/test_receivers.py index c4c823b20..fde96ceea 100644 --- a/src/bpp/tests/test_soft_delete/test_receivers.py +++ b/src/bpp/tests/test_soft_delete/test_receivers.py @@ -83,3 +83,85 @@ def test_hard_delete_tworzy_log(wydawnictwo_ciagle, superuser): assert log.user == superuser assert log.powod == "trwałe" assert log.pbn_queue_entry is None + + +@pytest.mark.django_db +def test_soft_delete_tworzy_log_z_userem(wydawnictwo_ciagle, superuser): + """Wariant z jawnym kontekstem — tak woła kod bez własnego ``delete()``.""" + with soft_delete_context(user=superuser, reason="duplikat"): + wydawnictwo_ciagle.delete() + log = _logi(wydawnictwo_ciagle, SoftDeleteLog.Akcja.DELETE).get() + assert log.user == superuser + assert log.powod == "duplikat" + + +@pytest.mark.django_db +def test_soft_delete_bez_usera_loguje_none(wydawnictwo_ciagle): + """Operacja bez zalogowanego usera (celery, skrypt, scalanie duplikatów). + + ``user=None`` jest POPRAWNYM wynikiem, nie awarią — „nie wiadomo kto" + jest uczciwsze niż podstawienie pierwszego lepszego konta. + """ + wydawnictwo_ciagle.delete() + log = _logi(wydawnictwo_ciagle, SoftDeleteLog.Akcja.DELETE).get() + assert log.user is None + assert log.powod == "" + + +@pytest.mark.django_db +def test_delete_z_argumentem_user_loguje_usera(wydawnictwo_ciagle, superuser): + """REALNE API: ``delete(user=, reason=)``, bez ręcznego kontekstu. + + Fazy 02/04 przyjmowały te argumenty i je porzucały („konsumuje je + SoftDeleteLog z fazy 06"). Ten test pilnuje, że faza 06 domknęła + obietnicę — bez niego cały mechanizm atrybucji działałby wyłącznie dla + wołających, którzy sami wejdą w context manager, czyli dla nikogo. + """ + wydawnictwo_ciagle.delete(user=superuser, reason="zdublowany import") + log = _logi(wydawnictwo_ciagle, SoftDeleteLog.Akcja.DELETE).get() + assert log.user == superuser + assert log.powod == "zdublowany import" + + +@pytest.mark.django_db +def test_restore_z_argumentem_user_loguje_usera(wydawnictwo_ciagle, superuser): + wydawnictwo_ciagle.delete(user=superuser, reason="pomyłka") + wydawnictwo_ciagle.restore(user=superuser) + log = _logi(wydawnictwo_ciagle, SoftDeleteLog.Akcja.RESTORE).get() + assert log.user == superuser + + +@pytest.mark.django_db +def test_autor_delete_z_userem_loguje_usera(autor_jan_kowalski, superuser): + """``Autor`` ma własny ``delete()`` (faza 04) — osobna ścieżka do wpięcia.""" + autor_jan_kowalski.delete(user=superuser, reason="duplikat osoby") + log = _logi(autor_jan_kowalski, SoftDeleteLog.Akcja.DELETE).get() + assert log.user == superuser + assert log.powod == "duplikat osoby" + + +@pytest.mark.django_db +def test_kaskada_autorstw_dziedziczy_usera(wydawnictwo_ciagle_z_autorem, superuser): + """Kaskada ``*_Autor`` loguje się z tym samym userem i powodem. + + Soft-delete publikacji z N autorami emituje 1 + N sygnałów, bo wąska + kaskada fazy 02 kasuje każdy wiersz osobno. Log jest wierny sygnałom: + powstaje wpis dla rekordu i dla każdego autorstwa. Atrybucja przenosi + się przez reentrancję context managera — gdyby zawiodła, wiersze + ``*_Autor`` miałyby ``user=None`` mimo świadomej decyzji operatora. + """ + from bpp.models import Wydawnictwo_Ciagle_Autor + + autorstwo = wydawnictwo_ciagle_z_autorem.autorzy_set.get() + wydawnictwo_ciagle_z_autorem.delete(user=superuser, reason="wycofany artykuł") + + log_rekordu = _logi(wydawnictwo_ciagle_z_autorem, SoftDeleteLog.Akcja.DELETE).get() + assert log_rekordu.user == superuser + + log_autorstwa = SoftDeleteLog.objects.get( + content_type=ContentType.objects.get_for_model(Wydawnictwo_Ciagle_Autor), + object_id=autorstwo.pk, + akcja=SoftDeleteLog.Akcja.DELETE, + ) + assert log_autorstwa.user == superuser + assert log_autorstwa.powod == "wycofany artykuł" diff --git a/src/bpp/tests/test_soft_delete/test_soft_delete_context.py b/src/bpp/tests/test_soft_delete/test_soft_delete_context.py index 67363bffb..c889082cf 100644 --- a/src/bpp/tests/test_soft_delete/test_soft_delete_context.py +++ b/src/bpp/tests/test_soft_delete/test_soft_delete_context.py @@ -43,6 +43,27 @@ def test_context_zagniezdzony_przywraca_zewnetrzny(django_user_model, db): assert current_soft_delete_user() is None +def test_context_pominiety_argument_dziedziczy_zewnetrzny(django_user_model, db): + """Wejście bez ``user`` NIE zeruje atrybucji z zewnątrz. + + ``delete()`` faz 02/04 zakłada ten kontekst zawsze — także gdy wołający + nie podał ``user=``. Gdyby brak argumentu zerował, wzorzec + ``with soft_delete_context(user=X): obj.delete()`` dawałby log + z ``user=None``, czyli oba przewidziane sposoby atrybucji wykluczałyby + się nawzajem. + """ + u = django_user_model.objects.create(username="zewnetrzny") + with soft_delete_context(user=u, reason="powod zewnetrzny"): + with soft_delete_context(): + assert current_soft_delete_user() == u + assert current_soft_delete_reason() == "powod zewnetrzny" + + # Jawnie podany argument nadal wygrywa nad odziedziczonym. + with soft_delete_context(reason="powod wewnetrzny"): + assert current_soft_delete_user() == u + assert current_soft_delete_reason() == "powod wewnetrzny" + + def test_context_czysci_takze_przy_wyjatku(django_user_model, db): """``delete()`` może paść (np. ``ProtectedError`` guardu fazy 04). From d0c53a76e6fe38535b0fa495e4b8e23ecb5bcb33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sun, 16 Aug 2026 23:22:10 +0200 Subject: [PATCH 05/18] feat(soft-delete): receivery kolejkuja PBN (WYCOFANIE/WYSYLKA) Task 5 planu POMINIETY zgodnie z handoffem -- zakolejkuj_wycofanie/wysylke juz istnieja z fazy 05a, a shim z planu tworzyl wpisy golym objects.create(), wiec omijalby sprobuj_utowrzyc_wpis (TOCTOU, uczelnia, operacja). GATE PO TYPIE, NIE PO pbn_uid -- plan opieral sie tu na nieprawdzie. Twierdzil (jako fakt zweryfikowany), ze "Autor i *_Autor nie maja pbn_uid", i proponowal uniwersalny getattr(instance, "pbn_uid_id", None). Autor.pbn_uid ISTNIEJE (FK do pbn_api.Scientist, autor.py), wiec ten gate wstawilby AUTORA do kolejki eksportu PUBLIKACJI i odpalil dla niego wysylke do PBN. Ma na to test. Sam pbn_uid nie wystarczylby takze od drugiej strony: RESTORE wola zakolejkuj_wysylke, ktora swiadomie NIE MA gate'u na pbn_uid (faza 05a), wiec przyjelaby kazdy wiersz *_Autor przywracany kaskada -- publikacja z N autorami dawalaby N zbednych zlecen. Gate isinstance( BppPublikacjaSoftDeleteMixin) zamyka oba przypadki naraz; test liczy wpisy. UCZELNIA WYPROWADZANA Z REKORDU (decyzja wlasciciela). Receiver nie ma requestu, z ktorego wszyscy pozostali wolajacy biora tenanta (Uczelnia.objects.get_for_request), a bez uczelni _pozyskaj_klienta_pbn() spada na "jedyna-albo-glosny-blad" i w multi-hosted wycofanie konczy sie FINISHED_ERROR. uczelnia_rekordu() ODWRACA regule przynaleznosci z fazy 05b (naleza_wydawnictwa / naleza_prace), zamiast definiowac druga, konkurencyjna; odwracamy zamiast wolac wprost, bo cerif_export zalezy od bpp, nie odwrotnie. global_objects w tym odczycie jest KONIECZNE, nie ostrozonosciowe: waska kaskada fazy 02 kasuje wiersze *_Autor PRZED wyslaniem post_soft_delete rodzica, wiec objects zwrocilby pustke i uczelnia wychodzilaby None przy KAZDYM kasowaniu. Potwierdzone mutacja: podmiana na objects wywala dokladnie dwa testy, ktore tego pilnuja. Dwuznacznosc (praca wspolautorska miedzy uczelniami) -> None, nie zgadywanie: "pierwsza z brzegu" wyslalaby wycofanie przez konto PBN cudzego tenanta. Odnotowane ograniczenie: pbn_status="" nie odroznia "nie bylo czego kolejkowac" od "pominieto, bo rekord juz czeka w kolejce" -- obie funkcje kolejkujace zwracaja None i receiver nie ma ich jak rozroznic. Udokumentowane testem test_restore_przy_niezakonczonym_wycofaniu_nie_dubluje_wpisu. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/models/soft_delete.py | 50 ++++ src/bpp/receivers/soft_delete.py | 63 ++++- .../tests/test_soft_delete/test_receivers.py | 253 ++++++++++++++++++ 3 files changed, 363 insertions(+), 3 deletions(-) diff --git a/src/bpp/models/soft_delete.py b/src/bpp/models/soft_delete.py index e82c41c69..ac22a2ba5 100644 --- a/src/bpp/models/soft_delete.py +++ b/src/bpp/models/soft_delete.py @@ -421,6 +421,56 @@ def _autorstwa_do_kaskady(self): return [] return list(getattr(self, NAZWA_RELACJI_AUTORSTW).all()) + # --- atrybucja tenanta ---------------------------------------------- + + def uczelnia_rekordu(self): + """Uczelnia, do której należy rekord, albo ``None`` przy dwuznaczności. + + PO CO: wpisy ``PBN_Export_Queue`` tworzone przez receivery fazy 06 + nie mają requestu, z którego reszta kodu bierze tenanta + (``Uczelnia.objects.get_for_request``). Bez uczelni + ``_pozyskaj_klienta_pbn()`` spada na „jedyna-albo-głośny-błąd", więc + w multi-hosted wycofanie oświadczeń kończyłoby się + ``FINISHED_ERROR``. + + REGUŁA JEST ODWRÓCENIEM PRZYNALEŻNOŚCI Z FAZY 05b — ``naleza_ + wydawnictwa`` / ``naleza_prace`` (``cerif_export/providers/ + publikacje.py``). Nie definiujemy drugiej, konkurencyjnej: gdy + tamta się zmieni, ta musi pójść za nią. Odwracamy, zamiast wołać + wprost, bo ``cerif_export`` zależy od ``bpp``, nie odwrotnie. + + ``global_objects`` JEST KONIECZNE, nie ostrożnościowe: wąska + kaskada fazy 02 kasuje wiersze ``*_Autor`` PRZED wysłaniem + ``post_soft_delete`` rodzica. W momencie odczytu autorstwa są już + w koszu, więc ``objects`` zwróciłoby pustkę i uczelnia wychodziłaby + ``None`` przy każdym kasowaniu — czyli zawsze wtedy, kiedy jest + potrzebna. + + DWUZNACZNOŚĆ → ``None``. Praca współautorska między uczelniami nie + ma jednego właściciela, a wybranie „pierwszej z brzegu" wysłałoby + wycofanie przez konto PBN cudzego tenanta. ``None`` degraduje do + zachowania sprzed tej fazy: jedna uczelnia w bazie działa, kilka + daje głośny błąd na wpisie kolejki. + """ + from bpp.models.uczelnia import Uczelnia + + rel = self._relacja_autorstw() + if rel is None: + # Praca_Doktorska / Praca_Habilitacyjna — autor i jednostka + # siedzą na wierszu samej pracy (odpowiednik ``naleza_prace``). + uczelnie = {self.jednostka.uczelnia_id} + else: + uczelnie = set( + rel.related_model.global_objects.filter( + **{rel.field.name: self} + ).values_list("jednostka__uczelnia_id", flat=True) + ) + + uczelnie.discard(None) + if len(uczelnie) != 1: + return None + return Uczelnia.objects.filter(pk=uczelnie.pop()).first() + # --- kontrakt zapisu ------------------------------------------------ def save(self, *args, **kwargs): diff --git a/src/bpp/receivers/soft_delete.py b/src/bpp/receivers/soft_delete.py index d8fc054e1..356cdc090 100644 --- a/src/bpp/receivers/soft_delete.py +++ b/src/bpp/receivers/soft_delete.py @@ -11,13 +11,52 @@ from django.contrib.contenttypes.models import ContentType -from bpp.models.soft_delete import pk_dla_audytu +from bpp.models.soft_delete import BppPublikacjaSoftDeleteMixin, pk_dla_audytu from bpp.models.soft_delete_context import ( current_soft_delete_reason, current_soft_delete_user, ) from bpp.models.soft_delete_log import SoftDeleteLog +#: Wartości ``SoftDeleteLog.pbn_status`` (kontrakt PINNED planu fazy 06). +#: To OSOBNY słownik od ``PBN_Export_Queue.Operacja`` (``"wysylka"`` / +#: ``"wycofanie"``) — tamten opisuje wpis kolejki, ten opisuje, co receiver +#: z tym wpisem zrobił. +STATUS_WYCOFANIE = "WYCOFANIE" +STATUS_WYSYLKA = "WYSYLKA" + + +def _kolejkuj_pbn(instance, zakolejkuj, status): + """Zleca operację PBN dla PUBLIKACJI. Zwraca ``(wpis, status)``. + + GATE IDZIE PO TYPIE, nie po obecności ``pbn_uid``. Plan fazy 06 + proponował uniwersalne ``getattr(instance, "pbn_uid_id", None)``, + opierając się na twierdzeniu, że „``Autor`` i ``*_Autor`` nie mają + ``pbn_uid``". ``Autor.pbn_uid`` jednak ISTNIEJE (FK do + ``pbn_api.Scientist``), więc tamten gate wstawiłby autora do kolejki + eksportu PUBLIKACJI i odpalił dla niego wysyłkę. + + Sam ``pbn_uid`` nie wystarczyłby też od drugiej strony: przy RESTORE + wołamy ``zakolejkuj_wysylke``, która świadomie NIE MA gate'u na + ``pbn_uid`` (faza 05a) — przyjęłaby więc każdy wiersz ``*_Autor`` + przywracany kaskadą i publikacja z N autorami dałaby N zbędnych zleceń. + + ``uczelnia`` wyprowadzana z rekordu, bo receiver nie ma requestu — + patrz ``BppPublikacjaSoftDeleteMixin.uczelnia_rekordu()``. + """ + if not isinstance(instance, BppPublikacjaSoftDeleteMixin): + return None, "" + + wpis = zakolejkuj( + instance, + user=current_soft_delete_user(), + uczelnia=instance.uczelnia_rekordu(), + ) + # ``None`` znaczy „gate niespełniony ALBO rekord już czeka w kolejce" + # (idempotencja ``sprobuj_utowrzyc_wpis``). Obu przypadków nie da się + # tu rozróżnić — obie funkcje kolejkujące zwracają ``None``. + return wpis, (status if wpis is not None else "") + def _utworz_log(instance, akcja, pbn_queue_entry=None, pbn_status=""): return SoftDeleteLog.objects.create( @@ -32,11 +71,29 @@ def _utworz_log(instance, akcja, pbn_queue_entry=None, pbn_status=""): def on_post_soft_delete(sender, instance, **kwargs): - _utworz_log(instance, SoftDeleteLog.Akcja.DELETE) + """Kosz → zlecenie wycofania oświadczeń z PBN + wpis audytu.""" + from pbn_export_queue.operacje import zakolejkuj_wycofanie + + wpis, status = _kolejkuj_pbn(instance, zakolejkuj_wycofanie, STATUS_WYCOFANIE) + _utworz_log( + instance, + SoftDeleteLog.Akcja.DELETE, + pbn_queue_entry=wpis, + pbn_status=status, + ) def on_post_restore(sender, instance, **kwargs): - _utworz_log(instance, SoftDeleteLog.Akcja.RESTORE) + """Przywrócenie → zlecenie ponownej wysyłki do PBN + wpis audytu.""" + from pbn_export_queue.operacje import zakolejkuj_wysylke + + wpis, status = _kolejkuj_pbn(instance, zakolejkuj_wysylke, STATUS_WYSYLKA) + _utworz_log( + instance, + SoftDeleteLog.Akcja.RESTORE, + pbn_queue_entry=wpis, + pbn_status=status, + ) def on_post_hard_delete(sender, instance, **kwargs): diff --git a/src/bpp/tests/test_soft_delete/test_receivers.py b/src/bpp/tests/test_soft_delete/test_receivers.py index fde96ceea..8a40cdfc8 100644 --- a/src/bpp/tests/test_soft_delete/test_receivers.py +++ b/src/bpp/tests/test_soft_delete/test_receivers.py @@ -1,13 +1,35 @@ """Receivery sygnałów soft-delete → ``SoftDeleteLog`` (faza 06).""" +from unittest.mock import patch + import pytest from django.contrib.contenttypes.models import ContentType +from django.utils import timezone from django_softdelete.signals import post_hard_delete +from model_bakery import baker from bpp.models.soft_delete_context import soft_delete_context from bpp.models.soft_delete_log import SoftDeleteLog +@pytest.fixture +def bez_celery(): + """Kolejkowanie odpala ``task_sprobuj_wyslac_do_pbn.delay()``, a testy + biegną z ``CELERY_TASK_ALWAYS_EAGER`` — bez tego wysyłka do PBN + wykonałaby się synchronicznie, w środku ``delete()``. + """ + with patch("pbn_export_queue.tasks.task_sprobuj_wyslac_do_pbn") as mock_task: + yield mock_task + + +def _z_pbn_uid(rekord): + from pbn_api.models import Publication + + rekord.pbn_uid = baker.make(Publication) + rekord.save() + return rekord + + def _logi(instance, akcja): return SoftDeleteLog.objects.filter( content_type=ContentType.objects.get_for_model(instance), @@ -165,3 +187,234 @@ def test_kaskada_autorstw_dziedziczy_usera(wydawnictwo_ciagle_z_autorem, superus ) assert log_autorstwa.user == superuser assert log_autorstwa.powod == "wycofany artykuł" + + +# --- Task 6: kolejkowanie PBN z receiverów ----------------------------- + + +@pytest.mark.django_db +def test_soft_delete_z_pbn_uid_kolejkuje_wycofanie( + wydawnictwo_ciagle, superuser, bez_celery +): + from pbn_export_queue.models import PBN_Export_Queue + + _z_pbn_uid(wydawnictwo_ciagle) + wydawnictwo_ciagle.delete(user=superuser, reason="x") + + log = _logi(wydawnictwo_ciagle, SoftDeleteLog.Akcja.DELETE).get() + assert log.pbn_queue_entry is not None + assert log.pbn_status == "WYCOFANIE" + assert log.pbn_queue_entry.operacja == PBN_Export_Queue.Operacja.WYCOFANIE + assert log.pbn_queue_entry.zamowil == superuser + + +@pytest.mark.django_db +def test_soft_delete_bez_pbn_uid_nie_kolejkuje(wydawnictwo_ciagle, superuser): + """Bez PBN UID nic do PBN nie pojechało, więc nie ma czego wycofywać.""" + wydawnictwo_ciagle.delete(user=superuser) + + log = _logi(wydawnictwo_ciagle, SoftDeleteLog.Akcja.DELETE).get() + assert log.pbn_queue_entry is None + assert log.pbn_status == "" + + +@pytest.mark.django_db +def test_restore_kolejkuje_wysylke(wydawnictwo_ciagle, superuser, bez_celery): + """RESTORE kolejkuje wysyłkę także BEZ ``pbn_uid``. + + ``zakolejkuj_wysylke`` świadomie nie ma gate'u na ``pbn_uid`` (faza 05a): + rekord, który nigdy nie był w PBN, po przywróceniu ma prawo pojechać — + wysyłka dopiero nadaje UID. + """ + from pbn_export_queue.models import PBN_Export_Queue + + wydawnictwo_ciagle.delete(user=superuser) + wydawnictwo_ciagle.restore(user=superuser) + + log = _logi(wydawnictwo_ciagle, SoftDeleteLog.Akcja.RESTORE).get() + assert log.pbn_queue_entry is not None + assert log.pbn_status == "WYSYLKA" + assert log.pbn_queue_entry.operacja == PBN_Export_Queue.Operacja.WYSYLKA + + +@pytest.mark.django_db +def test_restore_przy_niezakonczonym_wycofaniu_nie_dubluje_wpisu( + wydawnictwo_ciagle, superuser, bez_celery +): + """Przywrócenie, gdy wycofanie WCIĄŻ czeka w kolejce, nie tworzy wpisu. + + ``sprobuj_utowrzyc_wpis`` odrzuca drugi AKTYWNY wpis dla tego samego + rekordu (``AlreadyEnqueuedError`` → ``None``), więc szybkie + „usuń i cofnij" zostawia w kolejce samo wycofanie. Log mówi wtedy + ``pbn_status=""``. + + ⚠️ To ta sama pusta wartość, co przy „nie było czego kolejkować" — + z samego logu nie odróżnisz „pominięto (już w kolejce)" od + „nie dotyczy". Świadome uproszczenie: obie funkcje kolejkujące zwracają + ``None`` i receiver nie ma ich jak rozróżnić bez dodatkowego zapytania. + """ + _z_pbn_uid(wydawnictwo_ciagle) + wydawnictwo_ciagle.delete(user=superuser) + wydawnictwo_ciagle.restore(user=superuser) + + log = _logi(wydawnictwo_ciagle, SoftDeleteLog.Akcja.RESTORE).get() + assert log.pbn_queue_entry is None + assert log.pbn_status == "" + + +@pytest.mark.django_db +def test_restore_po_zakonczonym_wycofaniu_kolejkuje_wysylke( + wydawnictwo_ciagle, superuser, bez_celery +): + """Realna sekwencja: wycofanie przetworzone, potem przywrócenie.""" + from pbn_export_queue.models import PBN_Export_Queue + + _z_pbn_uid(wydawnictwo_ciagle) + wydawnictwo_ciagle.delete(user=superuser) + + wycofanie = _logi(wydawnictwo_ciagle, SoftDeleteLog.Akcja.DELETE).get() + wycofanie.pbn_queue_entry.wysylke_zakonczono = timezone.now() + wycofanie.pbn_queue_entry.save() + + wydawnictwo_ciagle.restore(user=superuser) + + log = _logi(wydawnictwo_ciagle, SoftDeleteLog.Akcja.RESTORE).get() + assert log.pbn_status == "WYSYLKA" + assert log.pbn_queue_entry.operacja == PBN_Export_Queue.Operacja.WYSYLKA + + +@pytest.mark.django_db +def test_autor_z_pbn_uid_nie_trafia_do_kolejki( + autor_jan_kowalski, superuser, bez_celery +): + """``Autor`` MA ``pbn_uid`` — gate na sam atrybut byłby błędny. + + Plan fazy 06 twierdzi (jako fakt zweryfikowany), że „Autor i *_Autor nie + mają pbn_uid", i na tym opiera uniwersalny gate + ``getattr(instance, "pbn_uid_id", None)``. W kodzie ``Autor.pbn_uid`` + jest zwykłym FK (``autor.py``), więc tamten gate wstawiłby AUTORA do + kolejki eksportu PUBLIKACJI i odpalił dla niego wysyłkę do PBN. + Gate idzie po typie: wyłącznie ``BppPublikacjaSoftDeleteMixin``. + """ + from pbn_api.models import Scientist + from pbn_export_queue.models import PBN_Export_Queue + + autor_jan_kowalski.pbn_uid = baker.make(Scientist) + autor_jan_kowalski.save() + assert autor_jan_kowalski.pbn_uid_id is not None + + autor_jan_kowalski.delete(user=superuser, reason="duplikat") + + log = _logi(autor_jan_kowalski, SoftDeleteLog.Akcja.DELETE).get() + assert log.pbn_queue_entry is None + assert log.pbn_status == "" + assert not PBN_Export_Queue.objects.exists() + bez_celery.delay.assert_not_called() + + +@pytest.mark.django_db +def test_kaskada_autorstw_nie_kolejkuje_osobno( + wydawnictwo_ciagle_z_autorem, superuser, bez_celery +): + """Publikacja z N autorami → DOKŁADNIE JEDEN wpis kolejki. + + Kaskada fazy 02 emituje 1 + N sygnałów. Gdyby receiver kolejkował dla + każdego, powstałoby N zbędnych wpisów dla wierszy ``*_Autor`` — a przy + RESTORE szczególnie łatwo, bo ``zakolejkuj_wysylke`` nie ma gate'u na + ``pbn_uid`` i przyjęłaby cokolwiek. + """ + from pbn_export_queue.models import PBN_Export_Queue + + _z_pbn_uid(wydawnictwo_ciagle_z_autorem) + wydawnictwo_ciagle_z_autorem.delete(user=superuser) + + assert PBN_Export_Queue.objects.count() == 1 + wpis = PBN_Export_Queue.objects.get() + assert wpis.rekord_do_wysylki == wydawnictwo_ciagle_z_autorem + + +# --- Task 6: uczelnia wyprowadzona z rekordu --------------------------- + + +@pytest.mark.django_db +def test_wpis_kolejki_dostaje_uczelnie_rekordu( + wydawnictwo_ciagle_z_autorem, uczelnia, superuser, bez_celery +): + """Wpis z receivera niesie uczelnię — inaczej multi-hosted by go nie wysłał. + + Wszyscy pozostali wołający kolejki biorą uczelnię z requestu + (``Uczelnia.objects.get_for_request``). Receiver requestu nie ma, więc + odwraca regułę przynależności z fazy 05b (``naleza_wydawnictwa``): + uczelnia jednostek autorów rekordu. Bez tego ``_pozyskaj_klienta_pbn`` + spadłby na ``get_single_uczelnia_or_fail()`` i przy dwóch uczelniach + wycofanie kończyłoby się ``FINISHED_ERROR``. + """ + from pbn_export_queue.models import PBN_Export_Queue + + _z_pbn_uid(wydawnictwo_ciagle_z_autorem) + wydawnictwo_ciagle_z_autorem.delete(user=superuser) + + wpis = PBN_Export_Queue.objects.get() + assert wpis.uczelnia == uczelnia + + +@pytest.mark.django_db +def test_uczelnia_czytana_mimo_skasowanych_autorstw( + wydawnictwo_ciagle_z_autorem, uczelnia, superuser, bez_celery +): + """Atrybucja MUSI iść przez ``global_objects``, nie ``objects``. + + Wąska kaskada fazy 02 kasuje wiersze ``*_Autor`` PRZED wysłaniem + ``post_soft_delete`` rodzica. Gdy receiver pyta o uczelnię, autorstwa + są już w koszu — odczyt przez ``objects`` zwróciłby pustkę i uczelnia + wychodziłaby ``None`` przy KAŻDYM kasowaniu, czyli dokładnie wtedy, + gdy jest potrzebna. + """ + from pbn_export_queue.models import PBN_Export_Queue + + _z_pbn_uid(wydawnictwo_ciagle_z_autorem) + wydawnictwo_ciagle_z_autorem.delete(user=superuser) + + assert not wydawnictwo_ciagle_z_autorem.autorzy_set.exists() + assert PBN_Export_Queue.objects.get().uczelnia == uczelnia + + +@pytest.mark.django_db +def test_uczelnia_niejednoznaczna_daje_none( + wydawnictwo_ciagle_z_autorem, + autor_uczelnia2, + jednostka_uczelnia2, + typy_odpowiedzialnosci, + superuser, + bez_celery, +): + """Praca współautorska między uczelniami → ``uczelnia=None``, nie zgadywanie. + + Wybranie „pierwszej z brzegu" wysłałoby wycofanie przez konto PBN + niewłaściwego tenanta. ``None`` degraduje do zachowania sprzed fazy 06: + jedna uczelnia w bazie → działa, kilka → głośny błąd na wpisie kolejki. + """ + from pbn_export_queue.models import PBN_Export_Queue + + wydawnictwo_ciagle_z_autorem.dodaj_autora(autor_uczelnia2, jednostka_uczelnia2) + _z_pbn_uid(wydawnictwo_ciagle_z_autorem) + wydawnictwo_ciagle_z_autorem.delete(user=superuser) + + assert PBN_Export_Queue.objects.get().uczelnia is None + + +@pytest.mark.django_db +def test_praca_doktorska_bierze_uczelnie_z_jednostki( + praca_doktorska, uczelnia, superuser, bez_celery +): + """Prace dyplomowe nie mają through-modelu — atrybucja przez FK jednostki. + + Ta sama dwoistość, którą faza 05b rozstrzygnęła w ``naleza_prace`` + kontra ``naleza_wydawnictwa``. + """ + from pbn_export_queue.models import PBN_Export_Queue + + _z_pbn_uid(praca_doktorska) + praca_doktorska.delete(user=superuser) + + assert PBN_Export_Queue.objects.get().uczelnia == uczelnia From 7d6a78c041cec74d89fc3ff661eebad9792b1172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sun, 16 Aug 2026 23:29:19 +0200 Subject: [PATCH 06/18] feat(soft-delete): praca w koszu nie liczy sie do ewaluacji Cache_Punktacja_Autora i Cache_Punktacja_Dyscypliny NIE MAJA FK do publikacji -- kluczem jest tablica rekord_id = [content_type_id, pk]. Nie rusza ich wiec ani kaskada Django, ani waska kaskada *_Autor z fazy 02, ani triggery denormalizacji. Bez tego soft-deletowana praca nadal wnosila sloty i punkty do ewaluacji (luka nieobjeta zadnym innym mechanizmem, spec 2.5b). GATE WEZSZY NIZ W PLANIE, I TO JEST POPRAWKA FAKTOGRAFICZNA. Plan mowil "tylko 5 modeli publikacji". przelicz_punkty_dyscyplin() maja TRZY: Wydawnictwo_Ciagle, Wydawnictwo_Zwarte, Patent -- tylko one dziedzicza ModelZPrzeliczaniemDyscyplin. Prace dyplomowe nie maja nawet tej metody, wiec gate "5 modeli" wywalilby sie na AttributeError przy kasowaniu doktoratu. Gate przez isinstance(ModelZPrzeliczaniemDyscyplin) jest jednoczesnie odpowiedzia na oba ostrzezenia planu: zawezenie po typie nadawcy ORAZ gwarancja, ze kaskada *_Autor nie wyzwoli przeliczenia drugi raz (te wiersze tego abstraktu nie dziedzicza). Test liczy wywolania removeEntries: dokladnie jedno mimo 1+N sygnalow. RESTORE przelicza, a nie odtwarza z kopii: przez czas pobytu w koszu mogly sie zmienic dyscypliny autorow albo progi punktowe, wiec odtworzenie starych wartosci przywrociloby nieaktualny stan. POMIAR (krok 6b.3 planu): restore() z przeliczeniem = 46.7 ms dla rekordu z 2 autorami. Ponizej progu 1 s, wiec bez eskalacji do fazy 07 -- ale przy masowym przywracaniu koszt jest liniowy (100 rekordow ~ 4.7 s w jednym zadaniu HTTP), co odnotowuje handoff. Fixture bez_celery przeniesiony do conftest.py katalogu: testy biegna z CELERY_TASK_ALWAYS_EAGER, wiec bez atrapy zakolejkowanie wykonywaloby realna wysylke do PBN w srodku delete()/restore(). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/receivers/soft_delete.py | 49 +++++++++- src/bpp/tests/test_soft_delete/conftest.py | 16 ++++ .../test_soft_delete/test_cache_punktacji.py | 90 +++++++++++++++++++ .../tests/test_soft_delete/test_receivers.py | 12 --- 4 files changed, 154 insertions(+), 13 deletions(-) create mode 100644 src/bpp/tests/test_soft_delete/conftest.py create mode 100644 src/bpp/tests/test_soft_delete/test_cache_punktacji.py diff --git a/src/bpp/receivers/soft_delete.py b/src/bpp/receivers/soft_delete.py index 356cdc090..5156ffdf4 100644 --- a/src/bpp/receivers/soft_delete.py +++ b/src/bpp/receivers/soft_delete.py @@ -70,10 +70,45 @@ def _utworz_log(instance, akcja, pbn_queue_entry=None, pbn_status=""): ) +def _wylicza_punktacje(instance): + """Czy rekord wnosi wiersze do ``Cache_Punktacja_*``. + + Gate zawęża do ``Wydawnictwo_Ciagle``, ``Wydawnictwo_Zwarte`` + i ``Patent`` — tylko one dziedziczą ``ModelZPrzeliczaniemDyscyplin``. + Prace dyplomowe, ``Autor``, ``*_Autor`` i ``Element_Repozytorium`` + nie mają nawet metody ``przelicz_punkty_dyscyplin()``, więc bez gate'u + receiver wywracałby się na ``AttributeError``. + + Gate załatwia przy okazji drugą rzecz: kaskada ``*_Autor`` emituje + własne sygnały, a te wiersze tego abstraktu nie dziedziczą — więc + przeliczenie zachodzi RAZ, przy sygnale rodzica, a nie 1 + N razy. + """ + from bpp.models.abstract import ModelZPrzeliczaniemDyscyplin + + return isinstance(instance, ModelZPrzeliczaniemDyscyplin) + + +def _skasuj_punktacje(instance): + """Kosz zabiera rekordowi sloty i punkty w ewaluacji. + + ``Cache_Punktacja_*`` nie ma FK do publikacji (klucz to tablica + ``rekord_id = [content_type_id, pk]``), więc nie sprząta jej ani + kaskada Django, ani kaskada ``*_Autor`` z fazy 02, ani triggery + denormalizacji. Bez tego praca w koszu nadal liczyłaby się do + ewaluacji. + """ + from bpp.models.sloty.core import IPunktacjaCacher + + IPunktacjaCacher(instance).removeEntries() + + def on_post_soft_delete(sender, instance, **kwargs): """Kosz → zlecenie wycofania oświadczeń z PBN + wpis audytu.""" from pbn_export_queue.operacje import zakolejkuj_wycofanie + if _wylicza_punktacje(instance): + _skasuj_punktacje(instance) + wpis, status = _kolejkuj_pbn(instance, zakolejkuj_wycofanie, STATUS_WYCOFANIE) _utworz_log( instance, @@ -84,9 +119,21 @@ def on_post_soft_delete(sender, instance, **kwargs): def on_post_restore(sender, instance, **kwargs): - """Przywrócenie → zlecenie ponownej wysyłki do PBN + wpis audytu.""" + """Przywrócenie → ponowna wysyłka do PBN, punktacja i wpis audytu. + + Punktację PRZELICZAMY, a nie odtwarzamy z kopii: wiersze + ``Cache_Punktacja_*`` zostały skasowane, a przez czas pobytu w koszu + mogły się zmienić dyscypliny autorów albo progi punktowe. Odtworzenie + starych wartości przywróciłoby nieaktualny stan. + + ⚠️ To operacja LICZĄCA, nie ``UPDATE``. Przy masowym przywracaniu + z admina (faza 07) koszt rośnie liniowo — patrz notatka w handoffie. + """ from pbn_export_queue.operacje import zakolejkuj_wysylke + if _wylicza_punktacje(instance): + instance.przelicz_punkty_dyscyplin() + wpis, status = _kolejkuj_pbn(instance, zakolejkuj_wysylke, STATUS_WYSYLKA) _utworz_log( instance, diff --git a/src/bpp/tests/test_soft_delete/conftest.py b/src/bpp/tests/test_soft_delete/conftest.py new file mode 100644 index 000000000..e1df1142a --- /dev/null +++ b/src/bpp/tests/test_soft_delete/conftest.py @@ -0,0 +1,16 @@ +from unittest.mock import patch + +import pytest + + +@pytest.fixture +def bez_celery(): + """Odcina realną wysyłkę do PBN z receiverów fazy 06. + + Kolejkowanie kończy się ``task_sprobuj_wyslac_do_pbn.delay()``, a testy + biegną z ``CELERY_TASK_ALWAYS_EAGER = True`` (``settings/test.py``) — + bez tej atrapy zadanie wykonałoby się synchronicznie, w środku + ``delete()``/``restore()``, i próbowało pogadać z PBN-em. + """ + with patch("pbn_export_queue.tasks.task_sprobuj_wyslac_do_pbn") as mock_task: + yield mock_task diff --git a/src/bpp/tests/test_soft_delete/test_cache_punktacji.py b/src/bpp/tests/test_soft_delete/test_cache_punktacji.py new file mode 100644 index 000000000..767f6d148 --- /dev/null +++ b/src/bpp/tests/test_soft_delete/test_cache_punktacji.py @@ -0,0 +1,90 @@ +"""Praca w koszu nie liczy się do ewaluacji (faza 06, Task 6b). + +Luka nieobjęta żadnym innym mechanizmem: ``Cache_Punktacja_Autora`` +i ``Cache_Punktacja_Dyscypliny`` NIE MAJĄ FK do publikacji — kluczem jest +tablica ``rekord_id = [content_type_id, pk]``. Nie rusza ich więc ani +kaskada Django, ani wąska kaskada ``*_Autor`` z fazy 02, ani triggery +denormalizacji. Bez receiverów soft-deletowana praca nadal wnosiłaby sloty +i punkty do ewaluacji. +""" + +import pytest +from django.contrib.contenttypes.models import ContentType + +from bpp.models.cache.punktacja import ( + Cache_Punktacja_Autora, + Cache_Punktacja_Dyscypliny, +) + + +def _klucz(rekord): + # ``content_type`` nie jest atrybutem modeli publikacji (to property na + # RekordBase), więc bierzemy przez ContentType. + return [ContentType.objects.get_for_model(type(rekord)).pk, rekord.pk] + + +@pytest.mark.django_db +def test_soft_delete_kasuje_cache_punktacji(zwarte_z_dyscyplinami, bez_celery): + zw = zwarte_z_dyscyplinami + zw.przelicz_punkty_dyscyplin() + klucz = _klucz(zw) + + assert Cache_Punktacja_Autora.objects.filter(rekord_id=klucz).exists() + assert Cache_Punktacja_Dyscypliny.objects.filter(rekord_id=klucz).exists() + + zw.delete() + + assert not Cache_Punktacja_Autora.objects.filter(rekord_id=klucz).exists() + assert not Cache_Punktacja_Dyscypliny.objects.filter(rekord_id=klucz).exists() + + +@pytest.mark.django_db +def test_restore_przywraca_cache_punktacji(zwarte_z_dyscyplinami, bez_celery): + zw = zwarte_z_dyscyplinami + zw.przelicz_punkty_dyscyplin() + klucz = _klucz(zw) + + zw.delete() + assert not Cache_Punktacja_Autora.objects.filter(rekord_id=klucz).exists() + + zw.restore() + + assert Cache_Punktacja_Autora.objects.filter(rekord_id=klucz).exists() + assert Cache_Punktacja_Dyscypliny.objects.filter(rekord_id=klucz).exists() + + +@pytest.mark.django_db +def test_przeliczenie_zachodzi_raz_mimo_kaskady(zwarte_z_dyscyplinami, bez_celery): + """Kaskada ``*_Autor`` NIE MOŻE wyzwalać przeliczenia po raz drugi. + + Soft-delete publikacji z N autorami emituje 1 + N sygnałów. Gdyby + kasowanie punktacji zachodziło przy każdym, dokładalibyśmy N-1 zbędnych + par zapytań do każdego usunięcia. Gate po ``ModelZPrzeliczaniemDyscyplin`` + zawęża to do sygnału rodzica — wiersze ``*_Autor`` tego abstraktu nie + dziedziczą. + """ + from unittest.mock import patch + + zw = zwarte_z_dyscyplinami + zw.przelicz_punkty_dyscyplin() + assert zw.autorzy_set.count() >= 2, "test ma sens tylko dla >1 autorstwa" + + with patch("bpp.models.sloty.core.IPunktacjaCacher.removeEntries") as mock_remove: + zw.delete() + + assert mock_remove.call_count == 1 + + +@pytest.mark.django_db +def test_autor_nie_dotyka_cache_punktacji(autor_jan_kowalski, bez_celery): + """Receiver jest globalny — dla ``Autor`` ta gałąź nie może się odpalić. + + ``Autor`` nie ma ``przelicz_punkty_dyscyplin()``; bez gate'u receiver + wywaliłby się na ``AttributeError`` przy każdym soft-delete autora. + """ + from unittest.mock import patch + + with patch("bpp.models.sloty.core.IPunktacjaCacher") as mock_cacher: + autor_jan_kowalski.delete() + + mock_cacher.assert_not_called() diff --git a/src/bpp/tests/test_soft_delete/test_receivers.py b/src/bpp/tests/test_soft_delete/test_receivers.py index 8a40cdfc8..490befcb6 100644 --- a/src/bpp/tests/test_soft_delete/test_receivers.py +++ b/src/bpp/tests/test_soft_delete/test_receivers.py @@ -1,7 +1,5 @@ """Receivery sygnałów soft-delete → ``SoftDeleteLog`` (faza 06).""" -from unittest.mock import patch - import pytest from django.contrib.contenttypes.models import ContentType from django.utils import timezone @@ -12,16 +10,6 @@ from bpp.models.soft_delete_log import SoftDeleteLog -@pytest.fixture -def bez_celery(): - """Kolejkowanie odpala ``task_sprobuj_wyslac_do_pbn.delay()``, a testy - biegną z ``CELERY_TASK_ALWAYS_EAGER`` — bez tego wysyłka do PBN - wykonałaby się synchronicznie, w środku ``delete()``. - """ - with patch("pbn_export_queue.tasks.task_sprobuj_wyslac_do_pbn") as mock_task: - yield mock_task - - def _z_pbn_uid(rekord): from pbn_api.models import Publication From bb007a3e60d45dcb4b737671dd058953ef33460b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sun, 16 Aug 2026 23:40:09 +0200 Subject: [PATCH 07/18] feat(soft-delete): hard_delete na querysecie emituje sygnal + guard rejestracji TASK 8 (z handoffu 7, plan nie mial na to taska -- decyzja wlasciciela). Pakietowy hard_delete() na querysecie to goly super().delete() (django_softdelete/managers.py): jedno zapytanie bulk, ZERO sygnalow. Rekordy znikaly fizycznie bez jednego wpisu w SoftDeleteLog -- dokladnie ta klasa cichej utraty, przed ktora ten log ma chronic, i to na najbardziej prawdopodobnej drodze masowego kasowania (oproznianie kosza z admina fazy 07). Nadpisane w BppSoftDeleteQuerySet (objects + global_objects) i BppDeletedQuerySet (deleted_objects) -- razem pokrywaja wszystkie trzy managery BPP. Iteracja per instancja, wiec kazdy wiersz przechodzi przez BppPkPrzedHardDeleteMixin.hard_delete() i wysyla post_hard_delete. Koszt: N zapytan zamiast jednego. Swiadomy i zgodny z zasada, ktora rzadzi juz gate'em update() i waska kaskada fazy 02: operacja masowa nie ma prawa byc tansza kosztem pominiecia sygnalow. Pakiet stosuje ja zreszta sam -- jego SoftDeleteQuerySet.delete() rowniez iteruje po instancjach. Zwrotka zachowuje kontrakt Django (liczba, {etykieta: liczba}), co ma osobny test. TASK 7: dwa straznikami regresji. Pierwszy dowodzi, ze receivery sa podpiete przez BppConfig.ready(), a nie przez przypadkowy import -- bez niego cala reszta testow przechodzilaby takze wtedy, gdyby w produkcji nikt register() nie wolal. Drugi pilnuje idempotencji: ready() bywa wolane wielokrotnie, a bez dispatch_uid kazdy soft-delete tworzylby dwa identyczne wpisy, czyli audyt zaczalby zmyslac. Testy Taska 8 siegaja po _wydawnictwo_ciagle_maker wprost, bo fixture-factory wydawnictwo_ciagle_maker w fixtures/conftest_publications.py:81 nie ma dekoratora @pytest.fixture i pytest jej nie rejestruje (zastany martwy kod, nie ruszany). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/models/soft_delete.py | 45 +++++++ .../soft-delete-06-softdeletelog.feature.rst | 6 + .../tests/test_soft_delete/test_receivers.py | 120 ++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 src/bpp/newsfragments/soft-delete-06-softdeletelog.feature.rst diff --git a/src/bpp/models/soft_delete.py b/src/bpp/models/soft_delete.py index ac22a2ba5..e6e317de7 100644 --- a/src/bpp/models/soft_delete.py +++ b/src/bpp/models/soft_delete.py @@ -38,6 +38,7 @@ """ import uuid +from collections import defaultdict from django.core.exceptions import FieldDoesNotExist from django.db import transaction @@ -164,10 +165,47 @@ def raise_if_has_protected_children(instance, relations, label): ) +def hard_delete_per_instancja(queryset): + """``hard_delete()`` wiersz po wierszu, żeby leciał ``post_hard_delete``. + + DLACZEGO NIE BULK: pakietowy ``hard_delete()`` na querysecie to goły + ``super().delete()`` (``django_softdelete/managers.py``) — jedno + zapytanie, ZERO sygnałów. Rekordy znikały fizycznie i bez jednego wpisu + w ``SoftDeleteLog``, czyli dokładnie ta klasa cichej utraty, przed którą + ten log ma chronić. Najbardziej prawdopodobna droga do tej luki to + opróżnianie kosza z admina (faza 07). + + To ta sama zasada, którą kieruje się gate w ``BppSoftDeleteQuerySet. + update()`` i wąska kaskada fazy 02: operacja masowa nie ma prawa być + tańsza kosztem pominięcia sygnałów. Pakiet stosuje ją zresztą sam — + jego ``SoftDeleteQuerySet.delete()`` też iteruje po instancjach. + + KOSZT: N zapytań zamiast jednego. Świadomy — audyt masowego kasowania + jest wart więcej niż pojedynczy ``DELETE ... WHERE id IN (...)``. + + Zwrotka jak w Django: ``(łączna_liczba, {etykieta_modelu: liczba})``, + zsumowana po instancjach. + """ + laczna = 0 + liczniki = defaultdict(int) + # list(): iterujemy po materializowanej liście, bo kasujemy w trakcie. + for obiekt in list(queryset): + ile, per_model = obiekt.hard_delete() + laczna += ile + for etykieta, n in per_model.items(): + liczniki[etykieta] += n + return laczna, dict(liczniki) + + class BppSoftDeleteQuerySet(SoftDeleteQuerySet): """Gate: blokuje bulk-ustawienie deleted_at/restored_at przez .update() (omijałoby post_save, kaskadę *_Autor, SoftDeleteLog i reversion).""" + def hard_delete(self): + return hard_delete_per_instancja(self) + + hard_delete.alters_data = True + def update(self, **kwargs): if "deleted_at" in kwargs or "restored_at" in kwargs: raise RuntimeError( @@ -222,6 +260,13 @@ def restore(self, strict: bool = False, *args, **kwargs): restore.alters_data = True + def hard_delete(self): + """Opróżnianie kosza też musi zostawiać ślad — patrz + ``hard_delete_per_instancja()``.""" + return hard_delete_per_instancja(self) + + hard_delete.alters_data = True + class BppDeletedManager(DeletedManager): def get_queryset(self): diff --git a/src/bpp/newsfragments/soft-delete-06-softdeletelog.feature.rst b/src/bpp/newsfragments/soft-delete-06-softdeletelog.feature.rst new file mode 100644 index 000000000..cbf3e71d3 --- /dev/null +++ b/src/bpp/newsfragments/soft-delete-06-softdeletelog.feature.rst @@ -0,0 +1,6 @@ +Operacje usuwania rekordów do kosza, przywracania ich i kasowania trwale są +odtąd zapisywane w dzienniku „Log operacji soft-delete": kto, kiedy, z jakim +powodem oraz czy zlecono wycofanie lub ponowną wysyłkę oświadczeń do PBN. +Wrzucenie pracy do kosza usuwa też jej punktację z pamięci podręcznej +ewaluacji (praca w koszu nie wnosi już slotów ani punktów), a przywrócenie +przelicza ją na nowo. diff --git a/src/bpp/tests/test_soft_delete/test_receivers.py b/src/bpp/tests/test_soft_delete/test_receivers.py index 490befcb6..5bfda0227 100644 --- a/src/bpp/tests/test_soft_delete/test_receivers.py +++ b/src/bpp/tests/test_soft_delete/test_receivers.py @@ -18,6 +18,18 @@ def _z_pbn_uid(rekord): return rekord +def _kolejne_ciagle(nr): + """Kolejne wydawnictwo ciągłe obok tego z fixture'u. + + Fixture-factory ``wydawnictwo_ciagle_maker`` w ``fixtures/ + conftest_publications.py`` nie ma dekoratora ``@pytest.fixture``, więc + pytest go nie rejestruje — sięgamy po sam maker. + """ + from fixtures.conftest_publications import _wydawnictwo_ciagle_maker + + return _wydawnictwo_ciagle_maker(tytul_oryginalny=f"Hard delete {nr}") + + def _logi(instance, akcja): return SoftDeleteLog.objects.filter( content_type=ContentType.objects.get_for_model(instance), @@ -406,3 +418,111 @@ def test_praca_doktorska_bierze_uczelnie_z_jednostki( praca_doktorska.delete(user=superuser) assert PBN_Export_Queue.objects.get().uczelnia == uczelnia + + +# --- Task 8: hard_delete na querysecie też musi zostawiać ślad --------- + + +@pytest.mark.django_db +def test_hard_delete_na_querysecie_tworzy_logi(wydawnictwo_ciagle, superuser): + """Masowe twarde kasowanie NIE MOŻE być niewidoczne dla audytu. + + Pakietowy ``hard_delete()`` na querysecie to goły ``super().delete()`` — + jedno zapytanie bulk, bez ``post_hard_delete``, więc bez wpisu w logu. + Rekord znikał fizycznie i bez śladu, czyli dokładnie ta klasa cichej + utraty, przed którą ``SoftDeleteLog`` ma chronić. Handoff §7 przypisuje + tę lukę fazie 06. + """ + from bpp.models import Wydawnictwo_Ciagle + + pk_i = [wydawnictwo_ciagle.pk] + [_kolejne_ciagle(i).pk for i in range(2)] + ct = ContentType.objects.get_for_model(Wydawnictwo_Ciagle) + + with soft_delete_context(user=superuser, reason="czystka"): + Wydawnictwo_Ciagle.objects.filter(pk__in=pk_i).hard_delete() + + assert not Wydawnictwo_Ciagle.global_objects.filter(pk__in=pk_i).exists() + + logi = SoftDeleteLog.objects.filter( + content_type=ct, object_id__in=pk_i, akcja=SoftDeleteLog.Akcja.HARD_DELETE + ) + assert logi.count() == 3 + assert {log.object_id for log in logi} == set(pk_i) + assert all(log.user == superuser and log.powod == "czystka" for log in logi) + + +@pytest.mark.django_db +def test_hard_delete_na_deleted_objects_tworzy_logi(wydawnictwo_ciagle, superuser): + """Ta sama luka na ``deleted_objects`` — czyli na opróżnianiu kosza. + + To najbardziej prawdopodobna droga masowego twardego kasowania + w adminie fazy 07. + """ + from bpp.models import Wydawnictwo_Ciagle + + pk = wydawnictwo_ciagle.pk + wydawnictwo_ciagle.delete(user=superuser) + + Wydawnictwo_Ciagle.deleted_objects.filter(pk=pk).hard_delete() + + assert SoftDeleteLog.objects.filter( + content_type=ContentType.objects.get_for_model(Wydawnictwo_Ciagle), + object_id=pk, + akcja=SoftDeleteLog.Akcja.HARD_DELETE, + ).exists() + + +@pytest.mark.django_db +def test_hard_delete_na_querysecie_zwraca_licznik(wydawnictwo_ciagle): + """Kontrakt zwrotki ``(liczba, {etykieta: liczba})`` jak w Django.""" + from bpp.models import Wydawnictwo_Ciagle + + pk_i = [wydawnictwo_ciagle.pk, _kolejne_ciagle(9).pk] + + ile, liczniki = Wydawnictwo_Ciagle.objects.filter(pk__in=pk_i).hard_delete() + + assert liczniki["bpp.Wydawnictwo_Ciagle"] == 2 + assert ile >= 2 + + +# --- Task 7: rejestracja przez apps.ready() ---------------------------- + + +def test_receivery_zarejestrowane_przez_apps_ready(): + """Receivery mają być podpięte przez ``BppConfig.ready()``. + + Wszystkie pozostałe testy tego pliku przeszłyby także wtedy, gdyby + ``register()`` wołał ktoś inny (albo gdyby podpinał je import + ubocznie). Ten test dowodzi, że mechanizm działa w produkcji: nikt + w kodzie aplikacyjnym nie woła ``register()`` ręcznie. + """ + from django_softdelete.signals import ( + post_hard_delete, + post_restore, + post_soft_delete, + ) + + def _uids(sygnal): + # Signal.receivers: [(lookup_key, receiver, ...), ...], gdzie + # lookup_key == (dispatch_uid_albo_id_receivera, id_nadawcy). + return {klucz[0] for klucz, *_ in sygnal.receivers} + + assert "bpp.soft_delete.post_soft_delete" in _uids(post_soft_delete) + assert "bpp.soft_delete.post_restore" in _uids(post_restore) + assert "bpp.soft_delete.post_hard_delete" in _uids(post_hard_delete) + + +def test_rejestracja_jest_idempotentna(): + """Powtórne ``register()`` nie może zdublować receiverów. + + ``AppConfig.ready()`` bywa wołane więcej niż raz (m.in. przy + ``TransactionTestCase``). Bez ``dispatch_uid`` każdy soft-delete + tworzyłby wtedy dwa identyczne wpisy w logu — audyt zacząłby zmyślać. + """ + from django_softdelete.signals import post_soft_delete + + from bpp.receivers import soft_delete as receivery + + przed = len(post_soft_delete.receivers) + receivery.register() + assert len(post_soft_delete.receivers) == przed From 253f173b4e79bed22bf9bd58603ab1fe8910f787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sun, 16 Aug 2026 23:53:39 +0200 Subject: [PATCH 08/18] docs(soft-delete): handoff fazy 07 + lista modeli objetych receiverami Handoff zbiera cztery rozjazdy planu z kodem (Autor.pbn_uid, brak super().delete() w fazach 02/04, sprzecznosc dwoch sposobow atrybucji, gate punktacji na trzech a nie pieciu modelach), szosty model soft-delete znaleziony przez asercje nad apps.get_models(), pomiar kosztu restore() oraz wskazowki wprost dla admina fazy 07. Docstring receiverow wymienia teraz wszystkie objete modele -- z adnotacja, zeby tej listy NIE powielac w kodzie: zrodlem prawdy jest asercja testowa, bo wyliczanka z glowy juz dwukrotnie okazala sie niepelna. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- .../HANDOFF-soft-delete-faza-07.md | 226 ++++++++++++++++++ src/bpp/receivers/soft_delete.py | 15 +- 2 files changed, 237 insertions(+), 4 deletions(-) create mode 100644 docs/superpowers/HANDOFF-soft-delete-faza-07.md diff --git a/docs/superpowers/HANDOFF-soft-delete-faza-07.md b/docs/superpowers/HANDOFF-soft-delete-faza-07.md new file mode 100644 index 000000000..3e0a4f716 --- /dev/null +++ b/docs/superpowers/HANDOFF-soft-delete-faza-07.md @@ -0,0 +1,226 @@ +# Handoff: soft-delete, start fazy 07 + +> Po zamknięciu **fazy 06** (`SoftDeleteLog` + receivery sygnałów + atrybucja +> usera), 2026-08-16. Czytaj to zamiast odtwarzania historii z gita. + +--- + +## 1. Gdzie jesteśmy + +| | | +|---|---| +| Stan fazy 06 | gałąź `feat/soft-delete-06`, odbita od `feat/soft-delete-05b` | +| Plan fazy 06 | [`plans/2026-06-04-soft-delete-06-softdeletelog.md`](plans/2026-06-04-soft-delete-06-softdeletelog.md) — wykonany, z odstępstwami opisanymi w §3 | +| Migracja fazy 06 | `bpp/0503_softdeletelog` (jedna, tylko `CreateModel`) | +| Task 5 planu | **POMINIĘTY** zgodnie z handoffem fazy 06 — `zakolejkuj_*` istnieją z fazy 05a | + +### Stos PR-ów + +``` +#312 feat/soft-delete -> dev +#745 feat/soft-delete-04 -> feat/soft-delete +#755 feat/soft-delete-05 -> feat/soft-delete-04 +#767 feat/soft-delete-05b -> feat/soft-delete-05 + feat/soft-delete-06 -> feat/soft-delete-05b (faza 06, ta praca) +``` + +⚠️ Żaden PR ze stosu nie jest scalony. Faza 06 dziedziczy commity 05b. + +⚠️ **Baseline (`baseline-sql/`) nadal NIEŚWIEŻY** — stoi na `bpp/0487`. +Doszła `0503`. Odświeżenie robi się **raz, przy scalaniu** całego +`feat/soft-delete` do `dev`. + +⚠️ **CI NIE URUCHAMIA SIĘ na PR-ach do gałęzi `feat/soft-delete*`** — bez +zmian. Jedyną weryfikacją jest przebieg lokalny. + +--- + +## 2. Co faza 06 dostarcza (kontrakt dla fazy 07) + +```python +from bpp.models.soft_delete_context import soft_delete_context +from bpp.models.soft_delete_log import SoftDeleteLog +``` + +| Element | Gdzie | Uwaga | +|---|---|---| +| `SoftDeleteLog` | `bpp/models/soft_delete_log.py` | GFK + `akcja`/`user`/`powod` + `pbn_queue_entry`/`pbn_status` | +| `soft_delete_context(user=, reason=)` | `bpp/models/soft_delete_context.py` | thread-local; **pominięty argument DZIEDZICZY** (patrz §3.3) | +| `current_soft_delete_user/reason()` | tamże | akcesory dla receiverów | +| Trzy receivery | `bpp/receivers/soft_delete.py` | podpięte BEZ `sender=`, w `BppConfig.ready()` | +| `BppPkPrzedHardDeleteMixin` | `bpp/models/soft_delete.py` | **każdy** model soft-delete MUSI go mieć | +| `pk_dla_audytu(instance)` | tamże | pk po twardym skasowaniu; głośny `RuntimeError` bez mixinu | +| `BppPublikacjaSoftDeleteMixin.uczelnia_rekordu()` | tamże | atrybucja tenanta bez requestu | +| `hard_delete_per_instancja(queryset)` | tamże | queryset-owy `hard_delete()` z sygnałami | + +**Realne API atrybucji to `delete(user=, reason=)` / `restore(user=)`** — +fazy 02/04 przyjmowały te argumenty i je porzucały; faza 06 domknęła +obietnicę. Faza 07 (admin) ma po prostu przekazywać `request.user`. + +--- + +## 3. ⚠️ Cztery miejsca, w których plan rozjechał się z kodem + +Wszystkie wykryte PRZED napisaniem kodu albo przez padający test — nie po +fakcie. Trzy pierwsze to błędy rzeczowe w planie, nie zmiany zakresu. + +### 3.1 `Autor` MA `pbn_uid` (blokujące) + +Plan §„Detekcja publikacja z `pbn_uid`" twierdził — jako fakt zweryfikowany — +że „`Autor` i `*_Autor` nie mają `pbn_uid`", i na tym opierał uniwersalny gate +`getattr(instance, "pbn_uid_id", None)`. `Autor.pbn_uid` **istnieje** (FK do +`pbn_api.Scientist`, `autor.py`). Gate z planu wstawiłby **autora** do kolejki +eksportu **publikacji** i odpalił dla niego wysyłkę do PBN. + +Sam `pbn_uid` nie wystarczyłby też od drugiej strony: `zakolejkuj_wysylke` +świadomie NIE MA gate'u na `pbn_uid` (faza 05a), więc przy RESTORE przyjęłaby +każdy wiersz `*_Autor` przywracany kaskadą — publikacja z N autorami dałaby +N zbędnych zleceń. + +**Rozstrzygnięcie:** gate po typie — +`isinstance(instance, BppPublikacjaSoftDeleteMixin)`. Oba testy są w +`test_receivers.py`. + +### 3.2 Fazy 02/04 nie mogły wpiąć kontekstu tak, jak plan zakładał + +Plan zakładał, że owijają `super().delete()`. One **nie wołają +`super().delete()` w ogóle** — świadomie, żeby uniknąć refleksyjnej kaskady +pakietu (skasowałaby twardo `Autor_Jednostka`, `Autor_Dyscyplina` itd.). Same +ustawiają `deleted_at` i same wysyłają sygnał. Faza 06 zakłada więc kontekst +w czterech metodach (publikacje `delete`/`restore`, `Autor` `delete`/`restore`), +obejmując CAŁE ciało — kaskada `*_Autor` ma dziedziczyć atrybucję. + +### 3.3 Dwa sposoby atrybucji z planu wykluczały się nawzajem + +Wykryte przez padający test **z samego planu**. `delete()` zakłada kontekst +ZAWSZE, więc wywołanie bez `user=` wewnątrz jawnego +`soft_delete_context(user=X)` zerowało X i operacja szła do logu jako niczyja. + +**Reguła:** pominięty argument **dziedziczy**, nie zeruje. Siedzi w samym +context managerze, nie w czterech miejscach wywołań — inaczej następny model +soft-delete zgubiłby usera po cichu. Uzasadnienie: `None` znaczy „nie wiem", +a nie „wiem, że nikt". + +### 3.4 Gate punktacji obejmuje TRZY modele, nie pięć + +Plan (Task 6b) mówił „tylko 5 modeli publikacji". +`przelicz_punkty_dyscyplin()` mają wyłącznie `Wydawnictwo_Ciagle`, +`Wydawnictwo_Zwarte` i `Patent` — dziedziczą `ModelZPrzeliczaniemDyscyplin`. +Prace dyplomowe **nie mają tej metody**, więc gate „5 modeli" wywaliłby się +na `AttributeError` przy kasowaniu doktoratu. + +--- + +## 4. Szósty model soft-delete, o którym nie wiedział nikt + +Asercja testowa „każdy `SoftDeleteModel` ma `BppPkPrzedHardDeleteMixin`" +(nad `apps.get_models()`) natychmiast wykryła +**`zglos_publikacje.Zgloszenie_Publikacji`** — model soft-delete +**niezależny od faz 01–04**, używający pakietu od dawna na własne potrzeby. +Nie było go na żadnej liście: ani w planie, ani w handoffach faz 02/04/05. + +To powtórka lekcji z handoffu fazy 05a §4.2 (faza 04 pominęła czwartego +dziedzica abstraktu). **Nauka: wyliczanka modeli z głowy jest niepełna; +asercja nad `apps.get_models()` nie jest.** + +**Skutek do odnotowania:** operacje na `Zgloszenie_Publikacji` trafiają teraz +do `SoftDeleteLog`. Bez skutków PBN (gate z §3.1 obejmuje wyłącznie +publikacje). Jeśli to niepożądane, zawężenie jest jednolinijkowe — ale +domyślnie audyt jest szerszy, nie węższy. + +--- + +## 5. Fakt o pakiecie utrwalony testem + +`post_hard_delete` leci **po** `Model.delete()`, a kolektor Django zeruje +wtedy `pk` instancji. Receiver dostaje więc `instance.pk is None`. Naiwny +zapis dałby `object_id=None` → `IntegrityError` **w receiverze** → wywrócone +`hard_delete()`: audyt zepsułby operację, którą ma tylko obserwować. + +Stąd `BppPkPrzedHardDeleteMixin`. Test-sonda +(`test_pakiet_zeruje_pk_przed_wyslaniem_post_hard_delete`) pilnuje tego +założenia — gdy aktualizacja pakietu je zmieni, zapali się tam, a nie +w postaci logów czytających pole zapasowe. + +--- + +## 6. Czego faza 06 NIE domyka (świadomie) + +- **`pbn_status=""` jest dwuznaczne.** Nie odróżnia „nie było czego + kolejkować" od „pominięto, bo rekord już czeka w kolejce" — obie funkcje + kolejkujące zwracają `None` i receiver nie ma ich jak rozróżnić bez + dodatkowego zapytania. Praktyczna konsekwencja: szybkie „usuń i cofnij" + zostawia w kolejce samo WYCOFANIE, a log RESTORE pokazuje pusty status. + Udokumentowane testem + `test_restore_przy_niezakonczonym_wycofaniu_nie_dubluje_wpisu`. +- **`uczelnia_rekordu()` zwraca `None` przy dwuznaczności** (praca + współautorska między uczelniami). Wpis kolejki degraduje wtedy do + zachowania sprzed fazy 06: jedna uczelnia w bazie → działa, kilka → + głośny błąd. Wybranie „pierwszej z brzegu" wysłałoby wycofanie przez konto + PBN cudzego tenanta, więc to celowo nie jest zgadywane. +- **Reguła przynależności jest zduplikowana.** `uczelnia_rekordu()` odwraca + `naleza_wydawnictwa`/`naleza_prace` z `cerif_export` (faza 05b). Nie da się + jej wołać wprost, bo `cerif_export` zależy od `bpp`, nie odwrotnie. Gdy + tamta się zmieni, ta MUSI pójść za nią — dziś pilnują tego tylko testy + po obu stronach. +- **Brak UI dla logu.** `SoftDeleteLog` nie ma admina ani widoku — faza 07. +- **`SoftDeleteLog` nie niesie `pbn_uid`.** Dług z handoffu 05a §6 (twarde + skasowanie publikacji zostawia oświadczenia w PBN na zawsze) NIE jest + domknięty: log zapisuje `object_id`, nie PBN UID. Domknięcie wymaga + dołożenia pola i wypełnienia go w receiverze HARD_DELETE. + +--- + +## 7. Wskazówki wprost dla fazy 07 (admin kosza) + +- **Masowe przywracanie potrzebuje zadania w tle.** Zmierzone: `restore()` + z przeliczeniem punktacji to **46,7 ms** dla rekordu z 2 autorami. Poniżej + progu 1 s z planu, więc bez eskalacji — ale koszt jest liniowy: 100 + rekordów ≈ 4,7 s w jednym żądaniu HTTP. +- **Masowe twarde kasowanie jest teraz N zapytań, nie jedno.** + `hard_delete()` na querysecie iteruje per instancja, żeby leciały sygnały + (Task 8). Świadomy koszt — ale akcja admina nad dużym zaznaczeniem to + kandydat na zadanie w tle. +- **Admin ma request, więc ma tenanta.** Jeśli faza 07 zechce, żeby wpisy + kolejki niosły uczelnię z requestu zamiast wyprowadzanej z rekordu + (dokładniejsze dla prac współautorskich), naturalne miejsce to + rozszerzenie `soft_delete_context` o `uczelnia=`. Właściciel świadomie + wybrał wyprowadzanie z rekordu, bo działa też dla CLI i celery. + +--- + +## 8. Dług nadal otwarty (z faz 01–05b) + +| Sprawa | Stan | +|---|---| +| **Asymetria gate'u `.update(deleted_at=...)`** | `BppSoftDeleteQuerySet` blokuje, `BppDeletedQuerySet` **nie** — `Autor.deleted_objects...update(deleted_at=...)` rzuca, to samo na `Wydawnictwo_Ciagle` przechodzi. Wygląda na przeoczenie 05b, nie decyzję | +| **Kaskada `Jednostka` → `Autor`** | `aktualna_jednostka`/`aktualna_funkcja` nadal `CASCADE` | +| **`Autor.slug` `unique=True` bezwarunkowo** | husk trzyma slug zarezerwowany | +| **Wycieki ORM (kanarek `xfail(strict=True)`)** | bez zmian | +| **PR upstream `django-easy-audit`** | [#348](https://github.com/soynatan/django-easy-audit/pull/348) | +| **Brak UI dla `ProtectedError`** | w adminie gołe 500; faza 07 | +| **`Autor` nie ma kosza w adminie** | faza 07 | +| **`hard_delete()` na querysecie bez sygnału** | **ZAMKNIĘTE w fazie 06** (Task 8) | +| Słowniki (`Zrodlo`, `Konferencja`, `Projekt`, `Jednostka`) bez soft-delete | bez zmian | +| Pomiar `0492` i narzutu GiST | wciąż nikt nie zmierzył | +| **Strategia wydania** | bramka na fazie 07 | + +--- + +## 9. Proces — co się sprawdziło w fazie 06 + +- **Weryfikacja planu przed kodowaniem zwróciła się natychmiast.** Trzy + z czterech rozjazdów z §3 to błędy RZECZOWE w planie, a nie zmiany + zakresu. Najgroźniejszy (`Autor.pbn_uid`) był oznaczony w planie jako + „zweryfikowane" — etykieta wiarygodności nie zastępuje sprawdzenia. +- **Asercja nad `apps.get_models()` bije wyliczankę.** Znalazła szósty model + soft-delete w pierwszym uruchomieniu (§4). Ten sam błąd — lista modeli + z pamięci — wystąpił już w fazie 04. +- **Mutacja jako dowód, nie rytuał.** Podmiana `global_objects` → `objects` + w `uczelnia_rekordu()` wywaliła dokładnie dwa testy, które tego pilnują. + Potwierdziło to empirycznie, że kolejność „kaskada przed sygnałem rodzica" + jest realnym zagrożeniem, a nie teoretycznym. +- **Test z planu wykrył sprzeczność w samym planie.** `test_soft_delete_ + tworzy_log_z_userem` padł po wpięciu kontekstu i wymusił regułę + dziedziczenia (§3.3). Gdyby go pominąć jako „oczywisty", sprzeczność + wyszłaby dopiero w fazie 07. diff --git a/src/bpp/receivers/soft_delete.py b/src/bpp/receivers/soft_delete.py index 5156ffdf4..d9ee087a0 100644 --- a/src/bpp/receivers/soft_delete.py +++ b/src/bpp/receivers/soft_delete.py @@ -1,9 +1,16 @@ """Receivery sygnałów ``django-soft-delete`` → ``SoftDeleteLog``. -JEDEN punkt podpięcia dla WSZYSTKICH modeli soft-delete (publikacje, -``*_Autor``, ``Autor``, ``Element_Repozytorium``) — receivery są rejestrowane -bez ``sender=``, więc nowy model soft-delete jest logowany bez dopisywania -czegokolwiek tutaj. Rejestracja: ``BppConfig.ready()``. +JEDEN punkt podpięcia dla WSZYSTKICH modeli soft-delete — receivery są +rejestrowane bez ``sender=``, więc nowy model soft-delete jest logowany bez +dopisywania czegokolwiek tutaj. Rejestracja: ``BppConfig.ready()``. + +Objęte modele (stan 2026-08-16): 5 publikacji + 3 ``*_Autor`` (fazy 02/01), +``Autor`` (04), ``Element_Repozytorium`` (01) oraz +``zglos_publikacje.Zgloszenie_Publikacji`` — ten ostatni jest soft-delete +NIEZALEŻNIE od faz 01-04 i nie było go na żadnej liście w planach; znalazła +go dopiero asercja ``test_kazdy_model_soft_delete_zachowuje_pk`` nad +``apps.get_models()``. Nie wypisuj tej listy nigdzie w kodzie — to ta +asercja jest źródłem prawdy. Usera i powód wnosi thread-local ``soft_delete_context`` — sygnały pakietu ich nie niosą (patrz ``bpp/models/soft_delete_context.py``). From b4942a52c6b6a94474d42459ad2a17a65458ed66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Mon, 17 Aug 2026 00:11:00 +0200 Subject: [PATCH 09/18] docs(soft-delete): wynik weryfikacji fazy 06 + charakter dwoch timeoutow make tests-without-playwright: 9647 passed, 2 failed. Obie porazki to Timeout (>90 s), nie asercje, i NIE sa regresja fazy 06 -- czasy seryjne zmierzone na obu galeziach sa identyczne co do szumu (45.70/31.88 s na bazie wobec 43.16/32.33 s na fazie 06). Zaden z tych testow nie wykonuje operacji soft-delete, wiec receivery nie maja sie w nich gdzie odpalic; wklad fazy 06 do grafu migracji to jedno CreateModel. Oba testy maja ~2x zapasu do limitu 90 s, a -n auto odpala 10 workerow -- na wspoldzielonym hoscie to za malo. Ta sama klasa problemu co handoff fazy 05a 8. Kandydat do podniesienia timeoutu, poza zakresem fazy 06. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- .../HANDOFF-soft-delete-faza-07.md | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/docs/superpowers/HANDOFF-soft-delete-faza-07.md b/docs/superpowers/HANDOFF-soft-delete-faza-07.md index 3e0a4f716..4be917ef7 100644 --- a/docs/superpowers/HANDOFF-soft-delete-faza-07.md +++ b/docs/superpowers/HANDOFF-soft-delete-faza-07.md @@ -33,6 +33,35 @@ Doszła `0503`. Odświeżenie robi się **raz, przy scalaniu** całego ⚠️ **CI NIE URUCHAMIA SIĘ na PR-ach do gałęzi `feat/soft-delete*`** — bez zmian. Jedyną weryfikacją jest przebieg lokalny. +### Wynik weryfikacji fazy 06 + +`make tests-without-playwright`: **9647 passed, 2 failed, 4 skipped, +2 xfailed** (8:06). + +Obie porażki to `Failed: Timeout (>90.0s)` — NIE asercje: + +- `bpp/tests/test_soft_delete/test_kronika_usunieta.py::test_0499_odwracalna` +- `pbn_api/tests/test_migracja_dyscypliny_uuid_e2e.py::test_migracja_ + przechodzi_na_bazie_z_duplikatami` + +**Nie są regresją fazy 06** — zmierzone czasy seryjne na obu gałęziach: + +| Test | `feat/soft-delete-05b` | `feat/soft-delete-06` | +|---|---|---| +| `test_migracja_...duplikatami` | 45,70 s | 43,16 s | +| `test_0499_odwracalna` | 31,88 s | 32,33 s | + +Identyczne co do szumu. Żaden z nich nie wykonuje operacji soft-delete +(w `test_kronika_usunieta.py` nie ma ani jednego `.delete()`/`.restore()`/ +`hard_delete`), więc receivery nie mają się w nich gdzie odpalić; wkład +fazy 06 do grafu migracji to jedno `CreateModel`. + +⚠️ **To testy o ~2× zapasie do limitu 90 s**, a `-n auto` odpala 10 workerów. +Na współdzielonym hoście przekraczają limit — ta sama klasa problemu, co +w handoffie fazy 05a §8 (load 6+ wywracał start testcontainerów). Kandydat +do podniesienia `@pytest.mark.timeout` dla tych dwóch plików; poza zakresem +fazy 06. + --- ## 2. Co faza 06 dostarcza (kontrakt dla fazy 07) From eb2985821f431413825400aa3742dfbf910dd67e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 25 Aug 2026 16:49:06 +0200 Subject: [PATCH 10/18] feat(soft-delete): admin nad global_objects + filtr kosza MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BppSoftDeleteAdminMixin podmienia manager bazowy changelisty na global_objects, zeby dalo sie otworzyc i przywrocic rekord z kosza. Kosz chowa PokazSkasowaneFilter — brak parametru = tylko zywe (pakietowy SoftDeleteFilter pokazywalby wszystko, a po poszerzeniu querysetu nie ma juz nikogo innego, kto by kosz schowal). Semantyka wartosci zostaje pakietowa: is_deleted=true to rekordy skasowane. Mixin wpinany jako OSTATNI przed admin.ModelAdmin, wbrew planowi fazy 07 (kazal "PIERWSZY"). get_queryset nie moze wolac super() — musi podmienic manager — wiec na poczatku MRO uciolby caly lancuch, w tym SiteFilteredAdminMixin.get_queryset w AutorAdmin (zawezenie do wlasnej uczelni, FD#390). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/admin/helpers/mixins.py | 96 ++++++++++++++++++++ src/bpp/admin/wydawnictwo_ciagle.py | 8 +- src/bpp/tests/test_soft_delete/test_admin.py | 71 +++++++++++++++ 3 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 src/bpp/tests/test_soft_delete/test_admin.py diff --git a/src/bpp/admin/helpers/mixins.py b/src/bpp/admin/helpers/mixins.py index 4b5f7de6e..a82bdc61b 100644 --- a/src/bpp/admin/helpers/mixins.py +++ b/src/bpp/admin/helpers/mixins.py @@ -1,5 +1,6 @@ from django import forms from django.urls import reverse +from django_softdelete.filters import SoftDeleteFilter from bpp.models.system import Status_Korekty @@ -90,3 +91,98 @@ def has_delete_permission(self, request, obj=None): if obj.pbn_uid_id is not None: return False return super().has_delete_permission(request, obj) + + +class PokazSkasowaneFilter(SoftDeleteFilter): + """Filtr „kosz" dla changelisty modeli soft-delete. + + RÓŻNICA WOBEC PAKIETU JEST JEDNA: brak parametru w URL-u znaczy tutaj + „tylko żywe", a nie „wszystko". Pakietowy ``SoftDeleteFilter`` mapuje + ``self.value() or 'all'`` na ``'ALL'`` i zwraca cały queryset, więc + wejście na gołą changelistę pokazywałoby kosz wymieszany z żywymi + rekordami. Skoro ``get_queryset`` mixinu podaje ``global_objects``, + to ten filtr jest JEDYNYM miejscem, które chowa kosz — bez tej zmiany + nie schowałby go nikt. + + SEMANTYKA WARTOŚCI ZOSTAJE PAKIETOWA, celowo: ``is_deleted=true`` to + rekordy skasowane (pakiet mapuje ``'true'`` na + ``deleted_at__isnull=False``). Parametr w URL-u czyta się więc wprost + i da się go bezpiecznie zabookmarkować. + """ + + title = "Kosz" + + def lookups(self, request, model_admin): + return ( + ("true", "🗑️ Tylko skasowane"), + ("all", "Wszystkie (żywe + kosz)"), + ) + + def choices(self, changelist): + """Pierwsza pozycja to stan domyślny — a ten NIE znaczy „wszystkie". + + Odziedziczona etykieta „All" opisywałaby dokładnie odwrotność tego, + co filtr robi bez parametru. + """ + wybory = super().choices(changelist) + domyslny = next(iter(wybory)) + domyslny["display"] = "Bez kosza (tylko żywe)" + yield domyslny + yield from wybory + + def queryset(self, request, queryset): + wartosc = self.value() + if wartosc == "all": + return queryset + if wartosc == "true": + return queryset.filter(deleted_at__isnull=False) + return queryset.filter(deleted_at__isnull=True) + + +class BppSoftDeleteAdminMixin: + """Kosz w adminie dla modeli soft-delete (5 publikacji + ``Autor``). + + Daje: changelistę nad ``global_objects`` z filtrem kosza, akcje + „przywróć" / „usuń do kosza (z powodem)" / „usuń trwale" + (superuser-only) oraz JEDEN punkt wstrzyknięcia ``request.user``. + + ⚠️ MIEJSCE W LIŚCIE BAZ: **OSTATNIE**, tuż przed terminalnym + ``admin.ModelAdmin`` (albo przed konkretną klasą bazową admina, np. + ``Wydawnictwo_ZwarteAdmin_Baza``). NIE pierwsze. + + Plan fazy 07 kazał wpinać mixin jako PIERWSZY — to był błąd i wpiąłby + lukę wielotenantową. ``get_queryset`` poniżej nie woła ``super()`` + (nie ma jak: musi podmienić manager bazowy na ``global_objects``), + więc postawiony na początku MRO uciąłby cały łańcuch — w tym + ``SiteFilteredAdminMixin.get_queryset`` w ``AutorAdmin``, które zawęża + widok nie-superusera do jego uczelni (FD#390). Personel uczelni A + zobaczyłby i skasował autorów uczelni B. + + Na końcu łańcucha ``get_queryset`` tego mixinu jest jego PODSTAWĄ: + zwraca ``global_objects``, a wszystkie mixiny stojące wyżej nakładają + na to swoje filtry normalnie. Pozostałe nadpisane metody + (``get_actions``, ``get_list_filter``) działają z tej pozycji tak samo, + bo tamte implementacje w łańcuchu wołają ``super()`` i tylko DOKŁADAJĄ + pozycje. Strażnikiem tej decyzji jest + ``test_admin.py::test_staff_widzi_tylko_autorow_swojej_uczelni``. + """ + + def get_queryset(self, request): + """Podstawa łańcucha: ``global_objects`` (żywe + kosz). + + Skasowanego rekordu nie da się inaczej ani otworzyć, ani + przywrócić — changeform szuka obiektu w tym samym querysecie. + Kosz chowa dopiero ``PokazSkasowaneFilter``, więc domyślny widok + listy się nie zmienia. + """ + qs = self.model.global_objects.get_queryset() + ordering = self.get_ordering(request) + if ordering: + qs = qs.order_by(*ordering) + return qs + + def get_list_filter(self, request): + list_filter = list(super().get_list_filter(request) or []) + if PokazSkasowaneFilter not in list_filter: + list_filter.insert(0, PokazSkasowaneFilter) + return list_filter diff --git a/src/bpp/admin/wydawnictwo_ciagle.py b/src/bpp/admin/wydawnictwo_ciagle.py index 1100224a0..dc07c3f63 100644 --- a/src/bpp/admin/wydawnictwo_ciagle.py +++ b/src/bpp/admin/wydawnictwo_ciagle.py @@ -86,7 +86,11 @@ MODEL_Z_OPLATA_ZA_PUBLIKACJE_FIELDSET, MODEL_Z_POLAMI_EWALUACJI_PBN_FIELDSET, ) -from .helpers.mixins import OptionalPBNSaveMixin, RestrictDeletionWhenPBNUIDSetMixin +from .helpers.mixins import ( + BppSoftDeleteAdminMixin, + OptionalPBNSaveMixin, + RestrictDeletionWhenPBNUIDSetMixin, +) from .xlsx_export import resources from .xlsx_export.mixins import EksportDanychZFormatowanieMixin, ExportActionsMixin from .zglos_publikacje_helpers import UzupelniajWstepneDanePoNumerzeZgloszeniaMixin @@ -286,6 +290,8 @@ class Wydawnictwo_CiagleAdmin( ExportActionsMixin, DynamicColumnsMixin, RestrictDeletionWhenPBNUIDSetMixin, + # OSTATNI przed ModelAdmin — patrz docstring BppSoftDeleteAdminMixin. + BppSoftDeleteAdminMixin, admin.ModelAdmin, ): change_list_template = "admin/bpp/wydawnictwo_ciagle/change_list.html" diff --git a/src/bpp/tests/test_soft_delete/test_admin.py b/src/bpp/tests/test_soft_delete/test_admin.py new file mode 100644 index 000000000..8f31677f6 --- /dev/null +++ b/src/bpp/tests/test_soft_delete/test_admin.py @@ -0,0 +1,71 @@ +"""Faza 07: kosz w adminie — filtr, przywracanie, trwałe usuwanie, powód. + +KONTRAKT PARAMETRU FILTRA: ``?is_deleted=true`` pokazuje WYŁĄCZNIE kosz, +``?is_deleted=all`` żywe razem z koszem, brak parametru — tylko żywe. Ta +sama semantyka, co w pakietowym ``SoftDeleteFilter`` (``'true'`` mapuje +tam na ``deleted_at__isnull=False``), więc parametr w URL-u nie kłamie. +""" + +import pytest +from django.urls import reverse +from model_bakery import baker + +from bpp.models import Wydawnictwo_Ciagle + + +@pytest.mark.django_db +def test_changelist_domyslnie_ukrywa_kosz(superuser_client): + baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Żywa praca") + skasowany = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Praca w koszu") + skasowany.delete(reason="test") + + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + resp = superuser_client.get(url) + content = resp.content.decode("utf-8") + + assert resp.status_code == 200 + assert "Żywa praca" in content + assert "Praca w koszu" not in content + + +@pytest.mark.django_db +def test_filtr_pokazuje_wylacznie_kosz(superuser_client): + baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Żywa praca") + skasowany = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Praca w koszu") + skasowany.delete(reason="test") + + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + resp = superuser_client.get(url, {"is_deleted": "true"}) + content = resp.content.decode("utf-8") + + assert resp.status_code == 200 + assert "Praca w koszu" in content + assert "Żywa praca" not in content + + +@pytest.mark.django_db +def test_filtr_wszystkie_pokazuje_zywe_i_kosz(superuser_client): + baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Żywa praca") + skasowany = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Praca w koszu") + skasowany.delete(reason="test") + + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + resp = superuser_client.get(url, {"is_deleted": "all"}) + content = resp.content.decode("utf-8") + + assert resp.status_code == 200 + assert "Praca w koszu" in content + assert "Żywa praca" in content + + +@pytest.mark.django_db +def test_changeform_otwiera_skasowany_rekord(superuser_client): + """Bez ``global_objects`` w ``get_queryset`` admin dawałby 302/404 — + a bez otwarcia rekordu nie ma jak go przywrócić ani obejrzeć.""" + skasowany = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Praca w koszu") + skasowany.delete(reason="test") + + url = reverse("admin:bpp_wydawnictwo_ciagle_change", args=[skasowany.pk]) + resp = superuser_client.get(url) + + assert resp.status_code == 200 From c3590e326d4ac6c39ef8065c2648c1f946b38b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 25 Aug 2026 16:50:19 +0200 Subject: [PATCH 11/18] feat(soft-delete): jeden hook usera + delete_model/delete_queryset = kosz MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _soft_delete_user_context deleguje do soft_delete_context z fazy 06 i jest jedynym punktem, w ktory wchodzi request.user — takze dla hard_delete(), ktore (wbrew planowi fazy 07) NIE przyjmuje user=/reason= i innego kanalu atrybucji nie ma. Bez tego rekord i tak ladowal w koszu (delete() modelu jest miekkie), ale wpis SoftDeleteLog powstawal z user=None. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/admin/helpers/mixins.py | 67 ++++++++++++++++++++ src/bpp/tests/test_soft_delete/test_admin.py | 42 ++++++++++++ 2 files changed, 109 insertions(+) diff --git a/src/bpp/admin/helpers/mixins.py b/src/bpp/admin/helpers/mixins.py index a82bdc61b..7ce3bf11e 100644 --- a/src/bpp/admin/helpers/mixins.py +++ b/src/bpp/admin/helpers/mixins.py @@ -1,7 +1,10 @@ +from contextlib import contextmanager + from django import forms from django.urls import reverse from django_softdelete.filters import SoftDeleteFilter +from bpp.models.soft_delete_context import soft_delete_context from bpp.models.system import Status_Korekty @@ -186,3 +189,67 @@ def get_list_filter(self, request): if PokazSkasowaneFilter not in list_filter: list_filter.insert(0, PokazSkasowaneFilter) return list_filter + + # --- jeden punkt wstrzykniecia usera -------------------------------- + + @contextmanager + def _soft_delete_user_context(self, request): + """JEDEN punkt wstrzyknięcia ``request.user`` dla całego przepływu + kosza w adminie: ``delete_model``, ``delete_queryset``, + ``usun_do_kosza``, ``przywroc_zaznaczone``, ``usun_trwale_zaznaczone``. + + Deleguje do ``soft_delete_context`` z fazy 06 — thread-locala czytają + receivery sygnałów, bo sygnały pakietu ``django-soft-delete`` niosą + wyłącznie ``sender`` i ``instance``. + + DLACZEGO KONTEKST, SKORO ``delete()`` PRZYJMUJE ``user=``: bo + ``hard_delete()`` go NIE przyjmuje. ``BppPkPrzedHardDeleteMixin. + hard_delete()`` przekazuje argumenty prosto do pakietu, który o + żadnym userze nie wie — atrybucja trwałego usunięcia ma więc tylko + ten jeden kanał. Plan fazy 07 zakładał tu ``hard_delete(user=, + reason=)``; taka sygnatura nie istnieje. + + # SZEW reversion: tutaj (i tylko tutaj) dojdzie w przyszłości + # ``reversion.set_user(request.user)`` — jeden hook, nie dwa + # konkurencyjne. Patrz overview, „Kontrakty z reversion". + """ + with soft_delete_context( + user=request.user, reason=self._powod_z_requestu(request) + ): + yield + + def _powod_z_requestu(self, request): + """Powód operacji podany na stronie pośredniej (``usun_do_kosza``). + + Puste, gdy ścieżka nie pyta o powód — ``soft_delete_context`` + traktuje pusty łańcuch jako „nie wnoszę informacji" i dziedziczy + powód z kontekstu zewnętrznego, zamiast go zerować. + """ + if request.method != "POST": + return "" + return request.POST.get("powod", "") + + # --- kasowanie = kosz ----------------------------------------------- + + def delete_model(self, request, obj): + """Przycisk „Usuń" na changeformie przenosi do kosza. + + Bez tego nadpisania rekord i tak trafiłby do kosza (``delete()`` + modelu jest miękkie), ale BEZ atrybucji — wpis ``SoftDeleteLog`` + powstawałby z ``user=None``. + """ + with self._soft_delete_user_context(request): + obj.delete(user=request.user, reason=self._powod_z_requestu(request)) + + def delete_queryset(self, request, queryset): + """Akcja ``delete_selected`` — soft-delete PER INSTANCJA. + + Nigdy zbiorczo: kaskada ``*_Autor`` (faza 02), ``SoftDeleteLog`` + i sprzątanie ``Cache_Punktacja_*`` wiszą na sygnałach per obiekt, + a gate w ``BppSoftDeleteQuerySet.update()`` i tak blokuje bulk + ustawienie ``deleted_at``. + """ + with self._soft_delete_user_context(request): + powod = self._powod_z_requestu(request) + for obj in queryset: + obj.delete(user=request.user, reason=powod) diff --git a/src/bpp/tests/test_soft_delete/test_admin.py b/src/bpp/tests/test_soft_delete/test_admin.py index 8f31677f6..d121b1155 100644 --- a/src/bpp/tests/test_soft_delete/test_admin.py +++ b/src/bpp/tests/test_soft_delete/test_admin.py @@ -69,3 +69,45 @@ def test_changeform_otwiera_skasowany_rekord(superuser_client): resp = superuser_client.get(url) assert resp.status_code == 200 + + +def _log(model, pk, akcja): + """Najnowszy wpis ``SoftDeleteLog`` dla konkretnego rekordu. + + Filtr po ``content_type`` jest istotny: ``object_id`` nie jest unikalne + globalnie, a kaskada fazy 02 loguje przy okazji wiersze ``*_Autor``, + ktore z latwoscia trafiaja na te sama wartosc ``pk``. + """ + from django.contrib.contenttypes.models import ContentType + + from bpp.models import SoftDeleteLog + + return ( + SoftDeleteLog.objects.filter( + content_type=ContentType.objects.get_for_model(model), + object_id=pk, + akcja=akcja, + ) + .order_by("-timestamp", "-pk") + .first() + ) + + +@pytest.mark.django_db +def test_delete_w_adminie_soft_deletuje_i_zapisuje_usera(superuser, superuser_client): + """Przycisk „Usuń" na changeformie = kosz, nie fizyczne skasowanie.""" + obj = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Do kosza") + pk = obj.pk + url = reverse("admin:bpp_wydawnictwo_ciagle_delete", args=[pk]) + + assert superuser_client.get(url).status_code == 200 + + resp = superuser_client.post(url, {"post": "yes"}) + assert resp.status_code == 302 + + assert not Wydawnictwo_Ciagle.objects.filter(pk=pk).exists() + assert Wydawnictwo_Ciagle.global_objects.get(pk=pk).deleted_at is not None + + wpis = _log(Wydawnictwo_Ciagle, pk, "delete") + assert wpis is not None + assert wpis.user_id == superuser.pk From f56a936e7be4edf3fe12ba08cbef086ad8f450e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 25 Aug 2026 16:52:12 +0200 Subject: [PATCH 12/18] feat(soft-delete): akcja admina "Przywroc" przez ten sam hook usera MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Akcja dziala na querysecie changelisty (juz zawezonym przez filtry i przez SiteFilteredAdminMixin), a nie na swiezym deleted_objects — dociaganie po pk omijaloby zawezenie do wlasnej uczelni. Konsekwencja: przywracanie wymaga filtra "Tylko skasowane"; puste zaznaczenie tlumaczy komunikat zamiast cichego "przywrocono: 0". Restore pomija rekordy zywe — restore() publikacji przelicza punktacje i kolejkuje wysylke do PBN, wiec no-op nie bylby darmowy. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/admin/helpers/mixins.py | 52 ++++++++++++++++++++ src/bpp/tests/test_soft_delete/test_admin.py | 25 ++++++++++ 2 files changed, 77 insertions(+) diff --git a/src/bpp/admin/helpers/mixins.py b/src/bpp/admin/helpers/mixins.py index 7ce3bf11e..dac25bea0 100644 --- a/src/bpp/admin/helpers/mixins.py +++ b/src/bpp/admin/helpers/mixins.py @@ -1,6 +1,7 @@ from contextlib import contextmanager from django import forms +from django.contrib import admin, messages from django.urls import reverse from django_softdelete.filters import SoftDeleteFilter @@ -253,3 +254,54 @@ def delete_queryset(self, request, queryset): powod = self._powod_z_requestu(request) for obj in queryset: obj.delete(user=request.user, reason=powod) + + # --- akcje kosza ---------------------------------------------------- + + @admin.action(description="♻️ Przywróć zaznaczone (z kosza)") + def przywroc_zaznaczone(self, request, queryset): + """Przywraca rekordy z kosza przez ten sam hook usera. + + Zaznaczenie idzie z ``global_objects``, więc może zawierać rekordy + żywe — te pomijamy zamiast wołać na nich ``restore()``. To nie jest + kosmetyka: ``restore()`` publikacji przelicza punktację dyscyplin + (operacja licząca, nie ``UPDATE``) i kolejkuje wysyłkę do PBN, + więc „przywrócenie" nieskasowanego rekordu byłoby kosztownym + no-opem z fałszywym wpisem w ``SoftDeleteLog``. + """ + with self._soft_delete_user_context(request): + przywrocono = 0 + for obj in queryset: + if obj.deleted_at is None: + continue + obj.restore(user=request.user) + przywrocono += 1 + if not przywrocono: + # Najczestsza przyczyna: operator zaznaczyl rekordy na liscie + # bez filtra kosza, wiec do akcji nie doszedl ani jeden + # skasowany wiersz. Ciche "przywrocono: 0" wygladaloby jak + # awaria przywracania. + self.message_user( + request, + "Nie przywrócono nic — w zaznaczeniu nie było rekordów " + "z kosza. Ustaw filtr „Kosz” na „🗑️ Tylko skasowane”, " + "zaznacz rekordy i powtórz akcję.", + level=messages.WARNING, + ) + return + self.message_user( + request, + f"Przywrócono z kosza: {przywrocono}.", + level=messages.SUCCESS, + ) + + def get_actions(self, request): + """Dokłada akcje kosza do tych zebranych przez resztę łańcucha. + + Wołamy ``super()`` i tylko DOKŁADAMY klucze — dlatego mixin działa + także z ostatniej pozycji w MRO (patrz docstring klasy), mimo że + wyżej stoją ``AutorAdmin.get_actions`` i ``ExportActionsMixin. + get_actions``. + """ + actions = super().get_actions(request) + actions["przywroc_zaznaczone"] = self.get_action("przywroc_zaznaczone") + return actions diff --git a/src/bpp/tests/test_soft_delete/test_admin.py b/src/bpp/tests/test_soft_delete/test_admin.py index d121b1155..a74076922 100644 --- a/src/bpp/tests/test_soft_delete/test_admin.py +++ b/src/bpp/tests/test_soft_delete/test_admin.py @@ -111,3 +111,28 @@ def test_delete_w_adminie_soft_deletuje_i_zapisuje_usera(superuser, superuser_cl wpis = _log(Wydawnictwo_Ciagle, pk, "delete") assert wpis is not None assert wpis.user_id == superuser.pk + + +@pytest.mark.django_db +def test_akcja_przywroc_dziala_i_zapisuje_usera(superuser, superuser_client): + obj = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Wraca z kosza") + pk = obj.pk + obj.delete(reason="test") + assert not Wydawnictwo_Ciagle.objects.filter(pk=pk).exists() + + # Akcja dziala na tym, co widac na liscie — a kosz widac dopiero pod + # filtrem. Django POST-uje formularz akcji na biezacy URL RAZEM z + # query stringiem, wiec tak wyglada realny przeplyw operatora. + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + "?is_deleted=true" + resp = superuser_client.post( + url, + {"action": "przywroc_zaznaczone", "_selected_action": [str(pk)]}, + ) + assert resp.status_code in (200, 302) + + assert Wydawnictwo_Ciagle.objects.filter(pk=pk).exists() + assert Wydawnictwo_Ciagle.global_objects.get(pk=pk).deleted_at is None + + wpis = _log(Wydawnictwo_Ciagle, pk, "restore") + assert wpis is not None + assert wpis.user_id == superuser.pk From fcf341cdf774517c0b5a53c656ddce75572328ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 25 Aug 2026 16:55:17 +0200 Subject: [PATCH 13/18] feat(soft-delete): akcja "Usun trwale" superuser-only + fixture staff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hard_delete() nie przyjmuje user=/reason= (plan fazy 07 zakladal inaczej), wiec atrybucje niesie wylacznie kontekst z _soft_delete_user_context. Akcja kasuje WYLACZNIE rekordy z kosza. Cache_Punktacja_* nie ma FK do publikacji (klucz to tablica [content_type_id, pk]), wiec sprzata ja dopiero receiver post_soft_delete — twarde skasowanie rekordu zywego zostawiloby punktacje wskazujaca na nieistniejacy rekord i wciaz liczaca sie do ewaluacji. Fixture staff_user nalezy do grupy "wprowadzanie danych": staff BEZ grupy wywraca render menu admina 500-tka (menu.py:302 robi bezwarunkowe del menu.children[-1].children[-1] na pustej liscie) — bug niezalezny od soft-delete, odnotowany w handoffie. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/admin/helpers/mixins.py | 59 ++++++++++++++++++ src/bpp/tests/test_soft_delete/test_admin.py | 64 ++++++++++++++++++++ src/conftest.py | 49 +++++++++++++++ 3 files changed, 172 insertions(+) diff --git a/src/bpp/admin/helpers/mixins.py b/src/bpp/admin/helpers/mixins.py index dac25bea0..217e9b336 100644 --- a/src/bpp/admin/helpers/mixins.py +++ b/src/bpp/admin/helpers/mixins.py @@ -294,6 +294,58 @@ def przywroc_zaznaczone(self, request, queryset): level=messages.SUCCESS, ) + @admin.action(description="❌ Usuń TRWALE (nieodwracalnie, tylko superuser)") + def usun_trwale_zaznaczone(self, request, queryset): + """Opróżnianie kosza: fizyczne skasowanie rekordów. + + TYLKO Z KOSZA, nigdy wprost z rekordu żywego — i nie jest to + ostrożnościowy rytuał. ``Cache_Punktacja_*`` nie ma FK do + publikacji (klucz to tablica ``[content_type_id, pk]``), więc nie + sprząta jej ani kolektor Django, ani triggery. Kasuje ją dopiero + receiver ``post_soft_delete`` (faza 06). Twarde skasowanie rekordu + z pominięciem kosza zostawiłoby więc wiersze punktacji wskazujące + na nieistniejący rekord — i liczące się do ewaluacji. + + Podwójny gate na superusera (tu i w ``get_actions``) jest celowy: + ``get_actions`` decyduje o WIDOCZNOŚCI, ta metoda o WYKONANIU. + Django odrzuca akcje spoza ``get_actions``, ale to jest operacja + nieodwracalna — jeden mechanizm obronny na nią za mało. + """ + if not request.user.is_superuser: + self.message_user( + request, + "Trwałe usuwanie jest dostępne wyłącznie dla superużytkownika.", + level=messages.ERROR, + ) + return + + with self._soft_delete_user_context(request): + usunieto = 0 + pominieto = 0 + for obj in queryset: + if obj.deleted_at is None: + pominieto += 1 + continue + # hard_delete() NIE przyjmuje user=/reason= — atrybucję + # niesie wyłącznie kontekst powyżej. + obj.hard_delete() + usunieto += 1 + + if pominieto: + self.message_user( + request, + f"Pominięto rekordów spoza kosza: {pominieto}. Trwale usuwać " + "można wyłącznie to, co jest już w koszu — najpierw „🗑️ Usuń " + "do kosza”, potem „❌ Usuń TRWALE”.", + level=messages.WARNING, + ) + if usunieto: + self.message_user( + request, + f"Usunięto trwale (nieodwracalnie): {usunieto}.", + level=messages.SUCCESS, + ) + def get_actions(self, request): """Dokłada akcje kosza do tych zebranych przez resztę łańcucha. @@ -304,4 +356,11 @@ def get_actions(self, request): """ actions = super().get_actions(request) actions["przywroc_zaznaczone"] = self.get_action("przywroc_zaznaczone") + if request.user.is_superuser: + actions["usun_trwale_zaznaczone"] = self.get_action( + "usun_trwale_zaznaczone" + ) + else: + # Gdyby ktos dopisal ta akcje wyzej w lancuchu — zdejmij. + actions.pop("usun_trwale_zaznaczone", None) return actions diff --git a/src/bpp/tests/test_soft_delete/test_admin.py b/src/bpp/tests/test_soft_delete/test_admin.py index a74076922..925b56a00 100644 --- a/src/bpp/tests/test_soft_delete/test_admin.py +++ b/src/bpp/tests/test_soft_delete/test_admin.py @@ -136,3 +136,67 @@ def test_akcja_przywroc_dziala_i_zapisuje_usera(superuser, superuser_client): wpis = _log(Wydawnictwo_Ciagle, pk, "restore") assert wpis is not None assert wpis.user_id == superuser.pk + + +@pytest.mark.django_db +def test_usun_trwale_dostepne_dla_superusera(superuser, superuser_client): + obj = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Do trwałego usunięcia") + pk = obj.pk + obj.delete(reason="test") + + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + "?is_deleted=true" + assert b"usun_trwale_zaznaczone" in superuser_client.get(url).content + + resp = superuser_client.post( + url, + {"action": "usun_trwale_zaznaczone", "_selected_action": [str(pk)]}, + ) + assert resp.status_code in (200, 302) + assert not Wydawnictwo_Ciagle.global_objects.filter(pk=pk).exists() + + # Rekord zniknal fizycznie, wiec wpis audytu jest jedynym sladem, ze + # istnial — i ma nosic, KTO go skasowal. + wpis = _log(Wydawnictwo_Ciagle, pk, "hard_delete") + assert wpis is not None + assert wpis.user_id == superuser.pk + + +@pytest.mark.django_db +def test_usun_trwale_niedostepne_dla_staff(staff_client): + obj = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Próba przez staff") + pk = obj.pk + obj.delete(reason="test") + + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + "?is_deleted=true" + assert b"usun_trwale_zaznaczone" not in staff_client.get(url).content + + # Wymuszony POST tez nie usuwa trwale — akcji nie ma w get_actions, + # wiec Django odrzuca ja jako niedozwolony wybor. + resp = staff_client.post( + url, + {"action": "usun_trwale_zaznaczone", "_selected_action": [str(pk)]}, + ) + assert resp.status_code in (200, 302, 403) + assert Wydawnictwo_Ciagle.global_objects.filter(pk=pk).exists() + + +@pytest.mark.django_db +def test_usun_trwale_pomija_rekordy_spoza_kosza(superuser_client): + """Trwale kasujemy WYLACZNIE z kosza. + + ``Cache_Punktacja_*`` nie ma FK do publikacji, wiec sprzata ja dopiero + receiver ``post_soft_delete``. Twarde skasowanie rekordu zywego + zostawiloby wiersze punktacji wskazujace na nieistniejacy rekord. + """ + zywy = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Żywy, nie do kosza") + pk = zywy.pk + + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + "?is_deleted=all" + resp = superuser_client.post( + url, + {"action": "usun_trwale_zaznaczone", "_selected_action": [str(pk)]}, + follow=True, + ) + assert resp.status_code == 200 + assert Wydawnictwo_Ciagle.objects.filter(pk=pk).exists() + assert "Pominięto rekordów spoza kosza: 1" in resp.content.decode("utf-8") diff --git a/src/conftest.py b/src/conftest.py index 4a46346f1..516b05632 100644 --- a/src/conftest.py +++ b/src/conftest.py @@ -714,6 +714,55 @@ def superuser_client(client, superuser): return client +STAFF_USERNAME = "staff" +STAFF_PASSWORD = "staffpass" + + +@pytest.fixture +def staff_user(db): + """Uzytkownik z dostepem do admina, ale BEZ ``is_superuser``. + + Potrzebny wszedzie tam, gdzie testujemy roznice miedzy „staff" a + „superuser" — sam ``is_staff`` nie wystarczy, bo bez uprawnien + modelowych Django nie wpuszcza nawet na changeliste i test mierzylby + brak dostepu zamiast badanej reguly. + """ + from django.contrib.auth.models import Group, Permission + + from bpp.const import GR_WPROWADZANIE_DANYCH + + u = User.objects.create_user( + username=STAFF_USERNAME, + password=STAFF_PASSWORD, + email="staff@example.com", + ) + u.is_staff = True + u.save() + u.user_permissions.add( + *Permission.objects.filter( + content_type__app_label="bpp", + content_type__model__in=("wydawnictwo_ciagle", "autor"), + ) + ) + # Grupa „wprowadzanie danych" to nie ozdobnik. Staff BEZ zadnej grupy + # wywraca render menu admina 500-tka: `_add_group_submenus` dokleja + # submenu tylko czlonkom grupy, a zaraz potem bezwarunkowo robi + # `del menu.children[-1].children[-1]` (src/django_bpp/menu.py:302) — + # przy pustym `menu.children` leci IndexError. To bug niezalezny od + # soft-delete; fixture modeluje realnego redaktora, ktory te grupe ma. + grupa, _ = Group.objects.get_or_create(name=GR_WPROWADZANIE_DANYCH) + u.groups.add(grupa) + return u + + +@pytest.fixture +def staff_client(client, staff_user): + """Zalogowany ``staff_user`` (nie-superuser).""" + if not client.login(username=STAFF_USERNAME, password=STAFF_PASSWORD): + raise Exception("Cannot login staff") + return client + + @pytest.fixture def user_request_factory(test_user): """Fixture zwracający UserRequestFactory.""" From e08c3faa2be54d0d780d8b4d9e7a13bd05aed242 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 25 Aug 2026 16:57:12 +0200 Subject: [PATCH 14/18] feat(soft-delete): akcja "Usun do kosza" z powodem -> SoftDeleteLog.powod MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strona posrednia pyta o powod, bo strona potwierdzenia Django jest zbudowana wokol kolektora (co zniknie), a nie wokol metadanych operacji. delete_selected zostaje dzialajace — ta sama operacja, tylko bez uzasadnienia. Potwierdzenie przenosi dalej select_across i index: przy zaznaczeniu "wszystkie pasujace" Django ignoruje _selected_action i bierze caly queryset, wiec bez tego czesc rekordow po cichu nie trafilaby do kosza. Podglad listy jest przyciety do 50 pozycji (licznik z .count(), wiec liczba pozostaje prawdziwa). Test pilnuje tez, ze kaskada na *_Autor dziedziczy powod i usera rodzica. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/admin/helpers/mixins.py | 55 +++++++++++++++ src/bpp/tests/test_soft_delete/test_admin.py | 68 +++++++++++++++++++ .../admin/bpp/soft_delete_powod.html | 46 +++++++++++++ 3 files changed, 169 insertions(+) create mode 100644 src/django_bpp/templates/admin/bpp/soft_delete_powod.html diff --git a/src/bpp/admin/helpers/mixins.py b/src/bpp/admin/helpers/mixins.py index 217e9b336..4464b2125 100644 --- a/src/bpp/admin/helpers/mixins.py +++ b/src/bpp/admin/helpers/mixins.py @@ -2,6 +2,8 @@ from django import forms from django.contrib import admin, messages +from django.contrib.admin import helpers as admin_helpers +from django.template.response import TemplateResponse from django.urls import reverse from django_softdelete.filters import SoftDeleteFilter @@ -294,6 +296,58 @@ def przywroc_zaznaczone(self, request, queryset): level=messages.SUCCESS, ) + #: Ile rekordow wypisac na stronie posredniej z powodem. Zaznaczenie + #: "wszystkie pasujace" (``select_across``) potrafi objac dziesiatki + #: tysiecy wierszy — wypisanie wszystkich zabiloby te strone, a nikt + #: i tak nie czyta takiej listy. + LIMIT_PODGLADU_KOSZA = 50 + + @admin.action(description="🗑️ Usuń do kosza (z powodem)") + def usun_do_kosza(self, request, queryset): + """Kasowanie do kosza z powodem podanym na stronie pośredniej. + + DLACZEGO OSOBNA AKCJA, SKORO ``delete_selected`` TEŻ IDZIE DO KOSZA: + bo Django nie ma gdzie zapytać o powód. Jego strona potwierdzenia + jest zbudowana wokół kolektora (co jeszcze zniknie), nie wokół + metadanych operacji, a ``SoftDeleteLog.powod`` odpowiada na pytanie + „dlaczego", na które sam kolektor nie odpowie. + + ``delete_selected`` zostaje działające (kosz bez powodu) — to ta + sama operacja, tylko uboższa o uzasadnienie. + """ + if request.POST.get("powod_potwierdzony"): + with self._soft_delete_user_context(request): + powod = self._powod_z_requestu(request) + usunieto = 0 + for obj in queryset: + obj.delete(user=request.user, reason=powod) + usunieto += 1 + self.message_user( + request, + f"Przeniesiono do kosza: {usunieto}.", + level=messages.SUCCESS, + ) + return None + + ile = queryset.count() + podglad = list(queryset[: self.LIMIT_PODGLADU_KOSZA]) + context = { + **self.admin_site.each_context(request), + "title": "Usuń do kosza", + "obiekty": podglad, + "ile_obiektow": ile, + "ile_ukrytych": max(0, ile - len(podglad)), + "wybrane_pk": request.POST.getlist(admin_helpers.ACTION_CHECKBOX_NAME), + "opts": self.model._meta, + "action_checkbox_name": admin_helpers.ACTION_CHECKBOX_NAME, + # Przenosimy stan zaznaczenia dalej — bez ``select_across`` + # potwierdzenie zawezilo by "wszystkie pasujace" do biezacej + # strony i czesc rekordow po cichu nie trafilaby do kosza. + "action_index": request.POST.get("index", 0), + "select_across": request.POST.get("select_across", "0"), + } + return TemplateResponse(request, "admin/bpp/soft_delete_powod.html", context) + @admin.action(description="❌ Usuń TRWALE (nieodwracalnie, tylko superuser)") def usun_trwale_zaznaczone(self, request, queryset): """Opróżnianie kosza: fizyczne skasowanie rekordów. @@ -355,6 +409,7 @@ def get_actions(self, request): get_actions``. """ actions = super().get_actions(request) + actions["usun_do_kosza"] = self.get_action("usun_do_kosza") actions["przywroc_zaznaczone"] = self.get_action("przywroc_zaznaczone") if request.user.is_superuser: actions["usun_trwale_zaznaczone"] = self.get_action( diff --git a/src/bpp/tests/test_soft_delete/test_admin.py b/src/bpp/tests/test_soft_delete/test_admin.py index 925b56a00..9e40361ba 100644 --- a/src/bpp/tests/test_soft_delete/test_admin.py +++ b/src/bpp/tests/test_soft_delete/test_admin.py @@ -200,3 +200,71 @@ def test_usun_trwale_pomija_rekordy_spoza_kosza(superuser_client): assert resp.status_code == 200 assert Wydawnictwo_Ciagle.objects.filter(pk=pk).exists() assert "Pominięto rekordów spoza kosza: 1" in resp.content.decode("utf-8") + + +@pytest.mark.django_db +def test_akcja_usun_do_kosza_z_powodem_trafia_do_logu(superuser, superuser_client): + obj = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Z powodem") + pk = obj.pk + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + + # Krok 1: wybor akcji bez potwierdzenia -> strona posrednia z polem. + resp1 = superuser_client.post( + url, + {"action": "usun_do_kosza", "_selected_action": [str(pk)]}, + ) + assert resp1.status_code == 200 + tresc = resp1.content.decode("utf-8") + assert 'name="powod"' in tresc + assert "Z powodem" in tresc + # Nic sie jeszcze nie stalo: + assert Wydawnictwo_Ciagle.objects.filter(pk=pk).exists() + + # Krok 2: potwierdzenie z powodem. + resp2 = superuser_client.post( + url, + { + "action": "usun_do_kosza", + "_selected_action": [str(pk)], + "powod_potwierdzony": "1", + "powod": "Duplikat rekordu", + }, + ) + assert resp2.status_code in (200, 302) + + assert not Wydawnictwo_Ciagle.objects.filter(pk=pk).exists() + wpis = _log(Wydawnictwo_Ciagle, pk, "delete") + assert wpis is not None + assert wpis.powod == "Duplikat rekordu" + assert wpis.user_id == superuser.pk + + +@pytest.mark.django_db +def test_powod_dziedziczy_kaskada_na_autorstwa(superuser, superuser_client): + """Powod i user maja objac takze wiersze ``*_Autor`` kasowane kaskada. + + Kontekst fazy 06 obejmuje CALE cialo ``delete()``, wiec kaskada + dziedziczy atrybucje rodzica — bez tego wpisy autorstw byłyby niczyje + mimo swiadomej decyzji operatora. + """ + from bpp.models import Wydawnictwo_Ciagle_Autor + + obj = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Z autorem") + autorstwo = baker.make(Wydawnictwo_Ciagle_Autor, rekord=obj) + pk_autorstwa = autorstwo.pk + + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + superuser_client.post( + url, + { + "action": "usun_do_kosza", + "_selected_action": [str(obj.pk)], + "powod_potwierdzony": "1", + "powod": "Wycofanie calego rekordu", + }, + ) + + wpis = _log(Wydawnictwo_Ciagle_Autor, pk_autorstwa, "delete") + assert wpis is not None + assert wpis.powod == "Wycofanie calego rekordu" + assert wpis.user_id == superuser.pk diff --git a/src/django_bpp/templates/admin/bpp/soft_delete_powod.html b/src/django_bpp/templates/admin/bpp/soft_delete_powod.html new file mode 100644 index 000000000..5e038db10 --- /dev/null +++ b/src/django_bpp/templates/admin/bpp/soft_delete_powod.html @@ -0,0 +1,46 @@ +{% extends "admin/base_site.html" %} +{% load admin_urls %} + +{% block content %} +{# Strona pośrednia akcji "Usuń do kosza": pyta o powód operacji. #} +{# Powód wędruje przez reason= do SoftDeleteLog.powod (receiver fazy 06). #} + +

🗑️ Zaznaczone rekordy zostaną przeniesione do kosza.

+

+ Operacja jest odwracalna — rekordy wrócą akcją „♻️ Przywróć zaznaczone” + na liście z filtrem Kosz → 🗑️ Tylko skasowane. +

+ +

Do kosza trafi rekordów: {{ ile_obiektow }}.

+ +
    + {% for obj in obiekty %} +
  • {{ obj }}
    • + {% endfor %} +
+ +{% if ile_ukrytych %} + {# Przy zaznaczeniu "wszystkie pasujące" lista bywa ogromna — pokazujemy #} + {# próbkę, a nie materializujemy dziesiątek tysięcy rekordów na stronie. #} +

… oraz {{ ile_ukrytych }} dalszych (lista skrócona).

+{% endif %} + +
{% csrf_token %} + {% for pk in wybrane_pk %} + + {% endfor %} + + + + + +

+ +
+ +

+ + + Anuluj +
+{% endblock %} From 849189ac63e453a9167291538229b6716ecf3577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 25 Aug 2026 16:59:28 +0200 Subject: [PATCH 15/18] feat(soft-delete): kosz w pozostalych 5 adminach + szew recover reversion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wszedzie mixin wpiety jako OSTATNI przed baza terminalna, nie pierwszy. test_mixin_nie_znosi_zawezenia_do_uczelni_w_autoradmin jest strazem tej decyzji: przy wpieciu wg planu (PIERWSZY) get_queryset mixinu uciolby lancuch razem z SiteFilteredAdminMixin i personel jednej uczelni zobaczylby autorow drugiej. get_urls zostawia szew pod przyszly recover z django-reversion — recover wskrzesza rekord poza przeplywem soft-delete (bez WYSYLKA do PBN, bez SoftDeleteLog, bez przeliczenia punktacji, z pominieciem warunkowego unique na Autor.slug). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/admin/autor.py | 3 + src/bpp/admin/helpers/mixins.py | 14 ++++ src/bpp/admin/patent.py | 8 +- src/bpp/admin/praca_doktorska.py | 8 +- src/bpp/admin/praca_habilitacyjna.py | 8 +- src/bpp/admin/wydawnictwo_zwarte.py | 8 +- src/bpp/tests/test_soft_delete/test_admin.py | 83 ++++++++++++++++++++ 7 files changed, 128 insertions(+), 4 deletions(-) diff --git a/src/bpp/admin/autor.py b/src/bpp/admin/autor.py index 5330c8faf..662aa5b31 100644 --- a/src/bpp/admin/autor.py +++ b/src/bpp/admin/autor.py @@ -31,6 +31,7 @@ WydzialAutoraFilter, ) from .helpers.fieldsets import ADNOTACJE_FIELDSET, ZapiszZAdnotacjaMixin +from .helpers.mixins import BppSoftDeleteAdminMixin from .helpers.site_filtered import SiteFilteredAdminMixin from .helpers.widgets import CHARMAP_SINGLE_LINE from .xlsx_export import resources @@ -251,6 +252,8 @@ class AutorAdmin( EksportDanychMixin, BaseBppAdminMixin, DynamicColumnsMixin, + # OSTATNI przed baza terminalna — patrz docstring BppSoftDeleteAdminMixin. + BppSoftDeleteAdminMixin, admin.ModelAdmin, ): uczelnia_field_path = "aktualna_jednostka__uczelnia" diff --git a/src/bpp/admin/helpers/mixins.py b/src/bpp/admin/helpers/mixins.py index 4464b2125..f7f47469d 100644 --- a/src/bpp/admin/helpers/mixins.py +++ b/src/bpp/admin/helpers/mixins.py @@ -187,6 +187,20 @@ def get_queryset(self, request): qs = qs.order_by(*ordering) return qs + def get_urls(self): + """# SZEW reversion (odłożone). + + Gdy włączymy ``django-reversion``, jego widok „recover deleted" + (URL ``recover/``) musi tu zostać UKRYTY albo przekierowany na + ``restore()``. Recover wskrzesza rekord POZA przepływem + soft-delete: bez zlecenia ``WYSYLKA`` do PBN, bez wpisu + ``SoftDeleteLog``, bez przeliczenia punktacji i z pominięciem + warunkowego unique na ``Autor.slug`` (husk trzyma slug + zarezerwowany). Byłaby to druga, cicha ścieżka przywracania — + dokładnie to, czemu ta faza ma zapobiegać. + """ + return super().get_urls() + def get_list_filter(self, request): list_filter = list(super().get_list_filter(request) or []) if PokazSkasowaneFilter not in list_filter: diff --git a/src/bpp/admin/patent.py b/src/bpp/admin/patent.py index f37e8058b..152152d6e 100644 --- a/src/bpp/admin/patent.py +++ b/src/bpp/admin/patent.py @@ -21,7 +21,11 @@ POZOSTALE_MODELE_FIELDSET, AdnotacjeZDatamiMixin, ) -from .helpers.mixins import DomyslnyStatusKorektyMixin, Wycinaj_W_z_InformacjiMixin +from .helpers.mixins import ( + BppSoftDeleteAdminMixin, + DomyslnyStatusKorektyMixin, + Wycinaj_W_z_InformacjiMixin, +) from .wydawnictwo_zwarte import Wydawnictwo_ZwarteAdmin_Baza from .xlsx_export import resources from .xlsx_export.mixins import EksportDanychZFormatowanieMixin, ExportActionsMixin @@ -89,6 +93,8 @@ class Patent_Admin( AdnotacjeZDatamiMixin, EksportDanychZFormatowanieMixin, ExportActionsMixin, + # OSTATNI przed baza terminalna — patrz docstring BppSoftDeleteAdminMixin. + BppSoftDeleteAdminMixin, Wydawnictwo_ZwarteAdmin_Baza, ): djangoql_completion_enabled_by_default = False diff --git a/src/bpp/admin/praca_doktorska.py b/src/bpp/admin/praca_doktorska.py index 0d6626ebd..a92ef0b0e 100644 --- a/src/bpp/admin/praca_doktorska.py +++ b/src/bpp/admin/praca_doktorska.py @@ -32,7 +32,11 @@ POZOSTALE_MODELE_FIELDSET, AdnotacjeZDatamiMixin, ) -from .helpers.mixins import DomyslnyStatusKorektyMixin, Wycinaj_W_z_InformacjiMixin +from .helpers.mixins import ( + BppSoftDeleteAdminMixin, + DomyslnyStatusKorektyMixin, + Wycinaj_W_z_InformacjiMixin, +) from .wydawnictwo_ciagle import CleanDOIWWWPublicWWWMixin from .xlsx_export import resources from .xlsx_export.mixins import EksportDanychZFormatowanieMixin, ExportActionsMixin @@ -211,6 +215,8 @@ class Praca_DoktorskaAdmin( ConstanceScoringFieldsMixin, EksportDanychZFormatowanieMixin, ExportActionsMixin, + # OSTATNI przed baza terminalna — patrz docstring BppSoftDeleteAdminMixin. + BppSoftDeleteAdminMixin, Praca_Doktorska_Habilitacyjna_Admin_Base, ): resource_classes = [Praca_DoktorskaResource] diff --git a/src/bpp/admin/praca_habilitacyjna.py b/src/bpp/admin/praca_habilitacyjna.py index 7ea95a71a..bbfdf7a75 100644 --- a/src/bpp/admin/praca_habilitacyjna.py +++ b/src/bpp/admin/praca_habilitacyjna.py @@ -35,7 +35,11 @@ MODEL_ZE_SZCZEGOLAMI, POZOSTALE_MODELE_FIELDSET, ) -from .helpers.mixins import DomyslnyStatusKorektyMixin, Wycinaj_W_z_InformacjiMixin +from .helpers.mixins import ( + BppSoftDeleteAdminMixin, + DomyslnyStatusKorektyMixin, + Wycinaj_W_z_InformacjiMixin, +) from .praca_doktorska import Praca_Doktorska_Habilitacyjna_Admin_Base # @@ -208,6 +212,8 @@ class Praca_HabilitacyjnaAdmin( ConstanceScoringFieldsMixin, EksportDanychZFormatowanieMixin, ExportActionsMixin, + # OSTATNI przed baza terminalna — patrz docstring BppSoftDeleteAdminMixin. + BppSoftDeleteAdminMixin, Praca_Doktorska_Habilitacyjna_Admin_Base, ): resource_classes = [Praca_HabilitacyjnaResource] diff --git a/src/bpp/admin/wydawnictwo_zwarte.py b/src/bpp/admin/wydawnictwo_zwarte.py index d07cbff6b..edfaf974b 100644 --- a/src/bpp/admin/wydawnictwo_zwarte.py +++ b/src/bpp/admin/wydawnictwo_zwarte.py @@ -59,7 +59,11 @@ sprawdz_duplikaty_www_doi, ) from .helpers.constance_field_mixin import ConstanceScoringFieldsMixin -from .helpers.mixins import OptionalPBNSaveMixin, RestrictDeletionWhenPBNUIDSetMixin +from .helpers.mixins import ( + BppSoftDeleteAdminMixin, + OptionalPBNSaveMixin, + RestrictDeletionWhenPBNUIDSetMixin, +) from .nagroda import NagrodaInline # Proste tabele @@ -496,6 +500,8 @@ class Wydawnictwo_ZwarteAdmin( AdminCrossrefAPIMixin, AdminCrossrefPBNAPIMixin, RestrictDeletionWhenPBNUIDSetMixin, + # OSTATNI przed baza terminalna — patrz docstring BppSoftDeleteAdminMixin. + BppSoftDeleteAdminMixin, Wydawnictwo_ZwarteAdmin_Baza, ): change_list_template = "admin/bpp/wydawnictwo_zwarte/change_list.html" diff --git a/src/bpp/tests/test_soft_delete/test_admin.py b/src/bpp/tests/test_soft_delete/test_admin.py index 9e40361ba..a2c4d9c7c 100644 --- a/src/bpp/tests/test_soft_delete/test_admin.py +++ b/src/bpp/tests/test_soft_delete/test_admin.py @@ -268,3 +268,86 @@ def test_powod_dziedziczy_kaskada_na_autorstwa(superuser, superuser_client): assert wpis is not None assert wpis.powod == "Wycofanie calego rekordu" assert wpis.user_id == superuser.pk + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "model_name,admin_slug", + [ + ("Wydawnictwo_Zwarte", "wydawnictwo_zwarte"), + ("Patent", "patent"), + ("Praca_Doktorska", "praca_doktorska"), + ("Praca_Habilitacyjna", "praca_habilitacyjna"), + ("Autor", "autor"), + ], +) +def test_kosz_w_adminie_dla_kazdego_modelu(model_name, admin_slug, superuser_client): + import bpp.models + + model = getattr(bpp.models, model_name) + obj = baker.make(model) + pk = obj.pk + obj.delete(reason="test") + + # Changeform otwiera skasowany rekord (global_objects): + url_change = reverse(f"admin:bpp_{admin_slug}_change", args=[pk]) + assert superuser_client.get(url_change).status_code == 200 + + # Filtr kosza pokazuje skasowany, a akcje kosza sa oferowane: + url_list = reverse(f"admin:bpp_{admin_slug}_changelist") + "?is_deleted=true" + resp = superuser_client.get(url_list) + assert resp.status_code == 200 + assert b"przywroc_zaznaczone" in resp.content + assert b"usun_trwale_zaznaczone" in resp.content + assert b"usun_do_kosza" in resp.content + + # Przywracanie dziala: + resp_r = superuser_client.post( + url_list, + {"action": "przywroc_zaznaczone", "_selected_action": [str(pk)]}, + ) + assert resp_r.status_code in (200, 302) + assert model.objects.filter(pk=pk).exists() + + +@pytest.mark.django_db +def test_mixin_nie_znosi_zawezenia_do_uczelni_w_autoradmin(staff_user, rf): + """REGRESJA MRO: kosz nie ma prawa poszerzyc widoku poza wlasna uczelnie. + + Plan fazy 07 kazal wpinac ``BppSoftDeleteAdminMixin`` jako PIERWSZY. + Jego ``get_queryset`` nie woła ``super()`` (podmienia manager bazowy), + wiec z pierwszej pozycji uciolby cały łańcuch — w tym + ``SiteFilteredAdminMixin.get_queryset`` w ``AutorAdmin``, zawezajace + widok nie-superusera do jego uczelni (FD#390). Personel uczelni A + zobaczylby (i skasowal) autorow uczelni B. + + Ten test pilnuje, ze mixin stoi na koncu MRO — czyli ze jest PODSTAWA + lancucha, a nie jego obcieciem. + """ + from django.contrib.admin.sites import site + + from bpp.models import Autor, Autor_Jednostka, Jednostka, Uczelnia + + uczelnia_a = baker.make(Uczelnia, nazwa="Uczelnia A", skrot="UA") + uczelnia_b = baker.make(Uczelnia, nazwa="Uczelnia B", skrot="UB") + jednostka_a = baker.make(Jednostka, uczelnia=uczelnia_a) + jednostka_b = baker.make(Jednostka, uczelnia=uczelnia_b) + + autor_a = baker.make(Autor) + baker.make(Autor_Jednostka, autor=autor_a, jednostka=jednostka_a) + autor_b = baker.make(Autor) + baker.make(Autor_Jednostka, autor=autor_b, jednostka=jednostka_b) + autor_b_w_koszu = baker.make(Autor) + baker.make(Autor_Jednostka, autor=autor_b_w_koszu, jednostka=jednostka_b) + autor_b_w_koszu.delete(reason="test") + + request = rf.get("/admin/bpp/autor/") + request.user = staff_user + request._uczelnia = uczelnia_b + + qs = site._registry[Autor].get_queryset(request) + widoczne = set(qs.values_list("pk", flat=True)) + + assert autor_b.pk in widoczne, "wlasna uczelnia musi byc widoczna" + assert autor_b_w_koszu.pk in widoczne, "kosz wlasnej uczelni tez (global_objects)" + assert autor_a.pk not in widoczne, "CUDZA uczelnia NIE MOZE byc widoczna" From 865faa82c84034b265435795d36c914bcfb4be70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 25 Aug 2026 17:00:42 +0200 Subject: [PATCH 16/18] feat(soft-delete): admin lapie guard autora-z-pracami zamiast 500 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit usun_do_kosza to sciezka krytyczna: wlasna akcja nie przechodzi przez kolektor Django, ktory dla delete_selected zatrzymalby operacje na FK PROTECT — ProtectedError szedl prosto z Autor.delete() do 500. Komunikat bierzemy Z WYJATKU, nie piszemy wlasnego: guard zna liczbe i rodzaj powiazan i juz je opisuje po polsku, a drugi tekst rozjechalby sie z oryginalem przy pierwszej zmianie listy relacji. Lapanie jest per instancja — jeden zablokowany autor nie przewraca calego zaznaczenia. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- src/bpp/admin/helpers/mixins.py | 40 +++++++++++--- src/bpp/tests/test_soft_delete/test_admin.py | 58 ++++++++++++++++++++ 2 files changed, 89 insertions(+), 9 deletions(-) diff --git a/src/bpp/admin/helpers/mixins.py b/src/bpp/admin/helpers/mixins.py index f7f47469d..ed94f01ae 100644 --- a/src/bpp/admin/helpers/mixins.py +++ b/src/bpp/admin/helpers/mixins.py @@ -3,6 +3,7 @@ from django import forms from django.contrib import admin, messages from django.contrib.admin import helpers as admin_helpers +from django.db.models import ProtectedError from django.template.response import TemplateResponse from django.urls import reverse from django_softdelete.filters import SoftDeleteFilter @@ -248,6 +249,24 @@ def _powod_z_requestu(self, request): # --- kasowanie = kosz ----------------------------------------------- + def _soft_delete_jeden(self, request, obj, powod): + """Soft-delete jednego rekordu; ``False`` gdy guard fazy 04 zablokował. + + POKAZUJEMY KOMUNIKAT Z WYJĄTKU, nie własny. ``raise_if_has_protected_ + children`` zna liczbę i rodzaj powiązań i już je opisuje po polsku; + drugi tekst tutaj byłby kopią tej samej reguły, która rozjedzie się + z oryginałem przy pierwszej zmianie listy relacji. + + Łapiemy per instancja, żeby jeden zablokowany autor nie przewracał + całego zaznaczenia — reszta ma trafić do kosza. + """ + try: + obj.delete(user=request.user, reason=powod) + return True + except ProtectedError as e: + self.message_user(request, e.args[0], level=messages.ERROR) + return False + def delete_model(self, request, obj): """Przycisk „Usuń" na changeformie przenosi do kosza. @@ -256,7 +275,9 @@ def delete_model(self, request, obj): powstawałby z ``user=None``. """ with self._soft_delete_user_context(request): - obj.delete(user=request.user, reason=self._powod_z_requestu(request)) + # Guard rzuca ProtectedError takze tutaj — Django zrobi wtedy + # redirect z komunikatem bledu, a rekord zostanie. + self._soft_delete_jeden(request, obj, self._powod_z_requestu(request)) def delete_queryset(self, request, queryset): """Akcja ``delete_selected`` — soft-delete PER INSTANCJA. @@ -269,7 +290,7 @@ def delete_queryset(self, request, queryset): with self._soft_delete_user_context(request): powod = self._powod_z_requestu(request) for obj in queryset: - obj.delete(user=request.user, reason=powod) + self._soft_delete_jeden(request, obj, powod) # --- akcje kosza ---------------------------------------------------- @@ -334,13 +355,14 @@ def usun_do_kosza(self, request, queryset): powod = self._powod_z_requestu(request) usunieto = 0 for obj in queryset: - obj.delete(user=request.user, reason=powod) - usunieto += 1 - self.message_user( - request, - f"Przeniesiono do kosza: {usunieto}.", - level=messages.SUCCESS, - ) + if self._soft_delete_jeden(request, obj, powod): + usunieto += 1 + if usunieto: + self.message_user( + request, + f"Przeniesiono do kosza: {usunieto}.", + level=messages.SUCCESS, + ) return None ile = queryset.count() diff --git a/src/bpp/tests/test_soft_delete/test_admin.py b/src/bpp/tests/test_soft_delete/test_admin.py index a2c4d9c7c..f42879d2c 100644 --- a/src/bpp/tests/test_soft_delete/test_admin.py +++ b/src/bpp/tests/test_soft_delete/test_admin.py @@ -351,3 +351,61 @@ def test_mixin_nie_znosi_zawezenia_do_uczelni_w_autoradmin(staff_user, rf): assert autor_b.pk in widoczne, "wlasna uczelnia musi byc widoczna" assert autor_b_w_koszu.pk in widoczne, "kosz wlasnej uczelni tez (global_objects)" assert autor_a.pk not in widoczne, "CUDZA uczelnia NIE MOZE byc widoczna" + + +@pytest.mark.django_db +def test_soft_delete_autora_z_pracami_pokazuje_komunikat(superuser_client): + """Guard fazy 04 ma dawac komunikat, nie 500. + + ``usun_do_kosza`` jest tu sciezka krytyczna: wlasna akcja NIE + przechodzi przez kolektor Django (ktory dla ``delete_selected`` + zatrzymalby operacje na FK ``PROTECT``), wiec ``ProtectedError`` + wyleciałby prosto z ``Autor.delete()``. + """ + from bpp.models import Autor, Wydawnictwo_Ciagle_Autor + + autor = baker.make(Autor) + baker.make(Wydawnictwo_Ciagle_Autor, autor=autor) + + url = reverse("admin:bpp_autor_changelist") + resp = superuser_client.post( + url, + { + "action": "usun_do_kosza", + "_selected_action": [str(autor.pk)], + "powod_potwierdzony": "1", + "powod": "próba", + }, + follow=True, + ) + + assert resp.status_code == 200 + assert Autor.objects.filter(pk=autor.pk).exists() + tresc = resp.content.decode("utf-8").lower() + assert "nie można usunąć" in tresc + + +@pytest.mark.django_db +def test_guard_nie_blokuje_pozostalych_z_zaznaczenia(superuser_client): + """Jeden zablokowany rekord nie moze przewrocic calej operacji.""" + from bpp.models import Autor, Wydawnictwo_Ciagle_Autor + + zablokowany = baker.make(Autor) + baker.make(Wydawnictwo_Ciagle_Autor, autor=zablokowany) + wolny = baker.make(Autor) + + url = reverse("admin:bpp_autor_changelist") + resp = superuser_client.post( + url, + { + "action": "usun_do_kosza", + "_selected_action": [str(zablokowany.pk), str(wolny.pk)], + "powod_potwierdzony": "1", + "powod": "próba", + }, + follow=True, + ) + + assert resp.status_code == 200 + assert Autor.objects.filter(pk=zablokowany.pk).exists() + assert not Autor.objects.filter(pk=wolny.pk).exists() From 049c81a7649d7e8b6d7949812f4ff2792b7881a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 25 Aug 2026 17:17:39 +0200 Subject: [PATCH 17/18] test(soft-delete): domkniecie suity fazy 07 + newsfragment + handoff fazy 08 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Staff moze do kosza (z powodem, z atrybucja), ale akcja "Usun trwale" nie jest mu w ogole oferowana. delete_selected takze idzie przez nasz delete_queryset, czyli do kosza. Weryfikacja: 21 testow fazy, 197 w rodzinie soft-delete, 845 w regresji adminow (z Playwrightem), 9669 w make tests-without-playwright. Jedyna porazka to znany timeout test_0499_odwracalna pod -n auto — serialnie 32,98 s (faza 06 mierzyla 31,88/32,33 s), a faza 07 nie dokłada zadnej migracji. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- .../HANDOFF-soft-delete-faza-08.md | 328 ++++++++++++++++++ .../soft-delete-admin-kosz.feature.rst | 7 + src/bpp/tests/test_soft_delete/test_admin.py | 55 +++ 3 files changed, 390 insertions(+) create mode 100644 docs/superpowers/HANDOFF-soft-delete-faza-08.md create mode 100644 src/bpp/newsfragments/soft-delete-admin-kosz.feature.rst diff --git a/docs/superpowers/HANDOFF-soft-delete-faza-08.md b/docs/superpowers/HANDOFF-soft-delete-faza-08.md new file mode 100644 index 000000000..7780d5b42 --- /dev/null +++ b/docs/superpowers/HANDOFF-soft-delete-faza-08.md @@ -0,0 +1,328 @@ +# Handoff: soft-delete, start fazy 08 + +> Po zamknięciu **fazy 07** (kosz w adminie: filtr, przywracanie, trwałe +> usuwanie, powód), 2026-08-25. Czytaj to zamiast odtwarzania historii z gita. + +--- + +## 1. Gdzie jesteśmy + +| | | +|---|---| +| Stan fazy 07 | gałąź `feat/soft-delete-06` (faza 07 dopisana na tej samej gałęzi) | +| Plan fazy 07 | [`plans/2026-06-04-soft-delete-07-admin.md`](plans/2026-06-04-soft-delete-07-admin.md) — wykonany, z odstępstwami z §3 | +| Migracje fazy 07 | **ŻADNE.** Mixin admina nie dotyka schematu | +| Następny plan | [`plans/2026-06-04-soft-delete-08-testy-regresji.md`](plans/2026-06-04-soft-delete-08-testy-regresji.md) — 11 tasków, faza czysto testowa | + +### Stos PR-ów + +``` +#312 feat/soft-delete -> dev +#745 feat/soft-delete-04 -> feat/soft-delete +#755 feat/soft-delete-05 -> feat/soft-delete-04 +#767 feat/soft-delete-05b -> feat/soft-delete-05 + feat/soft-delete-06 -> feat/soft-delete-05b (fazy 06 ORAZ 07) +``` + +⚠️ Żaden PR ze stosu nie jest scalony. Bez zmian względem handoffu fazy 07. + +⚠️ **Baseline (`baseline-sql/`) nadal NIEŚWIEŻY** — stoi na `bpp/0487`, ostatnia +migracja to `0503` (faza 06). Faza 07 nic tu nie zmieniła. Odświeżenie robi się +**raz, przy scalaniu** całego `feat/soft-delete` do `dev`. + +⚠️ **CI NIE URUCHAMIA SIĘ na PR-ach do gałęzi `feat/soft-delete*`** — bez zmian. +Jedyną weryfikacją jest przebieg lokalny. + +### Wynik weryfikacji fazy 07 + +| Przebieg | Wynik | +|---|---| +| `pytest src/bpp/tests/test_soft_delete/test_admin.py` (suita fazy) | **21 passed** | +| `pytest src/bpp/tests/test_soft_delete/` (cała rodzina soft-delete) | **197 passed, 1 xfailed** | +| `pytest src/bpp/tests/ -k admin` (regresja adminów, z Playwrightem) | **845 passed** | +| `make tests-without-playwright` (`-n auto`) | **9669 passed, 1 failed, 4 skipped, 2 xfailed** (5:32) | +| `ruff check` / `ruff format --check` na plikach fazy | czysto (9 plików) | +| `pre-commit` na plikach fazy + oba hooki szablonowe | Passed | + +Jedyna porażka to `test_soft_delete/test_kronika_usunieta.py::test_0499_odwracalna` +— `Failed: Timeout (>90.0s)`, NIE asercja. **To nie jest regresja fazy 07:** + +- serialnie ten test przechodzi w **32,98 s** (zmierzone po fazie 07); faza 06 + mierzyła 31,88 s na `05b` i 32,33 s na `06` — identycznie co do szumu, +- faza 07 **nie dokłada żadnej migracji**, a to jest test odwracalności migracji + `0499`; nie wykonuje ani jednej operacji admina, +- to jeden z DWÓCH testów wskazanych już w handoffie fazy 07 §1 jako mające + ~2× zapasu do limitu 90 s i przekraczające go pod `-n auto` (10 workerów) na + współdzielonym hoście. Drugi (`pbn_api/…/test_migracja_dyscypliny_uuid_e2e`) + tym razem przeszedł — co potwierdza, że rzecz jest w obciążeniu, nie w kodzie. + +Kandydat do podniesienia `@pytest.mark.timeout` dla tych dwóch plików — +**wciąż poza zakresem**, teraz już drugą fazę z rzędu. + +--- + +--- + +## 2. Co faza 07 dostarcza (kontrakt dla fazy 08) + +Jedna klasa, `src/bpp/admin/helpers/mixins.py`: + +```python +from bpp.admin.helpers.mixins import BppSoftDeleteAdminMixin, PokazSkasowaneFilter +``` + +| Element | Uwaga | +|---|---| +| `BppSoftDeleteAdminMixin` | wpięty w 6 adminów; **OSTATNI** na liście baz, patrz §3.3 | +| `PokazSkasowaneFilter` | `?is_deleted=true` = tylko kosz, `=all` = wszystko, brak = tylko żywe | +| `_soft_delete_user_context(request)` | JEDEN punkt wstrzyknięcia usera; deleguje do `soft_delete_context` | +| `_soft_delete_jeden(request, obj, powod)` | soft-delete jednej instancji z obsługą guarda fazy 04 | +| akcja `usun_do_kosza` | strona pośrednia z polem „powód" → `SoftDeleteLog.powod` | +| akcja `przywroc_zaznaczone` | restore przez ten sam hook | +| akcja `usun_trwale_zaznaczone` | superuser-only, **tylko rekordy z kosza** | +| `templates/admin/bpp/soft_delete_powod.html` | faktyczna ścieżka: `src/django_bpp/templates/…` | +| fixture `staff_user` / `staff_client` | `src/conftest.py`; staff z grupą „wprowadzanie danych" | + +Objęte adminy: `Wydawnictwo_CiagleAdmin`, `Wydawnictwo_ZwarteAdmin`, +`Patent_Admin`, `Praca_DoktorskaAdmin`, `Praca_HabilitacyjnaAdmin`, `AutorAdmin`. + +**Poza zakresem (świadomie):** `Zgloszenie_Publikacji` — szósty model +soft-delete znaleziony w fazie 06 (§4 tamtego handoffu). Nie ma admina kosza, +bo plan fazy 07 obejmował 5 publikacji + `Autor`. Operacje na nim i tak trafiają +do `SoftDeleteLog` (receivery są podpięte bez `sender=`). + +--- + +## 3. ⚠️ Cztery miejsca, w których plan rozjechał się z kodem + +Wszystkie wykryte PRZED napisaniem kodu, przez weryfikację „PINNED kontraktów" +planu wobec źródeł. Trzy to błędy rzeczowe w planie, jeden — luka bezpieczeństwa. + +### 3.1 API atrybucji z planu nie istnieje (a overview mówił to wprost) + +Plan §„Kontrakty z fazą 06 (PINNED — używaj VERBATIM)" pinuje +`set_soft_delete_user` / `get_soft_delete_user` / `clear_soft_delete_user` +w `bpp/models/soft_delete.py`. **Żadna z tych funkcji nie istnieje.** Faza 06 +dostarczyła context manager `soft_delete_context(user=, reason=)` w +`bpp/models/soft_delete_context.py`. + +Co jest tu warte zapamiętania: **overview ostrzegał przed dokładnie tym błędem** +(`2026-06-04-soft-delete-00-overview.md:187` — „NIE wymyślać osobnego +`set/get/clear_soft_delete_user` — używać `soft_delete_context`"). Plan fazy 07 +powstał wcześniej i nie został zsynchronizowany. Etykieta „PINNED — używaj +VERBATIM" była więc *silniejsza* niż jej pokrycie w kodzie — to ta sama lekcja, +co `Autor.pbn_uid` w fazie 06 §3.1 („zweryfikowane" ≠ zweryfikowane). + +### 3.2 `hard_delete()` nie przyjmuje `user=` ani `reason=` + +Plan (Task 4) wołał `obj.hard_delete(user=request.user, reason=powod)` → +`TypeError`. `BppPkPrzedHardDeleteMixin.hard_delete(*args, **kwargs)` przekazuje +argumenty prosto do pakietu, który o userze nie wie. + +**Skutek projektowy, nie kosmetyczny:** to jest powód, dla którego +`_soft_delete_user_context` MUSI być context managerem, a nie „przekaż `user=` +do metody". Dla trwałego usuwania kontekst jest JEDYNYM kanałem atrybucji. + +### 3.3 „Mixin PIERWSZY" otwierało lukę wielotenantową (blokujące) + +Plan powtarza w Task 1 i Task 6: `BppSoftDeleteAdminMixin` ma być **PIERWSZY** +na liście baz, „żeby jego metody wygrywały". + +`get_queryset` tego mixinu **nie może wołać `super()`** — musi podmienić manager +bazowy na `global_objects`, a `super()` zwraca już przefiltrowany `objects`. +Z pierwszej pozycji w MRO ucina więc CAŁY łańcuch. W `AutorAdmin` w tym łańcuchu +stoi `SiteFilteredAdminMixin.get_queryset`, zawężające widok nie-superusera do +jego uczelni (`kiedykolwiek_zwiazani`, FD#390). Wpięcie wg planu dałoby +personelowi uczelni A widok i kasowanie autorów uczelni B. + +**Rozstrzygnięcie:** mixin wpinany jako **OSTATNI**, tuż przed terminalną bazą +(`admin.ModelAdmin` / `Wydawnictwo_ZwarteAdmin_Baza` / +`Praca_Doktorska_Habilitacyjna_Admin_Base`). Z tej pozycji jego `get_queryset` +jest PODSTAWĄ łańcucha — zwraca `global_objects`, a wszystkie mixiny wyżej +nakładają na to swoje filtry normalnie. Zweryfikowane: w tych 6 łańcuchach +jedynym innym `get_queryset(self, request)` jest ten z `SiteFilteredAdminMixin` +i woła `super()`; `get_actions` / `get_list_filter` w łańcuchu +(`AutorAdmin.get_actions`, `ExportActionsMixin.get_actions`) też wołają `super()` +i tylko DOKŁADAJĄ pozycje — więc z ostatniej pozycji działają tak samo. + +Strażnikiem jest `test_admin.py::test_mixin_nie_znosi_zawezenia_do_uczelni_w_autoradmin`. +**Nie usuwaj go przy refaktorze MRO** — to jedyne miejsce, które zapala się, +gdy ktoś „posprząta" kolejność baz z powrotem do wersji z planu. + +### 3.4 Plan odwracał semantykę parametru filtra na podstawie błędnego odczytu + +Plan (Task 1, Step 5) twierdzi, że pakietowy `SoftDeleteFilter` ma mylącą +semantykę (`'true'` → „Deleted Softly" przez mapę `{'true': False}`) i każe +odwrócić ją tak, by `?is_deleted=false` znaczyło „pokaż kosz". + +Pakiet czyta się jednak inaczej niż plan go przeczytał: `{'true': False}` trafia +do `filter(deleted_at__isnull=value)`, czyli `deleted_at IS NOT NULL` — a więc +`is_deleted=true` JUŻ znaczy „rekordy skasowane". Odwrócenie sprawiłoby, że +parametr w URL-u kłamie (bookmark `is_deleted=false` pokazywałby kosz). + +**Rozstrzygnięcie:** zostaje semantyka pakietu. `PokazSkasowaneFilter` zmienia +wyłącznie zachowanie DOMYŚLNE (brak parametru = tylko żywe zamiast wszystkiego), +bo po poszerzeniu `get_queryset` do `global_objects` nie ma już nikogo innego, +kto by kosz schował. + +--- + +## 4. Decyzje wykraczające poza plan (i ich uzasadnienia) + +### 4.1 Trwałe usuwanie WYŁĄCZNIE z kosza + +Plan nie ograniczał `usun_trwale_zaznaczone` do rekordów skasowanych. Ogranicza +je faza 07 — i nie jest to ostrożnościowy rytuał: + +`Cache_Punktacja_*` nie ma FK do publikacji (klucz to tablica +`[content_type_id, pk]`), więc nie sprząta jej ani kolektor Django, ani triggery. +Kasuje ją dopiero receiver `post_soft_delete` (faza 06, `_skasuj_punktacje`). +Twarde skasowanie rekordu Z POMINIĘCIEM kosza zostawiłoby więc wiersze punktacji +wskazujące na nieistniejący rekord — **i wciąż liczące się do ewaluacji**. +Rekordy spoza kosza są pomijane z komunikatem. + +To jest kandydat na test regresji w fazie 08 (Task 2/7 dotykają dokładnie tej +materii). + +### 4.2 Akcje działają na querysecie changelisty, nigdy na `deleted_objects` + +`przywroc_zaznaczone` można było zrobić „wygodniej": dociągnąć zaznaczone pk +z `Model.deleted_objects`, żeby akcja działała niezależnie od aktywnego filtra. +**Świadomie tego nie robimy** — dociąganie po pk omija każde zawężenie nałożone +przez łańcuch, w tym `SiteFilteredAdminMixin`. Personel jednej uczelni mógłby +przywrócić rekord drugiej, podając jego pk. + +Koszt: przywracanie wymaga wcześniejszego ustawienia filtra „🗑️ Tylko +skasowane". Puste zaznaczenie tłumaczy komunikat, zamiast cicho zwrócić +„przywrócono: 0". + +### 4.3 Strona pośrednia przenosi `select_across`, nie tylko pk-i + +Django przy `select_across=1` IGNORUJE `_selected_action` i podaje akcji cały +przefiltrowany queryset. Gdyby strona z powodem re-postowała same zaznaczone +pk-i, „zaznacz wszystkie 40 000 pasujących" skurczyłoby się po cichu do 100 +wierszy bieżącej strony — a operator byłby przekonany, że skasował całość. +Dlatego formularz niesie `select_across` i `index`. + +Podgląd listy jest przycięty do `LIMIT_PODGLADU_KOSZA = 50`; licznik idzie +z `.count()`, więc liczba pozostaje prawdziwa przy skróconej liście. + +### 4.4 Komunikat guarda bierzemy z wyjątku, nie piszemy własnego + +`raise_if_has_protected_children` (faza 04) zna liczbę i rodzaj powiązań i już +je opisuje po polsku. Drugi tekst w adminie byłby kopią tej samej reguły — +rozjechałby się z oryginałem przy pierwszej zmianie listy relacji. Admin +pokazuje `e.args[0]`. + +Łapanie jest **per instancja**: jeden zablokowany autor nie przewraca całego +zaznaczenia. + +--- + +## 5. Bug znaleziony po drodze, NIE naprawiony (poza zakresem) + +**Staff bez żadnej grupy dostaje 500 na każdej stronie admina.** + +`src/django_bpp/menu.py:302` robi bezwarunkowe +`del menu.children[-1].children[-1]`, podczas gdy `flt(...)` kilka linii wyżej +dokłada submenu tylko członkom grupy. Użytkownik `is_staff=True` bez grup ma +`menu.children == []` → `IndexError: list assignment index out of range`. + +Reprodukcja: dowolny staff bez grup, dowolna strona `/admin/…`. + +Fixture `staff_user` obchodzi to, dopisując użytkownika do grupy +`GR_WPROWADZANIE_DANYCH` — co przy okazji modeluje realnego redaktora, więc nie +jest to obejście „na siłę". Ale **bug istnieje w produkcji** i wywróci się przy +pierwszym koncie staff założonym bez grupy. Naprawa jest jednolinijkowa +(warunek na niepustą listę), tylko nie należy do soft-delete. + +--- + +## 6. Czego faza 07 NIE domyka (świadomie) + +- **Brak admina dla samego `SoftDeleteLog`.** Log powstaje, ale nie ma widoku, + w którym operator obejrzy „co, kto i dlaczego usunął". Plan fazy 07 tego nie + obejmował (mimo że handoff fazy 06 §6 zapowiadał „UI dla logu — faza 07"). + **To jest realna luka w domknięciu fazy** — bez niej `powod` zapisywany przez + `usun_do_kosza` jest widoczny wyłącznie przez ORM. +- **`Zgloszenie_Publikacji` bez kosza w adminie** — patrz §2. +- **Masowe operacje nadal synchroniczne.** Handoff fazy 06 §7 mierzył + `restore()` z przeliczeniem punktacji na 46,7 ms/rekord; 100 rekordów ≈ 4,7 s + w jednym żądaniu HTTP. `hard_delete()` na querysecie to N zapytań (faza 06, + Task 8). Zadanie w tle nadal nie istnieje — przy zaznaczeniu „wszystkie + pasujące" na dużej bazie akcja się wywali na timeoucie. +- **Rekord z `pbn_uid` — asymetria uprawnień.** `RestrictDeletionWhenPBNUIDSetMixin` + nadal zwraca `has_delete_permission=False` dla rekordu z `pbn_uid`, więc + przycisk „Usuń" na changeformie i `delete_selected` są dla niego niedostępne. + Własna akcja `usun_do_kosza` **działa** (akcje w Django nie przechodzą przez + `has_delete_permission`). Jest to zgodne z intencją planu („soft-delete rekordu + z `pbn_uid` realizujemy mimo to" — faza 05 istnieje właśnie po to, żeby + zakolejkować `WYCOFANIE`), ale dwie ścieżki do tej samej operacji mają różne + reguły dostępu. Jeśli to ma być spójne, właściwym miejscem jest jawne + `has_soft_delete_permission()` — plan je zapowiadał, ale nigdy nie zdefiniował. + +--- + +## 7. Dług nadal otwarty (z faz 01–06) + +Bez zmian względem handoffu fazy 07, z dwiema pozycjami zamkniętymi: + +| Sprawa | Stan | +|---|---| +| **Brak UI dla `ProtectedError`** | **ZAMKNIĘTE w fazie 07** (§4.4) | +| **`Autor` nie ma kosza w adminie** | **ZAMKNIĘTE w fazie 07** | +| **Brak admina dla `SoftDeleteLog`** | **NOWE/otwarte** — patrz §6 | +| **Staff bez grupy = 500 w adminie** | **NOWE/otwarte** — patrz §5 | +| **Asymetria gate'u `.update(deleted_at=...)`** | `BppSoftDeleteQuerySet` blokuje, `BppDeletedQuerySet` **nie** | +| **Kaskada `Jednostka` → `Autor`** | `aktualna_jednostka`/`aktualna_funkcja` nadal `CASCADE` | +| **`Autor.slug` `unique=True` bezwarunkowo** | husk trzyma slug zarezerwowany | +| **Wycieki ORM (kanarek `xfail(strict=True)`)** | bez zmian | +| **PR upstream `django-easy-audit`** | [#348](https://github.com/soynatan/django-easy-audit/pull/348) | +| **`SoftDeleteLog` nie niesie `pbn_uid`** | dług z 05a §6 nadal otwarty | +| **`pbn_status=""` dwuznaczne** | bez zmian (faza 06 §6) | +| **`uczelnia_rekordu()` → `None` przy dwuznaczności** | bez zmian | +| **Reguła przynależności zduplikowana z `cerif_export`** | bez zmian | +| Słowniki (`Zrodlo`, `Konferencja`, `Projekt`, `Jednostka`) bez soft-delete | bez zmian | +| Pomiar `0492` i narzutu GiST | wciąż nikt nie zmierzył | +| **Strategia wydania** | bramka nadal otwarta | + +--- + +## 8. Wskazówki wprost dla fazy 08 (suita regresji E2E) + +- **Fixture `staff_user`/`staff_client` już są** (`src/conftest.py`). Staff ma + uprawnienia modelowe do `wydawnictwo_ciagle` i `autor` oraz grupę + „wprowadzanie danych". Jeśli faza 08 potrzebuje staffu dla innego modelu — + poszerz `content_type__model__in`, nie twórz drugiego fixture'u. +- **`_log(model, pk, akcja)`** w `test_soft_delete/test_admin.py` filtruje + `SoftDeleteLog` po `content_type` ORAZ `object_id`. Plan fazy 07 filtrował po + samym `object_id` — to nie jest unikalne globalnie, a kaskada fazy 02 loguje + przy okazji wiersze `*_Autor`, które łatwo trafiają na tę samą wartość `pk`. + Skopiuj ten helper, nie wzorzec z planu. +- **Task 2 i Task 7 planu fazy 08** (`Cache_Punktacja_*`, ewaluacja) dotykają + dokładnie tej materii, co decyzja §4.1. Test „hard-delete rekordu żywego + zostawia osierocone wiersze punktacji" byłby dowodem, że ograniczenie jest + potrzebne — dziś opiera się na lekturze kodu, nie na eksperymencie. +- **Task 4 planu fazy 08** (regresja `SoftDeleteLog`) częściowo pokrywa się + z `test_soft_delete/test_admin.py` — sprawdź, zanim napiszesz duplikat. + +--- + +## 9. Proces — co się sprawdziło w fazie 07 + +- **Weryfikacja „PINNED" kontraktów planu wobec źródeł zwróciła się natychmiast.** + Cztery rozjazdy z §3, wszystkie wykryte przed napisaniem pierwszej linii kodu. + Najgroźniejszy (§3.3, luka wielotenantowa) nie zapaliłby się w żadnym teście + z planu — plan testował wyłącznie superuserem, a dla superusera + `SiteFilteredAdminMixin` i tak jest no-opem. **Test na regresję MRO trzeba było + dopisać, bo w planie go nie było.** +- **Overview był aktualniejszy niż plan fazy.** §3.1 to błąd, przed którym + overview ostrzegał explicite. Przy rozjeździe plan-vs-overview zaufaj temu, + co potwierdzasz w kodzie, a overview czytaj ZANIM zaczniesz task. +- **Pierwszy „padający test" bywa padający z niewłaściwego powodu.** Test akcji + „Przywróć" padał nie dlatego, że akcji nie było, ale dlatego, że akcja + dostawała puste zaznaczenie (domyślny filtr chowa kosz). Rozpoznanie tego + ujawniło realną własność bezpieczeństwa (§4.2), której plan nie widział. +- **Pomiar „co faktycznie robi pakiet" bije cytat z planu.** §3.4 — plan + streszczał zachowanie `SoftDeleteFilter` i streścił je odwrotnie. Trzy linie + kodu pakietu rozstrzygnęły sprawę w minutę. diff --git a/src/bpp/newsfragments/soft-delete-admin-kosz.feature.rst b/src/bpp/newsfragments/soft-delete-admin-kosz.feature.rst new file mode 100644 index 000000000..6727b572b --- /dev/null +++ b/src/bpp/newsfragments/soft-delete-admin-kosz.feature.rst @@ -0,0 +1,7 @@ +W panelu admina pojawił się „kosz" dla pięciu typów publikacji i dla +autorów: usunięcie rekordu jest odwracalne, filtr „Kosz" pokazuje rekordy +skasowane, akcja „♻️ Przywróć zaznaczone" je odzyskuje, a akcja „🗑️ Usuń +do kosza (z powodem)" zapisuje w dzienniku operacji powód usunięcia wraz +z osobą, która je wykonała. Nieodwracalne „❌ Usuń TRWALE" jest dostępne +wyłącznie dla superużytkownika i wyłącznie dla rekordów już będących +w koszu. diff --git a/src/bpp/tests/test_soft_delete/test_admin.py b/src/bpp/tests/test_soft_delete/test_admin.py index f42879d2c..aec707b50 100644 --- a/src/bpp/tests/test_soft_delete/test_admin.py +++ b/src/bpp/tests/test_soft_delete/test_admin.py @@ -409,3 +409,58 @@ def test_guard_nie_blokuje_pozostalych_z_zaznaczenia(superuser_client): assert resp.status_code == 200 assert Autor.objects.filter(pk=zablokowany.pk).exists() assert not Autor.objects.filter(pk=wolny.pk).exists() + + +@pytest.mark.django_db +def test_staff_moze_do_kosza_ale_nie_trwale(staff_user, staff_client): + """Podzial uprawnien: kosz dla staffu, nieodwracalne kasowanie nie.""" + obj = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Staff do kosza") + pk = obj.pk + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + + resp = staff_client.post( + url, + { + "action": "usun_do_kosza", + "_selected_action": [str(pk)], + "powod_potwierdzony": "1", + "powod": "staff kasuje", + }, + follow=True, + ) + assert resp.status_code == 200 + + # Kosz, nie fizyczne skasowanie: + assert not Wydawnictwo_Ciagle.objects.filter(pk=pk).exists() + assert Wydawnictwo_Ciagle.global_objects.filter(pk=pk).exists() + + wpis = _log(Wydawnictwo_Ciagle, pk, "delete") + assert wpis is not None + assert wpis.user_id == staff_user.pk + assert wpis.powod == "staff kasuje" + + # Akcja trwalego usuwania nie jest staffowi w ogole oferowana: + resp_list = staff_client.get(url + "?is_deleted=true") + assert b"usun_trwale_zaznaczone" not in resp_list.content + assert b"przywroc_zaznaczone" in resp_list.content + + +@pytest.mark.django_db +def test_delete_selected_tez_idzie_do_kosza(superuser, superuser_client): + """Domyslna akcja Django przechodzi przez nasz delete_queryset.""" + obj = baker.make(Wydawnictwo_Ciagle, tytul_oryginalny="Przez delete_selected") + pk = obj.pk + url = reverse("admin:bpp_wydawnictwo_ciagle_changelist") + + resp = superuser_client.post( + url, + {"action": "delete_selected", "_selected_action": [str(pk)], "post": "yes"}, + follow=True, + ) + assert resp.status_code == 200 + assert not Wydawnictwo_Ciagle.objects.filter(pk=pk).exists() + assert Wydawnictwo_Ciagle.global_objects.filter(pk=pk).exists() + + wpis = _log(Wydawnictwo_Ciagle, pk, "delete") + assert wpis is not None + assert wpis.user_id == superuser.pk From 027a2ba195e9e05833a5575ca5c9d0757e60f70c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 25 Aug 2026 23:28:03 +0200 Subject: [PATCH 18/18] docs(soft-delete): numer PR #792 w stosie handoffu fazy 08 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Galaz feat/soft-delete-06 (fazy 06 i 07) byla jedynym ogniwem stosu bez numeru PR — bo faza 06 nigdy nie zostala wypchnieta. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Drz3jfnuP864JmYxqfjsYK --- docs/superpowers/HANDOFF-soft-delete-faza-08.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/HANDOFF-soft-delete-faza-08.md b/docs/superpowers/HANDOFF-soft-delete-faza-08.md index 7780d5b42..c89ad8cb2 100644 --- a/docs/superpowers/HANDOFF-soft-delete-faza-08.md +++ b/docs/superpowers/HANDOFF-soft-delete-faza-08.md @@ -21,10 +21,13 @@ #745 feat/soft-delete-04 -> feat/soft-delete #755 feat/soft-delete-05 -> feat/soft-delete-04 #767 feat/soft-delete-05b -> feat/soft-delete-05 - feat/soft-delete-06 -> feat/soft-delete-05b (fazy 06 ORAZ 07) +#792 feat/soft-delete-06 -> feat/soft-delete-05b (fazy 06 ORAZ 07) ``` -⚠️ Żaden PR ze stosu nie jest scalony. Bez zmian względem handoffu fazy 07. +⚠️ Żaden PR ze stosu nie jest scalony. Nowość względem handoffu fazy 07: +gałąź `feat/soft-delete-06` (fazy 06 **i** 07) jest wypchnięta i ma wreszcie +własny PR — [#792](https://github.com/iplweb/bpp/pull/792). Wcześniej była +jedynym ogniwem stosu bez numeru, bo faza 06 nigdy nie została wypchnięta. ⚠️ **Baseline (`baseline-sql/`) nadal NIEŚWIEŻY** — stoi na `bpp/0487`, ostatnia migracja to `0503` (faza 06). Faza 07 nic tu nie zmieniła. Odświeżenie robi się