Merge branch 'develop-4' into non-named-cmsapps-error-fix

.github/workflows/test.yml

@@ -29,7 +29,7 @@ jobs:
 
     services:
       postgres:
-        image: postgres:17
+        image: postgres:latest
         env:
           POSTGRES_USER: postgres
           POSTGRES_PASSWORD: postgres

cms/models/placeholdermodel.py

@@ -3,7 +3,7 @@
 
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.contrib.contenttypes.models import ContentType
-from django.db import connection, models
+from django.db import connection, models, transaction
 from django.template.defaultfilters import title
 from django.utils.encoding import force_str
 from django.utils.translation import gettext_lazy as _
@@ -684,17 +684,22 @@ def delete_plugin(self, instance):
         :param instance: Plugin to add. It's position parameter needs to be set.
         :type instance: :class:`cms.models.pluginmodel.CMSPlugin` instance
         """
-        instance.get_descendants().delete()
-        instance.delete()
-        last_plugin = self.get_last_plugin(instance.language)
-
-        if last_plugin:
-            self._shift_plugin_positions(
-                instance.language,
-                start=instance.position,
-                offset=last_plugin.position,
-            )
-            self._recalculate_plugin_positions(instance.language)
+        with transaction.atomic():
+            # We're using raw sql - make the whole operation atomic
+            plugins = self.get_plugins(language=instance.language).count()  # 1st hit: Count plugins
+            descendants = instance._get_descendants_ids()  # 2nd hit: Get descendant ids
+            to_delete = [instance.pk] + descendants  # Instance plus descendants pk
+            self.cmsplugin_set.filter(pk__in=to_delete).delete()  # 3rd hit: Delete all plugins in one query
+
+            last_position = instance.position + len(descendants)  # Last position of deleted plugins
+            if last_position < plugins:
+                # Close the gap in the plugin tree (2 hits)
+                self._shift_plugin_positions(
+                    instance.language,
+                    start=instance.position,
+                    offset=plugins,
+                )
+                self._recalculate_plugin_positions(instance.language)
 
     def get_last_plugin(self, language):
         return self.get_plugins(language).last()
@@ -756,39 +761,15 @@ def _recalculate_plugin_positions(self, language):
         cursor = _get_database_cursor('write')
         db_vendor = _get_database_vendor('write')
 
-        if db_vendor == 'sqlite':
-            sql = (
-                'CREATE TEMPORARY TABLE temp AS '
-                'SELECT ID, ('
-                'SELECT COUNT(*)+1 FROM {0} t WHERE '
-                'placeholder_id={0}.placeholder_id AND language={0}.language '
-                'AND {0}.position > t.position'
-                ') AS new_position '
-                'FROM {0} WHERE placeholder_id=%s AND language=%s'
-            )
-            sql = sql.format(connection.ops.quote_name(CMSPlugin._meta.db_table))
-            cursor.execute(sql, [self.pk, language])
-
-            sql = (
-                'UPDATE {0} '
-                'SET position = (SELECT new_position FROM temp WHERE id={0}.id) '
-                'WHERE placeholder_id=%s AND language=%s'
-            )
-            sql = sql.format(connection.ops.quote_name(CMSPlugin._meta.db_table))
-            cursor.execute(sql, [self.pk, language])
-
-            sql = 'DROP TABLE temp'
-            sql = sql.format(connection.ops.quote_name(CMSPlugin._meta.db_table))
-            cursor.execute(sql)
-        elif db_vendor == 'postgresql':
+        if db_vendor in ('sqlite', 'postgresql'):
             sql = (
                 'UPDATE {0} '
-                'SET position = RowNbrs.RowNbr '
+                'SET position = subquery.new_pos '
                 'FROM ('
-                'SELECT  ID, ROW_NUMBER() OVER (ORDER BY position) AS RowNbr '
-                'FROM {0} WHERE placeholder_id=%s AND language=%s '
-                ') RowNbrs '
-                'WHERE {0}.id=RowNbrs.id'
+                '  SELECT  ID, ROW_NUMBER() OVER (ORDER BY position, id) AS new_pos '
+                '  FROM {0} WHERE placeholder_id=%s AND language=%s '
+                ') subquery '
+                'WHERE {0}.id=subquery.id'
             )
             sql = sql.format(connection.ops.quote_name(CMSPlugin._meta.db_table))
             cursor.execute(sql, [self.pk, language])

cms/models/pluginmodel.py

@@ -2,7 +2,7 @@
 import os
 import warnings
 from datetime import date
-from functools import lru_cache
+from functools import cache
 
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import connection, connections, models, router
@@ -19,7 +19,7 @@
 from cms.utils.urlutils import admin_reverse
 
 
-@lru_cache(maxsize=None)
+@cache
 def _get_descendants_cte():
     db_vendor = _get_database_vendor('read')
     if db_vendor == 'oracle':
@@ -60,7 +60,7 @@ def _get_database_cursor(action):
     return _get_database_connection(action).cursor()
 
 
-@lru_cache(maxsize=None)
+@cache
 def plugin_supports_cte():
     # This has to be as function because when it's a var it evaluates before
     # db is connected and we get OperationalError. MySQL version is retrieved

cms/tests/test_placeholder.py

@@ -1378,7 +1378,10 @@ def test_delete(self):
         for plugin in self.get_plugins().filter(parent__isnull=True):
             for plugin_id in [plugin.pk] + tree[plugin.pk]:
                 plugin_tree_all.remove(plugin_id)
+
+            plugin.refresh_from_db()
             self.placeholder.delete_plugin(plugin)
+
             new_tree = self.get_plugins().values_list('pk', 'position')
             expected = [(pk, pos) for pos, pk in enumerate(plugin_tree_all, 1)]
             self.assertSequenceEqual(new_tree, expected)
@@ -1610,6 +1613,24 @@ def test_move_to_placeholder_bottom(self):
 
 
 class PlaceholderNestedPluginTests(PlaceholderFlatPluginTests):
+    """
+    Same tests as for PlaceholderFlatPluginTests but now with a different plugin tree:
+
+    ::
+
+        Parent 1
+          Parent 2
+            Child
+        Parent 1
+          Parent 2
+            Child
+        Parent 1
+          Parent 2
+            Child
+        Parent 1
+          Parent 2
+            Child
+    """
 
     def create_plugins(self, placeholder):
         for i in range(1, 12, 3):
@@ -1654,7 +1675,10 @@ def test_delete_single(self):
         for plugin in self.get_plugins().filter(parent__isnull=True):
             for plugin_id in [plugin.pk] + tree[plugin.pk]:
                 plugin_tree_all.remove(plugin_id)
+
+            plugin.refresh_from_db()
             self.placeholder.delete_plugin(plugin)
+
             new_tree = self.get_plugins().values_list('pk', 'position')
             expected = [(pk, pos) for pos, pk in enumerate(plugin_tree_all, 1)]
             self.assertSequenceEqual(new_tree, expected)

cms/tests/test_views.py

@@ -101,6 +101,28 @@ def test_apphook_not_hooked(self):
             self.assertEqual(response.status_code, 200)
             self.apphook_clear()
 
+    def test_redirect_preview_in_edit_mode(self):
+
+        user = self.get_superuser()
+        page = create_page("page", "nav_playground.html", "fr")
+        page_content = create_page_content("en", "home", page, redirect="https://example.com")
+
+        page.set_as_homepage()
+
+        with self.login_user_context(user), force_language('fr'):
+            edit_url = get_object_edit_url(page_content, language='fr')
+            response = self.client.get(edit_url, follow=True)
+
+            expected = f"""
+                <div class="cms-screenblock">
+                <div class="cms-screenblock-inner">
+                <h1>This page has no preview!</h1>
+                <p>It is being redirected to: <a href="{page_content.redirect}">{page_content.redirect}</a></p>
+                </div>
+                </div>
+            """
+            self.assertContains(response, expected, count=1, html=True)
+
     def test_external_redirect(self):
         # test external redirect
         redirect_one = 'https://www.django-cms.org/'

cms/views.py

@@ -85,7 +85,6 @@ def details(request, slug):
     # Get a Page model object from the request
     site = get_current_site()
     page = get_page_from_request(request, use_path=slug)
-    toolbar = get_toolbar_from_request(request)
 
     if not page and not slug and not Page.objects.on_site(site).exists():
         # render the welcome page if the requested path is root "/"
@@ -190,9 +189,7 @@ def details(request, slug):
         redirect_url = _clean_redirect_url(redirect_url, request_language)
 
     if redirect_url:
-        if request.user.is_staff and toolbar.edit_mode_active:
-            toolbar.redirect_url = redirect_url
-        elif redirect_url not in own_urls:
+        if redirect_url not in own_urls:
             if get_cms_setting('REDIRECT_PRESERVE_QUERY_PARAMS'):
                 query_string = request.META.get('QUERY_STRING')
                 if query_string:
@@ -326,6 +323,11 @@ def render_object_endpoint(request, content_type_id, object_id, require_editable
     toolbar = get_toolbar_from_request(request)
     toolbar.set_object(content_type_obj)
 
+    if request.user.is_staff and toolbar.edit_mode_active:
+        redirect = getattr(content_type_obj, "redirect", None)
+        if isinstance(redirect, str):
+            toolbar.redirect_url = redirect
+
     if require_editable and not toolbar.object_is_editable():
         # If not editable, switch from edit to preview endpoint
         return HttpResponseRedirect(get_object_preview_url(content_type_obj))