Merge pull request #1572 from Princeton-CDH/feature/1568-people-browse

Add browse page for people (#1568)

geniza/entities/models.py

@@ -253,6 +253,9 @@ class Person(TrackChangesModel):
     )
     gender = models.CharField(max_length=1, choices=GENDER_CHOICES)
 
+    # minimum documents to show a page if has_page is False
+    MIN_DOCUMENTS = 10
+
     class Meta:
         verbose_name_plural = "People"
 
@@ -279,7 +282,10 @@ def __str__(self):
 
     def get_absolute_url(self):
         """url for this person"""
-        return reverse("entities:person", args=[self.slug])
+        if self.documents.count() >= self.MIN_DOCUMENTS or self.has_page == True:
+            return reverse("entities:person", args=[str(self.slug)])
+        else:
+            return None
 
     def merge_with(self, merge_people, user=None):
         """Merge the specified people into this one. Combines all metadata

geniza/entities/templates/entities/person_list.html

@@ -0,0 +1,117 @@
+{% extends 'base.html' %}
+{% load static i18n humanize %}
+
+{% block meta_title %}{{ page_title }}{% endblock meta_title %}
+{% block meta_description %}{{ page_description }}{% endblock meta_description %}
+
+{% block main %}
+    <h1>{{ page_title }}</h1>
+    <section id="person-list">
+        <h2>
+            {# Translators: number of search results #}
+            {% blocktranslate with count_humanized=paginator.count|intcomma count counter=paginator.count trimmed %}
+                1 result
+            {% plural %}
+                {{ count_humanized }} results
+            {% endblocktranslate %}
+        </h2>
+        {# list view table #}
+        <table>
+            <thead>
+                {# Translators: Person "name" column header on the browse page #}
+                <th>{% translate "Name" %}</th>
+                {# Translators: Person "dates of activity" column header on the browse page #}
+                {% comment %} <th>{% translate "Dates" %}</th> {% endcomment %}
+                {# Translators: Person "gender" column header on the browse page #}
+                <th>{% translate "Gender" %}</th>
+                {# Translators: Person "social role" column header on the browse page #}
+                <th>{% translate "Social role" %}</th>
+                <th class="related">
+                    <svg aria-hidden="true"><use xlink:href="{% static 'img/ui/all/all/related-documents-icon.svg' %}#related-documents" /></svg>
+                    {# Translators: Person "document count" column header on the browse page #}
+                    <span class="sr-only">{% translate "Number of related documents" %}</span>
+                </th>
+                <th class="related">
+                    <svg aria-hidden="true"><use xlink:href="{% static 'img/ui/all/all/related-people-icon.svg' %}#related-people" /></svg>
+                    {# Translators: Person "related people count" column header on the browse page #}
+                    <span class="sr-only">{% translate "Number of related people" %}</span>
+                </th>
+                <th class="related">
+                    <svg aria-hidden="true"><use xlink:href="{% static 'img/ui/all/all/related-places-icon.svg' %}#related-places" /></svg>
+                    {# Translators: Person "related place count" column header on the browse page #}
+                    <span class="sr-only">{% translate "Number of related places" %}</span>
+                </th>
+            </thead>
+            <tbody>
+                {% for person in people %}
+                    <tr>
+                        <td class="name">
+                            {% if person.get_absolute_url %}
+                                <a href="{{ person.get_absolute_url }}">{{ person }}</a>
+                            {% else %}
+                                <span>{{ person }}</span>
+                            {% endif %}
+                        </td>
+                        {% comment %} <td>{{ person.documents_date_range }}</td> {% endcomment %}
+                        <td>{{ person.get_gender_display }}</td>
+                        <td>{{ person.role }}</td>
+                        <td class="related">{{ person.documents.count }}</td>
+                        <td class="related">{{ person.relationships.count }}</td>
+                        <td class="related">{{ person.personplacerelation_set.count }}</td>
+                    </tr>
+                {% endfor %}
+            </tbody>
+        </table>
+        {# grid / mobile view #}
+        <div class="grid">
+            {% for person in people %}
+                <div class="person">
+                    {# Translators: accessible label for section showing metadata like name, gender, etc #}
+                    <dl aria-label="{% translate 'metadata' %}">
+                        <dt class="sr-only">{% translate "Name" %}</dt>
+                        <dd class="name">
+                            {% if person.get_absolute_url %}
+                                <a href="{{ person.get_absolute_url }}">{{ person }}</a>
+                            {% else %}
+                                <span>{{ person }}</span>
+                            {% endif %}
+                        </dd>
+                        <dt class="sr-only">{% translate "Gender" %}</dt>
+                        <dd>{{ person.get_gender_display }}</dd>
+                        {% comment %}<dt class="sr-only">{% translate 'Dates' %}</dt><dd></dd>{% endcomment %}
+                        <dt class="sr-only">{% translate "Social role" %}</dt>
+                        <dd>{{ person.role }}</dd>
+                        {# Translators: label for person description / bio #}
+                        <dt class="sr-only">{% translate "Description / Bio" %}</dt>
+                        <dd class="description">{{ person.description|truncatewords:15 }}</dd>
+                    </dl>
+                    {# Translators: accessible label for section showing counts of entries related to an entity #}
+                    <dl class="relations" aria-label="{% translate 'Related entries' %}">
+                        <dt class="sr-only">{% translate "Number of related people" %}</dt>
+                        <dd>
+                            <svg aria-hidden="true"><use xlink:href="{% static 'img/ui/all/all/related-people-icon.svg' %}#related-people" /></svg>
+                            <span>{{ person.relationships.count }}</span>
+                        </dd>
+                        <dt class="sr-only">{% translate "Number of related places" %}</dt>
+                        <dd>
+                            <svg aria-hidden="true"><use xlink:href="{% static 'img/ui/all/all/related-places-icon.svg' %}#related-places" /></svg>
+                            <span>{{ person.personplacerelation_set.count }}</span>
+                        </dd>
+                        <dt class="sr-only">{% translate "Number of related documents" %}</dt>
+                        <dd>
+                            <svg aria-hidden="true"><use xlink:href="{% static 'img/ui/all/all/related-documents-icon.svg' %}#related-documents" /></svg>
+                            <span>{{ person.documents.count }}</span>
+                        </dd>
+                    </dl>
+                </div>
+            {% endfor %}
+        </div>
+    </section>
+    <div class="pagination-container">
+        {# Translators: range of search results on the current page, out of total #}
+        {% blocktranslate with start=page_obj.start_index end=page_obj.end_index count_humanized=paginator.count|intcomma %}
+            <div>{{ start }} – {{ end }} of {{ count_humanized }}</div>
+        {% endblocktranslate %}
+        {% include "corpus/snippets/pagination.html" %}
+    </div>
+{% endblock %}

geniza/entities/templates/entities/snippets/person_header.html

@@ -1,5 +1,12 @@
 {% load i18n %}
 
+<div class="breadcrumbs-link-container">
+    <a href="{% url 'entities:person-list' %}">
+        {# Translators: Link to return to the list of people from a person page #}
+        {% translate "Back to People" %}
+    </a>
+</div>
+
 {# title #}
 <h1>{{ person }}</h1>
 
@@ -8,8 +15,7 @@ <h1>{{ person }}</h1>
     <div class="edit-link-container">
         <a class="edit-link" href="{% url 'admin:entities_person_change' person.pk %}" data-turbo="false">
             <i class="ph-pencil"></i>
-            {# Translators: Person edit link for admins #}
-            <span>{% translate 'Edit' %}</span>
+            <span>Edit</span>
         </a>
     </div>
 {% endif %}

geniza/entities/tests/test_entities_models.py

@@ -320,8 +320,19 @@ def test_merge_with_log_entries(self):
         )
 
     def test_get_absolute_url(self):
-        # should get person page url in user's language by slug
+        # should not have an absolute url if has_page is false and < MIN_DOCUMENTS associated docs
         person = Person.objects.create()
+        assert person.get_absolute_url() == None
+
+        # has_page is true, should get the url in user's language by slug
+        person.has_page = True
+        assert person.get_absolute_url() == "/en/people/%s/" % person.slug
+
+        # has_page is false but has >= MIN_DOCUMENTS, should get url
+        person.has_page = False
+        for _ in range(Person.MIN_DOCUMENTS):
+            d = Document.objects.create()
+            person.documents.add(d)
         assert person.get_absolute_url() == "/en/people/%s/" % person.slug
 
     def test_save(self):

geniza/entities/tests/test_entities_views.py

@@ -11,6 +11,7 @@
 from geniza.entities.views import (
     PersonAutocompleteView,
     PersonDetailView,
+    PersonListView,
     PersonMerge,
     PlaceAutocompleteView,
 )
@@ -193,7 +194,7 @@ def test_get_queryset(self, client):
         assert response.status_code == 404
 
         # should 200 on person with 10+ associated documents
-        for _ in range(PersonDetailView.MIN_DOCUMENTS):
+        for _ in range(Person.MIN_DOCUMENTS):
             d = Document.objects.create()
             person.documents.add(d)
         response = client.get(reverse("entities:person", args=(person.slug,)))
@@ -211,11 +212,37 @@ def test_get_context_data(self, client):
         assert response.context["page_type"] == "person"
 
 
+@pytest.mark.django_db
+class TestPersonListView:
+    def test_get_queryset(self):
+        # create people with and without diacritics in their names
+        p1 = Person.objects.create()
+        Name.objects.create(name="Example", content_object=p1, primary=True)
+        p2 = Person.objects.create()
+        Name.objects.create(name="Ḥalfon b. Menashshe", content_object=p2, primary=True)
+        p3 = Person.objects.create()
+        Name.objects.create(name="Zed", content_object=p3, primary=True)
+        Name.objects.create(name="Apple", content_object=p3, primary=False)
+        personlist_view = PersonListView()
+
+        # should order diacritics unaccented
+        qs = personlist_view.get_queryset()
+        assert qs.first().pk == p1.pk  # Example
+        assert qs[1].pk == p2.pk  # Halfon
+        # should order by primary name only
+        assert qs[2].pk == p3.pk  # Zed
+
+    def test_get_context_data(self, client):
+        # context should include "page_type": "people"
+        response = client.get(reverse("entities:person-list"))
+        assert response.context["page_type"] == "people"
+
+
 @pytest.mark.django_db
 class TestPersonDetailMixin:
     def test_get(self, client):
         # should redirect on past slug
-        person = Person.objects.create()
+        person = Person.objects.create(has_page=True)
         name1 = Name.objects.create(name="Imran", content_object=person, primary=True)
         person.generate_slug()
         person.save()
@@ -230,6 +257,3 @@ def test_get(self, client):
         response = client.get(reverse("entities:person", args=(old_slug,)))
         assert response.status_code == 301
         assert response.url == person.get_absolute_url()
-
-        # should still raise 404 if conditions aren't met (has_page or MIN_DOCUMENTS)
-        assert client.get(response.url).status_code == 404

geniza/entities/urls.py

@@ -5,6 +5,7 @@
 app_name = "entities"
 
 urlpatterns = [
+    path("people/", entities_views.PersonListView.as_view(), name="person-list"),
     path(
         "people/<slug:slug>/",
         entities_views.PersonDetailView.as_view(),

geniza/entities/views.py

@@ -8,7 +8,8 @@
 from django.urls import reverse
 from django.utils.safestring import mark_safe
 from django.utils.text import Truncator
-from django.views.generic import DetailView, FormView
+from django.utils.translation import gettext as _
+from django.views.generic import DetailView, FormView, ListView
 
 from geniza.entities.forms import PersonMergeForm
 from geniza.entities.models import PastPersonSlug, Person, Place
@@ -129,7 +130,6 @@ class PersonDetailView(PersonDetailMixin):
 
     model = Person
     context_object_name = "person"
-    MIN_DOCUMENTS = 10
 
     def page_title(self):
         """page title, for metadata; uses Person primary name"""
@@ -149,7 +149,9 @@ def get_queryset(self, *args, **kwargs):
                 doc_count=Count("documents", distinct=True),
             )
         )
-        return queryset.filter(Q(doc_count__gte=self.MIN_DOCUMENTS) | Q(has_page=True))
+        return queryset.filter(
+            Q(doc_count__gte=Person.MIN_DOCUMENTS) | Q(has_page=True)
+        )
 
     def get_context_data(self, **kwargs):
         """extend context data to add page metadata"""
@@ -163,3 +165,38 @@ def get_context_data(self, **kwargs):
             }
         )
         return context_data
+
+
+class PersonListView(ListView):
+    model = Person
+    context_object_name = "people"
+    template_name = "entities/person_list.html"
+    # Translators: title of people list/browse page
+    page_title = _("People")
+    # Translators: description of people list/browse page
+    page_description = _("Browse people present in Geniza documents.")
+    paginate_by = 50
+
+    def get_queryset(self, *args, **kwargs):
+        """modify queryset to sort and filter on people in the list"""
+        # order people by primary name unaccented
+        return (
+            Person.objects.filter(names__primary=True)
+            .annotate(
+                name_unaccented=ArrayAgg("names__name__unaccent", distinct=True),
+            )
+            .order_by("name_unaccented")
+        )
+
+    def get_context_data(self, **kwargs):
+        """extend context data to add page metadata"""
+        context_data = super().get_context_data(**kwargs)
+
+        context_data.update(
+            {
+                "page_title": self.page_title,
+                "page_description": self.page_description,
+                "page_type": "people",
+            }
+        )
+        return context_data