diff --git a/src/bpp/models/autor.py b/src/bpp/models/autor.py index 77cfef78c..9180d7ed7 100644 --- a/src/bpp/models/autor.py +++ b/src/bpp/models/autor.py @@ -15,7 +15,7 @@ RangeOperators, ) from django.contrib.postgres.search import SearchVectorField as VectorField -from django.core.exceptions import ValidationError +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.core.validators import RegexValidator from django.db import IntegrityError, models, transaction from django.db.models import CASCADE, SET_NULL, Count, Func, Q, Sum @@ -816,6 +816,9 @@ class Meta: # patrz migracja 0444_deferred_podstawowe_miejsce_pracy. def __str__(self): + komunikat = f"Budowanie reprezentacji tekstowej Autor_Jednostka (pk={self.pk})" + fallback = f"Autor_Jednostka #{self.pk if self.pk else 'nowy'}" + try: autor_str = str(self.autor) if self.autor_id else "???" jednostka_str = self.jednostka.skrot if self.jednostka_id else "???" @@ -824,13 +827,30 @@ def __str__(self): if self.funkcja_id and self.funkcja: buf = f"{autor_str} ↔ {self.funkcja.nazwa}, {jednostka_str}" return buf + except ObjectDoesNotExist: + # SPODZIEWANE, nie błąd aplikacji: str() bywa wołany na obiekcie, + # który wciąż żyje w pamięci, choć jego wiersz — i wiersz po + # drugiej stronie FK — już zniknął. Najpewniejszy znany nam + # wywołujący to audyt easyaudit, liczący ``object_repr`` w + # ``transaction.on_commit`` (ten sam mechanizm opisuje komentarz + # przy ``Jednostka.__str__``); traceback z Rollbara nie zawiera + # ramek wywołującego, więc nie zgadujemy dalej. + # + # Nie raportujemy tego do Rollbara: hash itemu obejmuje numer + # linii, więc KAŻDY deploy zakładał nowy item i alert szedł od + # nowa, mimo że aplikacja zachowywała się poprawnie. + # + # Uwaga: logger ``bpp.*`` nie ma dziś własnego handlera w + # ustawieniach, więc ten ślad ląduje na stderr przez + # ``logging.lastResort``. Diagnostyka jest zatem słaba — ale to + # osobny temat (konfiguracja LOGGING), nie powód, by zostawiać + # fałszywy alarm w Rollbarze. + zaloguj_polkniety_wyjatek(komunikat, logger=logger, do_rollbar=False) + return fallback except Exception: - zaloguj_polkniety_wyjatek( - f"Budowanie reprezentacji tekstowej Autor_Jednostka (pk={self.pk})", - logger=logger, - ) - # Fallback w przypadku jakichkolwiek błędów podczas usuwania - return f"Autor_Jednostka #{self.pk if self.pk else 'nowy'}" + # Cokolwiek innego jest naprawdę nieoczekiwane — raportuj. + zaloguj_polkniety_wyjatek(komunikat, logger=logger) + return fallback def clean(self, exclude=None): if self.rozpoczal_prace is not None and self.zakonczyl_prace is not None: diff --git a/src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst b/src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst new file mode 100644 index 000000000..afdad9d32 --- /dev/null +++ b/src/bpp/newsfragments/+autor-jednostka-str-wiszacy-fk.bugfix.rst @@ -0,0 +1,2 @@ +Usunięto fałszywy alarm w monitoringu błędów, zgłaszany przy kasowaniu autora. +Zachowanie samego kasowania nie ulega zmianie. diff --git a/src/bpp/tests/test_models/test_autor_jednostka_str.py b/src/bpp/tests/test_models/test_autor_jednostka_str.py new file mode 100644 index 000000000..c2b1b7b22 --- /dev/null +++ b/src/bpp/tests/test_models/test_autor_jednostka_str.py @@ -0,0 +1,70 @@ +"""Reprezentacja tekstowa ``Autor_Jednostka`` przy wiszącej referencji. + +Po skasowaniu autora ``str()`` bywa wołany na obiekcie ``Autor_Jednostka``, +który wciąż żyje w pamięci, choć jego wiersz (i wiersz autora) już zniknął — +m.in. przez audyt ``easyaudit``, który liczy ``object_repr`` w +``transaction.on_commit`` (patrz analogiczny komentarz w +``bpp/models/jednostka.py``). ``__str__`` ma na to fallback i się nie wywala, +ale raportował ten spodziewany przypadek do Rollbara jako błąd (#1098, #450). + +Testujemy OBIE gałęzie rozdzielenia: spodziewany ``ObjectDoesNotExist`` → +log bez Rollbara, wszystko inne → raport jak dotąd. +""" + +import logging + +import pytest + +from bpp.models.autor import Autor, Autor_Jednostka + + +@pytest.mark.django_db +def test_str_autor_jednostka_dziala_normalnie(autor_jan_kowalski, jednostka): + aj = Autor_Jednostka.objects.create(autor=autor_jan_kowalski, jednostka=jednostka) + + assert str(autor_jan_kowalski) in str(aj) + assert jednostka.skrot in str(aj) + + +@pytest.mark.django_db +def test_str_autor_jednostka_po_skasowaniu_autora_nie_raportuje( + autor_jan_kowalski, jednostka, mocker, caplog +): + """Wisząca referencja po kaskadzie → fallback, ale BEZ raportu do Rollbara.""" + report = mocker.patch("bpp.util.wyjatki.rollbar.report_exc_info") + + Autor_Jednostka.objects.create(autor=autor_jan_kowalski, jednostka=jednostka) + # Świeży obiekt z bazy — bez podpiętego w pamięci cache'u FK ``autor``, + # dokładnie tak jak w produkcyjnym tracebacku. + aj = Autor_Jednostka.objects.get(autor=autor_jan_kowalski, jednostka=jednostka) + Autor.objects.filter(pk=autor_jan_kowalski.pk).delete() + + with caplog.at_level(logging.ERROR, logger="bpp.models.autor"): + assert str(aj) == f"Autor_Jednostka #{aj.pk}" + + assert not report.called, ( + "Spodziewana wisząca referencja nie powinna iść do Rollbara" + ) + # Wyciszamy Rollbara, NIE diagnostykę — ślad w logu musi zostać. + assert any("Autor_Jednostka" in r.message for r in caplog.records) + + +@pytest.mark.django_db +def test_str_autor_jednostka_nadal_raportuje_nieoczekiwane_bledy( + autor_jan_kowalski, jednostka, mocker +): + """Druga gałąź: cokolwiek INNEGO niż DoesNotExist nadal idzie do Rollbara. + + Bez tego testu nic nie broni przed cofnięciem całego sensu tej zmiany — + zamianą ``except ObjectDoesNotExist`` z powrotem na ``except Exception``. + """ + report = mocker.patch("bpp.util.wyjatki.rollbar.report_exc_info") + aj = Autor_Jednostka.objects.create(autor=autor_jan_kowalski, jednostka=jednostka) + + # Awaria, która NIE jest wiszącą referencją — np. uszkodzony rekord autora. + mocker.patch( + "bpp.models.autor.Autor.__str__", side_effect=RuntimeError("coś padło") + ) + + assert str(aj) == f"Autor_Jednostka #{aj.pk}" + assert report.called, "Nieoczekiwany błąd MUSI trafić do Rollbara"