feat: Improve default copy method to also copy placeholders and plugi…

…ns (#345)

* feat: Let default_copy also copy PlaceholderRelationFields

* Clarify comment

* Call copy_relations if it exists in verisioned model

* Update tests

* Fix indent

* Fix test

* Update page content copy

* Fix creation_date for page content

* Fix linting issues

* More ruff fixes

* fix test

* Update tests/test_datastructures.py

Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>

* Improve tests

---------

Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>

djangocms_versioning/admin.py

@@ -1039,9 +1039,9 @@ def publish_view(self, request, object_id):
 
         requested_redirect = request.GET.get("next", None)
         if conf.ON_PUBLISH_REDIRECT in ("preview", "published"):
-            redirect_url=get_preview_url(version.content)
+            redirect_url = get_preview_url(version.content)
         else:
-            redirect_url=version_list_url(version.content)
+            redirect_url = version_list_url(version.content)
 
         if not version.can_be_published():
             self.message_user(request, _("Version cannot be published"), messages.ERROR)
@@ -1065,7 +1065,6 @@ def publish_view(self, request, object_id):
 
         return self._internal_redirect(requested_redirect, redirect_url)
 
-
     def _internal_redirect(self, url, fallback):
         """Helper function to check if the give URL is resolvable
         If resolvable, return the URL; otherwise, returns the fallback URL.
@@ -1080,7 +1079,6 @@ def _internal_redirect(self, url, fallback):
 
         return redirect(url)
 
-
     def unpublish_view(self, request, object_id):
         """Unpublishes the specified version and redirects back to the
         version changelist
@@ -1093,9 +1091,9 @@ def unpublish_view(self, request, object_id):
             )
 
         if conf.ON_PUBLISH_REDIRECT in ("preview", "published"):
-            redirect_url=get_preview_url(version.content)
+            redirect_url = get_preview_url(version.content)
         else:
-            redirect_url=version_list_url(version.content)
+            redirect_url = version_list_url(version.content)
 
         if not version.can_be_unpublished():
             self.message_user(
@@ -1420,7 +1418,8 @@ def changelist_view(self, request, extra_context=None):
             # Check if custom breadcrumb template defined, otherwise
             # fallback on default
             breadcrumb_templates = [
-                f"admin/djangocms_versioning/{breadcrumb_opts.app_label}/{breadcrumb_opts.model_name}/versioning_breadcrumbs.html",
+                f"admin/djangocms_versioning/{breadcrumb_opts.app_label}/"
+                f"{breadcrumb_opts.model_name}/versioning_breadcrumbs.html",
                 "admin/djangocms_versioning/versioning_breadcrumbs.html",
             ]
             extra_context["breadcrumb_template"] = select_template(breadcrumb_templates)

djangocms_versioning/cms_config.py

@@ -3,7 +3,7 @@
 from cms import __version__ as cms_version
 from cms.app_base import CMSAppConfig, CMSAppExtension
 from cms.extensions.models import BaseExtension
-from cms.models import PageContent, Placeholder
+from cms.models import PageContent
 from cms.utils.i18n import get_language_list, get_language_tuple
 from cms.utils.plugins import copy_plugins_to_placeholder
 from cms.utils.urlutils import admin_reverse
@@ -22,13 +22,14 @@
 )
 from django.utils.encoding import force_str
 from django.utils.functional import cached_property
+from django.utils.timezone import now
 from django.utils.translation import gettext_lazy as _
 from packaging.version import Version as PackageVersion
 
 from . import indicators
 from .admin import VersioningAdminMixin
 from .constants import INDICATOR_DESCRIPTIONS
-from .datastructures import BaseVersionableItem, VersionableItem
+from .datastructures import BaseVersionableItem, VersionableItem, default_copy
 from .exceptions import ConditionFailed
 from .helpers import (
     get_latest_admin_viewable_content,
@@ -188,36 +189,8 @@ def copy_page_content(original_content):
     """Copy the PageContent object and deepcopy its
     placeholders and plugins.
     """
-    # Copy content object
-    content_fields = {
-        field.name: getattr(original_content, field.name)
-        for field in PageContent._meta.fields
-        # Don't copy the pk as we're creating a new obj.
-        # The creation date should reflect the date it was copied on,
-        # so don't copy that either.
-        if field.name not in (PageContent._meta.pk.name, "creation_date")
-    }
-
-    # Use original manager to not create a new Version object here
-    new_content = PageContent._original_manager.create(**content_fields)
-
-    # Copy placeholders
-    new_placeholders = []
-    for placeholder in original_content.placeholders.all():
-        placeholder_fields = {
-            field.name: getattr(placeholder, field.name)
-            for field in Placeholder._meta.fields
-            # don't copy primary key because we're creating a new obj
-            # and handle the source field later
-            if field.name not in [Placeholder._meta.pk.name, "source"]
-        }
-        if placeholder.source:
-            placeholder_fields["source"] = new_content
-        new_placeholder = Placeholder.objects.create(**placeholder_fields)
-        # Copy plugins
-        placeholder.copy_plugins(new_placeholder)
-        new_placeholders.append(new_placeholder)
-    new_content.placeholders.add(*new_placeholders)
+    new_content = default_copy(original_content)
+    new_content.creation_date = now()
 
     # If pagecontent has an associated content or page extension, also copy this!
     for field in PageContent._meta.related_objects:
@@ -249,6 +222,7 @@ def on_page_content_publish(version):
     page._update_url_path_recursive(language)
     page.clear_cache(menu=True)
 
+
 def on_page_content_unpublish(version):
     """Url path and cache operations to do when a PageContent obj is unpublished"""
     page = version.content.page

djangocms_versioning/conditions.py

@@ -77,12 +77,14 @@ def inner(version, user):
             raise ConditionFailed(message)
     return inner
 
+
 def user_can_unlock(message: str) -> callable:
     def inner(version, user):
         if not user.has_perm("djangocms_versioning.delete_versionlock"):
             raise ConditionFailed(message)
     return inner
 
+
 def user_can_publish(message: str) -> callable:
     def inner(version, user):
         if not version.has_publish_permission(user):

djangocms_versioning/datastructures.py

@@ -1,5 +1,6 @@
 from itertools import chain
 
+from cms.models import Placeholder, PlaceholderRelationField
 from django.contrib.contenttypes.models import ContentType
 from django.db.models import Max, Prefetch
 from django.utils.functional import cached_property
@@ -197,12 +198,28 @@ def __getattr__(self, name):
         return getattr(self.to, name)
 
 
+def copy_placeholder(original_placeholder, new_content):
+    placeholder_fields = {
+        field.name: getattr(original_placeholder, field.name)
+        for field in Placeholder._meta.fields
+        if field.name not in [Placeholder._meta.pk.name, "source"]
+    }
+    if original_placeholder.source:
+        placeholder_fields["source"] = new_content
+    new_placeholder = Placeholder.objects.create(**placeholder_fields)
+    original_placeholder.copy_plugins(new_placeholder)
+    return new_placeholder
+
+
 def default_copy(original_content):
     """Copy all fields of the original content object exactly as they are
     and return a new content object which is different only in its pk.
 
-    NOTE: This will only work for very simple content objects. This will
-    throw exceptions on one2one and m2m relationships. And it might not
+    NOTE: This will only work for very simple content objects.
+
+    It copies placeholders and their plugins.
+
+    It will throw exceptions on one2one and m2m relationships. And it might not
     be the desired behaviour for some foreign keys (in some cases we
     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
@@ -218,5 +235,18 @@ 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
     }
-    # Use original manager to avoid creating a new draft version here!
-    return content_model._original_manager.create(**content_fields)
+    # Use original manager to not create a new Version object here
+    new_content = content_model._original_manager.create(**content_fields)
+
+    # Now copy PlaceholderRelationFields
+    for field in content_model._meta.private_fields:
+        # Copy PlaceholderRelationFields
+        if isinstance(field, PlaceholderRelationField):
+            # Copy placeholders
+            original_placeholders = getattr(original_content, field.name).all()
+            new_placeholders = [copy_placeholder(ph, new_content) for ph in original_placeholders]
+            getattr(new_content, field.name).add(*new_placeholders)
+        if hasattr(new_content, "copy_relations"):
+            if callable(new_content.copy_relations):
+                new_content.copy_relations()
+    return new_content

djangocms_versioning/management/__init__.py

@@ -1 +0,0 @@
-

djangocms_versioning/models.py

@@ -34,6 +34,7 @@
 lock_draft_error_message = _("Action Denied. The draft version is locked by {user}")
 permission_error_message = _("You do not have permission to perform this action")
 
+
 def allow_deleting_versions(collector, field, sub_objs, using):
     if ALLOW_DELETING_VERSIONS:
         models.SET_NULL(collector, field, sub_objs, using)

djangocms_versioning/templatetags/djangocms_versioning.py

@@ -11,6 +11,7 @@
 def url_version_list(content):
     return version_list_url(content)
 
+
 @register.filter
 def url_publish_version(content, user):
     if hasattr(content, "prefetched_versions"):
@@ -26,6 +27,7 @@ def url_publish_version(content, user):
             )
     return ""
 
+
 @register.filter
 def url_new_draft(content, user):
     if hasattr(content, "prefetched_versions"):
@@ -41,6 +43,7 @@ def url_new_draft(content, user):
             )
     return ""
 
+
 @register.filter
 def url_revert_version(content, user):
     if hasattr(content, "prefetched_versions"):

tests/test_datastructures.py

@@ -1,13 +1,15 @@
 import copy
 
-from cms.models import PageContent
+from cms import api
+from cms.models import PageContent, Placeholder
 from cms.test_utils.testcases import CMSTestCase
 from django.apps import apps
+from django.test import TestCase
 
 from djangocms_versioning.constants import ARCHIVED, PUBLISHED
 from djangocms_versioning.datastructures import VersionableItem, default_copy
 from djangocms_versioning.models import Version
-from djangocms_versioning.test_utils.factories import PollVersionFactory
+from djangocms_versioning.test_utils.factories import PageContentFactory, PollVersionFactory
 from djangocms_versioning.test_utils.people.models import PersonContent
 from djangocms_versioning.test_utils.polls.models import Poll, PollContent
 
@@ -171,3 +173,85 @@ def test_version_model_proxy_cached(self):
         self.assertEqual(
             id(versionable.version_model_proxy), id(versionable.version_model_proxy)
         )
+
+class DefaultCopyTestCase(TestCase):
+    def setUp(self):
+        self.original_content = PageContentFactory()
+
+    def test_default_copy_creates_new_instance(self):
+        new_content = default_copy(self.original_content)
+        self.assertNotEqual(self.original_content.pk, new_content.pk)
+        self.assertEqual(self.original_content.page, new_content.page)
+        self.assertEqual(self.original_content.language, new_content.language)
+
+    def test_default_copy_copies_placeholders(self):
+        placeholder = Placeholder.objects.create(slot="content")
+        self.original_content.placeholders.add(placeholder)
+        new_content = default_copy(self.original_content)
+        self.assertEqual(new_content.placeholders.count(), 1)
+        self.assertNotEqual(new_content.placeholders.first().pk, placeholder.pk)
+        self.assertEqual(new_content.placeholders.first().slot, placeholder.slot)
+
+    def test_default_copy_copies_plugins_within_placeholder(self):
+        # Create a placeholder and attach two different plugin types
+        placeholder = Placeholder.objects.create(slot="content")
+        plugin1 = api.add_plugin(
+            placeholder=placeholder,
+            plugin_type="TextPlugin",
+            language=self.original_content.language,
+            body="Sample text",
+        )
+        plugin2 = api.add_plugin(
+            placeholder=placeholder,
+            plugin_type="TextPlugin",
+            language=self.original_content.language,
+            body="Some other text",
+        )
+        self.original_content.placeholders.add(placeholder)
+
+        new_content = default_copy(self.original_content)
+        new_placeholder = new_content.placeholders.first()
+
+        # Ensure that the new placeholder has two plugins
+        self.assertEqual(new_placeholder.cmsplugin_set.count(), 2)
+
+        # Collect original and copied plugin IDs for comparison
+        original_plugin_ids = {plugin1.pk, plugin2.pk}
+        new_plugins = list(new_placeholder.cmsplugin_set.all())
+        for plugin in new_plugins:
+            self.assertNotIn(plugin.pk, original_plugin_ids)
+
+        # Verify that the copied plugins preserve type and key attributes
+        downcasted = [plugin.get_plugin_instance()[0] for plugin in new_plugins]
+        original = [plugin1, plugin2]
+        for orig_plugin, new_plugin in zip(original, downcasted):
+            self.assertEqual(orig_plugin.plugin_type, new_plugin.plugin_type)
+            self.assertEqual(orig_plugin.body, new_plugin.body)
+
+    def test_default_copy_copies_multiple_placeholders(self):
+        placeholders = [Placeholder.objects.create(slot=f"slot_{i}") for i in range(3)]
+        for placeholder in placeholders:
+            self.original_content.placeholders.add(placeholder)
+        new_content = default_copy(self.original_content)
+        self.assertEqual(new_content.placeholders.count(), len(placeholders))
+        for original in self.original_content.placeholders.all():
+            copied = new_content.placeholders.get(slot=original.slot)
+            self.assertNotEqual(copied.pk, original.pk)
+            self.assertEqual(copied.slot, original.slot)
+
+    def test_default_copy_calls_copy_relations_if_exists(self):
+        class MockContent(PageContent):
+            class Meta:
+                app_label = "cms"
+                proxy = True
+
+            def __init__(self, *args, **kwargs):
+                self.copy_relations_called = False
+                super().__init__(*args, **kwargs)
+
+            def copy_relations(self):
+                self.copy_relations_called = True
+
+        original_content = MockContent(language=self.original_content.language, page=self.original_content.page)
+        new_content = default_copy(original_content)
+        self.assertTrue(new_content.copy_relations_called)

tests/test_models.py

@@ -221,7 +221,7 @@ def test_copy_plugins_method_used(self):
         user = factories.UserFactory()
 
         with patch(
-            "djangocms_versioning.cms_config.Placeholder.copy_plugins"
+            "djangocms_versioning.datastructures.Placeholder.copy_plugins"
         ) as mocked_copy:
             new_version = original_version.copy(user)