Duplicate history cleanup

[fopina]

Management command to find (and remove) duplicate entries in history

Description

Management command to find (and remove) duplicate entries in history clean_duplicate_history

Related Issue

fixes #313

Motivation and Context

As discussed in #313 there are cases where we just .save() without checking if anything actually changed (as we didn't have the original data loaded).
For the same reason, doing that check in simple_history would have a big performance impact (force a SELECT before any UPDATE) for something that is not required by everyone (as I also prefer performance over clean history).

But we can have both (performance and clean history) if we clean up the history "later" hence I made a management command like this one over an year ago and been using it in quite a few apps so far.

Now I finally decided to rewrite it according to the code style of this project and share it :)

How Has This Been Tested?

Unit test made (and included) and manual testing using a test app (basically doing the same scripted in the unit test)

• Create instance
• Use admin to edit and save a few times (without changing data)
• Do it once again changing data
• Run command and verify that only 2 entries of history exist (creation and latest one where data actually changed)

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I have run the make format command to format my code
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[codecov-io]

Codecov Report

Merging #483 into master will increase coverage by 0.49%.
The diff coverage is 98.73%.

@@            Coverage Diff             @@
##           master     #483      +/-   ##
==========================================
+ Coverage   97.23%   97.73%   +0.49%     
==========================================
  Files          16       17       +1     
  Lines         724      794      +70     
  Branches       95      110      +15     
==========================================
+ Hits          704      776      +72     
+ Misses         10        8       -2     
  Partials       10       10

Impacted Files	Coverage Δ
...ory/management/commands/clean_duplicate_history.py	100% <100%> (ø)
...le_history/management/commands/populate_history.py	96.03% <92.3%> (+2.22%)	⬆️

---

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 cbb0ab4...4df7174. Read the comment docs.

[rossmechanic]

@fopina thanks for this! Do you mind trying to hit complete coverage on this PR so that coverage doesn't drop below 97%? Also, if you could add a confirmation prompt in the management command that would also be great. I think @kseever is going to look into using https://github.com/romgar/django-dirtyfields so that duplicates aren't saved in the first place, but this management command will be useful for purging all duplicates. Thanks again!

[fopina]

I've used dirtyfields before and I guess it should work well. But I don't really mind the duplicate history as long as it is eventually cleaned up, plus, I'm using version 1.9.0 in my apps and .diff_against wasn't there, but I had very similar function. Now with that helper it becomes easier for a future PR that highlights the actual changed fields in the admin interface :)

Changes to tests are mainly meant to increase coverage, took the opportunity to increase the original populate_history command coverage as well with a common small tweak.

[fopina]

Not really sure what to do about that failing check, let me know if something needs to be done :)

[fopina]

Should I do something else? Looking forward to know whether this is accepted and I upgrade the version of the package in my projects :)

[rossmechanic]

Could you add some documentation, add your name to AUTHORS.rst and add the change to CHANGES.rst?

[rossmechanic]

@fopina you still working on this?

[fopina]

Haven’t been able to come back to it yet, will do

…

On 5 Dec 2018, at 17:01, Ross Mechanic ***@***.***> wrote: @fopina you still working on this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[fopina]

Hope it looks good now :)

[rossmechanic]

Mind trying to do a bit of refactoring to solve the codeclimate issues?

[fopina]

I'm afraid I can't @rossmechanic
Although I understand/agree with flake8 rules for function/line complexity, codeclimate issues raised surpass me.. apart from the last one that I assume I can just make it an elif, I'm not sure how to fix the others.
Some I don't even understand, others I can't find a solution for them that won't trigger other issues or make code look "stupid" (as in moving 2 lines to separate functions used only in one place but in order to reset complexity counters), that's not really making code easier to understand as it means jumping back and forward between functions, imo..

[fopina]

Gave it a try though, 2 down :)

[rossmechanic]

Thanks for tryin @fopina . Rebasing should get rid of your Radon issues. I would definitely refactor the identical code block into a utils.py file. Make those changes and I'll merge with the codeclimate issues. Thanks!

[fopina]

There you go!

[rossmechanic]

Looks great! Thanks!