DDMAL / CantusDB

A new site for Cantus Database running under Django.
https://cantusdatabase.org
MIT License
5 stars 6 forks source link

Should we look into creating revisions for mapping commands #1659

Open dchiller opened 1 month ago

dchiller commented 1 month ago

Commands that perform bulk updates don't have revisions saved (I'm still working out how our various ModelForms are captured by django-reversion) but given the multiple snafus we've now had with mapping data (users and Cantus ID's chief among them), it may really be worth having these stored!

Edit: Create and Update view requests are captured through the Reversion middleware (https://django-reversion.readthedocs.io/en/latest/middleware.html). All POST, PUT, PATCH requests by default are wrapped in a reversion.

dchiller commented 1 month ago

Seems like this is very doable if we create reversions directly in our commands (see here: https://django-reversion.readthedocs.io/en/latest/api.html). Note however, that bulk actions on QuerySets (eg. update) don't trigger signals and therefore are not included (as opposed to actions on models themselves, like save).

A comment on the django-reversion github gives a workaround:

Bulk actions (such as Queryset.update(), Queryset.bulk_update, or Queryset.bulk_create()) do not send signals, so won’t be noticed by django-reversion. You can use add_to_revision with bulk_update or bulk_create as a workaround, but this may eliminate the performance advantage of doing bulk actions since this requires fetching the objects into memory (unlike update()) and reversion will do one query per new version object

dchiller commented 1 month ago

I'll try to implement this on #1661 and report back.