Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we look into creating revisions for mapping commands #1659

Open
dchiller opened this issue Oct 8, 2024 · 2 comments
Open

Should we look into creating revisions for mapping commands #1659

dchiller opened this issue Oct 8, 2024 · 2 comments

Comments

@dchiller
Copy link
Contributor

dchiller commented Oct 8, 2024

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
Copy link
Contributor Author

dchiller commented Oct 10, 2024

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant