Access foreign key of historic instance from model methods

[akhayyat]

Problem Statement

When accessing the related object of a historic instance's foreign key from a model method such as __str__(), the related object is looked up in the main table, not the history, potentially resulting in a DoesNotExist exception if that related object had been deleted.

Example:

models.py:

from django.db import models
from simple_history.models import HistoricalRecords, HistoricForeignKey

class Person(models.Model):
    name = models.CharField(max_length=200)
    history = HistoricalRecords(related_name="historical_records")

    def __str__(self):
        return self.name

class Car(models.Model):
    model = models.CharField(max_length=200)
    owner = HistoricForeignKey(Person, on_delete=models.PROTECT, related_name="cars")
    history = HistoricalRecords(related_name="historical_records")

    def __str__(self):
        return f"{self.model} ({self.owner})"

./manage.py shell:

>>> from cars.models import Person, Car
>>> p1=Person.objects.create(name="First Person")
>>> p2=Person.objects.create(name="Second Person")
>>> c = Car.objects.create(model="Explorer", owner=p1)
>>> c.history.all()
<HistoricalQuerySet [<HistoricalCar: Explorer (First Person) as of 2024-03-08 14:55:35.895298+00:00>]>
>>> c.owner=p2
>>> c.save()
>>> p1.delete()
(1, {'cars.Person': 1})
>>> c.history.all()
Traceback (most recent call last):
  File ".../test-django-history/.venv/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 236, in __get__
    rel_obj = self.field.get_cached_value(instance)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../test-django-history/.venv/lib/python3.11/site-packages/django/db/models/fields/mixins.py", line 15, in get_cached_value
    return instance._state.fields_cache[cache_name]
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^
KeyError: 'owner'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File ".../test-django-history/.venv/lib/python3.11/site-packages/django/db/models/query.py", line 379, in __repr__
    return "<%s %r>" % (self.__class__.__name__, data)
                                                 ^^^^
  File ".../test-django-history/.venv/lib/python3.11/site-packages/django/db/models/base.py", line 588, in __repr__
    return "<%s: %s>" % (self.__class__.__name__, self)
                                                  ^^^^
  File ".../test-django-history/.venv/lib/python3.11/site-packages/simple_history/models.py", line 562, in <lambda>
    "__str__": lambda self: "{} as of {}".format(
                            ^^^^^^^^^^^^^^^^^^^^^
  File ".../test-django-history/test_django_history/cars/models.py", line 19, in __str__
    return f"{self.model} ({self.owner})"
                            ^^^^^^^^^^
  File ".../test-django-history/.venv/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 254, in __get__
    rel_obj = self.get_object(instance)
              ^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../test-django-history/.venv/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 217, in get_object
    return qs.get(self.field.get_reverse_related_filter(instance))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../test-django-history/.venv/lib/python3.11/site-packages/django/db/models/query.py", line 649, in get
    raise self.model.DoesNotExist(
cars.models.Person.DoesNotExist: Person matching query does not exist.

The culprit here is the {self.owner} in Car.__str__(). If removed, no errors occur.

Note that this affects the admin as well. Browsing the history of this instance shows the same error, and the entire history becomes inaccessible from the admin ui.

Describe the solution you'd like

Being able to access historic instances of related objects when accessed through historic instance of the main model, to avoid DoesNotExist errors.

If that can be achieved implicitly, i.e. without any changes to the code above, that'd be great. But an explicit solution that would require modifying the code above would be OK, too. Right now, I can't tell from within the __str__() method whether the instance is current or retrieved from history, and if the latter, what's the history date.

Describe alternatives you've considered

Explicitly adding code that solves the problem. However, it's unclear to me what such code would be either.

[ddabble]

[Duplicates #521 and #533]

Thank you for reporting this! I'm planning on changing the implementation of the historical models' __str__() so that it returns a more generic string if a DoesNotExist error is raised.

In the meantime, however, could you try implementing one of the two methods detailed below? (This is from a draft change to the docs)

Overriding Methods of Historical Models

It's not possible to directly override methods of the auto-generated historical models, as they will always subclass any model passed as the bases argument. However, by subclassing HistoricalRecords and changing the return value of get_extra_fields() (and get_extra_fields_m2m() if you're history-tracking many-to-many fields), you can achieve the same result.

For example, the following code demonstrates two ways of overriding the historical model's __str__() method:

# Both methods below use the following model:
class Choice(models.Model):
    poll = models.ForeignKey(Poll, on_delete=models.CASCADE)
    choice_text = models.CharField(max_length=200)
    votes = models.IntegerField(default=0)

    def __str__(self):
        return f"{self.votes} votes for {self.choice_text} on {self.poll}"

    @staticmethod
    def get_historical_str(historical_choice):
        try:
            poll_str = str(historical_choice.poll)
        except Poll.DoesNotExist:
            poll_str = f"[deleted poll #{historical_choice.poll_id}]"
        return (
            f"{historical_choice.votes} votes for {historical_choice.choice_text}"
            f" on {poll_str} (saved: {historical_choice.history_date})"
        )


# --- Method #1, with the __str__() implementation in a custom historical model ---

class HistoricalRecordsWithoutStr(HistoricalRecords):
    # Do the same for `get_extra_fields_m2m()`, if history-tracking M2M fields
    # and you want to implement a custom `__str__()` method for
    # the historical through model objects.
    # (This would require creating a custom historical model for the
    # M2M through model and passing it as the `m2m_bases` argument to
    # `HistoricalRecordsWithoutStr` below.)
    def get_extra_fields(self, *args, **kwargs):
        extra_fields = super().get_extra_fields(*args, **kwargs)
        del extra_fields["__str__"]
        return extra_fields


class HistoricalChoice(models.Model):
    class Meta:
        abstract = True

    def __str__(self):
        return Choice.get_historical_str(self)


class Choice(models.Model):
    # ...
    history = HistoricalRecordsWithoutStr(bases=[HistoricalChoice])
    # ...


# --- Method #2, with the __str__() implementation in a custom HistoricalRecords ---

class HistoricalRecordsWithCustomChoiceStr(HistoricalRecords):
    # Do the same for `get_extra_fields_m2m()`, if history-tracking M2M fields
    # and you want to implement a custom `__str__()` method for
    # the historical through model objects.
    def get_extra_fields(self, *args, **kwargs):
        extra_fields = super().get_extra_fields(*args, **kwargs)
        extra_fields["__str__"] = lambda self: Choice.get_historical_str(self)
        return extra_fields


class Choice(models.Model):
    # ...
    history = HistoricalRecordsWithCustomChoiceStr()
    # ...