From 5e539efbd133db61c240312e97584cb263782a9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Mon, 20 Apr 2026 22:19:04 +0200 Subject: [PATCH 01/33] fix(migrations): merge leaf nodes 0005 w importer_publikacji MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../migrations/0006_merge_20260420_2212.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/importer_publikacji/migrations/0006_merge_20260420_2212.py diff --git a/src/importer_publikacji/migrations/0006_merge_20260420_2212.py b/src/importer_publikacji/migrations/0006_merge_20260420_2212.py new file mode 100644 index 000000000..2515ea0f8 --- /dev/null +++ b/src/importer_publikacji/migrations/0006_merge_20260420_2212.py @@ -0,0 +1,13 @@ +# Generated by Django 5.2.13 on 2026-04-20 20:12 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("importer_publikacji", "0005_alter_importsession_created_by"), + ("importer_publikacji", "0005_importsession_wydawnictwo_nadrzedne"), + ] + + operations = [] From 640197fd815b558dd168ef25897d7036f8d041b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Mon, 20 Apr 2026 22:19:34 +0200 Subject: [PATCH 02/33] =?UTF-8?q?feat(pbn=5Fapi):=20narz=C4=99dzie=20pbn?= =?UTF-8?q?=5Ftest=5Fwysylka=5Finteraktywna=20do=20audytu=20PBN?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .docker-build | 0 docs/pbn-wysylka-plan.md | 159 ++++++ ...+pbn-test-wysylka-interaktywna.feature.rst | 13 + .../commands/pbn_test_wysylka_interaktywna.py | 532 ++++++++++++++++++ .../test_pbn_test_wysylka_interaktywna.py | 492 ++++++++++++++++ 5 files changed, 1196 insertions(+) create mode 100644 .docker-build create mode 100644 docs/pbn-wysylka-plan.md create mode 100644 src/bpp/newsfragments/+pbn-test-wysylka-interaktywna.feature.rst create mode 100644 src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py create mode 100644 src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py diff --git a/.docker-build b/.docker-build new file mode 100644 index 000000000..e69de29bb diff --git a/docs/pbn-wysylka-plan.md b/docs/pbn-wysylka-plan.md new file mode 100644 index 000000000..a55745b47 --- /dev/null +++ b/docs/pbn-wysylka-plan.md @@ -0,0 +1,159 @@ +# Plan: bezpieczna wysyłka publikacji i oświadczeń do PBN + +Dokument opisuje plan zmian w mechanizmie wysyłki publikacji z BPP do PBN. +Celem długofalowym jest **rozdzielenie** wysyłki samego dzieła od wysyłki +oświadczeń instytucji, tak aby nieudana wysyłka publikacji nie kasowała +wcześniej istniejących oświadczeń w PBN. + +Plan jest **dwufazowy**. Ta gałąź (`feature/pbn-test-wysylka-interaktywna`) +realizuje wyłącznie **Fazę 1** — interaktywne narzędzie CLI służące do +empirycznego zbadania, jak PBN reaguje na poszczególne kroki, zanim +zdecydujemy o kształcie docelowej refaktoryzacji w Fazie 2. + +## Kontekst problemu + +Gdy w panelu uczelni włączona jest opcja +`uczelnia.pbn_api_kasuj_przed_wysylka=True`, obecny flow w +`PBNClient.sync_publication()` +(`src/pbn_api/client/publication_sync.py:467-539`) wygląda tak: + +1. **DELETE** oświadczeń publikacji w PBN + (`DELETE /api/v1/institutionProfile/publications/{id}` z `all: True`). +2. **POST** publikacji razem z oświadczeniami + (`POST /api/v1/publications`, JSON zawiera klucz `statements`). +3. **DOWNLOAD** publikacji (`GET /api/v1/publications/id/{id}`). +4. **DOWNLOAD** oświadczeń z PBN i synchronizacja lokalnej tabeli + `OswiadczenieInstytucji`. + +Problem: krok 2 bywa zawodny (HTTP 423 Locked, błąd walidacji, status +PBN „LOGED" itp.). Wtedy DELETE z kroku 1 już się wykonał, a POST z kroku +2 nie wszedł — w PBN zostaje publikacja **bez oświadczeń**, a lokalne dane +też już nie wrócą na profil instytucji bez ręcznego ponownego wysyłu +oświadczeń. User zgłasza utratę oświadczeń w tym scenariuszu. + +## Docelowy flow (Faza 2, do zaprojektowania po Fazie 1) + +Wstępny zamysł — do weryfikacji przez Fazę 1: + +1. POST publikacji przez endpoint **repozytoryjny** + `POST /api/v1/repositorium/publications` (JSON bez klucza `statements`, + przepuszczony przez `convert_js_with_statements_to_no_statements()`). + - FAIL ⇒ zwróć błąd, nie ruszamy oświadczeń. Stan w PBN nietknięty. + - OK ⇒ mamy `objectId`. +2. GET oświadczeń publikacji w PBN + (`GET /api/v1/institutionProfile/publications/page/statements?publicationId={objectId}`). +3. Porównanie tego, co jest w PBN, z tym, co wygenerował + `WydawnictwoPBNAdapter.pbn_get_api_statements()`. + - identyczne ⇒ koniec, nic nie robimy z oświadczeniami. + - różne ⇒ DELETE oświadczeń + (`DELETE /api/v1/institutionProfile/publications/{objectId}` z `all: True`) + + POST nowych przez + `POST /api/v2/institution-profile/statements`. +4. DOWNLOAD oświadczeń lokalnie (synchronizacja BPP z PBN, + reużywa `download_statements_of_publication()`). + +Gdy flaga `delete_statements_before_upload=False` — **zachowanie bez zmian**, +stary flow z `/api/v1/publications` pozostaje. + +**Niewiadome, które musimy zbadać zanim wdrożymy Fazę 2:** +- Jak PBN zachowuje się po wysyłce do endpointu repozytoryjnego + w przypadkach, gdy publikacja już istnieje (różne statusy: ACTIVE, + LOGED itp.). +- Czy `POST /api/v2/institution-profile/statements` wymaga uprzedniego + DELETE, czy sam potrafi nadpisać istniejący zestaw oświadczeń. +- Czy kolejność GET → porównanie → DELETE+POST jest wystarczająca, czy + trzeba obsłużyć dodatkowe stany pośrednie. + +## Faza 1 — narzędzie CLI (ta gałąź) + +### Co powstaje + +- `src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py` — + interaktywny REPL, który dla wybranej publikacji prowadzi użytkownika + krok po kroku przez pełen flow wysyłki. Po każdym kroku czeka na + `Enter` (lub `q` żeby przerwać). Dla każdego żądania HTTP pokazuje + metodę, URL, body; dla odpowiedzi — status, body (skrócone lub pełne). +- `src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py` — testy + jednostkowe z mockiem `input()` i `MockTransport`. +- `src/bpp/newsfragments/+pbn-test-wysylka-interaktywna.feature.rst` — + changelog towncrier. +- `.docker-build` — pusty plik w root repo, włącza build obrazu + Docker w CI (patrz `.github/workflows/build-docker-images.yml`). + +### Zakres narzędzia + +Narzędzie realizuje **tylko operacje zgodne ze specyfikacją PBN API** — +nie podejmuje prób wysyłania JSON-a z kluczem `statements` do endpointu +repozytoryjnego ani innych eksperymentów niezgodnych z API. Zakres +dostępnych kroków: + +1. **Pokaż publikację** — wybrany rekord (pk, tytuł, obecny PBN UID, + liczba oświadczeń lokalnych). +2. **Wygeneruj JSON publikacji** — wywołaj `WydawnictwoPBNAdapter`, + pokaż czy JSON zawiera klucz `statements`. +3. **Wybierz endpoint publikacji** — `/api/v1/publications` (all-in-one, + z oświadczeniami jeśli są w JSON) albo `/api/v1/repositorium/publications` + (wymusza JSON bez oświadczeń przez `convert_js_with_statements_to_no_statements`). +4. **Wyślij POST publikacji** — pokaż URL, body, po wysyłce status i JSON + odpowiedzi; wyciągnij `objectId`. +5. **Pobierz aktualne oświadczenia z PBN** — + `GET /api/v1/institutionProfile/publications/page/statements?publicationId={objectId}`. +6. **Porównaj z lokalnymi** — które identyczne, które w PBN nie ma, które + lokalnie nie ma. Decyzja użytkownika: czy kasować i nadpisywać. +7. **DELETE oświadczeń w PBN** (opcjonalnie) — + `DELETE /api/v1/institutionProfile/publications/{objectId}` z `all: True`. +8. **POST nowych oświadczeń** (opcjonalnie) — + `POST /api/v2/institution-profile/statements` z payloadem z + `WydawnictwoPBNAdapter.pbn_get_api_statements()`. +9. **Podsumowanie** — co poszło, jakie statusy, ile zajęło. + +Tryb `--dry-run` pokazuje wszystkie żądania, ale nic nie wysyła. + +### Wymagania niefunkcjonalne + +- **Nie modyfikuje lokalnej bazy BPP.** Nie tworzy, nie kasuje, nie + aktualizuje żadnych rekordów BPP (w tym `OswiadczenieInstytucji`, + `SentData`, `Publication`). Służy wyłącznie do audytu zachowania PBN. +- Reużywa istniejący `PBNClient` i `WydawnictwoPBNAdapter` — nie duplikuje + logiki budowania JSON ani wysyłki HTTP. +- Obsługa błędów: łapie `HttpException`, `PraceSerwisoweException`, + `NeedsPBNAuthorisationException`; pokazuje czytelny komunikat i + pozwala wrócić do menu wyboru (albo wyjść). +- Identyfikacja użytkownika PBN — wzorzec z + `pbn_wysylka_oswiadczen/tasks.py::get_pbn_client()`. + +### CLI argumenty + +``` +uv run python src/manage.py pbn_test_wysylka_interaktywna \ + --wydawnictwo-zwarte # albo --wydawnictwo-ciagle + --user # użytkownik z tokenem PBN + [--dry-run] # nic nie wysyłaj +``` + +## Faza 2 — refaktoryzacja `sync_publication` (osobna gałąź) + +Po ręcznych testach narzędziem z Fazy 1 i udokumentowaniu wyników +(osobny dokument `docs/pbn-wysylka-eksperymenty.md` w kolejnym PR) +implementujemy docelowy flow w `src/pbn_api/client/publication_sync.py`. +Plan dla Fazy 2 — szczegóły po zebraniu obserwacji z eksperymentów. + +## Testowanie (Faza 1) + +```bash +# Testy narzędzia (unit): +UV_NO_SYNC=1 uv run --all-extras pytest \ + src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py -n auto + +# Smoke test na preprod PBN (ręczny): +uv run python src/manage.py pbn_test_wysylka_interaktywna \ + --wydawnictwo-zwarte --user --dry-run +# a potem bez --dry-run, na rzeczywistej publikacji w preprod +``` + +## Wymagania CI + +- `.docker-build` w root repo ⇒ CI buduje obraz Docker dla tej gałęzi, + tak by user mógł uruchomić narzędzie w środowisku testowym. +- Pełny pipeline (`tests.yml`): lint + testy (non-playwright, serial, + playwright) muszą przejść. diff --git a/src/bpp/newsfragments/+pbn-test-wysylka-interaktywna.feature.rst b/src/bpp/newsfragments/+pbn-test-wysylka-interaktywna.feature.rst new file mode 100644 index 000000000..1b1612f57 --- /dev/null +++ b/src/bpp/newsfragments/+pbn-test-wysylka-interaktywna.feature.rst @@ -0,0 +1,13 @@ +Dodano interaktywne narzędzie CLI +``pbn_test_wysylka_interaktywna`` (Django management command) do +eksperymentalnego testowania flow wysyłki publikacji i oświadczeń do PBN +krok po kroku. Narzędzie prowadzi użytkownika przez kolejne fazy — +generowanie JSON publikacji, wybór endpointa (``/api/v1/publications`` +all-in-one albo ``/api/v1/repositorium/publications`` bez oświadczeń), +POST publikacji, GET i porównanie oświadczeń lokalnych z tym co jest w +PBN, DELETE oświadczeń i POST przez ``/api/v2/institution-profile/statements`` +— pokazując dla każdego kroku metodę HTTP, URL, body i odpowiedź +serwera. Narzędzie nie modyfikuje lokalnej bazy BPP i posiada tryb +``--dry-run``. Służy jako baza do audytu zachowania PBN API i +projektowania bezpieczniejszej kolejności operacji wysyłki (scenariusz: +nieudana wysyłka publikacji kasowała wcześniej istniejące oświadczenia). diff --git a/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py b/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py new file mode 100644 index 000000000..69303a770 --- /dev/null +++ b/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py @@ -0,0 +1,532 @@ +"""Interaktywne narzędzie do testowania wysyłki publikacji i oświadczeń do PBN. + +Narzędzie prowadzi użytkownika krok po kroku przez pełen flow wysyłki +wybranej publikacji do PBN. Dla każdego żądania HTTP pokazuje metodę, +URL, body (wysyłane dane), a dla odpowiedzi — status i treść. Między +krokami czeka na Enter. + +Narzędzie NIE modyfikuje lokalnej bazy BPP — służy wyłącznie do audytu +zachowania API PBN przed docelową refaktoryzacją ``sync_publication``. + +Tryb ``--dry-run`` pokazuje wszystko, ale nie wysyła niczego do PBN. + +Użycie: + uv run python src/manage.py pbn_test_wysylka_interaktywna \\ + --wydawnictwo-zwarte 123 --user-token [--dry-run] + + uv run python src/manage.py pbn_test_wysylka_interaktywna \\ + --wydawnictwo-ciagle 456 [--dry-run] +""" + +import json +import time +from typing import Any + +from django.core.management.base import CommandError + +from bpp.models import Wydawnictwo_Ciagle, Wydawnictwo_Zwarte +from pbn_api.adapters.wydawnictwo import WydawnictwoPBNAdapter +from pbn_api.const import ( + PBN_DELETE_PUBLICATION_STATEMENT, + PBN_GET_INSTITUTION_STATEMENTS, + PBN_POST_INSTITUTION_STATEMENTS_URL, + PBN_POST_PUBLICATION_NO_STATEMENTS_URL, + PBN_POST_PUBLICATIONS_URL, +) +from pbn_api.exceptions import ( + AccessDeniedException, + DaneLokalneWymagajaAktualizacjiException, + HttpException, + NeedsPBNAuthorisationException, + PraceSerwisoweException, + ResourceLockedException, +) +from pbn_api.management.commands.util import PBNBaseCommand +from pbn_api.models import OswiadczenieInstytucji + + +class UserAbort(Exception): + """Użytkownik przerwał flow (np. wpisał `q` na pytanie o Enter).""" + + +def _json_truncated(obj: Any, max_len: int = 800) -> str: + """Zwraca JSON sformatowany, ewentualnie skrócony do ``max_len`` znaków.""" + text = json.dumps(obj, indent=2, ensure_ascii=False, default=str) + if len(text) <= max_len: + return text + return text[:max_len] + f"\n... (obcięto, pełny JSON ma {len(text)} znaków)" + + +class Command(PBNBaseCommand): + help = ( + "Interaktywny REPL testujący wysyłkę publikacji i oświadczeń do PBN " + "krok po kroku. Nie modyfikuje lokalnej bazy BPP." + ) + + def add_arguments(self, parser): + super().add_arguments(parser) + parser.add_argument( + "--wydawnictwo-zwarte", + type=int, + help="PK rekordu Wydawnictwo_Zwarte do przetestowania", + ) + parser.add_argument( + "--wydawnictwo-ciagle", + type=int, + help="PK rekordu Wydawnictwo_Ciagle do przetestowania", + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="Pokazuj co byłoby wysyłane, ale nie wysyłaj niczego do PBN", + ) + parser.add_argument( + "--yes-all", + action="store_true", + help=( + "Automatycznie akceptuj wszystkie pytania Enter " + "(bez interakcji — do testów automatycznych)." + ), + ) + + def handle(self, app_id, app_token, base_url, user_token, *args, **options): + self.dry_run = options["dry_run"] + self.yes_all = options["yes_all"] + self.stats: list[tuple[str, str]] = [] + + if self.dry_run: + self._warn("TRYB DRY-RUN — żadne żądania nie będą wysłane do PBN.") + + publication = self._get_publication(options) + + try: + pbn_client = self.get_client(app_id, app_token, base_url, user_token) + except Exception as e: + raise CommandError(f"Nie mogę utworzyć klienta PBN: {e}") from e + + try: + self._run_flow(pbn_client, publication) + except UserAbort: + self._warn("Przerwano przez użytkownika.") + finally: + self._print_summary() + + # ------------------------- wybór publikacji ------------------------- + + def _get_publication(self, options): + pk_zwarte = options.get("wydawnictwo_zwarte") + pk_ciagle = options.get("wydawnictwo_ciagle") + + if bool(pk_zwarte) == bool(pk_ciagle): + raise CommandError( + "Podaj dokładnie jedno z: --wydawnictwo-zwarte " + "lub --wydawnictwo-ciagle ." + ) + + if pk_zwarte: + try: + return Wydawnictwo_Zwarte.objects.get(pk=pk_zwarte) + except Wydawnictwo_Zwarte.DoesNotExist as e: + raise CommandError( + f"Nie znaleziono Wydawnictwo_Zwarte o pk={pk_zwarte}" + ) from e + + try: + return Wydawnictwo_Ciagle.objects.get(pk=pk_ciagle) + except Wydawnictwo_Ciagle.DoesNotExist as e: + raise CommandError( + f"Nie znaleziono Wydawnictwo_Ciagle o pk={pk_ciagle}" + ) from e + + # ------------------------- główny flow ------------------------- + + def _run_flow(self, pbn_client, publication): + self._step_show_publication(publication) + js, bez_oswiadczen = self._step_generate_json(publication) + endpoint_choice = self._step_choose_endpoint(bez_oswiadczen) + object_id = self._step_post_publication( + pbn_client, publication, js, endpoint_choice + ) + if not object_id: + self._warn("Brak objectId z PBN — kończę flow. Sprawdź odpowiedź serwera.") + return + + pbn_statements = self._step_get_pbn_statements(pbn_client, object_id) + identyczne = self._step_compare_statements(publication, pbn_statements) + + if identyczne: + self._info("Oświadczenia w PBN identyczne z lokalnymi — nie ruszam ich.") + return + + if not self._prompt_yes_no( + "Czy skasować oświadczenia w PBN i wysłać nowe?", default=True + ): + self._info("Pominięto DELETE + POST oświadczeń (decyzja użytkownika).") + return + + self._step_delete_statements(pbn_client, object_id) + self._step_post_statements(pbn_client, publication) + + # ------------------------- kroki ------------------------- + + def _step_show_publication(self, publication): + self._header("KROK 1/8 — Wybrana publikacja") + lokalne = ( + OswiadczenieInstytucji.objects.filter( + publicationId_id=publication.pbn_uid_id + ).count() + if publication.pbn_uid_id + else 0 + ) + self._info(f"Typ: {type(publication).__name__}") + self._info(f"PK: {publication.pk}") + self._info(f"Tytuł: {publication.tytul_oryginalny[:100]}") + self._info(f"Rok: {publication.rok}") + self._info(f"PBN UID: {publication.pbn_uid_id or '(brak)'}") + self._info(f"Oświadczenia lokalnie (OswiadczenieInstytucji): {lokalne}") + self._prompt_enter() + + def _step_generate_json(self, publication): + self._header("KROK 2/8 — Generowanie JSON publikacji") + adapter = WydawnictwoPBNAdapter(publication) + js = adapter.pbn_get_json() + bez_oswiadczen = "statements" not in js + n_statements = len(js.get("statements", [])) if not bez_oswiadczen else 0 + self._info(f"Adapter: WydawnictwoPBNAdapter({publication!r})") + self._info( + f"JSON: {'BEZ oświadczeń' if bez_oswiadczen else 'Z oświadczeniami'}" + f" (klucz 'statements' {'NIE' if bez_oswiadczen else 'JEST'} w JSON)" + ) + if not bez_oswiadczen: + self._info(f"Oświadczeń w JSON: {n_statements}") + self._info("Preview JSON:") + self.stdout.write(_json_truncated(js, max_len=600)) + if self._prompt_yes_no("Pokazać pełny JSON?", default=False): + self.stdout.write(json.dumps(js, indent=2, ensure_ascii=False, default=str)) + self._prompt_enter() + return js, bez_oswiadczen + + def _step_choose_endpoint(self, bez_oswiadczen): + self._header("KROK 3/8 — Wybór endpointa wysyłki publikacji") + self._info( + f"Opcja [1]: POST {PBN_POST_PUBLICATIONS_URL} (all-in-one, JSON bez zmian)" + ) + self._info( + f"Opcja [2]: POST {PBN_POST_PUBLICATION_NO_STATEMENTS_URL} " + f"(wymusza JSON bez oświadczeń — convert_js_with_statements_to_no_statements)" + ) + if not bez_oswiadczen: + self._warn( + "UWAGA: JSON zawiera klucz 'statements'. Opcja [2] wyrzuci go " + "z JSON przez convert_js_with_statements_to_no_statements. " + "PBN w obu przypadkach zaakceptuje dokument zgodny ze spec." + ) + while True: + choice = self._prompt("Wybór [1/2] (q=wyjście): ") + if choice == "1": + return "publications" + if choice == "2": + return "repositorium" + if choice.lower() in ("q", "quit", "exit"): + raise UserAbort() + self._err("Nieprawidłowa opcja. Wpisz 1, 2 lub q.") + + def _step_post_publication(self, pbn_client, publication, js, endpoint_choice): + self._header("KROK 4/8 — POST publikacji do PBN") + + if endpoint_choice == "publications": + url = PBN_POST_PUBLICATIONS_URL + body = js + label = "post_publication (all-in-one)" + else: + url = PBN_POST_PUBLICATION_NO_STATEMENTS_URL + # Endpoint repozytoryjny nie przyjmuje oświadczeń w ciele, a spec + # PBN zaakceptuje JSON bez klucza `statements`. Usuwamy go jawnie + # — `convert_js_with_statements_to_no_statements` przekształca + # inne pola (givenNames→firstName itd.), ale samego klucza + # `statements` nie dotyka. + body_js = dict(js) + body_js.pop("statements", None) + body_js = pbn_client.convert_js_with_statements_to_no_statements(body_js) + body = [body_js] + label = "post_publication_no_statements (repozytorium)" + + self._print_http_request("POST", url, body, label=label) + + if self.dry_run: + self._info("[dry-run] Pomijam wysyłkę. Zwracam sztuczny objectId='DRY'.") + self.stats.append(("POST publikacji", "dry-run (pominięty)")) + self._prompt_enter() + return "DRY" + + if not self._prompt_yes_no("Wyślij teraz?", default=True): + self._info("Pominięto POST publikacji (decyzja użytkownika).") + self.stats.append(("POST publikacji", "pominięty")) + return None + + try: + response = pbn_client.transport.post(url, body=body) + except HttpException as e: + self._print_http_error(e) + self.stats.append(("POST publikacji", f"BŁĄD HTTP {e.status_code}")) + if self._prompt_yes_no( + "Wysyłka nie powiodła się. Kontynuować flow?", default=False + ): + return None + raise UserAbort() from e + except ( + AccessDeniedException, + NeedsPBNAuthorisationException, + ResourceLockedException, + PraceSerwisoweException, + ) as e: + self._err(f"{type(e).__name__}: {e}") + self.stats.append(("POST publikacji", f"BŁĄD {type(e).__name__}")) + raise UserAbort() from e + + self._print_http_response(response) + + object_id = self._extract_object_id(response, endpoint_choice) + self._info(f"Wyciągnięty objectId = {object_id!r}") + self.stats.append(("POST publikacji", f"OK, objectId={object_id}")) + self._prompt_enter() + return object_id + + def _step_get_pbn_statements(self, pbn_client, object_id): + self._header("KROK 5/8 — Pobranie aktualnych oświadczeń z PBN") + object_id_str = str(object_id) + url = PBN_GET_INSTITUTION_STATEMENTS + f"?publicationId={object_id_str}" + self._print_http_request( + "GET", url, body=None, label="get_institution_statements" + ) + + if self.dry_run or object_id == "DRY": + self._info("[dry-run] Pomijam GET. Zwracam pustą listę.") + self.stats.append(("GET oświadczeń PBN", "dry-run")) + self._prompt_enter() + return [] + + try: + result = list( + pbn_client.get_institution_statements_of_single_publication( + object_id_str, page_size=5120 + ) + ) + except HttpException as e: + self._print_http_error(e) + self.stats.append(("GET oświadczeń PBN", f"BŁĄD HTTP {e.status_code}")) + raise UserAbort() from e + + self._info(f"PBN zwrócił oświadczeń: {len(result)}") + self.stdout.write(_json_truncated(result, max_len=600)) + self.stats.append(("GET oświadczeń PBN", f"OK, {len(result)} oświadczeń")) + self._prompt_enter() + return result + + def _step_compare_statements(self, publication, pbn_statements): + self._header("KROK 6/8 — Porównanie oświadczeń lokalnych z PBN") + lokalne = ( + list( + OswiadczenieInstytucji.objects.filter( + publicationId_id=publication.pbn_uid_id + ).values("personId_id", "area", "institutionId_id") + ) + if publication.pbn_uid_id + else [] + ) + + def _key(stmt): + if isinstance(stmt, dict): + return ( + stmt.get("personId") or stmt.get("personId_id"), + str(stmt.get("area") or ""), + stmt.get("institutionId") or stmt.get("institutionId_id"), + ) + return (None, None, None) + + local_keys = {_key(x) for x in lokalne} + pbn_keys = {_key(x) for x in pbn_statements} + + only_local = local_keys - pbn_keys + only_pbn = pbn_keys - local_keys + common = local_keys & pbn_keys + + self._info(f"Identycznych (lokalne ∩ PBN): {len(common)}") + self._info(f"Tylko lokalnie (lokalne \\ PBN): {len(only_local)}") + self._info(f"Tylko w PBN (PBN \\ lokalne): {len(only_pbn)}") + + if only_local: + self._info(f" → lokalne bez odpowiednika w PBN: {list(only_local)[:5]}") + if only_pbn: + self._info(f" → w PBN bez odpowiednika lokalnie: {list(only_pbn)[:5]}") + + identyczne = not only_local and not only_pbn + self.stats.append( + ( + "Porównanie", + "identyczne" if identyczne else "różnice", + ) + ) + self._prompt_enter() + return identyczne + + def _step_delete_statements(self, pbn_client, object_id): + self._header("KROK 7/8 — DELETE oświadczeń w PBN") + url = PBN_DELETE_PUBLICATION_STATEMENT.format(publicationId=object_id) + body = {"all": True, "statementsOfPersons": []} + self._print_http_request( + "DELETE", url, body, label="delete_all_publication_statements" + ) + + if self.dry_run or object_id == "DRY": + self._info("[dry-run] Pomijam DELETE.") + self.stats.append(("DELETE oświadczeń", "dry-run")) + self._prompt_enter() + return + + if not self._prompt_yes_no("Wyślij DELETE?", default=True): + self._info("Pominięto DELETE (decyzja użytkownika).") + self.stats.append(("DELETE oświadczeń", "pominięty")) + return + + try: + response = pbn_client.delete_all_publication_statements(object_id) + except HttpException as e: + self._print_http_error(e) + self.stats.append(("DELETE oświadczeń", f"BŁĄD HTTP {e.status_code}")) + raise UserAbort() from e + except ResourceLockedException as e: + self._err(f"ResourceLocked: {e}") + self.stats.append(("DELETE oświadczeń", "Locked")) + raise UserAbort() from e + + self._print_http_response(response) + self.stats.append(("DELETE oświadczeń", "OK")) + self._prompt_enter() + + def _step_post_statements(self, pbn_client, publication): + self._header("KROK 8/8 — POST nowych oświadczeń") + try: + payload = WydawnictwoPBNAdapter(publication).pbn_get_api_statements() + except DaneLokalneWymagajaAktualizacjiException as e: + self._err( + f"Nie mogę przygotować payloadu oświadczeń: {e}. " + "Prawdopodobnie brak lokalnie PublikacjaInstytucji_V2 dla tego PBN UID." + ) + self.stats.append(("POST oświadczeń", f"błąd adaptera: {e}")) + return + + body = {"data": [payload]} + self._print_http_request( + "POST", + PBN_POST_INSTITUTION_STATEMENTS_URL, + body, + label="post_discipline_statements", + ) + + if self.dry_run: + self._info("[dry-run] Pomijam POST oświadczeń.") + self.stats.append(("POST oświadczeń", "dry-run")) + self._prompt_enter() + return + + if not self._prompt_yes_no("Wyślij POST oświadczeń?", default=True): + self._info("Pominięto POST oświadczeń (decyzja użytkownika).") + self.stats.append(("POST oświadczeń", "pominięty")) + return + + max_tries = 3 + attempt = 0 + while True: + attempt += 1 + try: + response = pbn_client.post_discipline_statements(body) + break + except HttpException as e: + self._print_http_error(e) + if e.status_code in (500, 423) and attempt < max_tries: + wait = 2**attempt + self._warn(f"Retry za {wait}s (próba {attempt}/{max_tries})...") + time.sleep(wait) + continue + self.stats.append( + ("POST oświadczeń", f"BŁĄD HTTP {e.status_code} (prób: {attempt})") + ) + raise UserAbort() from e + + self._print_http_response(response) + self.stats.append(("POST oświadczeń", f"OK (prób: {attempt})")) + self._prompt_enter() + + # ------------------------- helpers ------------------------- + + def _extract_object_id(self, response, endpoint_choice): + if endpoint_choice == "publications": + if isinstance(response, dict): + return response.get("objectId") + return None + if isinstance(response, list) and len(response) == 1: + item = response[0] + if isinstance(item, dict): + return item.get("id") or item.get("objectId") + return None + + def _print_http_request(self, method, url, body, label=""): + self._info(f"Wywołanie: {label}" if label else "Żądanie HTTP:") + self.stdout.write(self.style.HTTP_INFO(f" {method} {url}")) + if body is not None: + self.stdout.write(" body:") + self.stdout.write(_json_truncated(body, max_len=800)) + + def _print_http_response(self, response): + self.stdout.write(self.style.SUCCESS(" Odpowiedź:")) + self.stdout.write(_json_truncated(response, max_len=800)) + + def _print_http_error(self, exc: HttpException): + self._err(f"HTTP {exc.status_code} przy {exc.url}") + self.stdout.write(f" content: {(exc.content or '')[:500]}") + + def _header(self, text): + self.stdout.write("") + self.stdout.write(self.style.MIGRATE_HEADING(f"=== {text} ===")) + + def _info(self, text): + self.stdout.write(text) + + def _warn(self, text): + self.stdout.write(self.style.WARNING(text)) + + def _err(self, text): + self.stdout.write(self.style.ERROR(text)) + + def _prompt(self, msg): + # Ogólny prompt (np. wybór 1/2/q) — nigdy nie uwzględnia yes_all. + # yes_all wpływa tylko na proste pytania "[Enter] kontynuuj" oraz + # yes/no z wartością domyślną (patrz: _prompt_enter, _prompt_yes_no). + return input(msg) + + def _prompt_enter(self): + if self.yes_all: + return + ans = input("[Enter] kontynuuj / [q] wyjście: ") + if ans.strip().lower() in ("q", "quit", "exit"): + raise UserAbort() + + def _prompt_yes_no(self, msg, default=True): + if self.yes_all: + return default + hint = "[T/n]" if default else "[t/N]" + ans = input(f"{msg} {hint}: ").strip().lower() + if not ans: + return default + if ans in ("q", "quit", "exit"): + raise UserAbort() + return ans in ("t", "tak", "y", "yes") + + def _print_summary(self): + self._header("PODSUMOWANIE") + if not self.stats: + self._info("Brak wykonanych operacji.") + return + for name, status in self.stats: + self.stdout.write(f" {name:30s} → {status}") diff --git a/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py b/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py new file mode 100644 index 000000000..25524dc20 --- /dev/null +++ b/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py @@ -0,0 +1,492 @@ +"""Testy dla interaktywnego narzędzia CLI ``pbn_test_wysylka_interaktywna``. + +Narzędzie jest interaktywne i używa wejścia przez ``input()``. +W testach mockujemy ``builtins.input`` przez ``monkeypatch``, a transport +HTTP przez ``MockTransport`` dostarczany przez fixturę ``pbn_client``. +""" + +from io import StringIO + +import pytest +from django.core.management import call_command +from django.core.management.base import CommandError + +from fixtures.pbn_api import pbn_pageable_json +from pbn_api.const import ( + PBN_DELETE_PUBLICATION_STATEMENT, + PBN_GET_INSTITUTION_STATEMENTS, + PBN_POST_INSTITUTION_STATEMENTS_URL, + PBN_POST_PUBLICATION_NO_STATEMENTS_URL, + PBN_POST_PUBLICATIONS_URL, +) +from pbn_api.exceptions import HttpException +from pbn_api.management.commands import pbn_test_wysylka_interaktywna as cmd_mod + + +def _patch_get_client(monkeypatch, pbn_client): + """Zamienia Command.get_client() żeby zwracał mockowanego klienta.""" + monkeypatch.setattr( + cmd_mod.Command, + "get_client", + lambda self, *args, **kwargs: pbn_client, + ) + + +def _patch_input(monkeypatch, answers): + """Mockuje builtins.input — zwraca kolejno podane odpowiedzi. + + Po wyczerpaniu listy zwraca pusty string (Enter = kontynuuj). + """ + it = iter(answers) + + def _input(prompt=""): + try: + return next(it) + except StopIteration: + return "" + + monkeypatch.setattr("builtins.input", _input) + + +@pytest.mark.django_db +def test_wymaga_jednego_argumentu_publikacji(pbn_client, monkeypatch): + _patch_get_client(monkeypatch, pbn_client) + _patch_input(monkeypatch, []) + + with pytest.raises(CommandError, match="dokładnie jedno"): + call_command( + "pbn_test_wysylka_interaktywna", + stdout=StringIO(), + ) + + +@pytest.mark.django_db +def test_oba_argumenty_publikacji_to_blad(pbn_client, monkeypatch): + _patch_get_client(monkeypatch, pbn_client) + _patch_input(monkeypatch, []) + + with pytest.raises(CommandError, match="dokładnie jedno"): + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + "1", + "--wydawnictwo-ciagle", + "2", + stdout=StringIO(), + ) + + +@pytest.mark.django_db +def test_nieistniejaca_publikacja_zwarte(pbn_client, monkeypatch): + _patch_get_client(monkeypatch, pbn_client) + _patch_input(monkeypatch, []) + out = StringIO() + + with pytest.raises(CommandError, match="Nie znaleziono"): + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + "999999", + stdout=out, + ) + + +@pytest.mark.django_db +def test_dry_run_nie_wysyla_niczego_do_pbn( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, +): + """W trybie --dry-run żadne żądanie HTTP nie może wyjść przez transport.""" + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + + _patch_get_client(monkeypatch, pbn_client) + # kolejność promptów: preview JSON? n, wybór endpointa=1, [Enter]-y + _patch_input(monkeypatch, ["n", "1"]) + + out = StringIO() + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk), + "--dry-run", + "--yes-all", + stdout=out, + ) + + assert pbn_client.transport.input_values == {}, ( + "W trybie --dry-run transport nie powinien dostać żadnego żądania." + ) + output = out.getvalue() + assert "DRY-RUN" in output + assert "KROK 1/8" in output + assert "PODSUMOWANIE" in output + + +@pytest.mark.django_db +def test_happy_path_endpoint_publications( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, +): + """Pełen przebieg z endpointem /api/v1/publications (all-in-one).""" + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + + _patch_get_client(monkeypatch, pbn_client) + pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { + "objectId": pbn_publication.pk, + } + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_STATEMENTS + + f"?publicationId={pbn_publication.pk}&size=5120" + ] = pbn_pageable_json([]) + + # --yes-all akceptuje domyślnie, a dla wyboru endpointu robimy wejście "1" + # — _prompt w trybie yes-all zwraca "", a _step_choose_endpoint wtedy + # pętli dopóki nie dostanie prawidłowej odpowiedzi; dlatego wstrzykujemy "1". + _patch_input(monkeypatch, ["1"]) + + out = StringIO() + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk), + "--yes-all", + stdout=out, + ) + + # POST publikacji faktycznie poszedł do endpointu publications: + assert PBN_POST_PUBLICATIONS_URL in pbn_client.transport.input_values + # Endpoint repozytoryjny nie powinien być użyty: + assert ( + PBN_POST_PUBLICATION_NO_STATEMENTS_URL not in pbn_client.transport.input_values + ) + # Porównanie oświadczeń - lokalne puste, PBN puste - identyczne, + # więc DELETE i POST /v2/statements NIE powinny się odbyć: + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk + ) + assert url_delete not in pbn_client.transport.input_values + assert PBN_POST_INSTITUTION_STATEMENTS_URL not in pbn_client.transport.input_values + + output = out.getvalue() + assert "PODSUMOWANIE" in output + assert "KROK 4/8" in output + assert "identyczne" in output + + +@pytest.mark.django_db +def test_happy_path_endpoint_repositorium( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, +): + """Pełen przebieg z endpointem /api/v1/repositorium/publications (bez oświadczeń).""" + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + + _patch_get_client(monkeypatch, pbn_client) + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": pbn_publication.pk}, + ] + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_STATEMENTS + + f"?publicationId={pbn_publication.pk}&size=5120" + ] = pbn_pageable_json([]) + + _patch_input(monkeypatch, ["2"]) + + out = StringIO() + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk), + "--yes-all", + stdout=out, + ) + + # POST faktycznie do repozytorium: + assert PBN_POST_PUBLICATION_NO_STATEMENTS_URL in pbn_client.transport.input_values + # Endpoint all-in-one NIE powinien być użyty: + assert PBN_POST_PUBLICATIONS_URL not in pbn_client.transport.input_values + + # Dodatkowo: body wysłane do repozytorium NIE ma klucza "statements" — to + # kluczowa gwarancja bezpieczeństwa narzędzia (user zastrzegł: żadnych + # nie-spec wysyłek z `statements` na endpoint repozytoryjny). + body = pbn_client.transport.input_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL][ + "body" + ] + assert isinstance(body, list) and len(body) == 1 + assert "statements" not in body[0] + + +@pytest.mark.django_db +def test_nieprawidlowa_opcja_endpointa_potem_prawidlowa( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, +): + """Po błędnej odpowiedzi na wybór endpointa pętla prosi ponownie.""" + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + + _patch_get_client(monkeypatch, pbn_client) + pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { + "objectId": pbn_publication.pk, + } + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_STATEMENTS + + f"?publicationId={pbn_publication.pk}&size=5120" + ] = pbn_pageable_json([]) + + # najpierw bzdura, potem 1 (publications) + _patch_input(monkeypatch, ["foo", "1"]) + + out = StringIO() + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk), + "--yes-all", + stdout=out, + ) + + output = out.getvalue() + assert "Nieprawidłowa opcja" in output + assert PBN_POST_PUBLICATIONS_URL in pbn_client.transport.input_values + + +@pytest.mark.django_db +def test_quit_przy_wyborze_endpointa_konczy_flow( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, +): + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + + _patch_get_client(monkeypatch, pbn_client) + _patch_input(monkeypatch, ["q"]) + + out = StringIO() + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk), + "--yes-all", + stdout=out, + ) + + # Nic nie wyszło do transportu: + assert pbn_client.transport.input_values == {} + output = out.getvalue() + assert "Przerwano" in output + assert "PODSUMOWANIE" in output + + +@pytest.mark.django_db +def test_blad_http_na_post_publikacji_nie_crashuje( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, +): + """Gdy POST publikacji zwróci HttpException 423, narzędzie ma czytelnie + wypisać błąd i wrócić do podsumowania (nie robić traceback-crash).""" + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + + _patch_get_client(monkeypatch, pbn_client) + pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = HttpException( + 423, + PBN_POST_PUBLICATIONS_URL, + '{"message":"Locked","description":"Zablokowane"}', + ) + + _patch_input(monkeypatch, ["1"]) + + out = StringIO() + # Komenda powinna zakończyć się bez wyjątku — UserAbort jest łapany w handle(). + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk), + "--yes-all", + stdout=out, + ) + + output = out.getvalue() + assert "HTTP 423" in output + assert "PODSUMOWANIE" in output + # DELETE i POST /v2/statements NIE powinny się wykonać po błędzie: + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk + ) + assert url_delete not in pbn_client.transport.input_values + assert PBN_POST_INSTITUTION_STATEMENTS_URL not in pbn_client.transport.input_values + + +@pytest.mark.django_db +def test_rozne_oswiadczenia_triggeruja_delete_post_gdy_user_zgodzi( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + pbn_jednostka, + monkeypatch, +): + """Gdy lokalne oświadczenia różnią się od PBN i user zgodzi się, + narzędzie wysyła DELETE a następnie POST /v2/statements.""" + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + + _patch_get_client(monkeypatch, pbn_client) + pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { + "objectId": pbn_publication.pk, + } + # PBN zwraca 1 oświadczenie, które lokalnie nie istnieje → różnica + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_STATEMENTS + + f"?publicationId={pbn_publication.pk}&size=5120" + ] = pbn_pageable_json( + [ + { + "id": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + "addedTimestamp": "2020.05.06", + "institutionId": pbn_jednostka.pbn_uid_id, + "personId": pbn_autor.pbn_uid_id, + "publicationId": pbn_publication.pk, + "area": "999", + "inOrcid": True, + "type": "AUTHOR", + } + ] + ) + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk + ) + pbn_client.transport.return_values[url_delete] = [] + pbn_client.transport.return_values[PBN_POST_INSTITUTION_STATEMENTS_URL] = { + "data": [] + } + + # Podmieniamy pbn_get_api_statements żeby nie wymagał PublikacjaInstytucji_V2 + # (ten fixture go nie tworzy). Zwracamy sztuczny payload. + from pbn_api.adapters.wydawnictwo import WydawnictwoPBNAdapter + + monkeypatch.setattr( + WydawnictwoPBNAdapter, + "pbn_get_api_statements", + lambda self: { + "publicationUuid": "00000000-0000-0000-0000-000000000001", + "statements": [{"personId": "x"}], + }, + ) + + # 1=publications, 't'=tak (skasuj i wyślij) + _patch_input(monkeypatch, ["1", "t"]) + + out = StringIO() + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk), + "--yes-all", + stdout=out, + ) + + # DELETE i POST /v2/statements poszły: + assert url_delete in pbn_client.transport.input_values + assert PBN_POST_INSTITUTION_STATEMENTS_URL in pbn_client.transport.input_values + assert pbn_client.transport.input_values[url_delete]["delete"] is True + + output = out.getvalue() + assert "KROK 7/8" in output + assert "KROK 8/8" in output + + +@pytest.mark.django_db +def test_odmowa_delete_post_gdy_sa_roznice_ale_user_nie_chce( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + pbn_jednostka, + monkeypatch, +): + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + + _patch_get_client(monkeypatch, pbn_client) + pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { + "objectId": pbn_publication.pk, + } + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_STATEMENTS + + f"?publicationId={pbn_publication.pk}&size=5120" + ] = pbn_pageable_json( + [ + { + "id": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + "institutionId": pbn_jednostka.pbn_uid_id, + "personId": pbn_autor.pbn_uid_id, + "publicationId": pbn_publication.pk, + "area": "999", + "inOrcid": True, + "type": "AUTHOR", + } + ] + ) + + # Pełen flow bez --yes-all, bo chcemy dać user-owi odpowiedź "n" na pytanie + # o skasowanie oświadczeń (yes_all ignoruje decyzje yes/no i wraca do defaultów). + # Lista odpowiedzi (po kolei): + # 1) Enter po KROK 1 + # 2) "" (n na preview JSON, default=False) + # 3) Enter po KROK 2 + # 4) "1" — wybór endpointa: all-in-one + # 5) "" (Wyślij teraz? default=True) + # 6) Enter po KROK 4 + # 7) Enter po KROK 5 + # 8) Enter po KROK 6 + # 9) "n" — nie kasuj oświadczeń + _patch_input(monkeypatch, ["", "", "", "1", "", "", "", "", "n"]) + + out = StringIO() + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk), + stdout=out, + ) + + # DELETE nie powinien się wykonać: + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk + ) + assert url_delete not in pbn_client.transport.input_values + assert PBN_POST_INSTITUTION_STATEMENTS_URL not in pbn_client.transport.input_values + + output = out.getvalue() + assert "decyzja użytkownika" in output + + +def test_json_truncated_obcina_dlugi_tekst(): + big = {"key": "x" * 5000} + result = cmd_mod._json_truncated(big, max_len=100) + assert len(result) < 500 + assert "obcięto" in result + + +def test_json_truncated_nie_obcina_krotkiego_tekstu(): + small = {"a": 1} + result = cmd_mod._json_truncated(small, max_len=100) + assert "obcięto" not in result + assert '"a": 1' in result From b24dd0ba38f0424a8ee098f9bee77fa27edc11a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 10:59:07 +0200 Subject: [PATCH 03/33] ci(docker): buduj obrazy tylko z pull_request (nie z push do feature/**) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `). Efekt: jeden build per commit w PR zamiast dwóch. --- .github/workflows/build-docker-images.yml | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build-docker-images.yml b/.github/workflows/build-docker-images.yml index 9ebf24569..e0ef79463 100644 --- a/.github/workflows/build-docker-images.yml +++ b/.github/workflows/build-docker-images.yml @@ -2,15 +2,19 @@ name: Docker - oficjalne obrazy on: push: + # Tylko master — release flow. Push do feature/fix/hotfix nie + # triggeruje buildu, żeby nie duplikować: PR eventy (opened/ + # synchronize/reopened/labeled) już obsługują każdy commit w + # otwartym PR-ze. Wcześniej push + pull_request odpalały dwa + # równoległe buildy dla tego samego commita (marnowanie Docker + # Cloud Build minut). Dla branchy bez PR-a, ad-hoc build przez + # `gh workflow run build-docker-images.yml --ref `. branches: - master - - 'feature/**' - - 'fix/**' - - 'hotfix/**' pull_request: - # Buduje, gdy PR ma label `docker-build` (patrz check-flag). - # Typ `labeled` obsługuje moment dodania labela na już-pushniętym - # commicie — build odpala się od razu, bez konieczności nowego pushu. + # Buduje, gdy PR ma label `docker-build` lub gdy w drzewie jest + # `.docker-build` (patrz check-flag). Typ `labeled` obsługuje + # moment dodania labela na już-pushniętym commicie. types: [opened, synchronize, reopened, labeled] workflow_dispatch: # Ręczne wywołanie z GUI GitHub lub przez From 7dc56c7977082f9314b425fed59f63f80cd40486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 10:59:27 +0200 Subject: [PATCH 04/33] docs(pbn): rozszerzona instrukcja testowania pbn_test_wysylka_interaktywna MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- docs/pbn-wysylka-plan.md | 178 +++++++++++++++++++++++++++++++++++---- 1 file changed, 160 insertions(+), 18 deletions(-) diff --git a/docs/pbn-wysylka-plan.md b/docs/pbn-wysylka-plan.md index a55745b47..07851e982 100644 --- a/docs/pbn-wysylka-plan.md +++ b/docs/pbn-wysylka-plan.md @@ -122,38 +122,180 @@ Tryb `--dry-run` pokazuje wszystkie żądania, ale nic nie wysyła. - Identyfikacja użytkownika PBN — wzorzec z `pbn_wysylka_oswiadczen/tasks.py::get_pbn_client()`. -### CLI argumenty +## Jak testować narzędzie — krok po kroku +Ta sekcja to konkretna instrukcja uruchomienia narzędzia +`pbn_test_wysylka_interaktywna` — zarówno lokalnie, jak i w kontenerze +pre-prod zbudowanym przez CI. + +### 1. Testy jednostkowe (bez PBN) + +Szybka weryfikacja że narzędzie działa na poziomie kodu — używa +mockowanego transportu, nie wymaga tokena PBN ani dostępu do sieci. + +```bash +cd /sciezka/do/worktree +UV_NO_SYNC=1 uv run --all-extras pytest \ + src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py -n auto ``` -uv run python src/manage.py pbn_test_wysylka_interaktywna \ - --wydawnictwo-zwarte # albo --wydawnictwo-ciagle - --user # użytkownik z tokenem PBN - [--dry-run] # nic nie wysyłaj + +Powinieneś zobaczyć `13 passed`. + +### 2. Smoke test na preprod PBN (tryb dry-run) + +Dry-run pokazuje wszystkie żądania HTTP które *zostałyby* wysłane do +PBN, ale nic nie wysyła. Idealne do weryfikacji że narzędzie widzi +Twoją publikację i poprawnie generuje JSON. + +```bash +# Lokalnie (worktree ma własne .venv i testcontainers): +UV_NO_SYNC=1 uv run --all-extras python src/manage.py \ + pbn_test_wysylka_interaktywna \ + --wydawnictwo-zwarte 12345 \ + --dry-run ``` -## Faza 2 — refaktoryzacja `sync_publication` (osobna gałąź) +W tym trybie żaden token PBN nie jest wymagany — narzędzie nie wysyła +niczego. Naciskasz Enter między krokami, narzędzie wypisze każde +żądanie (METODA, URL, body). -Po ręcznych testach narzędziem z Fazy 1 i udokumentowaniu wyników -(osobny dokument `docs/pbn-wysylka-eksperymenty.md` w kolejnym PR) -implementujemy docelowy flow w `src/pbn_api/client/publication_sync.py`. -Plan dla Fazy 2 — szczegóły po zebraniu obserwacji z eksperymentów. +### 3. Rzeczywisty test na preprod PBN -## Testowanie (Faza 1) +Gdy upewniłeś się że dry-run wygląda OK, puść bez `--dry-run`. Wymaga +tokena PBN — można go podać przez `--user-token` albo skonfigurować +uczelnię (`Uczelnia.pbn_api_user`) w adminie. ```bash -# Testy narzędzia (unit): -UV_NO_SYNC=1 uv run --all-extras pytest \ - src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py -n auto +# Wariant A: token z parametru +UV_NO_SYNC=1 uv run --all-extras python src/manage.py \ + pbn_test_wysylka_interaktywna \ + --wydawnictwo-zwarte 12345 \ + --user-token -# Smoke test na preprod PBN (ręczny): -uv run python src/manage.py pbn_test_wysylka_interaktywna \ - --wydawnictwo-zwarte --user --dry-run -# a potem bez --dry-run, na rzeczywistej publikacji w preprod +# Wariant B: token zaciągnięty z Uczelnia.pbn_api_user (automatycznie, +# bez --user-token, jeśli uczelnia ma skonfigurowany pbn_api_user_id) +UV_NO_SYNC=1 uv run --all-extras python src/manage.py \ + pbn_test_wysylka_interaktywna \ + --wydawnictwo-ciagle 67890 ``` +### 4. Uruchomienie w obrazie Docker zbudowanym przez CI + +Po tym jak w tej gałęzi jest plik `.docker-build`, CI buduje obrazy +`iplweb/bpp_appserver:feature-pbn-test-wysylka-interaktywna` (tag = +nazwa brancha, tylko małe litery / kreski). Po zakończeniu buildu: + +```bash +# Pobierz obraz: +docker pull iplweb/bpp_appserver:feature-pbn-test-wysylka-interaktywna + +# Uruchom narzędzie w kontenerze (zakładam że bpp-deploy już stoi): +docker exec -it \ + python src/manage.py pbn_test_wysylka_interaktywna \ + --wydawnictwo-zwarte 12345 --dry-run +``` + +### 5. Co robi narzędzie (flow, 8 kroków) + +Między każdym krokiem naciskasz **Enter** aby kontynuować albo **q** +żeby przerwać (narzędzie zawsze wypisze podsumowanie na końcu, nawet +po q). + +1. **KROK 1/8** — info o publikacji (PK, tytuł, rok, aktualny PBN UID, + liczba lokalnych `OswiadczenieInstytucji`). +2. **KROK 2/8** — generowanie JSON przez `WydawnictwoPBNAdapter`. + Narzędzie pokaże czy JSON zawiera klucz `statements` i zapyta czy + pokazać pełną treść (default: `n`, tylko preview do 600 znaków). +3. **KROK 3/8** — wybór endpointa: `[1]` `/api/v1/publications` + (all-in-one, wysyła razem z oświadczeniami jeśli są w JSON) albo + `[2]` `/api/v1/repositorium/publications` (narzędzie usuwa klucz + `statements` i przepuszcza JSON przez + `convert_js_with_statements_to_no_statements` — zgodnie ze + specyfikacją PBN endpoint repozytoryjny nie przyjmuje oświadczeń). +4. **KROK 4/8** — POST publikacji. Narzędzie wypisze URL, body i prosi + o potwierdzenie. Po sukcesie wyciąga `objectId` z odpowiedzi PBN. +5. **KROK 5/8** — GET aktualnych oświadczeń z PBN dla tego objectId + (`/api/v1/institutionProfile/publications/page/statements + ?publicationId={id}`). +6. **KROK 6/8** — porównanie oświadczeń. Narzędzie pokazuje: + - ile oświadczeń jest identycznych (lokalne ∩ PBN), + - ile jest tylko lokalnie (lokalne \ PBN), + - ile jest tylko w PBN (PBN \ lokalne). + Jeśli sety są identyczne, narzędzie kończy flow (nie rusza + oświadczeń). Inaczej pyta czy kasować i nadpisywać. +7. **KROK 7/8** — DELETE oświadczeń w PBN + (`DELETE /api/v1/institutionProfile/publications/{objectId}` z + `{"all": true, "statementsOfPersons": []}`). Pytanie o potwierdzenie. +8. **KROK 8/8** — POST nowych oświadczeń + (`POST /api/v2/institution-profile/statements` z payloadem z + `WydawnictwoPBNAdapter.pbn_get_api_statements()`). Retry ×3 dla + HTTP 500/423 z exponential backoff. + +Narzędzie wypisuje **PODSUMOWANIE** z wynikami każdego kroku (OK / +dry-run / pominięty / BŁĄD HTTP XXX). + +### 6. Co sprawdzać ręcznie w preprod (checklist) + +Cel Fazy 1: zebrać empiryczne obserwacje zachowania PBN przed +decyzjami projektowymi Fazy 2. + +- [ ] **Publikacja z PBN UID + oświadczeniami lokalnymi**, wysłana + opcją `[1]` (`/api/v1/publications`) — potwierdzenie że PBN + zaakceptuje JSON z `statements` (znany case, weryfikacja baseline). +- [ ] **Ta sama publikacja** wysłana opcją `[2]` + (`/api/v1/repositorium/publications` bez `statements` w JSON) — + czy PBN zaakceptuje? Jaki jest `status` publikacji po wysyłce? + Czy oświadczenia wcześniej skojarzone z publikacją pozostają + nietknięte? +- [ ] **Sekwencja** opcja `[2]` → GET oświadczeń → jeśli się różnią, + `t` na DELETE → POST `/v2/statements`. Ma to być docelowy flow + Fazy 2 — chcemy wiedzieć że PBN jest z tym OK. +- [ ] **Edge case** — publikacja ze statusem "LOGED" (jeśli + napotkamy): co zwraca GET? Czy DELETE działa? Co zwraca POST + `/v2/statements`? +- [ ] **Publikacja bez PBN UID (nowa)** — opcja `[1]` powinna + zwrócić nowy `objectId`. Opcja `[2]` też (`.../repositorium`). +- [ ] **Publikacja bez oświadczeń lokalnych** — porównanie w KROK 6/8 + ma pokazać że lokalne są puste, a więc nie ma o czym rozmawiać. + +Obserwacje notujemy w osobnym pliku `docs/pbn-wysylka-eksperymenty.md` +(tworzony w osobnym PR), żeby były bazą do decyzji w Fazie 2. + +### 7. Rozwiązywanie problemów + +- **`NeedsPBNAuthorisationException`** — brak tokena PBN. Podaj + `--user-token ` albo skonfiguruj `Uczelnia.pbn_api_user_id` + (Django admin → Redagowanie → Uczelnia). +- **`BrakZdefiniowanegoObiektuUczelniaWSystemieError`** — stwórz + obiekt Uczelnia w adminie (tylko raz na instalację). +- **`DaneLokalneWymagajaAktualizacjiException`** w KROK 8/8 — brak + lokalnego `PublikacjaInstytucji_V2` dla tego PBN UID. Pobierz + ręcznie przez + `python src/manage.py pbn_pobierz_publikacje_z_instytucji_v2 + --user-token ` albo wyślij publikację najpierw opcją `[1]` + (PBN wtedy zwróci PublikacjaInstytucji_V2, a my ją pobierzemy). +- **`HttpException 423 Locked`** przy POST — publikacja lub zasób + zablokowany w PBN. Poczekaj chwilę i spróbuj ponownie (dla POST + `/v2/statements` narzędzie ma wbudowany retry ×3). +- **Pliki migracji** — jeśli podczas pytest widzisz konflikt leaf + nodes w `importer_publikacji`, zrób + `uv run python src/manage.py makemigrations --merge --noinput` + (nie modyfikuje istniejących migracji, dodaje nową merge-ową). + +## Faza 2 — refaktoryzacja `sync_publication` (osobna gałąź) + +Po ręcznych testach narzędziem z Fazy 1 i udokumentowaniu wyników +(osobny dokument `docs/pbn-wysylka-eksperymenty.md` w kolejnym PR) +implementujemy docelowy flow w `src/pbn_api/client/publication_sync.py`. +Plan dla Fazy 2 — szczegóły po zebraniu obserwacji z eksperymentów. + ## Wymagania CI - `.docker-build` w root repo ⇒ CI buduje obraz Docker dla tej gałęzi, tak by user mógł uruchomić narzędzie w środowisku testowym. - Pełny pipeline (`tests.yml`): lint + testy (non-playwright, serial, playwright) muszą przejść. +- Build Docker odpala się **raz** na commit (push do master lub + event pull_request) — od zmiany z 2026-04-21 push do + feature/fix/hotfix nie triggeruje już buildu (tylko PR events), + żeby uniknąć duplikatów. From ac077e520c7e96ecf3542d89c730d2e97c193207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 11:37:14 +0200 Subject: [PATCH 05/33] =?UTF-8?q?fix(pbn-test-narzedzie):=20por=C3=B3wnuj?= =?UTF-8?q?=20intencj=C4=99=20BPP=20(live)=20zamiast=20cache=20i=20pytaj?= =?UTF-8?q?=20osobno=20o=20DELETE/POST?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/pbn-wysylka-plan.md | 27 ++-- ...+pbn-test-narzedzie-fix-compare.bugfix.rst | 20 +++ .../commands/pbn_test_wysylka_interaktywna.py | 129 +++++++++++------- .../test_pbn_test_wysylka_interaktywna.py | 125 +++++++++++++++-- 4 files changed, 238 insertions(+), 63 deletions(-) create mode 100644 src/bpp/newsfragments/+pbn-test-narzedzie-fix-compare.bugfix.rst diff --git a/docs/pbn-wysylka-plan.md b/docs/pbn-wysylka-plan.md index 07851e982..869a969c0 100644 --- a/docs/pbn-wysylka-plan.md +++ b/docs/pbn-wysylka-plan.md @@ -217,19 +217,30 @@ po q). 5. **KROK 5/8** — GET aktualnych oświadczeń z PBN dla tego objectId (`/api/v1/institutionProfile/publications/page/statements ?publicationId={id}`). -6. **KROK 6/8** — porównanie oświadczeń. Narzędzie pokazuje: - - ile oświadczeń jest identycznych (lokalne ∩ PBN), - - ile jest tylko lokalnie (lokalne \ PBN), - - ile jest tylko w PBN (PBN \ lokalne). - Jeśli sety są identyczne, narzędzie kończy flow (nie rusza - oświadczeń). Inaczej pyta czy kasować i nadpisywać. +6. **KROK 6/8** — porównanie: **intencja BPP na żywo** vs **aktualnie + w PBN**. „Intencja BPP" to wynik + `WydawnictwoPBNAdapter.pbn_get_api_statements()` — generowany z + aktualnych `Wydawnictwo_*_Autor` + dyscyplin, czyli to co BPP + **wysłałby teraz**. **Nie** używamy lokalnego cache'a + `OswiadczenieInstytucji` (to snapshot poprzedniej synchronizacji + *PBN*, nie aktualnej intencji BPP — po skasowaniu autora cache + zostałby nieaktualny). Narzędzie pokazuje: + - ile oświadczeń jest identycznych (intencja ∩ PBN), + - ile jest tylko w intencji (intencja \ PBN, „do dodania"), + - ile jest tylko w PBN (PBN \ intencja, „do usunięcia"). + Następnie przechodzi do pytań o DELETE i POST (punkty 7-8). 7. **KROK 7/8** — DELETE oświadczeń w PBN (`DELETE /api/v1/institutionProfile/publications/{objectId}` z - `{"all": true, "statementsOfPersons": []}`). Pytanie o potwierdzenie. + `{"all": true, "statementsOfPersons": []}`). Narzędzie **zawsze + pyta** czy wykonać DELETE, nawet gdy porównanie zwróciło + identyczność. Domyślna wartość zależy od wyniku KROK 6/8: + „identyczne" → default `n`, „różnice" → default `t`. Pozwala to + wymusić DELETE dla testowania reakcji PBN. 8. **KROK 8/8** — POST nowych oświadczeń (`POST /api/v2/institution-profile/statements` z payloadem z `WydawnictwoPBNAdapter.pbn_get_api_statements()`). Retry ×3 dla - HTTP 500/423 z exponential backoff. + HTTP 500/423 z exponential backoff. **Zawsze pyta** — jak w + KROK 7/8 — z domyślną wartością zależną od identyczności. Narzędzie wypisuje **PODSUMOWANIE** z wynikami każdego kroku (OK / dry-run / pominięty / BŁĄD HTTP XXX). diff --git a/src/bpp/newsfragments/+pbn-test-narzedzie-fix-compare.bugfix.rst b/src/bpp/newsfragments/+pbn-test-narzedzie-fix-compare.bugfix.rst new file mode 100644 index 000000000..36df1ce33 --- /dev/null +++ b/src/bpp/newsfragments/+pbn-test-narzedzie-fix-compare.bugfix.rst @@ -0,0 +1,20 @@ +Poprawka w narzędziu CLI ``pbn_test_wysylka_interaktywna``: + +- krok porównania oświadczeń (KROK 6/8) używał lokalnego cache'a + ``OswiadczenieInstytucji`` (snapshot z poprzedniej synchronizacji + PBN) jako reprezentanta „stanu BPP", co powodowało fałszywą + identyczność po zmianach w rekordzie — skasowaniu autora, + zmianie/wypięciu dyscypliny lub innej edycji ``Wydawnictwo_*_Autor`` + (cache nie był re-synchronizowany, pokazywał stare 3 oświadczenia + nawet po faktycznym zmniejszeniu intencji BPP do 2). Narzędzie + teraz porównuje **intencję BPP na żywo** — to co by wygenerował + ``WydawnictwoPBNAdapter.pbn_get_api_statements()`` gdyby wysyłać + teraz — z aktualnym stanem PBN. Dodatkowo KROK 1/8 pokazuje zarówno + cache jak i intencję żywą, żeby od razu widać było rozjazd. +- narzędzie zawsze pyta osobno o DELETE oświadczeń i osobno o POST + oświadczeń, także gdy porównanie zwróciło identyczność — + użytkownik może wymusić operację np. dla empirycznego sprawdzenia + reakcji PBN (wcześniej flow kończył się wczesnym ``return`` po + identyczności bez opcji kontynuacji). Domyślna wartość pytania + zależy od wyniku porównania: „identyczne" → default ``n``, + „różnice" → default ``t``. diff --git a/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py b/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py index 69303a770..52e2f165e 100644 --- a/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py +++ b/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py @@ -154,36 +154,60 @@ def _run_flow(self, pbn_client, publication): pbn_statements = self._step_get_pbn_statements(pbn_client, object_id) identyczne = self._step_compare_statements(publication, pbn_statements) - if identyczne: - self._info("Oświadczenia w PBN identyczne z lokalnymi — nie ruszam ich.") - return - - if not self._prompt_yes_no( - "Czy skasować oświadczenia w PBN i wysłać nowe?", default=True + # Zawsze pytamy osobno o DELETE i POST — nawet gdy identyczne. + # Default zależy od wyniku porównania (False dla identycznych, + # True dla różnic), ale user ma ostatnie słowo. Pozwala to + # wymusić operację (np. żeby empirycznie sprawdzić jak PBN + # reaguje na „zbędny" DELETE+POST). + ctx = "identyczne — zwykle nie trzeba" if identyczne else "są różnice" + default_act = not identyczne + + if self._prompt_yes_no( + f"Czy skasować oświadczenia w PBN? ({ctx})", default=default_act ): - self._info("Pominięto DELETE + POST oświadczeń (decyzja użytkownika).") - return + self._step_delete_statements(pbn_client, object_id) + else: + self._info("Pominięto DELETE oświadczeń (decyzja użytkownika).") + self.stats.append(("DELETE oświadczeń", "pominięty")) - self._step_delete_statements(pbn_client, object_id) - self._step_post_statements(pbn_client, publication) + if self._prompt_yes_no( + f"Czy wysłać (POST) oświadczenia do PBN? ({ctx})", + default=default_act, + ): + self._step_post_statements(pbn_client, publication) + else: + self._info("Pominięto POST oświadczeń (decyzja użytkownika).") + self.stats.append(("POST oświadczeń", "pominięty")) # ------------------------- kroki ------------------------- def _step_show_publication(self, publication): self._header("KROK 1/8 — Wybrana publikacja") - lokalne = ( + + # Cache ostatniej synchronizacji z PBN (może być nieaktualny). + cache = ( OswiadczenieInstytucji.objects.filter( publicationId_id=publication.pbn_uid_id ).count() if publication.pbn_uid_id else 0 ) + # Intencja BPP — live count tego co by adapter wysłał teraz. + try: + intended_count = len( + WydawnictwoPBNAdapter(publication).pbn_get_json_statements() + ) + intended_label = str(intended_count) + except Exception as e: # noqa: BLE001 + intended_label = f"?? (błąd adaptera: {e})" + self._info(f"Typ: {type(publication).__name__}") self._info(f"PK: {publication.pk}") self._info(f"Tytuł: {publication.tytul_oryginalny[:100]}") self._info(f"Rok: {publication.rok}") self._info(f"PBN UID: {publication.pbn_uid_id or '(brak)'}") - self._info(f"Oświadczenia lokalnie (OswiadczenieInstytucji): {lokalne}") + self._info(f"Oświadczenia w cache (OswiadczenieInstytucji): {cache}") + self._info(f"Oświadczenia intencji BPP (live, adapter): {intended_label}") self._prompt_enter() def _step_generate_json(self, publication): @@ -324,43 +348,56 @@ def _step_get_pbn_statements(self, pbn_client, object_id): return result def _step_compare_statements(self, publication, pbn_statements): - self._header("KROK 6/8 — Porównanie oświadczeń lokalnych z PBN") - lokalne = ( - list( - OswiadczenieInstytucji.objects.filter( - publicationId_id=publication.pbn_uid_id - ).values("personId_id", "area", "institutionId_id") + self._header("KROK 6/8 — Porównanie: intencja BPP (live) vs PBN") + + # Intencja BPP: co wygenerowałby adapter GDYBY teraz wysłać — czyli + # aktualny stan autorów/dyscyplin w rekordzie. NIE używamy cache'a + # ``OswiadczenieInstytucji`` (to snapshot *PBN* z poprzedniego + # pobrania, nie BPP); po edycji w rekordzie — skasowaniu autora, + # dyscypliny, wypięciu — cache pozostałby nieaktualny. + try: + payload = WydawnictwoPBNAdapter(publication).pbn_get_api_statements() + intended = payload.get("statements", []) + except DaneLokalneWymagajaAktualizacjiException as e: + self._warn( + f"Nie mogę wygenerować intended-state " + f"(adapter wymaga UUID publikacji z PublikacjaInstytucji_V2): {e}" ) - if publication.pbn_uid_id - else [] - ) + self._info("Zwracam 'różnice' — user zdecyduje co robić.") + self.stats.append(("Porównanie", "nieznane (błąd adaptera)")) + self._prompt_enter() + return False # traktujemy jak "różne" def _key(stmt): - if isinstance(stmt, dict): - return ( - stmt.get("personId") or stmt.get("personId_id"), - str(stmt.get("area") or ""), - stmt.get("institutionId") or stmt.get("institutionId_id"), - ) - return (None, None, None) + if not isinstance(stmt, dict): + return (None, None, None) + # PBN v1 zwraca personId (mongoId), v2 statement payload używa + # personObjectId. Adapter zwraca personId. Bierzemy co jest. + return ( + stmt.get("personId") or stmt.get("personObjectId"), + str(stmt.get("area") or ""), + stmt.get("institutionId"), + ) - local_keys = {_key(x) for x in lokalne} + intended_keys = {_key(x) for x in intended} pbn_keys = {_key(x) for x in pbn_statements} - only_local = local_keys - pbn_keys - only_pbn = pbn_keys - local_keys - common = local_keys & pbn_keys + only_intended = intended_keys - pbn_keys + only_pbn = pbn_keys - intended_keys + common = intended_keys & pbn_keys - self._info(f"Identycznych (lokalne ∩ PBN): {len(common)}") - self._info(f"Tylko lokalnie (lokalne \\ PBN): {len(only_local)}") - self._info(f"Tylko w PBN (PBN \\ lokalne): {len(only_pbn)}") + self._info(f"Intencja BPP (live): {len(intended_keys)}") + self._info(f"Aktualnie w PBN: {len(pbn_keys)}") + self._info(f"Identycznych: {len(common)}") + self._info(f"Tylko w intencji (do dodania): {len(only_intended)}") + self._info(f"Tylko w PBN (do usunięcia): {len(only_pbn)}") - if only_local: - self._info(f" → lokalne bez odpowiednika w PBN: {list(only_local)[:5]}") + if only_intended: + self._info(f" → intencja bez PBN: {list(only_intended)[:5]}") if only_pbn: - self._info(f" → w PBN bez odpowiednika lokalnie: {list(only_pbn)[:5]}") + self._info(f" → w PBN bez intencji: {list(only_pbn)[:5]}") - identyczne = not only_local and not only_pbn + identyczne = not only_intended and not only_pbn self.stats.append( ( "Porównanie", @@ -384,10 +421,9 @@ def _step_delete_statements(self, pbn_client, object_id): self._prompt_enter() return - if not self._prompt_yes_no("Wyślij DELETE?", default=True): - self._info("Pominięto DELETE (decyzja użytkownika).") - self.stats.append(("DELETE oświadczeń", "pominięty")) - return + # Uwaga: zewnętrzny _prompt_yes_no w _run_flow już zapytał czy + # w ogóle robić DELETE — jeśli tu jesteśmy, user się zgodził. + # Nie dodajemy drugiego pytania "Wyślij DELETE?" dla prostoty. try: response = pbn_client.delete_all_publication_statements(object_id) @@ -430,10 +466,9 @@ def _step_post_statements(self, pbn_client, publication): self._prompt_enter() return - if not self._prompt_yes_no("Wyślij POST oświadczeń?", default=True): - self._info("Pominięto POST oświadczeń (decyzja użytkownika).") - self.stats.append(("POST oświadczeń", "pominięty")) - return + # Zewnętrzny _prompt_yes_no w _run_flow już zapytał czy w ogóle + # robić POST — jeśli tu jesteśmy, user się zgodził. Pomijamy + # drugie pytanie "Wyślij POST?" dla prostoty. max_tries = 3 attempt = 0 diff --git a/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py b/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py index 25524dc20..4eafd3553 100644 --- a/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py +++ b/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py @@ -48,6 +48,22 @@ def _input(prompt=""): monkeypatch.setattr("builtins.input", _input) +def _patch_intended_statements(monkeypatch, statements): + """Patchuje WydawnictwoPBNAdapter.pbn_get_api_statements() żeby + zwracał zadaną listę intended statements (bez wymagania PublikacjaInstytucji_V2). + """ + from pbn_api.adapters.wydawnictwo import WydawnictwoPBNAdapter + + monkeypatch.setattr( + WydawnictwoPBNAdapter, + "pbn_get_api_statements", + lambda self: { + "publicationUuid": "00000000-0000-0000-0000-000000000001", + "statements": list(statements), + }, + ) + + @pytest.mark.django_db def test_wymaga_jednego_argumentu_publikacji(pbn_client, monkeypatch): _patch_get_client(monkeypatch, pbn_client) @@ -103,6 +119,7 @@ def test_dry_run_nie_wysyla_niczego_do_pbn( pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() _patch_get_client(monkeypatch, pbn_client) + _patch_intended_statements(monkeypatch, []) # kolejność promptów: preview JSON? n, wybór endpointa=1, [Enter]-y _patch_input(monkeypatch, ["n", "1"]) @@ -137,6 +154,9 @@ def test_happy_path_endpoint_publications( pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() _patch_get_client(monkeypatch, pbn_client) + # Intended statements puste — tak samo jak PBN → identyczne → domyślnie + # nie robimy DELETE ani POST oświadczeń (default_act=False w yes_all). + _patch_intended_statements(monkeypatch, []) pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { "objectId": pbn_publication.pk, } @@ -145,9 +165,8 @@ def test_happy_path_endpoint_publications( + f"?publicationId={pbn_publication.pk}&size=5120" ] = pbn_pageable_json([]) - # --yes-all akceptuje domyślnie, a dla wyboru endpointu robimy wejście "1" - # — _prompt w trybie yes-all zwraca "", a _step_choose_endpoint wtedy - # pętli dopóki nie dostanie prawidłowej odpowiedzi; dlatego wstrzykujemy "1". + # --yes-all akceptuje domyślnie (Enter, yes/no z defaultem), a dla + # wyboru endpointu (niedomyślny prompt) musimy dostarczyć "1". _patch_input(monkeypatch, ["1"]) out = StringIO() @@ -191,6 +210,7 @@ def test_happy_path_endpoint_repositorium( pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() _patch_get_client(monkeypatch, pbn_client) + _patch_intended_statements(monkeypatch, []) pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ {"id": pbn_publication.pk}, ] @@ -237,6 +257,7 @@ def test_nieprawidlowa_opcja_endpointa_potem_prawidlowa( pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() _patch_get_client(monkeypatch, pbn_client) + _patch_intended_statements(monkeypatch, []) pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { "objectId": pbn_publication.pk, } @@ -445,9 +466,10 @@ def test_odmowa_delete_post_gdy_sa_roznice_ale_user_nie_chce( ] ) - # Pełen flow bez --yes-all, bo chcemy dać user-owi odpowiedź "n" na pytanie - # o skasowanie oświadczeń (yes_all ignoruje decyzje yes/no i wraca do defaultów). - # Lista odpowiedzi (po kolei): + # Pełen flow bez --yes-all, bo chcemy dać user-owi odpowiedź "n" na + # dwa pytania (DELETE i POST). yes_all ignoruje decyzje yes/no i + # wraca do defaultów, a default_act=True (są różnice) → DELETE+POST + # by się wykonały. Lista odpowiedzi (po kolei): # 1) Enter po KROK 1 # 2) "" (n na preview JSON, default=False) # 3) Enter po KROK 2 @@ -456,8 +478,9 @@ def test_odmowa_delete_post_gdy_sa_roznice_ale_user_nie_chce( # 6) Enter po KROK 4 # 7) Enter po KROK 5 # 8) Enter po KROK 6 - # 9) "n" — nie kasuj oświadczeń - _patch_input(monkeypatch, ["", "", "", "1", "", "", "", "", "n"]) + # 9) "n" — nie kasuj oświadczeń (DELETE) + # 10) "n" — nie wysyłaj oświadczeń (POST) + _patch_input(monkeypatch, ["", "", "", "1", "", "", "", "", "n", "n"]) out = StringIO() call_command( @@ -478,6 +501,92 @@ def test_odmowa_delete_post_gdy_sa_roznice_ale_user_nie_chce( assert "decyzja użytkownika" in output +@pytest.mark.django_db +def test_compare_uses_intended_not_cache_bug1( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + pbn_jednostka, + monkeypatch, +): + """Regression BUG 1: narzędzie ma porównywać intended (adapter) z PBN, + NIE cache OswiadczenieInstytucji. + + Scenariusz: cache (OswiadczenieInstytucji) ma 1 rekord (stary stan), + PBN zwraca ten sam 1 rekord. Intended z adaptera zwraca PUSTĄ listę + (user skasował autora lokalnie). Stary kod pokazałby "identyczne" + (cache 1 == PBN 1). Nowy kod musi pokazać "różnice" (intended 0 ≠ PBN 1). + """ + from datetime import date + + from model_bakery import baker + + from pbn_api.models import OswiadczenieInstytucji + + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + + # Cache (stary stan, niesprzątnięty) — 1 rekord OswiadczenieInstytucji: + baker.make( + OswiadczenieInstytucji, + publicationId=pbn_publication, + personId=pbn_autor.pbn_uid, + institutionId=pbn_jednostka.pbn_uid, + area=100, + addedTimestamp=date(2020, 1, 1), + ) + assert OswiadczenieInstytucji.objects.count() == 1 + + # Intended BPP (live) — PUSTE (jakby user skasował autora z rekordu): + _patch_intended_statements(monkeypatch, []) + + _patch_get_client(monkeypatch, pbn_client) + pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { + "objectId": pbn_publication.pk, + } + # PBN zwraca 1 oświadczenie (zgodne z cache): + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_STATEMENTS + + f"?publicationId={pbn_publication.pk}&size=5120" + ] = pbn_pageable_json( + [ + { + "id": "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb", + "institutionId": pbn_jednostka.pbn_uid_id, + "personId": pbn_autor.pbn_uid_id, + "publicationId": pbn_publication.pk, + "area": "100", + "type": "AUTHOR", + "inOrcid": True, + } + ] + ) + + # Wybieramy endpoint 1, nie kasujemy/nie wysyłamy (chcemy sprawdzić + # tylko KROK 6/8 output). + _patch_input( + monkeypatch, + ["", "", "", "1", "", "", "", "", "n", "n"], + ) + + out = StringIO() + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk), + stdout=out, + ) + + output = out.getvalue() + # Musi pokazać intencja 0 vs PBN 1 → różnice (NIE identyczne): + assert "Intencja BPP (live): 0" in output + assert "Aktualnie w PBN: 1" in output + assert "Tylko w PBN (do usunięcia): 1" in output + # A NIE pokazać że są identyczne (to byłby stary bug): + assert "Porównanie → różnice" in output + + def test_json_truncated_obcina_dlugi_tekst(): big = {"key": "x" * 5000} result = cmd_mod._json_truncated(big, max_len=100) From df6c5c65ba24fed7feaaa989c1661f436323f5e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 11:47:27 +0200 Subject: [PATCH 06/33] =?UTF-8?q?fix(tests):=20naprawa=202=20pre-existing?= =?UTF-8?q?=20failing=20test=C3=B3w=20na=20dev?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../test_admin_helpers/test_pbn_api_cli.py | 11 ++++++++++ .../tests/test_pbn_check.py | 20 +++++++++---------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/bpp/tests/test_admin_helpers/test_pbn_api_cli.py b/src/bpp/tests/test_admin_helpers/test_pbn_api_cli.py index 6ce6e287b..0efdac020 100644 --- a/src/bpp/tests/test_admin_helpers/test_pbn_api_cli.py +++ b/src/bpp/tests/test_admin_helpers/test_pbn_api_cli.py @@ -22,6 +22,12 @@ def test_TextNotificator(level): @pytest.mark.django_db def test_sprobuj_wyslac_do_pbn_celery(wydawnictwo_ciagle, admin_user, mocker): + # WydawnictwoPBNAdapter wymaga DOI lub www; bez tego rzuca DOIorWWWMissing + # zanim dojdziemy do walidacji tokena PBN — ustawiamy DOI, żeby test + # mógł sprawdzić późniejsze kroki (Uczelnia, token, sukces). + wydawnictwo_ciagle.doi = "10.1234/test" + wydawnictwo_ciagle.save() + cf = mommy.make("bpp.Charakter_Formalny", rodzaj_pbn=None) wydawnictwo_ciagle.charakter_formalny = cf wydawnictwo_ciagle.save() @@ -45,6 +51,11 @@ def test_sprobuj_wyslac_do_pbn_celery(wydawnictwo_ciagle, admin_user, mocker): uczelnia.pbn_app_name = uczelnia.pbn_app_token = "fubar" uczelnia.save() + # cli.py sprawdza ``user.pbn_token is None`` — model BppUser ma + # default="" (nie None), więc wymuszamy None in-memory żeby trafić + # w gałąź rzucającą NeedsPBNAuthorisationException. + admin_user.pbn_token = None + with pytest.raises(NeedsPBNAuthorisationException): sprobuj_wyslac_do_pbn_celery(admin_user, wydawnictwo_ciagle) diff --git a/src/importer_publikacji/tests/test_pbn_check.py b/src/importer_publikacji/tests/test_pbn_check.py index a8920605a..b831b6fa4 100644 --- a/src/importer_publikacji/tests/test_pbn_check.py +++ b/src/importer_publikacji/tests/test_pbn_check.py @@ -45,19 +45,19 @@ def test_check_pbn_skip_when_empty_doi(): assert _check_pbn_by_doi(session) is None -@patch("importer_publikacji.views._get_pbn_client") -def test_check_pbn_skip_when_client_fails(mock_import): - mock_import.side_effect = ValueError("Brak konfiguracji") +def test_check_pbn_skip_when_client_fails(): + # W views.py ``_get_pbn_client`` jest importowany lokalnie w ciele + # ``_check_pbn_by_doi`` (from .providers.pbn import _get_pbn_client) — + # nie jest atrybutem modułu ``importer_publikacji.views`` i nie można + # go zpatchować przez "importer_publikacji.views._get_pbn_client". + # Patchujemy u źródła (``importer_publikacji.providers.pbn``), co + # jest równoważne — lokalny import złapie zpatchowaną wersję. with patch( - "importer_publikacji.views._get_pbn_client", + "importer_publikacji.providers.pbn._get_pbn_client", side_effect=ValueError("Brak konfiguracji"), ): - with patch( - "importer_publikacji.providers.pbn._get_pbn_client", - side_effect=ValueError("Brak konfiguracji"), - ): - session = _make_session() - assert _check_pbn_by_doi(session) is None + session = _make_session() + assert _check_pbn_by_doi(session) is None def test_get_pbn_publication_by_doi_success(): From a0824fb3aa53dfc323c24d3e99193fe3c24c9c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 12:09:16 +0200 Subject: [PATCH 07/33] =?UTF-8?q?fix(pbn-test-narzedzie):=20popraw=20klucz?= =?UTF-8?q?e=20por=C3=B3wnania=20w=20KROK=206/8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../commands/pbn_test_wysylka_interaktywna.py | 40 +++++++++++++------ .../test_pbn_test_wysylka_interaktywna.py | 34 ++++++++++++++-- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py b/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py index 52e2f165e..16cfc273e 100644 --- a/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py +++ b/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py @@ -355,28 +355,42 @@ def _step_compare_statements(self, publication, pbn_statements): # ``OswiadczenieInstytucji`` (to snapshot *PBN* z poprzedniego # pobrania, nie BPP); po edycji w rekordzie — skasowaniu autora, # dyscypliny, wypięciu — cache pozostałby nieaktualny. + # + # Używamy ``pbn_get_json_statements()`` (surowa lista dict-ów + # przed konwersją w ``pbn_get_api_statements``, która usuwa + # ``disciplineId`` gdy jest ``disciplineUuid``). Surowy format + # zachowuje ``disciplineId`` (numerek MNiSW) i ``personObjectId`` + # — oba nam potrzebne do porównania z PBN GET response, gdzie są + # ``area`` i ``personId``. try: - payload = WydawnictwoPBNAdapter(publication).pbn_get_api_statements() - intended = payload.get("statements", []) - except DaneLokalneWymagajaAktualizacjiException as e: - self._warn( - f"Nie mogę wygenerować intended-state " - f"(adapter wymaga UUID publikacji z PublikacjaInstytucji_V2): {e}" - ) + intended = WydawnictwoPBNAdapter(publication).pbn_get_json_statements() + except Exception as e: # noqa: BLE001 + self._warn(f"Nie mogę wygenerować intencji BPP (adapter): {e}") self._info("Zwracam 'różnice' — user zdecyduje co robić.") self.stats.append(("Porównanie", "nieznane (błąd adaptera)")) self._prompt_enter() return False # traktujemy jak "różne" def _key(stmt): + """Klucz porównania: (person-mongoId, disciplineNumer). + + Mapowanie między formatami: + - PBN GET response (``/page/statements``): ``personId`` (mongoId), + ``area`` (string, numerek dyscypliny MNiSW np. "301"). + - Adapter ``pbn_get_json_statements()`` (przed konwersją): + ``personObjectId`` (mongoId), ``disciplineId`` (int, numerek + dyscypliny MNiSW). + Oba oznaczają to samo. + """ if not isinstance(stmt, dict): - return (None, None, None) - # PBN v1 zwraca personId (mongoId), v2 statement payload używa - # personObjectId. Adapter zwraca personId. Bierzemy co jest. + return (None, None) + person = stmt.get("personId") or stmt.get("personObjectId") + discipline = stmt.get("area") + if discipline is None: + discipline = stmt.get("disciplineId") return ( - stmt.get("personId") or stmt.get("personObjectId"), - str(stmt.get("area") or ""), - stmt.get("institutionId"), + str(person) if person else None, + str(discipline) if discipline is not None else "", ) intended_keys = {_key(x) for x in intended} diff --git a/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py b/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py index 4eafd3553..017800c41 100644 --- a/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py +++ b/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py @@ -49,19 +49,47 @@ def _input(prompt=""): def _patch_intended_statements(monkeypatch, statements): - """Patchuje WydawnictwoPBNAdapter.pbn_get_api_statements() żeby - zwracał zadaną listę intended statements (bez wymagania PublikacjaInstytucji_V2). + """Patchuje adapter żeby zwracał zadaną listę intended statements. + + Patch dotyczy: + - ``pbn_get_json_statements`` — używana w KROK 6/8 (porównanie). + Zwraca surową listę dict-ów (każdy może mieć personObjectId, + disciplineId, disciplineUuid, type itd.). + - ``pbn_get_api_statements`` — używana w KROK 8/8 (POST /v2/statements). + Zwraca ``{"publicationUuid": ..., "statements": [...]}``. + - ``__init__`` — dodatkowo ustawia ``pbn_wysylaj_bez_oswiadczen=True`` + na instancji. Potrzebne gdy ``statements=[]``: inaczej ``pbn_get_json`` + w KROK 2/8 wywoła ``StatementsMissing`` (bo artykuł/rozdział bez + statements jest walidowany jako błąd). W testach chcemy móc + symulować każdy stan — flaga wyłącza walidację. + + Fixture testowy ``pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina`` nie + tworzy ``PublikacjaInstytucji_V2``, bez czego ``pbn_get_api_statements`` + rzuciłoby ``DaneLokalneWymagajaAktualizacjiException``. """ from pbn_api.adapters.wydawnictwo import WydawnictwoPBNAdapter + statements_list = list(statements) + monkeypatch.setattr( + WydawnictwoPBNAdapter, + "pbn_get_json_statements", + lambda self, _lst=None: list(statements_list), + ) monkeypatch.setattr( WydawnictwoPBNAdapter, "pbn_get_api_statements", lambda self: { "publicationUuid": "00000000-0000-0000-0000-000000000001", - "statements": list(statements), + "statements": list(statements_list), }, ) + original_init = WydawnictwoPBNAdapter.__init__ + + def patched_init(self, *args, **kwargs): + original_init(self, *args, **kwargs) + self.pbn_wysylaj_bez_oswiadczen = True + + monkeypatch.setattr(WydawnictwoPBNAdapter, "__init__", patched_init) @pytest.mark.django_db From 16ebdfaac8d630c65aa85495b4290f3c86b9f9de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 12:49:11 +0200 Subject: [PATCH 08/33] =?UTF-8?q?fix(migrations):=200007=5Fmerge=20?= =?UTF-8?q?=E2=80=94=20=C5=82=C4=85czy=200006=20z=20branch=20i=200006=20z?= =?UTF-8?q?=20dev?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 71fe245f). 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. --- .../migrations/0007_merge_20260421_1248.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/importer_publikacji/migrations/0007_merge_20260421_1248.py diff --git a/src/importer_publikacji/migrations/0007_merge_20260421_1248.py b/src/importer_publikacji/migrations/0007_merge_20260421_1248.py new file mode 100644 index 000000000..3857f0ad4 --- /dev/null +++ b/src/importer_publikacji/migrations/0007_merge_20260421_1248.py @@ -0,0 +1,13 @@ +# Generated by Django 5.2.13 on 2026-04-21 10:48 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("importer_publikacji", "0006_merge_20260420_2212"), + ("importer_publikacji", "0006_merge_20260421_1100"), + ] + + operations = [] From daa66f4670189ea73bb69a1016f0d9fd9364c978 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 21:39:20 +0200 Subject: [PATCH 09/33] feat(pbn): StatementsResendFailedException + Uczelnia.pbn_kasuj_dyscypliny_selektywnie MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- src/bpp/admin/helpers/pbn_api/common.py | 7 ++-- src/bpp/admin/uczelnia.py | 2 +- ...zelnia_pbn_kasuj_dyscypliny_selektywnie.py | 32 ++++++++++++++++++ src/bpp/models/uczelnia.py | 14 ++++++-- src/bpp_setup_wizard/forms.py | 4 +-- src/bpp_setup_wizard/tests.py | 2 +- src/pbn_api/exceptions.py | 20 +++++++++++ .../management/commands/pbn_integrator.py | 33 +++++++++++-------- 8 files changed, 91 insertions(+), 23 deletions(-) create mode 100644 src/bpp/migrations/0414_uczelnia_pbn_kasuj_dyscypliny_selektywnie.py diff --git a/src/bpp/admin/helpers/pbn_api/common.py b/src/bpp/admin/helpers/pbn_api/common.py index 0dfa84a16..30ea00d1c 100644 --- a/src/bpp/admin/helpers/pbn_api/common.py +++ b/src/bpp/admin/helpers/pbn_api/common.py @@ -43,7 +43,7 @@ def sprawdz_wysylke_do_pbn_w_parametrach_uczelni(uczelnia): return uczelnia -def sprobuj_wyslac_do_pbn( +def sprobuj_wyslac_do_pbn( # noqa: C901 obj, pbn_client, uczelnia, notificator, force_upload=False, raise_exceptions=False ): # Sprawdź, czy wydawnictwo nadrzędne ma odpowoednik PBN: @@ -156,7 +156,6 @@ def sprobuj_wyslac_do_pbn( obj, notificator=notificator, force_upload=force_upload, - delete_statements_before_upload=uczelnia.pbn_api_kasuj_przed_wysylka, export_pk_zero=not uczelnia.pbn_api_nie_wysylaj_prac_bez_pk, always_affiliate_to_uid=( uczelnia.pbn_uid_id @@ -247,8 +246,8 @@ def sprobuj_wyslac_do_pbn( extra = "" if link_do_wyslanych: extra = ( - 'Kliknij, aby otworzyć widok wysłanych danych.' - % link_do_wyslanych + f'' + f"Kliknij, aby otworzyć widok wysłanych danych." ) notificator.warning( diff --git a/src/bpp/admin/uczelnia.py b/src/bpp/admin/uczelnia.py index e9c0367cc..bc2361b62 100644 --- a/src/bpp/admin/uczelnia.py +++ b/src/bpp/admin/uczelnia.py @@ -106,7 +106,7 @@ class UczelniaAdmin( "pbn_app_name", "pbn_app_token", "pbn_api_user", - "pbn_api_kasuj_przed_wysylka", + "pbn_kasuj_dyscypliny_selektywnie", "pbn_api_nie_wysylaj_prac_bez_pk", "pbn_api_afiliacja_zawsze_na_uczelnie", "pbn_wysylaj_bez_oswiadczen", diff --git a/src/bpp/migrations/0414_uczelnia_pbn_kasuj_dyscypliny_selektywnie.py b/src/bpp/migrations/0414_uczelnia_pbn_kasuj_dyscypliny_selektywnie.py new file mode 100644 index 000000000..4b0e4ddd1 --- /dev/null +++ b/src/bpp/migrations/0414_uczelnia_pbn_kasuj_dyscypliny_selektywnie.py @@ -0,0 +1,32 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bpp", "0413_bppuser_autor_onetoone"), + ] + + operations = [ + migrations.RemoveField( + model_name="uczelnia", + name="pbn_api_kasuj_przed_wysylka", + ), + migrations.AddField( + model_name="uczelnia", + name="pbn_kasuj_dyscypliny_selektywnie", + field=models.BooleanField( + default=True, + help_text=( + "Gdy zaznaczone: ``sync_publication`` usuwa oświadczenia " + "selektywnie per-osoba (DELETE ``/publications/{id}`` z " + "``{personId, role}``) i wysyła tylko brakujące. Gdy " + "odznaczone: usuwa wszystkie oświadczenia publikacji jednym " + "DELETE (``{all: True}``), a następnie wysyła wszystkie " + "lokalne jako batch. Wariant selektywny zachowuje metadata " + "PBN (``addedTimestamp`` itd.) dla identycznych rekordów." + ), + verbose_name="Kasuj oświadczenia selektywnie (per osoba)", + ), + ), + ] diff --git a/src/bpp/models/uczelnia.py b/src/bpp/models/uczelnia.py index 597381140..47c45b2d0 100644 --- a/src/bpp/models/uczelnia.py +++ b/src/bpp/models/uczelnia.py @@ -348,8 +348,18 @@ class Uczelnia(ModelZAdnotacjami, ModelZPBN_ID, NazwaISkrot, NazwaWDopelniaczu): pbn_app_token = models.CharField( "Token aplikacji w PBN", blank=True, default="", max_length=128 ) - pbn_api_kasuj_przed_wysylka = models.BooleanField( - "Kasuj oświadczenia rekordu przed wysłaniem do PBN", default=False + pbn_kasuj_dyscypliny_selektywnie = models.BooleanField( + "Kasuj oświadczenia selektywnie (per osoba)", + default=True, + help_text=( + "Gdy zaznaczone: ``sync_publication`` usuwa oświadczenia " + "selektywnie per-osoba (DELETE ``/publications/{id}`` z " + "``{personId, role}``) i wysyła tylko brakujące. Gdy " + "odznaczone: usuwa wszystkie oświadczenia publikacji jednym " + "DELETE (``{all: True}``), a następnie wysyła wszystkie " + "lokalne jako batch. Wariant selektywny zachowuje metadata " + "PBN (``addedTimestamp`` itd.) dla identycznych rekordów." + ), ) pbn_api_nie_wysylaj_prac_bez_pk = models.BooleanField( "Nie wysyłaj do PBN prac z PK=0", default=False diff --git a/src/bpp_setup_wizard/forms.py b/src/bpp_setup_wizard/forms.py index 8f9a5c190..6a11307f1 100644 --- a/src/bpp_setup_wizard/forms.py +++ b/src/bpp_setup_wizard/forms.py @@ -183,8 +183,8 @@ def save(self, commit=True): uczelnia = super().save(commit=False) # Set the fields that should always be True - uczelnia.pbn_api_kasuj_przed_wysylka = ( - True # Kasuj oświadczenia przed wysłaniem do PBN + uczelnia.pbn_kasuj_dyscypliny_selektywnie = ( + True # Selektywny DELETE oświadczeń per-osoba (nowy flow) ) uczelnia.pbn_api_nie_wysylaj_prac_bez_pk = ( True # Nie wysyłaj do PBN prac z PK=0 diff --git a/src/bpp_setup_wizard/tests.py b/src/bpp_setup_wizard/tests.py index 73362ab00..c979cadc9 100644 --- a/src/bpp_setup_wizard/tests.py +++ b/src/bpp_setup_wizard/tests.py @@ -263,7 +263,7 @@ def test_uczelnia_setup_create_uczelnia(admin_user): assert uczelnia.uzywaj_wydzialow is True # Verify the automatically set fields - assert uczelnia.pbn_api_kasuj_przed_wysylka is True + assert uczelnia.pbn_kasuj_dyscypliny_selektywnie is True assert uczelnia.pbn_api_nie_wysylaj_prac_bez_pk is True assert uczelnia.pbn_api_afiliacja_zawsze_na_uczelnie is True assert uczelnia.pbn_wysylaj_bez_oswiadczen is True diff --git a/src/pbn_api/exceptions.py b/src/pbn_api/exceptions.py index e6a36784a..074f7d39b 100644 --- a/src/pbn_api/exceptions.py +++ b/src/pbn_api/exceptions.py @@ -167,3 +167,23 @@ class BPPAutorPublicationLinkNotFound(Exception): ale autor nie jest powiązany z tą publikacją.""" pass + + +class StatementsResendFailedException(Exception): + """Podnoszony gdy synchronizacja oświadczeń z PBN nie powiodła się + po wyczerpaniu prób retry w ``sync_publication`` (GET/DELETE/POST). + + Publikacja została już wysłana do PBN (POST do endpointu repo OK), + ale kolejne kroki synchronizacji oświadczeń zawiodły. Klasyfikowany + w ``pbn_export_queue`` jako RETRY_LATER + TECHNICZNY. + """ + + def __init__(self, publication_pk, pbn_uid, last_error): + self.publication_pk = publication_pk + self.pbn_uid = pbn_uid + self.last_error = last_error + super().__init__( + f"Synchronizacja oświadczeń dla pracy pk={publication_pk} " + f"(PBN UID={pbn_uid}) nie powiodła się po wyczerpaniu prób: " + f"{last_error}" + ) diff --git a/src/pbn_integrator/management/commands/pbn_integrator.py b/src/pbn_integrator/management/commands/pbn_integrator.py index 156f2fdc3..bd1fbea2c 100644 --- a/src/pbn_integrator/management/commands/pbn_integrator.py +++ b/src/pbn_integrator/management/commands/pbn_integrator.py @@ -9,10 +9,12 @@ django.setup() -from pbn_api.exceptions import IntegracjaWylaczonaException -from pbn_api.management.commands.util import PBNBaseCommand -from pbn_integrator import utils as integrator -from pbn_integrator.utils import ( +# Importy poniżej muszą być po django.setup() — stąd noqa E402. +from bpp.models import Uczelnia # noqa: E402 +from pbn_api.exceptions import IntegracjaWylaczonaException # noqa: E402 +from pbn_api.management.commands.util import PBNBaseCommand # noqa: E402 +from pbn_integrator import utils as integrator # noqa: E402 +from pbn_integrator.utils import ( # noqa: E402 integruj_autorow_z_uczelni, integruj_instytucje, integruj_jezyki, @@ -42,8 +44,6 @@ wyswietl_niezmatchowane_ze_zblizonymi_tytulami, ) -from bpp.models import Uczelnia - def check_end_before(stage, end_before_stage): if end_before_stage == stage: @@ -54,13 +54,15 @@ class Command(PBNBaseCommand): def add_arguments(self, parser): super().add_arguments(parser) - parser.add_argument( - "--disable-multiprocessing", action="store_true", default=False - ), + ( + parser.add_argument( + "--disable-multiprocessing", action="store_true", default=False + ), + ) parser.add_argument("--start-from-stage", type=int, default=0) parser.add_argument("--end-before-stage", type=int, default=None) - parser.add_argument("--just-one-stage", action="store_true"), + (parser.add_argument("--just-one-stage", action="store_true"),) parser.add_argument("--clear-all", action="store_true", default=False) parser.add_argument("--clear-publications", action="store_true", default=False) @@ -148,7 +150,7 @@ def add_arguments(self, parser): "--disable-progress-bar", action="store_true", default=False ) - def handle( + def handle( # noqa: C901 self, app_id, app_token, @@ -193,7 +195,7 @@ def handle( delete_statements_before_upload, export_pk_zero, *args, - **options + **options, ): if disable_multiprocessing: integrator.CPU_COUNT = "single" @@ -417,7 +419,12 @@ def handle( export_pk_zero = not uczelnia.pbn_api_nie_wysylaj_prac_bez_pk if delete_statements_before_upload is None: - delete_statements_before_upload = uczelnia.pbn_api_kasuj_przed_wysylka + # Flaga deprecated — nowy sync_publication ignoruje ten argument + # (kontrolę DELETE+POST statements przejęła Uczelnia. + # pbn_kasuj_dyscypliny_selektywnie i algorytm diff). Zostawiamy + # None → False żeby backward compat z CLI ``--delete-statements- + # before-upload``. + delete_statements_before_upload = False synchronizuj_publikacje( client=client, From 3092d1bb1ba110d6c08800b83ded6f86a271e40d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 21:42:10 +0200 Subject: [PATCH 10/33] refactor(pbn): upload_publication zawsze przez endpoint repo + dead code removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- src/pbn_api/client/publication_sync.py | 150 +++++++++---------------- 1 file changed, 55 insertions(+), 95 deletions(-) diff --git a/src/pbn_api/client/publication_sync.py b/src/pbn_api/client/publication_sync.py index d5d4583d7..7db0d1f7e 100644 --- a/src/pbn_api/client/publication_sync.py +++ b/src/pbn_api/client/publication_sync.py @@ -17,7 +17,6 @@ from pbn_api.const import ( PBN_POST_PUBLICATION_FEE_URL, PBN_POST_PUBLICATION_NO_STATEMENTS_URL, - PBN_POST_PUBLICATIONS_URL, ) from pbn_api.exceptions import ( CannotDeleteStatementsException, @@ -42,9 +41,6 @@ class PublicationSyncMixin: """Mixin providing publication synchronization methods.""" - def post_publication(self, json): - return self.transport.post(PBN_POST_PUBLICATIONS_URL, body=json) - def convert_js_with_statements_to_no_statements(self, json): # PBN zmienił givenNames na firstName for elem in json.get("authors", []): @@ -151,18 +147,22 @@ def get_publication_fees_batch(self, publication_ids): return fees_map def _prepare_publication_json(self, rec, export_pk_zero, always_affiliate_to_uid): - """Prepare publication JSON data.""" + """Przygotowuje JSON publikacji do wysyłki przez endpoint repozytoryjny. + + Zawsze wywołuje ``convert_js_with_statements_to_no_statements``, bo + ``upload_publication`` używa teraz wyłącznie endpointu repo + ``/api/v1/repositorium/publications``, który nie przyjmuje klucza + ``statements`` w payload i wymaga konwersji formatu (``givenNames`` + → ``firstName``, ``abstracts`` w root, brak ``fee`` itp.). + """ js = WydawnictwoPBNAdapter( rec, export_pk_zero=export_pk_zero, always_affiliate_to_uid=always_affiliate_to_uid, ).pbn_get_json() - bez_oswiadczen = "statements" not in js - if bez_oswiadczen: - js = self.convert_js_with_statements_to_no_statements(js) - - return js, bez_oswiadczen + js = self.convert_js_with_statements_to_no_statements(js) + return js def _check_upload_needed(self, rec, js, force_upload): """Check if upload is needed.""" @@ -173,108 +173,61 @@ def _check_upload_needed(self, rec, js, force_upload): SentData.objects.get_for_rec(rec).last_updated_on ) - def _post_publication_data(self, js, bez_oswiadczen): - """Post publication data and extract objectId.""" - if not bez_oswiadczen: - ret = self.post_publication(js) - objectId = ret.get("objectId", None) - else: - ret = self.post_publication_no_statements(js) - if len(ret) != 1: - raise Exception( - "Lista zwróconych obiektów przy wysyłce pracy bez oświadczeń " - "różna od jednego. " - "Sytuacja nieobsługiwana, proszę o kontakt z autorem programu. " - ) - try: - objectId = ret[0].get("id", None) - except KeyError as e: - raise Exception( - f"Serwer zwrócił nieoczekiwaną odpowiedź. {ret=}" - ) from e + def _post_publication_data(self, js): + """POST publikacji do endpointu repo i wyciągnięcie ``objectId``.""" + ret = self.post_publication_no_statements(js) + if len(ret) != 1: + raise Exception( + "Lista zwróconych obiektów przy wysyłce pracy do repozytorium " + "różna od jednego. " + "Sytuacja nieobsługiwana, proszę o kontakt z autorem programu. " + ) + try: + objectId = ret[0].get("id", None) + except (KeyError, IndexError) as e: + raise Exception(f"Serwer zwrócił nieoczekiwaną odpowiedź. {ret=}") from e return ret, objectId - def _should_retry_validation_error(self, e): - """Check if HTTP exception is a retryable validation error.""" - return ( - e.status_code == 400 - and e.url == "/api/v1/publications" - and "Bad Request" in e.content - and "Validation failed." in e.content - ) - - def _retry_download_publication(self, objectId): - """Attempt to download publication data after validation error.""" - try: - publication = self.download_publication(objectId=objectId) - self.download_statements_of_publication(publication) - self.pobierz_publikacje_instytucji_v2(objectId=objectId) - except Exception: - rollbar.report_exc_info(sys.exc_info()) - logger.debug( - "Błąd podczas ponownego pobierania publikacji %s", - objectId, - exc_info=True, - ) - def upload_publication( self, rec, force_upload=False, export_pk_zero=None, always_affiliate_to_uid=None, - max_retries_on_validation_error=3, + max_retries_on_validation_error=3, # DEPRECATED: nieużywany, backward compat ): """ - Ta funkcja wysyła dane publikacji na serwer, w zależności od obecności oświadczeń - w JSONie (klucz: "statements") używa albo api /v1/ do wysyłki publikacji - "ze wszystkim", albo korzysta z api /v1/ repozytorialnego. - - Zwracane wyniki wyjściowe też różnią się w zależnosci od użytego API stąd też - ta funkcja stara się w miarę rozsądnie to ogarnąć. + Wysyła publikację do PBN przez endpoint repozytoryjny + ``POST /api/v1/repositorium/publications`` (zawsze, niezależnie od + obecności oświadczeń w JSON). Oświadczenia synchronizuje osobno + ``sync_publication`` przez endpoint ``/api/v2/institution-profile/statements``. + + Zwraca ``(objectId, ret, js, True)`` — ostatnie pole (``bez_oswiadczen``) + zachowane dla backward compat; zawsze ``True`` bo endpoint repo nigdy + nie wysyła oświadczeń w body publikacji. """ - js, bez_oswiadczen = self._prepare_publication_json( + js = self._prepare_publication_json( rec, export_pk_zero, always_affiliate_to_uid ) self._check_upload_needed(rec, js, force_upload) # Create or update SentData record BEFORE API call - sent_data = SentData.objects.create_or_update_before_upload(rec, js) # noqa - - retry_count = max_retries_on_validation_error - ret = None - objectId = None - - while True: - try: - ret, objectId = self._post_publication_data(js, bez_oswiadczen) - SentData.objects.mark_as_successful(rec, api_response_status=str(ret)) - break - - except HttpException as e: - if self._should_retry_validation_error(e): - retry_count -= 1 - if retry_count <= 0: - SentData.objects.mark_as_failed( - rec, exception=str(e), api_response_status=e.content - ) - raise e - - time.sleep(0.5) - self._retry_download_publication(objectId) - continue - - SentData.objects.mark_as_failed( - rec, exception=str(e), api_response_status=e.content - ) - raise e + SentData.objects.create_or_update_before_upload(rec, js) - except Exception as e: - SentData.objects.mark_as_failed(rec, exception=str(e)) - raise e + try: + ret, objectId = self._post_publication_data(js) + SentData.objects.mark_as_successful(rec, api_response_status=str(ret)) + except HttpException as e: + SentData.objects.mark_as_failed( + rec, exception=str(e), api_response_status=e.content + ) + raise + except Exception as e: + SentData.objects.mark_as_failed(rec, exception=str(e)) + raise - return objectId, ret, js, bez_oswiadczen + return objectId, ret, js, True def download_publication(self, doi=None, objectId=None): from pbn_api.models import Publication @@ -317,14 +270,21 @@ def pobierz_publikacje_instytucji_v2(self, objectId): return zapisz_publikacje_instytucji_v2(self, elem[0]) def _delete_statements_with_retry(self, pbn_uid_id, max_tries=5): - """Delete publication statements with retry on failure.""" + """Delete publication statements with retry on failure. + + Używane przez batch flow (``pbn_wysylka_oswiadczen/tasks.py``) oraz + przez nowy ``_delete_statements_batch`` helper w ``sync_publication``. + """ no_tries = max_tries while True: try: self.delete_all_publication_statements(pbn_uid_id) return True except CannotDeleteStatementsException as e: - if no_tries < 0: + # Warunek <= 0 (nie < 0): dla ``max_tries=5`` chcemy dokładnie + # 5 prób (no_tries: 5→4→3→2→1→0), po szóstej iteracji rzucamy. + # Wcześniejsze ``< 0`` pozwalało na 6 prób. + if no_tries <= 0: raise e no_tries -= 1 time.sleep(0.5) From 0c52b99825a9ff1b7a0a692449fe4e5ec609fab7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 21:44:36 +0200 Subject: [PATCH 11/33] =?UTF-8?q?feat(pbn):=20helpery=20split-flow=20synch?= =?UTF-8?q?ronizacji=20o=C5=9Bwiadcze=C5=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/pbn_api/client/publication_sync.py | 259 +++++++++++++++++++++++++ 1 file changed, 259 insertions(+) diff --git a/src/pbn_api/client/publication_sync.py b/src/pbn_api/client/publication_sync.py index 7db0d1f7e..efd97e486 100644 --- a/src/pbn_api/client/publication_sync.py +++ b/src/pbn_api/client/publication_sync.py @@ -29,6 +29,7 @@ PublicationDoesNotExistInInstitutionProfile, PublikacjaInstytucjiV2NieZnalezionaException, SameDataUploadedRecently, + StatementsResendFailedException, ZnalezionoWielePublikacjiInstytucjiV2Exception, ) from pbn_api.models.pbn_odpowiedzi_niepozadane import PBNOdpowiedziNiepozadane @@ -289,6 +290,264 @@ def _delete_statements_with_retry(self, pbn_uid_id, max_tries=5): no_tries -= 1 time.sleep(0.5) + # ----------- Helpery do nowego split-flow sync_publication ----------- + # Mapowanie kluczy porównania: + # - PBN GET /page/statements zwraca: {personId, area, type, institutionId, ...} + # - Adapter pbn_get_json_statements() zwraca: {personObjectId, disciplineId, + # disciplineUuid, type, ...} + # Klucz porównania: (person mongoId, discipline numerek). Oba na string. + # Selektywny DELETE używa (personId, role) — delete_publication_statement. + + _STATEMENT_RETRY_DELAYS = (2, 4, 8) # exponential backoff przy 3 próbach + + @staticmethod + def _statement_key_pbn(stmt): + """Klucz porównania dla oświadczenia z PBN GET response.""" + return ( + str(stmt.get("personId", "")), + str(stmt.get("area", "")), + ) + + @staticmethod + def _statement_key_intended(stmt): + """Klucz porównania dla oświadczenia z ``pbn_get_json_statements``.""" + return ( + str(stmt.get("personObjectId", "")), + str(stmt.get("disciplineId", "")), + ) + + def _diff_statements(self, pbn_statements, intended_statements): + """Porównuje zestaw oświadczeń PBN z intencją BPP. + + Zwraca (only_in_pbn, only_in_intended) jako sety kluczy + ``(person_mongoId, discipline_numerek)``: + + - ``only_in_pbn`` — do usunięcia z PBN (PBN ma, BPP nie chce) + - ``only_in_intended`` — do dodania do PBN (BPP chce, PBN nie ma) + """ + pbn_keys = {self._statement_key_pbn(s) for s in pbn_statements} + intended_keys = {self._statement_key_intended(s) for s in intended_statements} + return pbn_keys - intended_keys, intended_keys - pbn_keys + + def _report_statements_failure_and_raise( + self, publication_pk, objectId, last_error + ): + """Raportuje do Rollbar (level=warning) i rzuca StatementsResendFailedException.""" + try: + raise StatementsResendFailedException(publication_pk, objectId, last_error) + except StatementsResendFailedException: + rollbar.report_exc_info( + sys.exc_info(), + level="warning", + extra_data={ + "publication_pk": publication_pk, + "pbn_uid": str(objectId), + "last_error": str(last_error), + }, + ) + raise + + def _get_pbn_statements_with_retry(self, objectId, publication_pk, max_tries=3): + """Pobiera oświadczenia publikacji z PBN z retry (exponential backoff). + + Po wyczerpaniu prób: rollbar.report_exc_info(level="warning") oraz + raise ``StatementsResendFailedException``. + """ + last_error = None + for attempt in range(max_tries): + try: + return list( + self.get_institution_statements_of_single_publication( + str(objectId), 5120 + ) + ) + except Exception as e: + last_error = e + logger.warning( + "Błąd pobierania oświadczeń PBN dla %s, próba %d/%d: %s", + objectId, + attempt + 1, + max_tries, + e, + exc_info=True, + ) + if attempt < max_tries - 1: + time.sleep(self._STATEMENT_RETRY_DELAYS[attempt]) + + self._report_statements_failure_and_raise(publication_pk, objectId, last_error) + + def _delete_statements_selective( + self, objectId, pbn_statements_to_delete, publication_pk, max_tries=3 + ): + """Selektywne DELETE oświadczeń per-osoba (delete_publication_statement). + + Iteruje po liście oświadczeń PBN do usunięcia i wywołuje DELETE dla + każdego (klucz: personId + type z PBN GET response). Po wyczerpaniu + prób per oświadczenie: rollbar + raise StatementsResendFailedException. + """ + for stmt in pbn_statements_to_delete: + person_id = stmt.get("personId") + role = stmt.get("type") + last_error = None + success = False + for attempt in range(max_tries): + try: + self.delete_publication_statement(str(objectId), person_id, role) + success = True + break + except Exception as e: + last_error = e + logger.warning( + "Błąd DELETE oświadczenia (%s, %s) dla %s, próba %d/%d: %s", + person_id, + role, + objectId, + attempt + 1, + max_tries, + e, + exc_info=True, + ) + if attempt < max_tries - 1: + time.sleep(self._STATEMENT_RETRY_DELAYS[attempt]) + if not success: + self._report_statements_failure_and_raise( + publication_pk, objectId, last_error + ) + + def _delete_statements_batch(self, objectId, publication_pk, max_tries=3): + """Batch DELETE wszystkich oświadczeń publikacji z retry. + + Rzuca ``CannotDeleteStatementsException`` w górę (caller może + zignorować gdy PBN mówi że nie ma oświadczeń). Po wyczerpaniu prób + dla innych błędów: rollbar + raise StatementsResendFailedException. + """ + last_error = None + for attempt in range(max_tries): + try: + self.delete_all_publication_statements(str(objectId)) + return + except CannotDeleteStatementsException: + raise + except Exception as e: + last_error = e + logger.warning( + "Błąd batch DELETE oświadczeń dla %s, próba %d/%d: %s", + objectId, + attempt + 1, + max_tries, + e, + exc_info=True, + ) + if attempt < max_tries - 1: + time.sleep(self._STATEMENT_RETRY_DELAYS[attempt]) + + self._report_statements_failure_and_raise(publication_pk, objectId, last_error) + + def _post_statements_with_retry(self, rec, objectId, publication_pk, max_tries=3): + """POST oświadczeń publikacji do ``/api/v2/institution-profile/statements``. + + Wymaga lokalnego ``PublikacjaInstytucji_V2`` (``pbn_get_api_statements`` + rzuca ``DaneLokalneWymagajaAktualizacjiException`` gdy brak) — + sync_publication wywołuje ``pobierz_publikacje_instytucji_v2`` przed + tym helperem, więc V2 powinno istnieć. + + Retry z exponential backoff. Po wyczerpaniu: rollbar + raise + ``StatementsResendFailedException``. + """ + # Może rzucić DaneLokalneWymagajaAktualizacjiException — propaguje + # do callera (sync_publication), który loguje warning zamiast crash. + payload = WydawnictwoPBNAdapter(rec).pbn_get_api_statements() + body = {"data": [payload]} + + last_error = None + for attempt in range(max_tries): + try: + self.post_discipline_statements(body) + return + except Exception as e: + last_error = e + logger.warning( + "Błąd POST oświadczeń dla %s, próba %d/%d: %s", + objectId, + attempt + 1, + max_tries, + e, + exc_info=True, + ) + if attempt < max_tries - 1: + time.sleep(self._STATEMENT_RETRY_DELAYS[attempt]) + + self._report_statements_failure_and_raise(publication_pk, objectId, last_error) + + def _sync_statements_with_pbn( + self, rec, objectId, kasuj_selektywnie, notificator=None + ): + """Synchronizuje oświadczenia publikacji z PBN po wysyłce publikacji. + + Algorytm: + 1. GET aktualnych oświadczeń z PBN + 2. Intencja BPP z ``WydawnictwoPBNAdapter.pbn_get_json_statements()`` + 3. Diff (klucz: person mongoId + numerek dyscypliny) + 4a. PBN ma + BPP nie chce → DELETE (selektywnie lub batch) + 4b. PBN nie ma + BPP chce → POST /v2/statements + 4c. Różnice (oba) → DELETE brakujących + POST dodatkowych + 4d. Identyczne → nic + + Args: + rec: rekord BPP (Wydawnictwo_Ciagle/Wydawnictwo_Zwarte) + objectId: PBN UID publikacji + kasuj_selektywnie: True=per-osoba DELETE, False=batch delete_all + notificator: opcjonalny logger UI + + Raises: + StatementsResendFailedException: gdy retry operacji się wyczerpie + DaneLokalneWymagajaAktualizacjiException: gdy POST potrzebuje + V2 którego nie ma lokalnie (propagowana — caller loguje + warning zamiast crashować) + """ + publication_pk = rec.pk + + pbn_statements = self._get_pbn_statements_with_retry(objectId, publication_pk) + intended = WydawnictwoPBNAdapter(rec).pbn_get_json_statements() + + only_in_pbn, only_in_intended = self._diff_statements(pbn_statements, intended) + + if not only_in_pbn and not only_in_intended: + if notificator is not None: + notificator.info( + "Oświadczenia w PBN identyczne z intencją BPP — bez zmian." + ) + return + + if only_in_pbn: + if kasuj_selektywnie: + # Zbuduj listę oświadczeń do usunięcia z PBN (pełne dict-y + # zachowują personId + type, potrzebne dla delete_publication_statement). + stmts_to_delete = [ + s + for s in pbn_statements + if self._statement_key_pbn(s) in only_in_pbn + ] + self._delete_statements_selective( + objectId, stmts_to_delete, publication_pk + ) + else: + try: + self._delete_statements_batch(objectId, publication_pk) + except CannotDeleteStatementsException: + # PBN mówi że nie ma oświadczeń — akceptowalne, kontynuuj + pass + + if only_in_intended: + self._post_statements_with_retry(rec, objectId, publication_pk) + + if notificator is not None: + notificator.info( + f"Zsynchronizowano oświadczenia: " + f"skasowano z PBN {len(only_in_pbn)}, " + f"dodano do PBN {len(only_in_intended)}." + ) + def _handle_no_objectid(self, notificator, ret, js, pub): """Handle case when server doesn't return object ID.""" msg = ( From 200cb6e4e171452d7131ebc080046252a4026fd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 21:46:13 +0200 Subject: [PATCH 12/33] feat(pbn): sync_publication nowy split flow (POST repo + osobna synchronizacja statements) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/pbn_api/client/publication_sync.py | 80 +++++++++++++++++--------- 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/src/pbn_api/client/publication_sync.py b/src/pbn_api/client/publication_sync.py index efd97e486..17a2b8f5d 100644 --- a/src/pbn_api/client/publication_sync.py +++ b/src/pbn_api/client/publication_sync.py @@ -21,6 +21,7 @@ from pbn_api.exceptions import ( CannotDeleteStatementsException, CannotUploadPublicationFee, + DaneLokalneWymagajaAktualizacjiException, HttpException, NoFeeDataException, NoPBNUIDException, @@ -688,59 +689,60 @@ def sync_publication( # noqa: C901 pub, notificator=None, force_upload=False, - delete_statements_before_upload=False, + delete_statements_before_upload=False, # DEPRECATED: ignorowany export_pk_zero=None, always_affiliate_to_uid=None, ): """ - @param delete_statements_before_upload: gdy True, kasuj oświadczenia publikacji - przed wysłaniem (jeżeli posiada PBN UID) + Synchronizuje publikację BPP z PBN w dwóch niezależnych krokach: + + 1. POST publikacji do endpointu repozytoryjnego + ``/api/v1/repositorium/publications`` (bez oświadczeń w body). + 2. Synchronizacja oświadczeń (po sukcesie kroku 1): GET aktualnego + stanu w PBN, porównanie z intencją BPP, selektywne DELETE/POST. + + Gdy POST publikacji zawiedzie — oświadczenia w PBN pozostają + nietknięte (ważna gwarancja bezpieczeństwa). Gdy POST publikacji + OK ale synchronizacja oświadczeń nie — rzuca + ``StatementsResendFailedException`` (klasyfikowane w + ``pbn_export_queue`` jako RETRY_LATER). + + :param delete_statements_before_upload: DEPRECATED, ignorowany — + zachowany w sygnaturze dla backward compat. Nowa logika zawsze + synchronizuje oświadczenia po wysyłce publikacji (split flow), + a tryb kasowania sterowany jest przez + ``Uczelnia.pbn_kasuj_dyscypliny_selektywnie``. """ - pub = self.eventually_coerce_to_publication(pub) + from bpp.models import Uczelnia - if ( - delete_statements_before_upload - and hasattr(pub, "pbn_uid_id") - and pub.pbn_uid_id is not None - ): - try: - self._delete_statements_with_retry(pub.pbn_uid_id) - force_upload = True - except CannotDeleteStatementsException: - pass + pub = self.eventually_coerce_to_publication(pub) - objectId, ret, js, bez_oswiadczen = self.upload_publication( + # KROK 1: POST publikacji do endpointu repo (zawsze) + objectId, ret, js, _bez_oswiadczen = self.upload_publication( pub, force_upload=force_upload, export_pk_zero=export_pk_zero, always_affiliate_to_uid=always_affiliate_to_uid, ) - if bez_oswiadczen and notificator is not None: - notificator.info( - "Rekord nie posiada oświadczeń - wysłano wyłącznie do repozytorium PBN." - ) - if not objectId: self._handle_no_objectid(notificator, ret, js, pub) return + # KROK 2: pobierz lokalnie Publication (obiekt mongodb) publication = self.download_publication(objectId=objectId) - # Update SentData with the publication link now that it exists in the database + # Update SentData with the publication link now that it exists try: sent_data = SentData.objects.get_for_rec(pub) if sent_data.pbn_uid_id is None: sent_data.pbn_uid_id = publication.pk sent_data.save() except SentData.DoesNotExist: - # This shouldn't happen if upload_publication was called, - # but handle gracefully + # This shouldn't happen if upload_publication was called pass - if not bez_oswiadczen: - self._download_statements_with_retry(publication, objectId, notificator) - + # KROK 3: obsługa zmiany/konfliktu PBN UID if pub.pbn_uid_id != objectId: if pub.pbn_uid_id is not None: self._handle_uid_change(pub, objectId, notificator, js, ret) @@ -755,6 +757,32 @@ def sync_publication( # noqa: C901 pub.pbn_uid = publication pub.save() + # KROK 4: pobierz lokalnie PublikacjaInstytucji_V2 (potrzebne do + # ``pbn_get_api_statements``, które wymaga UUID publikacji z V2). + # ``_download_statements_with_retry`` pobiera też V2 jako efekt + # uboczny (patrz pobierz_publikacje_instytucji_v2 w helperze). + self._download_statements_with_retry(publication, objectId, notificator) + + # KROK 5: synchronizacja oświadczeń (split flow, po wysyłce publikacji) + uczelnia = Uczelnia.objects.get_default() + kasuj_selektywnie = ( + uczelnia.pbn_kasuj_dyscypliny_selektywnie if uczelnia else True + ) + try: + self._sync_statements_with_pbn( + pub, objectId, kasuj_selektywnie, notificator + ) + except DaneLokalneWymagajaAktualizacjiException as e: + # Brak lokalnego PublikacjaInstytucji_V2 — nie da się wysłać + # oświadczeń. Logujemy warning i kontynuujemy (publikacja + # została już wysłana w KROK 1, to nie jest fatal error). + logger.warning("Brak V2 dla %s — pomijam sync oświadczeń: %s", objectId, e) + if notificator is not None: + notificator.warning( + f"Nie mogę zsynchronizować oświadczeń (brak lokalnych " + f"danych V2): {e}" + ) + return publication def eventually_coerce_to_publication(self, pub: Model | str) -> Model: From 678e1c440ae25d38ff3f603b03c9f0c60f30c177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 21:48:28 +0200 Subject: [PATCH 13/33] =?UTF-8?q?refactor(pbn=5Fintegrator):=20usu=C5=84?= =?UTF-8?q?=20delete=5Fstatements=5Fbefore=5Fupload?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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ć). --- .../management/commands/pbn_integrator.py | 13 ------------- src/pbn_integrator/utils/synchronization.py | 8 -------- 2 files changed, 21 deletions(-) diff --git a/src/pbn_integrator/management/commands/pbn_integrator.py b/src/pbn_integrator/management/commands/pbn_integrator.py index bd1fbea2c..7643273c7 100644 --- a/src/pbn_integrator/management/commands/pbn_integrator.py +++ b/src/pbn_integrator/management/commands/pbn_integrator.py @@ -138,9 +138,6 @@ def add_arguments(self, parser): parser.add_argument("--force-upload", action="store_true", default=False) parser.add_argument("--only-bad", action="store_true", default=False) parser.add_argument("--only-new", action="store_true", default=False) - parser.add_argument( - "--delete-statements-before-upload", action="store_true", default=None - ) parser.add_argument( "--export-pk-zero", action="store_true", @@ -192,7 +189,6 @@ def handle( # noqa: C901 only_bad, only_new, disable_progress_bar, - delete_statements_before_upload, export_pk_zero, *args, **options, @@ -418,19 +414,10 @@ def handle( # noqa: C901 if export_pk_zero is None: export_pk_zero = not uczelnia.pbn_api_nie_wysylaj_prac_bez_pk - if delete_statements_before_upload is None: - # Flaga deprecated — nowy sync_publication ignoruje ten argument - # (kontrolę DELETE+POST statements przejęła Uczelnia. - # pbn_kasuj_dyscypliny_selektywnie i algorytm diff). Zostawiamy - # None → False żeby backward compat z CLI ``--delete-statements- - # before-upload``. - delete_statements_before_upload = False - synchronizuj_publikacje( client=client, force_upload=force_upload, only_bad=only_bad, only_new=only_new, - delete_statements_before_upload=delete_statements_before_upload, export_pk_zero=export_pk_zero, ) diff --git a/src/pbn_integrator/utils/synchronization.py b/src/pbn_integrator/utils/synchronization.py index 4dcb2780c..fa41eaf09 100644 --- a/src/pbn_integrator/utils/synchronization.py +++ b/src/pbn_integrator/utils/synchronization.py @@ -78,7 +78,6 @@ def _synchronizuj_pojedyncza_publikacje( # noqa: C901 rec, force_upload=False, export_pk_zero=None, - delete_statements_before_upload=False, ): """Synchronize a single publication to PBN. @@ -87,14 +86,12 @@ def _synchronizuj_pojedyncza_publikacje( # noqa: C901 rec: BPP record. force_upload: Whether to force upload. export_pk_zero: Export pk zero setting. - delete_statements_before_upload: Whether to delete statements before upload. """ try: client.sync_publication( rec, force_upload=force_upload, export_pk_zero=export_pk_zero, - delete_statements_before_upload=delete_statements_before_upload, ) except SameDataUploadedRecently: pass @@ -182,7 +179,6 @@ def synchronizuj_publikacje( only_new=False, skip=0, export_pk_zero=None, - delete_statements_before_upload=False, ): """Synchronize publications to PBN. @@ -196,7 +192,6 @@ def synchronizuj_publikacje( only_new: Export only records that don't have an entry in SentData. skip: Number of records to skip. export_pk_zero: Export pk zero setting. - delete_statements_before_upload: Whether to delete statements before upload. """ assert not (only_bad and only_new), "Te parametry wykluczają się wzajemnie" # @@ -225,7 +220,6 @@ def synchronizuj_publikacje( client, rec, force_upload=force_upload, - delete_statements_before_upload=delete_statements_before_upload, export_pk_zero=export_pk_zero, ) @@ -237,7 +231,6 @@ def synchronizuj_publikacje( client, rec, force_upload=force_upload, - delete_statements_before_upload=delete_statements_before_upload, export_pk_zero=export_pk_zero, ) @@ -267,7 +260,6 @@ def synchronizuj_publikacje( client, rec, force_upload=force_upload, - delete_statements_before_upload=delete_statements_before_upload, export_pk_zero=export_pk_zero, ) From 02d13dc4d310ad702c9976f0e6427601c95bf3c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 22:21:14 +0200 Subject: [PATCH 14/33] test(pbn): zaktualizuj testy sync_publication dla nowego split-flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/pbn_api/client/publication_sync.py | 18 +- src/pbn_api/tests/test_bpp_admin_helpers.py | 61 +- src/pbn_api/tests/test_client_helpers.py | 80 ++- src/pbn_api/tests/test_client_sync.py | 747 ++++++++++++++------ src/pbn_api/tests/test_client_upload.py | 56 +- 5 files changed, 682 insertions(+), 280 deletions(-) diff --git a/src/pbn_api/client/publication_sync.py b/src/pbn_api/client/publication_sync.py index 17a2b8f5d..9a79ae3da 100644 --- a/src/pbn_api/client/publication_sync.py +++ b/src/pbn_api/client/publication_sync.py @@ -757,11 +757,19 @@ def sync_publication( # noqa: C901 pub.pbn_uid = publication pub.save() - # KROK 4: pobierz lokalnie PublikacjaInstytucji_V2 (potrzebne do - # ``pbn_get_api_statements``, które wymaga UUID publikacji z V2). - # ``_download_statements_with_retry`` pobiera też V2 jako efekt - # uboczny (patrz pobierz_publikacje_instytucji_v2 w helperze). - self._download_statements_with_retry(publication, objectId, notificator) + # KROK 4: pobierz lokalnie PublikacjaInstytucji_V2 (wymagane przez + # ``pbn_get_api_statements`` w ``_post_statements_with_retry``). + # Przy okazji odświeża lokalny cache ``OswiadczenieInstytucji`` + # (best effort — błąd tu nie blokuje synchronizacji oświadczeń, + # bo ``_sync_statements_with_pbn`` zrobi własne GET dla porównania). + try: + self._download_statements_with_retry(publication, objectId, notificator) + except Exception as e: + logger.warning( + "Odświeżenie lokalnego cache oświadczeń dla %s nie powiodło się: %s", + objectId, + e, + ) # KROK 5: synchronizacja oświadczeń (split flow, po wysyłce publikacji) uczelnia = Uczelnia.objects.get_default() diff --git a/src/pbn_api/tests/test_bpp_admin_helpers.py b/src/pbn_api/tests/test_bpp_admin_helpers.py index f52b7a4a8..2d9902232 100644 --- a/src/pbn_api/tests/test_bpp_admin_helpers.py +++ b/src/pbn_api/tests/test_bpp_admin_helpers.py @@ -10,10 +10,10 @@ from pbn_api.client import ( PBN_GET_INSTITUTION_STATEMENTS, PBN_GET_PUBLICATION_BY_ID_URL, - PBN_POST_PUBLICATIONS_URL, ) from pbn_api.const import ( PBN_GET_INSTITUTION_PUBLICATIONS_V2, + PBN_POST_INSTITUTION_STATEMENTS_URL, PBN_POST_PUBLICATION_NO_STATEMENTS_URL, ) from pbn_api.exceptions import AccessDeniedException @@ -147,7 +147,9 @@ def test_sprobuj_wyslac_do_pbn_inny_exception( ): req = rf.get("/") - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = ZeroDivisionError + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = ( + ZeroDivisionError + ) with middleware(req): sprobuj_wyslac_do_pbn_gui( @@ -164,7 +166,9 @@ def test_sprobuj_wyslac_do_pbn_inny_blad( ): req = rf.get("/") - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = Exception("test") + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = ( + Exception("test") + ) with middleware(req): sprobuj_wyslac_do_pbn_gui( @@ -181,7 +185,9 @@ def test_sprobuj_wyslac_do_pbn_z_oswiadczeniami( ): req = rf.get("/") - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = {"objectId": "123"} + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": "123"} + ] pbn_client.transport.return_values[ PBN_GET_PUBLICATION_BY_ID_URL.format(id="123") ] = MOCK_RETURNED_MONGODB_DATA @@ -194,37 +200,58 @@ def test_sprobuj_wyslac_do_pbn_z_oswiadczeniami( pbn_client.transport.return_values[ PBN_GET_INSTITUTION_PUBLICATIONS_V2 + "?publicationId=123&size=10" ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) + # Publikacja ma autorów z dyscyplinami → intencja BPP != puste PBN → + # sync_statements wykona POST /v2/statements (i potrzebuje mocka). + pbn_client.transport.return_values[PBN_POST_INSTITUTION_STATEMENTS_URL] = { + "data": [] + } with middleware(req): sprobuj_wyslac_do_pbn_gui( req, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, pbn_client=pbn_client ) - msg = get_messages(req) - assert "zostały zaktualizowane" in list(msg)[0].message + msg = list(get_messages(req)) + # Może być kilka wiadomości (info o sync oświadczeń + success końcowy). + assert any("zostały zaktualizowane" in m.message for m in msg) @pytest.mark.django_db def test_sprobuj_wyslac_do_pbn_bez_oswiadczen_sukces( pbn_wydawnictwo_zwarte_z_charakterem, pbn_client, rf, pbn_uczelnia ): + """Uczelnia z pbn_wysylaj_bez_oswiadczen=True pozwala na wysyłkę prac bez dyscyplin.""" req = rf.get("/") + pbn_uczelnia.pbn_wysylaj_bez_oswiadczen = True + pbn_uczelnia.save() + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ {"id": "123"} ] pbn_client.transport.return_values[ PBN_GET_PUBLICATION_BY_ID_URL.format(id="123") ] = MOCK_RETURNED_MONGODB_DATA + pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=456)] = ( + MOCK_RETURNED_MONGODB_DATA + ) + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_PUBLICATIONS_V2 + "?publicationId=123&size=10" + ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_STATEMENTS + "?publicationId=123&size=5120" + ] = pbn_pageable_json([]) with middleware(req): sprobuj_wyslac_do_pbn_gui( req, pbn_wydawnictwo_zwarte_z_charakterem, pbn_client=pbn_client ) - msg = get_messages(req) - assert "nie posiada oświadczeń" in list(msg)[0].message - assert "zostały zaktualizowane" in list(msg)[1].message + msg = list(get_messages(req)) + # Po refaktoryzacji: sync_publication nie rozróżnia "bez oświadczeń" vs + # "z oświadczeniami" (zawsze repo endpoint). Wiadomość o sukcesie + # zawsze pojawia się po udanej wysyłce. + assert any("zaktualizowane" in m.message for m in msg) @pytest.mark.django_db @@ -247,7 +274,9 @@ def test_sprobuj_wyslac_do_pbn_ostrzezenie_brak_dyscypliny_autora( autor.pbn_uid_id = None autor.save() - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = {"objectId": "123"} + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": "123"} + ] pbn_client.transport.return_values[ PBN_GET_PUBLICATION_BY_ID_URL.format(id="123") ] = MOCK_RETURNED_MONGODB_DATA @@ -287,9 +316,9 @@ def test_sprobuj_wyslac_do_pbn_przychodzi_istniejacy_pbn_uid_dla_nowego_rekordu( pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save(update_fields=["pbn_uid"]) # To jest odpowiedź z PBNu gdzie zwrotnie przyjdzie objectId = MOCK_MONGO_ID - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": MOCK_MONGO_ID - } + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": MOCK_MONGO_ID} + ] pbn_client.transport.return_values[ PBN_GET_PUBLICATION_BY_ID_URL.format(id=MOCK_MONGO_ID) ] = MOCK_RETURNED_MONGODB_DATA @@ -339,9 +368,9 @@ def test_sprobuj_wyslac_do_pbn_przychodzi_inny_pbn_uid_dla_starego_rekordu( pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save(update_fields=["pbn_uid"]) # To jest odpowiedź z PBNu gdzie zwrotnie przyjdzie objectId = MOCK_MONGO_ID*2 - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": MOCK_MONGO_ID * 2 - } + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": MOCK_MONGO_ID * 2} + ] pbn_client.transport.return_values[ PBN_GET_PUBLICATION_BY_ID_URL.format(id=MOCK_MONGO_ID * 2) ] = MOCK_RETURNED_MONGODB_DATA diff --git a/src/pbn_api/tests/test_client_helpers.py b/src/pbn_api/tests/test_client_helpers.py index 0e40a4e81..995ae73f1 100644 --- a/src/pbn_api/tests/test_client_helpers.py +++ b/src/pbn_api/tests/test_client_helpers.py @@ -13,12 +13,16 @@ from bpp.admin.helpers.pbn_api.gui import sprobuj_wyslac_do_pbn_gui from fixtures import MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA from fixtures.pbn_api import MOCK_RETURNED_MONGODB_DATA, pbn_pageable_json +from pbn_api.adapters.wydawnictwo import WydawnictwoPBNAdapter from pbn_api.client import ( PBN_GET_INSTITUTION_STATEMENTS, PBN_GET_PUBLICATION_BY_ID_URL, - PBN_POST_PUBLICATIONS_URL, ) -from pbn_api.const import PBN_GET_INSTITUTION_PUBLICATIONS_V2 +from pbn_api.const import ( + PBN_GET_INSTITUTION_PUBLICATIONS_V2, + PBN_POST_INSTITUTION_STATEMENTS_URL, + PBN_POST_PUBLICATION_NO_STATEMENTS_URL, +) from pbn_api.models import Institution, Publication from pbn_api.tests.utils import middleware @@ -55,6 +59,7 @@ def test_helpers_wysylka_z_uid_uczelni( pbn_uczelnia, admin_user, pbn_client, + monkeypatch, ): odpowiednik = baker.make(Institution, mongoId="PBN_UID_UCZELNI----") @@ -67,9 +72,21 @@ def test_helpers_wysylka_z_uid_uczelni( pbn_uczelnia.pbn_integracja = pbn_uczelnia.pbn_aktualizuj_na_biezaco = True pbn_uczelnia.save() - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk - } + # Ten test bada UID uczelni w body, nie synchronizację statements — + # wymuszamy pustą intencję żeby _sync_statements_with_pbn nie próbował + # POST /v2/statements. Uczelnia musi mieć pbn_wysylaj_bez_oswiadczen + # żeby adapter.pbn_get_json nie rzucił StatementsMissing. + pbn_uczelnia.pbn_wysylaj_bez_oswiadczen = True + pbn_uczelnia.save() + monkeypatch.setattr( + WydawnictwoPBNAdapter, + "pbn_get_json_statements", + lambda self, _lst=None: [], + ) + + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk} + ] pbn_client.transport.return_values[ PBN_GET_PUBLICATION_BY_ID_URL.format( id=pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk @@ -84,8 +101,12 @@ def test_helpers_wysylka_z_uid_uczelni( ) pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_STATEMENTS + "?publicationId=123&size=5120" + PBN_GET_INSTITUTION_STATEMENTS + + f"?publicationId={pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk}&size=5120" ] = pbn_pageable_json([]) + pbn_client.transport.return_values[PBN_POST_INSTITUTION_STATEMENTS_URL] = { + "data": [] + } req = rf.get("/") req._uczelnia = pbn_uczelnia @@ -95,13 +116,17 @@ def test_helpers_wysylka_z_uid_uczelni( sprobuj_wyslac_do_pbn_gui(req, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) msg = list(get_messages(req)) - assert len(msg) == 1 - # assert str(msg[0]).find("nie posiada oświadczeń") > -1 - assert str(msg[0]).find("y zaktualizowane") > -1 + # Nowy flow może wyemitować dodatkowe info-messages o sync statements + # (np. "Oświadczenia identyczne"); szukamy końcowego success-message. + assert any("y zaktualizowane" in str(m) for m in msg) - iv = pbn_client.transport.input_values["/api/v1/publications"] - assert iv["body"]["authors"][0]["affiliations"][0] == odpowiednik.pk - assert iv["body"]["institutions"][odpowiednik.pk]["objectId"] == odpowiednik.pk + # Po refaktoryzacji: endpoint repo zwraca body jako lista [js]; autorzy + # po ``convert_js_with_statements_to_no_statements`` używają pola + # ``firstName`` zamiast ``givenNames`` (konwersja w adapterze). + iv = pbn_client.transport.input_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] + body = iv["body"][0] + assert body["authors"][0]["affiliations"][0] == odpowiednik.pk + assert body["institutions"][odpowiednik.pk]["objectId"] == odpowiednik.pk @pytest.mark.django_db @@ -113,6 +138,7 @@ def test_helpers_wysylka_bez_uid_uczelni( pbn_jednostka, admin_user, pbn_client, + monkeypatch, ): odpowiednik = baker.make(Institution, mongoId="PBN_UID_UCZELNI----") @@ -125,9 +151,18 @@ def test_helpers_wysylka_bez_uid_uczelni( pbn_uczelnia.pbn_integracja = pbn_uczelnia.pbn_aktualizuj_na_biezaco = True pbn_uczelnia.save() - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk - } + # Ten test bada afiliację jednostki w body, nie sync statements. + pbn_uczelnia.pbn_wysylaj_bez_oswiadczen = True + pbn_uczelnia.save() + monkeypatch.setattr( + WydawnictwoPBNAdapter, + "pbn_get_json_statements", + lambda self, _lst=None: [], + ) + + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk} + ] pbn_client.transport.return_values[ PBN_GET_PUBLICATION_BY_ID_URL.format( id=pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk @@ -144,8 +179,12 @@ def test_helpers_wysylka_bez_uid_uczelni( ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_STATEMENTS + "?publicationId=123&size=5120" + PBN_GET_INSTITUTION_STATEMENTS + + f"?publicationId={pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk}&size=5120" ] = pbn_pageable_json([]) + pbn_client.transport.return_values[PBN_POST_INSTITUTION_STATEMENTS_URL] = { + "data": [] + } req = rf.get("/") req._uczelnia = pbn_uczelnia @@ -155,11 +194,12 @@ def test_helpers_wysylka_bez_uid_uczelni( sprobuj_wyslac_do_pbn_gui(req, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) msg = list(get_messages(req)) - assert len(msg) == 1 and str(msg[0]).find("y zaktualizowane") > -1 + assert any("y zaktualizowane" in str(m) for m in msg) - iv = pbn_client.transport.input_values["/api/v1/publications"] - assert iv["body"]["authors"][0]["affiliations"][0] == pbn_jednostka.pbn_uid_id + iv = pbn_client.transport.input_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] + body = iv["body"][0] + assert body["authors"][0]["affiliations"][0] == pbn_jednostka.pbn_uid_id assert ( - iv["body"]["institutions"][pbn_jednostka.pbn_uid_id]["objectId"] + body["institutions"][pbn_jednostka.pbn_uid_id]["objectId"] == pbn_jednostka.pbn_uid_id ) diff --git a/src/pbn_api/tests/test_client_sync.py b/src/pbn_api/tests/test_client_sync.py index 6a42b96b6..5cc47631c 100644 --- a/src/pbn_api/tests/test_client_sync.py +++ b/src/pbn_api/tests/test_client_sync.py @@ -1,69 +1,131 @@ """ Tests for PBNClient sync_publication method. +Po refaktoryzacji w Commitach 2-4 (patrz docs/pbn-wysylka-plan.md): +- ``upload_publication`` ZAWSZE wysyła przez endpoint repozytoryjny + ``POST /api/v1/repositorium/publications`` (bez oświadczeń w body), + odpowiedź w formacie ``[{"id": ...}]``. +- ``sync_publication`` synchronizuje oświadczenia osobno przez + ``_sync_statements_with_pbn`` — GET aktualnego stanu w PBN, diff z + intencją BPP (``pbn_get_json_statements``), selektywne DELETE (lub + batch — sterowane ``Uczelnia.pbn_kasuj_dyscypliny_selektywnie``) + + POST przez ``/api/v2/institution-profile/statements``. + For upload tests, see test_client_upload.py For discipline tests, see test_client_disciplines.py For helper/GUI tests, see test_client_helpers.py """ +import time +from unittest.mock import patch + import pytest -from bpp.decorators import json from fixtures import MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA from fixtures.pbn_api import MOCK_RETURNED_MONGODB_DATA, pbn_pageable_json +from pbn_api.adapters.wydawnictwo import WydawnictwoPBNAdapter from pbn_api.client import ( PBN_DELETE_PUBLICATION_STATEMENT, PBN_GET_INSTITUTION_STATEMENTS, PBN_GET_PUBLICATION_BY_ID_URL, - PBN_POST_PUBLICATIONS_URL, ) -from pbn_api.const import PBN_GET_INSTITUTION_PUBLICATIONS_V2 -from pbn_api.exceptions import HttpException, PKZeroExportDisabled +from pbn_api.const import ( + PBN_GET_INSTITUTION_PUBLICATIONS_V2, + PBN_POST_INSTITUTION_STATEMENTS_URL, + PBN_POST_PUBLICATION_NO_STATEMENTS_URL, +) +from pbn_api.exceptions import ( + HttpException, + PKZeroExportDisabled, + StatementsResendFailedException, +) from pbn_api.models import Publication, SentData +def _patch_intended_statements(monkeypatch, statements): + """Patch adapter żeby zwracał zadaną listę intended statements. + + Patchuje OBIE metody adaptera: + - ``pbn_get_json_statements`` — używana w ``_sync_statements_with_pbn`` + (porównanie z PBN); format ``[{personObjectId, disciplineId, type}, ...]`` + - ``pbn_get_api_statements`` — używana w ``_post_statements_with_retry`` + (POST /v2/statements); zwraca ``{publicationUuid, statements}`` + + Ustawia też ``pbn_wysylaj_bez_oswiadczen=True`` na instancji adaptera, + żeby ``StatementsMissing`` nie wywalił ``pbn_get_json`` w KROK 1 + (walidacja w adapterze wymaga klucza ``statements`` w JSON gdy flaga + False, a my tu symulujemy różne stany). + """ + statements_list = list(statements) + monkeypatch.setattr( + WydawnictwoPBNAdapter, + "pbn_get_json_statements", + lambda self, _lst=None: list(statements_list), + ) + monkeypatch.setattr( + WydawnictwoPBNAdapter, + "pbn_get_api_statements", + lambda self: { + "publicationUuid": "00000000-0000-0000-0000-000000000001", + "statements": list(statements_list), + }, + ) + original_init = WydawnictwoPBNAdapter.__init__ + + def patched_init(self, *args, **kwargs): + original_init(self, *args, **kwargs) + self.pbn_wysylaj_bez_oswiadczen = True + + monkeypatch.setattr(WydawnictwoPBNAdapter, "__init__", patched_init) + + +def _setup_common_mocks(pbn_client, object_id, pbn_statements=None): + """Ustawia standardowe odpowiedzi mockowe dla sync_publication flow. + + Mockuje: POST repo, download_publication, V2 institution publications, + GET statements (pusta lub podana lista). + + Uwaga: ``object_id`` przekazujemy w formacie natywnym (int albo str) + — PBN GET endpointy formatują URL przez ``.format(id=...)``, a POST + body dostaje wartość jak jest (typ zachowany dla porównania z + ``pub.pbn_uid_id`` po sync). + """ + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": object_id} + ] + pbn_client.transport.return_values[ + PBN_GET_PUBLICATION_BY_ID_URL.format(id=object_id) + ] = MOCK_RETURNED_MONGODB_DATA + pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=456)] = ( + MOCK_RETURNED_MONGODB_DATA + ) + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_PUBLICATIONS_V2 + f"?publicationId={object_id}&size=10" + ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_STATEMENTS + f"?publicationId={object_id}&size=5120" + ] = pbn_pageable_json(list(pbn_statements or [])) + + +# ============================================================ +# Podstawowe scenariusze (happy paths) — sync_publication +# ============================================================ + + @pytest.mark.django_db def test_sync_publication_to_samo_id( pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, pbn_publication, - pbn_autor, - pbn_jednostka, + monkeypatch, ): + """Publikacja ma już pbn_uid, PBN zwraca to samo ID — pbn_uid_id nie zmienia się.""" pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() - stare_id = pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": pbn_publication.pk - } - pbn_client.transport.return_values[ - PBN_GET_PUBLICATION_BY_ID_URL.format(id=pbn_publication.pk) - ] = MOCK_RETURNED_MONGODB_DATA - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_PUBLICATIONS_V2 + "?publicationId=123&size=10" - ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) - pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=456)] = ( - MOCK_RETURNED_MONGODB_DATA - ) - - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_STATEMENTS + "?publicationId=123&size=5120" - ] = pbn_pageable_json( - [ - { - "id": "eaec3254-2eb1-44d9-8c3c-e68fc2a48bd9", - "addedTimestamp": "2020.05.06", - "institutionId": pbn_jednostka.pbn_uid_id, - "personId": pbn_autor.pbn_uid_id, - "publicationId": pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id, - "area": "200", - "inOrcid": True, - "type": "FOOBAR", - } - ] - ) + _patch_intended_statements(monkeypatch, []) + _setup_common_mocks(pbn_client, pbn_publication.pk, pbn_statements=[]) pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) @@ -74,25 +136,14 @@ def test_sync_publication_to_samo_id( @pytest.mark.django_db def test_sync_publication_tekstowo_podane_id( - pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, pbn_publication + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, ): - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": pbn_publication.pk - } - pbn_client.transport.return_values[ - PBN_GET_PUBLICATION_BY_ID_URL.format(id=pbn_publication.pk) - ] = MOCK_RETURNED_MONGODB_DATA - - pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=456)] = ( - MOCK_RETURNED_MONGODB_DATA - ) - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_PUBLICATIONS_V2 + "?publicationId=123&size=10" - ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) - - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_STATEMENTS + "?publicationId=123&size=5120" - ] = pbn_pageable_json([]) + """Argument w formacie 'model:pk' jest konwertowany przez eventually_coerce_to_publication.""" + _patch_intended_statements(monkeypatch, []) + _setup_common_mocks(pbn_client, pbn_publication.pk, pbn_statements=[]) pbn_client.sync_publication( f"wydawnictwo_zwarte:{pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk}" @@ -104,33 +155,25 @@ def test_sync_publication_tekstowo_podane_id( @pytest.mark.django_db def test_sync_publication_nowe_id( - pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, pbn_publication + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, ): + """Nowa publikacja bez pbn_uid_id — PBN nadaje ID, ustawiamy lokalnie.""" assert pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id is None - stare_id = pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id - - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": pbn_publication.pk - } - pbn_client.transport.return_values[ - PBN_GET_PUBLICATION_BY_ID_URL.format(id=pbn_publication.pk) - ] = MOCK_RETURNED_MONGODB_DATA - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_STATEMENTS + "?publicationId=123&size=5120" - ] = pbn_pageable_json([]) - pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=456)] = ( - MOCK_RETURNED_MONGODB_DATA - ) - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_PUBLICATIONS_V2 + "?publicationId=123&size=10" - ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) + _patch_intended_statements(monkeypatch, []) + _setup_common_mocks(pbn_client, pbn_publication.pk, pbn_statements=[]) pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) - pbn_publication.refresh_from_db() - assert pbn_publication.versions[0]["baz"] == "quux" - assert stare_id != pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.refresh_from_db() + # Po refresh_from_db pbn_uid_id to string (CharField PK), mock zwraca + # int — porównujemy przez str() dla tolerancji typu. + assert str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id) == str( + pbn_publication.pk + ) @pytest.mark.django_db @@ -139,39 +182,24 @@ def test_sync_publication_wysylka_z_zerowym_pk( pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, pbn_publication, pbn_uczelnia, + monkeypatch, ): + """Flaga ``export_pk_zero`` kontroluje czy prace z PK=0 są wysyłane.""" pbn_uczelnia.pbn_api_nie_wysylaj_prac_bez_pk = True pbn_uczelnia.save() pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.punkty_kbn = 0 pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": pbn_publication.pk - } - pbn_client.transport.return_values[ - PBN_GET_PUBLICATION_BY_ID_URL.format(id=pbn_publication.pk) - ] = MOCK_RETURNED_MONGODB_DATA - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_PUBLICATIONS_V2 + "?publicationId=123&size=10" - ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) - pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=456)] = ( - MOCK_RETURNED_MONGODB_DATA - ) - - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_STATEMENTS + "?publicationId=123&size=5120" - ] = pbn_pageable_json([]) - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_PUBLICATIONS_V2 + "?publicationId=123&size=10" - ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) + _patch_intended_statements(monkeypatch, []) + _setup_common_mocks(pbn_client, pbn_publication.pk, pbn_statements=[]) - # To pójdzie + # export_pk_zero=True — pójdzie pbn_client.sync_publication( pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, export_pk_zero=True ) - # To nie pójdzie + # export_pk_zero=False — rzuci PKZeroExportDisabled with pytest.raises(PKZeroExportDisabled): pbn_client.sync_publication( pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, export_pk_zero=False @@ -179,188 +207,457 @@ def test_sync_publication_wysylka_z_zerowym_pk( @pytest.mark.django_db -def test_sync_publication_kasuj_oswiadczenia_przed_wszystko_dobrze( +def test_upload_and_sync_publication_without_existing_publication( + pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, monkeypatch +): + """Regression: Publication nie istnieje lokalnie przy upload — SentData ma + być poprawnie zaktualizowany linkiem po download_publication.""" + from fixtures.pbn_api import MOCK_MONGO_ID + + new_object_id = MOCK_MONGO_ID + assert not Publication.objects.filter(pk=new_object_id).exists() + + _patch_intended_statements(monkeypatch, []) + _setup_common_mocks(pbn_client, new_object_id, pbn_statements=[]) + + publication = pbn_client.sync_publication( + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina + ) + + assert Publication.objects.filter(pk=new_object_id).exists() + sent_data = SentData.objects.get_for_rec( + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina + ) + assert sent_data.pbn_uid_id == new_object_id + assert sent_data.pbn_uid == publication + assert sent_data.submitted_successfully is True + + +# ============================================================ +# Endpoint: zawsze /api/v1/repositorium/publications +# ============================================================ + + +@pytest.mark.django_db +def test_sync_publication_zawsze_endpoint_repo( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, +): + """Upload zawsze idzie przez endpoint repo, nie /api/v1/publications.""" + _patch_intended_statements(monkeypatch, []) + _setup_common_mocks(pbn_client, pbn_publication.pk, pbn_statements=[]) + + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) + + # Potwierdzamy że wysyłka poszła do endpointu repozytoryjnego + assert PBN_POST_PUBLICATION_NO_STATEMENTS_URL in pbn_client.transport.input_values + # Body bez klucza "statements" + body = pbn_client.transport.input_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL][ + "body" + ] + assert isinstance(body, list) and len(body) == 1 + assert "statements" not in body[0] + + +# ============================================================ +# Synchronizacja statements: 4 scenariusze (diff + flagi) +# ============================================================ + + +@pytest.mark.django_db +def test_sync_statements_identyczne_nic_nie_wysyla( pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, pbn_publication, pbn_autor, pbn_jednostka, + monkeypatch, ): - pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication - pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + """PBN i intencja identyczne — brak DELETE, brak POST /v2/statements.""" + _patch_intended_statements( + monkeypatch, + [ + { + "personObjectId": pbn_autor.pbn_uid_id, + "disciplineId": 301, + "type": "AUTHOR", + } + ], + ) + _setup_common_mocks( + pbn_client, + pbn_publication.pk, + pbn_statements=[ + { + "id": "aaa", + "personId": pbn_autor.pbn_uid_id, + "area": "301", + "type": "AUTHOR", + "institutionId": pbn_jednostka.pbn_uid_id, + } + ], + ) - stare_id = pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": pbn_publication.pk - } - pbn_client.transport.return_values[ - PBN_GET_PUBLICATION_BY_ID_URL.format(id=pbn_publication.pk) - ] = MOCK_RETURNED_MONGODB_DATA - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_PUBLICATIONS_V2 + "?publicationId=123&size=10" - ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) - pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=456)] = ( - MOCK_RETURNED_MONGODB_DATA + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk ) + assert url_delete not in pbn_client.transport.input_values + assert PBN_POST_INSTITUTION_STATEMENTS_URL not in pbn_client.transport.input_values - pbn_client.transport.return_values[ - PBN_DELETE_PUBLICATION_STATEMENT.format(publicationId=pbn_publication.pk) - ] = [] - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_STATEMENTS + "?publicationId=123&size=5120" - ] = pbn_pageable_json( + +@pytest.mark.django_db +def test_sync_statements_pbn_puste_bpp_ma_post_only( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + monkeypatch, +): + """PBN puste, BPP ma intencję — POST do /v2/statements, brak DELETE.""" + _patch_intended_statements( + monkeypatch, [ { - "id": "eaec3254-2eb1-44d9-8c3c-e68fc2a48bd9", - "addedTimestamp": "2020.05.06", - "institutionId": pbn_jednostka.pbn_uid_id, - "personId": pbn_autor.pbn_uid_id, - "publicationId": pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id, - "area": "200", - "inOrcid": True, - "type": "FOOBAR", + "personObjectId": pbn_autor.pbn_uid_id, + "disciplineId": 301, + "type": "AUTHOR", } - ] + ], ) + _setup_common_mocks(pbn_client, pbn_publication.pk, pbn_statements=[]) + pbn_client.transport.return_values[PBN_POST_INSTITUTION_STATEMENTS_URL] = { + "data": [] + } - pbn_client.sync_publication( - pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, - delete_statements_before_upload=True, - ) + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) - pbn_publication.refresh_from_db() - assert pbn_publication.versions[0]["baz"] == "quux" - assert stare_id == pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk + ) + assert url_delete not in pbn_client.transport.input_values + assert PBN_POST_INSTITUTION_STATEMENTS_URL in pbn_client.transport.input_values @pytest.mark.django_db -def test_sync_publication_kasuj_oswiadczenia_przed_blad_400_nie_zaburzy( +def test_sync_statements_pbn_ma_bpp_puste_selektywnie_delete( pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, pbn_publication, pbn_autor, pbn_jednostka, + pbn_uczelnia, + monkeypatch, ): - pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication - pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() - - stare_id = pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id + """Selektywny mode + PBN ma, BPP puste → DELETE per-osoba, brak POST.""" + pbn_uczelnia.pbn_kasuj_dyscypliny_selektywnie = True + pbn_uczelnia.save() - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": pbn_publication.pk - } - pbn_client.transport.return_values[ - PBN_GET_PUBLICATION_BY_ID_URL.format(id=pbn_publication.pk) - ] = MOCK_RETURNED_MONGODB_DATA - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_PUBLICATIONS_V2 - + f"?publicationId={pbn_publication.pk}&size=10" - ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) - pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=456)] = ( - MOCK_RETURNED_MONGODB_DATA + _patch_intended_statements(monkeypatch, []) + _setup_common_mocks( + pbn_client, + pbn_publication.pk, + pbn_statements=[ + { + "id": "aaa", + "personId": pbn_autor.pbn_uid_id, + "area": "301", + "type": "AUTHOR", + "institutionId": pbn_jednostka.pbn_uid_id, + } + ], + ) + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk ) + pbn_client.transport.return_values[url_delete] = [] + + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) + + # DELETE wykonany (per osoba) z body {statementsOfPersons: [{personId, role}]} + assert url_delete in pbn_client.transport.input_values + body = pbn_client.transport.input_values[url_delete]["body"] + assert "statementsOfPersons" in body + assert body["statementsOfPersons"][0]["personId"] == pbn_autor.pbn_uid_id + assert body["statementsOfPersons"][0]["role"] == "AUTHOR" + # POST nie ma co robić bo BPP puste + assert PBN_POST_INSTITUTION_STATEMENTS_URL not in pbn_client.transport.input_values - url = PBN_DELETE_PUBLICATION_STATEMENT.format(publicationId=pbn_publication.pk) - err_json = { - "code": 400, - "message": "Bad Request", - "description": "Validation failed.", - "details": { - "publicationId": "Nie można usunąć oświadczeń. Nie istnieją oświadczenia " - "dla publikacji (id = {pbn_publication.pk}) i instytucji (id = XXX)." - }, - } - pbn_client.transport.return_values[url] = HttpException( - 400, url, json.dumps(err_json) +@pytest.mark.django_db +def test_sync_statements_pbn_ma_bpp_puste_batch_delete( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + pbn_jednostka, + pbn_uczelnia, + monkeypatch, +): + """Batch mode + PBN ma, BPP puste → delete_all, brak POST.""" + pbn_uczelnia.pbn_kasuj_dyscypliny_selektywnie = False + pbn_uczelnia.save() + + _patch_intended_statements(monkeypatch, []) + _setup_common_mocks( + pbn_client, + pbn_publication.pk, + pbn_statements=[ + { + "id": "aaa", + "personId": pbn_autor.pbn_uid_id, + "area": "301", + "type": "AUTHOR", + "institutionId": pbn_jednostka.pbn_uid_id, + } + ], ) + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk + ) + pbn_client.transport.return_values[url_delete] = [] - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_STATEMENTS + "?publicationId=123&size=5120" - ] = pbn_pageable_json( + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) + + # Batch DELETE ma body {all: True, statementsOfPersons: []} + assert url_delete in pbn_client.transport.input_values + body = pbn_client.transport.input_values[url_delete]["body"] + assert body == {"all": True, "statementsOfPersons": []} + + +@pytest.mark.django_db +def test_sync_statements_roznice_selektywnie( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + pbn_jednostka, + pbn_uczelnia, + monkeypatch, +): + """Różnice w selektywnym trybie: DELETE tylko nieistniejących lokalnie + + POST brakujących.""" + pbn_uczelnia.pbn_kasuj_dyscypliny_selektywnie = True + pbn_uczelnia.save() + + # Intencja: autor X z dyscypliną 100 + _patch_intended_statements( + monkeypatch, [ { - "id": "eaec3254-2eb1-44d9-8c3c-e68fc2a48bd9", - "addedTimestamp": "2020.05.06", - "institutionId": pbn_jednostka.pbn_uid_id, + "personObjectId": pbn_autor.pbn_uid_id, + "disciplineId": 100, + "type": "AUTHOR", + } + ], + ) + # PBN: ten sam autor X ale z dyscypliną 301 + _setup_common_mocks( + pbn_client, + pbn_publication.pk, + pbn_statements=[ + { + "id": "aaa", "personId": pbn_autor.pbn_uid_id, - "publicationId": pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id, - "area": "200", - "inOrcid": True, - "type": "FOOBAR", + "area": "301", + "type": "AUTHOR", + "institutionId": pbn_jednostka.pbn_uid_id, } - ] + ], ) - - pbn_client.sync_publication( - pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, - delete_statements_before_upload=True, + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk ) + pbn_client.transport.return_values[url_delete] = [] + pbn_client.transport.return_values[PBN_POST_INSTITUTION_STATEMENTS_URL] = { + "data": [] + } - pbn_publication.refresh_from_db() - assert pbn_publication.versions[0]["baz"] == "quux" - assert stare_id == pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid_id + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) + + # Klucz (autor, 301) różni się od (autor, 100) → DELETE + POST + assert url_delete in pbn_client.transport.input_values + assert PBN_POST_INSTITUTION_STATEMENTS_URL in pbn_client.transport.input_values + + +# ============================================================ +# Error handling: retry + rollbar + StatementsResendFailedException +# ============================================================ @pytest.mark.django_db -def test_upload_and_sync_publication_without_existing_publication( - pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina +def test_sync_publication_get_statements_retry_wyczerpane_raises( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, ): - """ - Regression test for foreign key violation issue. + """GET statements zawodzi 3 razy → StatementsResendFailedException + rollbar.""" + _patch_intended_statements(monkeypatch, []) + _setup_common_mocks(pbn_client, pbn_publication.pk, pbn_statements=[]) + # Nadpisujemy GET statements żeby rzucał + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_STATEMENTS + + f"?publicationId={pbn_publication.pk}&size=5120" + ] = HttpException(500, "/api/v1/.../page/statements", "Server Error") - Tests that upload_publication() doesn't fail when the Publication record - doesn't exist yet in the local database, and that sync_publication() - properly updates SentData with the publication link after downloading it. - """ - # Use the same objectId as in MOCK_RETURNED_MONGODB_DATA - from fixtures.pbn_api import MOCK_MONGO_ID + # Mock sleep i rollbar żeby test nie był powolny i weryfikujemy call + monkeypatch.setattr(time, "sleep", lambda *_: None) - new_object_id = MOCK_MONGO_ID + with patch("pbn_api.client.publication_sync.rollbar.report_exc_info") as mock_rb: + with pytest.raises(StatementsResendFailedException) as exc_info: + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) - # Ensure Publication doesn't exist - assert not Publication.objects.filter(pk=new_object_id).exists() + # Rollbar wywołany raz z level=warning + assert mock_rb.called + call_kwargs = mock_rb.call_args.kwargs + assert call_kwargs.get("level") == "warning" + assert "publication_pk" in call_kwargs.get("extra_data", {}) + assert "pbn_uid" in call_kwargs.get("extra_data", {}) - # Mock API response for upload (called internally by sync_publication) - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": new_object_id - } + # Exception ma publication_pk i pbn_uid + assert exc_info.value.pbn_uid == pbn_publication.pk - # Mock API response for download_publication - pbn_client.transport.return_values[ - PBN_GET_PUBLICATION_BY_ID_URL.format(id=new_object_id) - ] = MOCK_RETURNED_MONGODB_DATA - # Mock for objectId 456 from MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA - pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=456)] = ( - MOCK_RETURNED_MONGODB_DATA +@pytest.mark.django_db +def test_sync_publication_selektywny_delete_retry_wyczerpane_raises( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + pbn_jednostka, + pbn_uczelnia, + monkeypatch, +): + """Selektywny DELETE zawodzi 3 razy → StatementsResendFailedException.""" + pbn_uczelnia.pbn_kasuj_dyscypliny_selektywnie = True + pbn_uczelnia.save() + + _patch_intended_statements(monkeypatch, []) + _setup_common_mocks( + pbn_client, + pbn_publication.pk, + pbn_statements=[ + { + "id": "aaa", + "personId": pbn_autor.pbn_uid_id, + "area": "301", + "type": "AUTHOR", + "institutionId": pbn_jednostka.pbn_uid_id, + } + ], + ) + # DELETE zawsze zawodzi + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk + ) + pbn_client.transport.return_values[url_delete] = HttpException( + 500, url_delete, "Server Error" ) - # Mock empty statements response - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_STATEMENTS + f"?publicationId={new_object_id}&size=5120" - ] = pbn_pageable_json([]) + monkeypatch.setattr(time, "sleep", lambda *_: None) - # Mock institution publications v2 response - pbn_client.transport.return_values[ - PBN_GET_INSTITUTION_PUBLICATIONS_V2 + f"?publicationId={new_object_id}&size=10" - ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) + with patch("pbn_api.client.publication_sync.rollbar.report_exc_info"): + with pytest.raises(StatementsResendFailedException): + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) - # Call sync_publication() - this internally calls upload_publication() - # which should succeed without FK error, then download_publication() - # which should create the Publication and update SentData - publication = pbn_client.sync_publication( - pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina + +@pytest.mark.django_db +def test_sync_publication_post_v2_statements_retry_wyczerpane_raises( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + monkeypatch, +): + """POST /v2/statements zawodzi 3 razy → StatementsResendFailedException.""" + _patch_intended_statements( + monkeypatch, + [ + { + "personObjectId": pbn_autor.pbn_uid_id, + "disciplineId": 100, + "type": "AUTHOR", + } + ], + ) + _setup_common_mocks(pbn_client, pbn_publication.pk, pbn_statements=[]) + pbn_client.transport.return_values[PBN_POST_INSTITUTION_STATEMENTS_URL] = ( + HttpException(500, PBN_POST_INSTITUTION_STATEMENTS_URL, "Server Error") ) - # Verify Publication now exists in database - assert Publication.objects.filter(pk=new_object_id).exists() + monkeypatch.setattr(time, "sleep", lambda *_: None) - # Verify SentData was created and updated with the publication link - sent_data = SentData.objects.get_for_rec( - pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina + with patch("pbn_api.client.publication_sync.rollbar.report_exc_info"): + with pytest.raises(StatementsResendFailedException): + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) + + +# ============================================================ +# Edge case: POST publikacji zawodzi — statements nietknięte +# ============================================================ + + +@pytest.mark.django_db +def test_sync_publication_post_repo_fail_statements_nietkniete( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, +): + """POST publikacji zawodzi → statements w PBN nie są ruszane.""" + _patch_intended_statements(monkeypatch, []) + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = ( + HttpException( + 500, PBN_POST_PUBLICATION_NO_STATEMENTS_URL, "Internal Server Error" + ) ) - assert sent_data.pbn_uid_id == new_object_id - assert sent_data.pbn_uid == publication - assert sent_data.submitted_successfully is True + + with pytest.raises(HttpException): + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) + + # Nic nie powinno polecieć do PBN poza failed POST + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk + ) + assert url_delete not in pbn_client.transport.input_values + assert PBN_POST_INSTITUTION_STATEMENTS_URL not in pbn_client.transport.input_values + assert ( + PBN_GET_INSTITUTION_STATEMENTS + + f"?publicationId={pbn_publication.pk}&size=5120" + not in pbn_client.transport.input_values + ) + + +# ============================================================ +# Unit test: _diff_statements key mapping +# ============================================================ + + +def test_diff_statements_key_mapping(pbn_client): + """Klucz porównania PBN vs intended używa (person, discipline) jako string.""" + pbn_stmts = [ + {"personId": "abc123", "area": "301", "type": "AUTHOR"}, + {"personId": "def456", "area": "502", "type": "EDITOR"}, + ] + intended = [ + {"personObjectId": "abc123", "disciplineId": 301, "type": "AUTHOR"}, + {"personObjectId": "ghi789", "disciplineId": 200, "type": "AUTHOR"}, + ] + only_pbn, only_intended = pbn_client._diff_statements(pbn_stmts, intended) + + # (abc123, 301) — w obu, więc w żadnym diff + # (def456, 502) — tylko w PBN + # (ghi789, 200) — tylko w intended + assert only_pbn == {("def456", "502")} + assert only_intended == {("ghi789", "200")} + + +def test_diff_statements_empty_sets(pbn_client): + """Puste zestawy → puste diff.""" + only_pbn, only_intended = pbn_client._diff_statements([], []) + assert only_pbn == set() + assert only_intended == set() diff --git a/src/pbn_api/tests/test_client_upload.py b/src/pbn_api/tests/test_client_upload.py index bcc6ddf97..a6dd3f2a6 100644 --- a/src/pbn_api/tests/test_client_upload.py +++ b/src/pbn_api/tests/test_client_upload.py @@ -10,7 +10,6 @@ from model_bakery import baker from pbn_api.adapters.wydawnictwo import WydawnictwoPBNAdapter -from pbn_api.client import PBN_POST_PUBLICATIONS_URL from pbn_api.const import PBN_POST_PUBLICATION_NO_STATEMENTS_URL from pbn_api.exceptions import SameDataUploadedRecently from pbn_api.models import Publication, SentData @@ -24,14 +23,20 @@ class PBNTestClientException(Exception): def test_PBNClient_test_upload_publication_nie_trzeba( pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina ): - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = {"objectId": None} - - # Create SentData with submitted_successfully=True to trigger SameDataUploadedRecently - sent_data = SentData.objects.create_or_update_before_upload( # noqa - pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, - WydawnictwoPBNAdapter( - pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina - ).pbn_get_json(), + # Po refaktoryzacji upload_publication zawsze używa endpointu repo. + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": None} + ] + + # SentData musi być tworzony z JSON po konwersji (bez statements, + # firstName etc.) bo nowy upload_publication robi ten sam convert + # przed porównaniem w ``check_if_upload_needed``. + js = WydawnictwoPBNAdapter( + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina + ).pbn_get_json() + js = pbn_client.convert_js_with_statements_to_no_statements(js) + SentData.objects.create_or_update_before_upload( + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, js ) baker.make(Publication, pk="test-123") @@ -49,7 +54,7 @@ def test_PBNClient_test_upload_publication_nie_trzeba( def test_PBNClient_test_upload_publication_exception( pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina ): - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = ( + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = ( PBNTestClientException("nei") ) @@ -61,22 +66,33 @@ def test_PBNClient_test_upload_publication_exception( def test_PBNClient_test_upload_publication_wszystko_ok( pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, pbn_publication ): - pbn_client.transport.return_values[PBN_POST_PUBLICATIONS_URL] = { - "objectId": pbn_publication.pk - } + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": pbn_publication.pk} + ] objectId, ret, js, bez_oswiadczen = pbn_client.upload_publication( pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina ) assert objectId == pbn_publication.pk + # Po refaktoryzacji ``bez_oswiadczen`` zawsze True (endpoint repo) + assert bez_oswiadczen is True @pytest.mark.django_db def test_PBNClient_post_publication_no_statements( - pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, uczelnia + pbn_client, pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, uczelnia, monkeypatch ): + """Smoke test że sync_publication używa endpoint repo dla pracy bez dyscyplin. + + Uczelnia z ``pbn_wysylaj_bez_oswiadczen=True`` pozwala na wysyłkę prac + bez oświadczeń (inaczej adapter rzuca StatementsMissing w pbn_get_json). + """ from fixtures.pbn_api import MOCK_RETURNED_MONGODB_DATA from pbn_api.client import PBN_GET_PUBLICATION_BY_ID_URL + from pbn_api.const import ( + PBN_GET_INSTITUTION_PUBLICATIONS_V2, + PBN_GET_INSTITUTION_STATEMENTS, + ) uczelnia.pbn_wysylaj_bez_oswiadczen = True uczelnia.save() @@ -87,6 +103,18 @@ def test_PBNClient_post_publication_no_statements( pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=123)] = ( MOCK_RETURNED_MONGODB_DATA ) + pbn_client.transport.return_values[PBN_GET_PUBLICATION_BY_ID_URL.format(id=456)] = ( + MOCK_RETURNED_MONGODB_DATA + ) + from fixtures import MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA + from fixtures.pbn_api import pbn_pageable_json + + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_PUBLICATIONS_V2 + "?publicationId=123&size=10" + ] = pbn_pageable_json(MOCK_RETURNED_INSTITUTION_PUBLICATION_V2_DATA) + pbn_client.transport.return_values[ + PBN_GET_INSTITUTION_STATEMENTS + "?publicationId=123&size=5120" + ] = pbn_pageable_json([]) pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.autorzy_set.all().update( dyscyplina_naukowa=None From a7186d920888f66679505b3fafab4f034995b087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 22:24:31 +0200 Subject: [PATCH 15/33] feat(pbn_export_queue): RETRY_LATER handling dla StatementsResendFailedException MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/pbn_export_queue/models.py | 14 ++++ .../tests/test_pbn_queue_send.py | 64 ++++++++++++------- 2 files changed, 56 insertions(+), 22 deletions(-) diff --git a/src/pbn_export_queue/models.py b/src/pbn_export_queue/models.py index 725e4ddc5..234fef4aa 100644 --- a/src/pbn_export_queue/models.py +++ b/src/pbn_export_queue/models.py @@ -22,6 +22,7 @@ PKZeroExportDisabled, PraceSerwisoweException, ResourceLockedException, + StatementsResendFailedException, WillNotExportError, ) @@ -241,6 +242,19 @@ def _handle_retry_exception(self, exc): self.save() return SendStatus.RETRY_LATER + if isinstance(exc, StatementsResendFailedException): + # Publikacja została wysłana do PBN (POST /repositorium OK), + # ale synchronizacja oświadczeń (GET/DELETE/POST /v2) wyczerpała + # retry w sync_publication. Ponawiamy po kilku minutach — + # zwykle chwilowa niedostępność PBN. + self.dopisz_komunikat( + f"Synchronizacja oświadczeń nie powiodła się po wyczerpaniu prób " + f"(PBN UID={exc.pbn_uid}): {exc.last_error}. " + f"Ponowię wysyłkę za kilka minut..." + ) + self.save() + return SendStatus.RETRY_LATER + return None def _handle_exclude_exception(self, exc): diff --git a/src/pbn_export_queue/tests/test_pbn_queue_send.py b/src/pbn_export_queue/tests/test_pbn_queue_send.py index 2f87db3b0..05a1c0751 100644 --- a/src/pbn_export_queue/tests/test_pbn_queue_send.py +++ b/src/pbn_export_queue/tests/test_pbn_queue_send.py @@ -20,6 +20,7 @@ PKZeroExportDisabled, PraceSerwisoweException, StatementsMissing, + StatementsResendFailedException, ) from pbn_export_queue.models import PBN_Export_Queue, RodzajBledu, SendStatus @@ -64,6 +65,39 @@ def test_send_to_pbn_record_deleted_but_already_finished( result = queue_item.send_to_pbn() assert result == SendStatus.FINISHED_OKAY + def test_send_to_pbn_statements_resend_failed_exception( + self, wydawnictwo_ciagle, admin_user + ): + """Test RETRY_LATER when StatementsResendFailedException is raised. + + Scenariusz: POST publikacji do /repositorium powiódł się, ale + kolejne kroki (GET/DELETE/POST /v2/statements) wyczerpały retry + w sync_publication. Kolejka ma ponowić za kilka minut. + """ + queue_item = baker.make( + PBN_Export_Queue, + rekord_do_wysylki=wydawnictwo_ciagle, + zamowil=admin_user, + ) + + with patch( + "bpp.admin.helpers.pbn_api.cli.sprobuj_wyslac_do_pbn_celery" + ) as mock_send: + mock_send.side_effect = StatementsResendFailedException( + publication_pk=wydawnictwo_ciagle.pk, + pbn_uid="abc-123", + last_error="HTTP 500: Server Error", + ) + + result = queue_item.send_to_pbn() + + assert result == SendStatus.RETRY_LATER + queue_item.refresh_from_db() + assert queue_item.ilosc_prob == 1 + assert "Synchronizacja oświadczeń" in queue_item.komunikat + assert "abc-123" in queue_item.komunikat + assert "Ponowię" in queue_item.komunikat + def test_send_to_pbn_prace_serwisowe_exception( self, wydawnictwo_ciagle, admin_user ): @@ -330,9 +364,7 @@ class TestHttpExceptionValidationClassification: where validation errors were incorrectly classified as technical errors. """ - def test_http_400_with_details_isbn_duplicate( - self, wydawnictwo_ciagle, admin_user - ): + def test_http_400_with_details_isbn_duplicate(self, wydawnictwo_ciagle, admin_user): """ISBN duplicate error should be MERYTORYCZNY (regression test for ID 20)""" queue_item = baker.make( PBN_Export_Queue, @@ -349,9 +381,7 @@ def test_http_400_with_details_isbn_duplicate( "message": "Bad Request", "description": "Validation failed.", "details": { - "isbn": ( - "Publikacja o identycznym ISBN lub ISMN już istnieje!" - ) + "isbn": ("Publikacja o identycznym ISBN lub ISMN już istnieje!") }, } ) @@ -364,9 +394,7 @@ def test_http_400_with_details_isbn_duplicate( assert queue_item.rodzaj_bledu == RodzajBledu.MERYTORYCZNY assert "Błąd walidacji po stronie PBN" in queue_item.komunikat - def test_http_400_with_details_doi_duplicate( - self, wydawnictwo_ciagle, admin_user - ): + def test_http_400_with_details_doi_duplicate(self, wydawnictwo_ciagle, admin_user): """DOI duplicate error should be MERYTORYCZNY (regression test for ID 19)""" queue_item = baker.make( PBN_Export_Queue, @@ -395,9 +423,7 @@ def test_http_400_with_details_doi_duplicate( queue_item.refresh_from_db() assert queue_item.rodzaj_bledu == RodzajBledu.MERYTORYCZNY - def test_http_400_with_details_invalid_year( - self, wydawnictwo_ciagle, admin_user - ): + def test_http_400_with_details_invalid_year(self, wydawnictwo_ciagle, admin_user): """Invalid year error should be MERYTORYCZNY (regression test for ID 11)""" queue_item = baker.make( PBN_Export_Queue, @@ -415,8 +441,7 @@ def test_http_400_with_details_invalid_year( "description": "Validation failed.", "details": { "year": ( - "Rok publikacji nie może być późniejszy" - " od roku bieżącego!" + "Rok publikacji nie może być późniejszy od roku bieżącego!" ) }, } @@ -452,8 +477,7 @@ def test_http_400_with_details_missing_book_id( "description": "Validation failed.", "details": { "book.id": ( - "Identyfikator źródła rozdziału (książki)" - " jest wymagany!" + "Identyfikator źródła rozdziału (książki) jest wymagany!" ) }, } @@ -549,9 +573,7 @@ def test_http_500_stays_techniczny(self, wydawnictwo_ciagle, admin_user): with patch( "bpp.admin.helpers.pbn_api.cli.sprobuj_wyslac_do_pbn_celery" ) as mock: - error_json = json.dumps( - {"code": 500, "message": "Internal Server Error"} - ) + error_json = json.dumps({"code": 500, "message": "Internal Server Error"}) mock.side_effect = HttpException(500, "/api/v1/publications", error_json) result = queue_item.send_to_pbn() @@ -560,9 +582,7 @@ def test_http_500_stays_techniczny(self, wydawnictwo_ciagle, admin_user): queue_item.refresh_from_db() assert queue_item.rodzaj_bledu == RodzajBledu.TECHNICZNY - def test_statements_missing_is_merytoryczny( - self, wydawnictwo_ciagle, admin_user - ): + def test_statements_missing_is_merytoryczny(self, wydawnictwo_ciagle, admin_user): """ StatementsMissing error should be MERYTORYCZNY (business error - missing author statements/disciplines) From 3067b6050468b8134552eab7abd40b46f6eb905e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 21 Apr 2026 22:26:26 +0200 Subject: [PATCH 16/33] docs(pbn): changelog + update pbn-wysylka-plan.md dla Fazy 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/pbn-wysylka-plan.md | 62 +++++++++++++++++-- ...pbn-sync-publication-split-flow.bugfix.rst | 28 +++++++++ 2 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 src/bpp/newsfragments/+pbn-sync-publication-split-flow.bugfix.rst diff --git a/docs/pbn-wysylka-plan.md b/docs/pbn-wysylka-plan.md index 869a969c0..5c24c4293 100644 --- a/docs/pbn-wysylka-plan.md +++ b/docs/pbn-wysylka-plan.md @@ -293,12 +293,62 @@ Obserwacje notujemy w osobnym pliku `docs/pbn-wysylka-eksperymenty.md` `uv run python src/manage.py makemigrations --merge --noinput` (nie modyfikuje istniejących migracji, dodaje nową merge-ową). -## Faza 2 — refaktoryzacja `sync_publication` (osobna gałąź) - -Po ręcznych testach narzędziem z Fazy 1 i udokumentowaniu wyników -(osobny dokument `docs/pbn-wysylka-eksperymenty.md` w kolejnym PR) -implementujemy docelowy flow w `src/pbn_api/client/publication_sync.py`. -Plan dla Fazy 2 — szczegóły po zebraniu obserwacji z eksperymentów. +## Faza 2 — refaktoryzacja `sync_publication` (**zaimplementowana, PR #164**) + +Refaktoryzacja została wykonana na tej samej gałęzi +`feature/pbn-test-wysylka-interaktywna` (8 commitów, cała seria w PR #164). +Narzędzie CLI `pbn_test_wysylka_interaktywna` zostaje jako diagnostyka — +aktualna `sync_publication` ma tą samą logikę wewnętrznie. + +### Flow docelowy (zaimplementowany) + +1. `upload_publication(rec, ...)` → ZAWSZE `POST /api/v1/repositorium/publications` + (`post_publication_no_statements`), niezależnie od obecności oświadczeń + w JSON. Konwersja przez `convert_js_with_statements_to_no_statements()`. +2. `download_publication(objectId)` + update `SentData.pbn_uid`. +3. Obsługa zmiany/konfliktu PBN UID (`_handle_uid_change`/`_conflict`). +4. `_download_statements_with_retry()` — best effort odświeżenie lokalnego + cache `OswiadczenieInstytucji` + pobranie `PublikacjaInstytucji_V2` + (potrzebne dla `pbn_get_api_statements`). Błąd tutaj = warning log, + flow kontynuuje. +5. `_sync_statements_with_pbn(rec, objectId, kasuj_selektywnie)`: + - GET aktualnych oświadczeń z PBN (retry x3 + rollbar + raise + `StatementsResendFailedException` po wyczerpaniu). + - Diff z `pbn_get_json_statements()` — klucz `(person mongoId, dyscyplina numerek)`. + - Selektywny DELETE per-osoba (`delete_publication_statement(pub_id, personId, role)`) + gdy `Uczelnia.pbn_kasuj_dyscypliny_selektywnie=True` (default), + batch `delete_all_publication_statements` gdy `False`. + - POST batch `/api/v2/institution-profile/statements` dla brakujących. + - Każdy krok z retry x3 + rollbar level=warning + `StatementsResendFailedException`. + +### Zmiany modelu + +- Usunięto `Uczelnia.pbn_api_kasuj_przed_wysylka` (migracja 0414). +- Dodano `Uczelnia.pbn_kasuj_dyscypliny_selektywnie` BooleanField default=True. +- Zachowano `Uczelnia.pbn_wysylaj_bez_oswiadczen` (semantyka: odmawia + wysyłki publikacji bez oświadczeń — walidacja w adapterze `pbn_get_json`). + +### Nowy wyjątek + +`pbn_api.exceptions.StatementsResendFailedException(publication_pk, pbn_uid, last_error)` +— podnoszony po wyczerpaniu retry dla GET/DELETE/POST oświadczeń. +Klasyfikowany w `pbn_export_queue._handle_retry_exception` jako +`RETRY_LATER`. + +### Historia commitów (PR #164) + +1. Exception class + model Uczelnia + migracja + admin +2. Dead code removal (`post_publication`, `_should_retry_validation_error`, + `_retry_download_publication`) + bug fix `_delete_statements_with_retry` + (`< 0` → `<= 0`) +3. Helpery: `_diff_statements`, `_get_pbn_statements_with_retry`, + `_delete_statements_selective`, `_delete_statements_batch`, + `_post_statements_with_retry`, `_sync_statements_with_pbn` +4. Refaktoryzacja `sync_publication` (nowy split flow) +5. Aktualizacja callerów (usunięcie `delete_statements_before_upload`) +6. Aktualizacja 5 plików testowych (37 testów, nowe scenariusze) +7. Handling `StatementsResendFailedException` w `pbn_export_queue` + test +8. Changelog + docs ## Wymagania CI diff --git a/src/bpp/newsfragments/+pbn-sync-publication-split-flow.bugfix.rst b/src/bpp/newsfragments/+pbn-sync-publication-split-flow.bugfix.rst new file mode 100644 index 000000000..e4e3d51e6 --- /dev/null +++ b/src/bpp/newsfragments/+pbn-sync-publication-split-flow.bugfix.rst @@ -0,0 +1,28 @@ +Refaktoryzacja wysyłki publikacji do PBN (``sync_publication``): publikacja +jest zawsze wysyłana przez endpoint repozytoryjny +``POST /api/v1/repositorium/publications`` (bez oświadczeń w body), a +dyscypliny/oświadczenia synchronizowane są w osobnym kroku przez +``/api/v2/institution-profile/statements`` dopiero po potwierdzeniu +wysyłki publikacji. Dzięki temu nieudana wysyłka publikacji (np. HTTP +423 Locked albo inna przejściowa awaria PBN) nie kasuje już istniejących +oświadczeń w profilu instytucji — wcześniej kasowanie działo się przed +POST i tracono dane przy każdym niepowodzeniu. + +Algorytm synchronizacji oświadczeń: GET aktualnego stanu PBN, porównanie +z intencją BPP (``WydawnictwoPBNAdapter.pbn_get_json_statements()``) +przez klucz ``(personId, disciplineId)``, selektywne DELETE (per-osoba +przez ``delete_publication_statement(personId, role)``) brakujących w +BPP + POST dodatkowych. Tryb kasowania sterowany nową flagą +``Uczelnia.pbn_kasuj_dyscypliny_selektywnie`` (domyślnie ``True``; +``False`` używa ``delete_all`` + POST batch). + +Nowy wyjątek ``StatementsResendFailedException`` (w +``pbn_api.exceptions``) jest podnoszony gdy retry x3 z exponential +backoff (2s/4s/8s) na GET/DELETE/POST /v2/statements się wyczerpie. +Klasyfikowany w ``pbn_export_queue`` jako ``RETRY_LATER`` — kolejka +ponowi wysyłkę za kilka minut. + +Usunięto pole ``Uczelnia.pbn_api_kasuj_przed_wysylka`` (obsolete — +stary pre-upload DELETE zastąpiony nowym algorytmem diff po wysyłce). +Flaga ``Uczelnia.pbn_wysylaj_bez_oswiadczen`` pozostaje z dotychczasową +semantyką (odmawia wysyłki publikacji bez oświadczeń). From d42c70a9fd4cb91af9f854203e929a7dea534eb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Wed, 22 Apr 2026 20:05:00 +0200 Subject: [PATCH 17/33] fix(pbn): NameError releaseDateYear + priorytet openaccess_data_opublikowania MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/pbn_api/adapters/wydawnictwo.py | 31 +++++++++++++++++++++----- src/pbn_api/client/publication_sync.py | 22 ++++++++++-------- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/pbn_api/adapters/wydawnictwo.py b/src/pbn_api/adapters/wydawnictwo.py index f0508dafe..8b54128b7 100644 --- a/src/pbn_api/adapters/wydawnictwo.py +++ b/src/pbn_api/adapters/wydawnictwo.py @@ -237,16 +237,35 @@ def _build_open_access_base_fields(self) -> dict: return oa def _build_open_access_release_date(self, oa: dict) -> None: - """Add release date fields to OpenAccess dict.""" - if self.original.public_dostep_dnia is not None: + """Ustawia pola daty udostępnienia OpenAccess. + + Priorytet źródeł (od najbardziej dedykowanego): + + 1. ``openaccess_data_opublikowania`` — pole dedykowane OpenAccess + (``ModelZOpenAccess``), ustawiane przez importer z PBN i przez + redakcję ręcznie. To powinno być podstawowe źródło. + 2. ``public_dostep_dnia`` — data wolnego dostępu do strony WWW + (``ModelZWWW``). Fallback dla starszych rekordów bez dedykowanej + daty OpenAccess. + 3. ``dostep_dnia`` — data płatnego dostępu. Ostatni fallback. + + Gdy żadna z dat nie jest wypełniona, pola ``releaseDate``, + ``releaseDateMonth``, ``releaseDateYear`` NIE są ustawiane. + Wcześniejsza implementacja wstawiała hardcoded + ``{"releaseDateMonth": "JANUARY", "releaseDateYear": str(rok)}``, + co wysyłało do PBN nieprawdziwe dane (styczeń niezależnie od + faktycznego miesiąca publikacji). Lepiej pominąć pola i zmusić + PBN do zwrócenia validation error (lub zaakceptowania, jeśli + spec pozwala), niż wysyłać kłamliwy miesiąc. + """ + data_oa = getattr(self.original, "openaccess_data_opublikowania", None) + if data_oa is not None: + oa["releaseDate"] = str(data_oa) + elif self.original.public_dostep_dnia is not None: oa["releaseDate"] = str(self.original.public_dostep_dnia) elif self.original.dostep_dnia is not None: oa["releaseDate"] = str(self.original.dostep_dnia) - if oa.get("releaseDate") is None: - oa["releaseDateMonth"] = "JANUARY" - oa["releaseDateYear"] = str(self.original.rok) - def _build_open_access(self, pub_type: str) -> dict | None: """Build OpenAccess data structure if all required fields are present.""" oa = self._build_open_access_base_fields() diff --git a/src/pbn_api/client/publication_sync.py b/src/pbn_api/client/publication_sync.py index 9a79ae3da..d5cfd1694 100644 --- a/src/pbn_api/client/publication_sync.py +++ b/src/pbn_api/client/publication_sync.py @@ -65,17 +65,21 @@ def convert_js_with_statements_to_no_statements(self, json): # OpenAccess modeArticle -> mode json = rename_dict_key(json, "modeArticle", "mode") - # OpenAccess releaseDateYear "2022" -> 2022 - if json.get("openAccess", False): - if isinstance(json["openAccess"], dict) and json["openAccess"].get( - "releaseDateYear" - ): + # OpenAccess releaseDateYear "2022" -> 2022 (int) + # Jeśli konwersja na int zawiedzie — zachowujemy oryginalną wartość + # (PBN zwróci validation error z jasnym komunikatem, jeśli format + # jest nieprawidłowy). Wcześniejsza implementacja miała NameError: + # zmienna ``i`` była zdefiniowana tylko wewnątrz ``try``, a + # bezwarunkowy assignment poza blokiem rzucał NameError gdy + # ``int()`` failowało. + if json.get("openAccess", False) and isinstance(json["openAccess"], dict): + value = json["openAccess"].get("releaseDateYear") + if value is not None: try: - i = int(json["openAccess"]["releaseDateYear"]) - except (ValueError, TypeError, AttributeError): + json["openAccess"]["releaseDateYear"] = int(value) + except (ValueError, TypeError): + # Nie ruszamy wartości — PBN wskaże problem w walidacji. pass - - json["openAccess"]["releaseDateYear"] = i return json def post_publication_no_statements(self, json): From 8d7d52fff583c06505787eec37973b581eb78e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Thu, 23 Apr 2026 09:02:13 +0200 Subject: [PATCH 18/33] =?UTF-8?q?fix(pbn):=20POST=20sync=20statements=20wy?= =?UTF-8?q?sy=C5=82a=20tylko=20brakuj=C4=85ce=20w=20PBN=20(selective=20mod?= =?UTF-8?q?e)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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ą). --- .../+pbn-post-statements-subset.bugfix.rst | 17 ++ src/pbn_api/client/publication_sync.py | 124 ++++++++++- src/pbn_api/tests/test_client_sync.py | 209 ++++++++++++++++++ 3 files changed, 341 insertions(+), 9 deletions(-) create mode 100644 src/bpp/newsfragments/+pbn-post-statements-subset.bugfix.rst diff --git a/src/bpp/newsfragments/+pbn-post-statements-subset.bugfix.rst b/src/bpp/newsfragments/+pbn-post-statements-subset.bugfix.rst new file mode 100644 index 000000000..236a6169f --- /dev/null +++ b/src/bpp/newsfragments/+pbn-post-statements-subset.bugfix.rst @@ -0,0 +1,17 @@ +Poprawka w ``sync_publication``: POST oświadczeń publikacji w trybie +selektywnym (``Uczelnia.pbn_kasuj_dyscypliny_selektywnie=True``, default) +wysyła teraz TYLKO oświadczenia brakujące w PBN (``only_in_intended``), +nie pełen zestaw lokalnych. Wcześniej kod wywoływał +``WydawnictwoPBNAdapter.pbn_get_api_statements()`` zwracające wszystkie +lokalne statements i POST-ował kompletny zestaw — także te oświadczenia, +które już były w PBN. Przy założeniu że API PBN może nie być +idempotentne (odrzucić duplikaty, utworzyć zduplikowane rekordy albo +zachowywać się nieprzewidywalnie), wysyłanie tylko brakujących jest +bezpieczniejsze — nie dublujemy żądań dla już istniejących oświadczeń, +co zachowuje ich metadata w PBN (``addedTimestamp`` itp.). + +Krok 3 algorytmu (PBN puste + BPP ma) nadal wysyła wszystkie oświadczenia +publikacji, bo w tym scenariuszu ``only_in_intended`` = wszystkie klucze +lokalne. Krok 5 (tryb batch, ``pbn_kasuj_dyscypliny_selektywnie=False``) +pozostaje bez zmian — po ``delete_all`` PBN jest puste, więc POST wysyła +pełen zestaw BPP (wipe+rewrite). diff --git a/src/pbn_api/client/publication_sync.py b/src/pbn_api/client/publication_sync.py index d5cfd1694..0cf5f1607 100644 --- a/src/pbn_api/client/publication_sync.py +++ b/src/pbn_api/client/publication_sync.py @@ -448,20 +448,109 @@ def _delete_statements_batch(self, objectId, publication_pk, max_tries=3): self._report_statements_failure_and_raise(publication_pk, objectId, last_error) - def _post_statements_with_retry(self, rec, objectId, publication_pk, max_tries=3): + @staticmethod + def _convert_stmt_for_api(stmt): + """Konwersja statement z formatu ``pbn_get_json_statements`` do formatu + akceptowanego przez ``POST /api/v2/institution-profile/statements``. + + Skopiowane z ``WydawnictwoPBNAdapter.pbn_get_api_statements._convert_stmt`` + (``src/pbn_api/adapters/wydawnictwo.py:201-210``). Gdy zmieni się tam + format, zmień też tutaj. + """ + stmt = dict(stmt) # shallow copy — nie modyfikujemy oryginalnego + if "disciplineId" in stmt and "disciplineUuid" in stmt: + del stmt["disciplineId"] + if "type" in stmt: + stmt["personRole"] = stmt.pop("type") + stmt.pop("personNaturalId", None) + return stmt + + def _build_post_statements_payload(self, rec, filter_keys=None): + """Buduje payload dla ``POST /api/v2/institution-profile/statements``. + + Gdy ``filter_keys`` jest ``None``, zwraca pełen payload z + ``WydawnictwoPBNAdapter.pbn_get_api_statements()`` (wszystkie + lokalne statements w formacie zgodnym z endpointem). + + Gdy ``filter_keys`` to set tupli + ``(personObjectId_str, disciplineId_str)``: + + - ``publicationUuid`` bierzemy z wynikowego ``pbn_get_api_statements`` + (to również wymusza wywołanie ``get_pbn_uuid`` w adapterze — + jeśli nie ma V2 lokalnie, rzuci ``DaneLokalneWymagajaAktualizacjiException``). + - Statements bierzemy z surowego ``pbn_get_json_statements()`` (format + przed konwersją, zawiera ``disciplineId`` używany jako część klucza + porównania). Filtrujemy po ``_statement_key_intended`` i przepuszczamy + każdy przez ``_convert_stmt_for_api``. + + Zwraca ``None`` gdy zestaw po filtrowaniu jest pusty (brak sensu + POST-ować pustą listę). + """ + adapter = WydawnictwoPBNAdapter(rec) + + # Zawsze wywołujemy pbn_get_api_statements — daje publicationUuid + # i pełen zestaw dla trybu bez-filtra. Może rzucić + # DaneLokalneWymagajaAktualizacjiException — propaguje do callera. + full_payload = adapter.pbn_get_api_statements() + + if filter_keys is None: + # Pełen zestaw (tryb batch — po delete_all POST-ujemy wszystko). + return full_payload + + if not filter_keys: + return None + + # Filtrowanie po kluczu ``(personObjectId, disciplineId)``. + # Klucz wymaga surowego disciplineId (``pbn_get_api_statements`` + # usuwa disciplineId gdy jest disciplineUuid, więc nie da się + # filtrować po full_payload). + filtered = [ + self._convert_stmt_for_api(s) + for s in adapter.pbn_get_json_statements() + if self._statement_key_intended(s) in filter_keys + ] + if not filtered: + return None + return { + "publicationUuid": full_payload["publicationUuid"], + "statements": filtered, + } + + def _post_statements_with_retry( + self, rec, objectId, publication_pk, filter_keys=None, max_tries=3 + ): """POST oświadczeń publikacji do ``/api/v2/institution-profile/statements``. - Wymaga lokalnego ``PublikacjaInstytucji_V2`` (``pbn_get_api_statements`` - rzuca ``DaneLokalneWymagajaAktualizacjiException`` gdy brak) — - sync_publication wywołuje ``pobierz_publikacje_instytucji_v2`` przed - tym helperem, więc V2 powinno istnieć. + Args: + rec: rekord BPP (Wydawnictwo_Ciagle/Wydawnictwo_Zwarte). + objectId: PBN UID publikacji (do logowania błędów). + publication_pk: PK rekordu BPP (do logowania błędów). + filter_keys: Optional[set] zestaw kluczy + ``(personObjectId_str, disciplineId_str)`` — gdy podany, + POST-ujemy tylko te statements których klucz jest w zestawie + (używane w krokach 3/4b algorytmu — wysyłamy tylko brakujące + w PBN, nie dublujemy istniejących). Gdy ``None`` — POST-ujemy + pełen zestaw lokalnych (używane w trybie batch). + max_tries: liczba prób retry (default 3). + + Wymaga lokalnego ``PublikacjaInstytucji_V2`` (wywołanie + ``get_pbn_uuid`` rzuca ``DaneLokalneWymagajaAktualizacjiException`` + gdy brak) — ``sync_publication`` wywołuje ``pobierz_publikacje_instytucji_v2`` + przed tym helperem, więc V2 powinno istnieć. Retry z exponential backoff. Po wyczerpaniu: rollbar + raise ``StatementsResendFailedException``. + + Gdy ``filter_keys`` jest pustym setem albo po filtrowaniu zestaw + jest pusty — metoda nie wykonuje POST-a (brak czego wysłać). """ - # Może rzucić DaneLokalneWymagajaAktualizacjiException — propaguje - # do callera (sync_publication), który loguje warning zamiast crash. - payload = WydawnictwoPBNAdapter(rec).pbn_get_api_statements() + # _build_post_statements_payload może rzucić + # DaneLokalneWymagajaAktualizacjiException — propaguje do callera + # (sync_publication), który loguje warning zamiast crash. + payload = self._build_post_statements_payload(rec, filter_keys=filter_keys) + if payload is None: + return # nic do wysłania + body = {"data": [payload]} last_error = None @@ -544,7 +633,24 @@ def _sync_statements_with_pbn( pass if only_in_intended: - self._post_statements_with_retry(rec, objectId, publication_pk) + if kasuj_selektywnie: + # Selective (kroki 3 i 4b algorytmu): wyślij TYLKO oświadczenia + # brakujące w PBN (``only_in_intended``). Nie dublujemy już + # istniejących — zakładamy że API PBN może sobie z duplikatami + # nie radzić (idempotentność nie jest gwarantowana). Ten sam + # filter działa dla obu scenariuszy (PBN puste vs PBN+BPP + # różnią się), bo w obu ``only_in_intended`` reprezentuje + # dokładnie "co trzeba dodać do PBN". + self._post_statements_with_retry( + rec, + objectId, + publication_pk, + filter_keys=only_in_intended, + ) + else: + # Batch: po ``delete_all`` PBN jest puste, więc POST-ujemy + # pełen zestaw lokalny (wipe+rewrite). Bez filtra. + self._post_statements_with_retry(rec, objectId, publication_pk) if notificator is not None: notificator.info( diff --git a/src/pbn_api/tests/test_client_sync.py b/src/pbn_api/tests/test_client_sync.py index 5cc47631c..f3853f88a 100644 --- a/src/pbn_api/tests/test_client_sync.py +++ b/src/pbn_api/tests/test_client_sync.py @@ -482,6 +482,215 @@ def test_sync_statements_roznice_selektywnie( assert url_delete in pbn_client.transport.input_values assert PBN_POST_INSTITUTION_STATEMENTS_URL in pbn_client.transport.input_values + # W selektywnym trybie POST wysyła TYLKO oświadczenia brakujące w PBN + # (only_in_intended) — nie pełen zestaw BPP. Sprawdzamy że payload + # zawiera dokładnie jeden statement (autor, dyscyplina 100). + post_body = pbn_client.transport.input_values[PBN_POST_INSTITUTION_STATEMENTS_URL][ + "body" + ] + statements_sent = post_body["data"][0]["statements"] + assert len(statements_sent) == 1 + assert statements_sent[0]["personObjectId"] == pbn_autor.pbn_uid_id + + +@pytest.mark.django_db +def test_sync_statements_pbn_subset_bpp_superset_tylko_brakujace( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + pbn_jednostka, + pbn_uczelnia, + monkeypatch, +): + """PBN = {(A, 301)}, BPP = {(A, 301), (B, 200)} — selektywny tryb. + + ``only_in_pbn`` = ∅ → brak DELETE. + ``only_in_intended`` = {(B, 200)} → POST tylko (B, 200). + + Weryfikacja że POST nie dubluje (A, 301) który już jest w PBN — + wysyłamy TYLKO brakujący (B, 200) zgodnie z algorytmem kroku 4b. + """ + pbn_uczelnia.pbn_kasuj_dyscypliny_selektywnie = True + pbn_uczelnia.save() + + autor_b_pbn_uid = "autor-B-mongo-id-xxxxxxxxxxxx" + + # Intencja BPP: autor A z dyscypliną 301 + autor B z dyscypliną 200 + _patch_intended_statements( + monkeypatch, + [ + { + "personObjectId": pbn_autor.pbn_uid_id, + "disciplineId": 301, + "disciplineUuid": "uuid-301", + "type": "AUTHOR", + }, + { + "personObjectId": autor_b_pbn_uid, + "disciplineId": 200, + "disciplineUuid": "uuid-200", + "type": "AUTHOR", + }, + ], + ) + # PBN ma tylko autora A z dyscypliną 301 (BPP jest supersetem) + _setup_common_mocks( + pbn_client, + pbn_publication.pk, + pbn_statements=[ + { + "id": "aaa", + "personId": pbn_autor.pbn_uid_id, + "area": "301", + "type": "AUTHOR", + "institutionId": pbn_jednostka.pbn_uid_id, + } + ], + ) + pbn_client.transport.return_values[PBN_POST_INSTITUTION_STATEMENTS_URL] = { + "data": [] + } + + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) + + # DELETE nie wywołany — (A, 301) jest w obu, only_in_pbn puste + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk + ) + assert url_delete not in pbn_client.transport.input_values + + # POST wysłany tylko dla (B, 200) — nie dublujemy (A, 301) + assert PBN_POST_INSTITUTION_STATEMENTS_URL in pbn_client.transport.input_values + post_body = pbn_client.transport.input_values[PBN_POST_INSTITUTION_STATEMENTS_URL][ + "body" + ] + statements_sent = post_body["data"][0]["statements"] + assert len(statements_sent) == 1 + assert statements_sent[0]["personObjectId"] == autor_b_pbn_uid + + +@pytest.mark.django_db +def test_sync_statements_pbn_puste_wysyla_wszystkie_w_selektywnym( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + pbn_uczelnia, + monkeypatch, +): + """Krok 3 algorytmu: PBN puste + BPP ma N oświadczeń → POST zawiera N. + + W selektywnym trybie filter_keys = only_in_intended, które w tym + scenariuszu = wszystkie klucze BPP (bo PBN jest pusty), więc POST + wysyła kompletny zestaw BPP — równoważne z "wyślij wszystkie". + """ + pbn_uczelnia.pbn_kasuj_dyscypliny_selektywnie = True + pbn_uczelnia.save() + + autor_b_pbn_uid = "autor-B-mongo-id-yyyyyyyyyyyy" + _patch_intended_statements( + monkeypatch, + [ + { + "personObjectId": pbn_autor.pbn_uid_id, + "disciplineId": 301, + "disciplineUuid": "uuid-301", + "type": "AUTHOR", + }, + { + "personObjectId": autor_b_pbn_uid, + "disciplineId": 200, + "disciplineUuid": "uuid-200", + "type": "AUTHOR", + }, + ], + ) + _setup_common_mocks(pbn_client, pbn_publication.pk, pbn_statements=[]) + pbn_client.transport.return_values[PBN_POST_INSTITUTION_STATEMENTS_URL] = { + "data": [] + } + + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) + + assert PBN_POST_INSTITUTION_STATEMENTS_URL in pbn_client.transport.input_values + post_body = pbn_client.transport.input_values[PBN_POST_INSTITUTION_STATEMENTS_URL][ + "body" + ] + statements_sent = post_body["data"][0]["statements"] + assert len(statements_sent) == 2 + person_ids = {s["personObjectId"] for s in statements_sent} + assert person_ids == {pbn_autor.pbn_uid_id, autor_b_pbn_uid} + + +@pytest.mark.django_db +def test_sync_statements_batch_mode_post_wszystkie( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + pbn_autor, + pbn_jednostka, + pbn_uczelnia, + monkeypatch, +): + """Batch mode + różnice: delete_all kasuje całość, POST wysyła wszystkie BPP. + + ``kasuj_selektywnie=False`` — po ``delete_all`` PBN jest puste, więc + mimo że diff dał ``only_in_intended = {nowy}`` i ``only_in_pbn = {stary}``, + POST musi wysłać PEŁNY zestaw BPP (nie tylko only_in_intended), + inaczej po delete_all stare oświadczenia znikną bez odtworzenia. + """ + pbn_uczelnia.pbn_kasuj_dyscypliny_selektywnie = False + pbn_uczelnia.save() + + _patch_intended_statements( + monkeypatch, + [ + { + "personObjectId": pbn_autor.pbn_uid_id, + "disciplineId": 100, + "disciplineUuid": "uuid-100", + "type": "AUTHOR", + } + ], + ) + _setup_common_mocks( + pbn_client, + pbn_publication.pk, + pbn_statements=[ + { + "id": "aaa", + "personId": pbn_autor.pbn_uid_id, + "area": "301", + "type": "AUTHOR", + "institutionId": pbn_jednostka.pbn_uid_id, + } + ], + ) + url_delete = PBN_DELETE_PUBLICATION_STATEMENT.format( + publicationId=pbn_publication.pk + ) + pbn_client.transport.return_values[url_delete] = [] + pbn_client.transport.return_values[PBN_POST_INSTITUTION_STATEMENTS_URL] = { + "data": [] + } + + pbn_client.sync_publication(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina) + + # Batch DELETE wykonany — kasuje wszystko + body_del = pbn_client.transport.input_values[url_delete]["body"] + assert body_del == {"all": True, "statementsOfPersons": []} + + # POST wysłał pełen zestaw BPP (tutaj 1 statement — autor z dyscypliną 100), + # bez filtra ``only_in_intended``. Sprawdzamy że personObjectId i + # disciplineId pasują do intencji BPP (nie do tego co było w PBN). + post_body = pbn_client.transport.input_values[PBN_POST_INSTITUTION_STATEMENTS_URL][ + "body" + ] + statements_sent = post_body["data"][0]["statements"] + assert len(statements_sent) == 1 + assert statements_sent[0]["personObjectId"] == pbn_autor.pbn_uid_id + # ============================================================ # Error handling: retry + rollbar + StatementsResendFailedException From c8d91a0d0421234abdbab20f1269da184f3c17b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Mon, 27 Apr 2026 17:40:31 +0200 Subject: [PATCH 19/33] =?UTF-8?q?feat(docker):=20branch=20alias=20r=C3=B3w?= =?UTF-8?q?nie=C5=BC=20w=20stopce=20dev=20build=C3=B3w?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) (cherry picked from commit b2837d623edd7d09448212ace96baf0f13176bc3) --- .github/workflows/build-docker-images.yml | 5 +++ docker-bake.hcl | 7 +++++ docker/bpp_base/Dockerfile | 6 +++- src/bpp/templatetags/bpp_version.py | 17 ++++++++++ .../test_templatetags/test_bpp_version.py | 31 ++++++++++++++++++- src/django_bpp/templates/base_footer.html | 2 +- 6 files changed, 65 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-docker-images.yml b/.github/workflows/build-docker-images.yml index 2541f9840..4084b79e7 100644 --- a/.github/workflows/build-docker-images.yml +++ b/.github/workflows/build-docker-images.yml @@ -264,6 +264,10 @@ jobs: # ENV, żeby stopka dla dev buildów wyświetlała "119-merge" obok # commit SHA. Wartość pochodzi ze stepa `tag` powyżej. BPP_IMAGE_TAG: ${{ steps.tag.outputs.final_tag }} + # Alias nazwy brancha (np. "feature-nowe-zglos-publikacje") — + # ustawiany przez step `tag` tylko dla pull_request eventów, + # pusty inaczej. Stopka pokaże ten alias obok PR#-merge. + BPP_BRANCH_TAG: ${{ steps.tag.outputs.branch_tag }} run: | docker buildx bake \ base appserver workerserver \ @@ -272,6 +276,7 @@ jobs: --set "base.args.GIT_SHA=${GIT_SHA}" \ --set "base.args.BPP_BUILD_FLAVOR=${BPP_BUILD_FLAVOR}" \ --set "base.args.BPP_IMAGE_TAG=${BPP_IMAGE_TAG}" \ + --set "base.args.BPP_BRANCH_TAG=${BPP_BRANCH_TAG}" \ --set '*.platform=linux/amd64' \ --allow=fs.read=/tmp \ --allow=fs.write=/tmp diff --git a/docker-bake.hcl b/docker-bake.hcl index 53af8aae6..8246c28e3 100644 --- a/docker-bake.hcl +++ b/docker-bake.hcl @@ -43,6 +43,12 @@ variable "BPP_IMAGE_TAG" { default = "unknown" } +# Alias = nazwa source brancha PR-a (sanityzowana). Empty dla master/non-PR +# pushy. Workflow przekazuje z steps.tag.outputs.branch_tag. +variable "BPP_BRANCH_TAG" { + default = "" +} + variable "TAG_LATEST" { default = "true" } @@ -84,6 +90,7 @@ target "base" { GIT_SHA = GIT_SHA BPP_BUILD_FLAVOR = BPP_BUILD_FLAVOR BPP_IMAGE_TAG = BPP_IMAGE_TAG + BPP_BRANCH_TAG = BPP_BRANCH_TAG } tags = TAG_LATEST == "true" ? [ "iplweb/bpp_base:${DOCKER_VERSION}", diff --git a/docker/bpp_base/Dockerfile b/docker/bpp_base/Dockerfile index 1b4d4ab74..60d5303b9 100644 --- a/docker/bpp_base/Dockerfile +++ b/docker/bpp_base/Dockerfile @@ -371,6 +371,9 @@ FROM python:${PYTHON_VERSION}-slim-${DEBIAN_VERSION} AS runtime ARG GIT_SHA=unknown ARG BPP_BUILD_FLAVOR=dev ARG BPP_IMAGE_TAG=unknown +# Sanityzowana nazwa source brancha (alias) — workflow ustawia tylko dla +# pull_request eventów. Dla master/feature push pozostaje pusty. +ARG BPP_BRANCH_TAG= RUN sed -i s/http/https/g /etc/apt/sources.list.d/debian.sources @@ -407,7 +410,8 @@ ENV PATH="/app/.venv/bin:$PATH" \ STATIC_ROOT=/app/staticroot \ BPP_GIT_SHA=${GIT_SHA} \ BPP_BUILD_FLAVOR=${BPP_BUILD_FLAVOR} \ - BPP_IMAGE_TAG=${BPP_IMAGE_TAG} + BPP_IMAGE_TAG=${BPP_IMAGE_TAG} \ + BPP_BRANCH_TAG=${BPP_BRANCH_TAG} WORKDIR /app diff --git a/src/bpp/templatetags/bpp_version.py b/src/bpp/templatetags/bpp_version.py index 3b4e75bfa..b5055998e 100644 --- a/src/bpp/templatetags/bpp_version.py +++ b/src/bpp/templatetags/bpp_version.py @@ -54,3 +54,20 @@ def bpp_image_tag(): if not tag or tag == "unknown": return "" return tag + + +@register.simple_tag +def bpp_branch_tag(): + """Alias = sanityzowana nazwa source brancha PR-a (np. + "feature-nowe-zglos-publikacje"). + + Workflow ustawia tylko dla pull_request eventów; dla master/feature + push bez PR pozostaje pusty (final_tag = nazwa brancha, więc + duplikacja byłaby zbędna). Master release zwraca pusty string. + """ + if _is_release(): + return "" + tag = os.environ.get("BPP_BRANCH_TAG", "") + if not tag: + return "" + return tag diff --git a/src/bpp/tests/test_templatetags/test_bpp_version.py b/src/bpp/tests/test_templatetags/test_bpp_version.py index 093c470da..f58e40488 100644 --- a/src/bpp/tests/test_templatetags/test_bpp_version.py +++ b/src/bpp/tests/test_templatetags/test_bpp_version.py @@ -1,4 +1,8 @@ -from bpp.templatetags.bpp_version import bpp_git_sha_short, bpp_image_tag +from bpp.templatetags.bpp_version import ( + bpp_branch_tag, + bpp_git_sha_short, + bpp_image_tag, +) def test_bpp_git_sha_short_zwraca_pusty_gdy_brak_env(monkeypatch): @@ -61,3 +65,28 @@ def test_bpp_image_tag_zachowuje_pelny_tag_brancha(monkeypatch): monkeypatch.setenv("BPP_IMAGE_TAG", "feature-nowe-zglos-publikacje") monkeypatch.setenv("BPP_BUILD_FLAVOR", "dev") assert bpp_image_tag() == "feature-nowe-zglos-publikacje" + + +def test_bpp_branch_tag_zwraca_pusty_gdy_brak_env(monkeypatch): + monkeypatch.delenv("BPP_BRANCH_TAG", raising=False) + monkeypatch.delenv("BPP_BUILD_FLAVOR", raising=False) + assert bpp_branch_tag() == "" + + +def test_bpp_branch_tag_zwraca_pusty_dla_release(monkeypatch): + monkeypatch.setenv("BPP_BRANCH_TAG", "feature-foo") + monkeypatch.setenv("BPP_BUILD_FLAVOR", "release") + assert bpp_branch_tag() == "" + + +def test_bpp_branch_tag_zwraca_alias_dla_dev(monkeypatch): + monkeypatch.setenv("BPP_BRANCH_TAG", "feature-nowe-zglos-publikacje") + monkeypatch.setenv("BPP_BUILD_FLAVOR", "dev") + assert bpp_branch_tag() == "feature-nowe-zglos-publikacje" + + +def test_bpp_branch_tag_pusty_string_traktowany_jak_brak(monkeypatch): + """Workflow przekazuje empty string dla master/non-PR — nie pokazuj.""" + monkeypatch.setenv("BPP_BRANCH_TAG", "") + monkeypatch.setenv("BPP_BUILD_FLAVOR", "dev") + assert bpp_branch_tag() == "" diff --git a/src/django_bpp/templates/base_footer.html b/src/django_bpp/templates/base_footer.html index 59dd204f7..b58529dd8 100644 --- a/src/django_bpp/templates/base_footer.html +++ b/src/django_bpp/templates/base_footer.html @@ -13,7 +13,7 @@ zalogowany/a jako {{ request.user.username }} - {% endif %} Oprogramowanie Bibliografia Publikacji Pracowników © - 2004-2026 iplweb; wersja {% load bpp_version %}{% bpp_version %}{% bpp_image_tag as img_tag %}{% bpp_git_sha_short as git_sha %}{% if img_tag or git_sha %} ({{ img_tag }}{% if img_tag and git_sha %}, {% endif %}{% if git_sha %}commit {{ git_sha }}{% endif %}){% endif %} + 2004-2026 iplweb; wersja {% load bpp_version %}{% bpp_version %}{% bpp_image_tag as img_tag %}{% bpp_branch_tag as branch_tag %}{% bpp_git_sha_short as git_sha %}{% if img_tag or branch_tag or git_sha %} ({{ img_tag }}{% if img_tag and branch_tag %}, {% endif %}{{ branch_tag }}{% if img_tag or branch_tag %}{% if git_sha %}, {% endif %}{% endif %}{% if git_sha %}commit {{ git_sha }}{% endif %}){% endif %}