diff --git a/src/bpp/newsfragments/link-do-pbn-multi-hosted.bugfix.rst b/src/bpp/newsfragments/link-do-pbn-multi-hosted.bugfix.rst new file mode 100644 index 000000000..4fbc6663e --- /dev/null +++ b/src/bpp/newsfragments/link-do-pbn-multi-hosted.bugfix.rst @@ -0,0 +1,8 @@ +Na stronie rekordu przycisk "Otwórz w PBN" prowadził do adresu ``None`` +w instalacjach obsługujących więcej niż jedną uczelnię. Link jest teraz +budowany na podstawie adresu PBN uczelni, której stronę ogląda użytkownik, +a gdy uczelni nie da się ustalić — przycisk nie jest pokazywany. + +Poprawka objęła też wariant opisu bibliograficznego +``browse/praca_tabela.html``: usunięty zdublowany znacznik zamykający +komórkę tabeli oraz podniesiony do HTTPS link do doi.org. diff --git a/src/bpp/templates/browse/praca_tabela.html b/src/bpp/templates/browse/praca_tabela.html index 5a6caa564..38bd03429 100644 --- a/src/bpp/templates/browse/praca_tabela.html +++ b/src/bpp/templates/browse/praca_tabela.html @@ -23,12 +23,14 @@ {{ praca.tekst_przed_pierwszym_autorem|default:"" }} + {# Jeden wezel , zamiast otwarcia i zamkniecia w osobnych #} + {# galeziach {% if %}. Tamten wzorzec djlint widzi jako sieroty #} + {# (H025) i gubil przez to parsowanie reszty pliku — a plik jest #} + {# zywym wariantem generatora opisu, wiec ma podlegac kontroli. #} + {# Semantyka bez zmian: admin/normal → link, inne prawdziwe #} + {# `links` → sam zapis, brak `links` → WERSALIKI. #} {% for autor in praca.autorzy_dla_opisu %}{% ifchanged autor.typ_odpowiedzialnosci %} - [{{ autor.typ_odpowiedzialnosci.skrot|upper }}] {% endifchanged %}{% if links == "admin" %} - - {% else %}{% if links == "normal" %}{% else %} - {% endif %}{% endif %}{% if links %}{{ autor.zapisany_jako }}{% else %} - {{ autor.zapisany_jako|upper }}{% endif %}{% if links == "admin" or links == "normal" %}{% endif %} + [{{ autor.typ_odpowiedzialnosci.skrot|upper }}] {% endifchanged %}{% if links == "admin" or links == "normal" %}{{ autor.zapisany_jako }}{% elif links %}{{ autor.zapisany_jako }}{% else %}{{ autor.zapisany_jako|upper }}{% endif %} {% if not forloop.last %}, {% else %}{{ praca.tekst_po_ostatnim_autorze|default:"" }}. {% endif %}{% endfor %} @@ -142,7 +144,7 @@ {% if praca.doi %} DOI - {{ praca.doi }} + {{ praca.doi }} {% endif %} @@ -192,11 +194,21 @@ PBN UID: - + {# Link do PBN buduje się z pbn_api_root uczelni oglądającego; #} + {# w multi-hosted bez tego degradował do napisu "None". Tag, #} + {# nie filtr: ten szablon renderuje też opis_bibliograficzny() #} + {# kontekstem bez `uczelnia`, a argument filtra Django rozwija #} + {# zachłannie → VariableDoesNotExist. #} + {% link_do_pbn praca as pbn_url %} + {% if pbn_url %} + + {% else %} + {{ praca.pbn_uid_id }} + {% endif %} {% if not request.user.is_anonymous and praca|link_do_pi:uczelnia %} - + {# Link do PBN buduje się z pbn_api_root uczelni oglądającego; #} + {# w multi-hosted bez tego degradował do napisu "None". #} + {% link_do_pbn praca as pbn_url %} + {% if pbn_url %} + + {% endif %} {% endif %} diff --git a/src/bpp/templatetags/prace.py b/src/bpp/templatetags/prace.py index b877990ab..13454b922 100644 --- a/src/bpp/templatetags/prace.py +++ b/src/bpp/templatetags/prace.py @@ -165,23 +165,86 @@ def jsonify(value): return mark_safe(result) +#: Wartownik odróżniający „filtr zawołany bez argumentu" od „argument podany, +#: ale uczelni nie dało się ustalić". To NIE to samo: pierwsze zostawia +#: legacy-fallback w metodzie modelu, drugie musi zwrócić brak linku. +_UCZELNIA_NIE_PODANA = object() + + +def _uczelnia_albo_none(uczelnia): + """Znormalizuj uczelnię z kontekstu szablonu do ``Uczelnia`` albo ``None``. + + Context processor ``bpp.context_processors.uczelnia`` wstawia do kontekstu + ``NiezdefiniowanaUczelnia`` (placeholder bez ``pk``), gdy z requestu nie da + się ustalić uczelni. Dla metod modelu to NIE jest uczelnia — przekazanie go + dalej wysypałoby render na ``pbn_api_root``. Rozpoznajemy po braku ``pk``. + """ + if getattr(uczelnia, "pk", None) is None: + return None + return uczelnia + + @register.filter(name="link_do_pi") -def link_do_pi(praca, uczelnia=None): +def link_do_pi(praca, uczelnia=_UCZELNIA_NIE_PODANA): """Zwróć link do Profilu Instytucji rekordu dla danej uczelni. Multi-hosted (audyt uczelnia, track 7b): templejt nie umie podać argumentu metodzie, więc filtr przekazuje uczelnię oglądającego (z kontekstu) do ``praca.link_do_pi(uczelnia)`` — link wskazuje na PBN-root TEJ uczelni i rozwiązuje wiersz ``PublikacjaInstytucji_V2`` otagowany TĄ uczelnią. - ``uczelnia=None`` (brak uczelni w kontekście) → brak linku (NIE ma - „uczelni domyślnej"). + + Gdy uczelnia zostaje podana, ale nie da się jej ustalić (placeholder + ``NiezdefiniowanaUczelnia``), zwracamy brak linku BEZ wołania metody. + Degradacja do ``link_do_pi(None)`` robiłaby lookup nie zawężony do + uczelni — a w multi-install dwa wiersze ``PublikacjaInstytucji_V2`` na + jeden ``objectId`` to stan POPRAWNY, więc ``MultipleObjectsReturned`` + wyzwoliłoby fałszywy alarm do Rollbara i mail do adminów za link, który + i tak zostanie ukryty. """ method = getattr(praca, "link_do_pi", None) if method is None: return None + + if uczelnia is _UCZELNIA_NIE_PODANA: + return method() + + uczelnia = _uczelnia_albo_none(uczelnia) + if uczelnia is None: + return None return method(uczelnia=uczelnia) +@register.simple_tag(takes_context=True, name="link_do_pbn") +def link_do_pbn(context, obiekt): + """Zwróć link do rekordu w PBN dla uczelni oglądającego. + + Użycie: ``{% link_do_pbn praca as pbn_url %}``. + + Multi-hosted: szablon nie umie podać argumentu metodzie, więc + ``{{ praca.link_do_pbn }}`` wołało ją bez uczelni → + ``get_single_uczelnia_or_none()`` przy >1 uczelni zwraca ``None`` → + metoda zwraca ``None`` → Django renderuje dosłownie napis ``None``. + + Dlaczego TAG z ``takes_context``, a nie filtr z argumentem ``uczelnia``: + Django rozwija argumenty filtrów zachłannie i poza blokiem ``try``, więc + ``{% with x=praca|link_do_pbn:uczelnia %}`` wysypuje się przez + ``VariableDoesNotExist`` wszędzie tam, gdzie ``uczelnia`` nie ma + w kontekście. A jest taki render: ``opis_bibliograficzny()`` renderuje + wariant szablonu kontekstem ``dict(praca=..., links=...)``, bez requestu + i bez context processorów. Tag czyta kontekst sam, więc brak zmiennej to + zwykłe „nie ma uczelni", a nie błąd. + + Brak uczelni → ``None`` (szablon ma wtedy nie renderować linku). Dla + ``opis_bibliograficzny_cache`` to jedyne poprawne zachowanie w + multi-install: opis jest cache'owany PER REKORD, nie per uczelnia, więc + link zależny od uczelni oglądającego przeciekłby między tenantami. + """ + method = getattr(obiekt, "link_do_pbn", None) + if method is None: + return None + return method(uczelnia=_uczelnia_albo_none(context.get("uczelnia"))) + + @register.simple_tag def opis_bibliograficzny_cache(pk): from bpp.models.cache import Rekord diff --git a/src/bpp/tests/test_models/test_opis_bibliograficzny_wariant_autorzy.py b/src/bpp/tests/test_models/test_opis_bibliograficzny_wariant_autorzy.py new file mode 100644 index 000000000..58b0e43f1 --- /dev/null +++ b/src/bpp/tests/test_models/test_opis_bibliograficzny_wariant_autorzy.py @@ -0,0 +1,76 @@ +"""Wariant opisu ``browse/praca_tabela.html`` — linkowanie autorów. + +Test charakteryzujący: spisuje zachowanie pętli autorów DLA KAŻDEGO trybu +``links`` **przed** refaktorem bloku ```` (rozbity na dwie gałęzie `{% if %}`, +przez co djlint widzi sieroty). Refaktor ma być bezzmianowy semantycznie — +ten test tego pilnuje. + +Kontrakt: + +* ``links="admin"`` → nazwisko w linku do admina, bez zmiany wielkości liter, +* ``links="normal"`` → nazwisko w linku do strony autora, j.w., +* ``links`` prawdziwe, ale inne → nazwisko bez linku, bez zmiany wielkości, +* ``links`` fałszywe → nazwisko WERSALIKAMI, bez linku. +""" + +import pytest +from model_bakery import baker + +from bpp.models import Autor, Jednostka, Wydawnictwo_Zwarte +from bpp.models.szablondlaopisubibliograficznego import ( + SzablonDlaOpisuBibliograficznego, +) + + +@pytest.fixture +def praca_z_autorem(db): + jednostka = baker.make(Jednostka, skupia_pracownikow=True) + autor = baker.make(Autor, nazwisko="Kowalski", imiona="Jan") + praca = baker.make(Wydawnictwo_Zwarte) + praca.dodaj_autora(autor, jednostka, zapisany_jako="Kowalski Jan", kolejnosc=0) + + SzablonDlaOpisuBibliograficznego.objects.update_or_create( + model=None, defaults={"nazwa_szablonu": "browse/praca_tabela.html"} + ) + return praca, autor + + +@pytest.mark.django_db +def test_wariant_autorzy_links_admin(praca_z_autorem): + praca, autor = praca_z_autorem + opis = praca.opis_bibliograficzny(links="admin") + + assert f"/admin/bpp/autor/{autor.pk}/change/" in opis + assert "Kowalski Jan" in opis + assert "KOWALSKI JAN" not in opis + + +@pytest.mark.django_db +def test_wariant_autorzy_links_normal(praca_z_autorem): + praca, autor = praca_z_autorem + opis = praca.opis_bibliograficzny(links="normal") + + assert f"/bpp/autor/{autor.slug}/" in opis + assert "Kowalski Jan" in opis + assert "KOWALSKI JAN" not in opis + + +@pytest.mark.django_db +def test_wariant_autorzy_links_inny_ciag(praca_z_autorem): + """``links`` prawdziwe, ale nie admin/normal: nazwisko bez linku.""" + praca, autor = praca_z_autorem + opis = praca.opis_bibliograficzny(links="cokolwiek") + + assert "Kowalski Jan" in opis + assert "KOWALSKI JAN" not in opis + assert " + +Zamiast tego link ma wskazywać PBN uczelni z hosta requestu — analogicznie do +``link_do_pi`` (filtr z uczelnią z kontekstu) i do strony autora (FD#390). +""" + +import pytest +from django.contrib.contenttypes.models import ContentType +from django.urls import reverse +from model_bakery import baker + +from bpp.models import Uczelnia, Wydawnictwo_Zwarte + + +@pytest.fixture +def dwie_uczelnie(db): + """Multi-homed: dwie uczelnie → ``get_single_uczelnia_or_none() is None``.""" + uczelnia_a = baker.make(Uczelnia, pbn_api_root="https://pbn-a.example.com") + uczelnia_a.site.domain = "uczelnia-a.example.com" + uczelnia_a.site.save() + + baker.make(Uczelnia) # druga → single == None (multi-homed) + return uczelnia_a + + +@pytest.mark.django_db +def test_link_do_pbn_na_stronie_rekordu_uzywa_uczelni_z_hosta( + client, settings, dwie_uczelnie +): + settings.ALLOWED_HOSTS = ["*"] + + from pbn_api.models import Publication + + publication = baker.make(Publication) + praca = baker.make(Wydawnictwo_Zwarte, pbn_uid=publication) + + url = reverse( + "bpp:browse_praca", + args=( + ContentType.objects.get(app_label="bpp", model="wydawnictwo_zwarte").pk, + praca.pk, + ), + ) + resp = client.get(url, HTTP_HOST="uczelnia-a.example.com", follow=True) + assert resp.status_code == 200 + + content = resp.content.decode("utf-8") + + oczekiwany = ( + f"https://pbn-a.example.com/core/#/publication/view/{publication.pk}/current" + ) + assert oczekiwany in content, ( + "Link do PBN na stronie rekordu powinien używać pbn_api_root uczelni " + "z hosta; zamiast tego degraduje do None (bug multi-homed)." + ) + assert 'data-open-url="None"' not in content + assert "data-open-url=None" not in content + + +@pytest.mark.django_db +def test_link_do_pbn_na_stronie_rekordu_single_install(client, settings): + """Regresja: w instalacji z JEDNĄ uczelnią link nadal się renderuje.""" + settings.ALLOWED_HOSTS = ["*"] + + from pbn_api.models import Publication + + uczelnia = baker.make(Uczelnia, pbn_api_root="https://pbn.example.com") + + publication = baker.make(Publication) + praca = baker.make(Wydawnictwo_Zwarte, pbn_uid=publication) + + url = reverse( + "bpp:browse_praca", + args=( + ContentType.objects.get(app_label="bpp", model="wydawnictwo_zwarte").pk, + praca.pk, + ), + ) + resp = client.get(url, HTTP_HOST=uczelnia.site.domain, follow=True) + assert resp.status_code == 200 + + content = resp.content.decode("utf-8") + assert ( + f"https://pbn.example.com/core/#/publication/view/{publication.pk}/current" + in content + )