Enable diffing m2m fields #932

thijskramer · 2021-12-20T14:58:31Z

At first @avalanchy did the initial work. After that https://github.com/bmedx made an excellent PR based on his work which wasn't completed, so I tried to make it work and created a PR myself, based on edx#1 and rebased on the current master.

Description

This PR enables tracking history of m2m relations. It works in my specific situation, however I would be happy to hear if there are any situations I haven't looked at, I should cover or simply could be improved.

Related Issue

#399 - Track history of M2M fields

Motivation and Context

A few days ago I received the question "how can I see who has removed an article from this category?" I immediately dove into the history I had enabled, but I quickly discovered that m2m relations weren't tracked. I started googling for solutions and stumbled upon the Pull Request @bmedx made for #399.
I installed his fork and after some minor tweaks and a rebase, I made it work for my situation.
It tracks each m2m change in a separate m2m_history model, so that you won't need a reference to the actual referenced model.

How Has This Been Tested?

Tests have been included, however I'm unsure if they are enough to cover all use cases.

Type of change

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 pre-commit run command to format and lint.
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.
I have added my name and/or github handle to AUTHORS.rst
I have added my change to CHANGES.rst
All new and existing tests passed.

simple_history/models.py

jeking3 · 2022-02-04T20:56:52Z

I wonder if perhaps the PR title is not quite right, since this seems to be doing more than allowing for a diff.

lazarust · 2022-03-17T13:31:53Z

@thijskramer It looks like some tests failed in the GH Action, could you look into that? This feature would be really helpful for me!

thijskramer · 2022-03-17T13:53:05Z

I'll take a look in the upcoming days!

…

On Thu, 17 Mar 2022 at 14:32, Thomas Lazarus ***@***.***> wrote: @thijskramer <https://github.com/thijskramer> It looks like some tests failed in the GH Action, could you look into that? This feature would be really helpful for me! — Reply to this email directly, view it on GitHub <#932 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJXH42JAPEE5VPM557ZSA3VAMX5LANCNFSM5KN4257A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

utapyngo · 2022-03-23T02:14:10Z

@lazarust can you work this around by converting your m2m relation into an explicit model and adding history to it?
I really want #866 to be released ASAP, and this PR has conflicts and broken tests.

lazarust

I've looked through this and it looks good to me! Hopefully it can get merged in soon!!

thijskramer · 2022-03-25T16:18:04Z

I have to fix a few tests before it can be merged, but I’m a bit short on time so I’d welcome any help!

…

On Fri, 25 Mar 2022 at 15:17, Thomas Lazarus ***@***.***> wrote: ***@***.**** approved this pull request. I've looked through this and it looks good to me! Hopefully it can get merged in soon!! — Reply to this email directly, view it on GitHub <#932 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJXH44NW6P5MR22DAZEQP3VBXDFTANCNFSM5KN4257A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

`django-simple-history` doesn't currently track changes to many-to-many relationships - which was the main point of this commit (i.e. to track the changes to `own_permissions`) - but once jazzband/django-simple-history#399 is resolved (likely through jazzband/django-simple-history#932), it will.

legau · 2022-06-14T09:35:40Z

I fixed your tests on thijskramer#20
This should be good once merged into your branch

legau · 2022-06-14T12:05:23Z

One more to go thijskramer#21 to resolve conflicts with master

thijskramer · 2022-06-14T12:06:52Z

I think it's cleaner to rebase this branch onto master, isn't it?

Working prototype with tests - Custom through models may not work correctly - Currently storing IDs for FKs, probably needs replacing Now using model instances instead of IDs when possible flake8 fixes Delete dead code, comments, and change to bulk create Clean up, add m2m support to diff_against Add basic tests for several m2m fields on one model Formatting fixes from Black Housekeeping, fixing Python 2.7, removing Python 3.4 - Py3.4 removed as coverage no longer supports it, could be pinned instead, but I went with this. remove python2 thing

legau · 2022-06-14T12:14:26Z

Indeed I tried that but with my own branch so it conflicts with your branch now

thijskramer · 2022-06-14T12:30:22Z

Indeed I tried that but with my own branch so it conflicts with your branch now

I've rebased my branch!

legau · 2022-06-15T13:29:29Z

I added inheritance support with thijskramer#22

ipeterov · 2022-06-18T16:04:36Z

Sorry about a comment that isn't immediately relevant, I just wanted to say that I really want the feature and I'm very happy to see the current activity.

Thank you @thijskramer and @legau for your work!

cpackard · 2022-08-31T15:47:19Z

hey all, any reason this couldn't be merged? m2m field tracking would be a huge plus!

codecov · 2022-09-27T15:58:47Z

Codecov Report

Merging #932 (603c7e3) into master (529dbe2) will decrease coverage by 0.53%.
The diff coverage is 87.87%.

@@            Coverage Diff             @@
##           master     #932      +/-   ##
==========================================
- Coverage   97.65%   97.11%   -0.54%     
==========================================
  Files          23       23              
  Lines        1149     1215      +66     
  Branches      222      237      +15     
==========================================
+ Hits         1122     1180      +58     
- Misses         12       17       +5     
- Partials       15       18       +3

Impacted Files	Coverage Δ
simple_history/models.py	`96.38% <87.87%> (-1.39%)`	⬇️

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

cpackard · 2022-09-28T14:00:51Z

@thijskramer @valberg @legau @johanneswilm thanks for all the work and merging this in 🙌

thijskramer commented Dec 20, 2021

View reviewed changes

simple_history/models.py Outdated Show resolved Hide resolved

thijskramer force-pushed the m2m-support branch from 0ad5b78 to 4efc956 Compare January 28, 2022 10:38

thijskramer force-pushed the m2m-support branch from 4efc956 to abeedda Compare February 4, 2022 18:56

thijskramer marked this pull request as draft February 4, 2022 19:19

thijskramer force-pushed the m2m-support branch from abeedda to 7bd18a1 Compare February 4, 2022 19:43

thijskramer marked this pull request as ready for review February 4, 2022 19:44

lazarust mentioned this pull request Mar 16, 2022

Time for a new release? #960

Closed

thijskramer force-pushed the m2m-support branch 2 times, most recently from c3444e8 to 4b7538d Compare March 23, 2022 07:32

lazarust previously approved these changes Mar 25, 2022

View reviewed changes

thijskramer dismissed lazarust’s stale review via 52133fa June 14, 2022 09:39

bmedx and others added 5 commits June 14, 2022 14:13

compare only specific fields when diffing between m2m relations

f9d5d1e

add docs to explain the usage of m2m tracking

6974028

add myself to the authors list

91c72c5

fix tests

8e28545

thijskramer force-pushed the m2m-support branch from 52133fa to 8e28545 Compare June 14, 2022 12:18

Merge branch 'master' into m2m-support

603c7e3

valberg approved these changes Sep 28, 2022

View reviewed changes

valberg merged commit 7042a64 into jazzband:master Sep 28, 2022

legau mentioned this pull request Sep 28, 2022

M2M support for inheritance and signals #1042

Merged

11 tasks

ddabble mentioned this pull request Sep 28, 2022

Track history of M2M fields #399

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable diffing m2m fields #932

Enable diffing m2m fields #932

thijskramer commented Dec 20, 2021

jeking3 commented Feb 4, 2022

lazarust commented Mar 17, 2022

thijskramer commented Mar 17, 2022 via email

utapyngo commented Mar 23, 2022

lazarust left a comment

thijskramer commented Mar 25, 2022 via email

legau commented Jun 14, 2022

legau commented Jun 14, 2022

thijskramer commented Jun 14, 2022

legau commented Jun 14, 2022

thijskramer commented Jun 14, 2022

legau commented Jun 15, 2022

ipeterov commented Jun 18, 2022

cpackard commented Aug 31, 2022

codecov bot commented Sep 27, 2022 •

edited

cpackard commented Sep 28, 2022

Enable diffing m2m fields #932

Enable diffing m2m fields #932

Conversation

thijskramer commented Dec 20, 2021

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Type of change

Checklist:

jeking3 commented Feb 4, 2022

lazarust commented Mar 17, 2022

thijskramer commented Mar 17, 2022 via email

utapyngo commented Mar 23, 2022

lazarust left a comment

Choose a reason for hiding this comment

thijskramer commented Mar 25, 2022 via email

legau commented Jun 14, 2022

legau commented Jun 14, 2022

thijskramer commented Jun 14, 2022

legau commented Jun 14, 2022

thijskramer commented Jun 14, 2022

legau commented Jun 15, 2022

ipeterov commented Jun 18, 2022

cpackard commented Aug 31, 2022

codecov bot commented Sep 27, 2022 • edited

Codecov Report

cpackard commented Sep 28, 2022

codecov bot commented Sep 27, 2022 •

edited