Merge pull request #1723 from Princeton-CDH/feature/1709-pdrtype-merge

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 type 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, transaction
 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,67 @@ 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():
+                # set type of each relationship to primary relation type
+                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.
+                # see https://docs.djangoproject.com/en/3.2/topics/db/transactions/#django.db.transaction.atomic
+                try:
+                    with transaction.atomic():
+                        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_forms.py

@@ -6,11 +6,12 @@
 from geniza.corpus.forms import FacetChoiceField
 from geniza.entities.forms import (
     PersonChoiceField,
+    PersonDocumentRelationTypeMergeForm,
     PersonListForm,
     PersonMergeForm,
     PlaceListForm,
 )
-from geniza.entities.models import Name, Person, PersonRole
+from geniza.entities.models import Name, Person, PersonDocumentRelationType, PersonRole
 
 
 class TestPersonChoiceField:
@@ -51,6 +52,31 @@ def test_init(self):
         assert people.last() not in mergeform.fields["primary_person"].queryset
 
 
+class TestPersonDocumentRelationTypeMergeForm:
+    @pytest.mark.django_db
+    def test_init(self):
+        # adapted from TestPersonMergeForm
+
+        # no error if ids not specified
+        PersonDocumentRelationTypeMergeForm()
+
+        # create test records
+        PersonDocumentRelationType.objects.bulk_create(
+            [PersonDocumentRelationType(name=f"test{i}") for i in range(4)]
+        )
+        # initialize with ids for all but the last
+        types = PersonDocumentRelationType.objects.all().order_by("pk")
+        ids = list(types.values_list("id", flat=True))
+        mergeform = PersonDocumentRelationTypeMergeForm(ids=ids[:-1])
+        # total should have all but one type
+        assert (
+            mergeform.fields["primary_relation_type"].queryset.count()
+            == types.count() - 1
+        )
+        # last type should not be an available choice
+        assert types.last() not in mergeform.fields["primary_relation_type"].queryset
+
+
 @pytest.mark.django_db
 class TestPersonListForm:
     def test_set_choices_from_facets(self, person, person_diacritic):