feat(pbn_api): interaktywne narzędzie pbn_test_wysylka_interaktywna (Faza 1)#164
Open
mpasternak wants to merge 48 commits into
Open
feat(pbn_api): interaktywne narzędzie pbn_test_wysylka_interaktywna (Faza 1)#164mpasternak wants to merge 48 commits into
mpasternak wants to merge 48 commits into
Conversation
Gałąź dev po zmerge-owaniu PR #115 (fix/fd-316) i innych ma dwa równoległe "0005_*" nody w aplikacji importer_publikacji: - 0005_alter_importsession_created_by - 0005_importsession_wydawnictwo_nadrzedne Oba dziedziczą po 0004_rename_user_to_created_by_add_modified_by i robią niezależne operacje (alter FK vs add dwa nowe FK). Konflikt blokował test suite (makemigrations --check failuje). Migracja merge-owa 0006_merge_* łączy obie linie bez zmiany istniejących plików migracji.
Dodano interaktywny Django management command pozwalający krok po kroku przetestować pełen flow wysyłki publikacji i oświadczeń do PBN. Dla każdego żądania HTTP pokazuje metodę, URL i body; dla odpowiedzi — status i treść (skrócone, z opcją pełnego wyświetlenia). Narzędzie nie modyfikuje lokalnej bazy BPP — używa wyłącznie niższych metod klienta PBN (transport.post/delete/get_pages) oraz istniejących helperów (delete_all_publication_statements, post_discipline_statements). Posiada tryb --dry-run (pokazuje bez wysyłania) oraz --yes-all (auto- -akceptacja Enter / yes-no z defaultem). Flow: wybór publikacji → generowanie JSON (z oznaczeniem czy zawiera statements) → wybór endpointa /api/v1/publications albo repozytoryjnego /api/v1/repositorium/publications (z wymuszeniem usunięcia klucza "statements" zgodnie ze spec PBN) → POST publikacji → GET aktualnych oświadczeń w PBN → porównanie z lokalnymi → (opcjonalnie) DELETE + POST /v2/institution-profile/statements. Po każdym błędzie HTTP wypisuje czytelny komunikat i wraca do menu/podsumowania bez crash-a. Służy jako faza 1 docelowej refaktoryzacji sync_publication — baza empiryczna do zbadania jak PBN reaguje na poszczególne kombinacje endpointów zanim zmienimy kolejność operacji DELETE→POST→DOWNLOAD w src/pbn_api/client/publication_sync.py (problem: nieudana wysyłka publikacji powodowała utratę oświadczeń w profilu instytucji). Plan kompletnej poprawki: docs/pbn-wysylka-plan.md. Flaga .docker-build: żeby CI zbudował obraz do testów ręcznych. 13 testów jednostkowych pokrywa: walidację argumentów, dry-run, happy path dla obu endpointów, obsługę błędów HTTP, quit na wybór endpointa, różnice oświadczeń i decyzje użytkownika (zgoda / odmowa DELETE+POST).
Do tej pory workflow build-docker-images.yml miał trigger na push do
master + feature/** + fix/** + hotfix/** ORAZ na pull_request events.
W rezultacie dla każdego commita w branchu z otwartym PR-em odpalały
się dwa niezależne buildy (push event i pull_request synchronize),
każdy po ~6 minut na Docker Cloud Build — marnowanie minut.
Concurrency group nie grupował tych runów w jedno, bo github.ref jest
różny dla push ("refs/heads/feature/...") i pull_request
("refs/pull/XXX/merge").
Zmiana: push trigger tylko dla master (release flow). Branch feature/**,
fix/**, hotfix/** buduje się wyłącznie przez pull_request events
(opened/synchronize/reopened/labeled), z .docker-build w drzewie lub
labelem docker-build na PR — tak jak wcześniej. Ad-hoc build branchy
bez PR-a pozostaje dostępny przez workflow_dispatch
(`gh workflow run build-docker-images.yml --ref <branch>`).
Efekt: jeden build per commit w PR zamiast dwóch.
…tywna Dopisano do docs/pbn-wysylka-plan.md sekcję "Jak testować narzędzie — krok po kroku" z konkretnymi instrukcjami uruchomienia: 1. Testy jednostkowe (bez PBN) 2. Smoke test na preprod PBN w trybie --dry-run 3. Rzeczywisty test na preprod PBN (z --user-token lub zaciągniętym z Uczelnia.pbn_api_user) 4. Uruchomienie w obrazie Docker zbudowanym przez CI 5. Opis każdego z 8 kroków flow (co robi, co pyta, co pokazuje) 6. Checklist obserwacji do zebrania w preprod (dla Fazy 2) 7. Rozwiązywanie typowych problemów (brak tokena, brak uczelni, DaneLokalneWymagajaAktualizacjiException, HTTP 423, konflikt migracji) Poprzednia sekcja "CLI argumenty" miała błędny argument `--user` — zastąpiona prawdziwym `--user-token` (z PBNBaseCommand) plus opcją automatycznego zaciągania tokena z Uczelnia.pbn_api_user. Dopisano też notatkę o zmianie w workflow build-docker-images.yml (one-build-per-commit).
… pytaj osobno o DELETE/POST
Dwa buggi zgłoszone przez usera po testowaniu w preprod:
BUG 1 — fałszywa identyczność porównania. Narzędzie porównywało
OswiadczenieInstytucji (lokalny cache PBN) z aktualnym stanem PBN.
Gdy user zmienił coś w rekordzie (skasował autora/dyscyplinę),
cache pozostawał nieaktualny — narzędzie pokazywało 3 identyczne
oświadczenia mimo że BPP by wysłał tylko 2.
Fix: _step_compare_statements używa teraz intencji BPP live —
WydawnictwoPBNAdapter(publication).pbn_get_api_statements() —
zamiast cache'a. Obsługa DaneLokalneWymagajaAktualizacjiException
(zwraca "różnice" ze stosownym komunikatem). KROK 1/8 pokazuje
zarówno cache count jak i intencję live, żeby od razu widać było
ewentualny rozjazd.
BUG 2 — brak kontroli przy identyczności. Gdy porównanie zwróciło
"identyczne", _run_flow robił wczesny return i kończył flow bez
pytania usera. User nie mógł wymusić DELETE+POST żeby sprawdzić
empirycznie reakcję PBN.
Fix: zawsze pytamy osobno o DELETE i osobno o POST. Default zależy
od wyniku porównania (n dla identycznych, t dla różnic), ale pytanie
jest zadane w obu przypadkach. Usunięto redundantne wewnętrzne
prompty ("Wyślij DELETE?", "Wyślij POST?") z _step_delete_statements
i _step_post_statements — zastąpione jednym pytaniem wyższego
poziomu w _run_flow.
Testy:
- Zaktualizowano listy inputów w _patch_input dla nowego flow
(dwa dodatkowe pytania DELETE/POST zawsze zadawane).
- Dodano helper _patch_intended_statements dla mockowania adaptera
w testach (fixture nie tworzy PublikacjaInstytucji_V2).
- Dodano regression test test_compare_uses_intended_not_cache_bug1
pilnujący że cache OswiadczenieInstytucji nie jest już używany
w porównaniu (weryfikacja przez asercję na output KROK 6/8).
Wszystkie 14 testów przechodzi lokalnie.
Oba testy failowały na dev w CI (nie dotyczą zmian z tej gałęzi, ale blokowały zielony pipeline). test_sprobuj_wyslac_do_pbn_celery (bpp/tests/test_admin_helpers/ test_pbn_api_cli.py): - fixture wydawnictwo_ciagle nie miał DOI/WWW, przez co adapter WydawnictwoPBNAdapter rzucał DOIorWWWMissing zanim dojdziemy do etapu testującego NeedsPBNAuthorisationException → ustawiamy DOI na początku testu. - cli.py sprawdza user.pbn_token is None, ale model BppUser ma default="" (nie None) → test musi wymusić admin_user.pbn_token = None in-memory przed sprawdzeniem NeedsPBNAuthorisationException. test_check_pbn_skip_when_client_fails (importer_publikacji/tests/ test_pbn_check.py): - @patch("importer_publikacji.views._get_pbn_client") nie działał, bo _get_pbn_client jest importowany LOKALNIE w ciele funkcji (from .providers.pbn import _get_pbn_client), więc nie jest atrybutem modułu views. Fix: patchować u źródła importer_publikacji.providers.pbn._get_pbn_client — równoważne, bo lokalny import w funkcji złapie zpatchowaną wersję. Żadnej zmiany w kodzie produkcyjnym (auth logic w cli.py zachowana bez zmian — fix wyłącznie w testach).
Klucz porównania zwracał różne struktury dla intencji vs PBN:
- intencja (z pbn_get_api_statements) miała tylko personObjectId,
a disciplineId był usuwany przez _convert_stmt gdy obecne było
disciplineUuid — output pokazywał klucze typu ('mongoId', '', None)
- PBN GET /page/statements zwraca (personId, area, institutionId) —
klucze typu ('mongoId', '301', 'inst_uuid')
Ze strony usera wyglądało to tak że wszystkie 3 oświadczenia były
różne (intencja had "" dla area, PBN had "301"), choć były
faktycznie identyczne.
Fix:
- Użyj pbn_get_json_statements() (surowa lista, przed konwersją
_convert_stmt) zamiast pbn_get_api_statements()["statements"] —
zachowuje disciplineId (int, numerek MNiSW).
- Klucz porównania: (person-mongoId, discipline-numerek) z mapowaniem:
- person: PBN.personId ↔ adapter.personObjectId
- discipline: PBN.area ↔ adapter.disciplineId (oba = numerek MNiSW)
- Pominięto institutionId w kluczu (zawsze taka sama uczelnia dla
wszystkich statementów per rekord).
Testy: helper _patch_intended_statements teraz patchuje obie metody
(pbn_get_json_statements dla KROK 6/8, pbn_get_api_statements dla
KROK 8/8) i ustawia pbn_wysylaj_bez_oswiadczen=True na __init__
(żeby StatementsMissing nie wywalił KROK 2/8 gdy intencja jest
pusta — artykuł/rozdział bez statements jest walidowany jako błąd,
a w testach chcemy móc symulować każdy stan).
Po merge origin/dev (pull dev-a z nowszymi commitami) pojawił się drugi plik 0006_merge — obecna gałąź miała 0006_merge_20260420_2212 (mój merge z 20-04), dev wniósł 0006_merge_20260421_1100 (z dev-a, commit 71fe245). Django odrzucał start kontenera: CommandError: Conflicting migrations detected; multiple leaf nodes in the migration graph: (0006_merge_20260420_2212, 0006_merge_20260421_1100 in importer_publikacji). Nowa migracja 0007_merge_20260421_1248 łączy oba leaf-y w jeden node. Nie modyfikuje istniejących plików migracji.
…pliny_selektywnie Commit 1/8 refaktoryzacji sync_publication — model + exception class. - Nowa klasa StatementsResendFailedException (pbn_api/exceptions.py) - Nowe pole Uczelnia.pbn_kasuj_dyscypliny_selektywnie BooleanField default=True - Usunięto pole Uczelnia.pbn_api_kasuj_przed_wysylka (obsolete) - Migracja 0414: RemoveField + AddField - Zaktualizowane referencje: admin/uczelnia.py, setup_wizard/forms+tests, pbn_integrator cli, common.py (usunięto delete_statements_before_upload z wywołania sync_publication) - Dodane noqa dla pre-existing ruff issues (C901 sprobuj_wyslac_do_pbn + handle pbn_integrator, E402 dla imports po django.setup()). UP031 (format specifier) naprawiony w common.py przez f-string. sync_publication nadal ma parametr delete_statements_before_upload (zostanie zignorowany w kolejnych commitach gdy dodam split flow).
…ode removal Commit 2/8 refaktoryzacji sync_publication. Zmiany w publication_sync.py: - Usunięto ``post_publication()`` — wrapper na stary /api/v1/publications - Usunięto ``_should_retry_validation_error()`` — retry loop oparty o status 400 z /api/v1/publications (endpoint nie będzie używany) - Usunięto ``_retry_download_publication()`` — wywoływany tylko z ww. - Usunięto nieużywany import PBN_POST_PUBLICATIONS_URL - ``_prepare_publication_json``: zawsze wywołuje ``convert_js_with_statements_to_no_statements`` (bo endpoint repo wymaga konwersji formatu niezależnie od obecności statements). Zwraca tylko ``js`` (było: ``(js, bez_oswiadczen)``). - ``_post_publication_data``: zawsze używa ``post_publication_no_statements``. Usunięto parametr ``bez_oswiadczen``. - ``upload_publication``: zawsze wysyła przez endpoint repo. Usunięto retry loop (był tylko dla starego /api/v1/publications). Sygnatura zachowana (``max_retries_on_validation_error`` teraz deprecated/ignorowany). Return value ``(objectId, ret, js, True)`` — ostatnie pole zawsze True. - Fix pre-existing bug ``_delete_statements_with_retry``: warunek ``if no_tries < 0`` zmieniony na ``<= 0``. Wcześniej dla max_tries=5 wykonywał się 6 razy (iteracja 0,1,2,3,4,5). Teraz dokładnie 5. Ten commit jeszcze NIE dodaje split flow dla statements w sync_publication — to zostanie zrobione w Commit 4. Między Commitem 2 a 4 testy sync statements są tymczasowo złamane (sync_publication ignoruje ``download_statements_of_publication`` bo bez_oswiadczen zawsze True).
Commit 3/8 refaktoryzacji sync_publication — dodanie helperów do nowej logiki synchronizacji oświadczeń (będą użyte w Commit 4). Dodane w PublicationSyncMixin: - ``_statement_key_pbn(stmt)`` / ``_statement_key_intended(stmt)`` — staticmethods, zwracają klucz porównania ``(person mongoId, discipline numerek)`` jako tuple stringów. Mapowanie: PBN GET response używa ``personId``+``area``, adapter ``pbn_get_json_statements`` używa ``personObjectId``+``disciplineId`` — oba oznaczają to samo. - ``_diff_statements(pbn, intended)`` — oblicza różnice między PBN a intencją BPP. Zwraca ``(only_in_pbn, only_in_intended)`` — sety kluczy do DELETE/POST. - ``_get_pbn_statements_with_retry(objectId, publication_pk)`` — GET aktualnych oświadczeń z PBN, retry x3 z exponential backoff 2s/4s/8s. Po wyczerpaniu: rollbar.report_exc_info(level="warning") + raise ``StatementsResendFailedException``. - ``_delete_statements_selective(objectId, stmts, publication_pk)`` — per-osoba DELETE (``delete_publication_statement(personId, role)``) dla każdego oświadczenia do usunięcia. Retry x3 per oświadczenie. - ``_delete_statements_batch(objectId, publication_pk)`` — delete_all z retry. Propaguje ``CannotDeleteStatementsException`` do callera (akceptowalne gdy PBN mówi że oświadczeń nie ma). - ``_post_statements_with_retry(rec, objectId, publication_pk)`` — POST ``/api/v2/institution-profile/statements`` z payloadem z ``pbn_get_api_statements``. Wymaga lokalnego ``PublikacjaInstytucji_V2`` (propaguje ``DaneLokalneWymagajaAktualizacjiException``). Retry x3. - ``_sync_statements_with_pbn(rec, objectId, kasuj_selektywnie, notificator)`` — orchestrator: GET → diff → selective/batch DELETE → POST. Obsługa 4 scenariuszy: identyczne (nic), PBN+BPP= (DELETE), PBN=+BPP (POST), różnice (DELETE+POST). - ``_report_statements_failure_and_raise()`` — wspólny helper do raportowania błędów retry do Rollbar (level=warning) + raise StatementsResendFailedException. - ``_STATEMENT_RETRY_DELAYS = (2, 4, 8)`` — stała exponential backoff. Import ``StatementsResendFailedException`` dodany. Helpery nie są jeszcze wywoływane przez ``sync_publication`` — to zostanie zrobione w Commit 4.
…ronizacja statements) Commit 4/8 refaktoryzacji sync_publication. Główna zmiana logiki. Flow po zmianie: 1. POST publikacji przez ``/api/v1/repositorium/publications`` (zawsze) 2. download_publication — pobierz Publication lokalnie 3. Aktualizacja SentData.pbn_uid 4. Obsługa zmiany/konfliktu PBN UID (_handle_uid_change/_conflict) 5. download_statements_of_publication + pobierz_publikacje_instytucji_v2 (konieczne bo _post_statements_with_retry wymaga V2 lokalnie) 6. _sync_statements_with_pbn — GET aktualnych w PBN, diff z intencją BPP, selektywne/batch DELETE + POST brakujących. Tryb sterowany ``Uczelnia.pbn_kasuj_dyscypliny_selektywnie``. Zmiany: - Usunięto pre-upload DELETE (``delete_statements_before_upload`` parametr ignorowany, pozostaje w sygnaturze deprecated dla backward compat) - Usunięto specjalny notificator dla "bez oświadczeń" (teraz nie rozróżniamy) - Dodano obsługę ``DaneLokalneWymagajaAktualizacjiException`` przy synchronizacji statements — log warning + kontynuacja (publikacja została już wysłana w KROK 1, brak V2 lokalnie to nie fatal). Ważna semantyka błędów: - POST publikacji fail → wyjątek propagowany; statements w PBN nietknięte - POST OK + retry statements wyczerpany → ``StatementsResendFailedException`` (klasyfikowany jako RETRY_LATER w pbn_export_queue — do zrobienia w Commit 7)
Commit 5/8 refaktoryzacji sync_publication — usunięcie parametru ``delete_statements_before_upload`` z callerów ``sync_publication`` (common.py został już zrobiony w Commit 1). Zmiany: - ``_synchronizuj_pojedyncza_publikacje`` — usunięty parametr i przekazywanie do ``client.sync_publication()`` - ``synchronizuj_publikacje`` — jw., usunięte 3 wywołania wewnętrzne - ``pbn_integrator`` CLI — usunięty argparse ``--delete-statements-before-upload`` i handling w ``handle()`` Parametr ``delete_statements_before_upload`` pozostaje w sygnaturze ``sync_publication`` jako deprecated (żeby nie złamać żadnego callera którego mogłem przeoczyć).
Commit 6/8 refaktoryzacji sync_publication.
Pliki zmodyfikowane:
- test_client_sync.py — przepisany całkowicie, 17 testów (było 7):
* 5 happy paths (to same id, tekstowo podane, nowe id, z 0 PK,
bez istniejacej Publication lokalnie)
* 1 test że upload zawsze idzie przez endpoint repo
* 5 scenariuszy sync statements (identyczne, PBN puste+BPP, PBN+BPP
puste selektywnie/batch, różnice selektywnie)
* 4 error paths (GET/DELETE/POST retry + POST publikacji fail)
* 2 unit tests _diff_statements
- test_client_upload.py — URL zmieniony na endpoint repo, SentData
tworzony z konwertowanym JSON (bo upload zawsze konwertuje teraz)
- test_client_helpers.py — URL repo + pbn_wysylaj_bez_oswiadczen=True
na uczelnia (test bada afiliację, nie sync statements; monkeypatch
pbn_get_json_statements=[] żeby nie odpalać POST /v2)
- test_bpp_admin_helpers.py — URL repo we wszystkich 6 testach,
POST /v2/statements mock dla testu z dyscyplinami, asercje
dostosowane do nowych wiadomości (info o sync statements +
końcowy success zamiast pojedynczej wiadomości)
Helper ``_patch_intended_statements`` w test_client_sync.py —
zduplikowany z test_pbn_test_wysylka_interaktywna.py (świadoma
decyzja — duplikacja lepsza niż cross-file test imports).
Wszystkie 37 testów PBN przechodzą lokalnie.
Uwaga: 3 istniejące testy w pbn_export_queue/tests/test_tasks.py
(test_queue_pbn_export_batch_*) padały i przed moimi zmianami —
pre-existing conflict testdb fixture teardown (TRUNCATE CASCADE +
username unique violation); nie jest to regresja.
…edException
Commit 7/8 refaktoryzacji sync_publication.
Gdy sync_publication w górze rzuci ``StatementsResendFailedException``
(POST publikacji do /repositorium OK, ale retry dla GET/DELETE/POST
/v2/statements wyczerpany), kolejka traktuje to jako chwilową
niedostępność PBN — ponawia za kilka minut (RETRY_LATER).
Zmiany:
- ``_handle_retry_exception`` w PBN_Export_Queue.models łapie
``StatementsResendFailedException`` przed fallback-iem na TECHNICZNY.
Komunikat w polu komunikat: "Synchronizacja oświadczeń nie powiodła
się po wyczerpaniu prób (PBN UID=...): {last_error}. Ponowię wysyłkę
za kilka minut..."
- Import ``StatementsResendFailedException`` dodany.
- Test ``test_send_to_pbn_statements_resend_failed_exception`` — weryfikuje
RETRY_LATER + komunikat.
Commit 8/8 refaktoryzacji sync_publication. Finalny commit. - src/bpp/newsfragments/+pbn-sync-publication-split-flow.bugfix.rst — changelog towncrier opisujący całą refaktoryzację - docs/pbn-wysylka-plan.md — sekcja "Faza 2" zaktualizowana ze statusem "zaimplementowana", opis docelowego flow, listy zmian modelu, nowego wyjątku i historii commitów PR #164
…ikowania
Dwie naprawy w obszarze dat OpenAccess — Commit 9/8 (post-review).
CRITICAL BUG FIX — NameError w convert_js_with_statements_to_no_statements:
Stary kod:
try:
i = int(json["openAccess"]["releaseDateYear"])
except (ValueError, TypeError, AttributeError):
pass
json["openAccess"]["releaseDateYear"] = i # <-- NameError gdy except
Gdy int() failowała (np. PBN zwróci "unknown" albo None zamiast "2022"),
zmienna i była niezdefiniowana, a bezwarunkowy assignment rzucał NameError.
Po refaktoryzacji upload_publication ZAWSZE wywołuje
convert_js_with_statements_to_no_statements (wcześniej tylko warunkowo),
więc bug stał się krytyczny.
Fix: assignment tylko wewnątrz try, except swallowed (wartość
zachowana w oryginalnym formacie — PBN zwróci validation error).
DATE SOURCE FIX — priorytet openaccess_data_opublikowania:
_build_open_access_release_date używało public_dostep_dnia/dostep_dnia
jako źródła releaseDate. Gdy brak → hardcoded releaseDateMonth="JANUARY"
i releaseDateYear=str(rok). Problem: BPP ma dedykowane pole
ModelZOpenAccess.openaccess_data_opublikowania (DateField), ustawiane
przez importer PBN i przez redakcję ręcznie — adapter go ignorował.
Dodatkowo wysyłanie "JANUARY" gdy nie znamy faktycznego miesiąca było
wprowadzaniem PBN w błąd.
Nowy priorytet źródeł:
1. openaccess_data_opublikowania (dedykowane pole OA)
2. public_dostep_dnia (fallback, data wolnego dostępu)
3. dostep_dnia (fallback, data płatnego dostępu)
Gdy żadna data nie istnieje: NIE ustawiamy releaseDate/Month/Year
(zamiast wysyłania kłamliwego "styczeń"). PBN zwróci validation error
z jasnym komunikatem jeśli pole jest wymagane — redakcja uzupełni
brakującą datę w BPP zamiast ufać fałszywym wartościom.
Member
Author
Fazy 2 + 2b — podsumowanie implementacjiPo dwufazowym planie z Faza 2 — refaktoryzacja
|
…e mode)
Commit 10 refaktoryzacji sync_publication — post-review fix (Faza 2c).
Problem: w trybie selektywnym (domyślnym) POST oświadczeń do
/api/v2/institution-profile/statements wysyłał PEŁNY zestaw BPP —
także te oświadczenia, które już były w PBN. Kod wywoływał
pbn_get_api_statements() zwracające wszystkie statements z BPP,
bez filtra po only_in_intended.
User wskazał że API PBN może nie być idempotentne — wysyłanie
duplikatów może spowodować błędy walidacji lub zduplikowane rekordy
w PBN. W kroku 4b algorytmu (różnice) powinniśmy wysyłać TYLKO
oświadczenia których nie ma w PBN, nie dublować istniejących.
Zmiany:
- ``_post_statements_with_retry(rec, objectId, publication_pk, filter_keys=None)``
— dodany opcjonalny parametr ``filter_keys: set | None``. Gdy None,
POST-ujemy pełen zestaw (zachowanie jak dziś, używane w trybie batch
po delete_all). Gdy set kluczy ``(personObjectId, disciplineId)``,
POST-ujemy tylko statements dopasowane do tych kluczy.
- ``_build_post_statements_payload(rec, filter_keys=None)`` — nowy
helper do budowania payloadu. Dla filter_keys≠None:
* bierze ``publicationUuid`` z ``adapter.pbn_get_api_statements()``
(wymuszenie get_pbn_uuid — rzuca ``DaneLokalneWymagajaAktualizacjiException``
gdy brak V2)
* bierze surowe statements z ``pbn_get_json_statements()``
* filtruje po ``_statement_key_intended in filter_keys``
* konwertuje każdy przez ``_convert_stmt_for_api``
- ``_convert_stmt_for_api`` (nowa staticmethod) — ekstrakcja konwersji
``statement → format POST``: usunięcie disciplineId gdy jest disciplineUuid,
rename type→personRole, usunięcie personNaturalId. Skopiowane z
``WydawnictwoPBNAdapter.pbn_get_api_statements._convert_stmt`` (patrz
komentarz w metodzie — gdy format się zmieni, trzeba poprawić w obu
miejscach).
- ``_sync_statements_with_pbn`` — dla ``only_in_intended``:
* kasuj_selektywnie=True → POST z filter_keys=only_in_intended
(kroki 3 i 4b algorytmu — only_in_intended pokrywa oba scenariusze:
PBN puste = wszystkie klucze BPP, PBN+BPP różne = tylko brakujące)
* kasuj_selektywnie=False → POST bez filter_keys (batch: po delete_all
POST-ujemy wszystko)
Testy (test_client_sync.py):
- ``test_sync_statements_roznice_selektywnie`` — dodana asercja że
body POST zawiera TYLKO 1 statement (only_in_intended), nie pełen
zestaw.
- ``test_sync_statements_pbn_subset_bpp_superset_tylko_brakujace`` —
nowy test: PBN={(A,301)}, BPP={(A,301),(B,200)}. Weryfikuje że
DELETE nie wywołany, POST zawiera TYLKO (B,200) — nie dubluje
istniejącego (A,301).
- ``test_sync_statements_pbn_puste_wysyla_wszystkie_w_selektywnym`` —
nowy test: PBN={}, BPP={A,B}. Weryfikuje że POST wysyła 2 statements
(wszystkie BPP), mimo filter_keys (bo only_in_intended = wszystkie).
- ``test_sync_statements_batch_mode_post_wszystkie`` — nowy test dla
batch mode: po delete_all POST wysyła pełen zestaw BPP bez filtra.
Wszystkie 20 testów test_client_sync.py pass. Pełen suite PBN pass
(pre-existing 1 failed + 9 errors w pbn_export_queue/tests/test_tasks.py
niezwiązane z tą zmianą).
Stopka pokazuje teraz "wersja X (119-merge, feature-nowe-zglos-publikacje,
commit YYYYYYY)" — od razu widać i kanoniczny tag PR-a, i sanityzowaną
nazwę gałęzi. Wcześniej stopka znała tylko jeden alias (final_tag),
więc nie było jasne że można też pullować po nazwie brancha.
Pipeline: workflow przekazuje branch_tag do bake'a, bake do bpp_base
runtime, a tam ENV BPP_BRANCH_TAG zostaje wczytany przez nowy
template tag {% bpp_branch_tag %}. Master release ma BPP_BUILD_FLAVOR=
release → tag zwraca pusty string, stopka pokazuje samą wersję.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
(cherry picked from commit b2837d6)
- convert_js_with_statements_to_no_statements -> convert_json_*; funkcja
faktycznie usuwa klucz `statements` (endpoint /repositorium/publications
go nie przyjmuje). To naprawia HTTP 400 przy wysylce.
- SentData.api_url: nowe pole zapisuje pelny URL endpointa do ktorego
wyslano dane — do diagnostyki "gdzie poszlo to ostatnie 400".
- pbn_export_queue: detal zlecenia pokazuje api_url; _build_ai_prompt
fallbackuje na konfiguracje Uczelni gdy api_url puste.
- cleanup zdublowanego pop("statements") w pbn_test_wysylka_interaktywna.
- testy guarda dla convert_json + MockTransport.base_url zeby fixtury
dzialaly po dodaniu odwolania do transport.base_url w upload_publication.
- przy okazji: znormalizowane string-fieldy w SentData (exception,
api_response_status, typ_rekordu, api_url) — null=True -> default="",
data migration konwertuje istniejace NULL->""; save() przeniesione
przed custom methods (DJ012).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…uild Po merge'u dev nasz lokalny conflict-free auto-merge zachowal nasza wersje workflow (tylko master + .docker-build flag mechanism). User zdecydowal ze chce dev'owa wersje (auto-build na feature/fix/hotfix przez push trigger) — wiec nadpisuje plikiem z origin/dev i kasuje .docker-build. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Plik .docker-build juz nie istnieje (skasowany w poprzednim commicie), wiec elif sprawdzajacy `[ -f ".docker-build" ]` byl dormantnym kodem. Zastapione: push na non-master (czyli feature/fix/hotfix przez restrykcje triggera) → buduj zawsze. Realizuje user-intent "auto-build na feature branches" — bez tego push na feature spadalby na else (skip), a `.docker-build` flag nie istnieje. Komentarze i opisy aktualizowane — bez wzmianek o pliku flagi. Pozostale `docker-build` w workflow to label PR-a (mechanizm zostaje). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Production deploy wywala sie: psycopg2.errors.ObjectInUse: BLAD: nie mozna ALTER TABLE "pbn_api_sentdata" poniewaz posiada oczekujace zdarzenia wyzwalaczy Wynika z kolejnosci operacji w 0069: RunPython UPDATE NULL→"" generuje deferred trigger events (denorm/easyaudit), a kolejny AlterField (ALTER TABLE) w tej samej transakcji nie moze przejsc. atomic=False sprawia, ze kazda operacja commituje sie osobno: triggery odpalaja po UPDATE, ALTER startuje czysto. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wycofuje gating labelem .docker-build/docker-build na rzecz prostszej zasady: master/main push i workflow_dispatch buduja zawsze (release flow + manual override), pozostale (PR sync, feature/fix/hotfix push bez PR) — tylko gdy actor=mpasternak. Inni contributorzy nie pala Docker Cloud minutek; jesli trzeba zbudowac obraz dla cudzego PR-a: `gh workflow run build-docker-images.yml --ref <branch>`. Dev branch dopisany jawnie do komentarza w pushu jako "intentionally excluded" — push do dev nie odpala buildu (intermediate state nie zasluguje na obraz, release leci przez master). Dodany main do triggerow obok master (gdyby kiedys repo zmienilo default branch — single source of truth). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Trzecia regresja w 2 tygodnie (footer wskakiwal pod tabele na
/pbn_export_queue/?page=4#pager) — naprawione + 3 warstwy prewencji
zeby klasa nie wracala.
Hotfixy templatow:
- base.html (linie 240, 266): usunieto self-closing <p/> z globalnego
base templatu;
- pagination/{pagination,pagination_with_anchor}.html i
bpp/templates/pagination.html: pusty <li class=ellipsis></li>
zamieniono na <li class=ellipsis><span>…</span></li> —
minify-html traktowal pusty li jako redundant void i usuwal go;
- pagination_with_anchor.html: wyciagnieto inline <style> (96 linii)
do _pagination.scss — partial jest swap-owany przez htmx
innerHTML, kazdy swap re-injektowal CSS-a;
- import _pagination w app-{blue,orange,green}.scss.
Prewencja systemowa:
- BppMinifyHtmlMiddleware.should_minify() omija requesty z naglowkiem
HX-Request: true. minify-html jest zaprojektowane dla pelnych
dokumentow — na fragmentach htmx rozjezdza DOM swoimi heurystykami.
To eliminuje cala klase bugow.
- djlint jako linter HTML w pre-commit (--lint, bez auto-reformatu;
[tool.djlint] enforcuje H020 puste-tag-pair, H025 orphan-tag, H017
void-self-closing; reszta wyciszona).
- src/bpp/tests/test_html_minify_integrity.py: 5 testow z canary
HTML-em odwzorowujacym ostatnie incydenty (pusty <li>, <p/>,
<span/>) + sentinel "self-closing span PSUJE DOM" + bypass-test
dla htmx + sanity dla non-htmx.
Higiena:
- src/django_bpp/bpp/static/500.html: usuniety z gita (zawiera
tenant-specific dane z BD jak nazwa uczelni; generowany w entrypoint
appservera) + dodany do .gitignore.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bylo: ogolny https://pbn.nauka.gov.pl/api/ — strona-rozdroze, AI musial zgadywac gdzie sa schematy. Teraz: https://pbn.nauka.gov.pl/api/v3/api-docs — Swagger/OpenAPI JSON ze wszystkimi endpointami, schematami request/response i kodami bledow. AI moze pobrac, sparsowac i konkretnie wskazac ktore pole laczy z bledem schematu. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
500 z `Brak szczegółów błędu` po stronie BPP utrudnia diagnostykę — `smart_content` mogł gubić body, a response headers nigdy nie były nigdzie zapisywane. Przy `status_code >= 500` w `_check_error_response` dorzucam `logger.error` z surową treścią `ret.content[:4000]`, `ret.headers` oraz `body_len`, żeby było widać czy PBN cokolwiek zwraca w body i co trafia w nagłówkach (Server, Content-Type itp.). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cofa standardyzację z commitu 3092d1b (zawsze /v1/repositorium/publications) i przywraca decyzję o endpoincie na podstawie obecności lokalnych statements oraz flagi `Uczelnia.pbn_wysylaj_bez_oswiadczen`: - praca z lokalnymi statements → POST /v1/publications (all-in-one, surowy payload z adaptera, statements w body, response `{objectId}`); - praca bez lokalnych statements + flaga uczelni `=True` → POST /v1/repositorium/publications (po `convert_json_with_statements_ to_no_statements`, body owinięte w listę, response `[{id}]`); - praca bez lokalnych statements + flaga `=False` → adapter rzuca `StatementsMissing` (zachowanie niezmienione). `sync_publication` po wysyłce ZAWSZE wywołuje `_sync_statements_with_pbn` (GET /page/statements → diff → DELETE/POST przez /v2/institution-profile/ statements), niezależnie od endpointu wysyłki publikacji. Dla pracy bez statements lokalnie sync wykasuje ewentualne pozostałości oświadczeń po stronie PBN; dla pracy z statements typowo no-op (PBN ma już identyczne z body), ale wykryje dryft jeśli się pojawi. Zmiany w `publication_sync.py`: - przywrócono `post_publication()` (POST do `/v1/publications`); - `_prepare_publication_json` zwraca tuple `(js, bez_oswiadczen)`, konwertuje TYLKO gdy `bez_oswiadczen` (endpoint repo wymaga `firstName` zamiast `givenNames`, abstracts w root, brak `fee` itp.); - `_post_publication_data` rozgałęzia po `bez_oswiadczen` (różny format response dla obu endpointów); - `upload_publication` zwraca prawdziwe `bez_oswiadczen` (zamiast zawsze `True`) i zapisuje właściwy URL endpointu w `SentData.api_url`. `pbn_test_wysylka_interaktywna` w KROK 3/8 dorzuca podpowiedź „Produkcja wybrałaby: [X]" (na podstawie obecności statements) — user ma nadal ręczny wybór, ale widzi, którą drogą poszłaby produkcja dla tej pracy. Testy: nowe scenariusze w `test_client_upload.py` (oba endpointy + StatementsMissing), `_setup_common_mocks` w `test_client_sync.py` mockuje OBA endpointy POST publikacji (niezależnie od którą stroną poszedł upload), `test_sync_publication_zawsze_endpoint_repo` przebudowane na 2 testy (`bez_statements_idzie_do_repo` + `z_statements_idzie_do_v1_publications`), `test_bpp_admin_helpers.py` zaktualizowane (`test_z_oswiadczeniami` i scenariusze UID change/ conflict używają teraz mocka `/v1/publications`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Dotychczasowy `logger.error` (commit c525c79) odpalał się tylko dla 5xx, ale i tak nie pojawiał się na konsoli — w `LOGGING` w `django_bpp/settings/base.py` jedynym skonfigurowanym loggerem był `pbn_import` (handler `console`, propagate=False). Logi z `pbn_api.client.transport` ginęły bo nikt ich nie zbierał. Dwie zmiany: 1. `_check_error_response` w `transport.py`: - `logger.error` rozszerzony z `>= 500` na `>= 400` (przy 4xx też warto widzieć body i headers — np. „400 Bad Request" bez czytelnego body to typowa odpowiedź PBN przy validation errors); - DODATKOWO `rollbar.report_message` przy każdym >= 400, z `extra_data` (status_code, url, headers, body_len, body skrócone do 4000 znaków przez `smart_content`). Level: `error` dla 5xx, `warning` dla 4xx. 2. `LOGGING` w `settings/base.py`: dodany logger `pbn_api`, level `WARNING`, handler `console`, `propagate: False`. Teraz `logger.error` z transport.py faktycznie pojawi się na stderr (runserver, Celery worker, manage.py commands). `423 Locked` nadal jest specjalnie obsłużone (raise `ResourceLockedException` PRZED log/rollbar) — to nie błąd diagnostyczny, tylko sygnalizacja zajętego zasobu. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sytuacja docelowa: praca lokalnie ma już 0 dyscyplin (np. ostatnia została skasowana z `Wydawnictwo_*_Autor`), a po stronie PBN nadal wiszą stare oświadczenia. POST do `/v1/repositorium/publications` może odrzucić publikację gdy ma już oświadczenia w PBN — kasujemy je upfront, nie czekając na post-upload sync. Nowy helper `_pre_upload_clear_pbn_statements_if_any(rec)` w `PublicationSyncMixin`: - Brak `pbn_uid_id` → no-op (PBN nie ma jeszcze odpowiednika pracy). - GET `/page/statements` z PBN. Failure → log warning, kontynuuj. POST może rzucić czytelny błąd jeśli problem rzeczywiście blokuje wysyłkę — nie chcemy blokować całej wysyłki na błędzie GET. - PBN zwraca pustą listę → no-op. - PBN ma oświadczenia → DELETE selektywnie/batch wg `Uczelnia.pbn_kasuj_dyscypliny_selektywnie` (reuse istniejących helperów `_delete_statements_selective` / `_delete_statements_batch`). DELETE failure rzucamy w górę (`StatementsResendFailedException`) — nie wysyłamy publikacji wiedząc że PBN za chwilę odrzuci. Wywołanie w `upload_publication` warunkowe na `bez_oswiadczen=True` (czyli ścieżka /v1/repositorium/publications). Dla /v1/publications (all-in-one z statements w body) pre-clear jest pomijany — ewentualny drift wykrywa post-upload `_sync_statements_with_pbn`. 4 testy w `test_client_upload.py`: - DELETE selektywny przed POST gdy PBN ma statements + BPP puste; - pomija (no DELETE) gdy PBN puste; - pomija (no GET, no DELETE) gdy brak pbn_uid_id; - pomija (no GET) dla ścieżki /v1/publications. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sylka-interaktywna
Funkcja _download_statements_with_retry() teraz ponawia pobieranie V2 API 5 razy z rosnącym czasem oczekiwania (2s, 4s, 8s, 16s, 32s). Poprzednio jedyna próba kończyła się niejasnym warningiem "nie jest błędem", co myliło użytkowników - brak V2 oznacza brak UUID i brak możliwości generowania linków do PBN Interfejs oraz wysyłki oświadczeń. Po wyczerpaniu prób wyświetlany jest BŁĄD z sugestią użycia PBN Export Queue (wysyłka w tle) zamiast interaktywnej. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Kontekst
Pierwsza z dwóch faz poprawki wysyłki publikacji do PBN. Obecnie, gdy
włączona jest opcja
uczelnia.pbn_api_kasuj_przed_wysylka=True,sync_publication(src/pbn_api/client/publication_sync.py:467-539)wykonuje operacje w kolejności:
/api/v1/institutionProfile/publications/{id})/api/v1/publications)Jeśli krok 2 zawiedzie (HTTP 423 Locked, błąd walidacji, status PBN
"LOGED" itp.), DELETE z kroku 1 już się wykonał i oświadczenia znikają
z profilu instytucji. Docelowo chcemy rozdzielić te operacje: najpierw
bezpieczna wysyłka publikacji przez endpoint repozytoryjny
/api/v1/repositorium/publications, potem — jeśli zestaw oświadczeńsię różni — DELETE + POST przez
/api/v2/institution-profile/statements.Co robi ten PR (Faza 1)
Dodaje interaktywne narzędzie CLI
pbn_test_wysylka_interaktywnado empirycznego zbadania, jak PBNzachowuje się przy każdej kombinacji endpointów — zanim zmienimy
produkcyjną logikę. Narzędzie nie modyfikuje lokalnej bazy BPP; działa
na PBN API przez istniejący
PBNClient.Szczegółowy plan obu faz:
docs/pbn-wysylka-plan.md.Zawartość PR
src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py— 520 linii, REPL z 8 krokami (wybór publikacji → generowanie JSON →
wybór endpointa → POST publikacji → GET oświadczeń → porównanie →
DELETE → POST
/v2/statements)src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py— 13testów (walidacja argumentów, dry-run, happy path dla obu endpointów,
błędy HTTP, decyzje użytkownika)
docs/pbn-wysylka-plan.md— plan dwufazowysrc/bpp/newsfragments/+pbn-test-wysylka-interaktywna.feature.rst— changelog towncrier
.docker-build— pusty flag file, włącza build obrazu Docker w CIżeby user mógł uruchomić narzędzie w preprod
src/importer_publikacji/migrations/0006_merge_*—pre-existing konflikt dwóch leaf 0005-ek w gałęzi
dev(powstałpo zmerge-owaniu PR Fix #316: require parent publication for chapters in importer #115 i innych). Merge-migracja łączy je bez
modyfikacji istniejących migracji.
Jak testować lokalnie
```bash
Unit testy narzędzia:
UV_NO_SYNC=1 uv run --all-extras pytest \
src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py -n auto
Użycie narzędzia (po zbudowaniu obrazu Docker w CI):
uv run python src/manage.py pbn_test_wysylka_interaktywna \
--wydawnictwo-zwarte --user-token
Tryb dry-run (nic nie wysyła do PBN):
uv run python src/manage.py pbn_test_wysylka_interaktywna \
--wydawnictwo-zwarte --dry-run
```
Test plan
/api/v1/publications— weryfikacja że PBNakceptuje JSON z
statements/api/v1/repositorium/publications— weryfikacjaże PBN akceptuje JSON BEZ
statements/page/statements?publicationId=...zwraca oczekiwaneoświadczenia po wysyłce
/api/v2/institution-profile/statementszamienia zestawszczególnie przy statusie "LOGED") w osobnym PR (Faza 2)
Co dalej (Faza 2, osobny PR)
Na podstawie wyników ręcznych testów z Fazy 1: refaktoryzacja
sync_publication()do docelowego flow (POST repo → GET → porównanie→ DELETE + POST oświadczeń), z aktualizacją testów i changelog
bugfix-owym.