diff --git a/auditlog/diff.py b/auditlog/diff.py index e6271497..8444e1ca 100644 --- a/auditlog/diff.py +++ b/auditlog/diff.py @@ -1,9 +1,20 @@ +from collections import namedtuple +from typing import Optional + from django.conf import settings from django.core.exceptions import ObjectDoesNotExist -from django.db.models import Model, NOT_PROVIDED, DateTimeField +from django.db.models import DateTimeField, Model, NOT_PROVIDED from django.utils import timezone from django.utils.encoding import smart_text +ImplicitField = namedtuple("Field", "name") +""" +We use this to imitate a :class:`~django.db.models.Field` object, to make a diff of implicit fields +not explicitly defined. For example, if you had a relationship called +``my_rel``, you may want to only diff ``my_rel_id``, so you don't +have to pull every relation. This can have a significant performance impact. +""" + def track_field(field): """ @@ -72,15 +83,13 @@ def get_field_value(obj, field): return value -def model_instance_diff(old, new): +def model_instance_diff(old: Optional[Model], new: Optional[Model]): """ Calculates the differences between two model instances. One of the instances may be ``None`` (i.e., a newly created model or deleted model). This will cause all fields with a value to have changed (from ``None``). :param old: The old state of the model instance. - :type old: Model :param new: The new state of the model instance. - :type new: Model :return: A dictionary with the names of the changed fields as keys and a two tuple of the old and new field values as value. :rtype: dict @@ -95,7 +104,7 @@ def model_instance_diff(old, new): diff = {} if old is not None and new is not None: - fields = set(old._meta.fields + new._meta.fields) + fields = set(get_fields_in_model(old) + get_fields_in_model(new)) model_fields = auditlog.get_model_fields(new._meta.model) elif old is not None: fields = set(get_fields_in_model(old)) @@ -109,7 +118,6 @@ def model_instance_diff(old, new): # Check if fields must be filtered if model_fields and (model_fields['include_fields'] or model_fields['exclude_fields']) and fields: - filtered_fields = [] if model_fields['include_fields']: filtered_fields = [field for field in fields if field.name in model_fields['include_fields']] @@ -120,11 +128,17 @@ def model_instance_diff(old, new): if field.name not in model_fields['exclude_fields']] fields = filtered_fields + fk_only_fields = set(model_fields['fk_only_fields']) for field in fields: - old_value = get_field_value(old, field) - new_value = get_field_value(new, field) + if field.name in fk_only_fields: + my_field = ImplicitField(field.column) + else: + my_field = field + old_value = get_field_value(old, my_field) + new_value = get_field_value(new, my_field) if old_value != new_value: + # Use original field name, even for `fk_only_fields` diff[field.name] = (smart_text(old_value), smart_text(new_value)) if len(diff) == 0: diff --git a/auditlog/registry.py b/auditlog/registry.py index 538f5dd1..27869fc9 100644 --- a/auditlog/registry.py +++ b/auditlog/registry.py @@ -30,7 +30,8 @@ def __init__(self, create: bool = True, update: bool = True, delete: bool = True self._signals.update(custom) def register(self, model: ModelBase = None, include_fields: Optional[List[str]] = None, - exclude_fields: Optional[List[str]] = None, mapping_fields: Optional[Dict[str, str]] = None): + exclude_fields: Optional[List[str]] = None, mapping_fields: Optional[Dict[str, str]] = None, + fk_only_fields: Optional[List[str]] = None): """ Register a model with auditlog. Auditlog will then track mutations on this model's instances. @@ -38,7 +39,7 @@ def register(self, model: ModelBase = None, include_fields: Optional[List[str]] :param include_fields: The fields to include. Implicitly excludes all other fields. :param exclude_fields: The fields to exclude. Overrides the fields to include. :param mapping_fields: Mapping from field names to strings in diff. - + :param fk_only_fields: Only record the FK value for these relational fields. """ if include_fields is None: @@ -47,6 +48,8 @@ def register(self, model: ModelBase = None, include_fields: Optional[List[str]] exclude_fields = [] if mapping_fields is None: mapping_fields = {} + if fk_only_fields is None: + fk_only_fields = [] def registrar(cls): """Register models for a given class.""" @@ -57,6 +60,7 @@ def registrar(cls): 'include_fields': include_fields, 'exclude_fields': exclude_fields, 'mapping_fields': mapping_fields, + 'fk_only_fields': fk_only_fields, } self._connect_signals(cls) @@ -104,6 +108,7 @@ def get_model_fields(self, model: ModelBase): 'include_fields': list(self._registry[model]['include_fields']), 'exclude_fields': list(self._registry[model]['exclude_fields']), 'mapping_fields': dict(self._registry[model]['mapping_fields']), + 'fk_only_fields': list(self._registry[model]['fk_only_fields']), } def _connect_signals(self, model): diff --git a/auditlog_tests/models.py b/auditlog_tests/models.py index 4eda784f..1524657e 100644 --- a/auditlog_tests/models.py +++ b/auditlog_tests/models.py @@ -212,6 +212,11 @@ class NoDeleteHistoryModel(models.Model): history = AuditlogHistoryField(delete_related=False) +class FKOnlyFieldTestModel(models.Model): + text = models.CharField(max_length=50) + related_model = models.ForeignKey(SimpleModel, on_delete=models.CASCADE) + + auditlog.register(AltPrimaryKeyModel) auditlog.register(UUIDPrimaryKeyModel) auditlog.register(ProxyModel) @@ -226,3 +231,7 @@ class NoDeleteHistoryModel(models.Model): auditlog.register(CharfieldTextfieldModel) auditlog.register(PostgresArrayFieldModel) auditlog.register(NoDeleteHistoryModel) +auditlog.register(FKOnlyFieldTestModel, + fk_only_fields=['related_model'], + mapping_fields={'related_model': 'related_model_pk'}, + ) diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index 9993f767..5f320c85 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -1,22 +1,22 @@ import datetime -import django + +from dateutil.tz import gettz from django.conf import settings -from django.contrib import auth -from django.contrib.auth.models import User, AnonymousUser +from django.contrib.auth.models import AnonymousUser, User +from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from django.db.models.signals import pre_save from django.http import HttpResponse -from django.test import TestCase, RequestFactory +from django.test import RequestFactory, TestCase from django.utils import dateformat, formats, timezone -from dateutil.tz import gettz from auditlog.middleware import AuditlogMiddleware from auditlog.models import LogEntry from auditlog.registry import auditlog -from auditlog_tests.models import SimpleModel, AltPrimaryKeyModel, UUIDPrimaryKeyModel, \ - ProxyModel, SimpleIncludeModel, SimpleExcludeModel, SimpleMappingModel, RelatedModel, \ - ManyRelatedModel, AdditionalDataIncludedModel, DateTimeFieldModel, ChoicesFieldModel, \ - CharfieldTextfieldModel, PostgresArrayFieldModel, NoDeleteHistoryModel +from auditlog_tests.models import AdditionalDataIncludedModel, AltPrimaryKeyModel, CharfieldTextfieldModel, \ + ChoicesFieldModel, DateTimeFieldModel, FKOnlyFieldTestModel, ManyRelatedModel, NoDeleteHistoryModel, \ + PostgresArrayFieldModel, ProxyModel, SimpleExcludeModel, SimpleIncludeModel, SimpleMappingModel, SimpleModel, \ + UUIDPrimaryKeyModel class SimpleModelTest(TestCase): @@ -642,3 +642,36 @@ def test_no_delete_related(self): list(entries.values_list('action', flat=True)), [LogEntry.Action.CREATE, LogEntry.Action.UPDATE, LogEntry.Action.DELETE] ) + + +class ForeignKeyOnlyFieldTest(TestCase): + def test_fk_only_field(self): + first_related_model = SimpleModel.objects.create(text="world") + second_related_model = SimpleModel.objects.create(text="goodbye") + instance = FKOnlyFieldTestModel.objects.create( + text="hello", + related_model=first_related_model, + ) + instance.related_model = second_related_model + instance.save() + instance.delete() + + fk_only_log_entries = LogEntry.objects.filter( + content_type=ContentType.objects.get(app_label="auditlog_tests", model="fkonlyfieldtestmodel") + ).order_by("pk") + + assert len(fk_only_log_entries) == 3 + created_log = fk_only_log_entries[0] + updated_log = fk_only_log_entries[1] + deleted_log = fk_only_log_entries[2] + + expected_create_change = [str(None), str(first_related_model.pk)] + expected_update_change = [str(first_related_model.pk), str(second_related_model.pk)] + expected_delete_change = [str(second_related_model.pk), str(None)] + + self.assertEquals(created_log.changes_dict["related_model"], expected_create_change) + self.assertEquals(created_log.changes_display_dict["related_model_pk"], expected_create_change) + self.assertEquals(updated_log.changes_dict["related_model"], expected_update_change) + self.assertEquals(updated_log.changes_display_dict["related_model_pk"], expected_update_change) + self.assertEquals(deleted_log.changes_dict["related_model"], expected_delete_change) + self.assertEquals(deleted_log.changes_display_dict["related_model_pk"], expected_delete_change) diff --git a/docs/source/usage.rst b/docs/source/usage.rst index 8aa15f0e..9b2741c9 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -32,7 +32,8 @@ It is recommended to place the register code (``auditlog.register(MyModel)``) at This ensures that every time your model is imported it will also be registered to log changes. Auditlog makes sure that each model is only registered once, otherwise duplicate log entries would occur. -**Excluding fields** +Excluding fields +```````````````` Fields that are excluded will not trigger saving a new log entry and will not show up in the recorded changes. @@ -50,7 +51,33 @@ For example, to exclude the field ``last_updated``, use:: Excluding fields -**Mapping fields** +Foreign Key Only Fields +``````````````````````` + +If your models have many relations, performance may be slow if following all relations to build the changes +between actions for logging. Primary key only fields will only have their primary key logged in changes, +instead of the entire related model. To designate fields as primary key only fields, pass as list of strings +as ``fk_only_fields`` to the ``register()`` call. + +.. code-block:: python + + class MyModel(modelsModel): + sku = models.CharField(max_length=20) + version = models.CharField(max_length=5) + product = models.CharField(max_length=50, verbose_name='Product Name') + other_model = models.ForeignKey("MyOtherModel", related_name="+", on_delete=models.PROTECT) + + auditlog.register(MyModel, fk_only_fields=["other_model"]) + +There is no validation done on ``register()``. If the field is not included in the :class:`LogEntry`'s +diff, including it in ``fk_only_fields`` has no effect. + +.. versionadded:: 1.0 + + Primary key only fields + +Mapping fields +`````````````` If you have field names on your models that aren't intuitive or user friendly you can include a dictionary of field mappings during the `register()` call.