jazzband / django-auditlog

A Django app that keeps a log of changes made to an object.
MIT License
1.15k stars 415 forks source link

Question: Datatype casting in JSON changes column #675

Open ulinja opened 1 month ago

ulinja commented 1 month ago

When logging changes to model instances, the changes column casts all python data types into strings, rather than their JSON equivalents:

and so on.

To illustrate this, consider the following model:

from django.db import models
from auditlog.registry import auditlog

class Person(models.Model):
    is_cool = models.BooleanField(default=False)
    age = models.IntegerField(
        null=True,
        default=None,
    )

auditlog.register(Person)

And with the following changes:

>>> from myapp.models import Person
>>> person = Person.objects.create()
>>> person.is_cool = True
>>> person.age = 42
>>> person.save()
>>> 
>>> from auditlog.models import LogEntry
>>> log_entry = LogEntry.objects.first()
>>> log_entry.changes
{'is_cool': ['False', 'True'], 'age': ['None', '42']}
>>> type(log_entry.changes['age'][0])
<class 'str'>
>>> type(log_entry.changes['age'][1])
<class 'str'>

The values in the changes object stored in the database just get turned into strings, such that all type information is lost. Here is what the changes column looks like in the database:

{"is_cool": ["False", "True"], "age": ["None", "42"]}

Instead, I would expect the following:

>>> log_entry.changes
{'is_cool': [False, True], 'age': [None, 42]}
>>> type(log_entry.changes['age'][0])
<class 'NoneType'>
>>> type(log_entry.changes['age'][1])
<class 'int'>

and the following JSON in the DB:

{"is_cool": [false, true], "age": [null, 42]}

Am I doing something wrong or is this behavior intended?

hramezani commented 1 month ago

Yes, it is a string. the model field's type is JSONField. but it seems the value type is string.

I think it can be json as the field type is JSONField. not sure how we can change it without introducing a breaking change.

Probably by introducing a config flag like STORE_JSON_CHANGES or something like that with the default value of False. then by enabling this flag the changes will be stored in json and we can remove the config in the next major release.

BTW, this is an initial idea and needs some more investigation. Unfortunately, I don't have time to investigate.

@aqeelat @aleh-rymasheuski do you have any idea here?