diff --git a/src/bpp/admin/helpers/pbn_api/common.py b/src/bpp/admin/helpers/pbn_api/common.py index 041996def..d0d7d75de 100644 --- a/src/bpp/admin/helpers/pbn_api/common.py +++ b/src/bpp/admin/helpers/pbn_api/common.py @@ -16,6 +16,7 @@ PraceSerwisoweException, ResourceLockedException, SameDataUploadedRecently, + WillNotExportError, ) from pbn_api.models import SentData @@ -246,6 +247,36 @@ def sprobuj_wyslac_do_pbn( # noqa: C901 return + except WillNotExportError as e: + # Rekordu nie da się wysłać, dopóki ktoś czegoś nie uzupełni — a NIE + # awaria kodu. Komunikat wyjątku jest już napisany po polsku i wprost + # mówi, czego brakuje, więc podajemy go redaktorowi zamiast + # generycznego „Kod błędu: …" z gałęzi niżej. + # + # Bez tej gałęzi wpadało to do `except Exception`, opatrzonego + # komentarzem „nie wiadomo, co to za problem" — i szło do Rollbara + # (itemy Rollbar #1475, #1473), mimo że system działał poprawnie. + # + # „lub konfigurację" w komunikacie jest celowe: ta gałąź łapie też + # przypadki, w których poprawka NIE leży w rekordzie — + # `CharakterFormalnyMissingPBNUID` (słownik charakterów formalnych) + # oraz gołe `WillNotExportError` podnoszone przez + # `Uczelnia.pbn_client()` przy braku autoryzacji w PBN. + # + # UWAGA NA KOLEJNOŚĆ: `PKZeroExportDisabled` (też podklasa + # WillNotExportError) ma własną gałąź WYŻEJ i musi tam zostać — ma + # inny, konfiguracyjny komunikat. Pilnuje tego parametryzacja + # testu `..._will_not_export_czytelny_komunikat_bez_rollbar`. + notificator.warning( + f'Rekord "{link_do_obiektu(obj)}" nie zostanie wysłany do PBN: {escape(e)}. ' + f"Popraw dane rekordu lub konfigurację i spróbuj ponownie. " + f"{open_in_pbn_link}{open_in_pi_link}" + ) + if raise_exceptions: + raise e + + return + except Exception as e: try: link_do_wyslanych = reverse( diff --git a/src/bpp/newsfragments/+pbn-will-not-export-czytelny-komunikat.bugfix.rst b/src/bpp/newsfragments/+pbn-will-not-export-czytelny-komunikat.bugfix.rst new file mode 100644 index 000000000..df685f06e --- /dev/null +++ b/src/bpp/newsfragments/+pbn-will-not-export-czytelny-komunikat.bugfix.rst @@ -0,0 +1,5 @@ +Przy wysyłce rekordu do PBN z panelu redagowania brakujące dane dają teraz +konkretny komunikat („Musi być DOI lub adres WWW", „Brak odpowiednika języka +w PBN") zamiast generycznego „Kod błędu: …". Dotyczy sytuacji, w których +rekordu po prostu nie da się wysłać, dopóki nie uzupełni się danych — +to nie jest awaria systemu i nie jest już jako awaria zgłaszane. diff --git a/src/pbn_api/tests/test_bpp_admin_helpers.py b/src/pbn_api/tests/test_bpp_admin_helpers.py index 4cdc648b6..278a9301b 100644 --- a/src/pbn_api/tests/test_bpp_admin_helpers.py +++ b/src/pbn_api/tests/test_bpp_admin_helpers.py @@ -21,7 +21,13 @@ PBN_GET_INSTITUTION_STATEMENTS, PBN_GET_PUBLICATION_BY_ID_URL, ) -from pbn_api.exceptions import AccessDeniedException, PBNValidationError +from pbn_api.exceptions import ( + AccessDeniedException, + DOIorWWWMissing, + LanguageMissingPBNUID, + PBNValidationError, + PKZeroExportDisabled, +) from pbn_api.models import Publication, SentData from pbn_api.tests.utils import middleware @@ -511,3 +517,87 @@ def test_sprobuj_wyslac_do_pbn_przychodzi_inny_pbn_uid_dla_starego_rekordu( msg = get_messages(req) assert "Wg danych z PBN zmodyfikowano PBN UID tego rekordu " in list(msg)[0].message + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "wyjatek,fragment_komunikatu", + [ + (DOIorWWWMissing("Musi być DOI lub adres WWW"), "Musi być DOI lub adres WWW"), + (LanguageMissingPBNUID("Brak odpowiednika języka"), "Brak odpowiednika języka"), + # PKZeroExportDisabled JEST podklasą WillNotExportError, ale ma własną + # gałąź WYŻEJ. Ten przypadek pilnuje kolejności gałęzi — bez niego + # przestawienie ich nie wywaliłoby żadnego testu, a redaktor + # dostawałby zły komunikat. + ( + PKZeroExportDisabled("nieużywane, liczy się komunikat z gałęzi"), + "Eksport prac z PK=0 jest wyłączony", + ), + ], +) +def test_sprobuj_wyslac_do_pbn_will_not_export_czytelny_komunikat_bez_rollbar( + pbn_wydawnictwo_zwarte_z_charakterem, + pbn_client, + rf, + pbn_uczelnia, + mocker, + wyjatek, + fragment_komunikatu, +): + """``WillNotExportError`` to brak danych w rekordzie, nie awaria kodu. + + Regresja (Rollbar #1475, #1473): te wyjątki wpadały do gałęzi + ``except Exception``, opatrzonej komentarzem „nie wiadomo, co to za + problem" — redaktor dostawał generyczne „Kod błędu: …", a Rollbar item + per wystąpienie. Tymczasem to zwykły komunikat walidacyjny: brakuje DOI, + brakuje odpowiednika języka w PBN. Sąsiednie gałęzie (``PKZeroExportDisabled``, + ``PBNValidationError``) od dawna robią to poprawnie. + """ + req = rf.get("/") + + report = mocker.patch("bpp.admin.helpers.pbn_api.common.rollbar.report_exc_info") + mocker.patch.object(pbn_client, "sync_publication", side_effect=wyjatek) + + with middleware(req): + sprobuj_wyslac_do_pbn_gui( + req, pbn_wydawnictwo_zwarte_z_charakterem, pbn_client=pbn_client + ) + + text = list(get_messages(req))[0].message + assert fragment_komunikatu in text, ( + "Redaktor musi zobaczyć KONKRETNY powód, nie 'Kod błędu: ...'" + ) + assert "Kod błędu" not in text + report.assert_not_called() # brak danych w rekordzie to NIE błąd kodu + + +@pytest.mark.django_db +def test_sprobuj_wyslac_do_pbn_will_not_export_escapuje_html( + pbn_wydawnictwo_zwarte_z_charakterem, pbn_client, rf, pbn_uczelnia, mocker +): + """Komunikat wyjątku trafia do `{{ message|safe }}` — musi być escape'owany. + + Treść NIE jest w pełni nasza: ``LanguageMissingPBNUID`` wstrzykuje nazwę + języka, a ``CharakterFormalnyMissingPBNUID`` nazwę charakteru formalnego — + oba ze słowników edytowalnych w adminie. Lustro istniejącego + ``..._validation_error_escapuje_html``. + """ + req = rf.get("/") + + mocker.patch("bpp.admin.helpers.pbn_api.common.rollbar.report_exc_info") + mocker.patch.object( + pbn_client, + "sync_publication", + side_effect=LanguageMissingPBNUID( + 'Język "" nie ma odpowiednika w PBN' + ), + ) + + with middleware(req): + sprobuj_wyslac_do_pbn_gui( + req, pbn_wydawnictwo_zwarte_z_charakterem, pbn_client=pbn_client + ) + + text = list(get_messages(req))[0].message + assert "