Wrong calculation of changes when creating a new record

[ts-mk]

I think there is a bug due to the recently introduced setting AUDITLOG_STORE_JSON_CHANGES and the way it works. When creating a new record, the changes field gets populated by fields that did not change.

Environment

Python: 3.12
django-auditlog: 3.2.1

Model

class TestModel(Model):
    text = models.CharField(default="abc")
    time = models.TimeField(null=True, blank=True)

Steps

1. Set AUDITLOG_STORE_JSON_CHANGES to False.
2. Create and save a new instance of the model with text="abc", time=None.
3. auditlog.receivers.log_create calls auditlog.receivers._create_log_entry.
4. auditlog.diff.model_instance_diff gets called with old being None and new being the new model instance. use_json_for_changes is False.
5. When old_value and new_value get calculated for the field time, their values are None and "None", which means that they do not match and the field time gets added to the diff variable as ("None", "None"), and then later LogEntry gets created with changes containing the same.

The same problem would happen for the DateField but not for the DateTimeField which has its own handling in auditlog.diff.get_field_value. The handling of that method's return value is inconsistent. Sometimes smart_str is applied, sometimes not.

[hramezani]

Thanks @ts-mk for reporting this.

@The-Alchemist can you take a look here?

[The-Alchemist]

I was able to reproduce and debug. I think this bug has more to do with the handling of defaults than JSON changes. It seems to be introduced in 3a58e0a.

BEFORE 3a58e0a:

        else:
            value = smart_str(getattr(obj, field.name, None))
            if type(value).__name__ == "__proxy__":
                value = str(value)
    except ObjectDoesNotExist:
        value = (
            field.default
            if getattr(field, "default", NOT_PROVIDED) is not NOT_PROVIDED
            else None
        )

AFTER:

        else:
            value = getattr(obj, field.name)
            if not use_json_for_changes:
                value = smart_str(value)
                if type(value).__name__ == "__proxy__":
                    value = str(value)
    except (ObjectDoesNotExist, AttributeError):
        return get_default_value()

getattr(obj, field.name, None) got changed to getattr(obj, field.name) so it throws an exception now instead and that calls get_default_value().

Here's a little unit test that needs to pass:

class Issue750Test(TestCase):
    def test_issue_750_without_json(self):
        dtm = Issue750TestModel(text="abc", time=None)
        dtm.save()
        log = dtm.history.latest()
        self.assertEqual(log.changes["text"], ["None", "abc"])
        self.assertNotIn("time", log.changes)

@2ykwang , what do you think?

[2ykwang]

@The-Alchemist Thanks for catching this and providing a unit test. I’ll take a closer look this week.