Improve changelogging backend usage

[jakub-krysl-ipf]

📝	TLDR: See Tip box below

When migrating ipfabric_netbox to use netbox_branching, I got stuck for quite some time figuring out why updating does not work. The ObjectChange was missing prechange_data and I could not see where it's created for the backend. After reading and understanding all the code related to creating the ObjectChange for given instance and finding nothing, I resorted to reading through all the docs. There I stumbled on this note in Customization -> Custom Scripts:

This put me on right track and I went through every call to alter existing objects and added instance.snapshot() before the instance.save(). There is one place where I had to do more extensive changes, usage of queryset.update_or_create(). Turns out the requirement for calling instance.snapshot() makes usage of this handy method impossible. This seems like a big downside.

But all this hassle seems really unnecessary.

Currently the instance.snapshot() is called in frontend (defined in views) when it actually is doing backend work. Also the requirement to call instance.snapshot() produces bug in NetBox itself, where it's forgotten on related objects update, for example #11444.

Tip

Let's have only single instance.snapshot() call in signals.py just before calling to_objectchange() instead and drop the rest.

netbox/netbox/core/signals.py

Line 79 in 0f4afbc

objectchange = instance.to_objectchange(action)

if event_type == OBJECT_UPDATED:
   instance.snapshot()

This assumes the instance is serializable at that point, which it should be since to_objectchange calls serialization for UPDATE right away.

netbox/netbox/netbox/models/features.py

Line 110 in 0f4afbc

objectchange.postchange_data = self.serialize_object(exclude=exclude)

[jeremystretch]

Let's have only single instance.snapshot() call in signals.py just before calling to_objectchange() instead and drop the rest.

snapshot() must be called before any changes are made to the instance in memory, to capture its original state. A second snapshot is saved after modifying the instance to capture its post-change state.

[jakub-krysl-ipf]

Oh so you're saying it would not work, since this is in post_save and so the instance is already changed.

And if we called the snapshot() in pre_save, we would need to query the instance from DB (to get original values) and pass them to the one being saved. Which would slow everything down since it adds another DB call for every save.

So instead snapshot() is called in view before the instance is modified.

Got it, thanks. 👍