Change diff to evaluate PKs for FK relationships

[sum-rock]

~~I am going to write up an issue to relate to this PR in the next day or two. At the moment, I'm focused on getting this fix merged into our fork to resolve some bloating on our LogEntry table.~~ (Done, see attached issue)

Take a look and let me know what you think of this fix. I did look through the current issues and couldn't find anyone else complaining about this so maybe we've got some anti-patterns going on in our code. The issue is only happening in some of our more complex systems but I'm sure we're not that unique. I'll highlight the test case that shows the fix.

After letting this sit for an hour or two I realized this change will fix another more insidious bug. Currently, 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.

In this commit you can see a test case that demonstrates the flaw and how it is resolved by this change.

---

The diff method now evaluates the primary keys for changes to determine if a new LogEntry should be created. Previously, the diff method was evaluating the string representation of the object. This was flawed because cases can occur when a parent object has in memory changes to its string string representation and the child related object is saved prior to these in memory changes being persisted. In these cases a new LogEntry object would be created erroneously. This cases is asserted with a test and a regression test will verify the bug.

The consequence of these updates is that the LogEntry.changes field now stores primary keys rather than string representations for related objects. To keep the changes dictionary display unaffected by this update, a method was added to the LogEntry model. This method looks up the object display string from the stored foreign key. Exceptions were written to handle backwards compatibility.

Issue

[codecov[bot]]

Codecov Report

Merging #420 (39e0be9) into master (c65c38e) will increase coverage by 0.10%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #420      +/-   ##
==========================================
+ Coverage   93.55%   93.66%   +0.10%     
==========================================
  Files          29       29              
  Lines         869      884      +15     
==========================================
+ Hits          813      828      +15     
  Misses         56       56              

Impacted Files	Coverage Δ
auditlog/diff.py	97.22% <100.00%> (+0.07%)	:arrow_up:
auditlog/models.py	88.58% <100.00%> (+0.61%)	:arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[hramezani]

I think this change is a breaking change as the patch is changing the representation of the related field. we need to add changelog for that and explain the change and the breaking change.

Also, It would be great if @alieh-rymasheuski can take a look.

[sum-rock]

@hramezani Sorry for the delay in returning to this. I've added a statement in the change log and updated the docstring as requested. Let me know if there is anything else I can do to help get this patch in the 2.2 release.

If it is critical to preserve the object representation in the changes field (instead of the FK), there are options but they involve more complexity and my estimation is that the complexity isn't worth it.

[hramezani]

Thanks @sum-rock for addressing the comments. As I mentioned in https://github.com/jazzband/django-auditlog/pull/420#issuecomment-1256233511 it is a breaking change and we can not include it in 2.2. we will include it in 3.0

[hramezani]

@sum-rock we are preparing v3.0. Could you please refresh this PR?

[sum-rock]

@hramezani This branch is now refreshed