django-auditlog icon indicating copy to clipboard operation
django-auditlog copied to clipboard

Fix trailing zeros in decimal

Open Liamhanninen opened this issue 4 years ago • 2 comments

This is my first pull request. I hope this can help. Please let me know if you have any questions or better ideas on how to fix this.

EXAMPLE ISSUE: The field's decimal places = 2. So the db stores .00. But in the model's save() method performs calculations on that field which result in python truncating the zeros to .0. This causes model_instance_diff() to incorrectly detect a change.

THE FIX: check if the field is a DecimalField. If so then remove all trailing zeros and the decimal. Used the code from this answer https://stackoverflow.com/questions/11227620/drop-trailing-zeros-from-decimal

Liamhanninen avatar Sep 06 '20 15:09 Liamhanninen

Codecov Report

Merging #261 into master will decrease coverage by 0.28%. The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #261      +/-   ##
==========================================
- Coverage   82.45%   82.17%   -0.29%     
==========================================
  Files          19       19              
  Lines         513      516       +3     
==========================================
+ Hits          423      424       +1     
- Misses         90       92       +2     
Impacted Files Coverage Δ
src/auditlog/diff.py 83.09% <50.00%> (-2.20%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c3b0604...e3d3892. Read the comment docs.

codecov-commenter avatar Sep 06 '20 15:09 codecov-commenter

I think this can be resolved in a smarter way, by finding an alternative to the string conversion that currently happens.

Most likely comparing bare values works, but this is something that may not be the case for all values. This way we can leverage the __eq__ magic method of Python.

Creating special cases for fields is something I rather not do, as you are likely to end up with many of them that all need to be maintained.

jjkester avatar Sep 08 '20 21:09 jjkester