fix(pbn): przepinaj osobę gdy PBN zmienił personId, zamiast ją pomijać

src/bpp/newsfragments/+pbn-polonuuid-kolizja.bugfix.rst

@@ -0,0 +1,8 @@
+Import osób z API instytucji PBN radzi sobie z osobami, którym PBN zmienił
+identyfikator. ``polonUuid`` (identyfikator z POL-onu) jest stabilną
+tożsamością osoby, a ``personId`` PBN potrafi zmienić — np. po scaleniu
+zdublowanych profili. Import dopasowywał wpis wyłącznie po ``personId``,
+więc taka osoba rozbijała się o unikalność ``polonUuid`` i była pomijana;
+teraz jej wpis jest przepinany na nowy identyfikator. Osoby bez
+``polonUuid`` są pomijane z czytelnym komunikatem w logu, zamiast trafiać
+do monitoringu jako „konflikt tożsamości".

src/pbn_integrator/tests/test_osoba_z_instytucji.py

@@ -0,0 +1,140 @@
+"""Testy zapisu osób z API instytucji PBN (`_zapisz_osobe_z_instytucji`).
+
+Kontekst: ``OsobaZInstytucji`` ma DWA klucze unikalne — ``personId``
+(OneToOne na ``Scientist``) oraz ``polonUuid``. PBN potrafi wystawić tę samą
+fizyczną osobę pod NOWYM ``personId``, zachowując jej ``polonUuid`` z POL-onu
+(np. po scaleniu zdublowanych profili). Import dopasowywał wiersz wyłącznie po
+``personId``, więc taka osoba leciała na INSERT i rozbijała się o unikalność
+``polonUuid``.
+"""
+
+import uuid
+
+import pytest
+from model_bakery import baker
+
+from pbn_api.models import Scientist
+from pbn_api.models.institution import Institution
+from pbn_api.models.osoba_z_instytucji import OsobaZInstytucji
+from pbn_integrator.utils.scientists import _zapisz_osobe_z_instytucji
+
+
+@pytest.fixture
+def instytucja(db):
+    return baker.make(Institution)
+
+
+def _person(scientist, instytucja, polon_uuid, **kw):
+    dane = {
+        "personId": scientist.pk,
+        "institutionId": instytucja.pk,
+        "firstName": "Jan",
+        "lastName": "Kowalski",
+        "institutionName": "Instytut Testowy",
+        "polonUuid": str(polon_uuid),
+        "phdStudent": False,
+        "title": "dr hab.",
+        "from": "2020-01-01",
+        "to": "2030-12-31",
+    }
+    dane.update(kw)
+    return dane
+
+
+@pytest.mark.django_db
+def test_zapisz_osobe_z_instytucji_zapisuje_wszystkie_pola(instytucja):
+    scientist = baker.make(Scientist)
+    polon_uuid = uuid.uuid4()
+
+    assert _zapisz_osobe_z_instytucji(_person(scientist, instytucja, polon_uuid))
+
+    osoba = OsobaZInstytucji.objects.get()
+    assert osoba.personId_id == scientist.pk
+    assert osoba.institutionId_id == instytucja.pk
+    assert str(osoba.polonUuid) == str(polon_uuid)
+    assert osoba.firstName == "Jan"
+    assert osoba.lastName == "Kowalski"
+    assert osoba.institutionName == "Instytut Testowy"
+    assert osoba.title == "dr hab."
+    assert osoba.phdStudent is False
+    assert str(osoba._from) == "2020-01-01"
+    assert str(osoba._to) == "2030-12-31"
+
+
+@pytest.mark.django_db
+def test_zapisz_osobe_bez_polonuuid_pomija_bez_raportu(instytucja, mocker):
+    """Brak ``polonUuid`` (kolumna NOT NULL) to nie „konflikt tożsamości".
+
+    Wcześniej dowiadywaliśmy się o tym okrężnie — przez IntegrityError na
+    NOT NULL, raportowany do Rollbara pod mylącą etykietą konfliktu.
+    """
+    report = mocker.patch("pbn_integrator.utils.scientists.rollbar.report_exc_info")
+    scientist = baker.make(Scientist)
+
+    dane = _person(scientist, instytucja, uuid.uuid4())
+    del dane["polonUuid"]
+
+    assert not _zapisz_osobe_z_instytucji(dane)
+    assert not report.called
+    assert not OsobaZInstytucji.objects.exists()
+
+
+@pytest.mark.django_db
+def test_zapisz_osobe_przepina_wpis_gdy_pbn_zmienil_personId(instytucja):
+    """Ten sam ``polonUuid`` pod nowym ``personId`` PRZEPINA istniejący wiersz.
+
+    Regresja (Rollbar #1523): leciał ``IntegrityError`` na
+    ``pbn_api_osobazinstytucji_polonUuid_key``, a osoba była pomijana —
+    czyli nowa tożsamość PBN nigdy nie trafiała do bazy i błąd wracał
+    przy każdym kolejnym imporcie.
+    """
+    polon_uuid = uuid.uuid4()
+    stary = baker.make(Scientist)
+    nowy = baker.make(Scientist)
+    nowa_instytucja = baker.make(Institution)
+
+    assert _zapisz_osobe_z_instytucji(_person(stary, instytucja, polon_uuid))
+    pierwotny_pk = OsobaZInstytucji.objects.get().pk
+
+    assert _zapisz_osobe_z_instytucji(
+        _person(nowy, nowa_instytucja, polon_uuid, firstName="Janina")
+    )
+
+    # Nadal JEDEN wiersz — ``polonUuid`` jest tożsamością osoby...
+    osoba = OsobaZInstytucji.objects.get()
+    # ...i to TEN SAM wiersz: przepięty, nie skasowany i odtworzony. FK z
+    # deduplikator_autorow (``main_osoba_z_instytucji``, on_delete=SET_NULL)
+    # przeżyłby podmianę jako NULL, więc pilnujemy pk.
+    assert osoba.pk == pierwotny_pk
+    assert osoba.personId_id == nowy.pk
+    assert osoba.firstName == "Janina"
+    # ``institutionId`` MUSI się odświeżyć — na nim stoi scoping per-uczelnia
+    # (patrz bpp/views/autocomplete/authors.py: institutionId_id == pbn_uid).
+    assert osoba.institutionId_id == nowa_instytucja.pk
+
+
+@pytest.mark.django_db
+def test_zapisz_osobe_pomija_gdy_tozsamosci_nie_da_sie_pogodzic(instytucja, mocker):
+    """Dwie osobne tożsamości PBN nie dają się scalić — pomijamy, nie wywalamy.
+
+    Gdy nowy ``personId`` ma JUŻ swój wiersz (z innym ``polonUuid``),
+    przepięcie zderza się z unikalnością ``personId``. Scalenie to decyzja
+    o danych, nie poprawka techniczna: raportujemy do Rollbara i idziemy
+    dalej, zamiast przerywać cały import.
+    """
+    report = mocker.patch("pbn_integrator.utils.scientists.rollbar.report_exc_info")
+    pierwsza = baker.make(Scientist)
+    druga = baker.make(Scientist)
+    uuid_a, uuid_b = uuid.uuid4(), uuid.uuid4()
+
+    assert _zapisz_osobe_z_instytucji(_person(pierwsza, instytucja, uuid_a))
+    assert _zapisz_osobe_z_instytucji(_person(druga, instytucja, uuid_b))
+
+    # Osoba spod uuid_a przychodzi teraz pod personId, który ma już swój wiersz:
+    assert not _zapisz_osobe_z_instytucji(_person(druga, instytucja, uuid_a))
+
+    assert report.called
+    # Raport musi nieść treść naruszonego ograniczenia — bez tego kolizja
+    # personId, NOT NULL i każdy inny IntegrityError wyglądają identycznie.
+    assert "constraint" in report.call_args.kwargs["extra_data"]
+    assert OsobaZInstytucji.objects.count() == 2

src/pbn_integrator/utils/scientists.py

@@ -71,54 +71,97 @@ def pobierz_i_zapisz_dane_jednej_osoby(
 def _zapisz_osobe_z_instytucji(person):
     """Save a person from institution to OsobaZInstytucji model.
 
-    Handles IntegrityError for polonUuid conflicts gracefully.
+    ``OsobaZInstytucji`` ma DWA klucze unikalne: ``personId`` (OneToOne na
+    ``Scientist``) i ``polonUuid``. To ``polonUuid`` — identyfikator z POL-onu
+    — jest stabilną tożsamością fizycznej osoby; ``personId`` PBN potrafi
+    zmienić (np. po scaleniu zdublowanych profili). Dlatego dopasowujemy
+    wiersz NAJPIERW po ``polonUuid``, a ``personId`` traktujemy jako zwykłe
+    pole do zaktualizowania. Odwrotna kolejność (match wyłącznie po
+    ``personId``) leciała na INSERT i rozbijała się o unikalność
+    ``polonUuid``, a osoba była pomijana — czyli nowa tożsamość PBN nigdy nie
+    trafiała do bazy i ten sam błąd wracał przy każdym kolejnym imporcie.
 
     Args:
         person: Person data dictionary from PBN API.
 
     Returns:
-        True if saved successfully, False if skipped due to polonUuid conflict.
+        True if saved successfully, False if the person was skipped — brak
+        ``polonUuid`` (tylko log) albo błąd integralności, np. kolizja
+        ``personId`` (log + Rollbar). Import całej kadry leci dalej.
     """
     from pbn_api.models.institution import Institution
     from pbn_api.models.osoba_z_instytucji import OsobaZInstytucji
 
+    polon_uuid = person.get("polonUuid")
+    if not polon_uuid:
+        # ``polonUuid`` jest NOT NULL, więc bez niego i tak nie ma czego
+        # zapisać. Mówimy to wprost, zamiast dowiadywać się tego okrężnie
+        # przez IntegrityError i raportować jako "konflikt tożsamości".
+        logger.info(
+            f"Pomijam osobę {person.get('personId')} — PBN nie podał polonUuid."
+        )
+        return False
+
+    dane = {
+        "firstName": person.get("firstName", ""),
+        "lastName": person.get("lastName", ""),
+        "institutionName": person.get("institutionName", ""),
+        "title": person.get("title") or "",
+        "phdStudent": person.get("phdStudent", False),
+        "_from": person.get("from"),
+        "_to": person.get("to"),
+    }
+
     try:
         with transaction.atomic():
-            OsobaZInstytucji.objects.update_or_create(
-                personId=Scientist.objects.get(pk=person["personId"]),
-                defaults={
-                    "firstName": person.get("firstName", ""),
-                    "lastName": person.get("lastName", ""),
-                    "institutionId": Institution.objects.get(
-                        pk=person["institutionId"]
-                    ),
-                    "institutionName": person.get("institutionName", ""),
-                    "title": person.get("title") or "",
-                    "polonUuid": person.get("polonUuid"),
-                    "phdStudent": person.get("phdStudent", False),
-                    "_from": person.get("from"),
-                    "_to": person.get("to"),
-                },
-            )
+            scientist = Scientist.objects.get(pk=person["personId"])
+            instytucja = Institution.objects.get(pk=person["institutionId"])
+
+            osoba = OsobaZInstytucji.objects.filter(polonUuid=polon_uuid).first()
+
+            if osoba is not None:
+                # Ta sama osoba z POL-onu — przepnij wiersz na (być może
+                # nowy) identyfikator PBN i odśwież dane.
+                osoba.personId = scientist
+                osoba.institutionId = instytucja
+                for pole, wartosc in dane.items():
+                    setattr(osoba, pole, wartosc)
+                osoba.save()
+            else:
+                OsobaZInstytucji.objects.update_or_create(
+                    personId=scientist,
+                    defaults={
+                        **dane,
+                        "institutionId": instytucja,
+                        "polonUuid": polon_uuid,
+                    },
+                )
         return True
     except IntegrityError as e:
-        if "polonUuid" in str(e):
-            # Loguj konflikt polonUuid do Rollbar jako ostrzeżenie
-            rollbar.report_exc_info(
-                sys.exc_info(),
-                extra_data={
-                    "personId": person.get("personId"),
-                    "polonUuid": person.get("polonUuid"),
-                    "firstName": person.get("firstName"),
-                    "lastName": person.get("lastName"),
-                },
-            )
-            logger.info(
-                f"UWAGA: Konflikt polonUuid dla osoby {person.get('personId')}: "
-                f"{person.get('polonUuid')}. Pomijam wpis (zalogowano do Rollbar)."
-            )
-            return False
-        raise  # Inne błędy IntegrityError propaguj
+        # Świadomie SZEROKO: import całej kadry uczelni nie ma padać przez
+        # jedną wadliwą osobę. Trafia tu m.in.:
+        #  - kolizja personId (nowy personId ma już swój wiersz z innym
+        #    polonUuid — scalenie dwóch tożsamości PBN to decyzja o danych,
+        #    nie poprawka techniczna),
+        #  - NOT NULL na polach, które PBN przysłał jako null,
+        #  - każdy inny błąd integralności.
+        # Dlatego do Rollbara idzie treść naruszonego ograniczenia — bez niej
+        # wszystkie te przypadki wyglądają w raporcie identycznie.
+        rollbar.report_exc_info(
+            sys.exc_info(),
+            extra_data={
+                "personId": person.get("personId"),
+                "polonUuid": polon_uuid,
+                "firstName": person.get("firstName"),
+                "lastName": person.get("lastName"),
+                "constraint": str(e),
+            },
+        )
+        logger.info(
+            f"UWAGA: Błąd integralności dla osoby {person.get('personId')} "
+            f"(polonUuid {polon_uuid}): {e}. Pomijam wpis (zalogowano do Rollbar)."
+        )
+        return False
 
 
 def _get_max_workers():