Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/bpp/newsfragments/link-do-pbn-multi-hosted.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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.
35 changes: 23 additions & 12 deletions src/bpp/templates/browse/praca_tabela.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@
</th>
<td>
{{ praca.tekst_przed_pierwszym_autorem|default:"" }}
{# Jeden wezel <a>, 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" %}
<a href="{% url "admin:bpp_autor_change" autor.autor.pk %}">
{% else %}{% if links == "normal" %}<a href="{% url "bpp:browse_autor" autor.autor.slug %}">{% else %}
{% endif %}{% endif %}{% if links %}{{ autor.zapisany_jako }}{% else %}
{{ autor.zapisany_jako|upper }}{% endif %}{% if links == "admin" or links == "normal" %}</a>{% endif %}
[{{ autor.typ_odpowiedzialnosci.skrot|upper }}] {% endifchanged %}{% if links == "admin" or links == "normal" %}<a href="{% if links == "admin" %}{% url "admin:bpp_autor_change" autor.autor.pk %}{% else %}{% url "bpp:browse_autor" autor.autor.slug %}{% endif %}">{{ autor.zapisany_jako }}</a>{% elif links %}{{ autor.zapisany_jako }}{% else %}{{ autor.zapisany_jako|upper }}{% endif %}
{% if not forloop.last %}, {% else %}{{ praca.tekst_po_ostatnim_autorze|default:"" }}.
{% endif %}{% endfor %}
</td>
Expand Down Expand Up @@ -142,7 +144,7 @@
{% if praca.doi %}
<tr>
<th>DOI</th>
<td><a target="_blank" href="http://doi.org/{{ praca.doi }}">{{ praca.doi }}</a></td>
<td><a target="_blank" href="https://doi.org/{{ praca.doi }}">{{ praca.doi }}</a></td>
</tr>
{% endif %}

Expand Down Expand Up @@ -192,11 +194,21 @@
<tr>
<th>PBN UID:</th>
<td>
<button class="button secondary"
type="button"
data-open-url="{{ praca.link_do_pbn }}" data-target="_blank">
🔗 {{ praca.pbn_uid_id }}
</button>
{# 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 %}
<button class="button secondary"
type="button"
data-open-url="{{ pbn_url }}" data-target="_blank">
🔗 {{ praca.pbn_uid_id }}
</button>
{% else %}
{{ praca.pbn_uid_id }}
{% endif %}
{% if not request.user.is_anonymous and praca|link_do_pi:uczelnia %}
<button class="button secondary"
type="button"
Expand Down Expand Up @@ -300,7 +312,6 @@

{% endif %}
</td>
</td>
</tr>
{% endif %}
<tr>
Expand Down
23 changes: 15 additions & 8 deletions src/bpp/templates/browse/praca_tabela_mono.html
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,10 @@ <h3 class="praca-mono__card-title">
<a href="{% url "bpp:browse_praca_by_slug" praca.wydawnictwo_nadrzedne.slug %}">Zobacz wydawnictwo nadrzędne</a>
{% endif %}
{% elif praca.wydawnictwo_nadrzedne_w_pbn %}
W: <a href="{{ praca.wydawnictwo_nadrzedne_w_pbn.link_do_pbn }}" target="_blank" title="Link do PBN">
{# Jak wyżej: link do PBN wymaga uczelni z kontekstu. Bez niej #}
{# pokazujemy sam opis wydawnictwa nadrzędnego, bez linku. #}
{% link_do_pbn praca.wydawnictwo_nadrzedne_w_pbn as pbn_url %}
W: {% if pbn_url %}<a href="{{ pbn_url }}" target="_blank" title="Link do PBN">{% endif %}
{{ praca.wydawnictwo_nadrzedne_w_pbn.title }}
{% if praca.wydawnictwo_nadrzedne_w_pbn.autorzy %}
/
Expand All @@ -163,8 +166,7 @@ <h3 class="praca-mono__card-title">
{% if not forloop.last %}, {% endif %}
{% endfor %}
{% endif %}
<span class="fi-link-external praca-mono__external-link-icon"></span>
</a>
{% if pbn_url %}<span class="fi-link-external praca-mono__external-link-icon"></span></a>{% endif %}
{% endif %}
{{ praca.informacje|default:""|znak_na_koncu:", "|safe }}
{{ praca.szczegoly|default:""|safe }}
Expand Down Expand Up @@ -290,11 +292,16 @@ <h3>
title="Kopiuj identyfikator">
<i class="fi-clipboard"></i>
</button>
<button class="button open-button" type="button"
data-open-url="{{ praca.link_do_pbn }}" data-target="_blank"
title="Otwórz w PBN">
<i class="fi-arrow-right"></i> Otwórz
</button>
{# 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 %}
<button class="button open-button" type="button"
data-open-url="{{ pbn_url }}" data-target="_blank"
title="Otwórz w PBN">
<i class="fi-arrow-right"></i> Otwórz
</button>
{% endif %}
</div>
</div>
{% endif %}
Expand Down
69 changes: 66 additions & 3 deletions src/bpp/templatetags/prace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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 ``<a>`` (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 "<a href" not in opis


@pytest.mark.django_db
def test_wariant_autorzy_bez_links(praca_z_autorem):
"""Brak ``links``: nazwisko WERSALIKAMI, bez linku."""
praca, autor = praca_z_autorem
opis = praca.opis_bibliograficzny(links=None)

assert "KOWALSKI JAN" in opis
assert "<a href" not in opis
43 changes: 43 additions & 0 deletions src/bpp/tests/test_models/test_opis_bibliograficzny_wariant_pbn.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
"""Wariant opisu bibliograficznego ``browse/praca_tabela.html`` a link do PBN.

``opis_bibliograficzny()`` renderuje szablon wskazany przez
``SzablonDlaOpisuBibliograficznego`` kontekstem ``dict(praca=..., links=...)``
— BEZ requestu, więc BEZ ``uczelnia`` (context processory nie biegną).

To pole minowe dla wszystkiego, co w tym szablonie odwołuje się do
``uczelnia``: Django rozwija argumenty filtrów zachłannie, a ``{% with %}``
— w odróżnieniu od ``{% if %}`` — NIE łapie ``VariableDoesNotExist``.
Efekt: generowanie opisu (i denormalizacja) wywala się wyjątkiem.
"""

import pytest
from model_bakery import baker

from bpp.models import Wydawnictwo_Zwarte
from bpp.models.szablondlaopisubibliograficznego import (
SzablonDlaOpisuBibliograficznego,
)


@pytest.mark.django_db
def test_opis_bibliograficzny_wariant_praca_tabela_z_pbn_uid():
"""Rekord z PBN UID + wariant ``praca_tabela.html`` = opis ma się wyrenderować."""
from pbn_api.models import Publication

publication = baker.make(Publication)
praca = baker.make(Wydawnictwo_Zwarte, pbn_uid=publication)

# Wariant ustawiamy PO zapisie rekordu: denorm woła opis_bibliograficzny()
# w pre_save na jeszcze-niezapisanej instancji, a ten szablon odwołuje się
# do relacji (``praca.streszczenia.exists``), więc na bezkluczowym obiekcie
# poleciałby ValueError — to osobna, wcześniejsza sprawa niż PBN.
SzablonDlaOpisuBibliograficznego.objects.update_or_create(
model=None, defaults={"nazwa_szablonu": "browse/praca_tabela.html"}
)

opis = praca.opis_bibliograficzny()

assert praca.tytul_oryginalny in opis
# Bez uczelni w kontekście nie da się zbudować adresu PBN — ma zostać
# sam identyfikator, a NIE napis "None" w atrybucie linku.
assert "None" not in opis
43 changes: 43 additions & 0 deletions src/bpp/tests/test_templatetags/test_link_do_pi_placeholder.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
"""Filtr ``link_do_pi`` a placeholder ``NiezdefiniowanaUczelnia``.

Gdy z requestu nie da się ustalić uczelni, context processor wstawia do
kontekstu placeholder (klasa bez ``pk``), a nie ``Uczelnia``. Filtr NIE może
w takiej sytuacji degradować do ``link_do_pi(None)``: metoda modelu robi
wtedy lookup ``PublikacjaInstytucji_V2`` NIE zawężony do uczelni, a dwa
wiersze na jeden ``objectId`` to w multi-install stan POPRAWNY. Efektem
byłby ``MultipleObjectsReturned`` → raport do Rollbara i mail do adminów
za link, który i tak zostanie ukryty.
"""

import uuid as uuid_module

import pytest
from model_bakery import baker

from bpp.context_processors.uczelnia import NiezdefiniowanaUczelnia
from bpp.models import Uczelnia
from bpp.templatetags.prace import link_do_pi
from pbn_api.models import Publication, PublikacjaInstytucji_V2


@pytest.mark.django_db
def test_link_do_pi_z_placeholderem_nie_robi_lookupu(mocker):
u1 = baker.make(Uczelnia, pbn_api_root="https://pbn-u1.example.com")
u2 = baker.make(Uczelnia, pbn_api_root="https://pbn-u2.example.com")

objectId = Publication.objects.create(mongoId=baker.random_gen.gen_string(20))
for uczelnia in (u1, u2):
PublikacjaInstytucji_V2.objects.create(
uuid=uuid_module.uuid4(),
objectId=objectId,
json_data={"title": "Test", "objectId": objectId.pk},
uczelnia=uczelnia,
)

raport = mocker.patch("rollbar.report_message")
mail = mocker.patch("django.core.mail.mail_admins")

assert link_do_pi(objectId, NiezdefiniowanaUczelnia) is None

raport.assert_not_called()
mail.assert_not_called()
Loading
Loading