Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix trailing zeros in decimal #261

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Liamhanninen
Copy link

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

@codecov-commenter
Copy link

codecov-commenter commented Sep 6, 2020

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%) ⬇️

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.

@jjkester
Copy link
Collaborator

jjkester commented Sep 8, 2020

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.

@Linkid Linkid linked an issue Mar 1, 2022 that may be closed by this pull request
@Linkid Linkid changed the title Fix for Issue #260 Fix trailing zeros in decimal Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrectly detecting change of zeroes in decimal place
3 participants