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

fix: Remove patching of PageContent by djangocms-versioning #7446

Merged
merged 22 commits into from
Dec 29, 2022

Conversation

fsbraun
Copy link
Sponsor Member

@fsbraun fsbraun commented Nov 25, 2022

Description

Patching easily introduces side-effects, especially if patches happen at diverse places in the code base. This PR removes the need for djangocms-versioning to patch the core.

Additional interface through CMSConfig

The toolbar is at the heart of django CMS. This PR allows apps to register a mixin for the cms.toolbar.toolbar.CMSToolbar class. All it has to do is to set

class MyCMSConfig(CMSAppConfig)
    cms_enabled = True
    cms_toolbar_mixin = MyMixinClass

djangocms-versioning (and other apps) can use this config instead of patching CMSToolbar. (Example for other use cases: Change the login process through the toolbar)

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 (django-cms/djangocms-versioning#300) 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.

This removes the need to patch the creation of PageContent objects.

Patches independent of versioning

Quite a few patches of djangocms-versioning are not connected to the versioning capability. They typically are improvement of the core and have been moved into the core with this PR.

Ability to "see" unpublished PageContent objects

A number of patches by djangocms-versioning enable to core to see unpublished PageContent objects. An example are the Page wizards which need to be able to access newly created PageContent objects. This is done by using the Django manager _base_manager in these selected cases.

Related resources

Checklist

  • I have opened this pull request against develop-4
  • I have added or modified the tests when changing logic
  • I have followed the conventional commits guidelines to add meaningful information into the changelog
  • I have read the contribution guidelines and I have joined #workgroup-pr-review on Slack to find a “pr review buddy” who is going to review my pull request.

Copy link
Contributor

@Aiky30 Aiky30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprisingly a lot of old references to draft still in places. Well found.

Some comments, 2 of any concern:
Page content creation user
Removal of force language, could be replaced / the original requirement sourced back to the wizard changes.

cms/cms_toolbars.py Outdated Show resolved Hide resolved
cms/models/contentmodels.py Show resolved Hide resolved
cms/utils/moderator.py Show resolved Hide resolved
menus/menu_pool.py Outdated Show resolved Hide resolved
cms/api.py Show resolved Hide resolved
cms/api.py Outdated Show resolved Hide resolved
cms/api.py Outdated Show resolved Hide resolved
cms/api.py Outdated Show resolved Hide resolved
cms/utils/helpers.py Show resolved Hide resolved
fsbraun and others added 6 commits December 14, 2022 20:55
Co-authored-by: Andrew Aikman <Aiky30@users.noreply.github.com>
Co-authored-by: Andrew Aikman <Aiky30@users.noreply.github.com>
Co-authored-by: Andrew Aikman <Aiky30@users.noreply.github.com>
Fix: flake8 line length
Copy link
Contributor

@Aiky30 Aiky30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Read through it thoroughly once again. Nice to have a proper solution to the patching finally.

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

Successfully merging this pull request may close these issues.

[BUG] CMS V4: Copying and pasting an unpublished page renders the page tree unusable
3 participants