django-fluent-pages
django-fluent-pages copied to clipboard
Add support for django-reversion.
So that a page can be reverted to a previous version.
@vdboor how hard do you think this one?
Reversion support would be great. Any tips on the roadblocks or how they might be worked around? I'd like to look into this as well. Alternatively, any other options for versioning that are compatible with fluent?
I'm not sure how hard this is, I didn't work with reversion before. There are a few things you'll have to overcome:
- Take the base model + all translations in a single "revision".
- Handle the custom content on the page, especially when combining this with fluent-contents
For the content items that fluent-contents provides, you could implement a versioning system within fluent-contents itself. Each ContentItem is linked to it's parent and we already implement multilingual support there by filtering on the 'language_code' field. It could be trivial to filter by "version" too if "0" is always the current version.
I've just discovered django-widgy last week (somewhat comparable to fluent-contents, we could even make a fluentcms-widgy package to add a "WidgyPage"). Sadly they only add versioning for the content, and not the whole page. Ideally I'd love to see the whole page getting revisioned/versioned.
I noticed that Aldryn is actively using django-parler for multilingual support, and they have written a version adapter for it. It's worth looking at, and if anyone would be able to port that into a module for parler, I'd be very happy!
See: https://github.com/aldryn/aldryn-reversion/blob/master/aldryn_reversion/admin.py https://github.com/aldryn/aldryn-reversion/blob/master/aldryn_reversion/core.py
FYI we are working on adding support for versioning of Fluent and Flat pages using django-reversion in this fork: https://github.com/ixc/django-fluent-pages/tree/reversion_support
The fork is partly based on https://github.com/aldryn/aldryn-reversion though without the support for plugins.
It is not quite mature enough to become a pull request just yet, but we are getting there and plan to help towards adding official versioning support if possible. In our environment basic versioning, revert, and restore of single-language Flat/Fluent pages is working. See the commit comments on the "reversion_support" branch for a rough guide to getting started.
While this branch is still under development any testing by, or feedback from, others in the community would be great!
@vdboor Maybe we should open a PR despite the code being very much in-flux, to have a better place to discuss? Whatever you prefer.
Wow, thanks a lot for the work so far! This will require some digging into the solution.
Interesting you took advantage of the FLUENT_PAGES_PARENT_ADMIN_MIXIN
setting.
Sorry for not paying attention to the GitHub tickets lately...
Based on looking through the code, there are a few things I wonder:
- Does this code already work with the latest django-reversion?
- Could it be made in such way that
FLUENT_PAGES_PARENT_ADMIN_MIXIN
doesn't have to be used? For example, automatically mix in the reversion support when reversion is available? Or would that cause too many issues? - Is there ever a reason to disable reversion support for pages when the rest of your project runs on reversion?
- The
ReversionRealtimeSignalProcessor
seems like a workaround. Can this be fixed for real, or should this be upstreamed to django-parler? - If this feature doesn't need to be in
INSTALLED_APPS
, it would make sense to upstream it to django-fluent-utils, so it can be integrated into django-fluent-blogs and django-fluent-faq too. - What is the relation with django-model-publisher? It seems like a nice replacement of the default status field.
I'm fine with keeping the discussion here, it's a 50/50 choice imho.
btw, if you have some sites running on django-fluent, I'd love to add those to https://django-fluent.org/showcase/ :)
Hi @vdboor thanks for the comments and questions. Sorry for the late reply, the code branch has been in a lot of flux and I wanted to get it (somewhat) more stable before replying.
Based on your feedback I have updated the integration to automatically support versioning by modifying the relevant admin classes to extend the integration versions if reversion
is installed and enabled. I tried to do this in minimally intrusive way, and it works but is fairly hacky. For example
- Does this code already work with the latest django-reversion?
It uses the latest django-reversion except for one outstanding pending fix for polymorphic page links, see https://github.com/etianen/django-reversion/pull/438 The maintainer of django-reversion Dave Hall has been super-helpful in merging and updating the library to work better in complex situations like for fluent pages - Automatically mix in reversion support and avoid requiring the
FLUENT_PAGES_PARENT_ADMIN_MIXIN
setting
Done - Is there ever a reason to disable reversion support for pages when the rest of your project runs on reversion?
Maybe not. I can imagine situations where some parts of a site usereversion
but you don't want fluent pages to do so, though it's unlikely. I would like to keep theFLUENT_PAGES_DISABLE_REVERSION
optional setting for now at least, as it provides a quick way to disable the versioning support to see whether or not it is causing a specific issue - The
ReversionRealtimeSignalProcessor
seems like a workaround. Can this be fixed for real, or should this be upstreamed to django-parler?
The term "workaround" is too kind, it's more of a hack. It would be good to fix it for real, though I'm not sure of the best way to do this. The problem may actually have been solved by recent changes in django-reversion to do strict revert/restore operations – I haven't had a chance to re-test - If this feature doesn't need to be in INSTALLED_APPS, it would make sense to upstream it to django-fluent-utils, so it can be integrated into django-fluent-blogs and django-fluent-faq too.
That sounds reasonable. It isn't something I will have time to take on in the near-term, but once it is working well it will hopefully be possible to relocate the bulk of the code. - What is the relation with django-model-publisher? It seems like a nice replacement of the default status field.
We are actually using the integration along with django-model-publisher though with a lot of code customisations. That could make for another good integration project in the future. - btw, if you have some sites running on django-fluent, I'd love to add those to https://django-fluent.org/showcase/ :)
Heh, nothing that's publicly visible just yet but that sounds like a good idea for when the site eventually goes live.
I should note that with the latest commit to our integration branch we are back to using the official 1.9.3 release of django-reversion, instead of our forked and patched version.