Related object diffs should be based on PK changes not string representations.

[sum-rock]

Summary

We've encountered two errors caused by the way auditlog detects updates to related objects on a ForeignKey. In one case, auditlog is detecting a change where no change has occurred (commission error). In the other case, auditlog is not detecting a change when it should (omission error).

Errors types

Commission Error

This may be less common than the omission error but it is problematic because it is causing significant bloat on the LogEntry table. If an object (e.g, Appointment) has an in memory change that effects its string representation and then an object that has a reverse relationship to that object (e.g., AppointmentAttendance) is saved without changes, auditlog will make a LogEntry for that action even though no update occurred.

It does this because when the diff method is looking for changes to the child object (AppointmentAttendance), it is comparing the new string representation for the parent object (Appointment) against the one for the persisted version of that object. Even though there was not change to the foreign key on the child object (e.g. AppointmentAttendance.appointment_id), auditlog detects the change in the string representation for the related object (Appointment) and incorrectly logs that an update has occurred.

Omission Error

In cases where there are more than one object with the same string representation (e.g., two or more "John Smith" objects in the Customer model) and a related object is updated to a different object with the same string representation (e.g., an update to a PhoneNumber object "+1555-555-5555" to belong to a different "John Smith" Customer object) auditlog is not able to capture that update.

This is because because the diff method is comparing object strings rather than primary keys and there is no guarantee that the str method is returning a unique value. Therefore, a portion of updates to ForeignKey fields are likely not being captured by auditlog on current installations.

Solution

A relatively simple solution to these issues is to have the diff method examine related object primary keys rather than the object's string representations when evaluating whether or not a change has occurred. See my PR for more details.

[lerela]

Hi @sum-rock

Thanks for this PR, str logging of FKs is also a pain point for us!

It seems that your fix does not account for m2m relations that get logged with instance.relation.add(other_instance). Those are still logged as strings, for instance: {'type': 'm2m', 'operation': 'delete', 'objects': ['My Object String Representation']}.

Did you just not run into this situation, or was there any reason not to include m2m in #420?

[anyidea]

It seems that string representations are more readable than pks in audit logs.

Could we set something like AUDITLOG_STRING_RELATIONS or something else in settings to switch between pk and object's string representations?