From 48b55b0bc8ab08de53f324fa1b0791d3982ad2de Mon Sep 17 00:00:00 2001 From: Tim Band Date: Thu, 14 May 2026 12:27:52 +0100 Subject: [PATCH 1/4] Serious work towards #494 speed up Varro --- src/rard/research/templatetags/get_item.py | 5 +++ .../research/templatetags/links_for_work.py | 37 ++++++++++--------- .../research/antiquarian_detail.html | 2 +- .../research/partials/ordered_work_area.html | 14 +++++-- src/rard/templates/research/work_detail.html | 2 +- 5 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/rard/research/templatetags/get_item.py b/src/rard/research/templatetags/get_item.py index ed0ca85f0..aad470486 100644 --- a/src/rard/research/templatetags/get_item.py +++ b/src/rard/research/templatetags/get_item.py @@ -6,3 +6,8 @@ @register.filter def get_item(item, selector): return item.get(selector, "") + + +@register.filter +def get_item_list(item, selector): + return item.get(selector, []) diff --git a/src/rard/research/templatetags/links_for_work.py b/src/rard/research/templatetags/links_for_work.py index e6489ca2d..10a7ba89d 100644 --- a/src/rard/research/templatetags/links_for_work.py +++ b/src/rard/research/templatetags/links_for_work.py @@ -4,33 +4,36 @@ @register.filter -def fragment_links_for_work(antiquarian, work): +def testimonium_links_for_work(antiquarian, work): args = {} if work: args["work"] = work else: args["work__isnull"] = True - return antiquarian.fragmentlinks.filter(**args).order_by("work_order") + return antiquarian.testimoniumlinks.filter(**args).order_by("work_order") @register.filter -def testimonium_links_for_work(antiquarian, work): - args = {} - if work: - args["work"] = work - else: - args["work__isnull"] = True - - return antiquarian.testimoniumlinks.filter(**args).order_by("work_order") +def fragment_links_by_work(antiquarian): + return get_links_by_work(antiquarian.fragmentlinks, "fragment") @register.filter -def appositum_links_for_work(antiquarian, work): - args = {} - if work: - args["work"] = work - else: - args["work__isnull"] = True +def testimonium_links_by_work(antiquarian): + return get_links_by_work(antiquarian.testimoniumlinks, "testimonium") - return antiquarian.appositumfragmentlinks.filter(**args).order_by("work_order") + +@register.filter +def appositum_links_by_work(antiquarian): + return get_links_by_work(antiquarian.appositumfragmentlinks, "anonymous_fragment") + + +def get_links_by_work(links, related): + work_to_object = {} + for obj in links.order_by("work_order").select_related(related): + w = obj.work_id + if w not in work_to_object: + work_to_object[w] = [] + work_to_object[w].append(obj) + return work_to_object diff --git a/src/rard/templates/research/antiquarian_detail.html b/src/rard/templates/research/antiquarian_detail.html index 0e06c12d7..4d53e91ef 100644 --- a/src/rard/templates/research/antiquarian_detail.html +++ b/src/rard/templates/research/antiquarian_detail.html @@ -1,5 +1,5 @@ {% extends "research/detail_base.html" %} -{% load i18n bootstrap4 object_lock links_for_work static %} +{% load i18n bootstrap4 object_lock static %} {% block title %}{{object.name}}{% endblock %} {% block heading %}{{ object.name }}{% endblock %} diff --git a/src/rard/templates/research/partials/ordered_work_area.html b/src/rard/templates/research/partials/ordered_work_area.html index aecfd8216..6cd6205a8 100644 --- a/src/rard/templates/research/partials/ordered_work_area.html +++ b/src/rard/templates/research/partials/ordered_work_area.html @@ -1,4 +1,4 @@ -{% load i18n links_for_work %} +{% load i18n links_for_work get_item %} {% with object=antiquarian %}
@@ -19,6 +19,9 @@ + {% with w2tlinks=object|testimonium_links_by_work %} + {% with w2flinks=object|fragment_links_by_work %} + {% with w2alinks=object|appositum_links_by_work %} {% for work in object.ordered_works.all %}
@@ -26,9 +29,9 @@
- {% with tlinks=object|testimonium_links_for_work:work %} - {% with flinks=object|fragment_links_for_work:work %} - {% with alinks=object|appositum_links_for_work:work %} + {% with tlinks=w2tlinks|get_item_list:work %} + {% with flinks=w2flinks|get_item_list:work %} + {% with alinks=w2alinks|get_item_list:work %}
{% if work.unknown == True and not flinks and not tlinks and not alinks %} @@ -108,6 +111,9 @@ {% endif %} {% endfor %} + {% endwith %} + {% endwith %} + {% endwith %}
diff --git a/src/rard/templates/research/work_detail.html b/src/rard/templates/research/work_detail.html index bcc4a9966..08e8fe8c2 100644 --- a/src/rard/templates/research/work_detail.html +++ b/src/rard/templates/research/work_detail.html @@ -1,5 +1,5 @@ {% extends "research/detail_base.html" %} -{% load i18n object_lock links_for_work bootstrap4 static humanize %} +{% load i18n object_lock bootstrap4 static humanize %} {% block title %}Work: {{object.name}} ({{object.antiquarian_set.first}}) {% if editing == 'introduction' %}: Editing Introduction {% endif %} {% endblock %} From 20d9dfe1c97cf991296a5a923a119250cb443548 Mon Sep 17 00:00:00 2001 From: Tim Band Date: Fri, 15 May 2026 18:38:00 +0100 Subject: [PATCH 2/4] Upgraded to Django4 --- src/config/settings/base.py | 2 - src/rard/research/forms.py | 8 +- .../migrations/0077_migrate_to_django4.py | 377 ++++++++++++++++++ src/rard/research/models/antiquarian.py | 2 +- src/rard/research/models/topic.py | 2 +- src/rard/research/models/work.py | 4 +- .../research/tests/models/test_antiquarian.py | 4 +- .../research/tests/models/test_fragment.py | 8 +- .../research/tests/models/test_link_scheme.py | 3 +- .../tests/models/test_text_object_field.py | 8 +- .../research/tests/views/test_fragment.py | 6 +- .../tests/views/test_fragment_links.py | 2 +- src/rard/research/views/fragment.py | 102 +++-- src/rard/research/views/mention.py | 2 +- src/rard/research/views/testimonium.py | 104 +++-- src/rard/utils/shared_functions.py | 9 +- src/requirements/base.txt | 10 +- src/requirements/local.txt | 8 +- 18 files changed, 512 insertions(+), 149 deletions(-) create mode 100644 src/rard/research/migrations/0077_migrate_to_django4.py diff --git a/src/config/settings/base.py b/src/config/settings/base.py index bf2fc4107..dd3fc1977 100644 --- a/src/config/settings/base.py +++ b/src/config/settings/base.py @@ -35,8 +35,6 @@ SITE_ID = 1 # https://docs.djangoproject.com/en/dev/ref/settings/#use-i18n USE_I18N = True -# https://docs.djangoproject.com/en/dev/ref/settings/#use-l10n -USE_L10N = True # https://docs.djangoproject.com/en/dev/ref/settings/#use-tz USE_TZ = True # https://docs.djangoproject.com/en/dev/ref/settings/#locale-paths diff --git a/src/rard/research/forms.py b/src/rard/research/forms.py index 5c0c4b12d..ecfc78477 100644 --- a/src/rard/research/forms.py +++ b/src/rard/research/forms.py @@ -346,9 +346,11 @@ def __init__(self, *args, **kwargs): def clean(self): cleaned_data = super().clean() if "books" in cleaned_data: - existing_book_numbers = [ - str(b.number) for b in self.instance.book_set.all() - ] + existing_book_numbers = ( + [str(b.number) for b in self.instance.book_set.all()] + if self.instance.pk + else [] + ) # cannot use book_set if instance isn't saved yet new_book_numbers = [ b["num"] for b in cleaned_data.get("books") if "num" in b ] diff --git a/src/rard/research/migrations/0077_migrate_to_django4.py b/src/rard/research/migrations/0077_migrate_to_django4.py new file mode 100644 index 000000000..e3a5c88cb --- /dev/null +++ b/src/rard/research/migrations/0077_migrate_to_django4.py @@ -0,0 +1,377 @@ +# Generated by Django 4.2.30 on 2026-05-14 11:00 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("research", "0076_add_folded_text"), + ] + + operations = [ + migrations.AlterField( + model_name="anonymousfragment", + name="commentary", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="commentary_for_%(class)s", + to="research.textobjectfield", + ), + ), + migrations.AlterField( + model_name="anonymousfragment", + name="duplicate_afs", + field=models.ManyToManyField( + blank=True, + related_name="%(class)s_duplicate_anonfragments", + to="research.anonymousfragment", + ), + ), + migrations.AlterField( + model_name="anonymousfragment", + name="duplicate_frags", + field=models.ManyToManyField( + blank=True, + related_name="%(class)s_duplicate_fragments", + to="research.fragment", + ), + ), + migrations.AlterField( + model_name="anonymousfragment", + name="duplicate_ts", + field=models.ManyToManyField( + blank=True, + related_name="%(class)s_duplicate_ts", + to="research.testimonium", + ), + ), + migrations.AlterField( + model_name="anonymousfragment", + name="public_commentary_mentions", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="public_commentary_mentions_for_%(class)s", + to="research.publiccommentarymentions", + ), + ), + migrations.AlterField( + model_name="antiquarian", + name="fragments", + field=models.ManyToManyField( + blank=True, + related_name="linked_%(class)ss", + through="research.FragmentLink", + to="research.fragment", + ), + ), + migrations.AlterField( + model_name="antiquarian", + name="introduction", + field=models.OneToOneField( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="introduction_for_%(class)s", + to="research.textobjectfield", + ), + ), + migrations.AlterField( + model_name="antiquarian", + name="testimonia", + field=models.ManyToManyField( + blank=True, + related_name="linked_%(class)ss", + through="research.TestimoniumLink", + to="research.testimonium", + ), + ), + migrations.AlterField( + model_name="appositumfragmentlink", + name="anonymous_fragment", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="%(class)ss_from", + to="research.anonymousfragment", + ), + ), + migrations.AlterField( + model_name="appositumfragmentlink", + name="antiquarian", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="%(class)ss", + to="research.antiquarian", + ), + ), + migrations.AlterField( + model_name="appositumfragmentlink", + name="book", + field=models.ForeignKey( + blank=True, + default=None, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="antiquarian_book_%(class)ss", + to="research.book", + ), + ), + migrations.AlterField( + model_name="appositumfragmentlink", + name="linked_to", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="%(class)ss_to", + to="research.fragment", + ), + ), + migrations.AlterField( + model_name="appositumfragmentlink", + name="work", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="antiquarian_work_%(class)ss", + to="research.work", + ), + ), + migrations.AlterField( + model_name="book", + name="introduction", + field=models.OneToOneField( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="introduction_for_%(class)s", + to="research.textobjectfield", + ), + ), + migrations.AlterField( + model_name="citingauthor", + name="introduction", + field=models.OneToOneField( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="introduction_for_%(class)s", + to="research.textobjectfield", + ), + ), + migrations.AlterField( + model_name="citingwork", + name="introduction", + field=models.OneToOneField( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="introduction_for_%(class)s", + to="research.textobjectfield", + ), + ), + migrations.AlterField( + model_name="fragment", + name="commentary", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="commentary_for_%(class)s", + to="research.textobjectfield", + ), + ), + migrations.AlterField( + model_name="fragment", + name="duplicate_afs", + field=models.ManyToManyField( + blank=True, + related_name="%(class)s_duplicate_anonfragments", + to="research.anonymousfragment", + ), + ), + migrations.AlterField( + model_name="fragment", + name="duplicate_frags", + field=models.ManyToManyField( + blank=True, + related_name="%(class)s_duplicate_fragments", + to="research.fragment", + ), + ), + migrations.AlterField( + model_name="fragment", + name="duplicate_ts", + field=models.ManyToManyField( + blank=True, + related_name="%(class)s_duplicate_ts", + to="research.testimonium", + ), + ), + migrations.AlterField( + model_name="fragment", + name="public_commentary_mentions", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="public_commentary_mentions_for_%(class)s", + to="research.publiccommentarymentions", + ), + ), + migrations.AlterField( + model_name="fragmentlink", + name="antiquarian", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="%(class)ss", + to="research.antiquarian", + ), + ), + migrations.AlterField( + model_name="fragmentlink", + name="book", + field=models.ForeignKey( + blank=True, + default=None, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="antiquarian_book_%(class)ss", + to="research.book", + ), + ), + migrations.AlterField( + model_name="fragmentlink", + name="fragment", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="antiquarian_%(class)ss", + to="research.fragment", + ), + ), + migrations.AlterField( + model_name="fragmentlink", + name="work", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="antiquarian_work_%(class)ss", + to="research.work", + ), + ), + migrations.AlterField( + model_name="testimonium", + name="commentary", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="commentary_for_%(class)s", + to="research.textobjectfield", + ), + ), + migrations.AlterField( + model_name="testimonium", + name="duplicate_afs", + field=models.ManyToManyField( + blank=True, + related_name="%(class)s_duplicate_anonfragments", + to="research.anonymousfragment", + ), + ), + migrations.AlterField( + model_name="testimonium", + name="duplicate_frags", + field=models.ManyToManyField( + blank=True, + related_name="%(class)s_duplicate_fragments", + to="research.fragment", + ), + ), + migrations.AlterField( + model_name="testimonium", + name="duplicate_ts", + field=models.ManyToManyField( + blank=True, + related_name="%(class)s_duplicate_ts", + to="research.testimonium", + ), + ), + migrations.AlterField( + model_name="testimonium", + name="public_commentary_mentions", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="public_commentary_mentions_for_%(class)s", + to="research.publiccommentarymentions", + ), + ), + migrations.AlterField( + model_name="testimoniumlink", + name="antiquarian", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="%(class)ss", + to="research.antiquarian", + ), + ), + migrations.AlterField( + model_name="testimoniumlink", + name="book", + field=models.ForeignKey( + blank=True, + default=None, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="antiquarian_book_%(class)ss", + to="research.book", + ), + ), + migrations.AlterField( + model_name="testimoniumlink", + name="testimonium", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="antiquarian_%(class)ss", + to="research.testimonium", + ), + ), + migrations.AlterField( + model_name="testimoniumlink", + name="work", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="antiquarian_work_%(class)ss", + to="research.work", + ), + ), + migrations.AlterField( + model_name="work", + name="introduction", + field=models.OneToOneField( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="introduction_for_%(class)s", + to="research.textobjectfield", + ), + ), + ] diff --git a/src/rard/research/models/antiquarian.py b/src/rard/research/models/antiquarian.py index bb4ce5bcd..e73f612b5 100644 --- a/src/rard/research/models/antiquarian.py +++ b/src/rard/research/models/antiquarian.py @@ -187,7 +187,7 @@ 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", models.F(("order")).asc(nulls_last=True) ) for count, link in enumerate(links): if link.order != count: diff --git a/src/rard/research/models/topic.py b/src/rard/research/models/topic.py index 2b1eede51..c72631b25 100644 --- a/src/rard/research/models/topic.py +++ b/src/rard/research/models/topic.py @@ -48,7 +48,7 @@ def reindex_fragment_links(self): # single db update with transaction.atomic(): links = TopicLink.objects.filter(topic=self).order_by( - models.F(("order")).asc(nulls_first=False) + models.F(("order")).asc(nulls_last=True) ) for count, link in enumerate(links): link.order = count diff --git a/src/rard/research/models/work.py b/src/rard/research/models/work.py index 63b5f48aa..08cac4003 100644 --- a/src/rard/research/models/work.py +++ b/src/rard/research/models/work.py @@ -28,7 +28,9 @@ def get_queryset(self): # Make sure anonymous works are at the top with nulls_first parameter return qs.annotate( - authors=StringAgg("worklink__antiquarian__order_name", delimiter=",") + authors=StringAgg( + "worklink__antiquarian__order_name", delimiter=",", default=None + ) ).order_by(models.F(("authors")).asc(nulls_first=True), "name", "order_year") diff --git a/src/rard/research/tests/models/test_antiquarian.py b/src/rard/research/tests/models/test_antiquarian.py index f8338830b..fece2bdb9 100644 --- a/src/rard/research/tests/models/test_antiquarian.py +++ b/src/rard/research/tests/models/test_antiquarian.py @@ -185,7 +185,7 @@ def mention_bibliography_item_in_text_object_field(bib: BibliographyItem, obj): ) aq1.bibliography_items.add(bib_init) aq1.save() - self.assertQuerysetEqual(aq1.bibliography_items.all(), [bib_init]) + self.assertQuerySetEqual(aq1.bibliography_items.all(), [bib_init]) fr1 = Fragment.objects.create(name="fr1") fr2 = Fragment.objects.create(name="fr2") @@ -221,7 +221,7 @@ def mention_bibliography_item_in_text_object_field(bib: BibliographyItem, obj): # bib init should have been removed, and all those mentioned by related # objects should be added. - self.assertQuerysetEqual(aq1.bibliography_items.all(), target_bibs) + self.assertQuerySetEqual(aq1.bibliography_items.all(), target_bibs) def test_collate_unknown(self): data = {"name": "John Smith", "re_code": "smitre001"} diff --git a/src/rard/research/tests/models/test_fragment.py b/src/rard/research/tests/models/test_fragment.py index d2a9dbbd5..56de003a0 100644 --- a/src/rard/research/tests/models/test_fragment.py +++ b/src/rard/research/tests/models/test_fragment.py @@ -231,11 +231,11 @@ def test_anonymous_apposita_asymmetry(self): it should not be a symmetric relationship""" self.anon1.anonymous_fragments.add(self.anon2) # First confirm anon1 has been added as an apposita of anon2 - self.assertQuerysetEqual(self.anon1.anonymous_fragments.all(), [self.anon2]) - self.assertQuerysetEqual(self.anon2.anonymous_apposita.all(), [self.anon1]) + self.assertQuerySetEqual(self.anon1.anonymous_fragments.all(), [self.anon2]) + self.assertQuerySetEqual(self.anon2.anonymous_apposita.all(), [self.anon1]) # Now confirm anon2 is not an apposita of anon1 - self.assertQuerysetEqual(self.anon2.anonymous_fragments.all(), []) - self.assertQuerysetEqual(self.anon1.anonymous_apposita.all(), []) + self.assertQuerySetEqual(self.anon2.anonymous_fragments.all(), []) + self.assertQuerySetEqual(self.anon1.anonymous_apposita.all(), []) class TestAnonymousTopicLink(TestCase): diff --git a/src/rard/research/tests/models/test_link_scheme.py b/src/rard/research/tests/models/test_link_scheme.py index 30f73e536..4b3ec122f 100644 --- a/src/rard/research/tests/models/test_link_scheme.py +++ b/src/rard/research/tests/models/test_link_scheme.py @@ -248,8 +248,9 @@ def test_deleting_antiquarian_removes_link(self): a1.fragments.add(fragment) # delete one antiquarian and all these links should go + a0_pk = a0.pk a0.delete() - self.assertEqual(0, FragmentLink.objects.filter(antiquarian=a0).count()) + self.assertEqual(0, FragmentLink.objects.filter(antiquarian_id=a0_pk).count()) # other antiquarian unaffected self.assertEqual( diff --git a/src/rard/research/tests/models/test_text_object_field.py b/src/rard/research/tests/models/test_text_object_field.py index 978af7527..d211df522 100644 --- a/src/rard/research/tests/models/test_text_object_field.py +++ b/src/rard/research/tests/models/test_text_object_field.py @@ -105,13 +105,13 @@ def test_update_mentions(self): antiquarian.introduction.content = mention_html antiquarian.introduction.save() # check the items are in the fragment_testimonia_mentions of TOF - self.assertQuerysetEqual( + self.assertQuerySetEqual( antiquarian.introduction.fragment_mentions.all(), [fragment] ) - self.assertQuerysetEqual( + self.assertQuerySetEqual( antiquarian.introduction.testimonium_mentions.all(), [testimonium] ) - self.assertQuerysetEqual( + self.assertQuerySetEqual( antiquarian.introduction.anonymousfragment_mentions.all(), [anon_frag] ) # check the reverse relationship via m2m was established @@ -126,7 +126,7 @@ def test_update_mentions(self): ) antiquarian.introduction.content = mention_html antiquarian.introduction.save() - self.assertQuerysetEqual( + self.assertQuerySetEqual( antiquarian.introduction.fragment_mentions.all(), [fragment] ) self.assertNotIn( diff --git a/src/rard/research/tests/views/test_fragment.py b/src/rard/research/tests/views/test_fragment.py index 670a2f313..5d0eac947 100644 --- a/src/rard/research/tests/views/test_fragment.py +++ b/src/rard/research/tests/views/test_fragment.py @@ -408,7 +408,7 @@ def test_converted_anonymous_fragment_maintains_apposita(self): self.anon_fragment_with_apposita ) # self.af2 was apposita to anon fragment, should now be apposita to new fragment - self.assertQuerysetEqual(new_unlinked_fragment.apposita.all(), [self.af2]) + self.assertQuerySetEqual(new_unlinked_fragment.apposita.all(), [self.af2]) def test_converted_unlinked_fragment_maintains_apposita(self): """If the original unlinked fragment had any apposita, those @@ -417,7 +417,7 @@ def test_converted_unlinked_fragment_maintains_apposita(self): self.unlinked_fragment_with_apposita ) # self.af1 was originally apposita to the unlinked fragment - self.assertQuerysetEqual(new_anon_fragment.anonymous_apposita.all(), [self.af1]) + self.assertQuerySetEqual(new_anon_fragment.anonymous_apposita.all(), [self.af1]) class TestMoveAnonymousTopicLinkView(TestCase): @@ -618,7 +618,7 @@ def test_compare_orders(self): list(response.context_data["object_list"].all()), ) - self.assertQuerysetEqual( + self.assertQuerySetEqual( response.context_data["object_list"], [self.aftl1, self.aftl3, self.aftl2, self.aftl3], ) diff --git a/src/rard/research/tests/views/test_fragment_links.py b/src/rard/research/tests/views/test_fragment_links.py index 836121721..2ad9f3845 100644 --- a/src/rard/research/tests/views/test_fragment_links.py +++ b/src/rard/research/tests/views/test_fragment_links.py @@ -192,7 +192,7 @@ def test_add_apposita_to_anonymous_fragment(self): self.assertEqual(self.af1.anonymous_fragments.count(), 0) view(request, pk=self.af1.pk) self.assertEqual(self.af1.anonymous_fragments.count(), 1) - self.assertQuerysetEqual(self.af1.anonymous_fragments.all(), [self.af2]) + self.assertQuerySetEqual(self.af1.anonymous_fragments.all(), [self.af2]) def test_unlink_apposita_and_anonymous_fragment(self): """Use RemoveAppositumFragmentLinkView to remove an existing appositum link between diff --git a/src/rard/research/views/fragment.py b/src/rard/research/views/fragment.py index 11ed0ce4a..03738ae3c 100644 --- a/src/rard/research/views/fragment.py +++ b/src/rard/research/views/fragment.py @@ -968,81 +968,71 @@ def get_context_data(self, *args, **kwargs): @method_decorator(require_POST, name="dispatch") class RemoveFragmentLinkView( - CheckLockMixin, LoginRequiredMixin, PermissionRequiredMixin, DeleteView + CheckLockMixin, LoginRequiredMixin, PermissionRequiredMixin, View ): - """When requesting link removal, one link will be removed/reassigned if from a work link - If from an antiquarian link, all links will be removed""" + """ + Just a POST endpoint for deleting a Fragment link. + + Two modes of operation: if the form has ``antiquarian_request`` then + this is the PK of the Fragment and the URL PK is for the Antiquarian; + in this case all the ``FragmentLinks`` between this Antiquarian and + this Fragment will be removed. + + If the form has no ``antiquarian_request`` then the URL PK is for the + ``FragmentLink`` and this will be removed. + """ check_lock_object = "fragment" model = FragmentLink + def get_object(self): + """Get the object that we need to check the lock for: this is the Fragment.""" + return self.fragment + def dispatch(self, request, *args, **kwargs): - # need to ensure we have the lock object view attribute - # initialised in dispatch - if "antiquarian_request" in request.POST: - antiquarian_pk = kwargs["pk"] - fragment_pk = request.POST.get("antiquarian_request") - self.get_antiquarian(antiquarian_pk) - self.get_fragment(fragment_pk) + pk = self.kwargs["pk"] + antiquarian_request = request.POST.get("antiquarian_request", None) + if antiquarian_request is None: + self.link = FragmentLink.objects.get(pk=pk) + self.fragment = self.link.fragment + self.antiquarian = self.link.antiquarian else: - self.get_fragment() + self.link = None + self.fragment = Fragment.objects.get(pk=antiquarian_request) + self.antiquarian = Antiquarian.objects.get(pk=pk) return super().dispatch(request, *args, **kwargs) # base class for both remove work and remove book from a fragment permission_required = ("research.change_fragment",) - def get_success_url(self, *args, **kwargs): + def get_success_url(self): return self.request.META.get( - "HTTP_REFERER", reverse("fragment:detail", kwargs={"pk": self.fragment.pk}) + "HTTP_REFERER", + reverse("fragment:detail", kwargs={"pk": self.fragment.pk}), ) - def get_antiquarian(self, *args, **kwargs): - if not getattr(self, "antiquarian", False): - pk = args[0] - self.antiquarian = Antiquarian.objects.get(pk=pk) - return self.antiquarian + def get_response(self): + return HttpResponseRedirect(self.get_success_url()) - def get_fragment(self, *args, **kwargs): - if not getattr(self, "fragment", False): - if "antiquarian_request" in self.request.POST: - pk = args[0] - self.fragment = Fragment.objects.get(pk=pk) - else: - self.fragment = self.get_object().fragment - return self.fragment + def post(self, request, *args, **kwargs): + return self.delete(request, *args, **kwargs) def delete(self, request, *args, **kwargs): - success_url = self.get_success_url() - fragment = self.get_fragment() - - if "antiquarian_request" in request.POST: - antiquarian = self.get_antiquarian() - antiquarian_fragmentlinks = FragmentLink.objects.filter( - antiquarian=antiquarian, fragment=fragment - ) - for link in antiquarian_fragmentlinks: - link.delete() - + if self.link is None: + self.model.objects.filter( + antiquarian=self.antiquarian, + fragment=self.fragment, + ).delete() + return self.get_response() + how_many_links = FragmentLink.objects.filter( + antiquarian=self.antiquarian, + fragment=self.fragment, + ).count() + if how_many_links == 1: + reassign_to_unknown(self.link) else: - self.object = self.get_object() - antiquarian = self.object.antiquarian - # Determine if it should reassign to unknown - # if no other links reassign to unknown - # otherwise delete the link - if ( - len( - FragmentLink.objects.filter( - antiquarian=antiquarian, fragment=fragment - ) - ) - == 1 - ): - reassign_to_unknown(self.object) - - else: - self.object.delete() - - return HttpResponseRedirect(success_url) + self.link.delete() + return self.get_response() class FragmentUpdateWorkLinkView( diff --git a/src/rard/research/views/mention.py b/src/rard/research/views/mention.py index 456e40a46..60e35892a 100644 --- a/src/rard/research/views/mention.py +++ b/src/rard/research/views/mention.py @@ -112,7 +112,7 @@ def bibliography_search(cls, keywords): def work_search(cls, keywords): qs = Work.objects.annotate( author_title=Concat( - StringAgg("antiquarian__name", delimiter=","), + StringAgg("antiquarian__name", delimiter=",", default=""), Value(" "), F("name"), output_field=CharField(), diff --git a/src/rard/research/views/testimonium.py b/src/rard/research/views/testimonium.py index c57704a69..c6864d907 100644 --- a/src/rard/research/views/testimonium.py +++ b/src/rard/research/views/testimonium.py @@ -6,7 +6,7 @@ from django.views.decorators.http import require_POST from django.views.generic import FormView, ListView from django.views.generic.detail import DetailView -from django.views.generic.edit import DeleteView, UpdateView +from django.views.generic.edit import DeleteView, UpdateView, View from rard.research.forms import ( TestimoniumAntiquariansForm, @@ -266,80 +266,70 @@ def post(self, request, *args, **kwargs): @method_decorator(require_POST, name="dispatch") class RemoveTestimoniumLinkView( - CheckLockMixin, LoginRequiredMixin, PermissionRequiredMixin, DeleteView + CheckLockMixin, LoginRequiredMixin, PermissionRequiredMixin, View ): - """When requesting link removal, one link will be removed/reassigned if from a work link - If from an antiquarian link, all links will be removed""" + """ + Just a POST endpoint for deleting a Testimonium link. + + Two modes of operation: if the form has ``antiquarian_request`` then + this is the PK of the Testimonium and the URL PK is for the Antiquarian; + in this case all the ``TestimoniumLinks`` between this Antiquarian and + this Testimonium will be removed. + + If the form has no ``antiquarian_request`` then the URL PK is for the + ``TestimoniumLink`` and this will be removed. + """ check_lock_object = "testimonium" model = TestimoniumLink + permission_required = ("research.change_testimonium",) + + def get_object(self): + """Get the object that we need to check the lock for: this is the Testimonium.""" + return self.testimonium + def dispatch(self, request, *args, **kwargs): - # need to ensure we have the lock object view attribute - # initialised in dispatch - if "antiquarian_request" in request.POST: - antiquarian_pk = kwargs["pk"] - testimonium_pk = request.POST.get("antiquarian_request") - self.get_antiquarian(antiquarian_pk) - self.get_testimonium(testimonium_pk) + pk = self.kwargs["pk"] + antiquarian_request = request.POST.get("antiquarian_request", None) + if antiquarian_request is None: + self.link = TestimoniumLink.objects.get(pk=pk) + self.testimonium = self.link.testimonium + self.antiquarian = self.link.antiquarian else: - self.get_testimonium() + self.link = None + self.testimonium = Testimonium.objects.get(pk=antiquarian_request) + self.antiquarian = Antiquarian.objects.get(pk=pk) return super().dispatch(request, *args, **kwargs) - permission_required = ("research.change_testimonium",) - - def get_success_url(self, *args, **kwargs): + def get_success_url(self): return self.request.META.get( "HTTP_REFERER", reverse("testimonium:detail", kwargs={"pk": self.testimonium.pk}), ) - def get_antiquarian(self, *args, **kwargs): - if not getattr(self, "antiquarian", False): - pk = args[0] - self.antiquarian = Antiquarian.objects.get(pk=pk) - return self.antiquarian + def get_response(self): + return HttpResponseRedirect(self.get_success_url()) - def get_testimonium(self, *args, **kwargs): - if not getattr(self, "testimonium", False): - if "antiquarian_request" in self.request.POST: - pk = args[0] - self.testimonium = Testimonium.objects.get(pk=pk) - else: - self.testimonium = self.get_object().testimonium - return self.testimonium + def post(self, request, *args, **kwargs): + return self.delete(request, *args, **kwargs) def delete(self, request, *args, **kwargs): - success_url = self.get_success_url() - testimonium = self.get_testimonium() - - if "antiquarian_request" in request.POST: - antiquarian = self.get_antiquarian() - antiquarian_testimoniumlinks = TestimoniumLink.objects.filter( - antiquarian=antiquarian, testimonium=testimonium - ) - for link in antiquarian_testimoniumlinks: - link.delete() - + if self.link is None: + self.model.objects.filter( + antiquarian=self.antiquarian, + testimonium=self.testimonium, + ).delete() + return self.get_response() + how_many_links = TestimoniumLink.objects.filter( + antiquarian=self.antiquarian, + testimonium=self.testimonium, + ).count() + if how_many_links == 1: + reassign_to_unknown(self.link) else: - self.object = self.get_object() - antiquarian = self.object.antiquarian - # Determine if it should reassign to unknown - # if no other links reassign to unknown - # otherwise delete the link - if ( - len( - TestimoniumLink.objects.filter( - antiquarian=antiquarian, testimonium=testimonium - ) - ) - == 1 - ): - reassign_to_unknown(self.object) - else: - self.object.delete() - - return HttpResponseRedirect(success_url) + self.link.delete() + return self.get_response() @method_decorator(require_POST, name="dispatch") diff --git a/src/rard/utils/shared_functions.py b/src/rard/utils/shared_functions.py index 0843f9730..963d0a1b7 100644 --- a/src/rard/utils/shared_functions.py +++ b/src/rard/utils/shared_functions.py @@ -34,7 +34,14 @@ def calculate_definite(organised_link_array): def reassign_to_unknown(worklink): - """Used in the Remove WorkLink Views""" + """ + Reassign a Testimonium, Fragment or Appositum as an unknown work. + + :params worklink: A ``TestimoniumLink``, ``FragmentLink`` or + ``AppositumLink``. The work will be assigned to the same + antiquarian it is currently assigned to, but an unknown work + of theirs. + """ worklink.work = worklink.antiquarian.unknown_work worklink.book = worklink.work.unknown_book worklink.definite_work = False diff --git a/src/requirements/base.txt b/src/requirements/base.txt index fe548eb90..2ed8558ca 100644 --- a/src/requirements/base.txt +++ b/src/requirements/base.txt @@ -8,10 +8,10 @@ hiredis==3.3.0 # https://github.com/redis/hiredis-py # Django # ------------------------------------------------------------------------------ -django==3.2.25 # https://www.djangoproject.com/ -django-environ==0.4.5 # https://github.com/joke2k/django-environ -django-model-utils==4.0.0 # https://github.com/jazzband/django-model-utils +django==4.2.30 # https://www.djangoproject.com/ +django-environ==0.13 # https://github.com/joke2k/django-environ +django-model-utils==5.0.0 # https://github.com/jazzband/django-model-utils django-redis==4.12.1 # https://github.com/jazzband/django-redis -django-bootstrap4==2.2.0 # https://pypi.org/project/django-bootstrap4/ +django-bootstrap4==26.1 # https://pypi.org/project/django-bootstrap4/ django-simple-history==2.12.0 # https://pypi.org/project/django-simple-history/ -django-crispy-forms==1.14.0 +django-crispy-forms==2.5 diff --git a/src/requirements/local.txt b/src/requirements/local.txt index 6e1122fde..cfa544c6c 100644 --- a/src/requirements/local.txt +++ b/src/requirements/local.txt @@ -1,6 +1,6 @@ -r base.txt -Werkzeug==1.0.1 # https://github.com/pallets/werkzeug +Werkzeug==3.1.8 # https://github.com/pallets/werkzeug ipdb==0.13.3 # https://github.com/gotcha/ipdb psycopg2==2.9.11 # https://github.com/psycopg/psycopg2 @@ -31,10 +31,6 @@ debugpy==1.6.7 # https://github.com/microsoft/debugpy factory-boy==3.0.1 # https://github.com/FactoryBoy/factory_boy django-debug-toolbar==3.8.1 # https://github.com/jazzband/django-debug-toolbar -django-extensions==3.0.8 # https://github.com/django-extensions/django-extensions +django-extensions==4.1 # https://github.com/django-extensions/django-extensions django-coverage-plugin==1.8.0 # https://github.com/nedbat/django_coverage_plugin pytest-django==4.11.1 # https://github.com/pytest-dev/pytest-django - -# Overrides -# ------------------------------------------------------------------------------ -jedi==0.17.2 # needs to be pinned to this as incompat with latest ipython From 50c11158ef2fcf489cdfac5207b64cdd7fd6b4fc Mon Sep 17 00:00:00 2001 From: tim-band <3266052+tim-band@users.noreply.github.com> Date: Fri, 22 May 2026 11:17:19 +0100 Subject: [PATCH 3/4] Update src/rard/templates/research/partials/ordered_work_area.html Thanks! That sorts it. Co-authored-by: Tom Couch --- src/rard/templates/research/partials/ordered_work_area.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rard/templates/research/partials/ordered_work_area.html b/src/rard/templates/research/partials/ordered_work_area.html index 6cd6205a8..88eb60581 100644 --- a/src/rard/templates/research/partials/ordered_work_area.html +++ b/src/rard/templates/research/partials/ordered_work_area.html @@ -29,9 +29,9 @@
- {% with tlinks=w2tlinks|get_item_list:work %} - {% with flinks=w2flinks|get_item_list:work %} - {% with alinks=w2alinks|get_item_list:work %} + {% with tlinks=w2tlinks|get_item_list:work.id %} + {% with flinks=w2flinks|get_item_list:work.id %} + {% with alinks=w2alinks|get_item_list:work.id %}
{% if work.unknown == True and not flinks and not tlinks and not alinks %} From 5a9eec613ad583b4ad16025a49e55b4f81f2fcd3 Mon Sep 17 00:00:00 2001 From: Tim Band Date: Fri, 22 May 2026 15:53:14 +0100 Subject: [PATCH 4/4] Removed duplicate link_text definitions --- .../partials/appositum_fragment_link_list_item.html | 4 ++-- .../research/partials/fragment_link_list_item.html | 4 ++-- .../templates/research/partials/ordered_work_area.html | 8 ++++---- .../research/partials/testimonium_link_list_item.html | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/rard/templates/research/partials/appositum_fragment_link_list_item.html b/src/rard/templates/research/partials/appositum_fragment_link_list_item.html index 081c57467..e5697796a 100644 --- a/src/rard/templates/research/partials/appositum_fragment_link_list_item.html +++ b/src/rard/templates/research/partials/appositum_fragment_link_list_item.html @@ -1,4 +1,4 @@ -{# Assumes a context object named 'link' and 'link_text'. Optional disable_link_controls to display but disable the link controls #} +{# Assumes a context object named 'link'. Optional disable_link_controls to display but disable the link controls #} {% load i18n name_in_context %} @@ -8,7 +8,7 @@
  • - {% with link_text=fragment|name_in_context:work %} + {% with link_text=fragment|name|safe_in_context:work|safe %} {% if perms.research.view_fragment %} {% include 'research/partials/render_definite_indicators.html' with link=link %} diff --git a/src/rard/templates/research/partials/fragment_link_list_item.html b/src/rard/templates/research/partials/fragment_link_list_item.html index 4bb4a8ebf..d4fa6ab5e 100644 --- a/src/rard/templates/research/partials/fragment_link_list_item.html +++ b/src/rard/templates/research/partials/fragment_link_list_item.html @@ -1,4 +1,4 @@ -{# Assumes a context object named 'link' and 'link_text'. Optional disable_link_controls to display but disable the link controls #} +{# Assumes a context object named 'link'. Optional disable_link_controls to display but disable the link controls #} {% load i18n name_in_context %} @@ -8,7 +8,7 @@
  • - {% with link_text=fragment|name_in_context:work %} + {% with link_text=fragment|name_in_context:work|safe %} {% if perms.research.view_fragment %} {% include 'research/partials/render_definite_indicators.html' with link=link %} diff --git a/src/rard/templates/research/partials/ordered_work_area.html b/src/rard/templates/research/partials/ordered_work_area.html index 88eb60581..d03020d93 100644 --- a/src/rard/templates/research/partials/ordered_work_area.html +++ b/src/rard/templates/research/partials/ordered_work_area.html @@ -9,7 +9,7 @@ {% with utlinks=object|testimonium_links_for_work:object.unknown_work %} {% for link in utlinks %} - {% include 'research/partials/testimonium_link_list_item.html' with link=link link_text=link.get_display_name|safe can_edit=can_edit has_object_lock=has_object_lock %} + {% include 'research/partials/testimonium_link_list_item.html' with link=link can_edit=can_edit has_object_lock=has_object_lock %} {% endfor %} {% endwith %} @@ -74,19 +74,19 @@ {% if tlinks %} {% for link in tlinks %} - {% include 'research/partials/testimonium_link_list_item.html' with link=link link_text=link.get_work_display_name_full|safe can_edit=False has_object_lock=has_object_lock %} + {% include 'research/partials/testimonium_link_list_item.html' with link=link can_edit=False has_object_lock=has_object_lock %} {% endfor %} {% endif %} {% if flinks %} {% for link in flinks %} - {% include 'research/partials/fragment_link_list_item.html' with link=link link_text=link.get_work_display_name_full|safe can_edit=False has_object_lock=has_object_lock %} + {% include 'research/partials/fragment_link_list_item.html' with link=link can_edit=False has_object_lock=has_object_lock %} {% endfor %} {% endif %} {% if alinks %} {% for link in alinks %} - {% include 'research/partials/appositum_fragment_link_list_item.html' with link=link link_text=link.get_work_display_name_full|safe can_edit=can_edit has_object_lock=has_object_lock %} + {% include 'research/partials/appositum_fragment_link_list_item.html' with link=link can_edit=can_edit has_object_lock=has_object_lock %} {% endfor %} {% endif %} diff --git a/src/rard/templates/research/partials/testimonium_link_list_item.html b/src/rard/templates/research/partials/testimonium_link_list_item.html index a5bf6bb4c..5a7694386 100644 --- a/src/rard/templates/research/partials/testimonium_link_list_item.html +++ b/src/rard/templates/research/partials/testimonium_link_list_item.html @@ -10,7 +10,7 @@
  • - {% with link_text=testimonium|name_in_context:work %} + {% with link_text=testimonium|name_in_context:work|safe %} {% if perms.research.view_testimonium %} {% include 'research/partials/render_definite_indicators.html' with link=link %}