Skip to content

Commit

Permalink
fix: Remove patching the django CMS core (#300)
Browse files Browse the repository at this point in the history
* Replace monkey patching by configuation
* test against modified core
* Remove monkeypatches
* Update tests
* Remove unnecessary get_admin_model_object_by_id
* Add: with_user test
* Update docs
  • Loading branch information
fsbraun authored Dec 29, 2022
1 parent 5fe601b commit 4567b34
Show file tree
Hide file tree
Showing 28 changed files with 601 additions and 731 deletions.
18 changes: 9 additions & 9 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ jobs:
matrix:
python-version: [ 3.8, 3.9, "3.10", "3.11" ] # latest release minus two
requirements-file: [
dj32_cms40.txt,
dj40_cms40.txt,
dj41_cms40.txt,
dj32_cms41.txt,
dj40_cms41.txt,
dj41_cms41.txt,
]

steps:
Expand Down Expand Up @@ -45,9 +45,9 @@ jobs:
matrix:
python-version: [ 3.8, 3.9, "3.10", "3.11" ] # latest release minus two
requirements-file: [
dj32_cms40.txt,
dj40_cms40.txt,
dj41_cms40.txt,
dj32_cms41.txt,
dj40_cms41.txt,
dj41_cms41.txt,
]

services:
Expand Down Expand Up @@ -90,9 +90,9 @@ jobs:
matrix:
python-version: [ 3.8, 3.9, "3.10", "3.11" ] # latest release minus two
requirements-file: [
dj32_cms40.txt,
dj40_cms40.txt,
dj41_cms40.txt,
dj32_cms41.txt,
dj40_cms41.txt,
dj41_cms41.txt,
]

services:
Expand Down
13 changes: 12 additions & 1 deletion djangocms_versioning/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,26 @@ class VersioningConfig(AppConfig):
verbose_name = _("django CMS Versioning")

def ready(self):
from cms.models import contentmodels, fields
from cms.signals import post_obj_operation, post_placeholder_operation

from . import monkeypatch # noqa: F401
from .handlers import (
update_modified_date,
update_modified_date_for_pagecontent,
update_modified_date_for_placeholder_source,
)
from .helpers import is_content_editable

# Add check to PlaceholderRelationField
fields.PlaceholderRelationField.default_checks += [is_content_editable]

# Remove uniqueness constraint from PageContent model to allow for different versions
pagecontent_unique_together = tuple(
set(contentmodels.PageContent._meta.unique_together) - set((("language", "page"),))
)
contentmodels.PageContent._meta.unique_together = pagecontent_unique_together

# Connect signals
post_save.connect(update_modified_date, dispatch_uid="versioning")
post_placeholder_operation.connect(
update_modified_date_for_placeholder_source, dispatch_uid="versioning"
Expand Down
86 changes: 79 additions & 7 deletions djangocms_versioning/cms_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,36 @@
from django.conf import settings
from django.contrib import messages
from django.contrib.admin.utils import flatten_fieldsets
from django.core.exceptions import ImproperlyConfigured
from django.http import HttpResponseForbidden
from django.core.exceptions import (
ImproperlyConfigured,
ObjectDoesNotExist,
PermissionDenied,
)
from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden
from django.utils.encoding import force_str
from django.utils.functional import cached_property
from django.utils.translation import gettext_lazy as _

from cms.app_base import CMSAppConfig, CMSAppExtension
from cms.models import PageContent, Placeholder
from cms.utils.i18n import get_language_tuple
from cms.utils import get_language_from_request
from cms.utils.i18n import get_language_list, get_language_tuple
from cms.utils.plugins import copy_plugins_to_placeholder

from . import indicators
from . import indicators, versionables
from .admin import VersioningAdminMixin
from .datastructures import BaseVersionableItem, VersionableItem
from .exceptions import ConditionFailed
from .helpers import (
get_latest_admin_viewable_page_content,
inject_generic_relation_to_version,
register_versionadmin_proxy,
replace_admin_for_models,
replace_default_manager,
replace_manager,
)
from .managers import AdminManagerMixin, PublishedContentManagerMixin
from .models import Version
from .plugin_rendering import CMSToolbarVersioningMixin


class VersioningCMSExtension(CMSAppExtension):
Expand Down Expand Up @@ -133,7 +142,9 @@ def handle_content_model_manager(self, cms_config):
one inheriting from PublishedContentManagerMixin.
"""
for versionable in cms_config.versioning:
replace_default_manager(versionable.content_model)
replace_manager(versionable.content_model, "objects", PublishedContentManagerMixin)
replace_manager(versionable.content_model, "admin_manager", AdminManagerMixin,
_group_by_key=[versionable.grouper_field_name] + list(versionable.extra_grouping_fields))

def handle_admin_field_modifiers(self, cms_config):
"""Allows for the transformation of a given field in the ExtendedVersionAdminMixin
Expand Down Expand Up @@ -183,7 +194,8 @@ def copy_page_content(original_content):
if field.name not in (PageContent._meta.pk.name, "creation_date")
}

new_content = PageContent.objects.create(**content_fields)
# Use original manager to not create a new Version object here
new_content = PageContent._original_manager.create(**content_fields)

# Copy placeholders
new_placeholders = []
Expand Down Expand Up @@ -282,6 +294,64 @@ def get_form(self, request, obj=None, **kwargs):
form.declared_fields[f_name].widget.attrs["readonly"] = True
return form

def get_queryset(self, request):
urls = ("cms_pagecontent_get_tree",)
queryset = super().get_queryset(request)
if request.resolver_match.url_name in urls:
versionable = versionables.for_content(queryset.model)

# TODO: Improve the grouping filters to use anything defined in the
# apps versioning config extra_grouping_fields
grouping_filters = {}
if 'language' in versionable.extra_grouping_fields:
grouping_filters['language'] = get_language_from_request(request)

return queryset.filter(pk__in=versionable.distinct_groupers(**grouping_filters))
return queryset

# CAVEAT:
# - PageContent contains the template, this can differ for each language,
# it is assumed that templates would be the same when copying from one language to another
# FIXME: The long term solution will require knowing:
# - why this view is an ajax call
# - where it should live going forwards (cms vs versioning)
# - A better way of making the feature extensible / modifiable for versioning
def copy_language(self, request, object_id):
target_language = request.POST.get('target_language')

# CAVEAT: Avoiding self.get_object because it sets the page cache,
# We don't want a draft showing to a regular site visitor!
# source_page_content = self.get_object(request, object_id=object_id)
source_page_content = PageContent._original_manager.get(pk=object_id)

if source_page_content is None:
raise self._get_404_exception(object_id)

page = source_page_content.page

if not target_language or target_language not in get_language_list(site_id=page.node.site_id):
return HttpResponseBadRequest(force_str(_("Language must be set to a supported language!")))

target_page_content = get_latest_admin_viewable_page_content(page, target_language)

# First check that we are able to edit the target
if not self.has_change_permission(request, obj=target_page_content):
raise PermissionDenied

for placeholder in source_page_content.get_placeholders():
# Try and get a matching placeholder, only if it exists
try:
target = target_page_content.get_placeholders().get(slot=placeholder.slot)
except ObjectDoesNotExist:
continue

plugins = placeholder.get_plugins_list(source_page_content.language)

if not target.has_add_plugins_permission(request.user, plugins):
return HttpResponseForbidden(force_str(_('You do not have permission to copy these plugins.')))
copy_plugins_to_placeholder(plugins, target, language=target_language)
return HttpResponse("ok")

def change_innavigation(self, request, object_id):
page_content = self.get_object(request, object_id=object_id)
version = Version.objects.get_for_content(page_content)
Expand All @@ -296,6 +366,7 @@ def change_innavigation(self, request, object_id):
class VersioningCMSConfig(CMSAppConfig):
"""Implement versioning for core cms models
"""
cms_enabled = True
djangocms_versioning_enabled = getattr(
settings, "VERSIONING_CMS_MODELS_ENABLED", True
)
Expand All @@ -314,6 +385,7 @@ class VersioningCMSConfig(CMSAppConfig):
content_admin_mixin=VersioningCMSPageAdminMixin,
)
]
cms_toolbar_mixin = CMSToolbarVersioningMixin
PageContent.add_to_class("is_editable", indicators.is_editable)
PageContent.add_to_class("content_indicator", indicators.content_indicator)
PageContent.add_to_class("__bool__", lambda self: self.versions.exists())
6 changes: 5 additions & 1 deletion djangocms_versioning/datastructures.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,9 @@ def default_copy(original_content):
would expect a version to copy some of its related objects as well).
In such cases a custom copy method must be defined and specified in
cms_config.py
NOTE: A custom copy method will need to use the content model's
_original_manage to create only a content model object and not also a Version object.
"""
content_model = original_content.__class__
content_fields = {
Expand All @@ -234,4 +237,5 @@ def default_copy(original_content):
# don't copy primary key because we're creating a new obj
if content_model._meta.pk.name != field.name
}
return content_model.objects.create(**content_fields)
# Use original manager to avoid creating a new draft version here!
return content_model._original_manager.create(**content_fields)
3 changes: 3 additions & 0 deletions djangocms_versioning/handlers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from django.utils import timezone

from cms.extensions.models import BaseExtension
from cms.operations import (
ADD_PLUGIN,
ADD_PLUGINS_FROM_PLACEHOLDER,
Expand All @@ -17,6 +18,8 @@


def _update_modified(instance):
if isinstance(instance, BaseExtension):
instance = instance.extended_object
if instance and _cms_extension().is_content_model_versioned(instance.__class__):
try:
version = Version.objects.get_for_content(instance)
Expand Down
28 changes: 16 additions & 12 deletions djangocms_versioning/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.contrib import admin
from django.contrib.contenttypes.fields import GenericRelation
from django.contrib.contenttypes.models import ContentType
from django.db import models
from django.db.models.sql.where import WhereNode
from django.urls import reverse

Expand All @@ -15,7 +16,6 @@

from . import versionables
from .constants import DRAFT, PUBLISHED
from .managers import PublishedContentManagerMixin
from .versionables import _cms_extension


Expand Down Expand Up @@ -105,30 +105,34 @@ def get_queryset(self, request):
admin_site.register(versionable.version_model_proxy, ProxiedAdmin)


def published_content_manager_factory(manager):
"""A class factory returning manager class with overriden
def manager_factory(manager, prefix, mixin):
"""A class factory returning a manager class with an added mixin to override for
versioning functionality.
:param manager: Existing manager class
:return: A subclass of `PublishedContentManagerMixin` and `manager`
"""
return type(
"Published" + manager.__name__,
(PublishedContentManagerMixin, manager),
prefix + manager.__name__,
(mixin, manager),
{"use_in_migrations": False},
)


def replace_default_manager(model):
if isinstance(model.objects, PublishedContentManagerMixin):
def replace_manager(model, manager, mixin, **kwargs):
if hasattr(model, manager) and isinstance(getattr(model, manager), mixin):
return
original_manager = model.objects.__class__
manager = published_content_manager_factory(original_manager)()
original_manager = getattr(model, manager).__class__ if hasattr(model, manager) else models.Manager
manager_object = manager_factory(original_manager, "Versioned", mixin)()
for key, value in kwargs.items():
setattr(manager_object, key, value)
model._meta.local_managers = [
manager for manager in model._meta.local_managers if manager.name != "objects"
mngr for mngr in model._meta.local_managers if mngr.name != manager
]
model.add_to_class("objects", manager)
model.add_to_class("_original_manager", original_manager())
model.add_to_class(manager, manager_object)
if manager == "objects":
# only safe the original default manager
model.add_to_class(f'_original_{"manager" if manager == "objects" else manager}', original_manager())


def inject_generic_relation_to_version(model):
Expand Down
71 changes: 71 additions & 0 deletions djangocms_versioning/managers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
import warnings
from copy import copy
from itertools import groupby

from django.contrib.auth import get_user_model
from django.db import models

from . import constants
from .constants import PUBLISHED
from .models import Version


class PublishedContentManagerMixin:
Expand All @@ -12,3 +21,65 @@ def get_queryset(self):
if not self.versioning_enabled:
return queryset
return queryset.filter(versions__state=PUBLISHED)

def create(self, *args, **kwargs):
obj = super().create(*args, **kwargs)
created_by = kwargs.get("created_by", None)
if not isinstance(created_by, get_user_model()):
created_by = getattr(self, "_user", None)
if created_by:
Version.objects.create(content=obj, created_by=created_by)
else:
warnings.warn(
f"No user has been supplied when creating a new {obj.__class__.__name__} object. "
f"No version could be created. Make sure that the creating code also creates a "
f"Version objects or use {obj.__class__.__name__}.objects.with_user(user).create(...)",
UserWarning, stacklevel=2,
)
return obj

def with_user(self, user):
if not isinstance(user, get_user_model()) and user is not None:
import inspect

curframe = inspect.currentframe()
callframe = inspect.getouterframes(curframe, 2)
calling_function = callframe[1][3]
raise ValueError(
f"With versioning enabled, {calling_function} requires a {get_user_model().__name__} instance "
f"to be passed as created_by argument"
)
new_manager = copy(self)
new_manager._user = user
return new_manager


class AdminQuerySet(models.QuerySet):
def _chain(self):
# Also clone group by key when chaining querysets!
clone = super()._chain()
clone._group_by_key = self._group_by_key
return clone

def current_content_iterator(self, **kwargs):
"""Returns generator (not a queryset) over current content versions. Current versions are either draft
versions or published versions (in that order)"""
qs = self.filter(versions__state__in=("draft", "published"))\
.order_by(*self._group_by_key)\
.prefetch_related("versions")
for grp, version_content in groupby(
qs,
lambda x: tuple(map(lambda key: getattr(x, key), self._group_by_key)) # get group key fields
):
first, second = next(version_content), next(version_content, None) # Max 2 results per group possible
yield first if second is None or first.versions.first().state == constants.DRAFT else second


class AdminManagerMixin:
versioning_enabled = True
_group_by_key = []

def get_queryset(self):
qs = AdminQuerySet(self.model, using=self._db)
qs._group_by_key = self._group_by_key
return qs
10 changes: 0 additions & 10 deletions djangocms_versioning/monkeypatch/__init__.py

This file was deleted.

Loading

0 comments on commit 4567b34

Please sign in to comment.