Remove the added / updated / deleted flag on version models

[etianen]

This feature was added to django-reversion in 1.4, in order to allow statistics to be gathered about additions / updates / deletes for apps that required them.

Well, turns out it was a pretty bad idea.

Prior to this, reversion had a simple, git-like model whereby Revisions represented complete snapshots of a related set of models at a point in time. The idea of them corresponding to additions / updates / deletions wasn't really relevant. The first revision mentioning a model represented it's "addition", and the last revision mentioning a model that no longer had a representation in the database represented it's "deletion". Recovering a deleted model was a simply case of instantiating it from it's last mentioning Revision.

Importantly, when a model was deleted, no Revision would be created.

By adding in the delete flag, it became a necessity to save a new revision when a model was deleted. However, when considering foreign relations, this opened up a whole can of worms.

The most obvious of these is bloating the revision/version database, but that's not the biggie.

Consider the following case:

class Person(models.Model):
    name = models.CharField(max_length=20)

class Pet(models.Model):
    owner = models.ForeignKey(Person)
    name = models.CharField(max_length=20)

revision 1:

• create person1
• create pet1 (owner person1)

revision 2:

• delete pet1

Asking reversion to list all revisions for person1 will return these two revisions. Reverting to revision 2 will be a no-op. Reverting to revision 1 will recreate pet1.

Asking reversion to list all revisions for pet1 will return the save two revisions. Reverting the whole revision will be a no-op, since you're asking reversion to return the models to a state where pet1 was deleted. However, if you just revert the Version model, you'll get a pet back.

In other words:

pet_version.revert()  # This will revert pet 1.
pet_version.revision.revert()  # This will not revert pet 1.

Complicated enough? Now it's time to consider the get_deleted method. In the current implementation, if you were to call get_deleted() on the Pet model, the version it returned would correspond to revision1, which isn't the latest revision in the database. This is because get_deleted is trying to return the revision that would allow the given model's revision to be recreated. If you were trying to access meta information stored at the time the model was deleted, then you have to use reversion.get_for_object_reference() instead.

And all this complexity just to make gathering statistics on number of deletes in the database easier?!

The madness has to end. When gathering statistics has made the whole of reversion much more complicated to understand for it's core functionality, something has gone wrong.

[andrewshkovskii]

I think you can create something like reversion statistics app/middleware witch will create a model where you can collect information about insert/update/delete actions on some models. And this will be an options to setup reversion in your app, and you will remove version's type from version model...

[etianen]

This sort of stats gathering doesn't actually have to be linked to reversion at all. It can just store the user, a generic foreign key to the model, and the action flag. Doing it with models signals is probably the way forward.

[etianen]

This has now been implemented.