django-simple-history icon indicating copy to clipboard operation
django-simple-history copied to clipboard

diff_against for m2m fields produces strange output

Open Kangaroux opened this issue 3 years ago • 1 comments

If you use diff_against and there's a m2m change, the change value is not a list of the object IDs, but instead a list of dictionaries. The dicts are more or less a serialization of the rows from the join table. Now instead of simply being able to iterate through the changes and see the old/new values, we need to handle m2m fields as a special case and extract the nested object IDs.

As a crude example, suppose you have the models Cinema and Film, and Cinema has a M2M relationship with Film. You might do something like cinema.films.set([film1, film2]).

If I were to change the films in this way, I could query the history to see the changes:

latest = cinema.history.latest()
diff = latest.diff_against(latest.prev_record)
change = diff.changes[0]
print("old:", change.old)
print("new:", change.new)

And what I would expect to be printed is the old and new list of film IDs (where 15, 19 are the IDs of film1 and film2)

old: []
new: [15, 19]

Instead you get a list of dicts where each dict is a pair of the cinema and film:

old: []
new: [{"cinema": 1, "film": 15}, {"cinema": 1, "film": 19}]

Kangaroux avatar Nov 08 '22 20:11 Kangaroux

I'm guessing it was implemented like that because the developer(s) wanted the diff to include potential changes in other fields on the M2M through model 🤔 However, tracking changes in the through model is currently not possible anyway (as long as Django ticket #13757 is not fixed), and if it were, I think it should only be done by registering history tracking directly with the through model.

Other than that, as far as I can tell, the only relevant information we should track for M2M relations is the PKs of the related model (Film in @Kangaroux's example), and so I see no reason to include anything other than the PKs in the diff lists. I can implement the necessary changes, if no one happens to do it before me 🙂

ddabble avatar Feb 26 '24 20:02 ddabble