improve performance, fix model_to_dict when .only or .defer are used

models_logging/__init__.py

@@ -25,10 +25,6 @@ def ignore(self, sender, instance) -> bool:
             return True
         elif self.ignore_changes is True:
             return True
-        elif instance.get_deferred_fields():
-            # if we do not ignore defered_fields
-            # we will catch exception (max recursion depth)
-            return True
         return False
 
     @property

models_logging/admin.py

@@ -1,38 +1,60 @@
 from functools import update_wrapper
 
-from django.urls import reverse
-from django.core.exceptions import PermissionDenied
+from django.apps import apps
 from django.contrib import admin
-from django.contrib.admin.utils import unquote
-from django.contrib.admin.filters import RelatedOnlyFieldListFilter
 from django.contrib import messages
-from django.urls import re_path
+from django.contrib.admin.filters import RelatedFieldListFilter
+from django.contrib.admin.utils import unquote
+from django.contrib.admin.views.main import ChangeList
+from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import PermissionDenied
+from django.db import connection
 from django.db import transaction
+from django.db.models.sql import Query
 from django.shortcuts import get_object_or_404, render, redirect
-from django.utils.html import format_html
+from django.template.response import TemplateResponse
+from django.urls import re_path
+from django.urls import reverse
 from django.utils.encoding import force_str
+from django.utils.html import format_html
+from django.utils.translation import gettext as _
 
+from .models import Change, Revision
 from .settings import CAN_DELETE_CHANGES, CAN_CHANGE_CHANGES, CAN_DELETE_REVISION, REVERT_IS_ALLOWED, \
     CHANGES_REVISION_LIMIT, ADDED
-from .models import Change, Revision
+
+
+class ChangeListWithFastCount(ChangeList):
+    def get_queryset(self, request):
+        qs = super().get_queryset(request)
+        self.query: Query = qs.query
+        qs.count = self.fast_count
+        return qs
+
+    def fast_count(self):
+        if not (self.query.group_by or self.query.where or self.query.distinct):
+            cursor = connection.cursor()
+            cursor.execute(
+                "SELECT reltuples FROM pg_class WHERE relname = %s",
+                [self.query.model._meta.db_table]
+            )
+            return int(cursor.fetchone()[0])
+        return self.query.get_count(using=self.queryset.db)
+
+
+class FastObjectsCountAdminModel(admin.ModelAdmin):
+    show_full_result_count = False
+
+    def get_changelist(self, request):
+        return ChangeListWithFastCount
 
 
 class HistoryAdmin(admin.ModelAdmin):
     object_history_template = "models_logging/object_history.html"
     history_latest_first = False
+    # If inline_models_history is '__all__' it will display changes for all models listed in `inlines`
     inline_models_history = '__all__'
 
-    def get_changes_queryset(self, object_id):
-        qs = Change.get_changes_by_obj(self.model, object_id,
-                                       related_objects=self.get_related_objects_for_changes(object_id))
-        if self.history_latest_first:
-            qs = qs.order_by('date_created')
-        return qs
-
-    def get_related_objects_for_changes(self, object_id):
-        return [m for m in self.model._meta.related_objects
-                if m.related_model in [i.model for i in self.inline_models_history]]
-
     def history_view(self, request, object_id, extra_context=None):
         """Renders the history view."""
         # Check if user has change permissions for model
@@ -42,6 +64,17 @@ def history_view(self, request, object_id, extra_context=None):
 
         assert isinstance(self.inline_models_history, (tuple, list)) or self.inline_models_history == '__all__'
 
+        # First check if the user can see this history.
+        model = self.model
+        obj = self.get_object(request, object_id)
+        if obj is None:
+            return self._get_obj_does_not_exist_redirect(
+                request, model._meta, object_id
+            )
+
+        if not self.has_view_or_change_permission(request, obj):
+            raise PermissionDenied
+
         # Compile the context.
         changes_admin = False
         if Change in admin.site._registry:
@@ -50,14 +83,48 @@ def history_view(self, request, object_id, extra_context=None):
         if self.inline_models_history == '__all__':
             self.inline_models_history = self.inlines
 
-        context = {"changes": self.get_changes_queryset(object_id), 'changes_admin': changes_admin}
+        context = {
+            **self.admin_site.each_context(request),
+            "changes": self.get_changes_queryset(obj),
+            'changes_admin': changes_admin,
+            "title": _("Change history: %s") % obj,
+            "subtitle": None,
+            "opts": model._meta,
+            "object": obj,
+        }
         context.update(extra_context or {})
-        return super(HistoryAdmin, self).history_view(request, object_id, context)
+        return TemplateResponse(
+            request,
+            self.object_history_template,
+            context,
+        )
+
+    def get_changes_queryset(self, obj):
+        qs = Change.get_changes_by_obj(
+            obj,
+            related_models=self.get_related_objects_for_changes()
+        )
+        if self.history_latest_first:
+            qs = qs.order_by('-date_created')
+        return qs
+
+    def get_related_objects_for_changes(self):
+        return [
+            m for m in self.model._meta.related_objects
+            if m.related_model in [i.model for i in self.inline_models_history]
+        ]
+
+
+class ContentTypeFilterForChange(RelatedFieldListFilter):
+    def field_choices(self, field, request, model_admin):
+        models_logging_config = apps.get_app_config('models_logging')
+        items = ContentType.objects.get_for_models(*models_logging_config.registered_models)
+        return [(item.pk, str(item)) for item in items.values()]
 
 
-class ChangeAdmin(admin.ModelAdmin):
+class ChangeAdmin(FastObjectsCountAdminModel):
     list_display = ['__str__', 'content_type', 'get_comment', 'get_link_admin_object', 'user']
-    list_filter = [('content_type', RelatedOnlyFieldListFilter), 'date_created', 'action']
+    list_filter = [('content_type', ContentTypeFilterForChange), 'date_created', 'action']
     change_form_template = 'models_logging/change_form.html'
     revert_form_template = 'models_logging/revert_changes_confirmation.html'
     search_fields = ['=object_id', '=id', '=revision__id']
@@ -152,7 +219,7 @@ def has_delete_permission(self, request, obj=None):
         return CAN_DELETE_CHANGES(request, obj) if callable(CAN_DELETE_CHANGES) else CAN_DELETE_CHANGES
 
 
-class RevisionAdmin(admin.ModelAdmin):
+class RevisionAdmin(FastObjectsCountAdminModel):
     inlines = [ChangeInline]
     list_display = ['__str__', 'comment', 'changes']
     list_filter = ['date_created']

models_logging/apps.py

@@ -9,4 +9,4 @@ class LoggingConfig(AppConfig):
 
     def ready(self):
         from .setup import models_register
-        models_register()
+        self.registered_models = models_register()

models_logging/helpers.py

@@ -1,3 +1,4 @@
+import copy
 from typing import Union, List
 
 from django.db.models.base import ModelBase
@@ -10,15 +11,23 @@
 
 def model_to_dict(instance, action=None):
     opts = instance._meta
-    ignore_fields = getattr(instance, "LOGGING_IGNORE_FIELDS", [])
+    ignore_fields = set(getattr(instance, "LOGGING_IGNORE_FIELDS", []))
     only_fields = getattr(instance, "LOGGING_ONLY_FIELDS", [])
-    if action != settings.DELETED and only_fields:
-        fnames = [f.attname for f in opts.fields if f.name in only_fields]
-    elif action != settings.DELETED and ignore_fields:
-        fnames = [f.attname for f in opts.fields if f.name not in ignore_fields]
-    else:
-        fnames = [f.attname for f in opts.fields]
-    data = {f: getattr(instance, f, None) for f in fnames}
+    if action != settings.DELETED:
+        ignore_fields.update(instance.get_deferred_fields())
+
+    fnames = [
+        f.attname for f in opts.fields
+        if f.name not in ignore_fields and f.attname not in ignore_fields and not only_fields or f.name in only_fields
+    ]
+
+    data = {}
+    for f in fnames:
+        fvalue = getattr(instance, f, None)
+        if isinstance(fvalue, (list, dict)):
+            fvalue = copy.deepcopy(fvalue)
+
+        data[f] = fvalue
     return data
 
 

models_logging/migrations/0009_alter_change_index_together.py

@@ -0,0 +1,18 @@
+# Generated by Django 4.1.11 on 2024-01-31 12:07
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("contenttypes", "0002_remove_content_type_name"),
+        ("models_logging", "0008_change_extras"),
+    ]
+
+    operations = [
+        migrations.AlterIndexTogether(
+            name="change",
+            index_together={("content_type", "object_id")},
+        ),
+    ]

models_logging/models.py

@@ -45,6 +45,7 @@ class Meta:
         ordering = ("-pk",)
         verbose_name = _('Changes of object')
         verbose_name_plural = _('All changes')
+        index_together = ('content_type', 'object_id')
 
     ACTIONS = (
         (ADDED, _("Added")),
@@ -57,7 +58,7 @@ class Meta:
     user = models.ForeignKey(LOGGING_USER_MODEL, blank=True, null=True, on_delete=models.SET_NULL,
                              verbose_name=_("User"), help_text=_("The user who created this changes."))
     object_id = models.TextField(
-        help_text=_("Primary key of the model under version control.")
+        help_text=_("Primary key of the model under version control."),
     )
     content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE,
                                      help_text="Content type of the model under version control.")
@@ -76,50 +77,39 @@ def __str__(self):
         return "Changes %s of %s <%s>" % (self.id, self.object_repr, self.date_created.strftime('%Y-%m-%d %H:%M:%S.%f'))
 
     @staticmethod
-    def get_changes_by_obj(model, obj_id, related_objects='__all__'):
+    def get_changes_by_obj(obj, related_models):
         """
         get changes of object by model and obj
-        :param model: class of models.Model
-        :param obj_id: pk
-        :param related_objects: can be "__all__" or list of models, if __all__ take changes of related objects to model
-        by default related_objects is OneToOne or ManyToOne relations, but
-        expressions for ForeignKey and ManyToMany added if related_objects is some like this
-         [m for m in self.model._meta.get_fields() if m.is_relation]
+        :param obj: instance of tracked Model
+        :param related_models: list of related models
         :return: queryset of Changes
         """
 
-        obj = model.objects.get(pk=obj_id)
-        history_objects = [{'content_type_id': ContentType.objects.get_for_model(model).id, 'values': [obj_id]}]
-        if related_objects == '__all__':
-            related_objects = model._meta.related_objects
-        for rel_model in related_objects:
+        base_qs = Change.objects.select_related("user")
+        changes_qs = base_qs.filter(content_type=ContentType.objects.get_for_model(obj.__class__), object_id=obj.pk)
+        for rel_model in related_models:
             if isinstance(rel_model, models.OneToOneRel):
                 try:
-                    values = [getattr(obj, rel_model.get_accessor_name()).pk]
+                    changes_qs = changes_qs.union(
+                        base_qs.filter(
+                            content_type=ContentType.objects.get_for_model(rel_model.related_model),
+                            object_id=getattr(obj, rel_model.get_accessor_name()).pk
+                        )
+                    )
                 except rel_model.related_model.DoesNotExist:
                     continue
             elif isinstance(rel_model, models.ManyToOneRel):
-                values = getattr(obj, rel_model.get_accessor_name()).annotate(
+                rel_objects_qs = getattr(obj, rel_model.get_accessor_name()).annotate(
                     pk_str=Cast('pk', output_field=models.TextField())
-                ).values_list('pk_str', flat=True)
-            elif isinstance(rel_model, models.ForeignKey):
-                values = [getattr(obj, rel_model.get_attname())]
-            elif isinstance(rel_model, models.ManyToManyField):
-                values = getattr(obj, rel_model.get_attname()).annotate(
-                    pk_str=Cast('pk', output_field=models.TextField())
-                ).values_list('pk_str', flat=True)
-            else:
-                continue
-
-            history_objects.append(
-                {'content_type_id': ContentType.objects.get_for_model(rel_model.related_model).id,
-                 'values': values}
-            )
-
-        qobj = models.Q()
-        for v in history_objects:
-            qobj.add(models.Q(content_type_id=v['content_type_id'], object_id__in=v['values']), models.Q.OR)
-        return Change.objects.filter(qobj).select_related('user')
+                ).values('pk_str')
+                changes_qs = changes_qs.union(
+                    base_qs.filter(
+                        content_type=ContentType.objects.get_for_model(rel_model.related_model),
+                        object_id__in=rel_objects_qs
+                    )
+                )
+
+        return changes_qs.order_by('date_created')
 
     def revert(self):
         with transaction.atomic():

models_logging/setup.py

@@ -1,13 +1,13 @@
-from django.db.models.signals import post_init, post_save, post_delete
+from django.db.models.signals import post_init, post_save, pre_delete, pre_save
 from django.apps.registry import apps
 
 from .settings import MODELS_FOR_LOGGING, MODELS_FOR_EXCLUDE
-from .signals import init_model_attrs, save_model, delete_model
+from .signals import init_model_attrs, save_model, delete_model, update_model_attrs
 
 
 def models_register():
+    registered_models = []
     if MODELS_FOR_LOGGING:
-        registered_models = []
         for app in MODELS_FOR_LOGGING:
             item = app.split('.')
             if item[-1] in [app_config.label for app_config in apps.get_app_configs()]:
@@ -21,5 +21,8 @@ def models_register():
 
         for model in registered_models:
             post_init.connect(init_model_attrs, sender=model)
+            pre_save.connect(update_model_attrs, sender=model)
             post_save.connect(save_model, sender=model)
-            post_delete.connect(delete_model, sender=model)
+            pre_delete.connect(delete_model, sender=model)
+
+    return registered_models

models_logging/signals.py

@@ -42,3 +42,17 @@ def _create_changes(object, action):
         _local.put_change_to_stack(change)
     else:
         change.save(using=LOGGING_DATABASE)
+
+
+def update_model_attrs(signal, sender, instance, **kwargs):
+    if not _local.ignore(sender, instance):
+        # if there are deferred fields which are changed still, we need to get old values from the DB
+        if instance.get_deferred_fields():
+            new_values = model_to_dict(instance)
+            if missed_fields := (set(new_values).difference(instance.__attrs)):
+                instance.refresh_from_db(fields=missed_fields)
+                for k in missed_fields:
+                    # Update __attrs with fields from the DB (old values)
+                    instance.__attrs[k] = getattr(instance, k)
+                    # set new values again
+                    setattr(instance, k, new_values[k])

setup.py

@@ -4,7 +4,7 @@
 
 setup(
     name='django-models-logging',
-    version='2.3.3',
+    version='2.4a3',
     packages=['models_logging'],
     url='https://github.com/legion-an/django-models-logging',
     package_data={'models_logging' : files},