Add related places to place detail page (#1696)

geniza/entities/models.py

@@ -1420,6 +1420,92 @@ def items_to_index(cls):
             "names", "documentplacerelation_set", "personplacerelation_set"
         )
 
+    def related_places(self):
+        """Set of all places related to this place, with relationship type,
+        taking into account converse relations"""
+
+        # adapted from Person.related_people
+
+        # gather all relationships with places, both entered from this place and
+        # entered from the place on the other side of the relationship
+        place_relations = (
+            self.place_a.annotate(
+                # boolean to indicate if we should use converse or regular relation type name
+                use_converse_typename=Value(True),
+                related_slug=F("place_a__slug"),
+                related_id=F("place_a"),
+            ).union(  # union instead of joins for efficiency
+                self.place_b.annotate(
+                    use_converse_typename=Value(False),
+                    related_slug=F("place_b__slug"),
+                    related_id=F("place_b"),
+                )
+            )
+            # have to use values_list (NOT values) here with one argument, otherwise strange things
+            # happen, possibly due to union(). TODO: see if this is fixed in later django versions.
+            .values_list("related_id")
+        )
+        relation_list = [
+            {
+                # this is the order of fields returned by SQL after the annotated union
+                "id": r[-1],
+                "slug": r[-2],
+                "use_converse_typename": r[-3],
+                "notes": r[-4],
+                "type_id": r[-5],
+            }
+            for r in place_relations
+        ]
+
+        # folow GenericForeignKey to find primary name for each related place
+        place_contenttype = ContentType.objects.get_for_model(Place).pk
+        names = Name.objects.filter(
+            object_id__in=[r["id"] for r in relation_list],
+            primary=True,
+            content_type_id=place_contenttype,
+        ).values("name", "object_id")
+        # dict keyed on related place id
+        names_dict = {n["object_id"]: n["name"] for n in names}
+
+        # grab name and converse_name for each relation type since we may need either
+        # (name if the relation was entered from self, converse if entered from related place)
+        types = PlacePlaceRelationType.objects.filter(
+            pk__in=[r["type_id"] for r in relation_list],
+        ).values("pk", "name", "converse_name")
+        # dict keyed on related place id
+        types_dict = {t["pk"]: t for t in types}
+
+        # update with new data & dedupe
+        prev_relation = None
+        # sort by id (dedupe by matching against previous id), then type id for type dedupe
+        for relation in sorted(relation_list, key=itemgetter("id", "type_id")):
+            relation.update(
+                {
+                    # get name from cached queryset dict
+                    "name": names_dict[relation["id"]],
+                    # use type.converse_name if this relation is reverse (and if the type has one)
+                    "type": types_dict[relation["type_id"]][
+                        "converse_name" if relation["use_converse_typename"] else "name"
+                    ]
+                    # fallback to type.name if converse_name doesn't exist
+                    or types_dict[relation["type_id"]]["name"],
+                }
+            )
+            # dedupe and combine type and notes
+            if prev_relation and prev_relation["id"] == relation["id"]:
+                # dedupe type by string matching since we can't match reverse relations by id
+                if relation["type"].lower() not in prev_relation["type"].lower():
+                    prev_relation["type"] += f", {relation['type']}".lower()
+                # simply combine notes with html line break
+                prev_relation["notes"] += (
+                    f"<br />{relation['notes']}" if relation["notes"] else ""
+                )
+                relation_list.remove(relation)
+            else:
+                prev_relation = relation
+
+        return relation_list
+
     def index_data(self):
         """data for indexing in Solr"""
         index_data = super().index_data()

geniza/entities/templates/entities/place_detail.html

@@ -86,5 +86,26 @@ <h2>
                 <p>{{ place.notes }}</p>
             </section>
         {% endif %}
+        {# related places #}
+        {% if related_places %}
+            <section class="related">
+                {# Translators: heading label for document related places #}
+                <h2>{% translate "Related Places" %}</h2>
+                <dl>
+                    {% regroup related_places by type as place_relations %}
+                    {% for relation_type in place_relations %}
+                        <dt>{{ relation_type.grouper }}</dt>
+                        <dd>
+                            {% for relation in relation_type.list %}
+                                {% url "entities:place" relation.slug as place_url %}
+                                <a data-turbo="false" href="{{ place_url }}" title="{{ relation.name }}">
+                                    {{ relation.name }}
+                                </a>{% if not forloop.last %}, {% endif %}
+                            {% endfor %}
+                        </dd>
+                    {% endfor %}
+                </dl>
+            </section>
+        {% endif %}
     </div>
 {% endblock main %}

geniza/entities/tests/test_entities_views.py

@@ -21,6 +21,8 @@
     PersonPlaceRelationType,
     PersonSolrQuerySet,
     Place,
+    PlacePlaceRelation,
+    PlacePlaceRelationType,
     PlaceSolrQuerySet,
 )
 from geniza.entities.views import (
@@ -660,6 +662,45 @@ def test_get_context_data(self, client):
         # context should include the maptiler token if one exists in settings
         assert response.context["maptiler_token"] == "example"
 
+    def test_related_places(self, client):
+        tlebanon = Place.objects.create(slug="tripoli-lebanon")
+        Name.objects.create(
+            name="Tripoli (Lebanon)", content_object=tlebanon, primary=True
+        )
+
+        # create some place-place relationshipss
+        tgreece = Place.objects.create(slug="tripoli-greece")
+        Name.objects.create(
+            name="Tripoli (Greece)", content_object=tgreece, primary=True
+        )
+        (nbc, _) = PlacePlaceRelationType.objects.get_or_create(
+            name="Not to be confused with"
+        )
+        PlacePlaceRelation.objects.create(place_a=tlebanon, place_b=tgreece, type=nbc)
+
+        # add an asymmetrical one from another place, it should show up too, but with converse name
+        zahriyeh = Place.objects.create(slug="zahriyeh")
+        Name.objects.create(name="Zahriyeh", content_object=zahriyeh, primary=True)
+        (city_neighborhood, _) = PlacePlaceRelationType.objects.get_or_create(
+            name="City", converse_name="Neighborhood"
+        )
+        PlacePlaceRelation.objects.create(
+            place_a=zahriyeh, place_b=tlebanon, type=city_neighborhood
+        )
+
+        response = client.get(reverse("entities:place", args=(tlebanon.slug,)))
+        assert len(response.context["related_places"]) == 2
+        # should be ordered by type name
+        assert (
+            response.context["related_places"][0]["type"]
+            == city_neighborhood.converse_name
+        )
+        assert response.context["related_places"][0]["use_converse_typename"] == True
+        assert response.context["related_places"][0]["name"] == str(zahriyeh)
+        assert response.context["related_places"][1]["type"] == nbc.name
+        assert response.context["related_places"][1]["use_converse_typename"] == False
+        assert response.context["related_places"][1]["name"] == str(tgreece)
+
 
 @pytest.mark.django_db
 class TestPersonDocumentsView:

geniza/entities/views.py

@@ -613,6 +613,9 @@ def get_context_data(self, **kwargs):
                 "page_description": self.page_description(),
                 "page_type": "place",
                 "maptiler_token": getattr(settings, "MAPTILER_API_TOKEN", ""),
+                "related_places": sorted(
+                    self.object.related_places(), key=lambda rp: rp["type"]
+                ),
             }
         )
         return context_data

sitemedia/scss/pages/_document.scss

@@ -189,39 +189,6 @@ main.document {
         }
         // 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);
         }
@@ -264,6 +231,46 @@ main.document {
     }
 }
 
+main.document,
+main.place {
+    // 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);
+                }
+            }
+        }
+    }
+}
+
 section#document-list,
 main.document {
     // description content

sitemedia/scss/pages/_person.scss

@@ -187,7 +187,8 @@ main.place {
         section.metadata dt,
         section.description h2,
         section.events h2,
-        section.notes h2 {
+        section.notes h2,
+        section.related h2 {
             margin-bottom: spacing.$spacing-xs;
             @include typography.meta-bold;
             font-size: typography.$text-size-lg;

sitemedia/scss/pages/_place.scss

@@ -1,5 +1,6 @@
 @use "../base/breakpoints";
 @use "../base/spacing";
+@use "../base/typography";
 
 main.place {
     * {
@@ -55,6 +56,12 @@ main.place {
             }
         }
     }
+    section.related {
+        margin: spacing.$spacing-md 0 0;
+        @include breakpoints.for-tablet-landscape-up {
+            margin: 2rem 0 0;
+        }
+    }
 }
 
 html.dark-mode main.place .map svg#marker {