Person-Document Relation Type merge (#1709)

geniza/entities/admin.py

@@ -50,7 +50,7 @@
     PlacePlaceRelation,
     PlacePlaceRelationType,
 )
-from geniza.entities.views import PersonMerge
+from geniza.entities.views import PersonDocumentRelationTypeMerge, PersonMerge
 from geniza.footnotes.models import Footnote
 
 
@@ -417,6 +417,43 @@ class PersonDocumentRelationTypeAdmin(TabbedTranslationAdmin, admin.ModelAdmin):
     search_fields = ("name",)
     ordering = ("name",)
 
+    @admin.display(description="Merge selected Person-Document relationships")
+    def merge_person_document_relation_types(self, request, queryset=None):
+        """Admin action to merge selected person-document relation types. This
+        action redirects to an intermediate page, which displays a form to
+        review for confirmation and choose the primary person before merging.
+        """
+        selected = request.POST.getlist("_selected_action")
+        if len(selected) < 2:
+            messages.error(
+                request,
+                "You must select at least two person-document relationships to merge",
+            )
+            return HttpResponseRedirect(
+                reverse("admin:entities_persondocumentrelationtype_changelist")
+            )
+        return HttpResponseRedirect(
+            "%s?ids=%s"
+            % (
+                reverse("admin:person-document-relation-type-merge"),
+                ",".join(selected),
+            ),
+            status=303,
+        )  # status code 303 means "See Other"
+
+    def get_urls(self):
+        """Return admin urls; adds custom URL for merging"""
+        urls = [
+            path(
+                "merge/",
+                PersonDocumentRelationTypeMerge.as_view(),
+                name="person-document-relation-type-merge",
+            ),
+        ]
+        return urls + super().get_urls()
+
+    actions = (merge_person_document_relation_types,)
+
 
 @admin.register(PersonPersonRelationType)
 class PersonPersonRelationTypeAdmin(TabbedTranslationAdmin, admin.ModelAdmin):

geniza/entities/forms.py

@@ -54,6 +54,45 @@ def __init__(self, *args, **kwargs):
         )
 
 
+class PersonDocumentRelationTypeChoiceField(forms.ModelChoiceField):
+    """Add a summary of each PersonDocumentRelationType to a form (used for merging)"""
+
+    label_template = get_template(
+        "entities/snippets/persondocumentrelationtype_option_label.html"
+    )
+
+    def label_from_instance(self, relation_type):
+        return self.label_template.render({"relation_type": relation_type})
+
+
+class PersonDocumentRelationTypeMergeForm(forms.Form):
+    primary_relation_type = PersonDocumentRelationTypeChoiceField(
+        label="Select primary person-document relationship",
+        queryset=None,
+        help_text=(
+            "Select the primary person-document relationship, which will be "
+            "used as the canonical entry. All associated relations and log "
+            "entries will be combined on the primary relationship."
+        ),
+        empty_label=None,
+        widget=forms.RadioSelect,
+    )
+
+    def __init__(self, *args, **kwargs):
+        ids = kwargs.get("ids", [])
+
+        # Remove the added kwarg so that the super method doesn't error
+        try:
+            del kwargs["ids"]
+        except KeyError:
+            pass
+
+        super().__init__(*args, **kwargs)
+        self.fields[
+            "primary_relation_type"
+        ].queryset = PersonDocumentRelationType.objects.filter(id__in=ids)
+
+
 class PersonPersonForm(forms.ModelForm):
     class Meta:
         model = PersonPersonRelation

geniza/entities/models.py

@@ -11,7 +11,7 @@
 from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
 from django.contrib.contenttypes.models import ContentType
 from django.core.validators import RegexValidator
-from django.db import models
+from django.db import IntegrityError, models
 from django.db.models import F, Q, Value
 from django.db.models.query import Prefetch
 from django.forms import ValidationError
@@ -1025,6 +1025,9 @@ class PersonDocumentRelationType(models.Model):
 
     name = models.CharField(max_length=255, unique=True)
     objects = PersonDocumentRelationTypeManager()
+    log_entries = GenericRelation(
+        LogEntry, related_query_name="persondocumentrelationtype"
+    )
 
     class Meta:
         verbose_name = "Person-Document relationship"
@@ -1041,6 +1044,64 @@ def objects_by_label(cls):
             for obj in cls.objects.all()
         }
 
+    def merge_with(self, merge_relation_types, user=None):
+        """Merge the specified relation types into this one. Combines all
+        relationships into this relation type and creates a log entry
+        documenting the merge.
+
+        Closely adapted from :class:`Person` merge."""
+
+        # if user is not specified, log entry will be associated with script
+        if user is None:
+            user = User.objects.get(username=settings.SCRIPT_USERNAME)
+
+        for rel_type in merge_relation_types:
+            # combine log entries
+            for log_entry in rel_type.log_entries.all():
+                # annotate and reassociate
+                # - modify change message to type which object this event applied to
+                log_entry.change_message = "%s [merged type %s (id = %d)]" % (
+                    log_entry.get_change_message(),
+                    str(rel_type),
+                    rel_type.pk,
+                )
+
+                # - associate with the primary relation type
+                log_entry.object_id = self.id
+                log_entry.content_type_id = ContentType.objects.get_for_model(
+                    PersonDocumentRelationType
+                )
+                log_entry.save()
+
+            # combine person-document relationships
+            for relationship in rel_type.persondocumentrelation_set.all():
+                relationship.type = self
+                # handle unique constraint violation (one relationship per type
+                # between doc and person): only reassign type if it doesn't
+                # create a duplicate, otherwise delete.
+                try:
+                    relationship.save()
+                except IntegrityError:
+                    relationship.delete()
+
+        # save current relation type with changes; delete merged relation types
+        self.save()
+        merged_types = ", ".join([str(rel_type) for rel_type in merge_relation_types])
+        for rel_type in merge_relation_types:
+            rel_type.delete()
+        # create log entry documenting the merge; include rationale
+        pdrtype_contenttype = ContentType.objects.get_for_model(
+            PersonDocumentRelationType
+        )
+        LogEntry.objects.log_action(
+            user_id=user.id,
+            content_type_id=pdrtype_contenttype.pk,
+            object_id=self.pk,
+            object_repr=str(self),
+            change_message="merged with %s" % (merged_types,),
+            action_flag=CHANGE,
+        )
+
 
 class PersonDocumentRelation(models.Model):
     """A relationship between a person and a document."""

geniza/entities/templates/admin/entities/persondocumentrelationtype/merge.html

@@ -0,0 +1,53 @@
+{% extends 'admin/base_site.html' %}
+
+{% load admin_urls static %}
+
+{% block extrastyle %}
+    {{ block.super }}
+    <link rel="stylesheet" href="{% static "admin/css/forms.css" %}">
+    <link rel="stylesheet" href="{% static "css/admin-local.css" %}">
+{% endblock %}
+
+{% block title %} Merge selected person-document relationships {% endblock %}
+
+{% block breadcrumbs %}
+    {% if not is_popup %}
+        <div class="breadcrumbs">
+            <a href="{% url 'admin:index' %}">Home</a>
+            &rsaquo;
+            <a href="{% url 'admin:app_list' app_label='entities' %}">Entities</a>
+            &rsaquo;
+            <a href="{% url 'admin:entities_persondocumentrelationtype_changelist'%}">Person-Document relationships</a>
+            &rsaquo;
+            Merge selected person-document relationships
+        </div>
+    {% endif %}
+{% endblock %}
+
+
+{% block content_title %}
+    <h1>Merge selected person-document relationships</h1>
+    <h2>Note: there is no automated way to unmerge! Please review to make sure these relationships should be merged before submitting the form.</h2>
+{% endblock %}
+
+{% block content %}
+    <form method="post" class="merge-document merge-relationtype">
+        {% csrf_token %}
+        {% if form.errors|length > 0 %}
+            <p class="errornote">
+                Please correct the error below.
+            </p>
+        {% endif %}
+        <fieldset class="module aligned">
+            <div class="form-row">
+                {{ form.primary_relation_type.label_tag }}
+                {{ form.primary_relation_type }}
+                <p class="help">{{ form.primary_relation_type.help_text|safe }}</p>
+            </div>
+
+            <div class="submit-row">
+                <input type="submit" value="Submit">
+            </div>
+        </field>
+    </form>
+{% endblock %}

geniza/entities/templates/entities/snippets/persondocumentrelationtype_option_label.html

@@ -0,0 +1,33 @@
+{# template snippet for displaying person-document relationship label on a form #}
+{# used on merge form to provide enough information to merge accurately #}
+
+<div class="merge-document-label">
+    <h2>{{ relation_type }}
+        <a target="_blank" href="{% url 'admin:entities_persondocumentrelationtype_change' relation_type.pk %}"
+           title="Go to this relationship's admin edit page">
+            <img src="/static/admin/img/icon-changelink.svg" alt="Change"></a>
+    </h2>
+    <details>
+        <summary>View all {{ relation_type.persondocumentrelation_set.count }} {{ relation_type }} relations</summary>
+        <ol>
+            {% for rel in relation_type.persondocumentrelation_set.all %}
+                <li>
+                    <div class="form-row">
+                        <label>{{ rel.type }} relation:</label>
+                        <div>
+                            <a target="_blank" href="{% url 'admin:entities_person_change' rel.person.pk %}">{{ rel.person }}</a>
+                            and
+                            <a target="_blank" href="{% url 'admin:corpus_document_change' rel.document.pk %}">{{ rel.document }}</a>
+                        </div>
+                    </div>
+                </li>
+            {% empty %}
+                <li>
+                    <div class="form-row">
+                        <label>No relations</label><div></div>
+                    </div>
+                </li>
+            {% endfor %}
+        </ol>
+    </details>
+</div>

geniza/entities/tests/test_entities_models.py

@@ -7,7 +7,6 @@
 from django.contrib.contenttypes.models import ContentType
 from django.forms import ValidationError
 from django.utils import timezone
-from modeltranslation.manager import MultilingualQuerySet
 from parasolr.django.indexing import ModelIndexable
 from slugify import slugify
 from unidecode import unidecode
@@ -706,6 +705,82 @@ def test_str(self):
         assert str(relation) == f"{recipient} relation: {goitein} and {doc}"
 
 
+@pytest.mark.django_db(transaction=True)
+class TestPersonDocumentRelationType:
+    def test_merge_with(self, person, person_multiname, document, join):
+        # create two PersonDocumentRelationTypes and some associations
+        rel_type = PersonDocumentRelationType.objects.create(name="test")
+        type_2 = PersonDocumentRelationType.objects.create(name="to be merged")
+        type_3 = PersonDocumentRelationType.objects.create(name="also merge me")
+        PersonDocumentRelation.objects.create(
+            type=rel_type, person=person, document=document
+        )
+        PersonDocumentRelation.objects.create(
+            type=type_2, person=person, document=document
+        )
+        PersonDocumentRelation.objects.create(
+            type=type_2, person=person_multiname, document=document
+        )
+        PersonDocumentRelation.objects.create(type=type_3, person=person, document=join)
+
+        # create some log entries
+        pdrtype_contenttype = ContentType.objects.get_for_model(
+            PersonDocumentRelationType
+        )
+        creation_date = timezone.make_aware(datetime(2023, 10, 12))
+        creator = User.objects.get_or_create(username="editor")[0]
+        type_2_str = str(type_2)
+        type_2_pk = type_2.pk
+        LogEntry.objects.bulk_create(
+            [
+                LogEntry(
+                    user_id=creator.id,
+                    content_type_id=pdrtype_contenttype.pk,
+                    object_id=type_2_pk,
+                    object_repr=type_2_str,
+                    change_message="first input",
+                    action_flag=ADDITION,
+                    action_time=creation_date,
+                ),
+                LogEntry(
+                    user_id=creator.id,
+                    content_type_id=pdrtype_contenttype.pk,
+                    object_id=type_2_pk,
+                    object_repr=type_2_str,
+                    change_message="major revision",
+                    action_flag=CHANGE,
+                    action_time=timezone.now(),
+                ),
+            ]
+        )
+        assert rel_type.persondocumentrelation_set.count() == 1
+        rel_type.merge_with([type_2, type_3])
+        # should skip the duplicate and add the others
+        assert rel_type.persondocumentrelation_set.count() == 3
+        # should delete other types and create merge log entry
+        assert not type_2.pk
+        assert not type_3.pk
+        assert LogEntry.objects.filter(
+            object_id=rel_type.pk,
+            change_message__contains=f"merged with {type_2}, {type_3}",
+        ).exists()
+        assert rel_type.log_entries.count() == 3
+        # based on default sorting, most recent log entry will be first
+        # - should document the merge event
+        merge_log = rel_type.log_entries.first()
+        assert merge_log.action_flag == CHANGE
+
+        # reassociated log entries should include merged type's name, id
+        assert (
+            " [merged type %s (id = %s)]" % (type_2_str, type_2_pk)
+            in rel_type.log_entries.all()[1].change_message
+        )
+        assert (
+            " [merged type %s (id = %s)]" % (type_2_str, type_2_pk)
+            in rel_type.log_entries.all()[2].change_message
+        )
+
+
 @pytest.mark.django_db
 class TestPlace:
     def test_str(self):