diff --git a/src/rard/research/migrations/0077_add_bibliographic_works.py b/src/rard/research/migrations/0077_add_bibliographic_works.py new file mode 100644 index 00000000..93e244f9 --- /dev/null +++ b/src/rard/research/migrations/0077_add_bibliographic_works.py @@ -0,0 +1,45 @@ +# Generated by Django 3.2.25 on 2026-06-25 11:52 + +from django.db import migrations, models + + +def add_bibliographic_works(apps, schema_editor): + """Run save on all Antiquarian objects to create + a new bibliographic work for each.""" + Antiquarian = apps.get_model("research", "Antiquarian") + Work = apps.get_model("research", "Work") + + # Create an explicit bibliographic Work for each Antiquarian. + for antiquarian in Antiquarian.objects.all(): + # If this antiquarian already has a bibliographic work, skip + if ( + Work.objects.filter(bibliographic=True, antiquarian__pk=antiquarian.pk) + .exists() + ): + continue + + w = Work.objects.create(name="Bibliographic Work", bibliographic=True) + # attach via the m2m on the Antiquarian instance + antiquarian.works.add(w) + w.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('research', '0076_add_folded_text'), + ] + + operations = [ + migrations.AddField( + model_name='historicalwork', + name='bibliographic', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='work', + name='bibliographic', + field=models.BooleanField(default=False), + ), + migrations.RunPython(add_bibliographic_works, reverse_code=migrations.RunPython.noop), + ] diff --git a/src/rard/research/migrations/0078_alter_worklink_options.py b/src/rard/research/migrations/0078_alter_worklink_options.py new file mode 100644 index 00000000..ea14f0d5 --- /dev/null +++ b/src/rard/research/migrations/0078_alter_worklink_options.py @@ -0,0 +1,17 @@ +# Generated by Django 3.2.25 on 2026-06-25 21:23 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('research', '0077_add_bibliographic_works'), + ] + + operations = [ + migrations.AlterModelOptions( + name='worklink', + options={'ordering': ['work__unknown', 'work__bibliographic', 'order']}, + ), + ] diff --git a/src/rard/research/models/antiquarian.py b/src/rard/research/models/antiquarian.py index bb4ce5bc..f846c310 100644 --- a/src/rard/research/models/antiquarian.py +++ b/src/rard/research/models/antiquarian.py @@ -8,7 +8,7 @@ from rard.research.models.mixins import HistoryModelMixin, TextObjectFieldMixin from rard.utils.basemodel import BaseModel, DatedModel, LockableModel, OrderableModel from rard.utils.decorators import disable_for_loaddata -from rard.utils.shared_functions import collate_uw_links +from rard.utils.shared_functions import collate_work_links from rard.utils.text_processors import make_plain_text @@ -16,7 +16,7 @@ class WorkLink(OrderableModel, models.Model): """Through-model for Work to Antiquarian, m2m""" class Meta: - ordering = ["work__unknown", "order"] + ordering = ["work__unknown", "work__bibliographic", "order"] def related_queryset(self): return self.__class__.objects.filter( @@ -156,13 +156,17 @@ def ordered_works(self): from rard.research.models import Work return Work.objects.filter(worklink__antiquarian=self).order_by( - "unknown", "worklink__order" + "unknown", "bibliographic", "worklink__order" ) @property def unknown_work(self): return self.works.filter(unknown=True).first() + @property + def bibliographic_work(self): + return self.works.filter(bibliographic=True).first() + def __str__(self): return self.name @@ -187,7 +191,9 @@ def reindex_work_links(self): # single db update with transaction.atomic(): links = WorkLink.objects.filter(antiquarian=self).order_by( - "work__unknown", models.F(("order")).asc(nulls_first=False) + "work__unknown", + "work__bibliographic", + models.F(("order")).asc(nulls_last=True), ) for count, link in enumerate(links): if link.order != count: @@ -335,6 +341,7 @@ def reindex_fragment_and_testimonium_links(self): .filter(work__isnull=False) .order_by( "-work__unknown", + "-work__bibliographic", "work__worklink__order", "work_order", ) @@ -396,12 +403,29 @@ def collate_unknown(instance): designated_unknown = unknown_works.first() other_unknown_works = unknown_works.exclude(pk=designated_unknown.pk) - collate_uw_links(instance, designated_unknown) + collate_work_links(instance, designated_unknown, other_unknown_works) other_unknown_works.delete() @disable_for_loaddata -def create_unknown_work(sender, instance, **kwargs): +def collate_bibliographic(instance): + """This makes sure there's only one bibliographic work per antiquarian and combines contents if otherwise""" + bibliographic_works = instance.works.filter(bibliographic=True).order_by("pk") + + if bibliographic_works.count() > 1: + designated_bibliographic = bibliographic_works.first() + other_bibliographic_works = bibliographic_works.exclude( + pk=designated_bibliographic.pk + ) + + collate_work_links( + instance, designated_bibliographic, other_bibliographic_works + ) + other_bibliographic_works.delete() + + +@disable_for_loaddata +def create_default_works(sender, instance, **kwargs): from rard.research.models import Work if not instance.unknown_work: @@ -417,6 +441,19 @@ def create_unknown_work(sender, instance, **kwargs): instance.unknown_work.antiquarian_set.add(instance) instance.unknown_work.save() + if not instance.bibliographic_work: + # create bibliographic work if doesn't exist + bibliographic_work = Work.objects.create( + name="Bibliographic Work", + bibliographic=True, + ) + bibliographic_work.antiquarian_set.add(instance) + bibliographic_work.save() + else: + # update existing + instance.bibliographic_work.antiquarian_set.add(instance) + instance.bibliographic_work.save() + collate_unknown(instance) @@ -445,7 +482,7 @@ def remove_stale_antiquarian_links(sender, instance, **kwargs): pre_delete.connect(remove_stale_antiquarian_links, sender=Antiquarian) -post_save.connect(create_unknown_work, sender=Antiquarian) +post_save.connect(create_default_works, sender=Antiquarian) Antiquarian.init_text_object_fields() diff --git a/src/rard/research/models/work.py b/src/rard/research/models/work.py index 63b5f48a..cbc31226 100644 --- a/src/rard/research/models/work.py +++ b/src/rard/research/models/work.py @@ -16,7 +16,7 @@ from rard.research.models.mixins import HistoryModelMixin, TextObjectFieldMixin from rard.utils.basemodel import BaseModel, DatedModel, LockableModel, OrderableModel from rard.utils.decorators import disable_for_loaddata -from rard.utils.shared_functions import collate_ub_links +from rard.utils.shared_functions import collate_book_links from rard.utils.text_processors import make_plain_text @@ -53,6 +53,8 @@ class Meta: unknown = models.BooleanField(default=False) + bibliographic = models.BooleanField(default=False) + introduction = models.OneToOneField( "TextObjectField", on_delete=models.SET_NULL, @@ -303,7 +305,7 @@ def collate_unknown(instance): designated_unknown = unknown_books.first() other_unknown_books = unknown_books.exclude(pk=designated_unknown.pk) - collate_ub_links(instance, designated_unknown) + collate_book_links(instance, designated_unknown, other_unknown_books) other_unknown_books.delete() diff --git a/src/rard/research/tests/forms/test_fragment.py b/src/rard/research/tests/forms/test_fragment.py index 1f5dae18..89a57cd1 100644 --- a/src/rard/research/tests/forms/test_fragment.py +++ b/src/rard/research/tests/forms/test_fragment.py @@ -103,8 +103,8 @@ def test_required_fields_no_work_selected(self): self.assertIsNone(form.fields["work"].initial) self.assertFalse(form.fields["work"].disabled) self.assertEqual( - form.fields["work"].queryset.count(), 2 - ) # includes unknown work + form.fields["work"].queryset.count(), 3 + ) # includes unknown work and bibliographic work self.assertTrue(form.fields["book"].disabled) self.assertEqual(form.fields["book"].queryset.count(), 0) diff --git a/src/rard/research/tests/models/test_antiquarian.py b/src/rard/research/tests/models/test_antiquarian.py index f8338830..fdaa0d9c 100644 --- a/src/rard/research/tests/models/test_antiquarian.py +++ b/src/rard/research/tests/models/test_antiquarian.py @@ -47,7 +47,7 @@ def test_display(self): def test_no_initial_works(self): data = {"name": "John Smith", "re_code": "smitre001"} a = Antiquarian.objects.create(**data) - self.assertEqual(a.works.filter(unknown=False).count(), 0) + self.assertEqual(a.works.filter(unknown=False, bibliographic=False).count(), 0) def test_can_have_multiple_works(self): data = {"name": "John Smith", "re_code": "smitre001"} @@ -55,7 +55,9 @@ def test_can_have_multiple_works(self): length = 10 for _ in range(0, length): a.works.create(name="name") - self.assertEqual(a.works.filter(unknown=False).count(), length) + self.assertEqual( + a.works.filter(unknown=False, bibliographic=False).count(), length + ) def test_introduction_created_with_antiquarian(self): data = {"name": "John Smith", "re_code": "smitre001"} @@ -245,6 +247,32 @@ def test_collate_unknown(self): self.assertEqual(a.fragmentlinks.all().count(), 1) self.assertEqual(a.fragmentlinks.first().work, a.unknown_work) + def test_collate_unknown_preserves_other_work_links(self): + data = {"name": "John Smith", "re_code": "smitre002"} + a = Antiquarian.objects.create(**data) + f = Fragment.objects.create(name="fragment") + keep_work = Work.objects.create(name="A Known Work") + a.works.add(keep_work) + FragmentLink.objects.create(antiquarian=a, fragment=f, work=keep_work) + + additional_unknown = Work.objects.create(unknown=True, name="Unknown Work") + additional_unknown.antiquarian_set.add(a) + FragmentLink.objects.create( + antiquarian=a, + fragment=f, + work=additional_unknown, + ) + + self.assertEqual(a.works.filter(unknown=True).count(), 2) + self.assertEqual(a.fragmentlinks.filter(work=keep_work).count(), 1) + self.assertEqual(a.fragmentlinks.filter(work=additional_unknown).count(), 1) + + collate_unknown(a) + + self.assertEqual(a.works.filter(unknown=True).count(), 1) + self.assertEqual(a.fragmentlinks.filter(work=keep_work).count(), 1) + self.assertEqual(a.fragmentlinks.filter(work=a.unknown_work).count(), 1) + class TestWorkLink(TestCase): def test_related_queryset(self): diff --git a/src/rard/research/tests/models/test_link_scheme.py b/src/rard/research/tests/models/test_link_scheme.py index 30f73e53..15170dde 100644 --- a/src/rard/research/tests/models/test_link_scheme.py +++ b/src/rard/research/tests/models/test_link_scheme.py @@ -418,7 +418,8 @@ def test_ordered_works(self): a.works.add(Work.objects.create(name=name)) self.assertEqual( - [w.name for w in a.ordered_works.all()], names + ["Unknown Work"] + [w.name for w in a.ordered_works.all()], + names + ["Bibliographic Work", "Unknown Work"], ) # try moving a work down in the order link = a.worklink_set.first() @@ -431,11 +432,16 @@ def test_ordered_works(self): "work two", "work one", "work three", + "Bibliographic Work", "Unknown Work", ], ) - link = a.worklink_set.exclude(work__unknown=True).last() + link = ( + a.worklink_set.exclude(work__unknown=True) + .exclude(work__bibliographic=True) + .last() + ) link.up() # should have reordered @@ -445,6 +451,7 @@ def test_ordered_works(self): "work two", "work three", "work one", + "Bibliographic Work", "Unknown Work", ], ) @@ -596,8 +603,9 @@ def _run_test_add_del_multi_works_updates_links(self, method): works = Work.objects.all() # set up creates a work called 'work', we've created - # four others here called 'another' and there is a default Unknown Work - self.assertEqual(ADD + 2, works.count()) + # four others here called 'another' and there are default + # Unknown and Bibliographic Works + self.assertEqual(ADD + 3, works.count()) # set the antiquarian works all at once self.antiquarian.works.set(works) @@ -605,10 +613,10 @@ def _run_test_add_del_multi_works_updates_links(self, method): ant_works = self.antiquarian.works.all() ant_fragmentlinks = self.antiquarian.fragmentlinks.all() - # check it worked - we should have 5 works in total + # check it worked - we should have 7 works in total self.assertEqual(works.count(), ant_works.count()) - # we should at this point have 5 sets + # we should at this point have 5 sets of 10 fragment links # 4 linked via the work 'another' and one other work = 5 expected = (self.NUM * ADD) + starting_fragmentlinks_count self.assertEqual(ant_fragmentlinks.count(), expected) @@ -623,7 +631,7 @@ def _run_test_add_del_multi_works_updates_links(self, method): self.antiquarian.works.set(Work.objects.none()) # set to empty # antiquarian should now have fewer links directly to it - expected = (ant_works.count()) * self.NUM + expected = (max(ant_works.count() - 1, 0)) * self.NUM self.assertEqual(ant_fragmentlinks.count(), expected) # the antiquarian links have been reordered for count, link in enumerate( @@ -636,9 +644,10 @@ def _run_test_add_del_multi_works_updates_links(self, method): nworks = Work.objects.count() # there should be no stray links lying around - self.assertEqual(FragmentLink.objects.all().count(), nfragments * nworks) + expected = nfragments * (nworks - 1) # ignore bibliographic work + self.assertEqual(FragmentLink.objects.all().count(), expected) - for work in Work.objects.all(): + for work in Work.objects.exclude(bibliographic=True).exclude(unknown=True): self.assertEqual(FragmentLink.objects.filter(work=work).count(), nfragments) # check that orphaned works have been reordered diff --git a/src/rard/research/tests/models/test_work.py b/src/rard/research/tests/models/test_work.py index 70f273e1..e95caf53 100644 --- a/src/rard/research/tests/models/test_work.py +++ b/src/rard/research/tests/models/test_work.py @@ -39,25 +39,29 @@ def test_ordering(self): # 1. check anonymous ordering self.assertEqual( - [w for w in Work.objects.exclude(unknown=True)], [worka, workb, workc] + [w for w in Work.objects.exclude(unknown=True).exclude(bibliographic=True)], + [worka, workb, workc], ) antc.works.add(workb) # this should now be last with anon works at the start self.assertEqual( - [w for w in Work.objects.exclude(unknown=True)], [worka, workc, workb] + [w for w in Work.objects.exclude(unknown=True).exclude(bibliographic=True)], + [worka, workc, workb], ) # the name of the antiquarian should put work c second antb.works.add(workc) self.assertEqual( - [w for w in Work.objects.exclude(unknown=True)], [worka, workc, workb] + [w for w in Work.objects.exclude(unknown=True).exclude(bibliographic=True)], + [worka, workc, workb], ) # even if we also add antc as an author of workc, as the name of # antiquarian antb should govern the order antc.works.add(workb) self.assertEqual( - [w for w in Work.objects.exclude(unknown=True)], [worka, workc, workb] + [w for w in Work.objects.exclude(unknown=True).exclude(bibliographic=True)], + [worka, workc, workb], ) # now, put worka as a work of anta and this should be first in the @@ -65,7 +69,8 @@ def test_ordering(self): # ahead of the others anta.works.add(worka) self.assertEqual( - [w for w in Work.objects.exclude(unknown=True)], [worka, workc, workb] + [w for w in Work.objects.exclude(unknown=True).exclude(bibliographic=True)], + [worka, workc, workb], ) # final test - antiquarian name takes precedence @@ -73,7 +78,8 @@ def test_ordering(self): antb.works.set([workb]) antc.works.set([worka]) self.assertEqual( - [w for w in Work.objects.exclude(unknown=True)], [workc, workb, worka] + [w for w in Work.objects.exclude(unknown=True).exclude(bibliographic=True)], + [workc, workb, worka], ) def test_required_fields(self): @@ -394,6 +400,36 @@ def test_collate_unknown(self): w.antiquarian_work_fragmentlinks.first().book, original_unknown ) + def test_collate_unknown_preserves_other_work_links(self): + data = {"name": "workname", "subtitle": "Subtitle"} + w = Work.objects.create(**data) + f = Fragment.objects.create(name="fragment") + keep_book = Book.objects.create(unknown=False, subtitle="Known Book", work=w) + FragmentLink.objects.create(fragment=f, work=w, book=keep_book) + + additional_unknown = Book.objects.create( + unknown=True, subtitle="Unknown Book", work=w + ) + FragmentLink.objects.create(fragment=f, work=w, book=additional_unknown) + + self.assertEqual(w.book_set.filter(unknown=True).count(), 2) + self.assertEqual( + w.antiquarian_work_fragmentlinks.filter(book=keep_book).count(), 1 + ) + self.assertEqual( + w.antiquarian_work_fragmentlinks.filter(book=additional_unknown).count(), 1 + ) + + collate_unknown(w) + + self.assertEqual(w.book_set.filter(unknown=True).count(), 1) + self.assertEqual( + w.antiquarian_work_fragmentlinks.filter(book=keep_book).count(), 1 + ) + self.assertEqual( + w.antiquarian_work_fragmentlinks.filter(book=w.unknown_book).count(), 1 + ) + class TestBook(TestCase): def setUp(self): diff --git a/src/rard/research/tests/views/test_antiquarian.py b/src/rard/research/tests/views/test_antiquarian.py index cb2fe7cb..0feaff3b 100644 --- a/src/rard/research/tests/views/test_antiquarian.py +++ b/src/rard/research/tests/views/test_antiquarian.py @@ -78,9 +78,12 @@ def test_create(self): antiquarian.lock(request.user) AntiquarianWorkCreateView.as_view()(request, pk=antiquarian.pk) - self.assertEqual(antiquarian.works.exclude(unknown=True).count(), 1) + self.assertEqual( + antiquarian.works.exclude(unknown=True).exclude(bibliographic=True).count(), + 1, + ) for key, val in data.items(): - self.assertEqual(getattr(antiquarian.works.first(), key), val) + self.assertEqual(getattr(antiquarian.ordered_works.first(), key), val) def test_bad_data(self): antiquarian = Antiquarian.objects.create() @@ -93,7 +96,10 @@ def test_bad_data(self): AntiquarianWorkCreateView.as_view()(request, pk=antiquarian.pk) # no work created - self.assertEqual(antiquarian.works.exclude(unknown=True).count(), 0) + self.assertEqual( + antiquarian.works.exclude(unknown=True).exclude(bibliographic=True).count(), + 0, + ) class TestAntiquarianDeleteView(TestCase): diff --git a/src/rard/research/tests/views/test_mentions.py b/src/rard/research/tests/views/test_mentions.py index 2b7e5177..4dc27c4e 100644 --- a/src/rard/research/tests/views/test_mentions.py +++ b/src/rard/research/tests/views/test_mentions.py @@ -215,8 +215,8 @@ def test_work_search(self): ) as mock_search_method: view.get_queryset() mock_search_method.assert_called_with([]) - # no search term - self.assertEqual(len(list(view.get_queryset())), 6) + # no search term: Unknown, Bibliographic and 1 extra for each antiquarian + self.assertEqual(len(list(view.get_queryset())), 9) # number as search: only applicable if number in antiquarian/name view.request = self.request( @@ -239,10 +239,17 @@ def test_work_search(self): "q": "wk:and", } ) - self.assertEqual(len(list(view.get_queryset())), 3) + # Should return andrew's work, andropov, + # the unknown work for Andrew, and the bibliographic work for Andrew + self.assertEqual(len(list(view.get_queryset())), 4) self.assertCountEqual( list(view.get_queryset()), - [self.andropov, self.provisions, self.andrew.unknown_work], + [ + self.andropov, + self.provisions, + self.andrew.unknown_work, + self.andrew.bibliographic_work, + ], ) def test_anonymous_fragment_search(self): diff --git a/src/rard/research/tests/views/test_search.py b/src/rard/research/tests/views/test_search.py index e6464ebd..290939b4 100644 --- a/src/rard/research/tests/views/test_search.py +++ b/src/rard/research/tests/views/test_search.py @@ -94,8 +94,8 @@ def test_search_queryset(self): url = reverse("search:home") request = RequestFactory().get(url, data=data) view.request = request - # should include the unknown works for them - self.assertEqual(5, len(view.get_queryset())) + # should include the unknown and bibliographic works for them + self.assertEqual(7, len(view.get_queryset())) def test_empty_search_redirects(self): # any empty sring should be ignored @@ -273,16 +273,37 @@ def do_search(search_function, keywords): w1 = Work.objects.create(name="work") w2 = Work.objects.create(name="nothing") self.assertEqual( - do_search(view.work_search, "work"), [w1, a1.unknown_work, a2.unknown_work] + do_search(view.work_search, "work"), + [ + w1, + a1.bibliographic_work, + a1.unknown_work, + a2.bibliographic_work, + a2.unknown_work, + ], ) self.assertEqual( - do_search(view.work_search, "WORK"), [w1, a1.unknown_work, a2.unknown_work] + do_search(view.work_search, "WORK"), + [ + w1, + a1.bibliographic_work, + a1.unknown_work, + a2.bibliographic_work, + a2.unknown_work, + ], ) self.assertEqual(do_search(view.work_search, "nothing"), [w2]) self.assertEqual(do_search(view.work_search, "NothInG"), [w2]) self.assertEqual( do_search(view.work_search, "*O*"), - [w2, w1, a1.unknown_work, a2.unknown_work], + [ + w2, + w1, + a1.bibliographic_work, + a1.unknown_work, + a2.bibliographic_work, + a2.unknown_work, + ], ) cw = CitingWork.objects.create(title="citing_work") diff --git a/src/rard/research/tests/views/test_work.py b/src/rard/research/tests/views/test_work.py index 45be7c9c..a3a6d67f 100644 --- a/src/rard/research/tests/views/test_work.py +++ b/src/rard/research/tests/views/test_work.py @@ -72,7 +72,9 @@ def test_create(self): WorkCreateView.as_view()(request) a = Antiquarian.objects.get(pk=a.pk) - self.assertEqual(a.works.exclude(unknown=True).count(), 1) + self.assertEqual( + a.works.exclude(unknown=True).exclude(bibliographic=True).count(), 1 + ) self.assertIn(a.unknown_work, a.works.all()) def test_create_with_books(self): @@ -93,7 +95,9 @@ def test_create_with_books(self): WorkCreateView.as_view()(request) a = Antiquarian.objects.get(pk=a.pk) - self.assertEqual(a.works.exclude(unknown=True).count(), 1) + self.assertEqual( + a.works.exclude(unknown=True).exclude(bibliographic=True).count(), 1 + ) w = a.works.first() self.assertEqual(Book.objects.filter(work=w, unknown=False).count(), 2) self.assertEqual(w.book_set.exclude(unknown=True).count(), 2) @@ -127,8 +131,12 @@ def test_update(self): work.lock(request.user) WorkUpdateView.as_view()(request, pk=work.pk) - self.assertEqual(a1.works.exclude(unknown=True).count(), 1) - self.assertEqual(a2.works.exclude(unknown=True).count(), 0) + self.assertEqual( + a1.works.exclude(unknown=True).exclude(bibliographic=True).count(), 1 + ) + self.assertEqual( + a2.works.exclude(unknown=True).exclude(bibliographic=True).count(), 0 + ) self.assertEqual(Work.objects.get(pk=work.pk).name, "first") work = Work.objects.create(name="name") @@ -145,8 +153,12 @@ def test_update(self): # view.form.instance = work view(request, pk=work.pk) - self.assertEqual(a1.works.exclude(unknown=True).count(), 1) - self.assertEqual(a2.works.exclude(unknown=True).count(), 1) + self.assertEqual( + a1.works.exclude(unknown=True).exclude(bibliographic=True).count(), 1 + ) + self.assertEqual( + a2.works.exclude(unknown=True).exclude(bibliographic=True).count(), 1 + ) self.assertEqual(Work.objects.get(pk=work.pk).name, "other") def test_update_with_books(self): diff --git a/src/rard/templates/research/partials/ordered_work_area.html b/src/rard/templates/research/partials/ordered_work_area.html index aecfd821..92f477c1 100644 --- a/src/rard/templates/research/partials/ordered_work_area.html +++ b/src/rard/templates/research/partials/ordered_work_area.html @@ -19,93 +19,92 @@ +
{% for work in object.ordered_works.all %} - -