fix(pbn): WillNotExportError to brak danych, nie awaria — czytelny komunikat#679
Open
mpasternak wants to merge 2 commits into
Open
fix(pbn): WillNotExportError to brak danych, nie awaria — czytelny komunikat#679mpasternak wants to merge 2 commits into
mpasternak wants to merge 2 commits into
Conversation
…munikat sprobuj_wyslac_do_pbn ma osobną gałąź except dla każdego spodziewanego przypadku (SameDataUploadedRecently, AccessDeniedException, PKZeroExportDisabled, NeedsPBNAuthorisationException, ResourceLockedException, PBNValidationError) — każda z konkretnym komunikatem dla redaktora i bez raportu do Rollbara. Pozostałe podklasy WillNotExportError (DOIorWWWMissing, LanguageMissingPBNUID, StatementsMissing, CharakterFormalnyMissingPBNUID) takiej gałęzi nie miały, więc wpadały do `except Exception` — opatrzonego komentarzem "w sumie nie wiadomo, co to za problem na tym etapie". Skutki: redaktor dostawał generyczne "Kod błędu: Musi być DOI lub adres WWW", a Rollbar item na każde wystąpienie (#1475, #1473), mimo że system działał poprawnie, a poprawka leży po stronie danych rekordu. Komunikat wyjątku jest już napisany po polsku i wprost mówi, czego brakuje — podajemy go redaktorowi wprost (escape'owany), z prośbą o uzupełnienie danych. PKZeroExportDisabled zostaje na swojej wcześniejszej gałęzi: to podklasa WillNotExportError, ale ma inny, konfiguracyjny komunikat. Kolejność gałęzi to gwarantuje, a istniejący test tego pilnuje. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH
- Twierdziłem w opisie, że "istniejący test pilnuje kolejności gałęzi". NIEPRAWDA: ciąg "Eksport prac z PK=0" nie występował w żadnym teście, a testy kolejki idą ścieżką raise_exceptions=True, gdzie obie gałęzie robią `raise e` — są więc na kolejność niewrażliwe. Przestawienie gałęzi nie wywaliłoby niczego. Dokładam PKZeroExportDisabled do parametryzacji; zweryfikowane mutacją (przeniesienie WillNotExportError ponad PKZeroExportDisabled → test pada). - escape(e) dało się usunąć bez żadnego czerwonego testu. Komunikaty zawierają dane sterowane przez użytkownika (LanguageMissingPBNUID wstrzykuje nazwę języka, CharakterFormalnyMissingPBNUID nazwę charakteru formalnego — oba ze słowników edytowalnych w adminie), a admin/base.html renderuje komunikaty przez `|safe`. Dokładam test escapowania, lustro istniejącego ..._validation_error_escapuje_html. - Sufiks "Uzupełnij dane rekordu" był mylący dla dwóch przypadków: CharakterFormalnyMissingPBNUID (poprawka w słowniku, nie w rekordzie) oraz gołego WillNotExportError podnoszonego przez Uczelnia.pbn_client() przy braku autoryzacji w PBN — ta ścieżka JEST osiągalna wewnątrz try, bo authorize() woła się leniwie przy 403. Teraz: "Popraw dane rekordu lub konfigurację". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
sprobuj_wyslac_do_pbnma osobną gałąźexceptdla każdego spodziewanegoprzypadku —
SameDataUploadedRecently,AccessDeniedException,PKZeroExportDisabled,NeedsPBNAuthorisationException,ResourceLockedException,PBNValidationError— każda z konkretnymkomunikatem dla redaktora i bez raportu do Rollbara.
Pozostałe podklasy
WillNotExportError(DOIorWWWMissing,LanguageMissingPBNUID,StatementsMissing,CharakterFormalnyMissingPBNUID)takiej gałęzi nie miały, więc wpadały do
except Exception— tegoz komentarzem:
Skutki:
błędu: Musi być DOI lub adres WWW" zamiast czytelnej instrukcji.
(#1475,
#1473) — mimo że
system działał poprawnie, a poprawka leży po stronie danych rekordu.
Warto zaznaczyć: kolejka eksportu (
pbn_export_queue/models.py) klasyfikuje tesame wyjątki poprawnie, jako
RodzajBledu.MERYTORYCZNY. Rozjazd był wyłączniew ścieżce z panelu admina.
Rozwiązanie
Osobna gałąź
except WillNotExportError, tuż przedexcept Exception.Komunikat wyjątku jest już napisany po polsku i wprost mówi, czego brakuje —
podajemy go redaktorowi (escape'owany), z prośbą o uzupełnienie danych.
PKZeroExportDisabledzostaje na swojej wcześniejszej gałęzi: to teżpodklasa
WillNotExportError, ale ma inny, konfiguracyjny komunikat („eksportprac z PK=0 jest wyłączony"). Kolejność gałęzi to gwarantuje, a istniejący
test_sprobuj_wyslac_do_pbn_pk_zerotego pilnuje.Testy
Nowy parametryzowany
test_sprobuj_wyslac_do_pbn_will_not_export_czytelny_komunikat_bez_rollbar(dla
DOIorWWWMissingiLanguageMissingPBNUID) — TDD, oba przypadki najpierwpadały na
assert "Kod błędu" not in text. Wzorowany na istniejącymtest_sprobuj_wyslac_do_pbn_validation_error_czytelny_komunikat_bez_rollbar,czyli na konwencji, którą repo już stosuje dla tej samej klasy problemu.
src/pbn_api/tests/test_bpp_admin_helpers.py— 18 passedsrc/pbn_export_queue— 166 passed (klasyfikacja w kolejce bez zmian)🤖 Generated with Claude Code
https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH