diff --git a/docs/superpowers/HANDOFF-soft-delete-faza-06.md b/docs/superpowers/HANDOFF-soft-delete-faza-06.md new file mode 100644 index 000000000..145c8387c --- /dev/null +++ b/docs/superpowers/HANDOFF-soft-delete-faza-06.md @@ -0,0 +1,218 @@ +# Handoff: soft-delete, start fazy 06 + +> Po zamknięciu **fazy 05a** (wycofanie z PBN), 2026-08-12. +> Czytaj to zamiast odtwarzania historii z gita. + +--- + +## 1. Gdzie jesteśmy + +| | | +|---|---| +| Stan fazy 05a | gałąź `feat/soft-delete-05`, PR do `feat/soft-delete-04` (stacked) | +| Punkt startowy fazy 06 | `feat/soft-delete-05` | +| Migracje fazy 05a | `pbn_export_queue/0011` (pole `operacja`), `pbn_api/0080` (`SentData.withdrawn_at`), `zglos_publikacje/0028` (state-only, dług fazy 04) | +| **Zakres rozdzielony** | nagrobki OAI-PMH/CERIF/REST **wyszły do fazy 05b** (decyzja właściciela 2026-08-10) | + +⚠️ **Faza 05 była w planie JEDNĄ fazą o dwóch niezależnych podsystemach.** +Wycofanie z PBN miało drobiazgowy plan (1286 linii); nagrobki miały baner +zakresu i **zero tasków**. Rozdzielone — patrz §5. + +⚠️ **Baseline (`baseline-sql/`) nadal NIEŚWIEŻY** — stoi na `bpp/0487`. +Zgodnie z CLAUDE.md odświeżenie robi się **raz, przy scalaniu** całego +`feat/soft-delete` do `dev`. Doszły trzy migracje, więc będzie ich więcej +do nadgonienia. + +⚠️ **CI NIE URUCHAMIA SIĘ na PR-ach do gałęzi `feat/soft-delete*`** — bez +zmian względem faz 02–05. Jedyną weryfikacją jest przebieg lokalny. + +--- + +## 2. Co faza 05a dostarcza fazie 06 + +Kontrakt **PINNED** — faza 06 woła to z receiverów sygnałów: + +```python +from pbn_export_queue.operacje import zakolejkuj_wycofanie, zakolejkuj_wysylke +``` + +| Element | Gdzie | Uwaga | +|---|---|---| +| `wycofaj_oswiadczenia(publikacja, client, uczelnia=None)` | `pbn_api/wycofanie.py` | JEDYNE miejsce wołające `delete_all_publication_statements` w kontekście soft-delete | +| `StatusWycofania` / `WynikWycofania` | `pbn_api/wycofanie.py` | `WYCOFANO` / `BRAK_OSWIADCZEN` / `POMINIETO` | +| `PBN_Export_Queue.Operacja` | `pbn_export_queue/models.py` | `WYSYLKA` (default) / `WYCOFANIE` | +| `zakolejkuj_wycofanie` / `zakolejkuj_wysylke` | `pbn_export_queue/operacje.py` | **funkcje modułowe**, nie metody managera | +| `pobierz_konto_techniczne()` / `NAZWA_KONTA_TECHNICZNEGO` | `pbn_export_queue/operacje.py` | `zamowil` dla operacji systemowych | +| `SentData.withdrawn_at` + `mark_as_withdrawn` | `pbn_api/models/sentdata.py` | per-uczelnia; `mark_as_successful` zeruje | + +⇒ **Faza 06 POMIJA swój Task 5 (shim `operacje.py`)** — te funkcje już +istnieją pod ścieżką, której szuka jej check. Shim z planu 06 tworzył wpisy +gołym `PBN_Export_Queue.objects.create(...)`, więc omijałby +`sprobuj_utowrzyc_wpis` (TOCTOU, `uczelnia`, `operacja`). + +### Rozstrzygnięcia, których plan 06 może się nie spodziewać + +- **`zakolejkuj_wysylke` NIE ma gate'u na `pbn_uid`**, choć kontrakt PINNED + planu 06 pisał „None gdy brak pbn_uid" dla OBU funkcji. Rekord, który + nigdy nie poszedł do PBN, po przywróceniu i tak ma prawo pojechać — + wysyłka dopiero nadaje PBN UID. Docstring mówi to wprost, żeby nikt nie + „naprawił" tego pod opis z planu 06. +- **`zamowil` to konto techniczne, nie `None`.** Dług, który spec §4.2 + i plany 05/06 przerzucały między sobą, jest zamknięty. + +--- + +## 3. ⚠️ DWA BLOKERY, których plan fazy 05 nie znał (i czego uczą) + +Oba miały **jedną przyczynę**: `check_if_record_still_exists()` +(`pbn_export_queue/models.py:170`) nie wiedział, PO CO pytamy. Faza 02 +dopisała do niego (commit `2e3b38611`): + +```python +if getattr(obiekt, "deleted_at", None) is not None: + return False +``` + +a wycofanie zlecamy **wyłącznie dla rekordów w koszu**. + +| | Objaw | Wykrywalność | +|---|---|---| +| **#1** `send_to_pbn()` woła guard przed `_zajmij_atomowo()` | gałąź WYCOFANIE = martwy kod, wpis kończy `FINISHED_ERROR` | jest ślad w bazie | +| **#2** `kolejka_wyczysc_wpisy_bez_rekordow()` (`tasks.py:105`) ma ten sam guard za kryterium | **kasuje zlecenie wycofania**, oświadczenia zostają w PBN | **brak śladu** | + +**Rozwiązanie:** guard świadomy operacji — odrzucenie soft-deleted +obowiązuje tylko dla `WYSYLKA`. Jedna zmiana, oba blokery, bo sprzątaczka +woła tę samą metodę na instancji. Rekord skasowany **twardo** nadal daje +`False` dla obu operacji (bez wiersza nie ma `pbn_uid`). + +Mutacja potwierdzająca (cofnięcie warunku) wywala oba testy naraz. + +**Nauka na przyszłe fazy:** predykat, który skleja „czy wiersz istnieje?" +(fakt o bazie) z „czy operator tego nie usunął?" (polityka), rozsypie się +przy wprowadzeniu drugiej operacji — i to w tylu miejscach, ilu ma +konsumentów. Ten miał trzech. + +--- + +## 4. Miny w testach rozbrojone przy okazji (NIE były długiem fazy 05) + +### 4.1 Testy e2e migracji wracały na zaszyty numer + +`pbn_api/tests/test_migracja_{dyscypliny_uuid,publikacja_instytucji}_e2e.py` +cofały bazę `MigrationExecutor`-em i przywracały ją do **stałej** +(`0077` / `0079`). Cofnięcie odapplikowuje wszystko powyżej, więc powrót do +stałej zostawiał bazę o tyle migracji w tyle, ile ich przybyło. + +Objaw po dodaniu `pbn_api/0080`: Playwrightowy test admina `SentData` padał +`UndefinedColumn` w miejscu bez związku z przyczyną. Reprodukcja +deterministyczna na dwóch plikach. + +Poprawka: `przywroc_czubek_migracji()` w `pbn_api/tests/migracje_e2e_utils.py` +— `leaf_nodes()` **całego grafu**. + +⚠️ **Zakres ma znaczenie i pierwsza wersja poprawki była za wąska.** +Przywracanie samego `pbn_api` zostawiało `pbn_integrator` bez tabel, bo +`pbn_integrator/0002` **zależy od `pbn_api/0079`**, a Django odapplikowuje +migracje zależne z innych aplikacji razem z tą, do której się cofamy. + +### 4.2 Faza 04 pominęła czwartego dziedzica abstraktu + +`makemigrations --check` był czerwony. Faza 04 przestawiła `autor` na +`PROTECT` w `BazaModeluOdpowiedzialnosciAutorow`, ale `bpp/0501` objęła +tylko modele z aplikacji `bpp`. **`zglos_publikacje.Zgloszenie_Publikacji_Autor` +dziedziczy ten sam abstrakt** — handoff fazy 04 mówił „dziedziczą 3 modele", +dziedziczy czwarty. + +Ochrona **działała** (on_delete żyje w Pythonie); brakowało księgowości +stanu. Poprawka: `zglos_publikacje/0028`, state-only, wzorzec `bpp/0501`. + +**Nauka:** zmiana `on_delete` na modelu ABSTRAKCYJNYM rozlewa się na +wszystkie aplikacje, które go dziedziczą, a `makemigrations` zgłasza to +per-aplikacja. Szukanie dziedziczących tylko w `bpp/` nie wystarcza. + +--- + +## 5. Faza 05b — nagrobki (przed fazą 07, nie przed 06) + +Wyszła z fazy 05 decyzją właściciela 2026-08-10. **Nie ma jeszcze specu ani +planu** — potrzebuje własnego cyklu brainstorming → spec → plan → PR. + +Punkt startowy rozpoznany: + +- `src/cerif_export/const.py:115` → **`DELETED_RECORD = "no"`**. To nie jest + „brak funkcji", to **obietnica w `Identify`**: harvester ma prawo nie pytać + przyrostowo o usunięcia. Zmiana na `persistent`/`transient` to zmiana + kontraktu, nie dopisanie atrybutu. +- Emisja nagłówka: `oai/czasowniki.py` (`_naglowek()`), `Identify` w `:198`. +- **Architektura providerów jest gotowa**: `ProviderEncji.strona()` + (`providers/base.py`) stronicuje keysetem po + `(COALESCE(ostatnio_zmieniony, EPOKA), pk)`, a soft-delete bumpuje + `ostatnio_zmieniony`. Husk wpadłby więc **naturalnie na właściwe miejsce + w kursorze**. Brakuje wyłącznie poszerzenia `queryset()` o kosz i flagi + „to nagrobek" na obiekcie. +- ⚠️ `z_datestampem()` niesie dwie zapisane blizny (`Trunc` do sekundy, + `tzinfo=UTC`) — obie o duplikatach na granicy strony. Nagrobki muszą iść + tą samą ścieżką. +- Modele soft-delete: publikacje (faza 02) + `Autor` (faza 04). Słowniki + (`Zrodlo`, `Konferencja`, `Projekt`, `Jednostka`) — **nie**. + +--- + +## 6. Czego faza 05a NIE domyka (świadomie) + +- **Twarde skasowanie publikacji zostawia oświadczenia w PBN na zawsze.** + Wycofanie czyta `pbn_uid` z rekordu, więc bez wiersza nie ma czego wołać — + kończy się `FINISHED_ERROR` (głośno, ale bezradnie). Właściwe rozwiązanie: + `SoftDeleteLog` fazy 06 niosący `pbn_uid` niezależnie od rekordu. Dziś + teoretyczne, bo guardy faz 02/04 blokują twarde kasowanie publikacji. +- **Konto techniczne nie ma tokenu PBN**, więc systemowe wycofanie kończy się + `FINISHED_ERROR` (`WillNotExportError`, błąd MERYTORYCZNY), dopóki + administrator nie ustawi mu `przedstawiaj_w_pbn_jako` na konto z ważnym + tokenem. To jest **udokumentowane i przetestowane jako GŁOŚNA porażka** — + ale znaczy, że mechanizm nie zadziała „z pudełka" bez tej konfiguracji. + Kandydat do UI/dokumentacji wdrożeniowej. +- **Brak receiverów sygnałów** — to zakres fazy 06. Faza 05a dostarcza + mechanizm i funkcje, NIE podpina ich do `post_soft_delete`/`post_restore`. +- **Wycofanie nie jest widoczne w UI rekordu.** Operator widzi je tylko + w kolejce eksportu (kolumna/filtr `operacja`). + +--- + +## 7. Dług nadal otwarty (z faz 01–04, stan bez zmian) + +| Sprawa | Stan | +|---|---| +| **Kaskada `Jednostka` → `Autor`** | `aktualna_jednostka`/`aktualna_funkcja` nadal `CASCADE`; skasowanie jednostki, w której autorzy nie mają prac, **twardo kasuje tych autorów**. Patrz handoff fazy 05, §3 | +| **`Autor.slug` `unique=True` bezwarunkowo** | husk trzyma slug zarezerwowany; zaboli w fazie 07 | +| **Wycieki ORM (kanarek `xfail(strict=True)`)** | bez zmian | +| **PR upstream `django-easy-audit`** | [#348](https://github.com/soynatan/django-easy-audit/pull/348) | +| **Brak UI dla `ProtectedError`** | w adminie gołe 500; faza 07 | +| **`Autor` nie ma kosza w adminie** | faza 07 | +| **`hard_delete()` na querysecie nie emituje `post_hard_delete`** | faza 06 | +| Pomiar `0492` i narzutu GiST | wciąż nikt nie zmierzył | +| **Strategia wydania** | bramka na fazie 07 | + +--- + +## 8. Proces — co się sprawdziło w fazie 05a + +- **Przegląd planu przed kodowaniem zwrócił się natychmiast.** Plan powstał + 2026-06-04, rewizje 08-06 i 08-07 sprawdzały kolejkę i klienta PBN — żadna + nie sprawdziła, co faza 02 dopisała do guardu. Gdyby Task 05.3 wykonać + „zgodnie z literą", powstałby martwy kod i cicha utrata zleceń. +- **Test, który nie odtwarza scenariusza, nie jest testem.** Test z planu + dla gałęzi WYCOFANIE **nie kasował rekordu**, więc przeszedłby także przed + poprawką guardu. Jedna linia (`wydawnictwo_ciagle.delete()`) zmieniła go + z ozdoby w regresję. +- **Mutacja jako dowód, nie jako rytuał.** Cofnięcie warunku na operacji + wywaliło oba testy blokerów naraz — to potwierdziło empirycznie, że + diagnoza „jedna przyczyna, dwa objawy" była trafna, a nie tylko wiarygodna. +- **Szeroka suita wykryła to, czego wąska nie mogła.** Obie miny z §4 + ujawniły się dopiero w pełnym przebiegu — jedna przez kolejność testów, + druga przez `makemigrations --check`. Wąskie przebiegi per-app były + zielone przez cały czas. +- **Host jest współdzielony.** W trakcie sesji równolegle biegły kontenery + innych gałęzi (`fix-wcag`, `fix-bibtex`, `soft-delete-03`) i load sięgał 6+, + co wywracało start testcontainerów na 120-sekundowym limicie. + `make clean-testcontainers` **ubiłby cudzą pracę** — nie wolno go odpalać + w ciemno. diff --git a/docs/superpowers/plans/2026-06-04-soft-delete-05-pbn-wycofanie.md b/docs/superpowers/plans/2026-06-04-soft-delete-05-pbn-wycofanie.md index 7e436d24d..a73d8468c 100644 --- a/docs/superpowers/plans/2026-06-04-soft-delete-05-pbn-wycofanie.md +++ b/docs/superpowers/plans/2026-06-04-soft-delete-05-pbn-wycofanie.md @@ -24,6 +24,94 @@ > Dopóki kasowanie jest rzadkie, luka w OAI-PMH jest teoretyczna; faza 07 > czyni kasowanie rutynowym i dopiero wtedy zaczyna realnie boleć. +> ✂️ **PODZIAŁ ZAKRESU 2026-08-10 (decyzja właściciela).** Ten plan realizuje +> **wyłącznie wycofanie z PBN** (Taski 05.0–05.9). **Nagrobki wychodzą do +> osobnej fazy 05b** i dostają własny cykl brainstorming → spec → plan → PR. +> +> Powód: to dwa niezależne podsystemy, które łączy tylko motyw („systemy +> zewnętrzne dowiadują się, że coś zniknęło"), a nie wspólny kod. Wycofanie +> z PBN ma gotowy, drobiazgowy plan; nagrobki miały **zero tasków** — sam +> baner rozszerzenia zakresu z 2026-08-08 nigdy nie doczekał się projektu. +> Wrzucenie obu w jeden PR dałoby PR-a nie do przejrzenia. Termin nagrobków +> (przed fazą 07) jest zachowany — nie zwalniamy z nich, tylko rozdzielamy. + +--- + +## ⚠️ REWIZJA 2026-08-10 — DWA BLOKERY, których ten plan nie znał + +> Ten plan powstał **2026-06-04**, a jego rewizje (08-06, 08-07) sprawdzały +> kolejkę i klienta PBN. Żadna nie sprawdziła, **co faza 02 dopisała do +> `check_if_record_still_exists()`** — a to przesądza o wykonalności Taska +> 05.3. Oba blokery mają jedną przyczynę: predykat „czy rekord nadal +> istnieje" nie wie, po co pytamy. + +`src/pbn_export_queue/models.py:190` (commit `2e3b38611`, faza 02, PR #741): + +```python +if getattr(obiekt, "deleted_at", None) is not None: + return False +``` + +Wycofanie oświadczeń zlecamy **wyłącznie dla rekordów, które właśnie trafiły +do kosza**. Ten guard odrzuca więc dokładnie te wpisy, które faza 05 tworzy. + +**Bloker #1 — gałąź WYCOFANIE nigdy się nie wykona.** `send_to_pbn()` +(`:473`) woła guard **przed** `_zajmij_atomowo()` (`:479`), a Task 05.3 każe +wstawić rozgałęzienie *po* `_zajmij_atomowo()`. Każdy wpis `WYCOFANIE` +kończyłby się na `error("Rekord został usunięty nim wysyłka była możliwa.")` +→ `FINISHED_ERROR`, a `withdraw_from_pbn()` byłby martwym kodem. + +**Bloker #2 — sprzątaczka kasuje zlecenia wycofania (CICHY).** +`kolejka_wyczysc_wpisy_bez_rekordow()` (`tasks.py:105`) iteruje po CAŁEJ +kolejce i `delete()`-uje wpisy, dla których guard zwraca `False`. Wpis +`WYCOFANIE` znika z kolejki, oświadczenia zostają w PBN, nie ma po tym +śladu. Wyścig między beatem sprzątającym a workerem, rozstrzygany losowo. +Ten jest gorszy od #1, bo #1 zostawia przynajmniej `FINISHED_ERROR` +z komunikatem. + +**ROZSTRZYGNIĘCIE (Opcja A): guard staje się świadomy operacji.** +Odrzucenie soft-deleted obowiązuje tylko dla `WYSYLKA`: + +```python +if ( + self.operacja != self.Operacja.WYCOFANIE + and getattr(obiekt, "deleted_at", None) is not None +): + return False +``` + +Dlaczego tak, a nie „rozgałęzienie na starcie `send_to_pbn()`": + +- naprawia **oba** blokery jedną zmianą — sprzątaczka woła tę samą metodę na + instancji, więc dziedziczy świadomość operacji **za darmo**; wariant + z wczesnym rozgałęzieniem leczy tylko #1 i zostawia #2 cichym, +- ścieżka `WYSYLKA` nietknięta (default pola to `WYSYLKA`), istniejący + `test_check_if_record_still_exists_with_deleted_record` + (`test_pbn_queue_status.py:97`) zostaje zielony bez zmian, +- kolejność wstawki w `send_to_pbn()` **dokładnie jak w oryginalnym Tasku + 05.3** — wycofanie nadal dziedziczy `_zajmij_atomowo()`, `ilosc_prob` + i ochronę przed dwoma workerami, +- brak duplikacji `_zajmij_atomowo()` i drugiego guardu „rekord zniknął". + +**Świadomie zaakceptowana konsekwencja:** wpis `WYCOFANIE` dla rekordu +skasowanego **twardo** (wiersza nie ma) nadal kończy się `FINISHED_ERROR` — +bez wiersza nie odczytamy `pbn_uid`. Oświadczenia zostają wtedy w PBN. +To poprawna „głośna porażka", ale należy ją odnotować w handoffie fazy 06: +`SoftDeleteLog` i tak ma nieść `pbn_status`, więc to on jest właściwym +miejscem na przechowanie `pbn_uid` niezależnie od rekordu. + +**Odrzucona opcja C:** trzymać `pbn_uid` na samym wpisie kolejki (nowe pole ++ migracja), żeby wycofanie w ogóle nie zależało od rekordu. Przeżyłoby +twarde kasowanie, ale rozjeżdża się z kontraktem, którego oczekuje faza 06, +a twarde kasowanie publikacji jest po fazach 02/04 zablokowane guardami. + +**Zmiany w tym planie wynikające z rewizji:** Task 05.3 dostaje krok +„guard świadomy operacji" **przed** krokiem z rozgałęzieniem; dochodzi +**Task 05.3a** (regresja sprzątaczki). Numery linii w „Stanie zastanym" +dryfnęły o ~10 w górę względem 2026-08-07 — patrz nagłówek tej sekcji. + +--- + > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. TDD: każdy krok najpierw PRAWDZIWY failing test → komenda + FAIL → PRAWDZIWA implementacja → komenda + PASS → commit. **Goal:** Rozszerzyć `pbn_export_queue` o operację `WYCOFANIE` (obok dotychczasowej `WYSYLKA`), tak by soft-delete publikacji mógł asynchronicznie wycofać oświadczenia dyscyplin z profilu instytucji PBN przez `client.delete_all_publication_statements(pbn_uid)`, z retry/locking/błędami jak istniejąca ścieżka wysyłki. Dostarczyć publiczne funkcje zakolejkowujące (`zakolejkuj_wycofanie`, `zakolejkuj_wysylke`) wołane potem z fazy 06, oraz zaktualizować `SentData` po udanym wycofaniu (`submitted_successfully=False` + znacznik `withdrawn_at`), bez kasowania wiersza. @@ -104,9 +192,11 @@ gdy wiersz trzyma inny worker. ⚠️ **Bloku „`wysylke_podjeto`/`ilosc_prob`" wewnątrz `send_to_pbn()` JUŻ NIE MA** — nie ma czego „przenosić" (Task 05.3 dawniej tak kazał). -- **`send_to_pbn()` (`:456`)**, kolejność: `refresh_from_db()` → wczesny zwrot +- **`send_to_pbn()` (`:466` — plan pisał `:456`)**, kolejność: + `refresh_from_db()` → wczesny zwrot `FINISHED_OKAY` gdy `wysylke_zakonczono is not None` → - `check_if_record_still_exists()` → `error(...)` → + **`check_if_record_still_exists()` (`:473` — ⚠️ BLOKER #1, patrz rewizja + 2026-08-10)** → `error(...)` → `if not self._zajmij_atomowo(): return SendStatus.LOCKED_ELSEWHERE` → import + `sprobuj_wyslac_do_pbn_celery(user=self.zamowil.get_pbn_user(), obj=self.rekord_do_wysylki, force_upload=True, **uczelnia=self.uczelnia**)` @@ -124,6 +214,20 @@ operacji** — wycofanie ma z niej korzystać, nie budować własnej drabinki `except`. +⚠️ **`check_if_record_still_exists()` (`:170`) ma DWÓCH konsumentów, nie +jednego.** Poza `send_to_pbn()` woła go `kolejka_wyczysc_wpisy_bez_rekordow()` +(`tasks.py:105`, BLOKER #2) oraz renderer alarmu Rollbara (`tasks.py:301`, +etykieta `` — po zmianie wpis WYCOFANIE pokaże prawdziwe +`str(rekord)`, co jest ulepszeniem: widać, czego dotyczy błąd). Każda zmiana +tego predykatu dotyka wszystkich trzech. + +📌 **GFK rozwiązuje rekord z kosza — sprawdzone w źródle Django, nie +założone.** `GenericForeignKey.__get__` (`contenttypes/fields.py:262`) woła +`ct.get_object_for_this_type()`, a ta (`contenttypes/models.py:179`) idzie +przez `_base_manager`, który faza 04 przypięła jako **niefiltrujący** +(handoff fazy 04, §4). Dlatego `self.rekord_do_wysylki` w gałęzi wycofania +zwróci soft-skasowaną publikację razem z jej `pbn_uid_id`. + `src/pbn_export_queue/tasks.py`: - `task_sprobuj_wyslac_do_pbn(pk)` — lock przez `cache.add(LOCK_PREFIX+pk)`, `wait_for_object`, `p.send_to_pbn()`, `match` na `SendStatus` (RETRY_* → @@ -483,6 +587,12 @@ Lock/`ilosc_prob`/`task_sprobuj_wyslac_do_pbn` działają niezmienione wysylke_zakonczono=None, ) + # ⚠️ KLUCZOWE (rewizja 2026-08-10): rekord MUSI być w koszu. + # Wycofania zlecamy wyłącznie dla soft-skasowanych publikacji, więc + # test bez tego kroku NIE odtwarza blokera #1 — przeszedłby także + # przed poprawką guardu i niczego by nie pilnował. + wydawnictwo_ciagle.delete() + mock_client = MagicMock() with patch.object( PBN_Export_Queue, "_pozyskaj_klienta_pbn", return_value=mock_client @@ -573,10 +683,31 @@ Lock/`ilosc_prob`/`task_sprobuj_wyslac_do_pbn` działają niezmienione (`POMINIETO` też kończy wpis sukcesem — prymityw zwraca wtedy komunikat „rekord nie ma PBN UID". To gate obronny: `zakolejkuj_wycofanie` takich wpisów nie tworzy.) +- [ ] ⚠️ **Implementacja — guard świadomy operacji (BLOKER #1, rewizja + 2026-08-10). ZRÓB TO PRZED ROZGAŁĘZIENIEM** — bez tego gałąź niżej jest + martwym kodem. W `check_if_record_still_exists()` (`models.py:190`) zawęź + warunek odrzucający rekordy z kosza: + ```python + # Dla WYCOFANIA soft-delete to stan OCZEKIWANY, nie powód + # przerwania: wycofujemy oświadczenia z PBN właśnie dlatego, że + # operator usunął rekord. Wiersz w bazie nadal jest (kasowanie + # miękkie to UPDATE deleted_at), więc pbn_uid da się odczytać — + # GenericForeignKey idzie przez _base_manager, który NIE filtruje. + # + # Dla WYSYŁKI przesłanka jest ta sama, a wniosek przeciwny: + # rekordu w koszu nie pchamy do PBN. Stąd warunek na operacji. + if ( + self.operacja != self.Operacja.WYCOFANIE + and getattr(obiekt, "deleted_at", None) is not None + ): + return False + ``` + Twardo skasowany rekord (`ObjectDoesNotExist`) nadal daje `False` dla obu + operacji — bez wiersza nie ma `pbn_uid`, więc nie ma czego wycofać. - [ ] **Implementacja — rozgałęzienie w `send_to_pbn`.** ⚠️ **Niczego nie przenosimy.** Wstaw DOKŁADNIE dwie linie między `_zajmij_atomowo()` - (`models.py:469-472`) a importem `sprobuj_wyslac_do_pbn_celery` - (`models.py:474`): + (`models.py:479-482`) a importem `sprobuj_wyslac_do_pbn_celery` + (`models.py:484`): ```python if not self._zajmij_atomowo(): # Inny worker zdążył zająć ten wiersz (row lock) albo go zakończył. @@ -601,6 +732,82 @@ Lock/`ilosc_prob`/`task_sprobuj_wyslac_do_pbn` działają niezmienione --- +### Task 05.3a — Sprzątaczka nie kasuje zleceń wycofania (BLOKER #2) + +> **Dodane 2026-08-10.** Tego tasku nie było w planie, bo plan nie wiedział +> o drugim konsumencie `check_if_record_still_exists()`. + +`kolejka_wyczysc_wpisy_bez_rekordow()` (`tasks.py:105`) iteruje po CAŁEJ +kolejce i kasuje wpisy, dla których guard zwraca `False`. Przed poprawką +z Taska 05.3 każdy wpis `WYCOFANIE` (rekord z definicji w koszu) padał jej +ofiarą: zlecenie znikało, oświadczenia zostawały w PBN, śladu brak. + +Poprawka guardu z 05.3 rozbraja to **automatycznie** — sprzątaczka woła tę +samą metodę na instancji wpisu. Ten task **nie dokłada implementacji**; +dokłada test, który to przypina, żeby przyszła zmiana guardu nie wskrzesiła +cichej utraty zleceń. + +**Files:** +- Test path: `src/pbn_export_queue/tests/test_operacja_wycofanie.py` + +- [ ] **Test regresyjny — wpis WYCOFANIE przeżywa sprzątaczkę, wpis WYSYLKA + nie.** Jeden test, dwie asercje — bo dowodem jest RÓŻNICA między + operacjami, nie samo przetrwanie: + ```python + @pytest.mark.django_db + def test_sprzataczka_nie_kasuje_zlecen_wycofania( + wydawnictwo_ciagle, wydawnictwo_zwarte, admin_user, uczelnia + ): + """Regresja blokera #2 (rewizja planu 2026-08-10). + + kolejka_wyczysc_wpisy_bez_rekordow() kasuje wpisy, których rekord + „już nie istnieje". Zanim guard poznał operację, soft-delete + publikacji sprawiał, że sprzątaczka kasowała WŁAŚNIE UTWORZONE + zlecenie wycofania — oświadczenia zostawały w PBN, a wyścig + z workerem celery rozstrzygał się losowo. + """ + from pbn_export_queue.tasks import kolejka_wyczysc_wpisy_bez_rekordow + + wycofanie = baker.make( + PBN_Export_Queue, + rekord_do_wysylki=wydawnictwo_ciagle, + zamowil=admin_user, + uczelnia=uczelnia, + operacja=PBN_Export_Queue.Operacja.WYCOFANIE, + wysylke_zakonczono=None, + ) + wysylka = baker.make( + PBN_Export_Queue, + rekord_do_wysylki=wydawnictwo_zwarte, + zamowil=admin_user, + uczelnia=uczelnia, + operacja=PBN_Export_Queue.Operacja.WYSYLKA, + wysylke_zakonczono=None, + ) + + wydawnictwo_ciagle.delete() + wydawnictwo_zwarte.delete() + + kolejka_wyczysc_wpisy_bez_rekordow() + + assert PBN_Export_Queue.objects.filter(pk=wycofanie.pk).exists(), ( + "sprzątaczka skasowała zlecenie WYCOFANIA — oświadczenia " + "zostaną w PBN i nikt się o tym nie dowie" + ) + assert not PBN_Export_Queue.objects.filter(pk=wysylka.pk).exists(), ( + "wpis WYSYLKA rekordu z kosza ma nadal znikać — to zachowanie " + "z fazy 02, którego nie wolno zepsuć przy okazji" + ) + ``` +- [ ] **Komenda + PASS:** `uv run pytest src/pbn_export_queue/tests/test_operacja_wycofanie.py::test_sprzataczka_nie_kasuje_zlecen_wycofania -x` → PASS (poprawka guardu z 05.3 już to załatwia). +- [ ] **Dowód, że test faktycznie pilnuje (mutacja obowiązkowa):** tymczasowo + cofnij warunek `self.operacja != self.Operacja.WYCOFANIE` w + `check_if_record_still_exists()` → test MUSI paść na pierwszej asercji. + Przywróć. Test, który przechodzi także bez poprawki, nie jest regresją. +- [ ] **Lint + Commit** (jawne ścieżki): `git commit -m "test(pbn_export_queue): sprzataczka kolejki nie kasuje zlecen wycofania"` + +--- + ### Task 05.4 — WYSYLKA dalej działa (regresja rozgałęzienia) Upewnij się, że dodanie gałęzi WYCOFANIE nie zmieniło ścieżki WYSYLKA: wpis z domyślną operacją nadal woła `sprobuj_wyslac_do_pbn_celery`, NIE `delete_all_publication_statements`. @@ -1190,7 +1397,40 @@ Rekord bywa wysyłany do PBN bez kolejki — ta sama ścieżka musi umieć wycof `grep -rn --include='*.py' "synchronizuj_publikacje" src/`) - Test: `src/pbn_integrator/tests/test_wycofanie_sync.py` -- [ ] **Krok 05.9.1 — ustal realny zbiór ścieżek synchronicznych.** Nie zgaduj; +> ✅ **USTALONE 2026-08-10 — ta faza NIE dokłada tu kodu. Uzasadnienie niżej.** +> +> `synchronizuj_publikacje` (`pbn_integrator/utils/synchronization.py:180`) +> to **wsadowy uploader**, nie ścieżka kasowania: iteruje publikacje „do +> synchronizacji" i je wysyła. Wołają go dwie komendy CLI +> (`pbn_uploader.py:12`, `pbn_integrator.py:392`) — stan bez zmian względem +> 2026-08-07. +> +> Decyzja #16 była trafna co do FAKTU (rekord bywa wysyłany **poza kolejką**), +> ale wysyłka synchroniczna nie rodzi potrzeby wycofania synchronicznego: +> **wycofanie wyzwala soft-delete, a ten zawsze idzie przez kolejkę** +> (receivery fazy 06 → `zakolejkuj_wycofanie`). Żadna ścieżka wsadowa nie +> kasuje dziś rekordów. +> +> Dodatkowo `wydawnictwa_zwarte_do_synchronizacji()` (`:46-47`) filtruje przez +> `Wydawnictwo_Zwarte.objects`, czyli menedżer, który od fazy 02 **pomija +> kosz** — rekord soft-skasowany po prostu wypada z wsadu. Nie ma tam czego +> podpinać. +> +> **Co ta faza faktycznie dostarcza dla wejścia synchronicznego:** prymityw +> `wycofaj_oswiadczenia(publikacja, client, uczelnia=None)` przyjmuje klienta +> **od wywołującego**, więc jest gotowym kontraktem dla dowolnej przyszłej +> ścieżki poza kolejką. `src/pbn_api/tests/test_wycofanie.py` woła go +> dokładnie w ten sposób (własny klient, bez kolejki) — czyli testuje +> właśnie kształt wejścia synchronicznego. Równoważność obu wejść jest więc +> zagwarantowana konstrukcyjnie: kolejka nie ma własnej implementacji, tylko +> cienki wrapper. +> +> ⚠️ Gdy kiedyś powstanie realna ścieżka synchronicznego kasowania (np. +> komenda „wyczyść rekordy z PBN"), MUSI wołać prymityw, a NIE +> `client.delete_all_publication_statements()` — inaczej `SentData` +> rozjedzie się między wejściami (niezmiennik §4.2 specu). + +- [ ] ~~**Krok 05.9.1 — ustal realny zbiór ścieżek synchronicznych.**~~ Nie zgaduj; wypisz wywołujących i rozstrzygnij, które z nich mogą wystąpić w kontekście soft-delete (management command? admin action? import?). Stan na 2026-08-07 (`grep`): `synchronizuj_publikacje` definiowana w @@ -1224,6 +1464,12 @@ Rekord bywa wysyłany do PBN bez kolejki — ta sama ścieżka musi umieć wycof `pbn_integrator/`. (`pbn_client` jest w site-packages, więc się tu nie pokaże — to poprawne.) + ✅ **Wynik 2026-08-10:** w produkcji doszła DOKŁADNIE jedna linia — + `src/pbn_api/wycofanie.py:68`. Trzy baseline'owe wystąpienia bez zmian. + W `pbn_export_queue/` są 3 trafienia, ale wszystkie w + `tests/test_operacja_wycofanie.py` (asercje na `MagicMock`), zero + w kodzie produkcyjnym. W `pbn_integrator/` — zero. + --- ### Task 05.8 — Pełna weryfikacja fazy + brak driftu migracji @@ -1270,6 +1516,11 @@ Rekord bywa wysyłany do PBN bez kolejki — ta sama ścieżka musi umieć wycof `submitted_successfully=False`, wiersz NIE skasowany — **na wierszu TEJ uczelni**. Restore→WYSYLKA→`mark_as_successful` zeruje `withdrawn_at`. +- **Guard `check_if_record_still_exists()` jest świadomy operacji** — + soft-delete blokuje WYSYŁKĘ, ale nie WYCOFANIE. Faza 06, dokładając + receivery sygnałów, dostaje to gotowe; nie musi omijać sprzątaczki + kolejki ani duplikować `_zajmij_atomowo()`. + ## Otwarte / do zgłoszenia poza tą fazą - **Spec §4.1 i indeks 00 cytują martwą ścieżkę** @@ -1279,6 +1530,22 @@ Rekord bywa wysyłany do PBN bez kolejki — ta sama ścieżka musi umieć wycof `retry w pbn_api/client/publication_sync.py`, gdzie `_delete_statements_with_retry` już nie mieszka. Poprawka specu/indeksu — poza zakresem tego planu (inny właściciel plików). +- **Nagrobki (OAI-PMH / CERIF / REST) wyszły do fazy 05b** — decyzja + właściciela 2026-08-10, patrz baner „PODZIAŁ ZAKRESU" na górze. Termin + (przed fazą 07) bez zmian. Punkt startowy dla 05b: `const.DELETED_RECORD` + = `"no"` w `src/cerif_export/const.py:115` (repozytorium **deklaruje + w `Identify`**, że usunięć nie ogłasza — zmiana tej wartości to zmiana + kontraktu wobec harvesterów, nie tylko dopisanie atrybutu do nagłówka). + Architektura providerów jest gotowa: `ProviderEncji.strona()` stronicuje + keysetem po `(COALESCE(ostatnio_zmieniony, EPOKA), pk)`, a soft-delete + bumpuje `ostatnio_zmieniony` — brakuje tylko poszerzenia `queryset()` + o kosz i flagi „to nagrobek" na obiekcie. +- **Twarde skasowanie publikacji zostawia oświadczenia w PBN na zawsze.** + Wycofanie czyta `pbn_uid` z rekordu, więc bez wiersza nie ma czego wołać + (kończy się `FINISHED_ERROR` — głośno, ale bezradnie). Właściwe + rozwiązanie to `SoftDeleteLog` fazy 06 niosący `pbn_uid` niezależnie od + rekordu. Po fazach 02/04 twarde kasowanie publikacji jest zablokowane + guardami, więc dziś to teoretyczne. - **Plan 06** nie wymaga zmian sygnatur (przypięliśmy jego wariant), ale jego **Task 5 (shim `operacje.py`) jest teraz martwy** i jego uwaga „`zamowil`… **To dług fazy 05**" jest już spełniona przez Task 05.4a — diff --git a/src/bpp/newsfragments/soft-delete-konto-techniczne.bugfix.rst b/src/bpp/newsfragments/soft-delete-konto-techniczne.bugfix.rst new file mode 100644 index 000000000..fbe467e0f --- /dev/null +++ b/src/bpp/newsfragments/soft-delete-konto-techniczne.bugfix.rst @@ -0,0 +1,4 @@ +Systemowe (bez zalogowanego użytkownika) usunięcie publikacji nie powoduje +już cichego pominięcia wycofania oświadczeń z PBN. Wcześniej taka operacja +kończyła się komunikatem „ten rekord jest już w kolejce", mimo że wpis +w kolejce w ogóle nie powstawał. diff --git a/src/bpp/newsfragments/soft-delete-pbn-wycofanie.feature.rst b/src/bpp/newsfragments/soft-delete-pbn-wycofanie.feature.rst new file mode 100644 index 000000000..044ff3126 --- /dev/null +++ b/src/bpp/newsfragments/soft-delete-pbn-wycofanie.feature.rst @@ -0,0 +1,6 @@ +Usunięcie publikacji wysłanej do PBN wycofuje teraz jej oświadczenia +dyscyplin z profilu instytucji; przywrócenie publikacji wysyła je ponownie. +Kolejka eksportu do PBN rozróżnia operację „wysyłka" i „wycofanie +oświadczeń" — widać to w panelu administracyjnym jako osobną kolumnę +i filtr. Sam obiekt publikacji w PBN nie jest kasowany, bo jest +współdzielony między instytucjami. diff --git a/src/pbn_api/migrations/0080_sentdata_withdrawn_at.py b/src/pbn_api/migrations/0080_sentdata_withdrawn_at.py new file mode 100644 index 000000000..1d80f9b50 --- /dev/null +++ b/src/pbn_api/migrations/0080_sentdata_withdrawn_at.py @@ -0,0 +1,23 @@ +# Generated by Django 5.2.16 on 2026-08-10 19:01 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("pbn_api", "0079_constraint_publikacja_instytucji"), + ] + + operations = [ + migrations.AddField( + model_name="sentdata", + name="withdrawn_at", + field=models.DateTimeField( + blank=True, + db_index=True, + help_text="Ustawiane po udanym wycofaniu oświadczeń dyscyplin z PBN (soft-delete publikacji). Zerowane przy ponownej udanej wysyłce.", + null=True, + verbose_name="Data wycofania oświadczeń", + ), + ), + ] diff --git a/src/pbn_api/models/sentdata.py b/src/pbn_api/models/sentdata.py index 1c706eed0..84f259181 100644 --- a/src/pbn_api/models/sentdata.py +++ b/src/pbn_api/models/sentdata.py @@ -92,8 +92,38 @@ def mark_as_successful( sd.pbn_uid_id = pbn_uid_id sd.api_response_status = api_response_status sd.exception = "" + # Symetria wycofania (faza 05 soft-delete): udana wysyłka znaczy, że + # oświadczenia znów są w PBN, więc ślad wycofania przestaje być + # prawdziwy. Czyścimy go DOPIERO tutaj, a nie w + # ``create_or_update_before_upload`` — gdyby wysyłka po restore + # padła, rekord zostałby jednocześnie „niewycofany" i bez + # oświadczeń w PBN, czyli w stanie, którego nikt by nie wykrył. + sd.withdrawn_at = None sd.save() + def mark_as_withdrawn(self, rec, api_response_status="", uczelnia=None): + """Oznacza rekord jako wycofany z PBN (oświadczenia usunięte). + + ``uczelnia`` jest w praktyce obowiązkowa (multi-hosted): wycofanie + dotyczy profilu KONKRETNEJ uczelni, a przy ≥2 wierszach + ``get_for_rec`` bez niej rzuci ``MultipleObjectsReturned``. Default + ``None`` istnieje wyłącznie dla zgodności z konwencją reszty + managera (``uczelnia`` jako ostatni kwarg). + + Wiersza SentData NIE kasujemy — zostaje dla audytu i re-matchingu + przy przywróceniu rekordu. ``submitted_successfully=False``, bo + rekord nie jest już wystawiony w profilu instytucji; samo to pole + nie odróżniłoby jednak „nigdy nie wysłane" od „wysłane i wycofane", + stąd osobny znacznik czasu. + """ + sd = self.get_for_rec(rec, uczelnia) + sd.submitted_successfully = False + sd.withdrawn_at = timezone.now() + if api_response_status: + sd.api_response_status = api_response_status + sd.save() + return sd + def mark_as_failed(self, rec, exception="", api_response_status="", uczelnia=None): """Mark existing record as failed after API call""" sd = self.get_for_rec(rec, uczelnia) @@ -243,6 +273,15 @@ class SentData(LinkDoPBNMixin, models.Model): blank=True, help_text="Kiedy dane zostały wysłane do PBN", ) + + withdrawn_at = models.DateTimeField( + "Data wycofania oświadczeń", + null=True, + blank=True, + db_index=True, + help_text="Ustawiane po udanym wycofaniu oświadczeń dyscyplin z PBN " + "(soft-delete publikacji). Zerowane przy ponownej udanej wysyłce.", + ) api_response_status = models.TextField( "Status odpowiedzi API", blank=True, diff --git a/src/pbn_api/tests/migracje_e2e_utils.py b/src/pbn_api/tests/migracje_e2e_utils.py new file mode 100644 index 000000000..52e1b5db4 --- /dev/null +++ b/src/pbn_api/tests/migracje_e2e_utils.py @@ -0,0 +1,40 @@ +"""Wspólne narzędzia testów e2e migracji ``pbn_api``. + +Testy, które puszczają prawdziwy ``MigrationExecutor`` w tył i w przód, +MUSZĄ posprzątać po sobie — inaczej worker testowy zostaje na starej +migracji i psuje wszystkie kolejne testy w tym procesie. +""" + +from django.db import connection +from django.db.migrations.executor import MigrationExecutor + + +def przywroc_czubek_migracji(): + """Doprowadź bazę workera do NAJNOWSZYCH migracji — WSZYSTKICH aplikacji. + + Dwie rzeczy, które łatwo tu zrobić za wąsko; obie kosztowały przebieg + suity: + + 1. **Czubek grafu, a nie zaszyty numer.** Cofnięcie do stanu „PRZED" + odapplikowuje wszystko powyżej, więc powrót do stałej zostawia bazę + o tyle migracji w tyle, ile ich od napisania testu przybyło. Tak + właśnie było: jeden test e2e wracał na ``0077``, drugi na ``0079``, + więc dodanie ``0080`` (``SentData.withdrawn_at``, faza 05 + soft-delete) wywróciło Playwrightowy test admina ``SentData`` + błędem ``UndefinedColumn``. + + 2. **Wszystkie aplikacje, nie tylko ``pbn_api``.** Migracje innych + aplikacji zależą od ``pbn_api`` (np. + ``pbn_integrator/0002_indeks_content_type_object_id`` wymaga + ``pbn_api/0079``), a Django odapplikowuje zależne migracje RAZEM + z tą, do której się cofamy. Przywracanie samego ``pbn_api`` + zostawiało więc ``pbn_integrator`` bez tabel i sypało + ``ProgrammingError: relacja … nie istnieje`` w ``test_mongodb_ops``. + + ``leaf_nodes()`` bez argumentu zwraca czubki CAŁEGO grafu, więc baza + wraca dokładnie tam, gdzie była przed testem — niezależnie od tego, + ile aplikacji zostało po drodze cofniętych. + """ + executor = MigrationExecutor(connection) + executor.loader.build_graph() + executor.migrate(executor.loader.graph.leaf_nodes()) diff --git a/src/pbn_api/tests/test_migracja_dyscypliny_uuid_e2e.py b/src/pbn_api/tests/test_migracja_dyscypliny_uuid_e2e.py index 2eb6a6be0..a91832ad5 100644 --- a/src/pbn_api/tests/test_migracja_dyscypliny_uuid_e2e.py +++ b/src/pbn_api/tests/test_migracja_dyscypliny_uuid_e2e.py @@ -14,6 +14,8 @@ from django.db import connection from django.db.migrations.executor import MigrationExecutor +from pbn_api.tests.migracje_e2e_utils import przywroc_czubek_migracji + PRZED = ("pbn_api", "0075_sentdata_fee_sent_sentdata_fee_uploaded_okay") PO = ("pbn_api", "0077_constrainty_uuid_dyscyplin") @@ -48,39 +50,42 @@ def _policz(sql, *params): def test_migracja_przechodzi_na_bazie_z_duplikatami(bez_reinstalacji_denorma): uuid_slownika, uuid_dyscypliny = uuid4(), uuid4() - MigrationExecutor(connection).migrate([PRZED]) - _wstaw_duplikaty(uuid_slownika, uuid_dyscypliny) + try: + MigrationExecutor(connection).migrate([PRZED]) + _wstaw_duplikaty(uuid_slownika, uuid_dyscypliny) - assert ( - _policz( - "SELECT count(*) FROM pbn_api_disciplinegroup WHERE uuid=%s", - uuid_slownika, + assert ( + _policz( + "SELECT count(*) FROM pbn_api_disciplinegroup WHERE uuid=%s", + uuid_slownika, + ) + == 2 ) - == 2 - ) - # to jest właściwy asert: migracja NIE wywala się na bazie z duplikatami - executor = MigrationExecutor(connection) - executor.loader.build_graph() - executor.migrate([PO]) + # to jest właściwy asert: migracja NIE wywala się na bazie z duplikatami + executor = MigrationExecutor(connection) + executor.loader.build_graph() + executor.migrate([PO]) - assert ( - _policz( - "SELECT count(*) FROM pbn_api_disciplinegroup WHERE uuid=%s", - uuid_slownika, + assert ( + _policz( + "SELECT count(*) FROM pbn_api_disciplinegroup WHERE uuid=%s", + uuid_slownika, + ) + == 1 ) - == 1 - ) - assert ( - _policz( - "SELECT count(*) FROM pbn_api_discipline WHERE uuid=%s", uuid_dyscypliny + assert ( + _policz( + "SELECT count(*) FROM pbn_api_discipline WHERE uuid=%s", uuid_dyscypliny + ) + == 1 ) - == 1 - ) - assert ( - _policz( - "SELECT count(*) FROM pg_constraint WHERE conname=%s", - "pbn_api_discipline_uuid_unikalny_w_slowniku", + assert ( + _policz( + "SELECT count(*) FROM pg_constraint WHERE conname=%s", + "pbn_api_discipline_uuid_unikalny_w_slowniku", + ) + == 1 ) - == 1 - ) + finally: + przywroc_czubek_migracji() diff --git a/src/pbn_api/tests/test_migracja_publikacja_instytucji_e2e.py b/src/pbn_api/tests/test_migracja_publikacja_instytucji_e2e.py index 6d747c83e..a075e8f34 100644 --- a/src/pbn_api/tests/test_migracja_publikacja_instytucji_e2e.py +++ b/src/pbn_api/tests/test_migracja_publikacja_instytucji_e2e.py @@ -16,6 +16,7 @@ from pbn_api.models import Institution, Publication, Scientist from pbn_api.models.publikacja_instytucji import PublikacjaInstytucji +from pbn_api.tests.migracje_e2e_utils import przywroc_czubek_migracji PRZED = ("pbn_api", "0077_constrainty_uuid_dyscyplin") PO = ("pbn_api", "0079_constraint_publikacja_instytucji") @@ -60,15 +61,23 @@ def test_migracja_przechodzi_na_bazie_z_duplikatami(bez_reinstalacji_denorma): == 1 ) finally: - # Baza MUSI wrócić na docelową migrację (0079) niezależnie od tego, - # czy powyższe asercje przeszły — inaczej worker testowy zostaje na + # Baza MUSI wrócić na NAJNOWSZĄ migrację niezależnie od tego, czy + # powyższe asercje przeszły — inaczej worker testowy zostaje na # 0077 (bez constraintu) i psuje WSZYSTKIE kolejne testy w tym # procesie kaskadą niezrozumiałych błędów zamiast jednego czytelnego # AssertionError z bloku try. # + # ⚠️ Czubek grafu, a NIE zaszyte ``PO``. Cofnięcie do ``PRZED`` + # odapplikowuje wszystko powyżej, więc powrót do stałej zostawia + # bazę o tyle migracji w tyle, ile przybyło ich od napisania testu. + # Zaszyte ``0079`` przestało wystarczać w chwili, gdy faza 05 + # soft-delete dołożyła ``0080`` (``SentData.withdrawn_at``): + # Playwrightowy test admina ``SentData`` zaczął padać na + # ``UndefinedColumn`` w miejscu bez związku z przyczyną. + # # Same dane testowe nie wymagają tu ręcznego kasowania: migracja # 0078 (RunPython dedup) dedupikuje dowolną liczbę pozostawionych # wierszy trójki jako część forward-migrate, więc nie ma osobnego # kroku „usuń dane" przed „odtwórz stan", który mógłby rzucić # wyjątkiem maskującym oryginalny AssertionError z bloku try. - MigrationExecutor(connection).migrate([PO]) + przywroc_czubek_migracji() diff --git a/src/pbn_api/tests/test_sentdata_per_uczelnia.py b/src/pbn_api/tests/test_sentdata_per_uczelnia.py index cb52ba399..72dd2736d 100644 --- a/src/pbn_api/tests/test_sentdata_per_uczelnia.py +++ b/src/pbn_api/tests/test_sentdata_per_uczelnia.py @@ -139,3 +139,33 @@ def test_backfill_logic_multi_install_leaves_null( # NULL-owy wiersz pozostaje nietknięty. assert SentData.objects.filter(uczelnia__isnull=True).count() == 1 + + +@pytest.mark.django_db +def test_mark_as_withdrawn_izolacja(uczelnia, uczelnia2, wydawnictwo_ciagle): + """Wycofanie zleca KONKRETNA uczelnia — nie wolno mu ruszyć drugiej. + + Wycofanie oświadczeń dotyczy profilu instytucji, a nie obiektu + publikacji w PBN (ten jest współdzielony). Gdyby ``mark_as_withdrawn`` + gubiło ``uczelnia``, soft-delete rekordu w U1 skasowałby ślad wysyłki + U2 — a przy dwóch wierszach ``get_for_rec`` bez uczelni rzuca + ``MultipleObjectsReturned``, więc błąd wyszedłby dopiero na produkcji + multi-hosted. + """ + rec = wydawnictwo_ciagle + d = {"type": "ARTICLE", "title": "x"} + + SentData.objects.create_or_update_before_upload(rec, d, uczelnia=uczelnia) + SentData.objects.create_or_update_before_upload(rec, d, uczelnia=uczelnia2) + SentData.objects.mark_as_successful(rec, uczelnia=uczelnia) + SentData.objects.mark_as_successful(rec, uczelnia=uczelnia2) + + SentData.objects.mark_as_withdrawn(rec, uczelnia=uczelnia) + + wycofana = SentData.objects.get_for_rec(rec, uczelnia) + assert wycofana.withdrawn_at is not None + assert wycofana.submitted_successfully is False + + nietknieta = SentData.objects.get_for_rec(rec, uczelnia2) + assert nietknieta.withdrawn_at is None + assert nietknieta.submitted_successfully is True diff --git a/src/pbn_api/tests/test_wycofanie.py b/src/pbn_api/tests/test_wycofanie.py new file mode 100644 index 000000000..ff530a161 --- /dev/null +++ b/src/pbn_api/tests/test_wycofanie.py @@ -0,0 +1,147 @@ +"""Prymityw wycofania oświadczeń dyscyplin z PBN (faza 05 soft-delete). + +Wycofanie ma DWA wejścia: asynchroniczne (kolejka ``pbn_export_queue``) +i synchroniczne (``synchronizuj_publikacje``, poza kolejką). Oba muszą +zostawiać identyczny stan ``SentData`` i identycznie rozumieć, co znaczy +sukces — dlatego logika mieszka w jednej wolnostojącej funkcji, a nie +w metodzie modelu kolejki. + +Podział odpowiedzialności (niezmiennik §4.2 specu): prymityw odpowiada za +semantykę „co znaczy sukces" i za stan ``SentData``; wywołujący — za +klasyfikację wyjątków i politykę ponawiania. Dlatego wyjątki PBN lecą stąd +w górę nietknięte. +""" + +from unittest.mock import MagicMock + +import pytest +from model_bakery import baker + +from pbn_api.exceptions import ( + CannotDeleteStatementsException, + HttpException, + PraceSerwisoweException, +) +from pbn_api.models import Publication, SentData +from pbn_api.wycofanie import ( + StatusWycofania, + wycofaj_oswiadczenia, +) + + +@pytest.fixture +def publikacja_w_pbn(wydawnictwo_ciagle): + """Publikacja, która realnie poszła do PBN (ma nadany PBN UID).""" + wydawnictwo_ciagle.pbn_uid = baker.make(Publication) + wydawnictwo_ciagle.save() + return wydawnictwo_ciagle + + +@pytest.fixture +def sent_data(publikacja_w_pbn, uczelnia): + return SentData.objects.create( + object=publikacja_w_pbn, + data_sent={}, + submitted_successfully=True, + uploaded_okay=True, + uczelnia=uczelnia, + ) + + +@pytest.mark.django_db +def test_wycofanie_wola_klienta_i_oznacza_sentdata( + publikacja_w_pbn, sent_data, uczelnia +): + client = MagicMock() + + wynik = wycofaj_oswiadczenia(publikacja_w_pbn, client, uczelnia=uczelnia) + + assert wynik.status == StatusWycofania.WYCOFANO + client.delete_all_publication_statements.assert_called_once_with( + publikacja_w_pbn.pbn_uid_id + ) + sd = SentData.objects.get_for_rec(publikacja_w_pbn, uczelnia) + assert sd.submitted_successfully is False + assert sd.withdrawn_at is not None + + +@pytest.mark.django_db +def test_brak_oswiadczen_to_sukces(publikacja_w_pbn, sent_data, uczelnia): + """``CannotDeleteStatementsException`` znaczy „nie było czego usuwać". + + To zaległa pułapka: pakiet ``pbn-client`` ponawia na tym wyjątku + w ``_delete_statements_with_retry``, bo tam kasowanie poprzedza WYSYŁKĘ + i brak oświadczeń jest przeszkodą. U nas stan docelowy („w PBN nie ma + oświadczeń tej publikacji") jest wtedy JUŻ osiągnięty — ponawianie + dokładałoby ruchu i kończyło się błędem zamiast sukcesem. + """ + client = MagicMock() + client.delete_all_publication_statements.side_effect = ( + CannotDeleteStatementsException("brak oświadczeń") + ) + + wynik = wycofaj_oswiadczenia(publikacja_w_pbn, client, uczelnia=uczelnia) + + assert wynik.status == StatusWycofania.BRAK_OSWIADCZEN + sd = SentData.objects.get_for_rec(publikacja_w_pbn, uczelnia) + assert sd.submitted_successfully is False + assert sd.withdrawn_at is not None + + +@pytest.mark.django_db +def test_brak_pbn_uid_pomija_bez_wolania_klienta(wydawnictwo_ciagle, uczelnia): + """Bez PBN UID nic do PBN nie poszło — nie ma czego wycofywać.""" + client = MagicMock() + assert wydawnictwo_ciagle.pbn_uid_id is None + + wynik = wycofaj_oswiadczenia(wydawnictwo_ciagle, client, uczelnia=uczelnia) + + assert wynik.status == StatusWycofania.POMINIETO + client.delete_all_publication_statements.assert_not_called() + + +@pytest.mark.django_db +def test_brak_wiersza_sentdata_to_nadal_sukces(publikacja_w_pbn, uczelnia): + """Rekord ma PBN UID, ale nigdy nie było wiersza SentData. + + Zdarza się przy rekordach zaimportowanych z PBN (UID przyszedł + z importu, nie z wysyłki). Oświadczenia i tak trzeba usunąć, a brak + wiersza nie jest błędem — nie ma po prostu czego oznaczać. + """ + assert not SentData.objects.filter(object_id=publikacja_w_pbn.pk).exists() + client = MagicMock() + + wynik = wycofaj_oswiadczenia(publikacja_w_pbn, client, uczelnia=uczelnia) + + assert wynik.status == StatusWycofania.WYCOFANO + client.delete_all_publication_statements.assert_called_once() + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "wyjatek", + [ + PraceSerwisoweException("okno serwisowe"), + HttpException(423, "url", "zasób zablokowany"), + ], + ids=["prace_serwisowe", "http"], +) +def test_wyjatki_pbn_leca_w_gore_bez_dotykania_sentdata( + publikacja_w_pbn, sent_data, uczelnia, wyjatek +): + """Prymityw NIE klasyfikuje wyjątków — robi to wywołujący. + + Gdyby połykał je tutaj, kolejka i ścieżka synchroniczna miałyby dwie + rozjeżdżające się tabele decyzji o ponawianiu. Stan ``SentData`` musi + zostać nietknięty: wycofanie się nie udało, więc oświadczenia nadal + są w PBN i rekord nadal jest „wysłany". + """ + client = MagicMock() + client.delete_all_publication_statements.side_effect = wyjatek + + with pytest.raises(type(wyjatek)): + wycofaj_oswiadczenia(publikacja_w_pbn, client, uczelnia=uczelnia) + + sd = SentData.objects.get_for_rec(publikacja_w_pbn, uczelnia) + assert sd.submitted_successfully is True + assert sd.withdrawn_at is None diff --git a/src/pbn_api/wycofanie.py b/src/pbn_api/wycofanie.py new file mode 100644 index 000000000..8f5a40213 --- /dev/null +++ b/src/pbn_api/wycofanie.py @@ -0,0 +1,89 @@ +"""Wycofanie oświadczeń dyscyplin publikacji z profilu instytucji w PBN. + +Jedno miejsce prawdy dla obu wejść: asynchronicznego (kolejka +``pbn_export_queue``, operacja ``WYCOFANIE``) i synchronicznego (wysyłka +poza kolejką). Dzięki temu stan ``SentData`` i rozumienie sukcesu są +identyczne niezależnie od tego, którędy przyszło zlecenie. +""" + +from dataclasses import dataclass + +from django.db import models + +from pbn_api.exceptions import CannotDeleteStatementsException +from pbn_api.models import SentData + + +class StatusWycofania(models.TextChoices): + WYCOFANO = "wycofano", "Wycofano oświadczenia" + BRAK_OSWIADCZEN = "brak", "Brak oświadczeń do wycofania" + POMINIETO = "pominieto", "Pominięto (rekord bez PBN UID)" + + +@dataclass +class WynikWycofania: + status: StatusWycofania + komunikat: str + + +def wycofaj_oswiadczenia(publikacja, client, uczelnia=None) -> WynikWycofania: + """Wycofuje oświadczenia dyscyplin publikacji z profilu instytucji PBN. + + Gate: publikacja bez ``pbn_uid`` → ``POMINIETO`` (nie błąd), bez + dotykania ``SentData`` i bez wołania klienta. + + **Obiektu publikacji w PBN NIE kasujemy.** Publikacja w PBN jest + współdzielona między instytucjami — usuwamy wyłącznie oświadczenia + dyscyplin NASZEJ instytucji, czyli to, co faktycznie deklarowaliśmy. + + Po sukcesie aktualizuje ``SentData`` WIERSZA TEJ UCZELNI: + ``submitted_successfully=False`` + ``withdrawn_at``. Brak wiersza + (``SentData.DoesNotExist``) nie jest błędem — rekord mógł dostać PBN + UID z importu, a nie z wysyłki; nie ma wtedy czego oznaczać. + + Wyjątki PBN (``PraceSerwisowe`` / ``ResourceLocked`` / ``Http`` / …) + PROPAGUJE — klasyfikuje je wywołujący (kolejka przez wspólne + ``_handle_pbn_exception``, ścieżka synchroniczna własną obsługą). + Prymityw odpowiada za semantykę „co znaczy sukces" i za stan + ``SentData``, NIE za politykę ponawiania. Rozdzielenie jest celowe: + wspólna klasyfikacja w jednym miejscu nie rozjedzie się między + wejściami. + + :param publikacja: rekord BPP (soft-skasowany — to normalny przypadek) + :param client: klient PBN uczelni (``uczelnia.pbn_client(token)``) + :param uczelnia: właściciel wiersza ``SentData``; w multi-hosted + obowiązkowa, bo przy ≥2 wierszach lookup bez niej rzuci + ``MultipleObjectsReturned`` + """ + if not getattr(publikacja, "pbn_uid_id", None): + return WynikWycofania( + status=StatusWycofania.POMINIETO, + komunikat=( + "Rekord nie ma PBN UID — nic nie zostało wysłane do PBN, " + "więc nie ma czego wycofywać." + ), + ) + + try: + client.delete_all_publication_statements(publikacja.pbn_uid_id) + except CannotDeleteStatementsException as exc: + # PBN mówi: nie ma czego usuwać. Dla NAS to stan docelowy, nie + # błąd — oświadczeń tej publikacji w profilu instytucji już nie + # ma. (Uwaga: pbn-client w _delete_statements_with_retry ponawia + # na tym samym wyjątku, bo tam kasowanie poprzedza wysyłkę i brak + # oświadczeń jest przeszkodą. Tu jest odwrotnie.) + status = StatusWycofania.BRAK_OSWIADCZEN + komunikat = f"PBN: brak oświadczeń do wycofania ({exc})." + else: + status = StatusWycofania.WYCOFANO + komunikat = ( + f"Wycofano oświadczenia dyscyplin z profilu instytucji w PBN " + f"(PBN UID={publikacja.pbn_uid_id})." + ) + + try: + SentData.objects.mark_as_withdrawn(publikacja, uczelnia=uczelnia) + except SentData.DoesNotExist: + komunikat += " Brak wiersza SentData — nie ma czego oznaczać." + + return WynikWycofania(status=status, komunikat=komunikat) diff --git a/src/pbn_export_queue/admin.py b/src/pbn_export_queue/admin.py index 4c168580f..4dace3d34 100644 --- a/src/pbn_export_queue/admin.py +++ b/src/pbn_export_queue/admin.py @@ -54,6 +54,7 @@ class PBN_Export_QueueAdmin( list_per_page = 10 list_display = [ "rekord_do_wysylki", + "operacja", "object_id", "zamowil", "wysylke_podjeto", @@ -67,6 +68,7 @@ class PBN_Export_QueueAdmin( list_filter = [ ZamowilUniqueFilter, + "operacja", "zakonczono_pomyslnie", "retry_after_user_authorised", ] @@ -76,6 +78,7 @@ class PBN_Export_QueueAdmin( readonly_fields = [ "object_id", "content_type", + "operacja", "zamowiono", "zamowil", "wysylke_podjeto", diff --git a/src/pbn_export_queue/migrations/0011_pbn_export_queue_operacja.py b/src/pbn_export_queue/migrations/0011_pbn_export_queue_operacja.py new file mode 100644 index 000000000..de69de16f --- /dev/null +++ b/src/pbn_export_queue/migrations/0011_pbn_export_queue_operacja.py @@ -0,0 +1,24 @@ +# Generated by Django 5.2.16 on 2026-08-10 18:56 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("pbn_export_queue", "0010_atomowa_kolejka_pbn"), + ] + + operations = [ + migrations.AddField( + model_name="pbn_export_queue", + name="operacja", + field=models.CharField( + choices=[("wysylka", "Wysyłka"), ("wycofanie", "Wycofanie oświadczeń")], + db_index=True, + default="wysylka", + help_text="Wycofanie usuwa oświadczenia dyscyplin publikacji z profilu instytucji w PBN (soft-delete rekordu). Nie kasuje samego obiektu publikacji w PBN — ten jest współdzielony między instytucjami.", + max_length=16, + verbose_name="Operacja", + ), + ), + ] diff --git a/src/pbn_export_queue/models.py b/src/pbn_export_queue/models.py index e5f2fc77f..6d520f26f 100644 --- a/src/pbn_export_queue/models.py +++ b/src/pbn_export_queue/models.py @@ -31,6 +31,29 @@ logger = logging.getLogger(__name__) +#: Nazwa częściowego unikatu z Meta.constraints — jedyny IntegrityError, +#: który wolno przetłumaczyć na domenowe „już w kolejce". +NAZWA_UNIKATU_AKTYWNEGO_WPISU = "pbn_export_queue_jeden_aktywny_wpis_na_rekord" + + +def _to_kolizja_aktywnego_wpisu(exc): + """Czy ten ``IntegrityError`` NAPRAWDĘ znaczy „już w kolejce"? + + psycopg wystawia nazwę naruszonego ograniczenia w + ``exc.__cause__.diag.constraint_name``; gdy jej nie ma (inny sterownik + albo backend) — fallback na tekst wyjątku. + + Tłumaczenie „w ciemno" połykało naruszenie NOT NULL na ``zamowil`` + (operacja systemowa bez użytkownika) i zamieniało brak wycofania + oświadczeń w PBN w niewinny komunikat „już w kolejce". + """ + diag = getattr(getattr(exc, "__cause__", None), "diag", None) + nazwa = getattr(diag, "constraint_name", None) + if nazwa: + return nazwa == NAZWA_UNIKATU_AKTYWNEGO_WPISU + return NAZWA_UNIKATU_AKTYWNEGO_WPISU in str(exc) + + class PBN_Export_QueueManager(models.Manager): def filter_rekord_do_wysylki(self, rekord): return self.filter( @@ -39,11 +62,21 @@ def filter_rekord_do_wysylki(self, rekord): wysylke_zakonczono=None, ) - def sprobuj_utowrzyc_wpis(self, user, rekord, uczelnia=None): + def sprobuj_utowrzyc_wpis(self, user, rekord, uczelnia=None, operacja=None): # Szybka ścieżka (przyjazny błąd bez trafiania w constraint bazy). if self.filter_rekord_do_wysylki(rekord).exists(): raise AlreadyEnqueuedError("ten rekord jest już w kolejce do wysyłki") + kwargs = { + "rekord_do_wysylki": rekord, + "zamowil": user, + "uczelnia": uczelnia, + } + if operacja is not None: + # Pominięcie zostawia default modelu (WYSYLKA), więc wszystkie + # dotychczasowe wywołania działają bez zmian. + kwargs["operacja"] = operacja + # Właściwe zabezpieczenie przed wyścigiem: między exists() a create() # inny proces mógł dodać aktywny wpis. Częściowy unikat # (content_type, object_id) WHERE wysylke_zakonczono IS NULL zamienia @@ -52,12 +85,14 @@ def sprobuj_utowrzyc_wpis(self, user, rekord, uczelnia=None): # unieważnić ewentualnej otaczającej transakcji. try: with transaction.atomic(): - return self.create( - rekord_do_wysylki=rekord, - zamowil=user, - uczelnia=uczelnia, - ) + return self.create(**kwargs) except IntegrityError as e: + # Tylko kolizja częściowego unikatu znaczy „już w kolejce". + # Każde inne naruszenie (NOT NULL na `zamowil` przy operacji + # systemowej, zerwany FK) MUSI polecieć w górę — inaczej + # zniknęłoby pod komunikatem sugerującym, że wszystko gra. + if not _to_kolizja_aktywnego_wpisu(e): + raise raise AlreadyEnqueuedError( "ten rekord jest już w kolejce do wysyłki" ) from e @@ -137,6 +172,21 @@ class PBN_Export_Queue(models.Model): help_text="Publikacja wykluczona z eksportu z przyczyn projektowych (nie błąd)", ) + class Operacja(models.TextChoices): + WYSYLKA = "wysylka", "Wysyłka" + WYCOFANIE = "wycofanie", "Wycofanie oświadczeń" + + operacja = models.CharField( + max_length=16, + choices=Operacja.choices, + default=Operacja.WYSYLKA, + db_index=True, + verbose_name="Operacja", + help_text="Wycofanie usuwa oświadczenia dyscyplin publikacji z profilu " + "instytucji w PBN (soft-delete rekordu). Nie kasuje samego obiektu " + "publikacji w PBN — ten jest współdzielony między instytucjami.", + ) + objects = PBN_Export_QueueManager() class Meta: @@ -184,10 +234,25 @@ def check_if_record_still_exists(self): # ⚠️ `get_object_for_this_type` pyta `_base_manager`, który z # definicji NIE filtruje (Django wymaga, żeby zwracał wszystkie # wiersze). Rekord soft-skasowany jest więc tą drogą nadal - # znajdowany, mimo że `objects` go nie pokazuje. Dla kolejki PBN + # znajdowany, mimo że `objects` go nie pokazuje. Dla WYSYŁKI # „w koszu" ma znaczyć „nie ma go" — inaczej wysyłalibyśmy do PBN # publikację, którą operator usunął. - if getattr(obiekt, "deleted_at", None) is not None: + # + # Dla WYCOFANIA ta sama przesłanka prowadzi do wniosku + # przeciwnego: oświadczenia wycofujemy WŁAŚNIE dlatego, że rekord + # trafił do kosza, więc soft-delete jest tu stanem oczekiwanym, + # a nie powodem przerwania. Bez tego warunku gałąź WYCOFANIE + # w `send_to_pbn()` byłaby martwym kodem, a sprzątaczka kolejki + # (`kolejka_wyczysc_wpisy_bez_rekordow`) po cichu kasowałaby + # zlecenia wycofania, zostawiając oświadczenia w PBN. + # + # Rekord skasowany TWARDO (wiersza nie ma) nadal daje False dla + # obu operacji — bez wiersza nie odczytamy `pbn_uid`, więc nie ma + # czego wycofywać. To świadoma, głośna porażka. + if ( + self.operacja != self.Operacja.WYCOFANIE + and getattr(obiekt, "deleted_at", None) is not None + ): return False if obiekt: @@ -426,6 +491,73 @@ def _handle_successful_send(self, sent_data, notificator): self.save() return SendStatus.FINISHED_OKAY + def _pozyskaj_klienta_pbn(self): + """Buduje klienta PBN dla TEGO wpisu kolejki. + + Uczelnia z FK wpisu (``self.uczelnia``) — nigdy „pierwsza z brzegu": + dawne API uczelni domyślnej zostało trwale usunięte i jest pilnowane + guardem ``bpp/tests/test_multihosted_get_default_guard.py``. Dla + wpisów legacy (``uczelnia_id is None``, sprzed migracji ``0009``) + jedyny dozwolony fallback to „jedyna-albo-głośny-błąd". + + Token: z konta PBN zamawiającego — ``get_pbn_user()`` respektuje + ``przedstawiaj_w_pbn_jako``, więc konto techniczne (operacje + systemowe) można podpiąć pod konto z ważnym tokenem bez zmiany kodu. + """ + from bpp.models import Uczelnia + + pbn_user = self.zamowil.get_pbn_user() + uczelnia = self.uczelnia or Uczelnia.objects.get_single_uczelnia_or_fail() + return uczelnia.pbn_client(pbn_user.pbn_token) + + def withdraw_from_pbn(self): + """Gałąź WYCOFANIE — cienkie wywołanie prymitywu. + + NIE woła klienta PBN bezpośrednio i NIE dotyka ``SentData`` — robi + to ``wycofaj_oswiadczenia()``, wspólne z wejściem synchronicznym. + Tu wyłącznie: pozyskanie klienta, wywołanie prymitywu i tłumaczenie + wyniku/wyjątku na ``SendStatus``. Klasyfikacja wyjątków PBN idzie + przez wspólne ``_handle_pbn_exception`` (ResourceLocked → + RETRY_LATER, PraceSerwisowe → RETRY_MUCH_LATER, HTTP 423 → + RETRY_LATER, …), czyli DOKŁADNIE tę samą tabelę co wysyłka. + + :return: SendStatus + """ + from pbn_api.wycofanie import wycofaj_oswiadczenia + + try: + client = self._pozyskaj_klienta_pbn() + except Exception as exc: + zaloguj_polkniety_wyjatek( + "Nie udało się zbudować klienta PBN do wycofania oświadczeń " + f"(PBN_Export_Queue pk={self.pk})", + logger=logger, + do_rollbar=False, # Rollbar w _handle_pbn_exception + ) + return self._handle_pbn_exception(exc) + + try: + wynik = wycofaj_oswiadczenia( + self.rekord_do_wysylki, client, uczelnia=self.uczelnia + ) + except Exception as exc: + zaloguj_polkniety_wyjatek( + "Błąd podczas wycofywania oświadczeń z PBN z kolejki eksportu " + f"(PBN_Export_Queue pk={self.pk})", + logger=logger, + do_rollbar=False, # Rollbar w _handle_pbn_exception + ) + return self._handle_pbn_exception(exc) + + # POMINIETO (rekord bez PBN UID) też kończy wpis sukcesem: nic nie + # poszło do PBN, więc stan docelowy jest osiągnięty. To gate + # obronny — `zakolejkuj_wycofanie` takich wpisów w ogóle nie tworzy. + self.wysylke_zakonczono = timezone.now() + self.zakonczono_pomyslnie = True + self.dopisz_komunikat(wynik.komunikat) + self.save() + return SendStatus.FINISHED_OKAY + def _zajmij_atomowo(self): """Atomowo zajmij wpis do wysyłki (zabezpieczenie przed dwoma workerami). @@ -481,6 +613,12 @@ def send_to_pbn(self): # On dokończy — bieżące zadanie kończymy bez ponawiania. return SendStatus.LOCKED_ELSEWHERE + # Rozgałęzienie PO zajęciu wiersza: wycofanie dziedziczy za darmo + # licznik `ilosc_prob`, znacznik `wysylke_podjeto` i ochronę przed + # dwoma workerami. Ścieżka WYSYLKA niżej — bez zmian. + if self.operacja == self.Operacja.WYCOFANIE: + return self.withdraw_from_pbn() + from bpp.admin.helpers.pbn_api.cli import sprobuj_wyslac_do_pbn_celery try: diff --git a/src/pbn_export_queue/operacje.py b/src/pbn_export_queue/operacje.py new file mode 100644 index 000000000..57de677f5 --- /dev/null +++ b/src/pbn_export_queue/operacje.py @@ -0,0 +1,116 @@ +"""Publiczny kontrakt kolejkowania operacji PBN. + +Funkcje MODUŁOWE (nie metody managera) — tak woła je faza 06 z receiverów +sygnałów: ``post_soft_delete`` → ``zakolejkuj_wycofanie``, ``post_restore`` +→ ``zakolejkuj_wysylke``. Jedno miejsce prawdy: wpisy powstają wyłącznie +przez ``sprobuj_utowrzyc_wpis``, żeby nie omijać zabezpieczenia TOCTOU, +FK ``uczelnia`` ani pola ``operacja``. +""" + +from pbn_export_queue.models import PBN_Export_Queue + +#: Login konta używanego jako ``zamowil`` przy operacjach systemowych. +NAZWA_KONTA_TECHNICZNEGO = "bpp-system" + + +def pobierz_konto_techniczne(): + """Konto ``zamowil`` dla operacji bez zalogowanego użytkownika. + + ``PBN_Export_Queue.zamowil`` jest NOT NULL, a soft-delete zlecony + sygnałem, celery albo scalaniem duplikatów nie ma requestu. Świadomie + NIE robimy ``zamowil`` nullable (spec §4.2) — psułoby to założenia + kolejki i raportów, w których „kto zlecił" jest kolumną. + + Konto jest nieaktywne i bez hasła: ma istnieć jako podmiot audytu, + nie jako sposób logowania. + + ⚠️ Konto nie ma własnego ``pbn_token``, więc samo z siebie nie wyśle + nic do PBN — ``_pozyskaj_klienta_pbn`` skończy się wtedy + ``WillNotExportError`` i wpis dostanie ``FINISHED_ERROR`` z błędem + merytorycznym. To celowo GŁOŚNA porażka, nie ciche pominięcie. + Obejście produkcyjne bez zmiany kodu: administrator ustawia temu kontu + ``przedstawiaj_w_pbn_jako`` na konto z ważnym tokenem PBN + (``get_pbn_user()`` samo podmieni użytkownika). + """ + from django.contrib.auth import get_user_model + + user, utworzono = get_user_model().objects.get_or_create( + username=NAZWA_KONTA_TECHNICZNEGO, + defaults={ + "first_name": "Konto", + "last_name": "systemowe BPP", + "is_active": False, + "is_staff": False, + "is_superuser": False, + }, + ) + if utworzono: + user.set_unusable_password() + user.save(update_fields=["password"]) + return user + + +def _zakolejkuj(instance, operacja, user=None, uczelnia=None): + from pbn_api.exceptions import AlreadyEnqueuedError + + # `tasks` importuje `models`, więc import na poziomie modułu zrobiłby + # cykl — stąd import lokalny. + from pbn_export_queue import tasks + + try: + wpis = PBN_Export_Queue.objects.sprobuj_utowrzyc_wpis( + user or pobierz_konto_techniczne(), + instance, + uczelnia=uczelnia, + operacja=operacja, + ) + except AlreadyEnqueuedError: + # Rekord czeka już w kolejce — idempotencja, nie błąd. + return None + + tasks.task_sprobuj_wyslac_do_pbn.delay(wpis.pk) + return wpis + + +def zakolejkuj_wysylke(instance, user=None, uczelnia=None): + """Tworzy wpis WYSYLKA i uruchamia wysyłkę w tle. + + Wołane m.in. przy przywróceniu publikacji z kosza (faza 06). + + **BEZ gate'u na ``pbn_uid``** — rekord bez PBN UID też ma prawo + pojechać do PBN, bo wysyłka dopiero go nadaje. (Kontrakt planu fazy 06 + opisywał „None gdy brak pbn_uid" dla obu funkcji; to świadome odejście + — nie „naprawiaj" go pod tamten opis, bo zablokowałoby wysyłkę rekordu, + który nigdy w PBN nie był.) + + Idempotentne: gdy rekord już czeka w kolejce → ``None``. + ``user=None`` → konto techniczne (``zamowil`` jest NOT NULL). + """ + return _zakolejkuj( + instance, + PBN_Export_Queue.Operacja.WYSYLKA, + user=user, + uczelnia=uczelnia, + ) + + +def zakolejkuj_wycofanie(instance, user=None, uczelnia=None): + """Tworzy wpis WYCOFANIE i uruchamia wycofanie oświadczeń w tle. + + Gate: tylko gdy rekord ma PBN UID — bez niego nic do PBN nie poszło, + więc nie ma czego wycofywać (``None``, no-op). + + Idempotentne: gdy rekord już czeka w kolejce → ``None``. + ``user=None`` → konto techniczne. NIGDY nie przekazujemy ``None`` do + ``zamowil``: naruszenie NOT NULL udawałoby wtedy „już w kolejce" + i wycofanie zniknęłoby po cichu. + """ + if not getattr(instance, "pbn_uid_id", None): + return None + + return _zakolejkuj( + instance, + PBN_Export_Queue.Operacja.WYCOFANIE, + user=user, + uczelnia=uczelnia, + ) diff --git a/src/pbn_export_queue/tests/test_operacja_wycofanie.py b/src/pbn_export_queue/tests/test_operacja_wycofanie.py new file mode 100644 index 000000000..11644bb27 --- /dev/null +++ b/src/pbn_export_queue/tests/test_operacja_wycofanie.py @@ -0,0 +1,518 @@ +"""Faza 05 soft-delete: wycofanie oświadczeń dyscyplin z PBN. + +Soft-delete publikacji, która poszła do PBN, musi wycofać jej oświadczenia +z profilu instytucji. Realizuje to nowa operacja ``WYCOFANIE`` w kolejce +eksportu, obok dotychczasowej ``WYSYLKA``. +""" + +from unittest.mock import MagicMock, patch + +import pytest +from model_bakery import baker + +from pbn_export_queue.models import PBN_Export_Queue, SendStatus + + +@pytest.fixture +def wpis_wycofania(wydawnictwo_ciagle, admin_user, uczelnia): + """Zlecenie wycofania dla publikacji, która poszła do PBN i trafiła + do kosza — czyli dokładnie sytuacja, w której faza 05 działa. + + Kolejność ma znaczenie: wpis powstaje PRZED skasowaniem rekordu, tak + jak w produkcji (receiver fazy 06 kolejkuje przy usuwaniu). + """ + from pbn_api.models import Publication, SentData + + wydawnictwo_ciagle.pbn_uid = baker.make(Publication) + wydawnictwo_ciagle.save() + SentData.objects.create( + object=wydawnictwo_ciagle, + data_sent={}, + submitted_successfully=True, + uploaded_okay=True, + uczelnia=uczelnia, + ) + wpis = baker.make( + PBN_Export_Queue, + rekord_do_wysylki=wydawnictwo_ciagle, + zamowil=admin_user, + uczelnia=uczelnia, + operacja=PBN_Export_Queue.Operacja.WYCOFANIE, + wysylke_zakonczono=None, + ) + wydawnictwo_ciagle.delete() + return wpis + + +@pytest.mark.django_db +def test_operacja_default_wysylka(wydawnictwo_ciagle, admin_user): + """Wpisy sprzed fazy 05 (i wszystkie tworzone bez jawnej operacji) + muszą nadal znaczyć „wyślij" — inaczej migracja zamieniłaby zaległą + kolejkę wysyłek w kolejkę wycofań.""" + wpis = baker.make( + PBN_Export_Queue, + rekord_do_wysylki=wydawnictwo_ciagle, + zamowil=admin_user, + ) + wpis.refresh_from_db() + assert wpis.operacja == PBN_Export_Queue.Operacja.WYSYLKA + assert PBN_Export_Queue.Operacja.WYSYLKA == "wysylka" + assert PBN_Export_Queue.Operacja.WYCOFANIE == "wycofanie" + + +@pytest.mark.django_db +def test_sentdata_mark_as_withdrawn(wydawnictwo_ciagle, uczelnia): + """Wycofanie zostawia ślad, przywrócenie go kasuje. + + Wiersza SentData NIE kasujemy: zostaje dla re-matchingu przy restore + i dla SoftDeleteLog fazy 06. Znacznik ``withdrawn_at`` odróżnia „nigdy + nie wysłane" od „wysłane, potem wycofane" — sam + ``submitted_successfully=False`` tych dwóch stanów nie rozróżnia. + + Izolację per-uczelnia sprawdza osobno + ``pbn_api/tests/test_sentdata_per_uczelnia.py`` (tam mieszka fixture + ``uczelnia2`` i reszta rodzeństwa tego zachowania). + """ + from pbn_api.models.sentdata import SentData + + SentData.objects.create( + object=wydawnictwo_ciagle, + data_sent={}, + submitted_successfully=True, + uploaded_okay=True, + uczelnia=uczelnia, + ) + + SentData.objects.mark_as_withdrawn(wydawnictwo_ciagle, uczelnia=uczelnia) + + sd = SentData.objects.get_for_rec(wydawnictwo_ciagle, uczelnia) + assert sd.submitted_successfully is False + assert sd.withdrawn_at is not None + + # restore → ponowna wysyłka zeruje znacznik wycofania + SentData.objects.mark_as_successful(wydawnictwo_ciagle, uczelnia=uczelnia) + sd = SentData.objects.get_for_rec(wydawnictwo_ciagle, uczelnia) + assert sd.submitted_successfully is True + assert sd.withdrawn_at is None + + +@pytest.mark.django_db +def test_wycofanie_wola_delete_all_statements( + wpis_wycofania, wydawnictwo_ciagle, uczelnia +): + """Gałąź WYCOFANIE dochodzi do klienta PBN i kończy wpis sukcesem. + + ⚠️ Rekord jest w KOSZU (fixture go kasuje) — i to jest cała trudność. + Do fazy 05 ``send_to_pbn()`` odrzucał takie wpisy guardem + ``check_if_record_still_exists()``, więc gałąź wycofania byłaby martwym + kodem. Test bez soft-delete'u przechodziłby także przed poprawką + i niczego by nie pilnował. + """ + from pbn_api.models import SentData + + mock_client = MagicMock() + with patch.object( + PBN_Export_Queue, "_pozyskaj_klienta_pbn", return_value=mock_client + ): + result = wpis_wycofania.send_to_pbn() + + assert result == SendStatus.FINISHED_OKAY + mock_client.delete_all_publication_statements.assert_called_once_with( + wydawnictwo_ciagle.pbn_uid_id + ) + wpis_wycofania.refresh_from_db() + assert wpis_wycofania.zakonczono_pomyslnie is True + assert wpis_wycofania.wysylke_zakonczono is not None + + sd = SentData.objects.get_for_rec(wydawnictwo_ciagle, uczelnia) + assert sd.submitted_successfully is False + assert sd.withdrawn_at is not None + + +@pytest.mark.django_db +def test_sprzataczka_nie_kasuje_zlecen_wycofania( + wydawnictwo_ciagle, wydawnictwo_zwarte, admin_user, uczelnia +): + """Regresja blokera #2 (rewizja planu 2026-08-10). + + ``kolejka_wyczysc_wpisy_bez_rekordow()`` kasuje wpisy, których rekord + „już nie istnieje", i używa do tego tego samego guardu co + ``send_to_pbn()``. Zanim guard poznał operację, soft-delete publikacji + sprawiał, że sprzątaczka kasowała WŁAŚNIE UTWORZONE zlecenie wycofania: + oświadczenia zostawały w PBN, śladu brak, a wyścig z workerem celery + rozstrzygał się losowo. + + Dowodem jest RÓŻNICA między operacjami, nie samo przetrwanie wpisu — + zachowanie dla WYSYLKI (fazy 02) musi zostać nietknięte. + """ + from pbn_export_queue.tasks import kolejka_wyczysc_wpisy_bez_rekordow + + wycofanie = baker.make( + PBN_Export_Queue, + rekord_do_wysylki=wydawnictwo_ciagle, + zamowil=admin_user, + uczelnia=uczelnia, + operacja=PBN_Export_Queue.Operacja.WYCOFANIE, + wysylke_zakonczono=None, + ) + wysylka = baker.make( + PBN_Export_Queue, + rekord_do_wysylki=wydawnictwo_zwarte, + zamowil=admin_user, + uczelnia=uczelnia, + operacja=PBN_Export_Queue.Operacja.WYSYLKA, + wysylke_zakonczono=None, + ) + + wydawnictwo_ciagle.delete() + wydawnictwo_zwarte.delete() + + kolejka_wyczysc_wpisy_bez_rekordow() + + assert PBN_Export_Queue.objects.filter(pk=wycofanie.pk).exists(), ( + "sprzątaczka skasowała zlecenie WYCOFANIA — oświadczenia zostaną " + "w PBN i nikt się o tym nie dowie" + ) + assert not PBN_Export_Queue.objects.filter(pk=wysylka.pk).exists(), ( + "wpis WYSYLKA rekordu z kosza ma nadal znikać — to zachowanie " + "z fazy 02, którego nie wolno zepsuć przy okazji" + ) + + +@pytest.mark.django_db +def test_wysylka_nie_wola_delete_all_statements( + wydawnictwo_ciagle, admin_user, uczelnia +): + """Dodanie gałęzi WYCOFANIE nie mogło przekierować WYSYŁKI. + + Rozgałęzienie siedzi na wspólnej ścieżce ``send_to_pbn()``, więc błąd + w warunku (albo default pola) zamieniłby zaległą kolejkę wysyłek + w kolejkę wycofań — i skasował oświadczenia rekordów, których nikt + nie usuwał. + """ + wpis = baker.make( + PBN_Export_Queue, + rekord_do_wysylki=wydawnictwo_ciagle, + zamowil=admin_user, + uczelnia=uczelnia, + operacja=PBN_Export_Queue.Operacja.WYSYLKA, + wysylke_zakonczono=None, + ) + + sent_data = MagicMock() + with ( + patch.object(PBN_Export_Queue, "_pozyskaj_klienta_pbn") as mock_klient, + patch( + "bpp.admin.helpers.pbn_api.cli.sprobuj_wyslac_do_pbn_celery", + return_value=(sent_data, ["ok"]), + ) as mock_send, + ): + result = wpis.send_to_pbn() + + assert result == SendStatus.FINISHED_OKAY + mock_send.assert_called_once() + # ścieżka WYSYLKA przekazuje uczelnię wpisu (multi-hosted) + assert mock_send.call_args.kwargs["uczelnia"] == uczelnia + # klienta wycofania nie budujemy w ogóle + mock_klient.assert_not_called() + + +@pytest.mark.django_db +def test_integrity_error_zamowil_nie_udaje_already_enqueued(wydawnictwo_ciagle): + """Prawdziwy ``IntegrityError`` nie może udawać „już w kolejce". + + ``sprobuj_utowrzyc_wpis`` tłumaczyło KAŻDY ``IntegrityError`` na + ``AlreadyEnqueuedError``, bo spodziewało się wyłącznie kolizji + częściowego unikatu. Operacja systemowa (soft-delete z sygnału, + z celery, ze scalania — bez ``request.user``) trafia jednak w NOT NULL + na ``zamowil`` i dostawała „ten rekord jest już w kolejce": + ``zakolejkuj_wycofanie`` zwracało None, oświadczenia zostawały w PBN, + a operator widział komunikat sugerujący, że wszystko jest w porządku. + + ``zamowil=None`` łamie NOT NULL, a NIE unikat aktywnego wpisu — więc + MUSI polecieć w górę jako ``IntegrityError``. + """ + from django.db import IntegrityError + + with pytest.raises(IntegrityError): + PBN_Export_Queue.objects.sprobuj_utowrzyc_wpis(None, wydawnictwo_ciagle) + + +@pytest.mark.django_db +def test_zakolejkuj_wycofanie_gate_brak_pbn_uid(wydawnictwo_ciagle, admin_user): + """Bez PBN UID nic nie poszło do PBN — nie kolejkujemy wycofania.""" + from pbn_export_queue.operacje import zakolejkuj_wycofanie + + assert wydawnictwo_ciagle.pbn_uid_id is None + with patch("pbn_export_queue.tasks.task_sprobuj_wyslac_do_pbn") as mock_task: + wpis = zakolejkuj_wycofanie(wydawnictwo_ciagle, user=admin_user) + + assert wpis is None + assert ( + PBN_Export_Queue.objects.filter_rekord_do_wysylki(wydawnictwo_ciagle).count() + == 0 + ) + mock_task.delay.assert_not_called() + + +@pytest.mark.django_db +def test_zakolejkuj_wycofanie_tworzy_wpis(wydawnictwo_ciagle, admin_user, uczelnia): + from pbn_api.models import Publication + from pbn_export_queue.operacje import zakolejkuj_wycofanie + + wydawnictwo_ciagle.pbn_uid = baker.make(Publication) + wydawnictwo_ciagle.save() + + with patch("pbn_export_queue.tasks.task_sprobuj_wyslac_do_pbn") as mock_task: + wpis = zakolejkuj_wycofanie( + wydawnictwo_ciagle, user=admin_user, uczelnia=uczelnia + ) + + assert wpis is not None + assert wpis.operacja == PBN_Export_Queue.Operacja.WYCOFANIE + assert wpis.uczelnia == uczelnia + mock_task.delay.assert_called_once_with(wpis.pk) + + +@pytest.mark.django_db +def test_zakolejkuj_wysylke_nie_ma_gate_na_pbn_uid(wydawnictwo_ciagle, admin_user): + """WYSYŁKA celowo NIE ma gate'u na ``pbn_uid``. + + Rekord, który nigdy nie poszedł do PBN, po przywróceniu i tak ma prawo + pojechać — wysyłka dopiero nadaje PBN UID. Kontrakt PINNED planu fazy + 06 mówi „None gdy brak pbn_uid" dla OBU funkcji; to rozstrzygnięcie + jest świadomym odejściem, żeby nikt go nie „naprawił" pod tamten opis. + """ + from pbn_export_queue.operacje import zakolejkuj_wysylke + + assert wydawnictwo_ciagle.pbn_uid_id is None + with patch("pbn_export_queue.tasks.task_sprobuj_wyslac_do_pbn") as mock_task: + wpis = zakolejkuj_wysylke(wydawnictwo_ciagle, user=admin_user) + + assert wpis is not None + assert wpis.operacja == PBN_Export_Queue.Operacja.WYSYLKA + mock_task.delay.assert_called_once_with(wpis.pk) + + +@pytest.mark.django_db +def test_zakolejkuj_idempotentne(wydawnictwo_ciagle, admin_user): + """Drugie zlecenie dla tego samego rekordu to no-op, nie błąd.""" + from pbn_api.models import Publication + from pbn_export_queue.operacje import zakolejkuj_wycofanie + + wydawnictwo_ciagle.pbn_uid = baker.make(Publication) + wydawnictwo_ciagle.save() + + with patch("pbn_export_queue.tasks.task_sprobuj_wyslac_do_pbn"): + pierwszy = zakolejkuj_wycofanie(wydawnictwo_ciagle, user=admin_user) + drugi = zakolejkuj_wycofanie(wydawnictwo_ciagle, user=admin_user) + + assert pierwszy is not None + assert drugi is None + assert ( + PBN_Export_Queue.objects.filter_rekord_do_wysylki(wydawnictwo_ciagle).count() + == 1 + ) + + +@pytest.mark.django_db +def test_zakolejkuj_wycofanie_bez_usera_uzywa_konta_technicznego( + wydawnictwo_ciagle, uczelnia +): + """Soft-delete systemowy (bez zalogowanego użytkownika) MUSI utworzyć wpis. + + ``zamowil`` jest NOT NULL, a sygnał/celery/scalanie nie mają requestu. + Zwrot ``None`` znaczyłby, że oświadczenia zostaną w PBN. + """ + from pbn_api.models import Publication + from pbn_export_queue.operacje import ( + NAZWA_KONTA_TECHNICZNEGO, + zakolejkuj_wycofanie, + ) + + wydawnictwo_ciagle.pbn_uid = baker.make(Publication) + wydawnictwo_ciagle.save() + + with patch("pbn_export_queue.tasks.task_sprobuj_wyslac_do_pbn"): + wpis = zakolejkuj_wycofanie(wydawnictwo_ciagle, user=None, uczelnia=uczelnia) + + assert wpis is not None, ( + "systemowy soft-delete MUSI utworzyć wpis wycofania — zwrot None " + "znaczy, że oświadczenia zostaną w PBN" + ) + assert wpis.zamowil.username == NAZWA_KONTA_TECHNICZNEGO + assert wpis.operacja == PBN_Export_Queue.Operacja.WYCOFANIE + assert wpis.zamowil.is_active is False + assert wpis.zamowil.has_usable_password() is False + + +@pytest.mark.django_db +def test_konto_techniczne_bez_tokenu_konczy_glosno(wydawnictwo_ciagle, uczelnia): + """Brak tokenu PBN kończy wpis BŁĘDEM, nie cichym sukcesem. + + Konto techniczne nie ma własnego ``pbn_token``, więc autoryzacja + w PBN rzuci ``WillNotExportError``. To jest akceptowalne WYŁĄCZNIE + dlatego, że kończy się głośno — wpis dostaje ``FINISHED_ERROR`` + z błędem merytorycznym i komunikatem wskazującym konfigurację. + Gdyby kończyło się ``FINISHED_OKAY`` albo cichym ``None``, operator + myślałby, że oświadczenia zostały wycofane. + + Obejście produkcyjne (bez zmiany kodu): administrator ustawia kontu + technicznemu ``przedstawiaj_w_pbn_jako`` na konto z ważnym tokenem. + + Test nie odtwarza pełnej ścieżki HTTP, bo ``authorize`` w transporcie + odpala się dopiero na odpowiedzi 403 — czyli po realnym żądaniu do + PBN. Sprawdzamy więc to, co faktycznie jest nasze: że wyjątek z + klienta trafia w klasyfikację jako błąd MERYTORYCZNY. + """ + from pbn_api.exceptions import WillNotExportError + from pbn_api.models import Publication + from pbn_export_queue.models import RodzajBledu + from pbn_export_queue.operacje import pobierz_konto_techniczne + + wydawnictwo_ciagle.pbn_uid = baker.make(Publication) + wydawnictwo_ciagle.save() + wpis = baker.make( + PBN_Export_Queue, + rekord_do_wysylki=wydawnictwo_ciagle, + zamowil=pobierz_konto_techniczne(), + uczelnia=uczelnia, + operacja=PBN_Export_Queue.Operacja.WYCOFANIE, + wysylke_zakonczono=None, + ) + wydawnictwo_ciagle.delete() + + with patch.object( + PBN_Export_Queue, + "_pozyskaj_klienta_pbn", + side_effect=WillNotExportError( + "Najpierw wykonaj autoryzację w PBN API za pomocą menu" + ), + ): + result = wpis.send_to_pbn() + + assert result == SendStatus.FINISHED_ERROR + wpis.refresh_from_db() + assert wpis.zakonczono_pomyslnie is False + assert wpis.rodzaj_bledu == RodzajBledu.MERYTORYCZNY + assert "autoryzacj" in wpis.komunikat.lower() + + +@pytest.mark.django_db +def test_pozyskaj_klienta_bierze_token_zamawiajacego( + wydawnictwo_ciagle, admin_user, uczelnia +): + """Klient budowany jest z uczelni WPISU i tokenu zamawiającego. + + Nigdy „pierwszej z brzegu" uczelni — w multi-hosted wycofanie dotyczy + profilu konkretnej instytucji. Token idzie przez ``get_pbn_user()``, + więc ``przedstawiaj_w_pbn_jako`` działa bez zmiany kodu. + """ + admin_user.pbn_token = "TOKEN-123" + admin_user.save() + wpis = baker.make( + PBN_Export_Queue, + rekord_do_wysylki=wydawnictwo_ciagle, + zamowil=admin_user, + uczelnia=uczelnia, + operacja=PBN_Export_Queue.Operacja.WYCOFANIE, + ) + + with patch.object(type(uczelnia), "pbn_client") as mock_pbn_client: + wpis._pozyskaj_klienta_pbn() + + mock_pbn_client.assert_called_once_with("TOKEN-123") + + +@pytest.mark.django_db +def test_wycofanie_brak_oswiadczen_to_sukces( + wpis_wycofania, wydawnictwo_ciagle, uczelnia +): + """Idempotencja przez CAŁĄ kolejkę, nie tylko w prymitywie. + + Powtórne wycofanie (albo wycofanie rekordu, którego oświadczeń nigdy + nie było) nie może zostawić wpisu w błędzie — inaczej operator + dostawałby czerwień za operację, która osiągnęła cel. + """ + from pbn_api.exceptions import CannotDeleteStatementsException + from pbn_api.models import SentData + + mock_client = MagicMock() + mock_client.delete_all_publication_statements.side_effect = ( + CannotDeleteStatementsException("brak oświadczeń") + ) + with patch.object( + PBN_Export_Queue, "_pozyskaj_klienta_pbn", return_value=mock_client + ): + result = wpis_wycofania.send_to_pbn() + + assert result == SendStatus.FINISHED_OKAY + wpis_wycofania.refresh_from_db() + assert wpis_wycofania.zakonczono_pomyslnie is True + assert ( + SentData.objects.get_for_rec(wydawnictwo_ciagle, uczelnia).withdrawn_at + is not None + ) + + +def _resource_locked(): + from pbn_api.exceptions import ResourceLockedException + + # ResourceLockedException dziedziczy po HttpException — potrzebuje + # (status_code, url, content), nie samego komunikatu. + return ResourceLockedException(423, "/v2/statements", "zasób zablokowany") + + +def _prace_serwisowe(): + from pbn_api.exceptions import PraceSerwisoweException + + return PraceSerwisoweException("okno serwisowe PBN") + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "zbuduj_wyjatek,oczekiwany_status", + [ + (_resource_locked, SendStatus.RETRY_LATER), + (_prace_serwisowe, SendStatus.RETRY_MUCH_LATER), + ], + ids=["resource_locked", "prace_serwisowe"], +) +def test_wycofanie_dziedziczy_tabele_retry_wysylki( + wpis_wycofania, wydawnictwo_ciagle, uczelnia, zbuduj_wyjatek, oczekiwany_status +): + """Wycofanie korzysta z TEJ SAMEJ klasyfikacji wyjątków co wysyłka. + + Dwa różne wyjątki, dwie różne polityki ponowienia — obie pochodzą + z ``_handle_pbn_exception``, a nie z osobnej drabinki ``except`` + w ``withdraw_from_pbn``. Gdyby wycofanie miało własną tabelę decyzji, + rozjechałaby się z wysyłką przy pierwszej zmianie. + + Wpis NIE może być zakończony, a ``SentData`` NIE oznaczone: wycofanie + się nie udało, więc oświadczenia nadal są w PBN. + """ + from pbn_api.models import SentData + + mock_client = MagicMock() + mock_client.delete_all_publication_statements.side_effect = zbuduj_wyjatek() + + with patch.object( + PBN_Export_Queue, "_pozyskaj_klienta_pbn", return_value=mock_client + ): + result = wpis_wycofania.send_to_pbn() + + assert result == oczekiwany_status + wpis_wycofania.refresh_from_db() + assert wpis_wycofania.wysylke_zakonczono is None + sd = SentData.objects.get_for_rec(wydawnictwo_ciagle, uczelnia) + assert sd.withdrawn_at is None + assert sd.submitted_successfully is True + + +def test_admin_pokazuje_operacje(): + """Superuser musi odróżnić w kolejce wycofanie od wysyłki.""" + from pbn_export_queue.admin import PBN_Export_QueueAdmin + + assert "operacja" in PBN_Export_QueueAdmin.list_display + assert "operacja" in PBN_Export_QueueAdmin.list_filter + assert "operacja" in PBN_Export_QueueAdmin.readonly_fields diff --git a/src/zglos_publikacje/migrations/0028_zgloszenie_autor_protect.py b/src/zglos_publikacje/migrations/0028_zgloszenie_autor_protect.py new file mode 100644 index 000000000..2bf459d5f --- /dev/null +++ b/src/zglos_publikacje/migrations/0028_zgloszenie_autor_protect.py @@ -0,0 +1,49 @@ +"""Domknięcie fazy 04 soft-delete: czwarty dziedzic abstraktu też dostaje PROTECT. + +Faza 04 przestawiła ``autor`` na ``PROTECT`` w klasie abstrakcyjnej +``BazaModeluOdpowiedzialnosciAutorow`` (``bpp/models/abstract/authors.py``), +ale migracja stanu ``bpp/0501`` objęła wyłącznie modele z aplikacji ``bpp``: +``Patent_Autor``, ``Praca_Doktorska``, ``Wydawnictwo_Ciagle_Autor``, +``Wydawnictwo_Zwarte_Autor``. Handoff fazy 04 mówił „dziedziczą 3 modele +``*_Autor``" — dziedziczy CZWARTY, ``Zgloszenie_Publikacji_Autor``, i mieszka +w innej aplikacji. ``makemigrations`` zgłasza takie rozjazdy per-aplikacja, +więc szukanie dziedziczących tylko w ``bpp/`` było niewystarczające. + +Ochrona sama w sobie DZIAŁAŁA już wcześniej: ``on_delete`` jest regułą +kolektora Django, żyjącą w Pythonie, więc pole zachowywało się jak +``PROTECT`` od chwili zmiany abstraktu. Brakowało wyłącznie księgowości +stanu — i to ona wywracała ``makemigrations --check``. + +MIGRACJA JEST STATE-ONLY, z tego samego powodu co ``bpp/0501``: ``on_delete`` +nie ma odpowiednika w schemacie (Django nie emituje ``ON DELETE`` w DDL, +kaskadę realizuje w ``Collector``), więc autogenerowany ``AlterField`` +wygenerowałby DROP + ADD CONSTRAINT — ``ACCESS EXCLUSIVE`` na czas walidacji +klucza obcego, koszt realny, korzyść zerowa, bo powstałby constraint +identyczny z istniejącym. +""" + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("bpp", "0502_autor_soft_delete"), + ("zglos_publikacje", "0027_zgloszenie_zaimportowane"), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + database_operations=[], + state_operations=[ + migrations.AlterField( + model_name="zgloszenie_publikacji_autor", + name="autor", + field=models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + to="bpp.autor", + ), + ), + ], + ), + ]