From 51df01159c9b9e2ed4a756f806d9c199225f3878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 28 Mar 2026 11:00:30 +0100 Subject: [PATCH] Fix #316: require parent publication when importing chapters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The publication importer now requires selecting a parent publication (wydawnictwo nadrzędne) when importing chapters. On the source step, if the charakter_formalny indicates a chapter, two Select2 autocomplete fields appear for BPP book or PBN publication selection. Validation ensures exactly one is provided. The parent publication is displayed in the review step and set on the created Wydawnictwo_Zwarte record. Co-Authored-By: Claude Opus 4.6 (1M context) --- report.md | 63 ++++++++++ src/importer_publikacji/forms.py | 12 ++ ...005_importsession_wydawnictwo_nadrzedne.py | 39 ++++++ src/importer_publikacji/models.py | 14 +++ .../partials/step_review.html | 18 +++ .../partials/step_source.html | 88 +++++++++++++ src/importer_publikacji/views.py | 116 +++++++++++++----- 7 files changed, 322 insertions(+), 28 deletions(-) create mode 100644 report.md create mode 100644 src/importer_publikacji/migrations/0005_importsession_wydawnictwo_nadrzedne.py diff --git a/report.md b/report.md new file mode 100644 index 000000000..20e294081 --- /dev/null +++ b/report.md @@ -0,0 +1,63 @@ +# Ticket #316: Możliwość stworzenia rozdziału bez wydawnictwa nadrzędnego + +## Problem + +The publication importer (`importer_publikacji`) allowed creating chapters +(`Wydawnictwo_Zwarte` records with `charakter_formalny.charakter_ogolny == "roz"`) +without a parent publication (`wydawnictwo_nadrzedne`). This created orphan chapter +records — chapters that don't belong to any book — which violates the data model's +semantic constraint. The bug was triggered by BibTeX imports of `@inbook`/`@incollection` +entries, where the importer's source step (step 3) had no UI for selecting a parent +publication. + +## Changes + +### 1. `src/importer_publikacji/models.py` +- Added two nullable ForeignKey fields to `ImportSession`: + - `wydawnictwo_nadrzedne` → `bpp.Wydawnictwo_Zwarte` + - `wydawnictwo_nadrzedne_w_pbn` → `pbn_api.Publication` + +### 2. `src/importer_publikacji/migrations/0005_importsession_wydawnictwo_nadrzedne.py` +- New migration adding both FK fields to `ImportSession`. + +### 3. `src/importer_publikacji/forms.py` +- Added `wydawnictwo_nadrzedne` and `wydawnictwo_nadrzedne_w_pbn` `ModelChoiceField` + fields to `SourceForm` (both `required=False`, conditional validation in view). + +### 4. `src/importer_publikacji/views.py` +- Added `_is_chapter()` helper to detect chapters by `charakter_ogolny`. +- Modified `_source_context()` to pass `is_chapter` flag and parent publication + objects for Select2 pre-population. +- Modified `SourceView.post()` to validate that chapters have exactly one parent + publication (either BPP or PBN, not both, not neither). +- Modified `_create_wydawnictwo_zwarte()` to set `wydawnictwo_nadrzedne` and/or + `wydawnictwo_nadrzedne_w_pbn` on the created record. + +### 5. `src/importer_publikacji/templates/.../step_source.html` +- Added conditional UI block for chapters: shows two Select2 autocomplete fields + for parent publication selection (BPP book or PBN publication). +- Added JavaScript to initialize Select2 AJAX on both fields using existing + autocomplete URLs (`wydawnictwo-nadrzedne-autocomplete`, + `wydawnictwo-nadrzedne-w-pbn-autocomplete`). + +### 6. `src/importer_publikacji/templates/.../step_review.html` +- Added display of parent publication in the review step summary table. + +## How to Verify + +1. Start the development server and open the publication importer. +2. Import a BibTeX `@inbook` or `@incollection` entry. +3. On step 2 (Verify), confirm the charakter formalny is set to a chapter type. +4. On step 3 (Source), verify: + - The parent publication section appears with "Rozdział wymaga wydawnictwa + nadrzędnego" callout. + - Two Select2 fields are shown: "Wydawnictwo nadrzędne (BPP)" and + "Wydawnictwo nadrzędne (PBN)". + - Attempting to proceed without selecting a parent shows validation error. + - Selecting both a BPP and PBN parent shows mutual exclusivity error. + - Selecting exactly one parent allows proceeding. +5. On step 5 (Review), verify the parent publication appears in the summary. +6. After creation, verify the `Wydawnictwo_Zwarte` record has `wydawnictwo_nadrzedne` + set correctly. +7. Import a regular book (non-chapter) and verify step 3 does NOT show the parent + publication fields. diff --git a/src/importer_publikacji/forms.py b/src/importer_publikacji/forms.py index 88e6f5f25..b4ad32cc1 100644 --- a/src/importer_publikacji/forms.py +++ b/src/importer_publikacji/forms.py @@ -12,8 +12,10 @@ Jezyk, Typ_KBN, Wydawca, + Wydawnictwo_Zwarte, Zrodlo, ) +from pbn_api.models import Publication as PBNPublication from .providers import ( get_available_providers, @@ -135,6 +137,16 @@ class SourceForm(forms.Form): max_length=256, required=False, ) + wydawnictwo_nadrzedne = forms.ModelChoiceField( + queryset=Wydawnictwo_Zwarte.objects.all(), + label="Wydawnictwo nadrzędne", + required=False, + ) + wydawnictwo_nadrzedne_w_pbn = forms.ModelChoiceField( + queryset=PBNPublication.objects.all(), + label="Wydawnictwo nadrzędne w PBN", + required=False, + ) class AuthorMatchForm(forms.Form): diff --git a/src/importer_publikacji/migrations/0005_importsession_wydawnictwo_nadrzedne.py b/src/importer_publikacji/migrations/0005_importsession_wydawnictwo_nadrzedne.py new file mode 100644 index 000000000..700a99776 --- /dev/null +++ b/src/importer_publikacji/migrations/0005_importsession_wydawnictwo_nadrzedne.py @@ -0,0 +1,39 @@ +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("bpp", "0001_initial"), + ("pbn_api", "0001_initial"), + ( + "importer_publikacji", + "0004_rename_user_to_created_by_add_modified_by", + ), + ] + + operations = [ + migrations.AddField( + model_name="importsession", + name="wydawnictwo_nadrzedne", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="bpp.wydawnictwo_zwarte", + verbose_name="wydawnictwo nadrzędne", + ), + ), + migrations.AddField( + model_name="importsession", + name="wydawnictwo_nadrzedne_w_pbn", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="pbn_api.publication", + verbose_name="wydawnictwo nadrzędne w PBN", + ), + ), + ] diff --git a/src/importer_publikacji/models.py b/src/importer_publikacji/models.py index 3bb79422c..75bf73ec3 100644 --- a/src/importer_publikacji/models.py +++ b/src/importer_publikacji/models.py @@ -90,6 +90,20 @@ class Status(models.TextChoices): blank=True, verbose_name="wydawca", ) + wydawnictwo_nadrzedne = models.ForeignKey( + "bpp.Wydawnictwo_Zwarte", + on_delete=models.SET_NULL, + null=True, + blank=True, + verbose_name="wydawnictwo nadrzędne", + ) + wydawnictwo_nadrzedne_w_pbn = models.ForeignKey( + "pbn_api.Publication", + on_delete=models.SET_NULL, + null=True, + blank=True, + verbose_name="wydawnictwo nadrzędne w PBN", + ) jezyk = models.ForeignKey( "bpp.Jezyk", on_delete=models.SET_NULL, diff --git a/src/importer_publikacji/templates/importer_publikacji/partials/step_review.html b/src/importer_publikacji/templates/importer_publikacji/partials/step_review.html index fe299c00b..93de78232 100644 --- a/src/importer_publikacji/templates/importer_publikacji/partials/step_review.html +++ b/src/importer_publikacji/templates/importer_publikacji/partials/step_review.html @@ -64,6 +64,24 @@
Dane publikacji
{{ session.wydawca }} {% endif %} + {% if session.wydawnictwo_nadrzedne %} + + Wydawnictwo nadrzędne + + {{ session.wydawnictwo_nadrzedne }} + + + {% endif %} + {% if session.wydawnictwo_nadrzedne_w_pbn %} + + + Wydawnictwo nadrzędne (PBN) + + + {{ session.wydawnictwo_nadrzedne_w_pbn }} + + + {% endif %} {% if data.volume %} Tom diff --git a/src/importer_publikacji/templates/importer_publikacji/partials/step_source.html b/src/importer_publikacji/templates/importer_publikacji/partials/step_source.html index 7149fbac9..8d7a3498a 100644 --- a/src/importer_publikacji/templates/importer_publikacji/partials/step_source.html +++ b/src/importer_publikacji/templates/importer_publikacji/partials/step_source.html @@ -47,6 +47,58 @@

Podaj wydawcę lub wpisz szczegóły wydawcy.

+ + {% if is_chapter %} +
+
+ + Rozdział wymaga wydawnictwa + nadrzędnego. + + Wybierz istniejącą książkę z BPP + lub publikację z PBN (jedno z dwóch). +
+
+
+ + +
+
+ + +
+
+ {% endif %} {% else %} {# Wydawnictwo ciągłe: źródło wymagane #}
@@ -112,3 +164,39 @@

Przetwarzanie...

+ +{% if is_chapter %} + +{% endif %} diff --git a/src/importer_publikacji/views.py b/src/importer_publikacji/views.py index a21e1c371..b431c8099 100644 --- a/src/importer_publikacji/views.py +++ b/src/importer_publikacji/views.py @@ -10,6 +10,7 @@ from django.urls import reverse from django.views import View +from bpp.const import CHARAKTER_OGOLNY_ROZDZIAL from bpp.models import ( Autor, Crossref_Mapper, @@ -425,6 +426,25 @@ def post(self, request, session_id): "Podaj wydawcę lub wpisz szczegóły wydawcy.", ) return _render_source_step(request, session, form=form) + + # Rozdział wymaga wydawnictwa nadrzędnego + if _is_chapter(session): + wn = form.cleaned_data.get("wydawnictwo_nadrzedne") + wn_pbn = form.cleaned_data.get("wydawnictwo_nadrzedne_w_pbn") + if not wn and not wn_pbn: + form.add_error( + "wydawnictwo_nadrzedne", + "Dla rozdziału wymagane jest wydawnictwo nadrzędne.", + ) + return _render_source_step(request, session, form=form) + if wn and wn_pbn: + form.add_error( + "wydawnictwo_nadrzedne", + "Podaj tylko jedno: wydawnictwo" + " nadrzędne lub wydawnictwo" + " nadrzędne w PBN.", + ) + return _render_source_step(request, session, form=form) else: if not form.cleaned_data.get("zrodlo"): form.add_error( @@ -436,6 +456,10 @@ def post(self, request, session_id): session.zrodlo = form.cleaned_data["zrodlo"] session.wydawca = form.cleaned_data["wydawca"] session.matched_data["wydawca_opis"] = form.cleaned_data.get("wydawca_opis", "") + session.wydawnictwo_nadrzedne = form.cleaned_data.get("wydawnictwo_nadrzedne") + session.wydawnictwo_nadrzedne_w_pbn = form.cleaned_data.get( + "wydawnictwo_nadrzedne_w_pbn" + ) session.status = ImportSession.Status.SOURCE_MATCHED session.modified_by = request.user session.save() @@ -850,40 +874,68 @@ def _render_verify_full(request, session, form=None): return _render_full_page(request, STEP_VERIFY, ctx) +def _is_chapter(session): + """Czy sesja dotyczy rozdziału (charakter_ogolny == 'roz').""" + return ( + session.charakter_formalny_id + and session.charakter_formalny.charakter_ogolny == CHARAKTER_OGOLNY_ROZDZIAL + ) + + +def _source_initial_from_session(session): + """Odczytaj initial z zapisanych wartości sesji.""" + initial = {} + if session.zrodlo_id: + initial["zrodlo"] = session.zrodlo_id + if session.wydawca_id: + initial["wydawca"] = session.wydawca_id + wydawca_opis = session.matched_data.get("wydawca_opis", "") + if wydawca_opis: + initial["wydawca_opis"] = wydawca_opis + if session.wydawnictwo_nadrzedne_id: + initial["wydawnictwo_nadrzedne"] = session.wydawnictwo_nadrzedne_id + if session.wydawnictwo_nadrzedne_w_pbn_id: + initial["wydawnictwo_nadrzedne_w_pbn"] = session.wydawnictwo_nadrzedne_w_pbn_id + return initial + + +def _source_initial_auto_match(session): + """Auto-matching źródła i wydawcy z normalized_data.""" + initial = {} + nd = session.normalized_data + source_title = nd.get("source_title") + if source_title: + src = Komparator.porownaj_container_title(source_title) + if src.rekord_po_stronie_bpp: + initial["zrodlo"] = src.rekord_po_stronie_bpp.pk + + publisher = nd.get("publisher") + if publisher: + pub = Komparator.porownaj_publisher(publisher) + if pub.rekord_po_stronie_bpp: + initial["wydawca"] = pub.rekord_po_stronie_bpp.pk + else: + initial["wydawca_opis"] = publisher + return initial + + def _source_context(request, session, form=None): """Przygotuj kontekst dla kroku źródła.""" - if form is None: - initial = {} + is_chapter = _is_chapter(session) - # Użyj wartości sesji gdy istnieją (user już submitował) - if session.zrodlo_id: - initial["zrodlo"] = session.zrodlo_id - if session.wydawca_id: - initial["wydawca"] = session.wydawca_id - wydawca_opis = session.matched_data.get("wydawca_opis", "") - if wydawca_opis: - initial["wydawca_opis"] = wydawca_opis - - # Auto-matching tylko gdy brak zapisanych wartości + if form is None: + initial = _source_initial_from_session(session) if not initial: - nd = session.normalized_data - source_title = nd.get("source_title") - if source_title: - src = Komparator.porownaj_container_title(source_title) - if src.rekord_po_stronie_bpp: - initial["zrodlo"] = src.rekord_po_stronie_bpp.pk - - publisher = nd.get("publisher") - if publisher: - pub = Komparator.porownaj_publisher(publisher) - if pub.rekord_po_stronie_bpp: - initial["wydawca"] = pub.rekord_po_stronie_bpp.pk - else: - initial["wydawca_opis"] = publisher - + initial = _source_initial_auto_match(session) form = SourceForm(initial=initial) - return {"session": session, "form": form} + return { + "session": session, + "form": form, + "is_chapter": is_chapter, + "wydawnictwo_nadrzedne_obj": (session.wydawnictwo_nadrzedne), + "wydawnictwo_nadrzedne_w_pbn_obj": (session.wydawnictwo_nadrzedne_w_pbn), + } def _render_source_step(request, session, form=None): @@ -1318,6 +1370,14 @@ def _create_wydawnictwo_zwarte(session, common_fields, normalized_data): common_fields["isbn"] = normalized_data.get("isbn") or "" common_fields["e_isbn"] = normalized_data.get("e_isbn") or "" + # Wydawnictwo nadrzędne (dla rozdziałów) + if session.wydawnictwo_nadrzedne_id: + common_fields["wydawnictwo_nadrzedne"] = session.wydawnictwo_nadrzedne + if session.wydawnictwo_nadrzedne_w_pbn_id: + common_fields["wydawnictwo_nadrzedne_w_pbn"] = ( + session.wydawnictwo_nadrzedne_w_pbn + ) + issue = normalized_data.get("issue") if issue: existing = common_fields.get("szczegoly", "")