From 97ea44636dd258cd75e09bac29ff44cef58a0853 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 23 Nov 2022 00:49:01 +0100 Subject: [PATCH 01/19] Replace monkey patching by configuation --- djangocms_versioning/cms_config.py | 76 ++++++++++++++- djangocms_versioning/datastructures.py | 6 +- djangocms_versioning/helpers.py | 2 + djangocms_versioning/managers.py | 29 ++++++ djangocms_versioning/monkeypatch/__init__.py | 4 - djangocms_versioning/monkeypatch/admin.py | 92 ------------------- djangocms_versioning/monkeypatch/menu.py | 23 ----- djangocms_versioning/monkeypatch/page.py | 64 +------------ .../monkeypatch/templatetags.py | 21 ----- djangocms_versioning/monkeypatch/toolbar.py | 26 ------ djangocms_versioning/plugin_rendering.py | 14 +++ djangocms_versioning/test_utils/factories.py | 16 +++- tests/test_monkeypatch.py | 3 +- 13 files changed, 137 insertions(+), 239 deletions(-) delete mode 100644 djangocms_versioning/monkeypatch/admin.py delete mode 100644 djangocms_versioning/monkeypatch/menu.py delete mode 100644 djangocms_versioning/monkeypatch/templatetags.py delete mode 100644 djangocms_versioning/monkeypatch/toolbar.py diff --git a/djangocms_versioning/cms_config.py b/djangocms_versioning/cms_config.py index 70c99f26..20f4088d 100644 --- a/djangocms_versioning/cms_config.py +++ b/djangocms_versioning/cms_config.py @@ -1,24 +1,30 @@ import collections +from cms.utils.plugins import copy_plugins_to_placeholder from django.conf import settings from django.contrib.admin.utils import flatten_fieldsets -from django.core.exceptions import ImproperlyConfigured +from django.core.exceptions import ImproperlyConfigured, PermissionDenied, ObjectDoesNotExist +from django.http import HttpResponseBadRequest, HttpResponseForbidden, HttpResponse +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_tuple, get_language_list +from . import versionables from .admin import VersioningAdminMixin from .datastructures import BaseVersionableItem, VersionableItem from .helpers import ( inject_generic_relation_to_version, register_versionadmin_proxy, replace_admin_for_models, - replace_default_manager, + replace_default_manager, get_latest_admin_viewable_page_content, ) from .models import Version +from .plugin_rendering import CMSToolbarVersioningMixin class VersioningCMSExtension(CMSAppExtension): @@ -178,7 +184,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 = [] @@ -277,10 +284,70 @@ 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: This monkeypatch exists to allow the language copy feature to work + # 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") + class VersioningCMSConfig(CMSAppConfig): """Implement versioning for core cms models """ + cms_enabled = True djangocms_versioning_enabled = getattr( settings, "VERSIONING_CMS_MODELS_ENABLED", True ) @@ -299,3 +366,4 @@ class VersioningCMSConfig(CMSAppConfig): content_admin_mixin=VersioningCMSPageAdminMixin, ) ] + cms_toolbar_mixin = CMSToolbarVersioningMixin diff --git a/djangocms_versioning/datastructures.py b/djangocms_versioning/datastructures.py index 0aa45f77..3597312e 100644 --- a/djangocms_versioning/datastructures.py +++ b/djangocms_versioning/datastructures.py @@ -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 = { @@ -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) diff --git a/djangocms_versioning/helpers.py b/djangocms_versioning/helpers.py index 8e4ed710..48e10b3a 100644 --- a/djangocms_versioning/helpers.py +++ b/djangocms_versioning/helpers.py @@ -129,6 +129,8 @@ def replace_default_manager(model): ] model.add_to_class("objects", manager) model.add_to_class("_original_manager", original_manager()) + model.add_to_class("get_admin_model_object_by_id", + classmethod(lambda cls, obj_id: cls._original_manager.get(pk=obj_id))) def inject_generic_relation_to_version(model): diff --git a/djangocms_versioning/managers.py b/djangocms_versioning/managers.py index 174aac75..8f62ecc8 100644 --- a/djangocms_versioning/managers.py +++ b/djangocms_versioning/managers.py @@ -1,4 +1,11 @@ +import warnings + +from django.contrib.auth import get_user_model + +from cms.utils.permissions import _thread_locals + from .constants import PUBLISHED +from .models import Version class PublishedContentManagerMixin: @@ -12,3 +19,25 @@ 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) or getattr(_thread_locals, "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()): + raise TypeError(f"User for with_user must be of type {get_user_model().__name__}") + self._user = user + return self diff --git a/djangocms_versioning/monkeypatch/__init__.py b/djangocms_versioning/monkeypatch/__init__.py index 22d7e53f..86d2433d 100644 --- a/djangocms_versioning/monkeypatch/__init__.py +++ b/djangocms_versioning/monkeypatch/__init__.py @@ -1,10 +1,6 @@ from . import ( # noqa: F401 - admin, checks, extensions, - menu, page, - templatetags, - toolbar, wizard, ) diff --git a/djangocms_versioning/monkeypatch/admin.py b/djangocms_versioning/monkeypatch/admin.py deleted file mode 100644 index a4f1024a..00000000 --- a/djangocms_versioning/monkeypatch/admin.py +++ /dev/null @@ -1,92 +0,0 @@ -from django.core.exceptions import ObjectDoesNotExist, PermissionDenied -from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden -from django.utils.encoding import force_str -from django.utils.translation import gettext_lazy as _ - -from cms import admin -from cms.models import PageContent -from cms.utils import get_language_from_request, helpers -from cms.utils.i18n import get_language_list -from cms.utils.plugins import copy_plugins_to_placeholder - -from djangocms_versioning import versionables -from djangocms_versioning.helpers import get_latest_admin_viewable_page_content - - -def get_queryset(func): - def inner(self, request): - urls = ("cms_pagecontent_get_tree",) - queryset = func(self, 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 - - return inner - - -admin.pageadmin.PageContentAdmin.get_queryset = get_queryset( - admin.pageadmin.PageContentAdmin.get_queryset -) - - -def get_admin_model_object_by_id(model_class, obj_id): - return model_class._original_manager.get(pk=obj_id) - - -helpers.get_admin_model_object_by_id = get_admin_model_object_by_id - - -# 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: This monkeypatch exists to allow the language copy feature to work -# 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") - - -admin.pageadmin.PageContentAdmin.copy_language = copy_language diff --git a/djangocms_versioning/monkeypatch/menu.py b/djangocms_versioning/monkeypatch/menu.py deleted file mode 100644 index 4ec9995f..00000000 --- a/djangocms_versioning/monkeypatch/menu.py +++ /dev/null @@ -1,23 +0,0 @@ -from cms.toolbar.utils import get_toolbar_from_request -from cms.utils.conf import get_cms_setting -from menus.menu_pool import MenuRenderer - - -def menu_renderer_cache_key(self): - prefix = get_cms_setting("CACHE_PREFIX") - - key = "%smenu_nodes_%s_%s" % (prefix, self.request_language, self.site.pk) - - if self.request.user.is_authenticated: - key += "_%s_user" % self.request.user.pk - - request_toolbar = get_toolbar_from_request(self.request) - - if request_toolbar.edit_mode_active or request_toolbar.preview_mode_active: - key += ":draft" - else: - key += ":public" - return key - - -MenuRenderer.cache_key = property(menu_renderer_cache_key) # noqa: E305 diff --git a/djangocms_versioning/monkeypatch/page.py b/djangocms_versioning/monkeypatch/page.py index b02a64c6..3b99ddfe 100644 --- a/djangocms_versioning/monkeypatch/page.py +++ b/djangocms_versioning/monkeypatch/page.py @@ -1,66 +1,4 @@ -from django.apps import apps -from django.contrib.auth import get_user_model - -from cms import api -from cms.models import Placeholder, pagemodel, titlemodels -from cms.utils.permissions import _thread_locals - -from djangocms_versioning.models import Version - - -User = get_user_model() - -cms_extension = apps.get_app_config("cms").cms_extension - - -def _get_title_cache(func): - def inner(self, language, fallback, force_reload): - prefetch_cache = getattr(self, "_prefetched_objects_cache", {}) - cached_page_content = prefetch_cache.get("pagecontent_set", []) - for page_content in cached_page_content: - self.title_cache[page_content.language] = page_content - language = func(self, language, fallback, force_reload) - return language - - return inner - - -pagemodel.Page._get_title_cache = _get_title_cache( - pagemodel.Page._get_title_cache -) # noqa: E305 - - -def get_placeholders(func): - def inner(self, language): - page_content = self.get_title_obj(language) - return Placeholder.objects.get_for_obj(page_content) - - return inner - - -pagemodel.Page.get_placeholders = get_placeholders( - pagemodel.Page.get_placeholders -) # noqa: E305 - - -def create_title(func): - def inner(language, title, page, **kwargs): - created_by = kwargs.get("created_by") - if not isinstance(created_by, User): - created_by = getattr(_thread_locals, "user", None) - assert created_by is not None, ( - "With versioning enabled, create_title requires a User instance" - " to be passed as created_by parameter" - ) - page_content = func(language, title, page, **kwargs) - Version.objects.create(content=page_content, created_by=created_by) - return page_content - - return inner - - -api.create_title = create_title(api.create_title) # noqa: E305 - +from cms.models import titlemodels pagecontent_unique_together = tuple( set(titlemodels.PageContent._meta.unique_together) - set((("language", "page"),)) diff --git a/djangocms_versioning/monkeypatch/templatetags.py b/djangocms_versioning/monkeypatch/templatetags.py deleted file mode 100644 index 709169e5..00000000 --- a/djangocms_versioning/monkeypatch/templatetags.py +++ /dev/null @@ -1,21 +0,0 @@ -from cms.templatetags import cms_admin -from cms.utils.urlutils import admin_reverse - - -@cms_admin.register.simple_tag(takes_context=False) -def get_admin_url_for_language(page, language): - # TODO Perhaps modify get_languages so that it returns - # only published languages, or add a separate function - # to do so in places like this. - existing_language = language in page.get_languages() - if existing_language: - page_content = page.get_title_obj(language, fallback=False) - existing_language = bool(page_content) - if not existing_language: - admin_url = admin_reverse("cms_pagecontent_add") - admin_url += "?cms_page={}&language={}".format(page.pk, language) - return admin_url - return admin_reverse("cms_pagecontent_change", args=[page_content.pk]) - - -cms_admin.get_admin_url_for_language = get_admin_url_for_language # noqa: E305 diff --git a/djangocms_versioning/monkeypatch/toolbar.py b/djangocms_versioning/monkeypatch/toolbar.py deleted file mode 100644 index bc2a7d43..00000000 --- a/djangocms_versioning/monkeypatch/toolbar.py +++ /dev/null @@ -1,26 +0,0 @@ -from functools import lru_cache - -from cms.toolbar import toolbar - -from djangocms_versioning.plugin_rendering import ( - VersionContentRenderer, - VersionStructureRenderer, -) - - -@lru_cache(16) -def content_renderer(self): - return VersionContentRenderer(request=self.request) - - -toolbar.CMSToolbar.content_renderer = property(content_renderer) # noqa: E305 - - -@lru_cache(16) -def structure_renderer(self): - return VersionStructureRenderer(request=self.request) - - -toolbar.CMSToolbar.structure_renderer = property( - structure_renderer -) # noqa: E305 diff --git a/djangocms_versioning/plugin_rendering.py b/djangocms_versioning/plugin_rendering.py index db38ad42..7571bbaf 100644 --- a/djangocms_versioning/plugin_rendering.py +++ b/djangocms_versioning/plugin_rendering.py @@ -1,3 +1,5 @@ +from functools import lru_cache + from cms.plugin_rendering import ContentRenderer, StructureRenderer from cms.utils.placeholder import rescan_placeholders_for_obj @@ -68,3 +70,15 @@ class VersionStructureRenderer(StructureRenderer): def render_plugin(self, instance, page=None): prefetch_versioned_related_objects(instance, self.toolbar) return super().render_plugin(instance, page) + + +class CMSToolbarVersioningMixin: + @property + @lru_cache(16) + def content_renderer(self): + return VersionContentRenderer(request=self.request) + + @property + @lru_cache(16) + def structure_renderer(self): + return VersionStructureRenderer(request=self.request) diff --git a/djangocms_versioning/test_utils/factories.py b/djangocms_versioning/test_utils/factories.py index 140ae8a9..5516bf4e 100644 --- a/djangocms_versioning/test_utils/factories.py +++ b/djangocms_versioning/test_utils/factories.py @@ -54,6 +54,12 @@ class Meta: abstract = True +class AbstractContentFactory(factory.django.DjangoModelFactory): + @classmethod + def _get_manager(cls, model_class): + return model_class._base_manager + + class PollFactory(factory.django.DjangoModelFactory): name = FuzzyText(length=6) @@ -69,6 +75,10 @@ class PollContentFactory(factory.django.DjangoModelFactory): class Meta: model = PollContent + @classmethod + def _get_manager(cls, model_class): + return model_class._base_manager + class PollVersionFactory(AbstractVersionFactory): content = factory.SubFactory(PollContentFactory) @@ -105,7 +115,7 @@ class Meta: model = BlogPost -class BlogContentFactory(factory.django.DjangoModelFactory): +class BlogContentFactory(AbstractContentFactory): blogpost = factory.SubFactory(BlogPostFactory) language = FuzzyChoice(["en", "fr", "it"]) text = FuzzyText(length=24) @@ -139,7 +149,7 @@ class Meta: model = IncorrectBlogPost -class IncorrectBlogContentFactory(factory.django.DjangoModelFactory): +class IncorrectBlogContentFactory(AbstractContentFactory): blogpost = factory.SubFactory(IncorrectBlogPostFactory) text = FuzzyText(length=24) @@ -193,7 +203,7 @@ class Meta: model = Page -class PageContentFactory(factory.django.DjangoModelFactory): +class PageContentFactory(AbstractContentFactory): page = factory.SubFactory(PageFactory) language = FuzzyChoice(["en", "fr", "it"]) title = FuzzyText(length=12) diff --git a/tests/test_monkeypatch.py b/tests/test_monkeypatch.py index 53a0e930..a1adbe33 100644 --- a/tests/test_monkeypatch.py +++ b/tests/test_monkeypatch.py @@ -174,10 +174,9 @@ def test_get_admin_model_object(self): With the mocked get_admin_model_object_by_id it is able to fetch objects in draft mode. """ - from cms.utils.helpers import get_admin_model_object_by_id version = PageVersionFactory() - content = get_admin_model_object_by_id(PageContent, version.content.pk) + content = PageContent.get_admin_model_object_by_id(version.content.pk) self.assertEqual(version.state, 'draft') self.assertEqual(content.pk, version.content.pk) From 123df4c6d3c59c8b57dfba0abd3475c54fb615dc Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 23 Nov 2022 00:53:43 +0100 Subject: [PATCH 02/19] fix isort --- djangocms_versioning/cms_config.py | 15 ++++++++++----- djangocms_versioning/monkeypatch/__init__.py | 7 +------ djangocms_versioning/monkeypatch/page.py | 1 + 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/djangocms_versioning/cms_config.py b/djangocms_versioning/cms_config.py index 20f4088d..8228df01 100644 --- a/djangocms_versioning/cms_config.py +++ b/djangocms_versioning/cms_config.py @@ -1,10 +1,13 @@ import collections -from cms.utils.plugins import copy_plugins_to_placeholder from django.conf import settings from django.contrib.admin.utils import flatten_fieldsets -from django.core.exceptions import ImproperlyConfigured, PermissionDenied, ObjectDoesNotExist -from django.http import HttpResponseBadRequest, HttpResponseForbidden, HttpResponse +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 _ @@ -12,16 +15,18 @@ from cms.app_base import CMSAppConfig, CMSAppExtension from cms.models import PageContent, Placeholder from cms.utils import get_language_from_request -from cms.utils.i18n import get_language_tuple, get_language_list +from cms.utils.i18n import get_language_list, get_language_tuple +from cms.utils.plugins import copy_plugins_to_placeholder from . import versionables from .admin import VersioningAdminMixin from .datastructures import BaseVersionableItem, VersionableItem from .helpers import ( + get_latest_admin_viewable_page_content, inject_generic_relation_to_version, register_versionadmin_proxy, replace_admin_for_models, - replace_default_manager, get_latest_admin_viewable_page_content, + replace_default_manager, ) from .models import Version from .plugin_rendering import CMSToolbarVersioningMixin diff --git a/djangocms_versioning/monkeypatch/__init__.py b/djangocms_versioning/monkeypatch/__init__.py index 86d2433d..de27545c 100644 --- a/djangocms_versioning/monkeypatch/__init__.py +++ b/djangocms_versioning/monkeypatch/__init__.py @@ -1,6 +1 @@ -from . import ( # noqa: F401 - checks, - extensions, - page, - wizard, -) +from . import checks, extensions, page, wizard # noqa: F401 diff --git a/djangocms_versioning/monkeypatch/page.py b/djangocms_versioning/monkeypatch/page.py index 3b99ddfe..d3e48dba 100644 --- a/djangocms_versioning/monkeypatch/page.py +++ b/djangocms_versioning/monkeypatch/page.py @@ -1,5 +1,6 @@ from cms.models import titlemodels + pagecontent_unique_together = tuple( set(titlemodels.PageContent._meta.unique_together) - set((("language", "page"),)) ) From d343911f59b3e883725301023e78b2e15592841a Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 23 Nov 2022 00:55:27 +0100 Subject: [PATCH 03/19] test against modified core --- tests/requirements/requirements_base.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/requirements/requirements_base.txt b/tests/requirements/requirements_base.txt index 6bcbdf6b..70897dc4 100644 --- a/tests/requirements/requirements_base.txt +++ b/tests/requirements/requirements_base.txt @@ -12,5 +12,5 @@ pyflakes>=2.1.1 python-dateutil # Unreleased django-cms 4.0 compatible packages -https://github.com/django-cms/django-cms/tarball/develop-4#egg=django-cms +git+https://github.com/fsbraun/django-cms@fix/remove-patching#egg=django-cms https://github.com/divio/djangocms-text-ckeditor/tarball/support/4.0.x#egg=djangocms-text-ckeditor From 4eef57fba37bfed6b88c07827e12d86cf2b4c573 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 23 Nov 2022 10:11:10 +0100 Subject: [PATCH 04/19] Remove monkeypatches --- djangocms_versioning/apps.py | 13 ++- djangocms_versioning/cms_config.py | 3 +- djangocms_versioning/handlers.py | 3 + djangocms_versioning/monkeypatch/__init__.py | 1 - djangocms_versioning/monkeypatch/checks.py | 6 -- .../monkeypatch/extensions.py | 101 ------------------ djangocms_versioning/monkeypatch/page.py | 7 -- djangocms_versioning/monkeypatch/wizard.py | 39 ------- 8 files changed, 16 insertions(+), 157 deletions(-) delete mode 100644 djangocms_versioning/monkeypatch/__init__.py delete mode 100644 djangocms_versioning/monkeypatch/checks.py delete mode 100644 djangocms_versioning/monkeypatch/extensions.py delete mode 100644 djangocms_versioning/monkeypatch/page.py delete mode 100644 djangocms_versioning/monkeypatch/wizard.py diff --git a/djangocms_versioning/apps.py b/djangocms_versioning/apps.py index 9dae394d..816fcbd9 100644 --- a/djangocms_versioning/apps.py +++ b/djangocms_versioning/apps.py @@ -8,15 +8,26 @@ class VersioningConfig(AppConfig): verbose_name = _("django CMS Versioning") def ready(self): + from cms.models import fields, titlemodels 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(titlemodels.PageContent._meta.unique_together) - set((("language", "page"),)) + ) + titlemodels.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" diff --git a/djangocms_versioning/cms_config.py b/djangocms_versioning/cms_config.py index 8228df01..067a1b3c 100644 --- a/djangocms_versioning/cms_config.py +++ b/djangocms_versioning/cms_config.py @@ -307,8 +307,7 @@ def get_queryset(self, request): # 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: This monkeypatch exists to allow the language copy feature to work - # The long term solution will require knowing: + # 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 diff --git a/djangocms_versioning/handlers.py b/djangocms_versioning/handlers.py index e2f244a5..95a41ee8 100644 --- a/djangocms_versioning/handlers.py +++ b/djangocms_versioning/handlers.py @@ -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, @@ -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) diff --git a/djangocms_versioning/monkeypatch/__init__.py b/djangocms_versioning/monkeypatch/__init__.py deleted file mode 100644 index de27545c..00000000 --- a/djangocms_versioning/monkeypatch/__init__.py +++ /dev/null @@ -1 +0,0 @@ -from . import checks, extensions, page, wizard # noqa: F401 diff --git a/djangocms_versioning/monkeypatch/checks.py b/djangocms_versioning/monkeypatch/checks.py deleted file mode 100644 index 57208f49..00000000 --- a/djangocms_versioning/monkeypatch/checks.py +++ /dev/null @@ -1,6 +0,0 @@ -from cms.models import fields - -from djangocms_versioning.helpers import is_content_editable - - -fields.PlaceholderRelationField.default_checks += [is_content_editable] diff --git a/djangocms_versioning/monkeypatch/extensions.py b/djangocms_versioning/monkeypatch/extensions.py deleted file mode 100644 index e98a32e6..00000000 --- a/djangocms_versioning/monkeypatch/extensions.py +++ /dev/null @@ -1,101 +0,0 @@ -from django.contrib.admin.options import csrf_protect_m -from django.core.exceptions import PermissionDenied -from django.http import HttpResponseRedirect -from django.urls import reverse - -from cms.extensions.admin import TitleExtensionAdmin -from cms.extensions.extension_pool import ExtensionPool -from cms.models import PageContent -from cms.utils.page_permissions import user_can_change_page - -from djangocms_versioning.handlers import _update_modified - - -def _copy_title_extensions(self, source_page, target_page, language, clone=False): - """ - djangocms-cms/extensions/admin.py, last changed in: divio/django-cms@2894ae8 - - The existing method ExtensionPool._copy_title_extensions will only ever - get published versions, we change the queries to get the latest draft version - with the _original_manager - """ - - source_title = PageContent._original_manager.filter( - page=source_page, language=language - ).first() - if target_page: - # the line below has been modified to accomodate versioning. - target_title = PageContent._original_manager.filter( - page=target_page, language=language - ).first() - else: - target_title = source_title.publisher_public - for extension in self.title_extensions: - for instance in extension.objects.filter(extended_object=source_title): - if clone: - instance.copy(target_title, language) - else: - instance.copy_to_public(target_title, language) - - -ExtensionPool._copy_title_extensions = _copy_title_extensions - - -def _save_model(self, request, obj, form, change): - """ - djangocms-cms/extensions/admin.py, last changed in: - django-cms/django-cms@61e7756a79de0db9671417b44235bbf8866c3c9f - - Ensure that the current page content object can be retrieved. A draft - object will return an empty set by default hence why we have to remove the - query manager here! - """ - if not change and 'extended_object' in request.GET: - extended_object = PageContent._original_manager.get( - pk=request.GET['extended_object'] - ) - obj.extended_object = extended_object - title = extended_object - else: - title = obj.extended_object - - if not user_can_change_page(request.user, page=title.page): - raise PermissionDenied() - - super(TitleExtensionAdmin, self).save_model(request, obj, form, change) - - # Ensure that we update the version modified date of the attached version - if title: - _update_modified(title) - - -TitleExtensionAdmin.save_model = _save_model - - -@csrf_protect_m -def _add_view(self, request, form_url='', extra_context=None): - """ - djangocms-cms/extensions/admin.py, last changed in: - django-cms/django-cms@61e7756a79de0db9671417b44235bbf8866c3c9f - - Ensure that the current page content object can be retrieved. A draft - object will return an empty set by default hence why we have to remove the - query manager here! - """ - extended_object_id = request.GET.get('extended_object', False) - if extended_object_id: - try: - title = PageContent._original_manager.get(pk=extended_object_id) - extension = self.model.objects.get(extended_object=title) - opts = self.model._meta - change_url = reverse('admin:%s_%s_change' % - (opts.app_label, opts.model_name), - args=(extension.pk,), - current_app=self.admin_site.name) - return HttpResponseRedirect(change_url) - except self.model.DoesNotExist: - pass - return super(TitleExtensionAdmin, self).add_view(request, form_url, extra_context) - - -TitleExtensionAdmin.add_view = _add_view diff --git a/djangocms_versioning/monkeypatch/page.py b/djangocms_versioning/monkeypatch/page.py deleted file mode 100644 index d3e48dba..00000000 --- a/djangocms_versioning/monkeypatch/page.py +++ /dev/null @@ -1,7 +0,0 @@ -from cms.models import titlemodels - - -pagecontent_unique_together = tuple( - set(titlemodels.PageContent._meta.unique_together) - set((("language", "page"),)) -) -titlemodels.PageContent._meta.unique_together = pagecontent_unique_together diff --git a/djangocms_versioning/monkeypatch/wizard.py b/djangocms_versioning/monkeypatch/wizard.py deleted file mode 100644 index 3cae7b08..00000000 --- a/djangocms_versioning/monkeypatch/wizard.py +++ /dev/null @@ -1,39 +0,0 @@ -from django.apps import apps - -from cms.cms_wizards import CMSPageWizard, CMSSubPageWizard -from cms.toolbar.utils import get_object_preview_url -from cms.utils.helpers import is_editable_model -from cms.wizards.wizard_base import Wizard - -from djangocms_versioning.constants import DRAFT - - -original_get_wizard_success_url = Wizard.get_success_url - - -def get_wizard_success_url(self, obj, **kwargs): # noqa: E302 - cms_extension = apps.get_app_config("djangocms_versioning").cms_extension - model = obj.__class__ - if cms_extension.is_content_model_versioned(model) and is_editable_model(model): - language = kwargs.get("language", None) - return get_object_preview_url(obj, language) - return original_get_wizard_success_url(self, obj, **kwargs) - - -Wizard.get_success_url = get_wizard_success_url # noqa: E305 - - -def get_page_wizard_success_url(self, obj, **kwargs): - language = kwargs["language"] - cms_extension = apps.get_app_config("djangocms_versioning").cms_extension - versionable_item = cms_extension.versionables_by_grouper[obj.__class__] - page_content = ( - versionable_item.for_grouper(obj) - .filter(language=language, versions__state=DRAFT) - .first() - ) - return get_wizard_success_url(self, page_content, **kwargs) - - -CMSPageWizard.get_success_url = get_page_wizard_success_url # noqa: E305 -CMSSubPageWizard.get_success_url = get_page_wizard_success_url From 1b4a87c86084533011f657dc1733a4bd3685aa02 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 23 Nov 2022 13:17:40 +0100 Subject: [PATCH 05/19] Update tests --- tests/test_extensions.py | 152 +++++++++ ...patch.py => test_integration_with_core.py} | 299 ++++++------------ 2 files changed, 248 insertions(+), 203 deletions(-) create mode 100644 tests/test_extensions.py rename tests/{test_monkeypatch.py => test_integration_with_core.py} (60%) diff --git a/tests/test_extensions.py b/tests/test_extensions.py new file mode 100644 index 00000000..7ab18f89 --- /dev/null +++ b/tests/test_extensions.py @@ -0,0 +1,152 @@ +from django.contrib import admin +from django.contrib.sites.models import Site +from django.test import RequestFactory + +from cms.extensions.extension_pool import ExtensionPool +from cms.test_utils.testcases import CMSTestCase +from cms.utils.urlutils import admin_reverse + +from djangocms_versioning.cms_config import copy_page_content +from djangocms_versioning.models import Version +from djangocms_versioning.test_utils.extended_polls.admin import PollExtensionAdmin +from djangocms_versioning.test_utils.extended_polls.models import PollTitleExtension +from djangocms_versioning.test_utils.extensions.models import ( + TestPageExtension, + TestTitleExtension, +) +from djangocms_versioning.test_utils.factories import ( + PageContentFactory, + PageVersionFactory, + PollTitleExtensionFactory, + TestTitleExtensionFactory, +) + + +class ExtensionTestCase(CMSTestCase): + def setUp(self): + self.version = PageVersionFactory(content__language="en") + de_pagecontent = PageContentFactory( + page=self.version.content.page, language="de" + ) + self.page = self.version.content.page + site = Site.objects.first() + self.new_page = self.page.copy( + site=site, + parent_node=self.page.node.parent, + translations=False, + permissions=False, + extensions=False, + ) + new_page_content = PageContentFactory(page=self.new_page, language='de') + self.new_page.title_cache[de_pagecontent.language] = new_page_content + + def test_copy_extensions(self): + """Try to copy the extension, without the monkeypatch this tests fails""" + extension_pool = ExtensionPool() + extension_pool.page_extensions = set([TestPageExtension]) + extension_pool.title_extensions = set([TestTitleExtension]) + extension_pool.copy_extensions( + self.page, self.new_page, languages=['de'] + ) + # No asserts, this test originally failed because the versioned manager was called + # in copy_extensions, now we call the original manager instead + # https://github.com/divio/djangocms-versioning/pull/201/files#diff-fc33dd7b5aa9b1645545cf48dfc9b4ecR19 + + def test_pagecontent_copy_method_creates_extension_title_extension_attached(self): + """ + The page content copy method should create a new title extension, if one is attached to it. + """ + page_content = self.version.content + poll_extension = PollTitleExtensionFactory(extended_object=page_content) + poll_extension.votes = 5 + + new_pagecontent = copy_page_content(page_content) + + self.assertNotEqual(new_pagecontent.polltitleextension, poll_extension) + self.assertEqual(page_content.polltitleextension.pk, poll_extension.pk) + self.assertNotEqual(page_content.polltitleextension.pk, new_pagecontent.polltitleextension.pk) + self.assertEqual(new_pagecontent.polltitleextension.votes, 5) + self.assertEqual(PollTitleExtension._base_manager.count(), 2) + + def test_pagecontent_copy_method_not_created_extension_title_extension_attached(self): + """ + The pagecontent copy method should not create a new title extension, if one isn't attached to the pagecontent + being copied + """ + new_pagecontent = copy_page_content(self.version.content) + + self.assertFalse(hasattr(new_pagecontent, "polltitleextension")) + self.assertEqual(PollTitleExtension._base_manager.count(), 0) + + def test_pagecontent_copy_method_creates_extension_multiple_title_extension_attached(self): + """ + The page content copy method should handle creation of multiple extensions + """ + page_content = self.version.content + poll_extension = PollTitleExtensionFactory(extended_object=page_content) + poll_extension.votes = 5 + title_extension = TestTitleExtensionFactory(extended_object=page_content) + + new_pagecontent = copy_page_content(page_content) + + self.assertNotEqual(new_pagecontent.polltitleextension, poll_extension) + self.assertEqual(page_content.polltitleextension.pk, poll_extension.pk) + self.assertNotEqual(new_pagecontent.testtitleextension, poll_extension) + self.assertEqual(page_content.testtitleextension.pk, poll_extension.pk) + self.assertNotEqual(new_pagecontent.polltitleextension, poll_extension) + self.assertNotEqual(new_pagecontent.testtitleextension, title_extension) + self.assertEqual(new_pagecontent.polltitleextension.votes, 5) + self.assertEqual(PollTitleExtension._base_manager.count(), 2) + self.assertEqual(TestTitleExtension._base_manager.count(), 2) + + def test_title_extension_admin_monkey_patch_save(self): + """ + When hitting the monkeypatched save method, with a draft pagecontent, ensure that we don't see failures + due to versioning overriding monkeypatches + """ + poll_extension = PollTitleExtensionFactory(extended_object=self.version.content) + model_site = PollExtensionAdmin(admin_site=admin.AdminSite(), model=PollTitleExtension) + test_url = admin_reverse("extended_polls_polltitleextension_change", args=(poll_extension.pk,)) + test_url += "?extended_object=%s" % self.version.content.pk + request = RequestFactory().post(path=test_url) + request.user = self.get_superuser() + + poll_extension.votes = 1 + model_site.save_model(request, poll_extension, form=None, change=False) + + self.assertEqual(PollTitleExtension.objects.first().votes, 1) + self.assertEqual(PollTitleExtension.objects.count(), 1) + + def test_title_extension_admin_monkey_patch_save_date_modified_updated(self): + """ + When making changes to an extended model that is attached to a PageContent via + the Title Extension the date modified in a version should also be updated to reflect + the correct date timestamp. + """ + poll_extension = PollTitleExtensionFactory(extended_object=self.version.content) + model_site = PollExtensionAdmin(admin_site=admin.AdminSite(), model=PollTitleExtension) + pre_changes_date_modified = Version.objects.get(id=self.version.pk).modified + test_url = admin_reverse("extended_polls_polltitleextension_change", args=(poll_extension.pk,)) + test_url += "?extended_object=%s" % self.version.content.pk + + request = RequestFactory().post(path=test_url) + request.user = self.get_superuser() + model_site.save_model(request, poll_extension, form=None, change=False) + + post_changes_date_modified = Version.objects.get(id=self.version.pk).modified + + self.assertNotEqual(pre_changes_date_modified, post_changes_date_modified) + + def test_title_extension_admin_monkeypatch_add_view(self): + """ + When hitting the add view, without the monkeypatch, the pagecontent queryset will be filtered to only show + published. Hit it with a draft, to make sure the monkeypatch works. + """ + with self.login_user_context(self.get_superuser()): + response = self.client.get( + admin_reverse("extended_polls_polltitleextension_add") + + "?extended_object=%s" % self.version.content.pk, + follow=True + ) + self.assertEqual(response.status_code, 200) + diff --git a/tests/test_monkeypatch.py b/tests/test_integration_with_core.py similarity index 60% rename from tests/test_monkeypatch.py rename to tests/test_integration_with_core.py index a1adbe33..437f25f4 100644 --- a/tests/test_monkeypatch.py +++ b/tests/test_integration_with_core.py @@ -29,136 +29,7 @@ ) -class MonkeypatchExtensionTestCase(CMSTestCase): - def setUp(self): - self.version = PageVersionFactory(content__language="en") - de_pagecontent = PageContentFactory( - page=self.version.content.page, language="de" - ) - self.page = self.version.content.page - site = Site.objects.first() - self.new_page = self.page.copy( - site=site, - parent_node=self.page.node.parent, - translations=False, - permissions=False, - extensions=False, - ) - new_page_content = PageContentFactory(page=self.new_page, language='de') - self.new_page.title_cache[de_pagecontent.language] = new_page_content - - def test_copy_extensions(self): - """Try to copy the extension, without the monkeypatch this tests fails""" - extension_pool = ExtensionPool() - extension_pool.page_extensions = set([TestPageExtension]) - extension_pool.title_extensions = set([TestTitleExtension]) - extension_pool.copy_extensions( - self.page, self.new_page, languages=['de'] - ) - # No asserts, this test originally failed because the versioned manager was called - # in copy_extensions, now we call the original manager instead - # https://github.com/divio/djangocms-versioning/pull/201/files#diff-fc33dd7b5aa9b1645545cf48dfc9b4ecR19 - - def test_pagecontent_copy_method_creates_extension_title_extension_attached(self): - """ - The page content copy method should create a new title extension, if one is attached to it. - """ - page_content = self.version.content - poll_extension = PollTitleExtensionFactory(extended_object=page_content) - poll_extension.votes = 5 - - new_pagecontent = copy_page_content(page_content) - - self.assertNotEqual(new_pagecontent.polltitleextension, poll_extension) - self.assertEqual(page_content.polltitleextension.pk, poll_extension.pk) - self.assertNotEqual(page_content.polltitleextension.pk, new_pagecontent.polltitleextension.pk) - self.assertEqual(new_pagecontent.polltitleextension.votes, 5) - self.assertEqual(PollTitleExtension._base_manager.count(), 2) - - def test_pagecontent_copy_method_not_created_extension_title_extension_attached(self): - """ - The pagecontent copy method should not create a new title extension, if one isn't attached to the pagecontent - being copied - """ - new_pagecontent = copy_page_content(self.version.content) - - self.assertFalse(hasattr(new_pagecontent, "polltitleextension")) - self.assertEqual(PollTitleExtension._base_manager.count(), 0) - - def test_pagecontent_copy_method_creates_extension_multiple_title_extension_attached(self): - """ - The page content copy method should handle creation of multiple extensions - """ - page_content = self.version.content - poll_extension = PollTitleExtensionFactory(extended_object=page_content) - poll_extension.votes = 5 - title_extension = TestTitleExtensionFactory(extended_object=page_content) - - new_pagecontent = copy_page_content(page_content) - - self.assertNotEqual(new_pagecontent.polltitleextension, poll_extension) - self.assertEqual(page_content.polltitleextension.pk, poll_extension.pk) - self.assertNotEqual(new_pagecontent.testtitleextension, poll_extension) - self.assertEqual(page_content.testtitleextension.pk, poll_extension.pk) - self.assertNotEqual(new_pagecontent.polltitleextension, poll_extension) - self.assertNotEqual(new_pagecontent.testtitleextension, title_extension) - self.assertEqual(new_pagecontent.polltitleextension.votes, 5) - self.assertEqual(PollTitleExtension._base_manager.count(), 2) - self.assertEqual(TestTitleExtension._base_manager.count(), 2) - - def test_title_extension_admin_monkey_patch_save(self): - """ - When hitting the monkeypatched save method, with a draft pagecontent, ensure that we don't see failures - due to versioning overriding monkeypatches - """ - poll_extension = PollTitleExtensionFactory(extended_object=self.version.content) - model_site = PollExtensionAdmin(admin_site=admin.AdminSite(), model=PollTitleExtension) - test_url = admin_reverse("extended_polls_polltitleextension_change", args=(poll_extension.pk,)) - test_url += "?extended_object=%s" % self.version.content.pk - request = RequestFactory().post(path=test_url) - request.user = self.get_superuser() - - poll_extension.votes = 1 - model_site.save_model(request, poll_extension, form=None, change=False) - - self.assertEqual(PollTitleExtension.objects.first().votes, 1) - self.assertEqual(PollTitleExtension.objects.count(), 1) - - def test_title_extension_admin_monkey_patch_save_date_modified_updated(self): - """ - When making changes to an extended model that is attached to a PageContent via - the Title Extension the date modified in a version should also be updated to reflect - the correct date timestamp. - """ - poll_extension = PollTitleExtensionFactory(extended_object=self.version.content) - model_site = PollExtensionAdmin(admin_site=admin.AdminSite(), model=PollTitleExtension) - pre_changes_date_modified = Version.objects.get(id=self.version.pk).modified - test_url = admin_reverse("extended_polls_polltitleextension_change", args=(poll_extension.pk,)) - test_url += "?extended_object=%s" % self.version.content.pk - - request = RequestFactory().post(path=test_url) - request.user = self.get_superuser() - model_site.save_model(request, poll_extension, form=None, change=False) - - post_changes_date_modified = Version.objects.get(id=self.version.pk).modified - - self.assertNotEqual(pre_changes_date_modified, post_changes_date_modified) - - def test_title_extension_admin_monkeypatch_add_view(self): - """ - When hitting the add view, without the monkeypatch, the pagecontent queryset will be filtered to only show - published. Hit it with a draft, to make sure the monkeypatch works. - """ - with self.login_user_context(self.get_superuser()): - response = self.client.get( - admin_reverse("extended_polls_polltitleextension_add") + - "?extended_object=%s" % self.version.content.pk, - follow=True - ) - self.assertEqual(response.status_code, 200) - - -class MonkeypatchTestCase(CMSTestCase): +class CMSToolbarTestCase(CMSTestCase): def test_content_renderer(self): """Test that cms.toolbar.toolbar.CMSToolbar.content_renderer is replaced with a property returning VersionContentRenderer @@ -168,50 +39,33 @@ def test_content_renderer(self): CMSToolbar(request).content_renderer.__class__, VersionContentRenderer ) - def test_get_admin_model_object(self): - """ - PageContent normally won't be able to fetch objects in draft. - With the mocked get_admin_model_object_by_id it is able to fetch objects - in draft mode. - """ - - version = PageVersionFactory() - content = PageContent.get_admin_model_object_by_id(version.content.pk) - - self.assertEqual(version.state, 'draft') - self.assertEqual(content.pk, version.content.pk) - - def test_success_url_for_cms_wizard(self): - from cms.cms_wizards import cms_page_wizard, cms_subpage_wizard - from cms.toolbar.utils import get_object_preview_url + def test_cmstoolbar_mixin(self): + from djangocms_versioning.cms_config import VersioningCMSConfig + from django.apps import apps - from djangocms_versioning.test_utils.polls.cms_wizards import poll_wizard + config = VersioningCMSConfig(apps) + self.assertTrue(issubclass(config.cms_toolbar_mixin, object)) - # Test against page creations in different languages. - version = PageVersionFactory(content__language="en") - self.assertEqual( - cms_page_wizard.get_success_url(version.content.page, language="en"), - get_object_preview_url(version.content), - ) - version = PageVersionFactory(content__language="en") - self.assertEqual( - cms_subpage_wizard.get_success_url(version.content.page, language="en"), - get_object_preview_url(version.content), - ) +class PageContentAdminTestCase(CMSTestCase): - version = PageVersionFactory(content__language="de") - self.assertEqual( - cms_page_wizard.get_success_url(version.content.page, language="de"), - get_object_preview_url(version.content, language="de"), + def test_get_admin_model_object(self): + """ + PageContent normally won't be able to fetch objects in draft. Test if the RequestToolbarForm + finds objects in draft mode. + """ + from cms.admin.forms import RequestToolbarForm + version = PageVersionFactory() + parameter = dict( + obj_id=version.object_id, + obj_type=f"{version.content_type.app_label}.{version.content_type.model}", ) + form = RequestToolbarForm(parameter) + self.assertTrue(form.is_valid()) - # Test against a model that doesn't have a PlaceholderRelationField - version = PollVersionFactory() - self.assertEqual( - poll_wizard.get_success_url(version.content), - version.content.get_absolute_url(), - ) + data = form.clean() + self.assertEqual(version.state, 'draft') + self.assertEqual(data["attached_obj"].pk, version.content.pk) def test_get_title_cache(self): """Check that patched Page._get_title_cache fills @@ -225,41 +79,7 @@ def test_get_title_cache(self): self.assertEqual({"en": version.content}, page.title_cache) -class MonkeypatchAdminTestCase(CMSTestCase): - - def test_default_cms_page_changelist_view_language_with_multi_language_content(self): - """A multi lingual page shows the correct values when - language filters / additional grouping values are set - using the default CMS PageContent view - """ - page = PageFactory(node__depth=1) - en_version1 = PageVersionFactory( - content__page=page, - content__language="en", - ) - fr_version1 = PageVersionFactory( - content__page=page, - content__language="fr", - ) - - # Use the tree endpoint which is what the pagecontent changelist depends on - changelist_url = admin_reverse("cms_pagecontent_get_tree") - with self.login_user_context(self.get_superuser()): - en_response = self.client.get(changelist_url, {"language": "en"}) - fr_response = self.client.get(changelist_url, {"language": "fr"}) - - # English values are only returned - self.assertEqual(200, en_response.status_code) - self.assertContains(en_response, en_version1.content.title) - self.assertNotContains(en_response, fr_version1.content.title) - - # French values are only returned - self.assertEqual(200, fr_response.status_code) - self.assertContains(fr_response, fr_version1.content.title) - self.assertNotContains(fr_response, en_version1.content.title) - - -class MonkeypatchPageAdminCopyLanguageTestCase(CMSTestCase): +class PageAdminCopyLanguageTestCase(CMSTestCase): def setUp(self): self.user = self.get_superuser() @@ -377,3 +197,76 @@ def test_copy_language_copies_from_page_with_different_placeholders(self): source_placeholder_different[0].djangocms_text_ckeditor_text.body, target_placeholder_different[0].djangocms_text_ckeditor_text.body ) + + +class PageContentTreeViewTestCase(CMSTestCase): + + def test_default_cms_page_changelist_view_language_with_multi_language_content(self): + """A multilingual page shows the correct values when + language filters / additional grouping values are set + using the default CMS PageContent view + """ + page = PageFactory(node__depth=1) + en_version1 = PageVersionFactory( + content__page=page, + content__language="en", + ) + fr_version1 = PageVersionFactory( + content__page=page, + content__language="fr", + ) + + # Use the tree endpoint which is what the pagecontent changelist depends on + changelist_url = admin_reverse("cms_pagecontent_get_tree") + with self.login_user_context(self.get_superuser()): + en_response = self.client.get(changelist_url, {"language": "en"}) + fr_response = self.client.get(changelist_url, {"language": "fr"}) + + # English values are only returned + self.assertEqual(200, en_response.status_code) + self.assertContains(en_response, en_version1.content.title) + self.assertNotContains(en_response, fr_version1.content.title) + + # French values are only returned + self.assertEqual(200, fr_response.status_code) + self.assertContains(fr_response, fr_version1.content.title) + self.assertNotContains(fr_response, en_version1.content.title) + + +class WizzardTestCase(CMSTestCase): + + def test_success_url_for_cms_wizard(self): + from cms.cms_wizards import cms_page_wizard, cms_subpage_wizard + from cms.toolbar.utils import get_object_preview_url + + from djangocms_versioning.test_utils.polls.cms_wizards import poll_wizard + + # Test against page creations in different languages. + version = PageVersionFactory(content__language="en") + self.assertEqual( + cms_page_wizard.get_success_url(version.content.page, language="en"), + get_object_preview_url(version.content), + ) + + version = PageVersionFactory(content__language="en") + self.assertEqual( + cms_subpage_wizard.get_success_url(version.content.page, language="en"), + get_object_preview_url(version.content), + ) + + version = PageVersionFactory(content__language="de") + self.assertEqual( + cms_page_wizard.get_success_url(version.content.page, language="de"), + get_object_preview_url(version.content, language="de"), + ) + + # Test against a model that doesn't have a PlaceholderRelationField + version = PollVersionFactory() + self.assertEqual( + poll_wizard.get_success_url(version.content), + version.content.get_absolute_url(), + ) + + + + From 0176c32b9459952ae34eca30ffa86112c194d956 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 23 Nov 2022 13:33:04 +0100 Subject: [PATCH 06/19] fix isort --- tests/test_integration_with_core.py | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/tests/test_integration_with_core.py b/tests/test_integration_with_core.py index 437f25f4..83589243 100644 --- a/tests/test_integration_with_core.py +++ b/tests/test_integration_with_core.py @@ -1,30 +1,13 @@ -from django.contrib import admin -from django.contrib.sites.models import Site -from django.test import RequestFactory - -from cms.extensions.extension_pool import ExtensionPool -from cms.models import PageContent from cms.test_utils.testcases import CMSTestCase from cms.toolbar.toolbar import CMSToolbar from cms.utils.urlutils import admin_reverse -from djangocms_versioning.cms_config import copy_page_content -from djangocms_versioning.models import Version from djangocms_versioning.plugin_rendering import VersionContentRenderer -from djangocms_versioning.test_utils.extended_polls.admin import PollExtensionAdmin -from djangocms_versioning.test_utils.extended_polls.models import PollTitleExtension -from djangocms_versioning.test_utils.extensions.models import ( - TestPageExtension, - TestTitleExtension, -) from djangocms_versioning.test_utils.factories import ( - PageContentFactory, PageFactory, PageVersionFactory, PlaceholderFactory, - PollTitleExtensionFactory, PollVersionFactory, - TestTitleExtensionFactory, TextPluginFactory, ) @@ -40,9 +23,10 @@ def test_content_renderer(self): ) def test_cmstoolbar_mixin(self): - from djangocms_versioning.cms_config import VersioningCMSConfig from django.apps import apps + from djangocms_versioning.cms_config import VersioningCMSConfig + config = VersioningCMSConfig(apps) self.assertTrue(issubclass(config.cms_toolbar_mixin, object)) From 34434a01d56bc2263843be00fc552cb1ea04f26b Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 23 Nov 2022 14:23:45 +0100 Subject: [PATCH 07/19] fix flake8 for tests --- tests/test_extensions.py | 2 +- tests/test_integration_with_core.py | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 7ab18f89..fe0e43e4 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -149,4 +149,4 @@ def test_title_extension_admin_monkeypatch_add_view(self): follow=True ) self.assertEqual(response.status_code, 200) - + diff --git a/tests/test_integration_with_core.py b/tests/test_integration_with_core.py index 83589243..f1c53edd 100644 --- a/tests/test_integration_with_core.py +++ b/tests/test_integration_with_core.py @@ -250,7 +250,3 @@ def test_success_url_for_cms_wizard(self): poll_wizard.get_success_url(version.content), version.content.get_absolute_url(), ) - - - - From 67b89e8c10747e562c6366e5db2885d60ebfcfe9 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 23 Nov 2022 14:29:47 +0100 Subject: [PATCH 08/19] no message --- tests/test_extensions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index fe0e43e4..ad2f404c 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -149,4 +149,3 @@ def test_title_extension_admin_monkeypatch_add_view(self): follow=True ) self.assertEqual(response.status_code, 200) - From 256e23a3e5e75915d218993a45a35827e7a10b46 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 23 Nov 2022 15:09:52 +0100 Subject: [PATCH 09/19] Remove unnecessary get_admin_model_object_by_id --- djangocms_versioning/helpers.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/djangocms_versioning/helpers.py b/djangocms_versioning/helpers.py index 48e10b3a..8e4ed710 100644 --- a/djangocms_versioning/helpers.py +++ b/djangocms_versioning/helpers.py @@ -129,8 +129,6 @@ def replace_default_manager(model): ] model.add_to_class("objects", manager) model.add_to_class("_original_manager", original_manager()) - model.add_to_class("get_admin_model_object_by_id", - classmethod(lambda cls, obj_id: cls._original_manager.get(pk=obj_id))) def inject_generic_relation_to_version(model): From 02ceb4b2c1dc81ca993aa35750a879832a4a7ff5 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Fri, 25 Nov 2022 12:26:16 +0100 Subject: [PATCH 10/19] Add: with_user test --- djangocms_versioning/managers.py | 10 ++++++---- tests/test_automatic_version_generation.py | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 tests/test_automatic_version_generation.py diff --git a/djangocms_versioning/managers.py b/djangocms_versioning/managers.py index 8f62ecc8..51c297de 100644 --- a/djangocms_versioning/managers.py +++ b/djangocms_versioning/managers.py @@ -1,4 +1,5 @@ import warnings +from copy import copy from django.contrib.auth import get_user_model @@ -24,7 +25,7 @@ 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) or getattr(_thread_locals, "user", None) + created_by = getattr(self, "_user", None) if created_by: Version.objects.create(content=obj, created_by=created_by) else: @@ -37,7 +38,8 @@ def create(self, *args, **kwargs): return obj def with_user(self, user): - if not isinstance(user, get_user_model()): + if not isinstance(user, get_user_model()) and user is not None: raise TypeError(f"User for with_user must be of type {get_user_model().__name__}") - self._user = user - return self + new_manager = copy(self) + new_manager._user = user + return new_manager diff --git a/tests/test_automatic_version_generation.py b/tests/test_automatic_version_generation.py new file mode 100644 index 00000000..ce2261db --- /dev/null +++ b/tests/test_automatic_version_generation.py @@ -0,0 +1,22 @@ +from cms.test_utils.testcases import CMSTestCase + +from djangocms_versioning.constants import DRAFT +from djangocms_versioning.models import Version +from djangocms_versioning.test_utils.polls.models import PollContent, Poll + + +class CheckDraftEditableTestCase(CMSTestCase): + def test_creation_wo_with_user(self): + + poll = Poll.objects.create(name="my test poll") + poll_content = PollContent.objects.create(poll=poll, language="en", text="Do you love django CMS?") + version = Version.objects.create(content=poll_content, created_by=self.get_superuser()) + self.assertEqual(version.state, DRAFT) + self.assertTrue(poll_content.versions.exists()) + + def test_creation_with_user(self): + poll = Poll.objects.create(name="my test poll") + user = self.get_superuser() + poll_content = PollContent.objects\ + .with_user(user=user).create(poll=poll, language="en", text="Do you love django CMS?") + self.assertTrue(poll_content.versions.exists()) From 4bc2851ee0748c45bb64317ac33727a34a8970c4 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Fri, 25 Nov 2022 13:02:04 +0100 Subject: [PATCH 11/19] Fix with_user and isort --- djangocms_versioning/managers.py | 2 -- tests/test_automatic_version_generation.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/djangocms_versioning/managers.py b/djangocms_versioning/managers.py index 51c297de..7c977475 100644 --- a/djangocms_versioning/managers.py +++ b/djangocms_versioning/managers.py @@ -3,8 +3,6 @@ from django.contrib.auth import get_user_model -from cms.utils.permissions import _thread_locals - from .constants import PUBLISHED from .models import Version diff --git a/tests/test_automatic_version_generation.py b/tests/test_automatic_version_generation.py index ce2261db..1a7c0934 100644 --- a/tests/test_automatic_version_generation.py +++ b/tests/test_automatic_version_generation.py @@ -2,7 +2,7 @@ from djangocms_versioning.constants import DRAFT from djangocms_versioning.models import Version -from djangocms_versioning.test_utils.polls.models import PollContent, Poll +from djangocms_versioning.test_utils.polls.models import Poll, PollContent class CheckDraftEditableTestCase(CMSTestCase): From fa334f7c7a78a5f4ecc826797038ed2324cb376a Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 30 Nov 2022 12:03:02 +0100 Subject: [PATCH 12/19] Update docs --- docs/versioning_integration.rst | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/docs/versioning_integration.rst b/docs/versioning_integration.rst index fc96124c..58a08061 100644 --- a/docs/versioning_integration.rst +++ b/docs/versioning_integration.rst @@ -120,6 +120,28 @@ and a :term:`copy function `. For simple model structures, the `d which we have used is sufficient, but in many cases you might need to write your own custom :term:`copy function ` (more on that below). +Once a model is registered for versioning its behaviour changes: + +1. It's default manager (``Model.objects``) only sees published versions of the model. See :term:``content model``. +2. It's ``Model.objects.create`` method now will not only create the :term:`content model` but also a corresponding ``Version`` model. Since the ``Version`` model requires a ``User`` object to track who created which version the correct way of creating a versioned :term:`content model` is:: + + Model.objects.with_user(request.user).create(...) + + In certain situations, e.g., when implementing a :term:`copy function`, this is not desirable. Use ``Model._original_manager.create(...)`` in such situations. + +.. note:: + + If you want to allow using your models with and without versioning enabled we suggest to add dummy manager to your model that will swallow the ``with_user()`` syntax. This way you can always create objects with:: + + class ModelManager(models.Manager): + def with_user(self, user): + return self + + class MyModel(models.Model): + objects = ModelManager() + + ... + For more details on how `cms_config.py` integration works please check the documentation for django-cms>=4.0. @@ -205,7 +227,7 @@ This is probably not how one would want things to work in this scenario, so to f # don't copy pk because we're creating a new obj if PostContent._meta.pk.name != field.name } - new_content = PostContent.objects.create(**content_fields) + new_content = PostContent._original_manager.create(**content_fields) original_polls = Poll.objects.filter(post_content=original_content) for poll in original_polls: poll_fields = { @@ -244,6 +266,10 @@ As you can see from the example above the :term:`copy function ` and returns the copied content object. We have customized it to create not just a new PostContent object (which `default_copy` would have done), but also new Poll and Answer objects. +.. note:: + + A custom copy method will need to use the content model's ``PostContent._original_manager`` to create only a content model object and not also a Version object which the ``PostContent.objects`` manager would have done! + Notice that we have not created new Category objects in this example. This is because the default behaviour actually suits Category objects fine. If the name of a category changed, we would not want to revert the whole site to use the old name of the category when reverting a PostContent object. From 64df334dbbc106425c4ba8f6323bf1f3797c0dfa Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Mon, 5 Dec 2022 22:35:28 +0100 Subject: [PATCH 13/19] fix isort --- djangocms_versioning/apps.py | 2 +- djangocms_versioning/cms_config.py | 2 +- djangocms_versioning/test_utils/extensions/models.py | 2 +- tests/test_extensions.py | 6 ++++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/djangocms_versioning/apps.py b/djangocms_versioning/apps.py index d3f18318..07009866 100644 --- a/djangocms_versioning/apps.py +++ b/djangocms_versioning/apps.py @@ -8,7 +8,7 @@ class VersioningConfig(AppConfig): verbose_name = _("django CMS Versioning") def ready(self): - from cms.models import fields, contentmodels + from cms.models import contentmodels, fields from cms.signals import post_obj_operation, post_placeholder_operation from .handlers import ( diff --git a/djangocms_versioning/cms_config.py b/djangocms_versioning/cms_config.py index 9535a2af..2fc4e961 100644 --- a/djangocms_versioning/cms_config.py +++ b/djangocms_versioning/cms_config.py @@ -28,7 +28,7 @@ replace_admin_for_models, replace_manager, ) -from .managers import PublishedContentManagerMixin, AdminManagerMixin +from .managers import AdminManagerMixin, PublishedContentManagerMixin from .models import Version from .plugin_rendering import CMSToolbarVersioningMixin diff --git a/djangocms_versioning/test_utils/extensions/models.py b/djangocms_versioning/test_utils/extensions/models.py index 836f2125..6d12edf8 100644 --- a/djangocms_versioning/test_utils/extensions/models.py +++ b/djangocms_versioning/test_utils/extensions/models.py @@ -1,4 +1,4 @@ -from cms.extensions.models import PageExtension, PageContentExtension +from cms.extensions.models import PageContentExtension, PageExtension class TestPageExtension(PageExtension): diff --git a/tests/test_extensions.py b/tests/test_extensions.py index a6719ed1..70479b56 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -9,10 +9,12 @@ from djangocms_versioning.cms_config import copy_page_content from djangocms_versioning.models import Version from djangocms_versioning.test_utils.extended_polls.admin import PollExtensionAdmin -from djangocms_versioning.test_utils.extended_polls.models import PollPageContentExtension +from djangocms_versioning.test_utils.extended_polls.models import ( + PollPageContentExtension, +) from djangocms_versioning.test_utils.extensions.models import ( - TestPageExtension, TestPageContentExtension, + TestPageExtension, ) from djangocms_versioning.test_utils.factories import ( PageContentFactory, From 4f505856a0f995298cfcacfdc288f349db2865e5 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 7 Dec 2022 15:38:44 +0100 Subject: [PATCH 14/19] Update tox tests add tests for current_content_iterator --- .github/workflows/test.yml | 6 +- djangocms_versioning/helpers.py | 4 +- djangocms_versioning/managers.py | 10 +-- .../{dj32_cms40.txt => dj32_cms41.txt} | 0 .../{dj40_cms40.txt => dj40_cms41.txt} | 0 .../{dj41_cms40.txt => dj41_cms41.txt} | 0 tests/test_content_models.py | 72 ++++++++++++++++++- tox.ini | 15 ++-- 8 files changed, 89 insertions(+), 18 deletions(-) rename tests/requirements/{dj32_cms40.txt => dj32_cms41.txt} (100%) rename tests/requirements/{dj40_cms40.txt => dj40_cms41.txt} (100%) rename tests/requirements/{dj41_cms40.txt => dj41_cms41.txt} (100%) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d5d68c33..6a5993be 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -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: diff --git a/djangocms_versioning/helpers.py b/djangocms_versioning/helpers.py index 6f7f30e5..c7ed7a4c 100644 --- a/djangocms_versioning/helpers.py +++ b/djangocms_versioning/helpers.py @@ -130,7 +130,9 @@ def replace_manager(model, manager, mixin, **kwargs): mngr for mngr in model._meta.local_managers if mngr.name != manager ] model.add_to_class(manager, manager_object) - model.add_to_class(f'_original_{"manager" if manager == "objects" else manager}', original_manager()) + 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): diff --git a/djangocms_versioning/managers.py b/djangocms_versioning/managers.py index 71172a45..f289c81c 100644 --- a/djangocms_versioning/managers.py +++ b/djangocms_versioning/managers.py @@ -55,9 +55,9 @@ def with_user(self, user): class AdminQuerySet(models.QuerySet): - def __init__(self, *args, **kwargs): - self._group_by_key = kwargs.pop("group_by_key", []) - super().__init__(*args, **kwargs) + def _chain(self, **kwargs): + # Also clone group by key when chaining querysets! + return super()._chain(_group_by_key=self._group_by_key, **kwargs) def current_content_iterator(self, **kwargs): """Returns generator (not a queryset) over current content versions. Current versions are either draft @@ -78,4 +78,6 @@ class AdminManagerMixin: _group_by_key = [] def get_queryset(self): - return AdminQuerySet(self.model, using=self._db, group_by_key=self._group_by_key) + qs = AdminQuerySet(self.model, using=self._db) + qs._group_by_key = self._group_by_key + return qs diff --git a/tests/requirements/dj32_cms40.txt b/tests/requirements/dj32_cms41.txt similarity index 100% rename from tests/requirements/dj32_cms40.txt rename to tests/requirements/dj32_cms41.txt diff --git a/tests/requirements/dj40_cms40.txt b/tests/requirements/dj40_cms41.txt similarity index 100% rename from tests/requirements/dj40_cms40.txt rename to tests/requirements/dj40_cms41.txt diff --git a/tests/requirements/dj41_cms40.txt b/tests/requirements/dj41_cms41.txt similarity index 100% rename from tests/requirements/dj41_cms40.txt rename to tests/requirements/dj41_cms41.txt diff --git a/tests/test_content_models.py b/tests/test_content_models.py index 663a6363..166e8db0 100644 --- a/tests/test_content_models.py +++ b/tests/test_content_models.py @@ -2,11 +2,13 @@ from django.db import models +from cms.models.contentmodels import PageContent from cms.test_utils.testcases import CMSTestCase -from djangocms_versioning import helpers +from djangocms_versioning import helpers, constants from djangocms_versioning.helpers import replace_manager -from djangocms_versioning.managers import PublishedContentManagerMixin +from djangocms_versioning.managers import AdminManagerMixin, PublishedContentManagerMixin +from djangocms_versioning.test_utils.factories import PageFactory, PageVersionFactory from djangocms_versioning.test_utils.people.models import Person @@ -19,10 +21,74 @@ def test_replace_default_manager(self): replace_manager(Person, "objects", PublishedContentManagerMixin) self.assertIn(PublishedContentManagerMixin, Person.objects.__class__.mro()) + self.assertFalse(hasattr(Person, "admin_manager")) + replace_manager(Person, "admin_manager", AdminManagerMixin) + self.assertIn(AdminManagerMixin, Person.admin_manager.__class__.mro()) + def test_replace_default_manager_twice(self): replace_manager(Person, "objects", PublishedContentManagerMixin) - with patch.object(helpers, "manager_factory") as mock: replace_manager(Person, "objects", PublishedContentManagerMixin) + mock.assert_not_called() + + original_manager = Person._original_manager + replace_manager(Person, "admin_manager", AdminManagerMixin) + with patch.object(helpers, "manager_factory") as mock: + replace_manager(Person, "admin_manager", AdminManagerMixin) mock.assert_not_called() + + # Replacing admin_manager did not overwrite _original_manager? + self.assertEqual(Person._original_manager, original_manager) + + +class AdminManagerTestCase(CMSTestCase): + def create_page_content(self, page, language, version_state): + version = PageVersionFactory(content__page=page, content__language=language) + if version_state == constants.PUBLISHED: + version.publish(self.get_superuser()) + elif version_state == constants.ARCHIVED: + version.archive(self.get_superuser()) + + def setUp(self) -> None: + self.pages1 = [PageFactory() for i in range(2)] + for page in self.pages1: + self.create_page_content(page, "en", constants.DRAFT) + self.create_page_content(page, "it", constants.PUBLISHED) + + self.pages2 = [PageFactory() for i in range(2)] + for page in self.pages2: + self.create_page_content(page, "en", constants.PUBLISHED) + self.create_page_content(page, "en", constants.DRAFT) + self.create_page_content(page, "it", constants.ARCHIVED) + self.create_page_content(page, "it", constants.PUBLISHED) + + def test_current_content_iterator(self): + # 12 PageContent versions in total + self.assertEqual(len(list( + PageContent.admin_manager.all() + )), 12) + + # 4 total PageContent versions for self.pages1 (2 pages x 2 languages) + self.assertEqual(len( + qs := PageContent.admin_manager.filter(page__in=self.pages1) + ), 4) + self.assertEqual(qs._group_by_key, ["page", "language"]) + self.assertEqual(len(list( + PageContent.admin_manager.filter(page__in=self.pages1).current_content_iterator() + )), 4, f"{list(PageContent.admin_manager.filter(page__in=self.pages1).current_content_iterator())}") + # 2 current PageContent versions for self.pages2 + self.assertEqual(len(list( + PageContent.admin_manager.filter(page__in=self.pages2).current_content_iterator() + )), 4) + + # Now unpublish all published in pages2 + for page in self.pages2: + for content in page.pagecontent_set.all(): + content.versions.first().unpublish(self.get_superuser()) + + + # 2 current PageContent versions for self.pages2 + self.assertEqual(len(list( + PageContent.admin_manager.filter(page__in=self.pages2).current_content_iterator() + )), 2) diff --git a/tox.ini b/tox.ini index aebbaee8..b1d3062a 100644 --- a/tox.ini +++ b/tox.ini @@ -2,7 +2,7 @@ envlist = flake8 isort - py{37,38,39}-dj{22,32}-sqlite + py{39}-dj{32}-sqlite skip_missing_interpreters=True @@ -10,13 +10,14 @@ skip_missing_interpreters=True deps = -r{toxinidir}/tests/requirements/requirements_base.txt - dj22: -r{toxinidir}/tests/requirements/django_22.txt - dj32: -r{toxinidir}/tests/requirements/django_32.txt + dj32: -r{toxinidir}/tests/requirements/dj32_cms41.txt + dj40: -r{toxinidir}/tests/requirements/dj40_cms41.txt + dj41: -r{toxinidir}/tests/requirements/dj41_cms41.txt basepython = - py37: python3.7 - py38: python3.8 py39: python3.9 + py310: python3.10 + py311: python3.10 commands = {envpython} --version @@ -29,5 +30,5 @@ commands = flake8 basepython = python3.8 [testenv:isort] -commands = isort --recursive --check-only --diff {toxinidir} -basepython = python3.8 +commands = isort --check-only --diff {toxinidir}/djangocms_versioning +basepython = python3.9 From 3ff35695b4fe94ea96be857b92c8bde8fd89e756 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 7 Dec 2022 16:01:55 +0100 Subject: [PATCH 15/19] fix test linter issues --- tests/test_content_models.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_content_models.py b/tests/test_content_models.py index 166e8db0..1c9436cb 100644 --- a/tests/test_content_models.py +++ b/tests/test_content_models.py @@ -5,9 +5,12 @@ from cms.models.contentmodels import PageContent from cms.test_utils.testcases import CMSTestCase -from djangocms_versioning import helpers, constants +from djangocms_versioning import constants, helpers from djangocms_versioning.helpers import replace_manager -from djangocms_versioning.managers import AdminManagerMixin, PublishedContentManagerMixin +from djangocms_versioning.managers import ( + AdminManagerMixin, + PublishedContentManagerMixin, +) from djangocms_versioning.test_utils.factories import PageFactory, PageVersionFactory from djangocms_versioning.test_utils.people.models import Person @@ -87,7 +90,6 @@ def test_current_content_iterator(self): for content in page.pagecontent_set.all(): content.versions.first().unpublish(self.get_superuser()) - # 2 current PageContent versions for self.pages2 self.assertEqual(len(list( PageContent.admin_manager.filter(page__in=self.pages2).current_content_iterator() From 7d931cda147223f75cbee8eb4f90c0aad45fd0cd Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 7 Dec 2022 16:14:16 +0100 Subject: [PATCH 16/19] Fix: django 4+ compatibility --- djangocms_versioning/managers.py | 6 ++++-- tox.ini | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/djangocms_versioning/managers.py b/djangocms_versioning/managers.py index f289c81c..d6ac5467 100644 --- a/djangocms_versioning/managers.py +++ b/djangocms_versioning/managers.py @@ -55,9 +55,11 @@ def with_user(self, user): class AdminQuerySet(models.QuerySet): - def _chain(self, **kwargs): + def _chain(self): # Also clone group by key when chaining querysets! - return super()._chain(_group_by_key=self._group_by_key, **kwargs) + 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 diff --git a/tox.ini b/tox.ini index b1d3062a..36dad075 100644 --- a/tox.ini +++ b/tox.ini @@ -2,7 +2,7 @@ envlist = flake8 isort - py{39}-dj{32}-sqlite + py{39.310,311}-dj{32,40,41}-sqlite skip_missing_interpreters=True @@ -27,7 +27,7 @@ commands = [testenv:flake8] commands = flake8 -basepython = python3.8 +basepython = python3.9 [testenv:isort] commands = isort --check-only --diff {toxinidir}/djangocms_versioning From d2e69cabd916906dd0f894f149d5dff152fd57ae Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 7 Dec 2022 16:31:31 +0100 Subject: [PATCH 17/19] correct typo in test requirements --- .github/workflows/test.yml | 12 ++++++------ djangocms_versioning/managers.py | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6a5993be..dc31ffc5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -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: @@ -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: diff --git a/djangocms_versioning/managers.py b/djangocms_versioning/managers.py index d6ac5467..cea974fc 100644 --- a/djangocms_versioning/managers.py +++ b/djangocms_versioning/managers.py @@ -58,7 +58,7 @@ 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 + clone._group_by_key = self._group_by_key return clone def current_content_iterator(self, **kwargs): From cf5abde8d26ad5cdea407254e434d2c12c7f61d0 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Thu, 8 Dec 2022 18:36:30 +0100 Subject: [PATCH 18/19] Fix test dependencies --- tests/requirements/requirements_base.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/requirements/requirements_base.txt b/tests/requirements/requirements_base.txt index bf162d88..5afe1c4c 100644 --- a/tests/requirements/requirements_base.txt +++ b/tests/requirements/requirements_base.txt @@ -14,5 +14,5 @@ mysqlclient==2.0.3 psycopg2 # Unreleased django-cms 4.0 compatible packages -https://github.com/fsbraun/django-cms/tarball/fix/publish-state#egg=django-cms +https://github.com/fsbraun/django-cms/tarball/fix/remove-patching#egg=django-cms https://github.com/django-cms/djangocms-text-ckeditor/tarball/support/4.0.x#egg=djangocms-text-ckeditor From d6d33dc573a62bd18c99a2db0e8c95dcf8f92e5e Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Wed, 14 Dec 2022 22:05:09 +0100 Subject: [PATCH 19/19] Clarify test --- tests/test_content_models.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_content_models.py b/tests/test_content_models.py index 1c9436cb..bfd862a6 100644 --- a/tests/test_content_models.py +++ b/tests/test_content_models.py @@ -73,9 +73,8 @@ def test_current_content_iterator(self): )), 12) # 4 total PageContent versions for self.pages1 (2 pages x 2 languages) - self.assertEqual(len( - qs := PageContent.admin_manager.filter(page__in=self.pages1) - ), 4) + qs = PageContent.admin_manager.filter(page__in=self.pages1) + self.assertEqual(len(qs), 4) self.assertEqual(qs._group_by_key, ["page", "language"]) self.assertEqual(len(list( PageContent.admin_manager.filter(page__in=self.pages1).current_content_iterator()