etianen / django-reversion

django-reversion is an extension to the Django web framework that provides version control for model instances.
https://django-reversion.readthedocs.io
BSD 3-Clause "New" or "Revised" License
3.03k stars 489 forks source link

Restore FK logic and best way to ignore fields #957

Open TonisPiip opened 8 months ago

TonisPiip commented 8 months ago

Hey, my project has been using this for a while, it's a neat app, but mainly it's being used to generate a changelog, so the the documented use-case, but that's not my question.

I tested this with a registered model which has a 1 to 1 relation on another registered model. I wanted to try out the admin system, when clicking the timestamp in the history I hit this issue: https://github.com/etianen/django-reversion/issues/387 As we have the modified timestamp excluded.

I allowed null values for that field, and it got pasted it for the main model, and then I got the same error when revisions tried to apply a revision to it's 1on1 related row, which has the same field.

Why is it doing that? Meaning; why is reversion applying a reversion to related rows? I can't find anything in the docs about it.

And also in the linked issue above, it was said it's a bad idea to exclude fields, but I wonder, where is that mentioned in the docs, or is there some best practices? Or is there some way to exclude fields, so they are still tracked, but not have the fields be considered when it's checking the logic for ignore_duplicates?

etianen commented 8 months ago

Hi @TonisPiip!

From the docs, the admin integration automatically follows some kinds of relation when auto-registering models:

If you’ve registered your models using reversion.register(), the admin class will use the configuration you specify there. Otherwise, the admin class will auto-register your model, following all inline model relations and parent superclasses. Customize the admin registration by overriding VersionAdmin.register().

Following a relation means that relation is always grouped into a a revision whenever the model is saved. Possibly this is what's causing your problems?

There's no current feature to ignore fields as part of the ignore_duplicates check. I'd be happy to take a PR for that feature, as it seems useful!

TonisPiip commented 8 months ago

Thanks for the pointer for the doc, I do fear that is not the issues, or maybe I'm not understanding it well enough.

The modeladmin which has been registered has no inlines, just a 1on1 field. Is there any logic for restoring that when you restore, and the model as a (FK or 1to1) relation to another registered model, that model also gets restored?

etianen commented 8 months ago

Are you using manual @reversion.register() calls for your models?

TonisPiip commented 1 month ago

Are you using manual @reversion.register() calls for your models?

@etianen Yes.

Is there a better method?