Merge pull request #1726 from Princeton-CDH/feature/1720-people-tags

Add tags to Person model, admin, search (#1720)

geniza/common/models.py

@@ -2,6 +2,7 @@
 
 from django.contrib.auth.models import User
 from django.db import models
+from django.db.models.functions.text import Lower
 from django.utils.safestring import mark_safe
 from modeltranslation.utils import fallbacks
 
@@ -110,3 +111,17 @@ def objects_by_label(cls):
             (obj.display_label_en or obj.name_en): obj
             for obj in cls.objects.all()
         }
+
+
+class TaggableMixin:
+    """Mixin for taggable models with convenience functions for generating lists of tags"""
+
+    def all_tags(self):
+        """comma delimited string of all tags for this instance"""
+        return ", ".join(t.name for t in self.tags.all())
+
+    all_tags.short_description = "tags"
+
+    def alphabetized_tags(self):
+        """tags in alphabetical order, case-insensitive sorting"""
+        return self.tags.order_by(Lower("name"))

geniza/corpus/models.py

@@ -18,7 +18,6 @@
 from django.core.validators import RegexValidator
 from django.db import models
 from django.db.models.functions import Concat
-from django.db.models.functions.text import Lower
 from django.db.models.query import Prefetch
 from django.db.models.signals import pre_delete
 from django.dispatch import receiver
@@ -41,6 +40,7 @@
 from geniza.annotations.models import Annotation
 from geniza.common.models import (
     DisplayLabelMixin,
+    TaggableMixin,
     TrackChangesModel,
     cached_class_property,
 )
@@ -521,7 +521,7 @@ def permalink(self):
         return absolutize_url(self.get_absolute_url().replace(f"/{lang}/", "/"))
 
 
-class Document(ModelIndexable, DocumentDateMixin, PermalinkMixin):
+class Document(ModelIndexable, DocumentDateMixin, PermalinkMixin, TaggableMixin):
     """A unified document such as a letter or legal document that
     appears on one or more fragments."""
 
@@ -749,16 +749,6 @@ def formatted_citation(self):
             f"{long_name}. {available_at} {self.permalink}, accessed {today}."
         )
 
-    def all_tags(self):
-        """comma delimited string of all tags for this document"""
-        return ", ".join(t.name for t in self.tags.all())
-
-    all_tags.short_description = "tags"
-
-    def alphabetized_tags(self):
-        """tags in alphabetical order, case-insensitive sorting"""
-        return self.tags.order_by(Lower("name"))
-
     def is_public(self):
         """admin display field indicating if doc is public or suppressed"""
         return self.status == self.PUBLIC

geniza/entities/admin.py

@@ -267,6 +267,14 @@ class PersonEventInline(admin.TabularInline):
 class PersonAdmin(TabbedTranslationAdmin, SortableAdminBase, admin.ModelAdmin):
     """Admin for Person entities in the PGP"""
 
+    list_display = (
+        "__str__",
+        "slug",
+        "gender",
+        "role",
+        "all_tags",
+        "has_page",
+    )
     search_fields = ("name_unaccented", "names__name")
     fields = (
         "slug",
@@ -276,6 +284,7 @@ class PersonAdmin(TabbedTranslationAdmin, SortableAdminBase, admin.ModelAdmin):
         "date",
         "automatic_date",
         "description",
+        "tags",
     )
     readonly_fields = ("automatic_date",)
     inlines = (

geniza/entities/migrations/0026_person_tags.py

@@ -0,0 +1,26 @@
+# Generated by Django 3.2.23 on 2025-01-23 17:51
+
+import taggit_selectize.managers
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("taggit", "0003_taggeditem_add_unique_index"),
+        ("entities", "0025_person_date"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="person",
+            name="tags",
+            field=taggit_selectize.managers.TaggableManager(
+                blank=True,
+                help_text="A comma-separated list of tags.",
+                related_name="tagged_person",
+                through="taggit.TaggedItem",
+                to="taggit.Tag",
+                verbose_name="Tags",
+            ),
+        ),
+    ]

geniza/entities/models.py

@@ -3,7 +3,6 @@
 from datetime import datetime
 from math import modf
 from operator import itemgetter
-from time import sleep
 
 from django.conf import settings
 from django.contrib.admin.models import CHANGE, LogEntry
@@ -22,9 +21,10 @@
 from parasolr.django import AliasedSolrQuerySet
 from parasolr.django.indexing import ModelIndexable
 from slugify import slugify
+from taggit_selectize.managers import TaggableManager
 from unidecode import unidecode
 
-from geniza.common.models import TrackChangesModel, cached_class_property
+from geniza.common.models import TaggableMixin, TrackChangesModel, cached_class_property
 from geniza.corpus.dates import DocumentDateMixin, PartialDate, standard_date_display
 from geniza.corpus.models import (
     DisplayLabelMixin,
@@ -313,7 +313,9 @@ def related_delete(sender, instance=None, raw=False, **_kwargs):
         PersonSignalHandlers.related_change(instance, raw, "delete")
 
 
-class Person(ModelIndexable, SlugMixin, DocumentDatableMixin, PermalinkMixin):
+class Person(
+    ModelIndexable, SlugMixin, DocumentDatableMixin, PermalinkMixin, TaggableMixin
+):
     """A person entity that appears within the PGP corpus."""
 
     names = GenericRelation(Name, related_query_name="person")
@@ -360,6 +362,8 @@ class Person(ModelIndexable, SlugMixin, DocumentDatableMixin, PermalinkMixin):
 
     log_entries = GenericRelation(LogEntry, related_query_name="document")
 
+    tags = TaggableManager(blank=True, related_name="tagged_person")
+
     # gender options
     MALE = "M"
     FEMALE = "F"
@@ -888,6 +892,7 @@ def index_data(self):
                         "type__name_en", flat=True
                     ).distinct()
                 ),
+                "tags_ss_lower": [t.name for t in self.tags.all()],
             }
         )
         solr_date_range = self.solr_date_range()
@@ -969,13 +974,31 @@ class PersonSolrQuerySet(AliasedSolrQuerySet):
         "document_relations": "document_relation_ss",
         "date_str": "date_str_s",
         "has_page": "has_page_b",
+        "tags": "tags_ss_lower",
     }
 
+    search_aliases = field_aliases.copy()
+    search_aliases.update(
+        {
+            # when searching, singular makes more sense for tags
+            "tag": field_aliases["tags"],
+        }
+    )
+    re_solr_fields = re.compile(
+        r"(%s):" % "|".join(key for key, val in search_aliases.items() if key != val),
+        flags=re.DOTALL,
+    )
+
     keyword_search_qf = "{!type=edismax qf=$people_qf pf=$people_pf v=$keyword_query}"
 
     def keyword_search(self, search_term):
         """Allow searching using keywords with the specified query and phrase match
         fields, and set the default operator to AND"""
+        if ":" in search_term:
+            # if any of the field aliases occur with a colon, replace with actual solr field
+            search_term = self.re_solr_fields.sub(
+                lambda x: "%s:" % self.search_aliases[x.group(1)], search_term
+            )
         query_params = {"keyword_query": search_term, "q.op": "AND"}
         return self.search(self.keyword_search_qf).raw_query_parameters(
             **query_params,
@@ -984,6 +1007,7 @@ def keyword_search(self, search_term):
     def get_highlighting(self):
         """dedupe highlights across variant fields (e.g. for other_names)"""
         highlights = super().get_highlighting()
+        highlights = {k: v for k, v in highlights.items() if v}
         for person in highlights.keys():
             other_names = set()
             # iterate through other_names_* fields to get all matches

geniza/entities/templates/entities/person_list.html

@@ -169,6 +169,19 @@ <h2>
                             <td class="dates">{{ person.date_str }}</td>
                             <td class="role">{{ person.role }}</td>
                             <td class="description">{{ person.description.0|truncatewords:15 }}</td>
+                            {# tags #}
+                            <td class="tags">{% if person.tags %}
+                                <ul class="tags">
+                                    {% spaceless %}
+                                        {% for tag in person.tags|alphabetize|slice:":5" %}
+                                            <li><a href='{% url "entities:person-list" %}?q=tag:"{{ tag }}"'>{{ tag }}</a></li>
+                                        {% endfor %}
+                                        {% if person.tags|length > 5 %}
+                                            <li class="more">(+ {{ person.tags|length|add:"-5" }} {% translate 'more' %})</li>
+                                        {% endif %}
+                                    {% endspaceless %}
+                                </ul>
+                            {% endif %}</td>
                             <td class="related documents">
                                 <span class="label">{% translate "Related Documents" %}</span>
                                 <span>{{ person.documents }}</span>

geniza/entities/tests/test_entities_models.py

@@ -462,6 +462,9 @@ def test_index_data(self, person, person_multiname, document):
         PersonDocumentRelation.objects.create(
             person=person, document=document, type=pdrtype
         )
+        tags = ["testtag", "tag2"]
+        for t in tags:
+            person.tags.add(t)
         index_data = person.index_data()
         assert index_data["slug_s"] == person.slug
         assert index_data["name_s"] == str(person)
@@ -478,6 +481,7 @@ def test_index_data(self, person, person_multiname, document):
         assert index_data["documents_i"] == 1
         assert index_data["people_i"] == index_data["places_i"] == 0
         assert index_data["document_relation_ss"] == [str(pdrtype)]
+        assert index_data["tags_ss_lower"] == tags
         assert index_data["date_dr"] == person.solr_date_range()
         assert index_data["date_str_s"] == person.date_str
         assert index_data["start_dating_i"] == PartialDate("1200").numeric_format()
@@ -504,6 +508,13 @@ def test_keyword_search(self):
                     "q.op": "AND",
                 }
             )
+            pqs.keyword_search("tag:community")
+            mocksearch.return_value.raw_query_parameters.assert_called_with(
+                **{
+                    "keyword_query": "tags_ss_lower:community",
+                    "q.op": "AND",
+                }
+            )
 
     def test_get_highlighting(self):
         pqs = PersonSolrQuerySet()

sitemedia/scss/pages/_people.scss

@@ -136,6 +136,7 @@ main.people {
                     "date date date"
                     "role role role"
                     "desc desc desc"
+                    "tags tags tags"
                     "reldocs relpeople relplaces";
                 @include breakpoints.for-tablet-landscape-up {
                     display: flex;
@@ -177,6 +178,12 @@ main.people {
                         margin-top: 0.375rem;
                     }
                 }
+                td.tags:not(:empty) {
+                    grid-area: tags;
+                    border-bottom: 1px solid var(--disabled);
+                    padding-bottom: 0.625rem;
+                    margin-bottom: 0.625rem;
+                }
                 td.related.documents {
                     grid-area: reldocs;
                 }
@@ -257,6 +264,16 @@ main.people {
                 display: none;
             }
         }
+        td.tags:not(:empty) {
+            border-bottom: 1px solid var(--disabled);
+            padding-bottom: 0.625rem;
+            margin-bottom: 0.625rem;
+            @include breakpoints.for-tablet-landscape-up {
+                ul {
+                    margin-top: -0.25rem;
+                }
+            }
+        }
     }
     @include breakpoints.for-tablet-landscape-up {
         .topheader-row:has(input#switcher:checked) ~ section#person-list table {
@@ -315,6 +332,9 @@ main.people {
                             display: none;
                         }
                     }
+                    &.tags:not(:empty) {
+                        padding: 0;
+                    }
                 }
                 & + tr {
                     margin-top: 0;