Skip to content

fix(#329): opis bibliograficzny z dysku zamiast dbtemplates (FD#329)#507

Open
mpasternak wants to merge 15 commits into
devfrom
fix-fd329-opis-z-dysku
Open

fix(#329): opis bibliograficzny z dysku zamiast dbtemplates (FD#329)#507
mpasternak wants to merge 15 commits into
devfrom
fix-fd329-opis-z-dysku

Conversation

@mpasternak

Copy link
Copy Markdown
Member

Naprawia Freshdesk FD#329https://iplweb.freshdesk.com/a/tickets/329
(Biblioteka Naukowa IHiT, bpp.ihit.waw.pl): rozdział z wydawnictwem
nadrzędnym pobranym z PBN nie pokazywał rodzica w opisie bibliograficznym.

Tło — dwie warstwy problemu

PR #409 naprawił render (szablon czyta rodzica z surowego PBN object.book
przez book_title), ale poprawka była niewidoczna w istniejących instalacjach,
bo wiersz dbtemplates opis_bibliograficzny.html zasłania plik z dysku
(loader dbtemplates pierwszy w łańcuchu). Doraźny drop_dbtemplate wymagał
ręcznego odpalenia u każdego klienta. Dodatkowo compare_dbtemplates kłamał
(czytał „dysk" przez get_template → dostawał treść z DB → zawsze „match").

Co robi ten PR (warstwa 2 „na serio")

Wyrywa opis_bibliograficzny.html z dbtemplates — dysk = jedyne źródło prawdy:

  • Model SzablonDlaOpisuBibliograficznego: FK template (→dbtemplates.Template,
    PROTECT) → pole nazwa_szablonu (CharField). Mapowanie per-model przeżywa
    (po nazwie). clean() waliduje przez get_template.
  • Migracja 0468 (atomic=False): add → backfill z template.name
    RemoveField → purge: kasuje wiersz dbtemplates (guard: tylko nazwy z
    plikiem na dysku) + czyści cache + oznacza rekordy dirty (rebuild denorma;
    async kolejka denorm odświeża w tle). perf(pbn_import): tnij szczyt pamięci importu źródeł (importuj_zrodla) #329 naprawia się samo przy
    podbiciu wydania
    — bez drop_dbtemplate, bez ręcznej akcji.
  • compare_dbtemplates naprawiony: nowy helper disk_template_source()
    czyta dysk z pominięciem loadera dbtemplates (+ kopiuje libraries/builtins,
    inaczej surowy Engine nie zna {% load prace %}). Repro-test łapie stary bug.
  • drop_dbtemplate odsprzęgnięty od usuniętego FK, współdzieli funkcję
    usun_dbtemplate_i_przebuduj (guard + delete + cache + rebuild).
  • Sprzątanie: adminy (list_display, template_updated), usunięta martwa
    fixture szablony i workaround _sync_opis_template_z_dysku w testach.

Proces

Spec przeszedł 2 rundy adversarialnego review (złapały m.in. BLOCKER: opis
idzie z @denormalized cache, nie z live-renderu → migracja MUSI robić rebuild).
Implementacja: 8 tasków TDD (subagenci), finalne whole-branch review na opus —
0 Critical/Important.

Testy

37/37 w dotkniętych obszarach (disk helper, shared fn, opis, drop/compare,
admin). Denorm end-to-end dowodzi naprawy #329 (rodzic z PBN object.book
w opisie po migracji). Migracja 0468 aplikuje się czysto na świeżym Postgresie.

Uwaga przy scalaniu

  • make baseline-update — dopiero PRZY scalaniu (reguła CLAUDE: nie w
    równoległych branchach). Migracja 0468 aplikuje się na baseline, więc CI-testy
    przechodzą; baseline odświeżamy raz, przy merge.
  • Reverse migracji jest nieodwracalny (świadomie, udokumentowane w specu).
  • FD#329 nie zamknie się automatycznie z GitHuba (to świadome — domyka „skill Bump pillow from 4.0.0 to 6.2.0 #2").

Spec: docs/superpowers/specs/2026-07-08-opis-bibliograficzny-dysk-zamiast-dbtemplates-design.md

🤖 Generated with Claude Code

mpasternak and others added 15 commits July 8, 2026 22:54
Warstwa 2 dla FD#329 "na serio": wyrwanie opis_bibliograficzny.html z
dbtemplates (dysk = jedyne zrodlo prawdy), zamiana FK dbtemplates.Template
na nazwa_szablonu w SzablonDlaOpisuBibliograficznego, migracja ktora kasuje
wiersz dbtemplates (fix samo-stosujacy sie przy podbiciu wydania) oraz
naprawa myslacego compare_dbtemplates (czytal DB zamiast dysku).

Refs #329

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…329)

Wchlaniam P1-P9 z adversarialnego self-review:
- P1 (BLOCKER): migracja musi oznaczyc rekordy dirty (opis idzie z denormu,
  nie live-render) — decyzja 1(a): rebuild_instances_of_models bez sync flush,
  async kolejka denorm odswieza w tle.
- P2: jawne wyczysc_cache_dbtemplate (delete przez model historyczny nie
  odpala sygnalow czyszczacych Redis).
- P3/P6: dopisani konsumenci FK (admin/templates.py, list_display, testy,
  fixtures, usuniecie _sync_opis_template_z_dysku).
- P4/2(A): kasowanie bezwarunkowe + log tresci (porownanie tresci nie
  odroznia starego standardu od kustomizacji po #409).
- P5: clean() przez get_template (nie 'na dysku') — brak sprzecznosci.
- P7: Engine(dirs=, app_dirs=True), nie loaders=.
- P8/P9: sprostowanie 'migracje #409' + baseline/_sync helper.

Refs #329

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Swiezy adversarialny review zlapal regresje, ktora wprowadzila poprzednia
rewizja (kasowanie "kazdej mapowanej nazwy bezwarunkowo"):

- BLOCKER (regresja): kasuj wiersz TYLKO gdy disk_template_source(name) is
  not None. DB-only custom bez pliku na dysku zostawiamy — inaczej dyndajaca
  nazwa_szablonu -> TemplateDoesNotExist -> opis wybucha przy kazdym flushu
  denorma. Guard rozbraja tez sprzecznosc z clean() 2(A).
- drop_dbtemplate: rebuild celuje przez nowe get_models_for_szablon(name),
  nie usuniete get_models_for_template(Template).
- reverse: migracja nieodwracalna (AddField NOT NULL na niepustej tabeli nie
  da sie odwrocic "best-effort").
- wyczysc_cache_dbtemplate przez transaction.on_commit (okno wyscigu).
- rebuild dziala na apps.get_model (sprostowane "potrzebuje realnych klas").
- atomic=False (mark-dirty setek tysiecy rekordow), multi-site cache TTL,
  fixture szablony bez konsumentow -> usunac, testowalnosc przez funkcje
  wyekstrahowana (brak django-test-migrations), sprostowanie flush_single legacy.

Refs #329

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rozklad na 8 taskow z konkretnym kodem: helper disk_template_source, naprawa
compare_dbtemplates, wspoldzielona usun_dbtemplate_i_przebuduj (guard+rebuild),
model FK->nazwa_szablonu + migracja, drop_dbtemplate odsprzegniecie, adminy,
usuniecie fixture, newsfragment. Spec: synchroniczne czyszczenie cache
(zamiast on_commit) — spojne z planem.

Refs #329

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…btemplates (#329)

Kopiuje libraries/builtins z domyślnego Engine — surowy Engine nie zna custom
tag-libów ({% load prace %} w opisie). Poprawka planu wzgledem pierwotnego kodu.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_filesystem_template_content używała get_template(), który idzie
łańcuchem loaderów z dbtemplates na pierwszym miejscu — dla nazwy
istniejącej w bazie zwracał treść z DB, więc porównanie było
DB-vs-DB (zawsze "match"). Teraz korzysta z disk_template_source
(Task 1), który czyta wyłącznie z dysku.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rma (#329)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ersz (#329)

Zamienia FK SzablonDlaOpisuBibliograficznego.template (->dbtemplates.Template,
PROTECT) na pole nazwa_szablonu (CharField). Migracja 0468: AddField +
backfill z istniejących wierszy Template + RemoveField(template) + purge
wiersza dbtemplate "opis_bibliograficzny.html" (usun_dbtemplate_i_przebuduj)
z rebuildem denorma, wołany PO RemoveField żeby PROTECT nie blokował.

Testy test_opis_bibliograficzny.py zmigrowane na nazwa_szablonu=; usunięty
helper _sync_opis_template_z_dysku (po purge wiersza render spada na dysk
bez potrzeby synchronizacji). Dopisane testy clean().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ja (#329)

Model SzablonDlaOpisuBibliograficznego stracił FK `template` (Task 4).
Komenda drop_dbtemplate używała jeszcze filter(template=...) i
get_models_for_template, więc była chwilowo popsuta. Przepisano ją na
współdzieloną funkcję usun_dbtemplate_i_przebuduj + get_models_for_szablon(name)
z guardem dysk-existence.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ście) (#329)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…329)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixture tworzyła wiersze dbtemplates ze starym FK (SzablonDlaOpisu-
Bibliograficznego -> Template z dbtemplates). Po wyrwaniu opisu
bibliograficznego z dbtemplates nie ma już konsumentów fixture w
testach - zweryfikowano przez git grep, że nazwa "szablony" nie
występuje jako argument żadnego testu, tylko w niepowiązanych
komentarzach/docstringach.

Usunięto też osierocony import pytest (używany wyłącznie przez
dekorator @pytest.fixture tej fixture).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Newsfragment domykający serię FD#329 (opis_bibliograficzny.html brany
z dysku zamiast dbtemplates). Przy okazji ręczne poprawki ruff w
plikach dotkniętych tą serią: UP008 (super() bez argumentów) w
admin/szablondlaopisubibliograficznego.py, I001 (kolejność importów)
w tests/test_admin/test_templateadmin.py oraz drobny trailing-blank-line
fix we fixtures/conftest.py.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI zlapal dwie rzeczy:
- Lint DJ012 x2 w przepisanym modelu (manager przed polami, Meta po __str__)
  — pre-existing na dev, ale "Lint changed files" widzi je bo plik przepisany.
  Przestawione wg Django style (pola -> manager -> Meta -> __str__).
- no_queries (wyd. ciagle/zwarte): +1 Site.objects.get_current() po skasowaniu
  wiersza dbtemplates opis_bibliograficzny.html. Zdiagnozowane: nie get_template
  (GT_CALLS puste), nie denorm (flush nie zmienia), stale O(1) indeksowane po PK,
  w produkcji amortyzowane przez SITE_CACHE (pytest-django czysci per-test).
  Prog 11->12 z komentarzem wyjasniajacym.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_lazy_mapper_nie_nadpisuje_istniejacego polegał na wierszu
zaseedowanym przez migrację 0467. Nowe pliki testowe z PR #507 przesunęły
granice pytest-split, przez co test wylądował w jednym shardzie z testem
transactional_db (własne DB per-worker), który truncatuje tabelę
Crossref_Mapper. Wtedy .filter(BOOK_CHAPTER).update(False) trafiał 0
wierszy, a _get_crossref_mapper tworzył świeży z domyślnym True → assert
False padał (flaky zależny od kolejności shardowania, nie od kodu #329).

Zamiana .filter().update() na update_or_create() gwarantuje warunek
wstępny „istnieje wiersz = False" niezależnie od tego, czy seed migracji
przetrwał, zachowując intencję testu (get_or_create nie nadpisuje
istniejącego wiersza).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant