diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 01f36a5e8..9cb99a317 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,8 +4,7 @@ repos: rev: 1.7.8 hooks: - id: bandit - args: - - "-x *test*.py" + exclude: /.*tests/ - repo: https://github.com/psf/black-pre-commit-mirror rev: 24.3.0 diff --git a/CHANGES.rst b/CHANGES.rst index 653c6951e..9f06150d7 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,6 +5,26 @@ Unreleased ---------- - Support custom History ``Manager`` and ``QuerySet`` classes (gh-1280) +- Renamed the (previously internal) admin template + ``simple_history/_object_history_list.html`` to + ``simple_history/object_history_list.html``, and added the field + ``SimpleHistoryAdmin.object_history_list_template`` for overriding it (gh-1128) +- Deprecated the undocumented template tag ``simple_history_admin_list.display_list()``; + it will be removed in version 3.8 (gh-1128) +- Added ``SimpleHistoryAdmin.get_history_queryset()`` for overriding which ``QuerySet`` + is used to list the historical records (gh-1128) +- Added ``SimpleHistoryAdmin.get_history_list_display()`` which returns + ``history_list_display`` by default, and made the latter into an actual field (gh-1128) +- ``ModelDelta`` and ``ModelChange`` (in ``simple_history.models``) are now immutable + dataclasses; their signatures remain unchanged (gh-1128) +- ``ModelDelta``'s ``changes`` and ``changed_fields`` are now sorted alphabetically by + field name. Also, if ``ModelChange`` is for an M2M field, its ``old`` and ``new`` + lists are sorted by the related object. This should help prevent flaky tests. (gh-1128) +- ``diff_against()`` has a new keyword argument, ``foreign_keys_are_objs``; + see usage in the docs under "History Diffing" (gh-1128) +- Added a "Changes" column to ``SimpleHistoryAdmin``'s object history table, listing + the changes between each historical record of the object; see the docs under + "Customizing the History Admin Templates" for overriding its template context (gh-1128) 3.5.0 (2024-02-19) ------------------ diff --git a/docs/admin.rst b/docs/admin.rst index 1b34f92c4..63329d110 100644 --- a/docs/admin.rst +++ b/docs/admin.rst @@ -69,6 +69,43 @@ admin class .. image:: screens/5_history_list_display.png + +Customizing the History Admin Templates +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +If you'd like to customize the HTML of ``SimpleHistoryAdmin``'s object history pages, +you can override the following attributes with the names of your own templates: + +- ``object_history_template``: The main object history page, which includes (inserts) + ``object_history_list_template``. +- ``object_history_list_template``: The table listing an object's historical records and + the changes made between them. +- ``object_history_form_template``: The form pre-filled with the details of an object's + historical record, which also allows you to revert the object to a previous version. + +If you'd like to only customize certain parts of the mentioned templates, look for +``block`` template tags in the source code that you can override - like the +``history_delta_changes`` block in ``simple_history/object_history_list.html``, +which lists the changes made between each historical record. + +Customizing Context +^^^^^^^^^^^^^^^^^^^ + +You can also customize the template context by overriding the following methods: + +- ``render_history_view()``: Called by both ``history_view()`` and + ``history_form_view()`` before the templates are rendered. Customize the context by + changing the ``context`` parameter. +- ``history_view()``: Returns a rendered ``object_history_template``. + Inject context by calling the super method with the ``extra_context`` argument. +- ``get_historical_record_context_helper()``: Returns an instance of + ``simple_history.template_utils.HistoricalRecordContextHelper`` that's used to format + some template context for each historical record displayed through ``history_view()``. + Customize the context by extending the mentioned class and overriding its methods. +- ``history_form_view()``: Returns a rendered ``object_history_form_template``. + Inject context by calling the super method with the ``extra_context`` argument. + + Disabling the option to revert an object ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/historical_model.rst b/docs/historical_model.rst index fbf931c4f..32447f185 100644 --- a/docs/historical_model.rst +++ b/docs/historical_model.rst @@ -554,7 +554,6 @@ You will see the many to many changes when diffing between two historical record informal = Category.objects.create(name="informal questions") official = Category.objects.create(name="official questions") p = Poll.objects.create(question="what's up?") - p.save() p.categories.add(informal, official) p.categories.remove(informal) diff --git a/docs/history_diffing.rst b/docs/history_diffing.rst index 3e409068c..7887c61ff 100644 --- a/docs/history_diffing.rst +++ b/docs/history_diffing.rst @@ -1,24 +1,110 @@ History Diffing =============== -When you have two instances of the same ``HistoricalRecord`` (such as the ``HistoricalPoll`` example above), -you can perform diffs to see what changed. This will result in a ``ModelDelta`` containing the following properties: +When you have two instances of the same historical model +(such as the ``HistoricalPoll`` example above), +you can perform a diff using the ``diff_against()`` method to see what changed. +This will return a ``ModelDelta`` object with the following attributes: -1. A list with each field changed between the two historical records -2. A list with the names of all fields that incurred changes from one record to the other -3. the old and new records. +- ``old_record`` and ``new_record``: The old and new history records +- ``changed_fields``: A list of the names of all fields that were changed between + ``old_record`` and ``new_record``, in alphabetical order +- ``changes``: A list of ``ModelChange`` objects - one for each field in + ``changed_fields``, in the same order. + These objects have the following attributes: -This may be useful when you want to construct timelines and need to get only the model modifications. + - ``field``: The name of the changed field + (this name is equal to the corresponding field in ``changed_fields``) + - ``old`` and ``new``: The old and new values of the changed field + + - For many-to-many fields, these values will be lists of dicts from the through + model field names to the primary keys of the through model's related objects. + The lists are sorted by the value of the many-to-many related object. + +This may be useful when you want to construct timelines and need to get only +the model modifications. .. code-block:: python - p = Poll.objects.create(question="what's up?") - p.question = "what's up, man?" - p.save() + poll = Poll.objects.create(question="what's up?") + poll.question = "what's up, man?" + poll.save() - new_record, old_record = p.history.all() + new_record, old_record = poll.history.all() delta = new_record.diff_against(old_record) for change in delta.changes: - print("{} changed from {} to {}".format(change.field, change.old, change.new)) + print(f"'{change.field}' changed from '{change.old}' to '{change.new}'") + + # Output: + # 'question' changed from 'what's up?' to 'what's up, man?' + +``diff_against()`` also accepts the following additional arguments: + +- ``excluded_fields`` and ``included_fields``: These can be used to either explicitly + exclude or include fields from being diffed, respectively. +- ``foreign_keys_are_objs``: + + - If ``False`` (default): The diff will only contain the raw primary keys of any + ``ForeignKey`` fields. + - If ``True``: The diff will contain the actual related model objects instead of just + the primary keys. + Deleted related objects (both foreign key objects and many-to-many objects) + will be instances of ``DeletedObject``, which only contain a ``model`` field with a + reference to the deleted object's model, as well as a ``pk`` field with the value of + the deleted object's primary key. + + Note that this will add extra database queries for each related field that's been + changed - as long as the related objects have not been prefetched + (using e.g. ``select_related()``). + + A couple examples showing the difference: + + .. code-block:: python + + # --- Effect on foreign key fields --- + + whats_up = Poll.objects.create(pk=15, name="what's up?") + still_around = Poll.objects.create(pk=31, name="still around?") + + choice = Choice.objects.create(poll=whats_up) + choice.poll = still_around + choice.save() + + new, old = choice.history.all() + + default_delta = new.diff_against(old) + # Printing the changes of `default_delta` will output: + # 'poll' changed from '15' to '31' + + delta_with_objs = new.diff_against(old, foreign_keys_are_objs=True) + # Printing the changes of `delta_with_objs` will output: + # 'poll' changed from 'what's up?' to 'still around?' + + # Deleting all the polls: + Poll.objects.all().delete() + delta_with_objs = new.diff_against(old, foreign_keys_are_objs=True) + # Printing the changes of `delta_with_objs` will now output: + # 'poll' changed from 'Deleted poll (pk=15)' to 'Deleted poll (pk=31)' + + + # --- Effect on many-to-many fields --- + + informal = Category.objects.create(pk=63, name="informal questions") + whats_up.categories.add(informal) + + new = whats_up.history.latest() + old = new.prev_record + + default_delta = new.diff_against(old) + # Printing the changes of `default_delta` will output: + # 'categories' changed from [] to [{'poll': 15, 'category': 63}] + + delta_with_objs = new.diff_against(old, foreign_keys_are_objs=True) + # Printing the changes of `delta_with_objs` will output: + # 'categories' changed from [] to [{'poll': , 'category': }] -``diff_against`` also accepts 2 arguments ``excluded_fields`` and ``included_fields`` to either explicitly include or exclude fields from being diffed. + # Deleting all the categories: + Category.objects.all().delete() + delta_with_objs = new.diff_against(old, foreign_keys_are_objs=True) + # Printing the changes of `delta_with_objs` will now output: + # 'categories' changed from [] to [{'poll': , 'category': DeletedObject(model=, pk=63)}] diff --git a/docs/screens/10_revert_disabled.png b/docs/screens/10_revert_disabled.png index 851fcfe00..e5ab87358 100644 Binary files a/docs/screens/10_revert_disabled.png and b/docs/screens/10_revert_disabled.png differ diff --git a/docs/screens/1_poll_history.png b/docs/screens/1_poll_history.png index e2f1b2413..713352f5e 100644 Binary files a/docs/screens/1_poll_history.png and b/docs/screens/1_poll_history.png differ diff --git a/docs/screens/2_revert.png b/docs/screens/2_revert.png index 3cba44148..c9a52546b 100644 Binary files a/docs/screens/2_revert.png and b/docs/screens/2_revert.png differ diff --git a/docs/screens/3_poll_reverted.png b/docs/screens/3_poll_reverted.png index 6b7f3e6f3..802bcab4e 100644 Binary files a/docs/screens/3_poll_reverted.png and b/docs/screens/3_poll_reverted.png differ diff --git a/docs/screens/4_history_after_poll_reverted.png b/docs/screens/4_history_after_poll_reverted.png index b49c75b58..264e8bb07 100644 Binary files a/docs/screens/4_history_after_poll_reverted.png and b/docs/screens/4_history_after_poll_reverted.png differ diff --git a/docs/screens/5_history_list_display.png b/docs/screens/5_history_list_display.png index 6b1f03396..fc07c5680 100644 Binary files a/docs/screens/5_history_list_display.png and b/docs/screens/5_history_list_display.png differ diff --git a/simple_history/admin.py b/simple_history/admin.py index 34e3033fe..6a55e2e1c 100644 --- a/simple_history/admin.py +++ b/simple_history/admin.py @@ -1,3 +1,5 @@ +from typing import Any, Sequence + from django import http from django.apps import apps as django_apps from django.conf import settings @@ -6,6 +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.shortcuts import get_object_or_404, render from django.urls import re_path, reverse from django.utils.encoding import force_str @@ -13,13 +16,19 @@ from django.utils.text import capfirst from django.utils.translation import gettext as _ +from .manager import HistoricalQuerySet, HistoryManager +from .models import HistoricalChanges +from .template_utils import HistoricalRecordContextHelper from .utils import get_history_manager_for_model, get_history_model_for_model SIMPLE_HISTORY_EDIT = getattr(settings, "SIMPLE_HISTORY_EDIT", False) class SimpleHistoryAdmin(admin.ModelAdmin): + history_list_display = [] + object_history_template = "simple_history/object_history.html" + object_history_list_template = "simple_history/object_history_list.html" object_history_form_template = "simple_history/object_history_form.html" def get_urls(self): @@ -46,41 +55,43 @@ def history_view(self, request, object_id, extra_context=None): pk_name = opts.pk.attname history = getattr(model, model._meta.simple_history_manager_attribute) object_id = unquote(object_id) - action_list = history.filter(**{pk_name: object_id}) - if not isinstance(history.model.history_user, property): - # Only select_related when history_user is a ForeignKey (not a property) - action_list = action_list.select_related("history_user") - history_list_display = getattr(self, "history_list_display", []) + historical_records = self.get_history_queryset( + request, history, pk_name, object_id + ) + history_list_display = self.get_history_list_display(request) # If no history was found, see whether this object even exists. try: obj = self.get_queryset(request).get(**{pk_name: object_id}) except model.DoesNotExist: try: - obj = action_list.latest("history_date").instance - except action_list.model.DoesNotExist: + obj = historical_records.latest("history_date").instance + except historical_records.model.DoesNotExist: raise http.Http404 if not self.has_view_history_or_change_history_permission(request, obj): raise PermissionDenied - # Set attribute on each action_list entry from admin methods + # Set attribute on each historical record from admin methods for history_list_entry in history_list_display: value_for_entry = getattr(self, history_list_entry, None) if value_for_entry and callable(value_for_entry): - for list_entry in action_list: - setattr(list_entry, history_list_entry, value_for_entry(list_entry)) + for record in historical_records: + setattr(record, history_list_entry, value_for_entry(record)) + + self.set_history_delta_changes(request, historical_records) content_type = self.content_type_model_cls.objects.get_for_model( get_user_model() ) - admin_user_view = "admin:{}_{}_change".format( content_type.app_label, content_type.model, ) + context = { "title": self.history_view_title(request, obj), - "action_list": action_list, + "object_history_list_template": self.object_history_list_template, + "historical_records": historical_records, "module_name": capfirst(force_str(opts.verbose_name_plural)), "object": obj, "root_path": getattr(self.admin_site, "root_path", None), @@ -97,6 +108,73 @@ def history_view(self, request, object_id, extra_context=None): request, self.object_history_template, context, **extra_kwargs ) + def get_history_queryset( + self, request, history_manager: HistoryManager, pk_name: str, object_id: Any + ) -> QuerySet: + """ + Return a ``QuerySet`` of all historical records that should be listed in the + ``object_history_list_template`` template. + This is used by ``history_view()``. + + :param request: + :param history_manager: + :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: 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]: + """ + Return a sequence containing the names of additional fields to be displayed on + the object history page. These can either be fields or properties on the model + or the history model, or methods on the admin class. + """ + return self.history_list_display + + def get_historical_record_context_helper( + self, request, historical_record: HistoricalChanges + ) -> HistoricalRecordContextHelper: + """ + Return an instance of ``HistoricalRecordContextHelper`` for formatting + the template context for ``historical_record``. + """ + return HistoricalRecordContextHelper(self.model, historical_record) + + def set_history_delta_changes( + self, + request, + historical_records: Sequence[HistoricalChanges], + foreign_keys_are_objs=True, + ): + """ + Add a ``history_delta_changes`` attribute to all historical records + except the first (oldest) one. + + :param request: + :param historical_records: + :param foreign_keys_are_objs: Passed to ``diff_against()`` when calculating + the deltas; see its docstring for details. + """ + previous = None + for current in historical_records: + if previous is None: + previous = current + continue + # Related objects should have been prefetched in `get_history_queryset()` + delta = previous.diff_against( + current, foreign_keys_are_objs=foreign_keys_are_objs + ) + helper = self.get_historical_record_context_helper(request, previous) + previous.history_delta_changes = helper.context_for_delta_changes(delta) + + previous = current + def history_view_title(self, request, obj): if self.revert_disabled(request, obj) and not SIMPLE_HISTORY_EDIT: return _("View history: %s") % force_str(obj) diff --git a/simple_history/locale/ar/LC_MESSAGES/django.po b/simple_history/locale/ar/LC_MESSAGES/django.po index 109e825e4..c7234735e 100644 --- a/simple_history/locale/ar/LC_MESSAGES/django.po +++ b/simple_history/locale/ar/LC_MESSAGES/django.po @@ -60,30 +60,6 @@ msgstr "تغيير" msgid "Deleted" msgstr "تمت إزالته" -#: simple_history/templates/simple_history/_object_history_list.html:9 -msgid "Object" -msgstr "عنصر" - -#: simple_history/templates/simple_history/_object_history_list.html:13 -msgid "Date/time" -msgstr "التاريخ/الوقت" - -#: simple_history/templates/simple_history/_object_history_list.html:14 -msgid "Comment" -msgstr "تعليق" - -#: simple_history/templates/simple_history/_object_history_list.html:15 -msgid "Changed by" -msgstr "تغير من قبل" - -#: simple_history/templates/simple_history/_object_history_list.html:16 -msgid "Change reason" -msgstr "سبب التغير" - -#: simple_history/templates/simple_history/_object_history_list.html:37 -msgid "None" -msgstr "فارغ" - #: simple_history/templates/simple_history/object_history.html:11 msgid "" "Choose a date from the list below to revert to a previous version of this " @@ -121,6 +97,30 @@ msgstr "اضغط على زر 'استرجاع' ادناه للاسترجاع له msgid "Press the 'Change History' button below to edit the history." msgstr "اضغط على زر 'تعديل سجل التغيرات' ادناه لتعديل التاريخ." +#: simple_history/templates/simple_history/object_history_list.html:9 +msgid "Object" +msgstr "عنصر" + +#: simple_history/templates/simple_history/object_history_list.html:13 +msgid "Date/time" +msgstr "التاريخ/الوقت" + +#: simple_history/templates/simple_history/object_history_list.html:14 +msgid "Comment" +msgstr "تعليق" + +#: simple_history/templates/simple_history/object_history_list.html:15 +msgid "Changed by" +msgstr "تغير من قبل" + +#: simple_history/templates/simple_history/object_history_list.html:16 +msgid "Change reason" +msgstr "سبب التغير" + +#: simple_history/templates/simple_history/object_history_list.html:42 +msgid "None" +msgstr "فارغ" + #: simple_history/templates/simple_history/submit_line.html:4 msgid "Revert" msgstr "استرجاع" diff --git a/simple_history/locale/cs_CZ/LC_MESSAGES/django.po b/simple_history/locale/cs_CZ/LC_MESSAGES/django.po index 0f8dd76e6..aa8dabca6 100644 --- a/simple_history/locale/cs_CZ/LC_MESSAGES/django.po +++ b/simple_history/locale/cs_CZ/LC_MESSAGES/django.po @@ -59,30 +59,6 @@ msgstr "Změněno" msgid "Deleted" msgstr "Smazáno" -#: simple_history/templates/simple_history/_object_history_list.html:9 -msgid "Object" -msgstr "Objekt" - -#: simple_history/templates/simple_history/_object_history_list.html:13 -msgid "Date/time" -msgstr "Datum/čas" - -#: simple_history/templates/simple_history/_object_history_list.html:14 -msgid "Comment" -msgstr "Komentář" - -#: simple_history/templates/simple_history/_object_history_list.html:15 -msgid "Changed by" -msgstr "Změnil" - -#: simple_history/templates/simple_history/_object_history_list.html:16 -msgid "Change reason" -msgstr "Důvod změny" - -#: simple_history/templates/simple_history/_object_history_list.html:37 -msgid "None" -msgstr "Žádné" - #: simple_history/templates/simple_history/object_history.html:11 msgid "" "Choose a date from the list below to revert to a previous version of this " @@ -121,6 +97,30 @@ msgstr "Stisknutím tlačítka 'Vrátit změny' se vrátíte k této verzi objek msgid "Press the 'Change History' button below to edit the history." msgstr "Chcete-li historii upravit, stiskněte tlačítko 'Změnit historii'" +#: simple_history/templates/simple_history/object_history_list.html:9 +msgid "Object" +msgstr "Objekt" + +#: simple_history/templates/simple_history/object_history_list.html:13 +msgid "Date/time" +msgstr "Datum/čas" + +#: simple_history/templates/simple_history/object_history_list.html:14 +msgid "Comment" +msgstr "Komentář" + +#: simple_history/templates/simple_history/object_history_list.html:15 +msgid "Changed by" +msgstr "Změnil" + +#: simple_history/templates/simple_history/object_history_list.html:16 +msgid "Change reason" +msgstr "Důvod změny" + +#: simple_history/templates/simple_history/object_history_list.html:42 +msgid "None" +msgstr "Žádné" + #: simple_history/templates/simple_history/submit_line.html:4 msgid "Revert" msgstr "Vrátit změny" diff --git a/simple_history/locale/de/LC_MESSAGES/django.po b/simple_history/locale/de/LC_MESSAGES/django.po index 37ac7542e..e38d45689 100644 --- a/simple_history/locale/de/LC_MESSAGES/django.po +++ b/simple_history/locale/de/LC_MESSAGES/django.po @@ -48,30 +48,6 @@ msgstr "Geändert" msgid "Deleted" msgstr "Gelöscht" -#: simple_history/templates/simple_history/_object_history_list.html:9 -msgid "Object" -msgstr "Objekt" - -#: simple_history/templates/simple_history/_object_history_list.html:13 -msgid "Date/time" -msgstr "Datum/Uhrzeit" - -#: simple_history/templates/simple_history/_object_history_list.html:14 -msgid "Comment" -msgstr "Kommentar" - -#: simple_history/templates/simple_history/_object_history_list.html:15 -msgid "Changed by" -msgstr "Geändert von" - -#: simple_history/templates/simple_history/_object_history_list.html:16 -msgid "Change reason" -msgstr "Änderungsgrund" - -#: simple_history/templates/simple_history/_object_history_list.html:37 -msgid "None" -msgstr "Keine/r" - #: simple_history/templates/simple_history/object_history.html:11 msgid "" "Choose a date from the list below to revert to a previous version of this " @@ -108,6 +84,30 @@ msgstr "" msgid "Or press the 'Change History' button to edit the history." msgstr "Oder wählen Sie 'Historie ändern', um diese zu bearbeiten." +#: simple_history/templates/simple_history/object_history_list.html:9 +msgid "Object" +msgstr "Objekt" + +#: simple_history/templates/simple_history/object_history_list.html:13 +msgid "Date/time" +msgstr "Datum/Uhrzeit" + +#: simple_history/templates/simple_history/object_history_list.html:14 +msgid "Comment" +msgstr "Kommentar" + +#: simple_history/templates/simple_history/object_history_list.html:15 +msgid "Changed by" +msgstr "Geändert von" + +#: simple_history/templates/simple_history/object_history_list.html:16 +msgid "Change reason" +msgstr "Änderungsgrund" + +#: simple_history/templates/simple_history/object_history_list.html:42 +msgid "None" +msgstr "Keine/r" + #: simple_history/templates/simple_history/submit_line.html:3 msgid "Revert" msgstr "Wiederherstellen" diff --git a/simple_history/locale/fr/LC_MESSAGES/django.po b/simple_history/locale/fr/LC_MESSAGES/django.po index 883afbad8..6528938c8 100644 --- a/simple_history/locale/fr/LC_MESSAGES/django.po +++ b/simple_history/locale/fr/LC_MESSAGES/django.po @@ -59,30 +59,6 @@ msgstr "Modifié" msgid "Deleted" msgstr "Effacé" -#: .\simple_history\templates\simple_history\_object_history_list.html:9 -msgid "Object" -msgstr "Objet" - -#: .\simple_history\templates\simple_history\_object_history_list.html:13 -msgid "Date/time" -msgstr "Date/heure" - -#: .\simple_history\templates\simple_history\_object_history_list.html:14 -msgid "Comment" -msgstr "Commentaire" - -#: .\simple_history\templates\simple_history\_object_history_list.html:15 -msgid "Changed by" -msgstr "Modifié par" - -#: .\simple_history\templates\simple_history\_object_history_list.html:16 -msgid "Change reason" -msgstr "Raison de la modification" - -#: .\simple_history\templates\simple_history\_object_history_list.html:37 -msgid "None" -msgstr "Aucun" - #: .\simple_history\templates\simple_history\object_history.html:11 msgid "" "Choose a date from the list below to revert to a previous version of this " @@ -125,6 +101,30 @@ msgid "Press the 'Change History' button below to edit the history." msgstr "" "Cliquez sur le bouton 'Historique' ci-dessous pour modifier l'historique." +#: .\simple_history\templates\simple_history\object_history_list.html:9 +msgid "Object" +msgstr "Objet" + +#: .\simple_history\templates\simple_history\object_history_list.html:13 +msgid "Date/time" +msgstr "Date/heure" + +#: .\simple_history\templates\simple_history\object_history_list.html:14 +msgid "Comment" +msgstr "Commentaire" + +#: .\simple_history\templates\simple_history\object_history_list.html:15 +msgid "Changed by" +msgstr "Modifié par" + +#: .\simple_history\templates\simple_history\object_history_list.html:16 +msgid "Change reason" +msgstr "Raison de la modification" + +#: .\simple_history\templates\simple_history\object_history_list.html:42 +msgid "None" +msgstr "Aucun" + #: .\simple_history\templates\simple_history\submit_line.html:4 msgid "Revert" msgstr "Rétablir" diff --git a/simple_history/locale/id/LC_MESSAGES/django.po b/simple_history/locale/id/LC_MESSAGES/django.po index 86e9e2848..b649d6b39 100644 --- a/simple_history/locale/id/LC_MESSAGES/django.po +++ b/simple_history/locale/id/LC_MESSAGES/django.po @@ -58,30 +58,6 @@ msgstr "Diubah" msgid "Deleted" msgstr "Dihapus" -#: .\simple_history\templates\simple_history\_object_history_list.html:9 -msgid "Object" -msgstr "Objek" - -#: .\simple_history\templates\simple_history\_object_history_list.html:13 -msgid "Date/time" -msgstr "Tanggal/waktu" - -#: .\simple_history\templates\simple_history\_object_history_list.html:14 -msgid "Comment" -msgstr "Komentar" - -#: .\simple_history\templates\simple_history\_object_history_list.html:15 -msgid "Changed by" -msgstr "Diubah oleh" - -#: .\simple_history\templates\simple_history\_object_history_list.html:16 -msgid "Change reason" -msgstr "Alasan perubahan" - -#: .\simple_history\templates\simple_history\_object_history_list.html:37 -msgid "None" -msgstr "Tidak ada" - #: .\simple_history\templates\simple_history\object_history.html:11 msgid "" "Choose a date from the list below to revert to a previous version of this " @@ -122,6 +98,30 @@ msgstr "" msgid "Press the 'Change History' button below to edit the history." msgstr "Tekan tombol 'Ubah Riwayat' di bawah ini untuk mengubah riwayat." +#: .\simple_history\templates\simple_history\object_history_list.html:9 +msgid "Object" +msgstr "Objek" + +#: .\simple_history\templates\simple_history\object_history_list.html:13 +msgid "Date/time" +msgstr "Tanggal/waktu" + +#: .\simple_history\templates\simple_history\object_history_list.html:14 +msgid "Comment" +msgstr "Komentar" + +#: .\simple_history\templates\simple_history\object_history_list.html:15 +msgid "Changed by" +msgstr "Diubah oleh" + +#: .\simple_history\templates\simple_history\object_history_list.html:16 +msgid "Change reason" +msgstr "Alasan perubahan" + +#: .\simple_history\templates\simple_history\object_history_list.html:42 +msgid "None" +msgstr "Tidak ada" + #: .\simple_history\templates\simple_history\submit_line.html:4 msgid "Revert" msgstr "Kembalikan" diff --git a/simple_history/locale/nb/LC_MESSAGES/django.mo b/simple_history/locale/nb/LC_MESSAGES/django.mo index 89ba69d3c..594a7b899 100644 Binary files a/simple_history/locale/nb/LC_MESSAGES/django.mo and b/simple_history/locale/nb/LC_MESSAGES/django.mo differ diff --git a/simple_history/locale/nb/LC_MESSAGES/django.po b/simple_history/locale/nb/LC_MESSAGES/django.po index e2167f858..334f336b2 100644 --- a/simple_history/locale/nb/LC_MESSAGES/django.po +++ b/simple_history/locale/nb/LC_MESSAGES/django.po @@ -8,7 +8,7 @@ msgstr "" "Project-Id-Version: django-simple-history\n" "Report-Msgid-Bugs-To: \n" "POT-Creation-Date: 2023-07-09 13:55+0200\n" -"PO-Revision-Date: 2023-07-09 12:31+0200\n" +"PO-Revision-Date: 2024-04-11 19:34+0200\n" "Last-Translator: Anders \n" "Language-Team: Norwegian Bokmål \n" "Language: nb\n" @@ -19,31 +19,31 @@ msgstr "" # Dette er en tittel, ikke en handlingsbeskrivelse, så f.eks. # "Se/Vis (endrings)historikk" hadde ikke fungert så bra -#: simple_history/admin.py:102 +#: simple_history/admin.py:109 #, python-format msgid "View history: %s" msgstr "Endringshistorikk: %s" -#: simple_history/admin.py:104 +#: simple_history/admin.py:111 #, python-format msgid "Change history: %s" msgstr "Endringshistorikk: %s" -#: simple_history/admin.py:110 +#: simple_history/admin.py:117 #, python-format msgid "The %(name)s \"%(obj)s\" was changed successfully." msgstr "%(name)s «%(obj)s» ble endret." -#: simple_history/admin.py:116 +#: simple_history/admin.py:123 msgid "You may edit it again below" msgstr "Du kan redigere videre nedenfor" -#: simple_history/admin.py:217 +#: simple_history/admin.py:224 #, python-format msgid "View %s" msgstr "Se %s" -#: simple_history/admin.py:219 +#: simple_history/admin.py:226 #, python-format msgid "Revert %s" msgstr "Tilbakestill %s" @@ -60,29 +60,10 @@ msgstr "Endret" msgid "Deleted" msgstr "Slettet" -#: simple_history/templates/simple_history/_object_history_list.html:9 -msgid "Object" -msgstr "Objekt" - -#: simple_history/templates/simple_history/_object_history_list.html:13 -msgid "Date/time" -msgstr "Dato/tid" - -#: simple_history/templates/simple_history/_object_history_list.html:14 -msgid "Comment" -msgstr "Kommentar" - -#: simple_history/templates/simple_history/_object_history_list.html:15 -msgid "Changed by" -msgstr "Endret av" - -#: simple_history/templates/simple_history/_object_history_list.html:16 -msgid "Change reason" -msgstr "Endringsårsak" - -#: simple_history/templates/simple_history/_object_history_list.html:37 -msgid "None" -msgstr "Ingen" +#: simple_history/models.py:1124 +#, python-format +msgid "Deleted %(type_name)s" +msgstr "Slettet %(type_name)s" #: simple_history/templates/simple_history/object_history.html:11 msgid "" @@ -125,6 +106,34 @@ msgstr "" msgid "Press the 'Change History' button below to edit the history." msgstr "Trykk på 'Endre historikk'-knappen under for å endre historikken." +#: simple_history/templates/simple_history/object_history_list.html:9 +msgid "Object" +msgstr "Objekt" + +#: simple_history/templates/simple_history/object_history_list.html:13 +msgid "Date/time" +msgstr "Dato/tid" + +#: simple_history/templates/simple_history/object_history_list.html:14 +msgid "Comment" +msgstr "Kommentar" + +#: simple_history/templates/simple_history/object_history_list.html:15 +msgid "Changed by" +msgstr "Endret av" + +#: simple_history/templates/simple_history/object_history_list.html:16 +msgid "Change reason" +msgstr "Endringsårsak" + +#: simple_history/templates/simple_history/object_history_list.html:17 +msgid "Changes" +msgstr "Endringer" + +#: simple_history/templates/simple_history/object_history_list.html:42 +msgid "None" +msgstr "Ingen" + #: simple_history/templates/simple_history/submit_line.html:4 msgid "Revert" msgstr "Tilbakestill" diff --git a/simple_history/locale/pl/LC_MESSAGES/django.po b/simple_history/locale/pl/LC_MESSAGES/django.po index 161b014c2..d420d2ebc 100644 --- a/simple_history/locale/pl/LC_MESSAGES/django.po +++ b/simple_history/locale/pl/LC_MESSAGES/django.po @@ -57,26 +57,6 @@ msgid "" msgstr "" "Wybierz datę z poniższej listy aby przywrócić poprzednią wersję tego obiektu." -#: templates/simple_history/object_history.html:17 -msgid "Object" -msgstr "Obiekt" - -#: templates/simple_history/object_history.html:18 -msgid "Date/time" -msgstr "Data/czas" - -#: templates/simple_history/object_history.html:19 -msgid "Comment" -msgstr "Komentarz" - -#: templates/simple_history/object_history.html:20 -msgid "Changed by" -msgstr "Zmodyfikowane przez" - -#: templates/simple_history/object_history.html:38 -msgid "None" -msgstr "Brak" - #: templates/simple_history/object_history.html:46 msgid "This object doesn't have a change history." msgstr "Ten obiekt nie ma historii zmian." @@ -103,6 +83,26 @@ msgstr "Naciśnij przycisk „Przywróć” aby przywrócić tę wersję obiektu msgid "Or press the 'Change History' button to edit the history." msgstr "Lub naciśnij przycisk „Historia zmian” aby edytować historię." +#: templates/simple_history/object_history_list.html:9 +msgid "Object" +msgstr "Obiekt" + +#: templates/simple_history/object_history_list.html:13 +msgid "Date/time" +msgstr "Data/czas" + +#: templates/simple_history/object_history_list.html:14 +msgid "Comment" +msgstr "Komentarz" + +#: templates/simple_history/object_history_list.html:15 +msgid "Changed by" +msgstr "Zmodyfikowane przez" + +#: templates/simple_history/object_history_list.html:42 +msgid "None" +msgstr "Brak" + #: templates/simple_history/submit_line.html:3 msgid "Revert" msgstr "Przywróć" diff --git a/simple_history/locale/pt_BR/LC_MESSAGES/django.po b/simple_history/locale/pt_BR/LC_MESSAGES/django.po index b328efa9c..f5286ece4 100644 --- a/simple_history/locale/pt_BR/LC_MESSAGES/django.po +++ b/simple_history/locale/pt_BR/LC_MESSAGES/django.po @@ -57,26 +57,6 @@ msgstr "" "Escolha a data desejada na lista a seguir para reverter as modificações " "feitas nesse objeto." -#: simple_history/templates/simple_history/object_history.html:17 -msgid "Object" -msgstr "Objeto" - -#: simple_history/templates/simple_history/object_history.html:18 -msgid "Date/time" -msgstr "Data/hora" - -#: simple_history/templates/simple_history/object_history.html:19 -msgid "Comment" -msgstr "Comentário" - -#: simple_history/templates/simple_history/object_history.html:20 -msgid "Changed by" -msgstr "Modificado por" - -#: simple_history/templates/simple_history/object_history.html:38 -msgid "None" -msgstr "-" - #: simple_history/templates/simple_history/object_history.html:46 msgid "This object doesn't have a change history." msgstr "Esse objeto não tem um histórico de modificações." @@ -104,6 +84,26 @@ msgstr "" msgid "Or press the 'Change History' button to edit the history." msgstr "Ou clique em 'Histórico de Modificações' para modificar o histórico." +#: simple_history/templates/simple_history/object_history_list.html:9 +msgid "Object" +msgstr "Objeto" + +#: simple_history/templates/simple_history/object_history_list.html:13 +msgid "Date/time" +msgstr "Data/hora" + +#: simple_history/templates/simple_history/object_history_list.html:14 +msgid "Comment" +msgstr "Comentário" + +#: simple_history/templates/simple_history/object_history_list.html:15 +msgid "Changed by" +msgstr "Modificado por" + +#: simple_history/templates/simple_history/object_history_list.html:42 +msgid "None" +msgstr "-" + #: simple_history/templates/simple_history/submit_line.html:3 msgid "Revert" msgstr "Reverter" diff --git a/simple_history/locale/ru_RU/LC_MESSAGES/django.po b/simple_history/locale/ru_RU/LC_MESSAGES/django.po index 9865513d7..c50888eae 100644 --- a/simple_history/locale/ru_RU/LC_MESSAGES/django.po +++ b/simple_history/locale/ru_RU/LC_MESSAGES/django.po @@ -50,26 +50,6 @@ msgstr "Изменено" msgid "Deleted" msgstr "Удалено" -#: templates/simple_history/_object_history_list.html:9 -msgid "Object" -msgstr "Объект" - -#: templates/simple_history/_object_history_list.html:13 -msgid "Date/time" -msgstr "Дата/время" - -#: templates/simple_history/_object_history_list.html:14 -msgid "Comment" -msgstr "Комментарий" - -#: templates/simple_history/_object_history_list.html:15 -msgid "Changed by" -msgstr "Изменено" - -#: templates/simple_history/_object_history_list.html:36 -msgid "None" -msgstr "None" - #: templates/simple_history/object_history.html:11 msgid "" "Choose a date from the list below to revert to a previous version of this " @@ -105,6 +85,30 @@ msgstr "" msgid "Or press the 'Change History' button to edit the history." msgstr "Или нажмите кнопку 'Изменить запись', чтобы изменить историю." +#: templates/simple_history/object_history_list.html:9 +msgid "Object" +msgstr "Объект" + +#: templates/simple_history/object_history_list.html:13 +msgid "Date/time" +msgstr "Дата/время" + +#: templates/simple_history/object_history_list.html:14 +msgid "Comment" +msgstr "Комментарий" + +#: templates/simple_history/object_history_list.html:15 +msgid "Changed by" +msgstr "Изменено" + +#: templates/simple_history/object_history_list.html:16 +msgid "Change reason" +msgstr "Причина изменения" + +#: templates/simple_history/object_history_list.html:42 +msgid "None" +msgstr "None" + #: templates/simple_history/submit_line.html:3 msgid "Revert" msgstr "Восстановить" @@ -112,7 +116,3 @@ msgstr "Восстановить" #: templates/simple_history/submit_line.html:4 msgid "Change History" msgstr "Изменить запись" - -#: templates/simple_history/_object_history_list.html:16 -msgid "Change reason" -msgstr "Причина изменения" diff --git a/simple_history/locale/ur/LC_MESSAGES/django.po b/simple_history/locale/ur/LC_MESSAGES/django.po index dc9c014e2..a55605eb9 100644 --- a/simple_history/locale/ur/LC_MESSAGES/django.po +++ b/simple_history/locale/ur/LC_MESSAGES/django.po @@ -58,30 +58,6 @@ msgstr "بدل گیا" msgid "Deleted" msgstr "حذف کر دیا گیا" -#: .\simple_history\templates\simple_history\_object_history_list.html:9 -msgid "Object" -msgstr "آبجیکٹ" - -#: .\simple_history\templates\simple_history\_object_history_list.html:13 -msgid "Date/time" -msgstr "تاریخ/وقت" - -#: .\simple_history\templates\simple_history\_object_history_list.html:14 -msgid "Comment" -msgstr "تبصرہ" - -#: .\simple_history\templates\simple_history\_object_history_list.html:15 -msgid "Changed by" -msgstr "کی طرف سے تبدیل" - -#: .\simple_history\templates\simple_history\_object_history_list.html:16 -msgid "Change reason" -msgstr "تبدیلی کا سبب" - -#: .\simple_history\templates\simple_history\_object_history_list.html:37 -msgid "None" -msgstr "کوئی نہیں" - #: .\simple_history\templates\simple_history\object_history.html:11 msgid "" "Choose a date from the list below to revert to a previous version of this " @@ -121,6 +97,30 @@ msgstr "" msgid "Press the 'Change History' button below to edit the history." msgstr "تاریخ میں ترمیم کرنے کے لیے نیچے دیے گئے 'تاریخ کو تبدیل کریں' کے بٹن کو دبائیں." +#: .\simple_history\templates\simple_history\object_history_list.html:9 +msgid "Object" +msgstr "آبجیکٹ" + +#: .\simple_history\templates\simple_history\object_history_list.html:13 +msgid "Date/time" +msgstr "تاریخ/وقت" + +#: .\simple_history\templates\simple_history\object_history_list.html:14 +msgid "Comment" +msgstr "تبصرہ" + +#: .\simple_history\templates\simple_history\object_history_list.html:15 +msgid "Changed by" +msgstr "کی طرف سے تبدیل" + +#: .\simple_history\templates\simple_history\object_history_list.html:16 +msgid "Change reason" +msgstr "تبدیلی کا سبب" + +#: .\simple_history\templates\simple_history\object_history_list.html:42 +msgid "None" +msgstr "کوئی نہیں" + #: .\simple_history\templates\simple_history\submit_line.html:4 msgid "Revert" msgstr "تبدیلی واپس کریں" diff --git a/simple_history/locale/zh_Hans/LC_MESSAGES/django.po b/simple_history/locale/zh_Hans/LC_MESSAGES/django.po index c69ba1ce8..0fe74ebfe 100644 --- a/simple_history/locale/zh_Hans/LC_MESSAGES/django.po +++ b/simple_history/locale/zh_Hans/LC_MESSAGES/django.po @@ -58,30 +58,6 @@ msgstr "已修改" msgid "Deleted" msgstr "已删除" -#: simple_history/templates/simple_history/_object_history_list.html:9 -msgid "Object" -msgstr "记录对象" - -#: simple_history/templates/simple_history/_object_history_list.html:13 -msgid "Date/time" -msgstr "日期/时间" - -#: simple_history/templates/simple_history/_object_history_list.html:14 -msgid "Comment" -msgstr "备注" - -#: simple_history/templates/simple_history/_object_history_list.html:15 -msgid "Changed by" -msgstr "修改人" - -#: simple_history/templates/simple_history/_object_history_list.html:16 -msgid "Change reason" -msgstr "修改原因" - -#: simple_history/templates/simple_history/_object_history_list.html:37 -msgid "None" -msgstr "无" - #: simple_history/templates/simple_history/object_history.html:11 msgid "" "Choose a date from the list below to revert to a previous version of this " @@ -119,6 +95,30 @@ msgstr "按下面的“还原”按钮还原记录到当前版本。" msgid "Press the 'Change History' button below to edit the history." msgstr "按下面的“修改历史记录”按钮编辑历史记录。" +#: simple_history/templates/simple_history/object_history_list.html:9 +msgid "Object" +msgstr "记录对象" + +#: simple_history/templates/simple_history/object_history_list.html:13 +msgid "Date/time" +msgstr "日期/时间" + +#: simple_history/templates/simple_history/object_history_list.html:14 +msgid "Comment" +msgstr "备注" + +#: simple_history/templates/simple_history/object_history_list.html:15 +msgid "Changed by" +msgstr "修改人" + +#: simple_history/templates/simple_history/object_history_list.html:16 +msgid "Change reason" +msgstr "修改原因" + +#: simple_history/templates/simple_history/object_history_list.html:42 +msgid "None" +msgstr "无" + #: simple_history/templates/simple_history/submit_line.html:4 msgid "Revert" msgstr "还原" diff --git a/simple_history/manager.py b/simple_history/manager.py index 97d745281..d8f0b53e8 100644 --- a/simple_history/manager.py +++ b/simple_history/manager.py @@ -91,6 +91,18 @@ def latest_of_each(self): ) return latest_historics + def _select_related_history_tracked_objs(self): + """ + A convenience method that calls ``select_related()`` with all the names of + the model's history-tracked ``ForeignKey`` fields. + """ + field_names = [ + field.name + for field in self.model.tracked_fields + if isinstance(field, models.ForeignKey) + ] + return self.select_related(*field_names) + def _clone(self): c = super()._clone() c._as_instances = self._as_instances diff --git a/simple_history/models.py b/simple_history/models.py index 9ff107a98..86209847b 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -2,7 +2,9 @@ import importlib import uuid import warnings +from dataclasses import dataclass from functools import partial +from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Sequence, Type, Union import django from django.apps import apps @@ -29,9 +31,7 @@ from django.utils.text import format_lazy from django.utils.translation import gettext_lazy as _ -from simple_history import utils - -from . import exceptions +from . import exceptions, utils from .manager import ( SIMPLE_HISTORY_REVERSE_ATTR_NAME, HistoricalQuerySet, @@ -44,13 +44,17 @@ pre_create_historical_m2m_records, pre_create_historical_record, ) -from .utils import get_change_reason_from_object try: from asgiref.local import Local as LocalContext except ImportError: from threading import local as LocalContext +if TYPE_CHECKING: + ModelTypeHint = models.Model +else: + ModelTypeHint = object + registered_models = {} @@ -666,7 +670,7 @@ def get_change_reason_for_object(self, instance, history_type, using): Get change reason for object. Customize this method to automatically fill change reason from context. """ - return get_change_reason_from_object(instance) + return utils.get_change_reason_from_object(instance) def m2m_changed(self, instance, action, attr, pk_set, reverse, **_): if hasattr(instance, "skip_history_when_saving"): @@ -684,11 +688,8 @@ def create_historical_record_m2ms(self, history_instance, instance): insert_rows = [] - # `m2m_field_name()` is part of Django's internal API - through_field_name = field.m2m_field_name() - + through_field_name = utils.get_m2m_field_name(field) rows = through_model.objects.filter(**{through_field_name: instance}) - for row in rows: insert_row = {"history": history_instance} @@ -942,13 +943,34 @@ def __get__(self, instance, owner): return self.model(**values) -class HistoricalChanges: - def diff_against(self, old_history, excluded_fields=None, included_fields=None): +class HistoricalChanges(ModelTypeHint): + def diff_against( + self, + old_history: "HistoricalChanges", + excluded_fields: Iterable[str] = None, + included_fields: Iterable[str] = None, + *, + foreign_keys_are_objs=False, + ) -> "ModelDelta": + """ + :param old_history: + :param excluded_fields: The names of fields to exclude from diffing. + This takes precedence over ``included_fields``. + :param included_fields: The names of the only fields to include when diffing. + If not provided, all history-tracked fields will be included. + :param foreign_keys_are_objs: If ``False``, the returned diff will only contain + the raw PKs of any ``ForeignKey`` fields. + If ``True``, the diff will contain the actual related model objects + instead of just the PKs; deleted related objects will be instances of + ``DeletedObject``. + Note that passing ``True`` will necessarily query the database if the + related objects have not been prefetched (using e.g. + ``select_related()``). + """ if not isinstance(old_history, type(self)): raise TypeError( - ("unsupported type(s) for diffing: " "'{}' and '{}'").format( - type(self), type(old_history) - ) + "unsupported type(s) for diffing:" + f" '{type(self)}' and '{type(old_history)}'" ) if excluded_fields is None: excluded_fields = set() @@ -966,59 +988,172 @@ def diff_against(self, old_history, excluded_fields=None, included_fields=None): ) m2m_fields = set(included_m2m_fields).difference(excluded_fields) + changes = [ + *self._get_field_changes_for_diff( + old_history, fields, foreign_keys_are_objs + ), + *self._get_m2m_field_changes_for_diff( + old_history, m2m_fields, foreign_keys_are_objs + ), + ] + # Sort by field (attribute) name, to ensure a consistent order + changes.sort(key=lambda change: change.field) + changed_fields = [change.field for change in changes] + return ModelDelta(changes, changed_fields, old_history, self) + + def _get_field_changes_for_diff( + self, + old_history: "HistoricalChanges", + fields: Iterable[str], + foreign_keys_are_objs: bool, + ) -> List["ModelChange"]: + """Helper method for ``diff_against()``.""" changes = [] - changed_fields = [] old_values = model_to_dict(old_history, fields=fields) - current_values = model_to_dict(self, fields=fields) + new_values = model_to_dict(self, fields=fields) for field in fields: old_value = old_values[field] - current_value = current_values[field] + new_value = new_values[field] + + if old_value != new_value: + field_meta = self._meta.get_field(field) + if foreign_keys_are_objs and isinstance(field_meta, ForeignKey): + # Set the fields to their related model objects instead of + # the raw PKs from `model_to_dict()` + def get_value(record, foreign_key): + try: + value = getattr(record, field) + # `value` seems to be None (without raising this exception) + # if the object has not been refreshed from the database + except ObjectDoesNotExist: + value = None + + if value is None: + value = DeletedObject(field_meta.related_model, foreign_key) + return value + + old_value = get_value(old_history, old_value) + new_value = get_value(self, new_value) + + change = ModelChange(field, old_value, new_value) + changes.append(change) - if old_value != current_value: - changes.append(ModelChange(field, old_value, current_value)) - changed_fields.append(field) + return changes - # Separately compare m2m fields: - for field in m2m_fields: - # First retrieve a single item to get the field names from: - reference_history_m2m_item = ( - getattr(old_history, field).first() or getattr(self, field).first() - ) - history_field_names = [] - if reference_history_m2m_item: - # Create a list of field names to compare against. - # The list is generated without the primary key of the intermediate - # table, the foreign key to the history record, and the actual 'history' - # field, to avoid false positives while diffing. - history_field_names = [ - f.name - for f in reference_history_m2m_item._meta.fields - if f.editable and f.name not in ["id", "m2m_history_id", "history"] - ] + def _get_m2m_field_changes_for_diff( + self, + old_history: "HistoricalChanges", + m2m_fields: Iterable[str], + foreign_keys_are_objs: bool, + ) -> List["ModelChange"]: + """Helper method for ``diff_against()``.""" + changes = [] - old_rows = list(getattr(old_history, field).values(*history_field_names)) - new_rows = list(getattr(self, field).values(*history_field_names)) + for field in m2m_fields: + original_field_meta = self.instance_type._meta.get_field(field) + reverse_field_name = utils.get_m2m_reverse_field_name(original_field_meta) + # Sort the M2M rows by the related object, to ensure a consistent order + old_m2m_qs = getattr(old_history, field).order_by(reverse_field_name) + new_m2m_qs = getattr(self, field).order_by(reverse_field_name) + m2m_through_model_opts = new_m2m_qs.model._meta + + # Create a list of field names to compare against. + # The list is generated without the PK of the intermediate (through) + # table, the foreign key to the history record, and the actual `history` + # field, to avoid false positives while diffing. + through_model_fields = [ + f.name + for f in m2m_through_model_opts.fields + if f.editable and f.name not in ["id", "m2m_history_id", "history"] + ] + old_rows = list(old_m2m_qs.values(*through_model_fields)) + new_rows = list(new_m2m_qs.values(*through_model_fields)) if old_rows != new_rows: + if foreign_keys_are_objs: + fk_fields = [ + f + for f in through_model_fields + if isinstance(m2m_through_model_opts.get_field(f), ForeignKey) + ] + + # Set the through fields to their related model objects instead of + # the raw PKs from `values()` + def rows_with_foreign_key_objs(m2m_qs): + def get_value(obj, through_field): + try: + value = getattr(obj, through_field) + # If the related object has been deleted, `value` seems to + # usually already be None instead of raising this exception + except ObjectDoesNotExist: + value = None + + if value is None: + meta = m2m_through_model_opts.get_field(through_field) + foreign_key = getattr(obj, meta.attname) + value = DeletedObject(meta.related_model, foreign_key) + return value + + # Replicate the format of the return value of QuerySet.values() + return [ + { + through_field: get_value(through_obj, through_field) + for through_field in through_model_fields + } + for through_obj in m2m_qs.select_related(*fk_fields) + ] + + old_rows = rows_with_foreign_key_objs(old_m2m_qs) + new_rows = rows_with_foreign_key_objs(new_m2m_qs) + change = ModelChange(field, old_rows, new_rows) changes.append(change) - changed_fields.append(field) - return ModelDelta(changes, changed_fields, old_history, self) + return changes + +@dataclass(frozen=True) +class DeletedObject: + model: Type[models.Model] + pk: Any + def __str__(self): + deleted_model_str = _("Deleted %(type_name)s") % { + "type_name": self.model._meta.verbose_name, + } + return f"{deleted_model_str} (pk={self.pk})" + + +# Either: +# - The value of a foreign key field: +# - If ``foreign_keys_are_objs=True`` is passed to ``diff_against()``: +# Either the related object or ``DeletedObject``. +# - Otherwise: +# The PK of the related object. +# +# - The value of a many-to-many field: +# A list of dicts from the through model field names to either: +# - If ``foreign_keys_are_objs=True`` is passed to ``diff_against()``: +# Either the through model's related objects or ``DeletedObject``. +# - Otherwise: +# The PK of the through model's related objects. +# +# - Any of the other possible values of a model field. +ModelChangeValue = Union[Any, DeletedObject, List[Dict[str, Union[Any, DeletedObject]]]] + + +@dataclass(frozen=True) class ModelChange: - def __init__(self, field_name, old_value, new_value): - self.field = field_name - self.old = old_value - self.new = new_value + field: str + old: ModelChangeValue + new: ModelChangeValue +@dataclass(frozen=True) class ModelDelta: - def __init__(self, changes, changed_fields, old_record, new_record): - self.changes = changes - self.changed_fields = changed_fields - self.old_record = old_record - self.new_record = new_record + changes: Sequence[ModelChange] + changed_fields: Sequence[str] + old_record: HistoricalChanges + new_record: HistoricalChanges diff --git a/simple_history/registry_tests/tests.py b/simple_history/registry_tests/tests.py index ff55f7389..2217d06a8 100644 --- a/simple_history/registry_tests/tests.py +++ b/simple_history/registry_tests/tests.py @@ -51,7 +51,7 @@ def get_history(model): self.assertRaises(AttributeError, get_history, User) self.assertEqual(len(User.histories.all()), 0) - user = User.objects.create(username="bob", password="pass") # nosec + user = User.objects.create(username="bob", password="pass") self.assertEqual(len(User.histories.all()), 1) self.assertEqual(len(user.histories.all()), 1) diff --git a/simple_history/template_utils.py b/simple_history/template_utils.py new file mode 100644 index 000000000..ae02d02e9 --- /dev/null +++ b/simple_history/template_utils.py @@ -0,0 +1,249 @@ +import dataclasses +from os.path import commonprefix +from typing import Any, Dict, Final, List, Tuple, Type, Union + +from django.db.models import ManyToManyField, Model +from django.utils.html import conditional_escape +from django.utils.safestring import SafeString, mark_safe +from django.utils.text import capfirst + +from .models import HistoricalChanges, ModelChange, ModelChangeValue, ModelDelta +from .utils import get_m2m_reverse_field_name + + +def conditional_str(obj: Any) -> str: + """ + Converts ``obj`` to a string, unless it's already one. + """ + if isinstance(obj, str): + return obj + return str(obj) + + +def is_safe_str(s: Any) -> bool: + """ + Returns whether ``s`` is a (presumably) pre-escaped string or not. + + This relies on the same ``__html__`` convention as Django's ``conditional_escape`` + does. + """ + return hasattr(s, "__html__") + + +class HistoricalRecordContextHelper: + """ + Class containing various utilities for formatting the template context for + a historical record. + """ + + DEFAULT_MAX_DISPLAYED_DELTA_CHANGE_CHARS: Final = 100 + + def __init__( + self, + model: Type[Model], + historical_record: HistoricalChanges, + *, + max_displayed_delta_change_chars=DEFAULT_MAX_DISPLAYED_DELTA_CHANGE_CHARS, + ): + self.model = model + self.record = historical_record + + self.max_displayed_delta_change_chars = max_displayed_delta_change_chars + + def context_for_delta_changes(self, delta: ModelDelta) -> List[Dict[str, Any]]: + """ + Return the template context for ``delta.changes``. + By default, this is a list of dicts with the keys ``"field"``, + ``"old"`` and ``"new"`` -- corresponding to the fields of ``ModelChange``. + + :param delta: The result from calling ``diff_against()`` with another historical + record. Its ``old_record`` or ``new_record`` field should have been + assigned to ``self.record``. + """ + context_list = [] + for change in delta.changes: + formatted_change = self.format_delta_change(change) + context_list.append( + { + "field": formatted_change.field, + "old": formatted_change.old, + "new": formatted_change.new, + } + ) + return context_list + + def format_delta_change(self, change: ModelChange) -> ModelChange: + """ + Return a ``ModelChange`` object with fields formatted for being used as + template context. + """ + old = self.prepare_delta_change_value(change, change.old) + new = self.prepare_delta_change_value(change, change.new) + + old, new = self.stringify_delta_change_values(change, old, new) + + field_meta = self.model._meta.get_field(change.field) + return dataclasses.replace( + change, + field=capfirst(field_meta.verbose_name), + old=old, + new=new, + ) + + def prepare_delta_change_value( + self, + change: ModelChange, + value: ModelChangeValue, + ) -> Any: + """ + Return the prepared value for the ``old`` and ``new`` fields of ``change``, + before it's passed through ``stringify_delta_change_values()`` (in + ``format_delta_change()``). + + For example, if ``value`` is a list of M2M related objects, it could be + "prepared" by replacing the related objects with custom string representations. + + :param change: + :param value: Either ``change.old`` or ``change.new``. + """ + 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 stringify_delta_change_values( + self, change: ModelChange, old: Any, new: Any + ) -> Tuple[SafeString, SafeString]: + """ + Called by ``format_delta_change()`` after ``old`` and ``new`` have been + prepared by ``prepare_delta_change_value()``. + + Return a tuple -- ``(old, new)`` -- where each element has been + escaped/sanitized and turned into strings, ready to be displayed in a template. + These can be HTML strings (remember to pass them through ``mark_safe()`` *after* + escaping). + + If ``old`` or ``new`` are instances of ``list``, the default implementation will + use each list element's ``__str__()`` method, and also reapply ``mark_safe()`` + if all the passed elements are safe strings. + """ + + def stringify_value(value: Any) -> Union[str, SafeString]: + # If `value` is a list, stringify each element using `str()` instead of + # `repr()` (the latter is the default when calling `list.__str__()`) + if isinstance(value, list): + string = f"[{', '.join(map(conditional_str, value))}]" + # If all elements are safe strings, reapply `mark_safe()` + if all(map(is_safe_str, value)): + string = mark_safe(string) # nosec + else: + string = conditional_str(value) + return string + + old_str, new_str = stringify_value(old), stringify_value(new) + diff_display = self.get_obj_diff_display() + old_short, new_short = diff_display.common_shorten_repr(old_str, new_str) + # Escape *after* shortening, as any shortened, previously safe HTML strings have + # likely been mangled. Other strings that have not been shortened, should have + # their "safeness" unchanged. + return conditional_escape(old_short), conditional_escape(new_short) + + def get_obj_diff_display(self) -> "ObjDiffDisplay": + """ + Return an instance of ``ObjDiffDisplay`` that will be used in + ``stringify_delta_change_values()`` to display the difference between + the old and new values of a ``ModelChange``. + """ + return ObjDiffDisplay(max_length=self.max_displayed_delta_change_chars) + + +class ObjDiffDisplay: + """ + A class grouping functions and settings related to displaying the textual + difference between two (or more) objects. + ``common_shorten_repr()`` is the main method for this. + + The code is based on + https://github.com/python/cpython/blob/v3.12.0/Lib/unittest/util.py#L8-L52. + """ + + def __init__( + self, + *, + max_length=80, + placeholder_len=12, + min_begin_len=5, + min_end_len=5, + min_common_len=5, + ): + self.max_length = max_length + self.placeholder_len = placeholder_len + self.min_begin_len = min_begin_len + self.min_end_len = min_end_len + self.min_common_len = min_common_len + self.min_diff_len = max_length - ( + min_begin_len + + placeholder_len + + min_common_len + + placeholder_len + + min_end_len + ) + assert self.min_diff_len >= 0 # nosec + + def common_shorten_repr(self, *args: Any) -> Tuple[str, ...]: + """ + Returns ``args`` with each element converted into a string representation. + If any of the strings are longer than ``self.max_length``, they're all shortened + so that the first differences between the strings (after a potential common + prefix in all of them) are lined up. + """ + args = tuple(map(conditional_str, args)) + max_len = max(map(len, args)) + if max_len <= self.max_length: + return args + + prefix = commonprefix(args) + prefix_len = len(prefix) + + common_len = self.max_length - ( + max_len - prefix_len + self.min_begin_len + self.placeholder_len + ) + if common_len > self.min_common_len: + assert ( + self.min_begin_len + + self.placeholder_len + + self.min_common_len + + (max_len - prefix_len) + < self.max_length + ) # nosec + prefix = self.shorten(prefix, self.min_begin_len, common_len) + return tuple(f"{prefix}{s[prefix_len:]}" for s in args) + + prefix = self.shorten(prefix, self.min_begin_len, self.min_common_len) + return tuple( + prefix + self.shorten(s[prefix_len:], self.min_diff_len, self.min_end_len) + for s in args + ) + + def shorten(self, s: str, prefix_len: int, suffix_len: int) -> str: + skip = len(s) - prefix_len - suffix_len + if skip > self.placeholder_len: + suffix_index = len(s) - suffix_len + s = self.shortened_str(s[:prefix_len], skip, s[suffix_index:]) + return s + + def shortened_str(self, prefix: str, num_skipped_chars: int, suffix: str) -> str: + """ + Return a shortened version of the string representation of one of the args + passed to ``common_shorten_repr()``. + This should be in the format ``f"{prefix}{skip_str}{suffix}"``, where + ``skip_str`` is a string indicating how many characters (``num_skipped_chars``) + of the string representation were skipped between ``prefix`` and ``suffix``. + """ + return f"{prefix}[{num_skipped_chars:d} chars]{suffix}" diff --git a/simple_history/templates/simple_history/_object_history_list.html b/simple_history/templates/simple_history/_object_history_list.html deleted file mode 100644 index 0dd575cc5..000000000 --- a/simple_history/templates/simple_history/_object_history_list.html +++ /dev/null @@ -1,46 +0,0 @@ -{% load i18n %} -{% load url from simple_history_compat %} -{% load admin_urls %} -{% load getattribute from getattributes %} - - - - - - {% for column in history_list_display %} - - {% endfor %} - - - - - - - - {% for action in action_list %} - - - {% for column in history_list_display %} - - - - - - {% endfor %} - -
{% trans 'Object' %}{% trans column %}{% trans 'Date/time' %}{% trans 'Comment' %}{% trans 'Changed by' %}{% trans 'Change reason' %}
{{ action.history_object }}{{ action|getattribute:column }} - {% endfor %} - {{ action.history_date }}{{ action.get_history_type_display }} - {% if action.history_user %} - {% url admin_user_view action.history_user_id as admin_user_url %} - {% if admin_user_url %} - {{ action.history_user }} - {% else %} - {{ action.history_user }} - {% endif %} - {% else %} - {% trans "None" %} - {% endif %} - - {{ action.history_change_reason }} -
diff --git a/simple_history/templates/simple_history/object_history.html b/simple_history/templates/simple_history/object_history.html index 163679b22..58c3d4536 100644 --- a/simple_history/templates/simple_history/object_history.html +++ b/simple_history/templates/simple_history/object_history.html @@ -1,8 +1,5 @@ {% extends "admin/object_history.html" %} {% load i18n %} -{% load url from simple_history_compat %} -{% load admin_urls %} -{% load display_list from simple_history_admin_list %} {% block content %} @@ -10,8 +7,8 @@ {% if not revert_disabled %}

{% blocktrans %}Choose a date from the list below to revert to a previous version of this object.{% endblocktrans %}

{% endif %}
- {% if action_list %} - {% display_list %} + {% if historical_records %} + {% include object_history_list_template %} {% else %}

{% trans "This object doesn't have a change history." %}

{% endif %} diff --git a/simple_history/templates/simple_history/object_history_list.html b/simple_history/templates/simple_history/object_history_list.html new file mode 100644 index 000000000..9356ddc09 --- /dev/null +++ b/simple_history/templates/simple_history/object_history_list.html @@ -0,0 +1,67 @@ +{% load i18n %} +{% load url from simple_history_compat %} +{% load admin_urls %} +{% load getattribute from getattributes %} + + + + + + {% for column in history_list_display %} + + {% endfor %} + + + + + + + + + {% for record in historical_records %} + + + {% for column in history_list_display %} + + {% endfor %} + + + + + + + {% endfor %} + +
{% trans 'Object' %}{% trans column %}{% trans 'Date/time' %}{% trans 'Comment' %}{% trans 'Changed by' %}{% trans 'Change reason' %}{% trans 'Changes' %}
+ + {{ record.history_object }} + + {{ record|getattribute:column }}{{ record.history_date }}{{ record.get_history_type_display }} + {% if record.history_user %} + {% url admin_user_view record.history_user_id as admin_user_url %} + {% if admin_user_url %} + {{ record.history_user }} + {% else %} + {{ record.history_user }} + {% endif %} + {% else %} + {% trans "None" %} + {% endif %} + + {{ record.history_change_reason }} + + {% block history_delta_changes %} + {% if record.history_delta_changes %} +
    + {% for change in record.history_delta_changes %} +
  • + {{ change.field }}: + {{ change.old }} + {# Add some spacing, and prevent having the arrow point to the edge of the page if `new` is wrapped #} +  →  {{ change.new }} +
    • + {% endfor %} +
+ {% endif %} + {% endblock %} +
diff --git a/simple_history/templatetags/simple_history_admin_list.py b/simple_history/templatetags/simple_history_admin_list.py index e9c598628..b6546784c 100644 --- a/simple_history/templatetags/simple_history_admin_list.py +++ b/simple_history/templatetags/simple_history_admin_list.py @@ -1,8 +1,15 @@ +import warnings + from django import template register = template.Library() -@register.inclusion_tag("simple_history/_object_history_list.html", takes_context=True) +@register.inclusion_tag("simple_history/object_history_list.html", takes_context=True) def display_list(context): + warnings.warn( + "'include' the context variable 'object_history_list_template' instead." + " This will be removed in version 3.8.", + DeprecationWarning, + ) return context diff --git a/simple_history/tests/admin.py b/simple_history/tests/admin.py index 24cf5f7ac..cc6aaf903 100644 --- a/simple_history/tests/admin.py +++ b/simple_history/tests/admin.py @@ -1,6 +1,8 @@ from django.contrib import admin +from django.utils.safestring import SafeString, mark_safe from simple_history.admin import SimpleHistoryAdmin +from simple_history.template_utils import HistoricalRecordContextHelper from simple_history.tests.external.models import ExternalModelWithCustomUserIdField from .models import ( @@ -12,8 +14,10 @@ FileModel, Paper, Person, + Place, Planet, Poll, + PollWithManyToMany, ) @@ -43,14 +47,36 @@ 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) +class HistoricalPollWithManyToManyContextHelper(HistoricalRecordContextHelper): + def prepare_delta_change_value(self, change, value): + display_value = super().prepare_delta_change_value(change, value) + if change.field == "places": + assert isinstance(display_value, list) + assert all(isinstance(place, Place) for place in display_value) + + places = sorted(display_value, key=lambda place: place.name) + display_value = list(map(self.place_display, places)) + return display_value + + @staticmethod + def place_display(place: Place) -> SafeString: + return mark_safe(f"{place.name}") + + +class PollWithManyToManyAdmin(SimpleHistoryAdmin): + def get_historical_record_context_helper(self, request, historical_record): + return HistoricalPollWithManyToManyContextHelper(self.model, historical_record) + + 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, PollWithManyToManyAdmin) diff --git a/simple_history/tests/generated_file_checks/check_translations.py b/simple_history/tests/generated_file_checks/check_translations.py index f74eade85..05126e376 100644 --- a/simple_history/tests/generated_file_checks/check_translations.py +++ b/simple_history/tests/generated_file_checks/check_translations.py @@ -1,4 +1,4 @@ -import subprocess # nosec +import subprocess import sys from glob import glob from pathlib import Path @@ -44,12 +44,12 @@ def main(): call_command("compilemessages") log("\nRunning 'git status'...") - result = subprocess.run( # nosec + result = subprocess.run( ["git", "status", "--porcelain"], check=True, stdout=subprocess.PIPE, ) - assert result.stderr is None # nosec + assert result.stderr is None stdout = result.stdout.decode() if stdout: log_err(f"Unexpected changes found in the workspace:\n\n{stdout}") @@ -61,7 +61,7 @@ def main(): sys.exit(1) else: # Print the human-readable status to the console - subprocess.run(["git", "status"]) # nosec + subprocess.run(["git", "status"]) if __name__ == "__main__": diff --git a/simple_history/tests/tests/test_admin.py b/simple_history/tests/tests/test_admin.py index 2b441030b..02b5bdc52 100644 --- a/simple_history/tests/tests/test_admin.py +++ b/simple_history/tests/tests/test_admin.py @@ -10,10 +10,12 @@ from django.test.client import RequestFactory from django.test.utils import override_settings from django.urls import reverse +from django.utils.dateparse import parse_datetime from django.utils.encoding import force_str from simple_history.admin import SimpleHistoryAdmin from simple_history.models import HistoricalRecords +from simple_history.template_utils import HistoricalRecordContextHelper from simple_history.tests.external.models import ExternalModelWithCustomUserIdField from simple_history.tests.tests.utils import ( PermissionAction, @@ -29,8 +31,10 @@ Employee, FileModel, Person, + Place, Planet, Poll, + PollWithManyToMany, State, ) @@ -86,6 +90,169 @@ def test_history_list(self): self.assertContains(response, "A random test reason") self.assertContains(response, self.user.username) + def test_history_list_contains_diff_changes(self): + self.login() + poll = Poll(question="why?", pub_date=today) + poll._history_user = self.user + poll.save() + + poll_history_url = get_history_url(poll) + response = self.client.get(poll_history_url) + self.assertContains(response, "Changes") + # The poll hasn't had any of its fields changed after creation, + # so these values should not be present + self.assertNotContains(response, "Question:") + self.assertNotContains(response, "why?") + self.assertNotContains(response, "Date published:") + + poll.question = "how?" + poll.save() + response = self.client.get(poll_history_url) + self.assertContains(response, "Question:") + self.assertContains(response, "why?") + self.assertContains(response, "how?") + self.assertNotContains(response, "Date published:") + + poll.question = "when?" + poll.pub_date = parse_datetime("2024-04-04 04:04:04") + poll.save() + response = self.client.get(poll_history_url) + self.assertContains(response, "Question:") + self.assertContains(response, "why?") + self.assertContains(response, "how?") + self.assertContains(response, "when?") + self.assertContains(response, "Date published:") + 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) + poll1_pk = poll1.pk + poll2 = Poll.objects.create(question="how?", pub_date=today) + poll2_pk = poll2.pk + 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 = f"Poll object ({poll1_pk})" + self.assertNotContains(response, expected_old_poll) + expected_new_poll = f"Poll object ({poll2_pk})" + 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, f"Deleted poll (pk={poll1_pk})") + self.assertContains(response, f"Deleted poll (pk={poll2_pk})") + + @patch( + # Test without the customization in PollWithManyToMany's admin class + "simple_history.tests.admin.HistoricalPollWithManyToManyContextHelper", + HistoricalRecordContextHelper, + ) + 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") + place1_pk = place1.pk + place2 = Place.objects.create(name="There") + place2_pk = place2.pk + 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 = ( + f"[Place object ({place1_pk}), Place object ({place2_pk})]" + ) + 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) + expected_new_places = ( + f"[Deleted place (pk={place1_pk}), Deleted place (pk={place2_pk})]" + ) + self.assertContains(response, expected_new_places) + + def test_history_list_doesnt_contain_too_long_diff_changes(self): + self.login() + + def create_and_change_poll(*, initial_question, changed_question) -> Poll: + poll = Poll(question=initial_question, pub_date=today) + poll._history_user = self.user + poll.save() + poll.question = changed_question + poll.save() + return poll + + repeated_chars = ( + HistoricalRecordContextHelper.DEFAULT_MAX_DISPLAYED_DELTA_CHANGE_CHARS + ) + + # Number of characters right on the limit + poll1 = create_and_change_poll( + initial_question="A" * repeated_chars, + changed_question="B" * repeated_chars, + ) + response = self.client.get(get_history_url(poll1)) + self.assertContains(response, "Question:") + self.assertContains(response, "A" * repeated_chars) + self.assertContains(response, "B" * repeated_chars) + + # Number of characters just over the limit + poll2 = create_and_change_poll( + initial_question="A" * (repeated_chars + 1), + changed_question="B" * (repeated_chars + 1), + ) + response = self.client.get(get_history_url(poll2)) + self.assertContains(response, "Question:") + self.assertContains(response, f"{'A' * 61}[35 chars]AAAAA") + self.assertContains(response, f"{'B' * 61}[35 chars]BBBBB") + + def test_overriding__historical_record_context_helper__with_custom_m2m_string(self): + self.login() + + place1 = Place.objects.create(name="Place 1") + place2 = Place.objects.create(name="Place 2") + place3 = Place.objects.create(name="Place 3") + poll = PollWithManyToMany.objects.create(question="why?", pub_date=today) + poll.places.add(place1, place2) + poll.places.set([place3]) + + response = self.client.get(get_history_url(poll)) + self.assertContains(response, "Places:") + self.assertContains(response, "[]") + self.assertContains(response, "[Place 1, Place 2]") + self.assertContains(response, "[Place 3]") + def test_history_list_custom_fields(self): model_name = self.user._meta.model_name self.assertEqual(model_name, "customuser") diff --git a/simple_history/tests/tests/test_deprecation.py b/simple_history/tests/tests/test_deprecation.py new file mode 100644 index 000000000..7c8ca9990 --- /dev/null +++ b/simple_history/tests/tests/test_deprecation.py @@ -0,0 +1,13 @@ +import unittest + +from simple_history import __version__ +from simple_history.templatetags.simple_history_admin_list import display_list + + +class DeprecationWarningTest(unittest.TestCase): + def test__display_list__warns_deprecation_and_is_yet_to_be_removed(self): + with self.assertWarns(DeprecationWarning): + display_list({}) + # DEV: `display_list()` (and the file `simple_history_admin_list.py`) should be + # removed when 3.8 is released + self.assertLess(__version__, "3.8") diff --git a/simple_history/tests/tests/test_manager.py b/simple_history/tests/tests/test_manager.py index 30e5ec11a..a10eda51c 100644 --- a/simple_history/tests/tests/test_manager.py +++ b/simple_history/tests/tests/test_manager.py @@ -8,7 +8,7 @@ from simple_history.manager import SIMPLE_HISTORY_REVERSE_ATTR_NAME -from ..models import Document, Poll, RankedDocument +from ..models import Choice, Document, Poll, RankedDocument User = get_user_model() @@ -371,3 +371,37 @@ def test_bulk_history_create_with_change_reason(self): ] ) ) + + +class PrefetchingMethodsTestCase(TestCase): + def setUp(self): + d = datetime(3021, 1, 1, 10, 0) + self.poll1 = Poll.objects.create(question="why?", pub_date=d) + self.poll2 = Poll.objects.create(question="how?", pub_date=d) + self.choice1 = Choice.objects.create(poll=self.poll1, votes=1) + self.choice2 = Choice.objects.create(poll=self.poll1, votes=2) + self.choice3 = Choice.objects.create(poll=self.poll2, votes=3) + + def test__select_related_history_tracked_objs__prefetches_expected_objects(self): + num_choices = Choice.objects.count() + self.assertEqual(num_choices, 3) + + def access_related_objs(records): + for record in records: + self.assertIsInstance(record.poll, Poll) + + # Without prefetching: + with self.assertNumQueries(1): + historical_records = Choice.history.all() + self.assertEqual(len(historical_records), num_choices) + with self.assertNumQueries(num_choices): + access_related_objs(historical_records) + + # With prefetching: + with self.assertNumQueries(1): + historical_records = ( + Choice.history.all()._select_related_history_tracked_objs() + ) + self.assertEqual(len(historical_records), num_choices) + with self.assertNumQueries(0): + access_related_objs(historical_records) diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index d24cb1d2a..c2eb327b7 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -1,9 +1,9 @@ +import dataclasses import unittest import uuid import warnings from datetime import datetime, timedelta -import django from django.apps import apps from django.conf import settings from django.contrib.auth import get_user_model @@ -19,8 +19,10 @@ from simple_history.exceptions import RelatedNameConflictError from simple_history.models import ( SIMPLE_HISTORY_REVERSE_ATTR_NAME, + DeletedObject, HistoricalRecords, ModelChange, + ModelDelta, is_historic, to_historic, ) @@ -28,7 +30,6 @@ pre_create_historical_m2m_records, pre_create_historical_record, ) -from simple_history.tests.custom_user.models import CustomUser from simple_history.tests.tests.utils import ( database_router_override_settings, database_router_override_settings_history_in_diff_db, @@ -86,7 +87,6 @@ ModelWithSingleNoDBIndexUnique, MultiOneToOne, MyOverrideModelNameRegisterMethod1, - OverrideModelNameAsCallable, OverrideModelNameUsingBaseModel1, Person, Place, @@ -697,11 +697,13 @@ def test_history_diff_includes_changed_fields(self): new_record, old_record = p.history.all() with self.assertNumQueries(0): delta = new_record.diff_against(old_record) - expected_change = ModelChange("question", "what's up?", "what's up, man") - self.assertEqual(delta.changed_fields, ["question"]) - self.assertEqual(delta.old_record, old_record) - self.assertEqual(delta.new_record, new_record) - self.assertEqual(expected_change.field, delta.changes[0].field) + expected_delta = ModelDelta( + [ModelChange("question", "what's up?", "what's up, man?")], + ["question"], + old_record, + new_record, + ) + self.assertEqual(delta, expected_delta) def test_history_diff_does_not_include_unchanged_fields(self): p = Poll.objects.create(question="what's up?", pub_date=today) @@ -720,11 +722,148 @@ def test_history_diff_includes_changed_fields_of_base_model(self): new_record, old_record = r.history.all() with self.assertNumQueries(0): delta = new_record.diff_against(old_record) - expected_change = ModelChange("name", "McDonna", "DonnutsKing") - self.assertEqual(delta.changed_fields, ["name"]) - self.assertEqual(delta.old_record, old_record) - self.assertEqual(delta.new_record, new_record) - self.assertEqual(expected_change.field, delta.changes[0].field) + expected_delta = ModelDelta( + [ModelChange("name", "McDonna", "DonnutsKing")], + ["name"], + old_record, + new_record, + ) + self.assertEqual(delta, expected_delta) + + def test_history_diff_arg__foreign_keys_are_objs__returns_expected_fk_values(self): + poll1 = Poll.objects.create(question="why?", pub_date=today) + poll1_pk = poll1.pk + poll2 = Poll.objects.create(question="how?", pub_date=tomorrow) + poll2_pk = poll2.pk + choice = Choice.objects.create(poll=poll1, choice="hmm", votes=3) + choice.poll = poll2 + choice.choice = "idk" + choice.votes = 0 + choice.save() + new_record, old_record = choice.history.all() + + # Test with the default value of `foreign_keys_are_objs` + with self.assertNumQueries(0): + delta = new_record.diff_against(old_record) + expected_pk_changes = [ + ModelChange("choice", "hmm", "idk"), + ModelChange("poll", poll1_pk, poll2_pk), + ModelChange("votes", 3, 0), + ] + expected_pk_delta = ModelDelta( + expected_pk_changes, ["choice", "poll", "votes"], old_record, new_record + ) + self.assertEqual(delta, expected_pk_delta) + + # Test with `foreign_keys_are_objs=True` + with self.assertNumQueries(2): # Once for each poll in the new record + delta = new_record.diff_against(old_record, foreign_keys_are_objs=True) + choice_changes, _poll_changes, votes_changes = expected_pk_changes + # The PKs should now instead be their corresponding model objects + expected_obj_changes = [ + choice_changes, + ModelChange("poll", poll1, poll2), + votes_changes, + ] + expected_obj_delta = dataclasses.replace( + expected_pk_delta, changes=expected_obj_changes + ) + self.assertEqual(delta, expected_obj_delta) + + # --- Delete the polls and do the same tests again --- + + Poll.objects.all().delete() + old_record.refresh_from_db() + new_record.refresh_from_db() + + # Test with the default value of `foreign_keys_are_objs` + with self.assertNumQueries(0): + delta = new_record.diff_against(old_record) + self.assertEqual(delta, expected_pk_delta) + + # Test with `foreign_keys_are_objs=True` + with self.assertNumQueries(2): # Once for each poll in the new record + delta = new_record.diff_against(old_record, foreign_keys_are_objs=True) + # The model objects should now instead be instances of `DeletedObject` + expected_obj_changes = [ + choice_changes, + ModelChange( + "poll", DeletedObject(Poll, poll1_pk), DeletedObject(Poll, poll2_pk) + ), + votes_changes, + ] + expected_obj_delta = dataclasses.replace( + expected_pk_delta, changes=expected_obj_changes + ) + self.assertEqual(delta, expected_obj_delta) + + def test_history_diff_arg__foreign_keys_are_objs__returns_expected_m2m_values(self): + poll = PollWithManyToMany.objects.create(question="why?", pub_date=today) + place1 = Place.objects.create(name="Here") + place1_pk = place1.pk + place2 = Place.objects.create(name="There") + place2_pk = place2.pk + poll.places.add(place1, place2) + new_record, old_record = poll.history.all() + + # Test with the default value of `foreign_keys_are_objs` + with self.assertNumQueries(2): # Once for each record + delta = new_record.diff_against(old_record) + expected_pk_change = ModelChange( + "places", + [], + [ + {"pollwithmanytomany": poll.pk, "place": place1_pk}, + {"pollwithmanytomany": poll.pk, "place": place2_pk}, + ], + ) + expected_pk_delta = ModelDelta( + [expected_pk_change], ["places"], old_record, new_record + ) + self.assertEqual(delta, expected_pk_delta) + + # Test with `foreign_keys_are_objs=True` + with self.assertNumQueries(2 * 2): # Twice for each record + delta = new_record.diff_against(old_record, foreign_keys_are_objs=True) + # The PKs should now instead be their corresponding model objects + expected_obj_change = dataclasses.replace( + expected_pk_change, + new=[ + {"pollwithmanytomany": poll, "place": place1}, + {"pollwithmanytomany": poll, "place": place2}, + ], + ) + expected_obj_delta = dataclasses.replace( + expected_pk_delta, changes=[expected_obj_change] + ) + self.assertEqual(delta, expected_obj_delta) + + # --- Delete the places and do the same tests again --- + + Place.objects.all().delete() + old_record.refresh_from_db() + new_record.refresh_from_db() + + # Test with the default value of `foreign_keys_are_objs` + with self.assertNumQueries(2): # Once for each record + delta = new_record.diff_against(old_record) + self.assertEqual(delta, expected_pk_delta) + + # Test with `foreign_keys_are_objs=True` + with self.assertNumQueries(2 * 2): # Twice for each record + delta = new_record.diff_against(old_record, foreign_keys_are_objs=True) + # The model objects should now instead be instances of `DeletedObject` + expected_obj_change = dataclasses.replace( + expected_obj_change, + new=[ + {"pollwithmanytomany": poll, "place": DeletedObject(Place, place1_pk)}, + {"pollwithmanytomany": poll, "place": DeletedObject(Place, place2_pk)}, + ], + ) + expected_obj_delta = dataclasses.replace( + expected_obj_delta, changes=[expected_obj_change] + ) + self.assertEqual(delta, expected_obj_delta) def test_history_table_name_is_not_inherited(self): def assert_table_name(obj, expected_table_name): @@ -759,8 +898,8 @@ def test_history_diff_with_excluded_fields(self): new_record, old_record = p.history.all() with self.assertNumQueries(0): delta = new_record.diff_against(old_record, excluded_fields=("question",)) - self.assertEqual(delta.changed_fields, []) - self.assertEqual(delta.changes, []) + expected_delta = ModelDelta([], [], old_record, new_record) + self.assertEqual(delta, expected_delta) def test_history_diff_with_included_fields(self): p = Poll.objects.create(question="what's up?", pub_date=today) @@ -769,13 +908,17 @@ def test_history_diff_with_included_fields(self): new_record, old_record = p.history.all() with self.assertNumQueries(0): delta = new_record.diff_against(old_record, included_fields=[]) - self.assertEqual(delta.changed_fields, []) - self.assertEqual(delta.changes, []) + expected_delta = ModelDelta([], [], old_record, new_record) + self.assertEqual(delta, expected_delta) with self.assertNumQueries(0): delta = new_record.diff_against(old_record, included_fields=["question"]) - self.assertEqual(delta.changed_fields, ["question"]) - self.assertEqual(len(delta.changes), 1) + expected_delta = dataclasses.replace( + expected_delta, + changes=[ModelChange("question", "what's up?", "what's up, man?")], + changed_fields=["question"], + ) + self.assertEqual(delta, expected_delta) def test_history_diff_with_non_editable_field(self): p = PollWithNonEditableField.objects.create( @@ -786,8 +929,13 @@ def test_history_diff_with_non_editable_field(self): new_record, old_record = p.history.all() with self.assertNumQueries(0): delta = new_record.diff_against(old_record) - self.assertEqual(delta.changed_fields, ["question"]) - self.assertEqual(len(delta.changes), 1) + expected_delta = ModelDelta( + [ModelChange("question", "what's up?", "what's up, man?")], + ["question"], + old_record, + new_record, + ) + self.assertEqual(delta, expected_delta) def test_history_with_unknown_field(self): p = Poll.objects.create(question="what's up?", pub_date=today) @@ -1003,6 +1151,14 @@ def assert_tracked_fields_equal(model, expected_field_names): PollWithHistoricalIPAddress, ["id", "question", "pub_date"], ) + assert_tracked_fields_equal( + PollWithManyToMany, + ["id", "question", "pub_date"], + ) + assert_tracked_fields_equal( + Choice, + ["id", "poll", "choice", "votes"], + ) assert_tracked_fields_equal( ModelWithCustomAttrOneToOneField, ["id", "poll"], @@ -1922,7 +2078,6 @@ def test_self_field(self): class ManyToManyWithSignalsTest(TestCase): def setUp(self): self.model = PollWithManyToManyWithIPAddress - # self.historical_through_model = self.model.history. self.places = ( Place.objects.create(name="London"), Place.objects.create(name="Paris"), @@ -1962,10 +2117,28 @@ def test_diff(self): new = self.poll.history.first() old = new.prev_record - delta = new.diff_against(old) - - self.assertEqual("places", delta.changes[0].field) - self.assertEqual(2, len(delta.changes[0].new)) + with self.assertNumQueries(2): # Once for each record + delta = new.diff_against(old) + expected_delta = ModelDelta( + [ + ModelChange( + "places", + [], + [ + { + "pollwithmanytomanywithipaddress": self.poll.pk, + "place": place.pk, + "ip_address": "192.168.0.1", + } + for place in self.places + ], + ) + ], + ["places"], + old, + new, + ) + self.assertEqual(delta, expected_delta) class ManyToManyCustomIDTest(TestCase): @@ -2242,48 +2415,46 @@ def test_diff_against(self): self.poll.places.add(self.place) add_record, create_record = self.poll.history.all() - delta = add_record.diff_against(create_record) + with self.assertNumQueries(2): # Once for each record + delta = add_record.diff_against(create_record) expected_change = ModelChange( "places", [], [{"pollwithmanytomany": self.poll.pk, "place": self.place.pk}] ) - self.assertEqual(delta.changed_fields, ["places"]) - self.assertEqual(delta.old_record, create_record) - self.assertEqual(delta.new_record, add_record) - self.assertEqual(expected_change.field, delta.changes[0].field) - - self.assertListEqual(expected_change.new, delta.changes[0].new) - self.assertListEqual(expected_change.old, delta.changes[0].old) + expected_delta = ModelDelta( + [expected_change], ["places"], create_record, add_record + ) + self.assertEqual(delta, expected_delta) - delta = add_record.diff_against(create_record, included_fields=["places"]) - self.assertEqual(delta.changed_fields, ["places"]) - self.assertEqual(delta.old_record, create_record) - self.assertEqual(delta.new_record, add_record) - self.assertEqual(expected_change.field, delta.changes[0].field) + with self.assertNumQueries(2): # Once for each record + delta = add_record.diff_against(create_record, included_fields=["places"]) + self.assertEqual(delta, expected_delta) - delta = add_record.diff_against(create_record, excluded_fields=["places"]) - self.assertEqual(delta.changed_fields, []) - self.assertEqual(delta.old_record, create_record) - self.assertEqual(delta.new_record, add_record) + with self.assertNumQueries(0): + delta = add_record.diff_against(create_record, excluded_fields=["places"]) + expected_delta = dataclasses.replace( + expected_delta, changes=[], changed_fields=[] + ) + self.assertEqual(delta, expected_delta) self.poll.places.clear() # First and third records are effectively the same. del_record, add_record, create_record = self.poll.history.all() - delta = del_record.diff_against(create_record) + with self.assertNumQueries(2): # Once for each record + delta = del_record.diff_against(create_record) self.assertNotIn("places", delta.changed_fields) + with self.assertNumQueries(2): # Once for each record + delta = del_record.diff_against(add_record) # Second and third should have the same diffs as first and second, but with # old and new reversed expected_change = ModelChange( "places", [{"place": self.place.pk, "pollwithmanytomany": self.poll.pk}], [] ) - delta = del_record.diff_against(add_record) - self.assertEqual(delta.changed_fields, ["places"]) - self.assertEqual(delta.old_record, add_record) - self.assertEqual(delta.new_record, del_record) - self.assertEqual(expected_change.field, delta.changes[0].field) - self.assertListEqual(expected_change.new, delta.changes[0].new) - self.assertListEqual(expected_change.old, delta.changes[0].old) + expected_delta = ModelDelta( + [expected_change], ["places"], add_record, del_record + ) + self.assertEqual(delta, expected_delta) @override_settings(**database_router_override_settings) @@ -2291,7 +2462,7 @@ class MultiDBExplicitHistoryUserIDTest(TestCase): databases = {"default", "other"} def setUp(self): - self.user = get_user_model().objects.create( # nosec + self.user = get_user_model().objects.create( username="username", email="username@test.com", password="top_secret" ) @@ -2332,10 +2503,10 @@ def test_history_user_does_not_exist(self): class RelatedNameTest(TestCase): def setUp(self): - self.user_one = get_user_model().objects.create( # nosec + self.user_one = get_user_model().objects.create( username="username_one", email="first@user.com", password="top_secret" ) - self.user_two = get_user_model().objects.create( # nosec + self.user_two = get_user_model().objects.create( username="username_two", email="second@user.com", password="top_secret" ) diff --git a/simple_history/tests/tests/test_template_utils.py b/simple_history/tests/tests/test_template_utils.py new file mode 100644 index 000000000..b094588f9 --- /dev/null +++ b/simple_history/tests/tests/test_template_utils.py @@ -0,0 +1,314 @@ +from datetime import datetime +from typing import Tuple + +from django.test import TestCase +from django.utils.dateparse import parse_datetime +from django.utils.safestring import mark_safe + +from simple_history.models import ModelChange, ModelDelta +from simple_history.template_utils import HistoricalRecordContextHelper, is_safe_str + +from ...tests.models import Choice, Place, Poll, PollWithManyToMany + + +class HistoricalRecordContextHelperTestCase(TestCase): + + def test__context_for_delta_changes__basic_usage_works_as_expected(self): + # --- Text and datetimes --- + + old_date = "2021-01-01 12:00:00" + poll = Poll.objects.create(question="old?", pub_date=parse_datetime(old_date)) + new_date = "2021-01-02 12:00:00" + poll.question = "new?" + poll.pub_date = parse_datetime(new_date) + poll.save() + + new, old = poll.history.all() + expected_context_list = [ + { + "field": "Date published", + "old": old_date, + "new": new_date, + }, + { + "field": "Question", + "old": "old?", + "new": "new?", + }, + ] + self.assert__context_for_delta_changes__equal( + Poll, old, new, expected_context_list + ) + + # --- Foreign keys and ints --- + + poll1 = Poll.objects.create(question="1?", pub_date=datetime.now()) + poll2 = Poll.objects.create(question="2?", pub_date=datetime.now()) + choice = Choice.objects.create(poll=poll1, votes=1) + choice.poll = poll2 + choice.votes = 10 + choice.save() + + new, old = choice.history.all() + expected_context_list = [ + { + "field": "Poll", + "old": f"Poll object ({poll1.pk})", + "new": f"Poll object ({poll2.pk})", + }, + { + "field": "Votes", + "old": "1", + "new": "10", + }, + ] + self.assert__context_for_delta_changes__equal( + Choice, old, new, expected_context_list + ) + + # --- M2M objects, text and datetimes (across 3 records) --- + + poll = PollWithManyToMany.objects.create( + question="old?", pub_date=parse_datetime(old_date) + ) + poll.question = "new?" + poll.pub_date = parse_datetime(new_date) + poll.save() + place1 = Place.objects.create(name="Place 1") + place2 = Place.objects.create(name="Place 2") + poll.places.add(place1, place2) + + newest, _middle, oldest = poll.history.all() + expected_context_list = [ + # (The dicts should be sorted by the fields' attribute names) + { + "field": "Places", + "old": "[]", + "new": f"[Place object ({place1.pk}), Place object ({place2.pk})]", + }, + { + "field": "Date published", + "old": old_date, + "new": new_date, + }, + { + "field": "Question", + "old": "old?", + "new": "new?", + }, + ] + self.assert__context_for_delta_changes__equal( + PollWithManyToMany, oldest, newest, expected_context_list + ) + + def assert__context_for_delta_changes__equal( + self, model, old_record, new_record, expected_context_list + ): + delta = new_record.diff_against(old_record, foreign_keys_are_objs=True) + context_helper = HistoricalRecordContextHelper(model, new_record) + context_list = context_helper.context_for_delta_changes(delta) + self.assertListEqual(context_list, expected_context_list) + + def test__context_for_delta_changes__with_string_len_around_character_limit(self): + now = datetime.now() + + def test_context_dict( + *, initial_question, changed_question, expected_old, expected_new + ) -> None: + poll = Poll.objects.create(question=initial_question, pub_date=now) + poll.question = changed_question + poll.save() + new, old = poll.history.all() + + expected_context_dict = { + "field": "Question", + "old": expected_old, + "new": expected_new, + } + self.assert__context_for_delta_changes__equal( + Poll, old, new, [expected_context_dict] + ) + # Flipping the records should produce the same result (other than also + # flipping the expected "old" and "new" values, of course) + expected_context_dict = { + "field": "Question", + "old": expected_new, + "new": expected_old, + } + self.assert__context_for_delta_changes__equal( + Poll, new, old, [expected_context_dict] + ) + + # Check the character limit used in the assertions below + self.assertEqual( + HistoricalRecordContextHelper.DEFAULT_MAX_DISPLAYED_DELTA_CHANGE_CHARS, 100 + ) + + # Number of characters right on the limit + test_context_dict( + initial_question=f"Y{'A' * 99}", + changed_question=f"W{'A' * 99}", + expected_old=f"Y{'A' * 99}", + expected_new=f"W{'A' * 99}", + ) + + # Over the character limit, with various ways that a shared prefix affects how + # the shortened strings are lined up with each other + test_context_dict( + initial_question=f"Y{'A' * 100}", + changed_question=f"W{'A' * 100}", + expected_old=f"Y{'A' * 60}[35 chars]AAAAA", + expected_new=f"W{'A' * 60}[35 chars]AAAAA", + ) + test_context_dict( + initial_question=f"{'A' * 100}Y", + changed_question=f"{'A' * 100}W", + expected_old=f"AAAAA[13 chars]{'A' * 82}Y", + expected_new=f"AAAAA[13 chars]{'A' * 82}W", + ) + test_context_dict( + initial_question=f"{'A' * 100}Y", + changed_question=f"{'A' * 199}W", + expected_old="AAAAA[90 chars]AAAAAY", + expected_new=f"AAAAA[90 chars]{'A' * 66}[34 chars]AAAAW", + ) + test_context_dict( + initial_question=f"{'A' * 50}Y{'E' * 100}", + changed_question=f"{'A' * 50}W{'E' * 149}", + expected_old=f"AAAAA[40 chars]AAAAAY{'E' * 60}[35 chars]EEEEE", + expected_new=f"AAAAA[40 chars]AAAAAW{'E' * 60}[84 chars]EEEEE", + ) + test_context_dict( + initial_question=f"{'A' * 50}Y{'E' * 149}", + changed_question=f"{'A' * 149}W{'E' * 50}", + expected_old=f"AAAAA[40 chars]AAAAAY{'E' * 60}[84 chars]EEEEE", + expected_new=f"AAAAA[40 chars]{'A' * 66}[84 chars]EEEEE", + ) + + # Only similar prefixes are detected and lined up; + # similar parts later in the strings are not + test_context_dict( + initial_question=f"{'Y' * 100}{'A' * 50}", + changed_question=f"{'W' * 100}{'A' * 50}{'H' * 50}", + expected_old=f"{'Y' * 61}[84 chars]AAAAA", + expected_new=f"{'W' * 61}[134 chars]HHHHH", + ) + + # Both "old" and "new" under the character limit + test_context_dict( + initial_question="A" * 10, + changed_question="A" * 100, + expected_old="A" * 10, + expected_new="A" * 100, + ) + # "new" just over the limit, but with "old" too short to be shortened + test_context_dict( + initial_question="A" * 10, + changed_question="A" * 101, + expected_old="A" * 10, + expected_new=f"{'A' * 71}[25 chars]AAAAA", + ) + # Both "old" and "new" under the character limit + test_context_dict( + initial_question="A" * 99, + changed_question="A" * 100, + expected_old="A" * 99, + expected_new="A" * 100, + ) + # "new" just over the limit, and "old" long enough to be shortened (which is + # done even if it's shorter than the character limit) + test_context_dict( + initial_question="A" * 99, + changed_question="A" * 101, + expected_old=f"AAAAA[13 chars]{'A' * 81}", + expected_new=f"AAAAA[13 chars]{'A' * 83}", + ) + + def test__context_for_delta_changes__preserves_html_safe_strings(self): + def get_context_dict_old_and_new(old_value, new_value) -> Tuple[str, str]: + # The field doesn't really matter, as long as it exists on the model + # passed to `HistoricalRecordContextHelper` + change = ModelChange("question", old_value, new_value) + # (The record args are not (currently) used in the default implementation) + delta = ModelDelta([change], ["question"], None, None) + context_helper = HistoricalRecordContextHelper(Poll, None) + (context_dict,) = context_helper.context_for_delta_changes(delta) + return context_dict["old"], context_dict["new"] + + # Strings not marked as safe should be escaped + old_string = "Hey" + new_string = "Hello" + old, new = get_context_dict_old_and_new(old_string, new_string) + self.assertEqual(old, "<i>Hey</i>") + self.assertEqual(new, "<b>Hello</b>") + # The result should still be marked safe as part of being escaped + self.assertTrue(is_safe_str(old) and is_safe_str(new)) + + # Strings marked as safe should be kept unchanged... + old_safe_string = mark_safe("Hey") + new_safe_string = mark_safe("Hello") + old, new = get_context_dict_old_and_new(old_safe_string, new_safe_string) + self.assertEqual(old, old_safe_string) + self.assertEqual(new, new_safe_string) + self.assertTrue(is_safe_str(old) and is_safe_str(new)) + + # ...also if one is safe and the other isn't... + old_string = "Hey" + new_safe_string = mark_safe("Hello") + old, new = get_context_dict_old_and_new(old_string, new_safe_string) + self.assertEqual(old, "<i>Hey</i>") + self.assertEqual(new, new_safe_string) + self.assertTrue(is_safe_str(old) and is_safe_str(new)) + + # ...unless at least one of them is too long, in which case they should both be + # properly escaped - including mangled tags + old_safe_string = mark_safe(f"

{'A' * 1000}

") + new_safe_string = mark_safe("

Hello

") + old, new = get_context_dict_old_and_new(old_safe_string, new_safe_string) + # (`` has been mangled) + expected_old = f"<p><strong>{'A' * 61}[947 chars]></p>" + self.assertEqual(old, expected_old) + self.assertEqual(new, "<p><strong>Hello</strong></p>") + self.assertTrue(is_safe_str(old) and is_safe_str(new)) + + # Unsafe strings inside lists should also be escaped + old_list = ["Hey", "Hey"] + new_list = ["Hello", "Hello"] + old, new = get_context_dict_old_and_new(old_list, new_list) + self.assertEqual(old, "[Hey, <i>Hey</i>]") + self.assertEqual(new, "[<b>Hello</b>, Hello]") + self.assertTrue(is_safe_str(old) and is_safe_str(new)) + + # Safe strings inside lists should be kept unchanged... + old_safe_list = [mark_safe("Hey"), mark_safe("Hey")] + new_safe_list = [mark_safe("Hello"), mark_safe("Hello")] + old, new = get_context_dict_old_and_new(old_safe_list, new_safe_list) + self.assertEqual(old, "[Hey, Hey]") + self.assertEqual(new, "[Hello, Hello]") + self.assertTrue(is_safe_str(old) and is_safe_str(new)) + + # ...but not when not all elements are safe... + old_half_safe_list = [mark_safe("Hey"), "Hey"] + new_half_safe_list = [mark_safe("Hello"), "Hello"] + old, new = get_context_dict_old_and_new(old_half_safe_list, new_half_safe_list) + self.assertEqual(old, "[Hey, <i>Hey</i>]") + self.assertEqual(new, "[<b>Hello</b>, Hello]") + self.assertTrue(is_safe_str(old) and is_safe_str(new)) + + # ...and also not when some of the elements are too long + old_safe_list = [mark_safe("Hey"), mark_safe(f"{'A' * 1000}")] + new_safe_list = [mark_safe("Hello"), mark_safe(f"{'B' * 1000}")] + old, new = get_context_dict_old_and_new(old_safe_list, new_safe_list) + self.assertEqual(old, f"[Hey, <i>{'A' * 53}[947 chars]</i>]") + self.assertEqual(new, f"[<b>Hello</b>, {'B' * 47}[949 chars]BBBB]") + self.assertTrue(is_safe_str(old) and is_safe_str(new)) + + # HTML tags inside too long strings should be properly escaped - including + # mangled tags + old_safe_list = [mark_safe(f"

{'A' * 1000}

")] + new_safe_list = [mark_safe(f"{'B' * 1000}")] + old, new = get_context_dict_old_and_new(old_safe_list, new_safe_list) + # (Tags have been mangled at the end of the strings) + self.assertEqual(old, f"[<h1><i>{'A' * 55}[950 chars]/h1>]") + self.assertEqual(new, f"[<strong>{'B' * 54}[951 chars]ong>]") + self.assertTrue(is_safe_str(old) and is_safe_str(new)) diff --git a/simple_history/tests/tests/test_utils.py b/simple_history/tests/tests/test_utils.py index e7da5af52..7db701d98 100644 --- a/simple_history/tests/tests/test_utils.py +++ b/simple_history/tests/tests/test_utils.py @@ -1,3 +1,4 @@ +import unittest from datetime import datetime from unittest import skipUnless from unittest.mock import Mock, patch @@ -14,9 +15,16 @@ Document, Place, Poll, + PollChildBookWithManyToMany, + PollChildRestaurantWithManyToMany, PollWithAlternativeManager, PollWithExcludeFields, PollWithHistoricalSessionAttr, + PollWithManyToMany, + PollWithManyToManyCustomHistoryID, + PollWithManyToManyWithIPAddress, + PollWithSelfManyToMany, + PollWithSeveralManyToMany, PollWithUniqueQuestion, Street, ) @@ -24,12 +32,71 @@ bulk_create_with_history, bulk_update_with_history, get_history_manager_for_model, + get_history_model_for_model, + get_m2m_field_name, + get_m2m_reverse_field_name, update_change_reason, ) User = get_user_model() +class GetM2MFieldNamesTestCase(unittest.TestCase): + def test__get_m2m_field_name__returns_expected_value(self): + def field_names(model): + history_model = get_history_model_for_model(model) + # Sort the fields, to prevent flaky tests + fields = sorted(history_model._history_m2m_fields, key=lambda f: f.name) + return [get_m2m_field_name(field) for field in fields] + + self.assertListEqual(field_names(PollWithManyToMany), ["pollwithmanytomany"]) + self.assertListEqual( + field_names(PollWithManyToManyCustomHistoryID), + ["pollwithmanytomanycustomhistoryid"], + ) + self.assertListEqual( + field_names(PollWithManyToManyWithIPAddress), + ["pollwithmanytomanywithipaddress"], + ) + self.assertListEqual( + field_names(PollWithSeveralManyToMany), ["pollwithseveralmanytomany"] * 3 + ) + self.assertListEqual( + field_names(PollChildBookWithManyToMany), + ["pollchildbookwithmanytomany"] * 2, + ) + self.assertListEqual( + field_names(PollChildRestaurantWithManyToMany), + ["pollchildrestaurantwithmanytomany"] * 2, + ) + self.assertListEqual( + field_names(PollWithSelfManyToMany), ["from_pollwithselfmanytomany"] + ) + + def test__get_m2m_reverse_field_name__returns_expected_value(self): + def field_names(model): + history_model = get_history_model_for_model(model) + # Sort the fields, to prevent flaky tests + fields = sorted(history_model._history_m2m_fields, key=lambda f: f.name) + return [get_m2m_reverse_field_name(field) for field in fields] + + self.assertListEqual(field_names(PollWithManyToMany), ["place"]) + self.assertListEqual(field_names(PollWithManyToManyCustomHistoryID), ["place"]) + self.assertListEqual(field_names(PollWithManyToManyWithIPAddress), ["place"]) + self.assertListEqual( + field_names(PollWithSeveralManyToMany), ["book", "place", "restaurant"] + ) + self.assertListEqual( + field_names(PollChildBookWithManyToMany), ["book", "place"] + ) + self.assertListEqual( + field_names(PollChildRestaurantWithManyToMany), ["place", "restaurant"] + ) + self.assertListEqual( + field_names(PollWithSelfManyToMany), ["to_pollwithselfmanytomany"] + ) + + class BulkCreateWithHistoryTestCase(TestCase): def setUp(self): self.data = [ diff --git a/simple_history/utils.py b/simple_history/utils.py index 321ec147d..a5bafeafc 100644 --- a/simple_history/utils.py +++ b/simple_history/utils.py @@ -57,6 +57,30 @@ def get_app_model_primary_key_name(model): return model._meta.pk.name +def get_m2m_field_name(m2m_field: ManyToManyField) -> str: + """ + Returns the field name of an M2M field's through model that corresponds to the model + the M2M field is defined on. + + E.g. for a ``votes`` M2M field on a ``Poll`` model that references a ``Vote`` model + (and with a default-generated through model), this function would return ``"poll"``. + """ + # This method is part of Django's internal API + return m2m_field.m2m_field_name() + + +def get_m2m_reverse_field_name(m2m_field: ManyToManyField) -> str: + """ + Returns the field name of an M2M field's through model that corresponds to the model + the M2M field references. + + E.g. for a ``votes`` M2M field on a ``Poll`` model that references a ``Vote`` model + (and with a default-generated through model), this function would return ``"vote"``. + """ + # This method is part of Django's internal API + return m2m_field.m2m_reverse_field_name() + + def bulk_create_with_history( objs, model,