django-cms / djangocms-versioning

General purpose versioning package for Django CMS 4 and above.
Other
33 stars 29 forks source link

fix: Remove patching the django CMS core #300

Closed fsbraun closed 1 year ago

fsbraun commented 1 year ago

Description

Patching easily introduces side-effects, especially if patches happen at diverse places in the code base. This PR removes all patches to the django CMS core.

Mixin for CMSToolbar using django CMS new config option to provide a mixin for the cms.toolbar.toolbar.CMSToolbar class.

See this PR.

Creating Version instances when a versioned model is created

So far, djangocms-versioning puts the burden of creating Version objects for versioned models on the app owning the versioned models. To keep the core agnostic of versioning capabilities. This PR lets djangocms-versioning automatically create a draft Version object as soon as a versioned object is created. This is achieved by modifying the versioned model's manager's create method.

Since the Version model requires a User object the following syntax has been introduced:

MyVersionedModel.with_user(request.user).create(...)

The with_user() statement tells versioning which user the first version of that object will be assigned to and also automatically creates a Version object.

If the with_user() statement is missing, djangocms-verisoning will issue a warning and not create a Version object. It will not raise an exception to stay backwards compatible.

Patches to PageContentAdmin

These patches have been moved to the already existing VersioningCMSPageAdminMixin where they belong.

Patches to PageExtension and PageContentExtension

These can be handled by allowing to already used signals to forward changes to the extension objects to the extended objects.

Performance/feature patches

Patches which are not related to versioning functionality have been moved to the core with this PR.

Changes to Django's ORM

Versioning introduces changes to Django's handling of versioned models. These changes are, of course, retained:

Compatibility

This PR will not work with versions 4.0.x of django cms since they do not offer the CMSToolbar configuration.

Related resources

Checklist

codecov[bot] commented 1 year ago

Codecov Report

Merging #300 (7b60aca) into master (5fe601b) will increase coverage by 1.06%. The diff coverage is 86.88%.

@@            Coverage Diff             @@
##           master     #300      +/-   ##
==========================================
+ Coverage   88.68%   89.75%   +1.06%     
==========================================
  Files          76       67       -9     
  Lines        2334     2196     -138     
  Branches      298      293       -5     
==========================================
- Hits         2070     1971      -99     
+ Misses        203      170      -33     
+ Partials       61       55       -6     
Impacted Files Coverage Δ
djangocms_versioning/cms_config.py 80.29% <79.54%> (-0.92%) :arrow_down:
djangocms_versioning/managers.py 82.35% <83.72%> (+7.35%) :arrow_up:
djangocms_versioning/datastructures.py 95.55% <100.00%> (ø)
djangocms_versioning/handlers.py 97.14% <100.00%> (+0.26%) :arrow_up:
djangocms_versioning/helpers.py 91.11% <100.00%> (+0.20%) :arrow_up:
djangocms_versioning/plugin_rendering.py 54.16% <100.00%> (+12.06%) :arrow_up:
djangocms_versioning/test_utils/factories.py 94.50% <100.00%> (+0.21%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

lgtm-com[bot] commented 1 year ago

This pull request introduces 1 alert when merging 02ceb4b2c1dc81ca993aa35750a879832a4a7ff5 into 31f14df4a7624d4be60282c11ec38f0108026e69 - view on LGTM.com

new alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed :rocket:. For more information, please check out our post on the GitHub blog.