More properly display related objs in admin diff

`ForeignKey` objects are now displayed using their `__str__()` methods.
`ManyToManyField` objects are displayed using their `__repr__()` methods
(which call `__str__()` by default) - since they're converted to a
string while inside a list.

Discussion:
I could've made `format_history_delta_change_value()` return a list of
normal `str()` representations (instead of `repr()`) for M2M fields,
but decided against it, as it would have prevented users from taking
advantage of the "meta" code in our implementation of the method for
getting the actual M2M objects. I guess it can be done once jazzband#1058 has
a fix, though - as then the "meta" code wouldn't be needed.

simple_history/admin.py

@@ -8,7 +8,7 @@
 from django.contrib.admin.utils import unquote
 from django.contrib.auth import get_permission_codename, get_user_model
 from django.core.exceptions import PermissionDenied
-from django.db.models import QuerySet
+from django.db.models import ManyToManyField, QuerySet
 from django.shortcuts import get_object_or_404, render
 from django.template.defaultfilters import truncatechars
 from django.urls import re_path, reverse
@@ -17,9 +17,13 @@
 from django.utils.text import capfirst
 from django.utils.translation import gettext as _
 
-from .manager import HistoryManager
+from .manager import HistoricalQuerySet, HistoryManager
 from .models import HistoricalChanges, ModelChange, PKOrRelatedObj
-from .utils import get_history_manager_for_model, get_history_model_for_model
+from .utils import (
+    get_history_manager_for_model,
+    get_history_model_for_model,
+    get_m2m_reverse_field_name,
+)
 
 SIMPLE_HISTORY_EDIT = getattr(settings, "SIMPLE_HISTORY_EDIT", False)
 
@@ -120,10 +124,12 @@ def get_history_queryset(
         :param pk_name: The name of the original model's primary key field.
         :param object_id: The primary key of the object whose history is listed.
         """
-        qs = history_manager.filter(**{pk_name: object_id})
+        qs: HistoricalQuerySet = history_manager.filter(**{pk_name: object_id})
         if not isinstance(history_manager.model.history_user, property):
             # Only select_related when history_user is a ForeignKey (not a property)
             qs = qs.select_related("history_user")
+        # Prefetch related objects to reduce the number of DB queries when diffing
+        qs = qs.select_related_history_tracked_objs()
         return qs
 
     def get_history_list_display(self, request) -> Sequence[str]:
@@ -135,17 +141,28 @@ def get_history_list_display(self, request) -> Sequence[str]:
         return self.history_list_display
 
     def set_history_delta_changes(
-        self, historical_records: Sequence[HistoricalChanges]
+        self,
+        historical_records: Sequence[HistoricalChanges],
+        foreign_keys_are_objs=True,
     ):
-        # Add a `history_delta_changes` attribute to all historical records
-        # except the first (oldest) one
+        """
+        Add a ``history_delta_changes`` attribute to all historical records
+        except the first (oldest) one.
+
+        :param historical_records:
+        :param foreign_keys_are_objs: Passed to ``diff_against()`` when getting deltas;
+               see its docstring for details.
+        """
         previous = None
         current = None
         for current in historical_records:
             if previous is None:
                 previous = current
                 continue
-            delta = previous.diff_against(current)
+            # Related objects should have been prefetched in `get_history_queryset()`
+            delta = previous.diff_against(
+                current, foreign_keys_are_objs=foreign_keys_are_objs
+            )
             previous.history_delta_changes = [
                 self.history_delta_change_context(change, previous)
                 for change in delta.changes
@@ -193,7 +210,16 @@ def format_history_delta_change_value(
         :param change:
         :param historical_record: The table row the returned value will be displayed on.
         """
-        return value
+        field_meta = self.model._meta.get_field(change.field)
+        if isinstance(field_meta, ManyToManyField):
+            reverse_field_name = get_m2m_reverse_field_name(field_meta)
+            # Display a list of only the instances of the M2M field's related model
+            display_value = [
+                obj_values_dict[reverse_field_name] for obj_values_dict in value
+            ]
+        else:
+            display_value = value
+        return display_value
 
     def history_view_title(self, request, obj):
         if self.revert_disabled(request, obj) and not SIMPLE_HISTORY_EDIT:

simple_history/tests/admin.py

@@ -14,6 +14,7 @@
     Person,
     Planet,
     Poll,
+    PollWithManyToMany,
 )
 
 
@@ -43,14 +44,15 @@ def test_method(self, obj):
     history_list_display = ["title", "test_method"]
 
 
-admin.site.register(Poll, SimpleHistoryAdmin)
-admin.site.register(Choice, ChoiceAdmin)
-admin.site.register(Person, PersonAdmin)
 admin.site.register(Book, SimpleHistoryAdmin)
+admin.site.register(Choice, ChoiceAdmin)
+admin.site.register(ConcreteExternal, SimpleHistoryAdmin)
 admin.site.register(Document, SimpleHistoryAdmin)
-admin.site.register(Paper, SimpleHistoryAdmin)
 admin.site.register(Employee, SimpleHistoryAdmin)
-admin.site.register(ConcreteExternal, SimpleHistoryAdmin)
 admin.site.register(ExternalModelWithCustomUserIdField, SimpleHistoryAdmin)
 admin.site.register(FileModel, FileModelAdmin)
+admin.site.register(Paper, SimpleHistoryAdmin)
+admin.site.register(Person, PersonAdmin)
 admin.site.register(Planet, PlanetAdmin)
+admin.site.register(Poll, SimpleHistoryAdmin)
+admin.site.register(PollWithManyToMany, SimpleHistoryAdmin)

simple_history/tests/tests/test_admin.py

@@ -30,8 +30,10 @@
     Employee,
     FileModel,
     Person,
+    Place,
     Planet,
     Poll,
+    PollWithManyToMany,
     State,
 )
 
@@ -122,6 +124,75 @@ def test_history_list_contains_diff_changes(self):
         self.assertContains(response, "2021-01-01 10:00:00")
         self.assertContains(response, "2024-04-04 04:04:04")
 
+    def test_history_list_contains_diff_changes_for_foreign_key_fields(self):
+        self.login()
+        poll1 = Poll.objects.create(question="why?", pub_date=today)
+        poll2 = Poll.objects.create(question="how?", pub_date=today)
+        # Assert that the PKs have the values used in the assertions below
+        self.assertListEqual([poll1.pk, poll2.pk], [1, 2])
+        choice = Choice(poll=poll1, votes=1)
+        choice._history_user = self.user
+        choice.save()
+        choice_history_url = get_history_url(choice)
+
+        # Before changing the poll:
+        response = self.client.get(choice_history_url)
+        self.assertNotContains(response, "Poll:")
+        expected_old_poll = "Poll object (1)"
+        self.assertNotContains(response, expected_old_poll)
+        expected_new_poll = "Poll object (2)"
+        self.assertNotContains(response, expected_new_poll)
+
+        # After changing the poll:
+        choice.poll = poll2
+        choice.save()
+        response = self.client.get(choice_history_url)
+        self.assertContains(response, "Poll:")
+        self.assertContains(response, expected_old_poll)
+        self.assertContains(response, expected_new_poll)
+
+        # After deleting all polls:
+        Poll.objects.all().delete()
+        response = self.client.get(choice_history_url)
+        self.assertContains(response, "Poll:")
+        self.assertContains(response, "DeletedObject(pk=1)")
+        self.assertContains(response, "DeletedObject(pk=2)")
+
+    def test_history_list_contains_diff_changes_for_m2m_fields(self):
+        self.login()
+        poll = PollWithManyToMany(question="why?", pub_date=today)
+        poll._history_user = self.user
+        poll.save()
+        place1 = Place.objects.create(name="Here")
+        place2 = Place.objects.create(name="There")
+        # Assert that the PKs have the values used in the assertions below
+        self.assertListEqual([place1.pk, place2.pk], [1, 2])
+        poll_history_url = get_history_url(poll)
+
+        # Before adding places:
+        response = self.client.get(poll_history_url)
+        self.assertNotContains(response, "Places:")
+        expected_old_places = "[]"
+        self.assertNotContains(response, expected_old_places)
+        expected_new_places = (
+            "[<Place: Place object (1)>, <Place: Place object (2)>]"
+        )
+        self.assertNotContains(response, expected_new_places)
+
+        # After adding places:
+        poll.places.add(place1, place2)
+        response = self.client.get(poll_history_url)
+        self.assertContains(response, "Places:")
+        self.assertContains(response, expected_old_places)
+        self.assertContains(response, expected_new_places)
+
+        # After deleting all places:
+        Place.objects.all().delete()
+        response = self.client.get(poll_history_url)
+        self.assertContains(response, "Places:")
+        self.assertContains(response, expected_old_places)
+        self.assertContains(response, "[DeletedObject(pk=1), DeletedObject(pk=2)]")
+
     def test_history_list_doesnt_contain_too_long_diff_changes(self):
         self.login()
         repeated_chars = Poll._meta.get_field("question").max_length - 1