From 2331d6e4164f93ef04f9aee4c3e699c3a0fb7cef Mon Sep 17 00:00:00 2001 From: Ben Silverman Date: Wed, 24 Jul 2024 13:17:59 -0400 Subject: [PATCH] Add related entities to document detail page (#1625) --- .../templates/corpus/document_detail.html | 38 +++++++++ geniza/corpus/tests/test_corpus_views.py | 77 +++++++++++++++++++ geniza/corpus/views.py | 7 ++ sitemedia/scss/base/_typography.scss | 11 +++ sitemedia/scss/components/_transcription.scss | 1 + sitemedia/scss/pages/_document.scss | 48 +++++++++++- 6 files changed, 178 insertions(+), 4 deletions(-) diff --git a/geniza/corpus/templates/corpus/document_detail.html b/geniza/corpus/templates/corpus/document_detail.html index c1a9ea4dc..243a0d6a6 100644 --- a/geniza/corpus/templates/corpus/document_detail.html +++ b/geniza/corpus/templates/corpus/document_detail.html @@ -162,6 +162,44 @@

{% translate 'Description' %}

{{ document.description|pgp_urlize }}

+ {# related people #} + {% if related_people.exists %} +
+ {# Translators: heading label for document related people #} +

{% translate "Related People" %}

+
+ {% regroup related_people by type as person_relations %} + {% for relation_type in person_relations %} +
{{ relation_type.grouper }}
+
+ {% for relation in relation_type.list %} + {% if relation.person.get_absolute_url %}{{ relation.person }}{% else %}{{ relation.person }}{% endif %}{% if not forloop.last %}, {% endif %} + {% endfor %} +
+ {% endfor %} +
+
+ {% endif %} + + {# related places #} + {% if related_places.exists %} +
+ {# Translators: heading label for document related places #} +

{% translate "Related Places" %}

+
+ {% regroup related_places.all by type as place_relations %} + {% for relation_type in place_relations %} +
{{ relation_type.grouper }}
+
+ {% for relation in relation_type.list %} + {{ relation.place }}{% if not forloop.last %}, {% endif %} + {% endfor %} +
+ {% endfor %} +
+
+ {% endif %} + {% if document.tags.exists %}
{# Translators: label for tags on a document #} diff --git a/geniza/corpus/tests/test_corpus_views.py b/geniza/corpus/tests/test_corpus_views.py index cb26897b2..5c380ced9 100644 --- a/geniza/corpus/tests/test_corpus_views.py +++ b/geniza/corpus/tests/test_corpus_views.py @@ -36,6 +36,13 @@ old_pgp_tabulate_data, pgp_metadata_for_old_site, ) +from geniza.entities.models import ( + DocumentPlaceRelation, + Person, + PersonDocumentRelation, + PersonDocumentRelationType, + Place, +) from geniza.footnotes.forms import SourceChoiceForm from geniza.footnotes.models import Creator, Footnote, Source, SourceType @@ -176,6 +183,76 @@ def test_get_context_data(self, client, document, source): assert "transcription" in response.context["default_shown"] assert "transcription" not in response.context["disabled"] + # related people and places should be empty querysets + assert response.context["related_people"].count() == 0 + assert response.context["related_places"].count() == 0 + + # add related people + abu = Person.objects.create(slug="abu-imran") + ezra = Person.objects.create(slug="ezra-b-hillel") + nahray = Person.objects.create(slug="nahray") + (author, _) = PersonDocumentRelationType.objects.get_or_create(name="Author") + (recipient, _) = PersonDocumentRelationType.objects.get_or_create( + name="Recipient" + ) + PersonDocumentRelation.objects.create( + person=ezra, type=recipient, document=document + ) + PersonDocumentRelation.objects.create( + person=abu, type=recipient, document=document + ) + PersonDocumentRelation.objects.create( + person=nahray, type=author, document=document + ) + response = client.get(reverse("corpus:document", args=(document.pk,))) + assert response.context["related_people"].count() == 3 + # should sort alphabetically by type, then slug (name) + assert response.context["related_people"].first().person.pk == nahray.pk + assert response.context["related_people"][1].person.pk == abu.pk + + # add related place + fustat = Place.objects.create(slug="fustat") + DocumentPlaceRelation.objects.create(place=fustat, document=document) + assert response.context["related_places"].count() == 1 + assert response.context["related_places"].first().place.pk == fustat.pk + + def test_related_entities( + self, client, document, person, person_diacritic, person_multiname + ): + # add related people + person.has_page = True + person.save() + (author, _) = PersonDocumentRelationType.objects.get_or_create(name="Author") + (recipient, _) = PersonDocumentRelationType.objects.get_or_create( + name="Recipient" + ) + PersonDocumentRelation.objects.create( + person=person_multiname, type=recipient, document=document + ) + PersonDocumentRelation.objects.create( + person=person_diacritic, type=recipient, document=document + ) + PersonDocumentRelation.objects.create( + person=person, type=author, document=document + ) + # add related place + fustat = Place.objects.create(slug="fustat") + DocumentPlaceRelation.objects.create(place=fustat, document=document) + + # should group "recipient" people together and join their names by comma + response = client.get(reverse("corpus:document", args=(document.pk,))) + # should be "Halfon, Zed" = recipients + print(response.content) + assertContains(response, f"{person_diacritic}, {person_multiname}", html=True) + # should link to author because has_page=True + assertContains( + response, f'' + ) + # should link to place + assertContains( + response, f'' + ) + @pytest.mark.django_db def test_old_pgp_tabulate_data(): diff --git a/geniza/corpus/views.py b/geniza/corpus/views.py index 55cea70eb..2db628aaa 100644 --- a/geniza/corpus/views.py +++ b/geniza/corpus/views.py @@ -384,6 +384,13 @@ def get_context_data(self, **kwargs): for panel in ["images", "translation", "transcription"] if panel not in available_panels ], + # related entities: sorted by type for grouping, and slug for alphabetization + "related_people": self.object.persondocumentrelation_set.order_by( + "type__name", "person__slug" + ), + "related_places": self.object.documentplacerelation_set.order_by( + "type__name", "place__slug" + ), } ) return context_data diff --git a/sitemedia/scss/base/_typography.scss b/sitemedia/scss/base/_typography.scss index 6fa57d567..d17135b78 100644 --- a/sitemedia/scss/base/_typography.scss +++ b/sitemedia/scss/base/_typography.scss @@ -265,6 +265,17 @@ $text-size-7xl: 2.25rem; // = 36px line-height: 1.5; } +// small body text, e.g. for descriptions in search results +@mixin body-sm { + font-family: fonts.$primary; + font-size: $text-size-sm; + line-height: 17 / 14; + @include breakpoints.for-tablet-landscape-up { + font-size: $text-size-lg; + line-height: 25 / 18; + } +} + // label for unpublished records in scholarship records view @mixin unpublished { @include caption; diff --git a/sitemedia/scss/components/_transcription.scss b/sitemedia/scss/components/_transcription.scss index c6eb2e32c..e2a4c95f9 100644 --- a/sitemedia/scss/components/_transcription.scss +++ b/sitemedia/scss/components/_transcription.scss @@ -96,6 +96,7 @@ background-color: var(--background-light); cursor: default; &:after, + &:before, svg { color: var(--disabled); } diff --git a/sitemedia/scss/pages/_document.scss b/sitemedia/scss/pages/_document.scss index 0957ee913..36ae8e3b7 100644 --- a/sitemedia/scss/pages/_document.scss +++ b/sitemedia/scss/pages/_document.scss @@ -80,8 +80,8 @@ main.document { @include breakpoints.for-tablet-landscape-up { margin-left: 35px; } - :first-child { - margin-top: spacing.$spacing-md; + > :first-child { + margin-top: spacing.$spacing-sm; @include breakpoints.for-tablet-landscape-up { margin-top: 0px; } @@ -102,9 +102,10 @@ main.document { margin-left: 0; } } - // "what's in the PGP" and "description" + // "what's in the PGP", "description", "related people/places" section.content-stats, - section.description { + section.description, + section.related { margin: spacing.$spacing-md 0 0; @include breakpoints.for-tablet-landscape-up { margin: 2rem 0 0; @@ -165,6 +166,7 @@ main.document { font-size: typography.$text-size-xl; @include breakpoints.for-tablet-landscape-up { font-size: typography.$text-size-4xl; + padding-bottom: 2px; } } li.translation-count::before { @@ -185,6 +187,44 @@ main.document { padding-left: 1.25rem; } } + // related entities + section.related { + dl { + margin: 0 0 0 1.25rem; + dt { + padding-top: 0.5rem; + @include typography.body-italic; + } + dd + dt { + padding-top: 1rem; + margin-top: 1rem; + border-top: 1px solid var(--tabs-bottom); + } + dd { + @include typography.body-sm; + } + @include breakpoints.for-tablet-landscape-up { + display: grid; + grid-template-columns: 33% 1fr; + margin: 1.25rem 0 0 1.25rem; + gap: 0.5rem 0; + dt, + dd { + padding-top: 0; + @include typography.body; + font-style: normal; + } + dd + dt, + dd + dt + dd { + margin-top: 0; + padding-top: 0.5rem; + border-top: 1px solid var(--tabs-bottom); + } + } + } + padding-bottom: 2rem; + border-bottom: 1px solid var(--disabled); + } // link to download transcription section.transcription-link { margin-top: spacing.$spacing-md;