fix(pbn): WillNotExportError to brak danych, nie awaria — czytelny komunikat

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(

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.

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 "<script>alert(1)</script>" 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 "<script>" not in text
+    assert "&lt;script&gt;" in text