8000 Fixed #6000 -- Placeholder inheritance takes precendence over fallbac… · django-cms/django-cms@2e60c71 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2e60c71

Browse files
czpythonczpython
committed
Fixed #6000 -- Placeholder inheritance takes precendence over fallback code
Bundled a fix for deeply nested placeholder inheritance.
1 parent 9aba239 commit 2e60c71

File tree

5 files changed

+119
-109
lines changed

5 files changed

+119
-109
lines changed

‎CHANGELOG.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
with placeholders.
88
* Fixed a bug where placeholder inheritance wouldn't work if the inherited placeholder
99
is cached in an ancestor page.
10+
* Fixed a regression where the code following a ``{% placeholder x or %}`` declaration,
11+
was rendered before attempting to inherit content from parent pages.
1012

1113

1214
=== 3.4.4 (2017-06-15) ===

‎cms/plugin_rendering.py

Lines changed: 47 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from django.utils.safestring import mark_safe
1313

1414
from cms.cache.placeholder import get_placeholder_cache, set_placeholder_cache
15-
from cms.exceptions import PlaceholderNotFound
1615
from cms.plugin_processors import (plugin_meta_context_processor, mark_safe_plugin_processor)
1716
from cms.toolbar.utils import get_toolbar_from_request
1817
from cms.utils import get_language_from_request
@@ -272,74 +271,61 @@ def render_placeholder(self, placeholder, context, language=None, page=None,
272271
context.pop()
273272
return mark_safe(toolbar_content + placeholder_content)
274273

275-
def render_page_placeholder(self, slot, context, inherit, nodelist=None):
276-
current_page = self.current_page
277-
278-
if not current_page:
274+
def render_page_placeholder(self, slot, context, inherit,
275+
page=None, nodelist=None, editable=True):
276+
if not self.current_page:
277+
# This method should only be used when rendering a cms page.
279278
return ''
280279

281-
content = self._render_page_placeholder(
282-
context=context,
283-
slot=slot,
284-
page=current_page,
285-
editable=True,
286-
nodelist=nodelist,
287-
)
280+
current_page = page or self.current_page
281+
placeholder_cache = self._placeholders_by_page_cache
288282

289-
if content or not current_page.parent_id:
290-
return content
283+
if current_page.pk not in placeholder_cache:
284+
# Instead of loading plugins for this one placeholder
285+
# try and load them for all placeholders on the page.
286+
self._preload_placeholders_for_page(current_page)
291287

292-
# don't display inherited plugins in edit mode, so that the user doesn't
293-
# mistakenly edit/delete them. This is a fix for issue #1303. See the discussion
294-
# there for possible enhancements
295-
if not inherit or self.toolbar.edit_mode:
296-
return content
288+
try:
289+
placeholder = placeholder_cache[current_page.pk][slot]
290+
except KeyError:
291+
content = ''
292+
else:
293+
content = self.render_placeholder(
294+
placeholder,
295+
context=context,
296+
page=current_page,
297+
editable=editable,
298+
use_cache=True,
299+
nodelist=None,
300+
)
297301

298-
if current_page.parent_id not in self._placeholders_by_page_cache:
302+
should_inherit = (
303+
inherit
304+
and not content and current_page.parent_id
299305
# The placeholder cache is primed when the first placeholder
300306
# is loaded. If the current page's parent is not in there,
301307
# it means its cache was never primed as it wasn't necessary.
302-
return content
303-
304-
cache_enabled = self.placeholder_cache_is_enabled()
305-
306-
for page in _get_page_ancestors(current_page):
307-
page_placeholders = self._placeholders_by_page_cache[page.pk]
308-
309-
try:
310-
placeholder = page_placeholders[slot]
311-
except KeyError:
312-
continue
313-
314-
try:
315-
has_prefetched_plugins = bool(placeholder._plugins_cache)
316-
except AttributeError:
317-
has_prefetched_plugins = False
318-
319-
if cache_enabled and not has_prefetched_plugins:
320-
cached_content = self._get_cached_placeholder_content(
321-
placeholder=placeholder,
322-
site_id=page.site_id,
323-
language=self.request_language,
324-
)
325-
has_cached_content = cached_content is not None
326-
else:
327-
has_cached_content = False
328-
329-
if has_prefetched_plugins or has_cached_content:
330-
# nodelist is set to None to avoid rendering the nodes inside
331-
# a {% placeholder or %} block tag.
332-
# When placeholder inheritance is used, we only care about placeholders
333-
# with plugins.
334-
inherited_content = self.render_placeholder(
335-
placeholder,
336-
context=context,
337-
page=page,
338-
editable=False,
339-
use_cache=True,
340-
nodelist=None,
341-
)
342-
return inherited_content
308+
and current_page.parent_id in placeholder_cache
309+
# don't display inherited plugins in edit mode, so that the user doesn't
310+
# mistakenly edit/delete them. This is a fix for issue #1303. See the discussion
311+
# there for possible enhancements
312+
and not self.toolbar.edit_mode
313+
)
314+
315+
if should_inherit:
316+
# nodelist is set to None to avoid rendering the nodes inside
317+
# a {% placeholder or %} block tag.
318+
content = self.render_page_placeholder(
319+
slot,
320+
context,
321+
inherit=True,
322+
page=current_page.parent,
323+
nodelist=None,
324+
editable=False,
325+
)
326+
327+
if not content and nodelist:
328+
return nodelist.render(context)
343329
return content
344330

345331
def render_static_placeholder(self, static_placeholder, context, nodelist=None):
@@ -509,49 +495,6 @@ def _get_cached_placeholder_content(self, placeholder, site_id, language):
509495
language_cache[placeholder.pk] = cached_value
510496
return language_cache.get(placeholder.pk)
511497

512-
def _get_page_placeholder(self, context, page, slot):
513-
"""
514-
Returns a Placeholder instance attached to page that
515-
matches the given slot.
516-
517-
A PlaceholderNotFound is raised if the placeholder is
518-
not present on the page template.
519-
"""
520-
placeholder_cache = self._placeholders_by_page_cache
521-
522-
if page.pk not in placeholder_cache:
523-
# Instead of loading plugins for this one placeholder
524-
# try and load them for all placeholders on the page.
525-
self._preload_placeholders_for_page(page)
526-
527-
try:
528-
placeholder = placeholder_cache[page.pk][slot]
529-
except KeyError:
530-
message = '"%s" placeholder not found' % slot
531-
raise PlaceholderNotFound(message)
532-
return placeholder
533-
534-
def _render_page_placeholder(self, context, slot, page, editable=True, nodelist=None):
535-
"""
536-
Renders a placeholder attached to a page.
537-
"""
538-
try:
539-
placeholder = self._get_page_placeholder(context, page, slot)
540-
except PlaceholderNotFound:
541-
if nodelist:
542-
return nodelist.render(context)
543-
return ''
544-
545-
content = self.render_placeholder(
546-
placeholder,
547-
context=context,
548-
page=page,
549-
editable=editable,
550-
use_cache=True,
551-
nodelist=nodelist,
552-
)
553-
return content
554-
555498
def _preload_placeholders_for_page(self, page, slots=None, inherit=False):
556499
"""
557500
Populates the internal plugin cache of each placeholder

‎cms/test_utils/project/templates/tests/rendering/inherit_with_or.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{% load cms_tags %}|{% placeholder "main" inherit or %}<p>Ultimate fallback</p>{% endplaceholder %}|

‎cms/tests/test_rendering.py

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
TEMPLATE_NAME = 'tests/rendering/base.html'
1818
INHERIT_TEMPLATE_NAME = 'tests/rendering/inherit.html'
19+
INHERIT_WITH_OR_TEMPLATE_NAME = 'tests/rendering/inherit_with_or.html'
1920

2021

2122
def sample_plugin_processor(instance, placeholder, rendered_content, original_context):
@@ -40,6 +41,7 @@ def sample_plugin_context_processor(instance, placeholder, original_context):
4041
CMS_TEMPLATES=[
4142
(TEMPLATE_NAME, TEMPLATE_NAME),
4243
(INHERIT_TEMPLATE_NAME, INHERIT_TEMPLATE_NAME),
44+
(INHERIT_WITH_OR_TEMPLATE_NAME, INHERIT_WITH_OR_TEMPLATE_NAME),
4345
('extra_context.html', 'extra_context.html')
4446
],
4547
)
@@ -87,6 +89,30 @@ def setUp(self):
8789
'reverse_id': u'renderingtestcase-reverse-id6',
8890
'text_sub': u'RenderingTestCase-sub6',
8991
}
92+
self.test_data7 = {
93+
'title': u'RenderingTestCase-title7',
94+
'slug': u'RenderingTestCase-slug7',
95+
'reverse_id': u'renderingtestcase-reverse-id7',
96+
'text_sub': u'RenderingTestCase-sub7',
97+
}
98+
self.test_data8 = {
99+
'title': u'RenderingTestCase-title8',
100+
'slug': u'RenderingTestCase-slug8',
101+
'reverse_id': u'renderingtestcase-reverse-id8',
102+
'text_sub': u'RenderingTestCase-sub8',
103+
}
104+
self.test_data9 = {
105+
'title': u'RenderingTestCase-title9',
106+
'slug': u'RenderingTestCase-slug9',
107+
'reverse_id': u'renderingtestcase-reverse-id9',
108+
'text_sub': u'RenderingTestCase-sub9',
109+
}
110+
self.test_data10 = {
111+
'title': u'RenderingTestCase-title10',
112+
'slug': u'RenderingTestCase-slug10',
113+
'reverse_id': u'renderingtestcase-reverse-id10',
114+
'text_sub': u'RenderingTestCase-sub10',
115+
}
90116
self.insert_test_content()
91117

92118
def insert_test_content(self):
@@ -161,6 +187,19 @@ def insert_test_content(self):
161187
add_plugin(self.test_placeholders6['sub'], 'TextPlugin', 'en',
162188
body=self.test_data6['text_sub'])
163189
p6.publish('en')
190+
p7 = create_page(self.test_data7['title'], INHERIT_TEMPLATE_NAME, 'en',
191+
slug=self.test_data7['slug'], parent=p6,
192+
reverse_id=self.test_data7['reverse_id'], published=True)
193+
p8 = create_page(self.test_data8['title'], INHERIT_WITH_OR_TEMPLATE_NAME, 'en',
194+
slug=self.test_data8['slug'], parent=p7,
195+
reverse_id=self.test_data8['reverse_id'], published=True)
196+
197+
p9 = create_page(self.test_data9['title'], INHERIT_WITH_OR_TEMPLATE_NAME, 'en',
198+
slug=self.test_data9['slug'],
199+
reverse_id=self.test_data9['reverse_id'], published=True)
200+
p10 = create_page(self.test_data10['title'] 10000 , INHERIT_WITH_OR_TEMPLATE_NAME, 'en',
201+
slug=self.test_data10['slug'], parent=p9,
202+
reverse_id=self.test_data10['reverse_id'], published=True)
164203

165204
# Reload test pages
166205
self.test_page = self.reload(p.publisher_public)
@@ -169,6 +208,10 @@ def insert_test_content(self):
169208
self.test_page4 = self.reload(p4.publisher_public)
170209
self.test_page5 = self.reload(p5.publisher_public)
171210
self.test_page6 = self.reload(p6.publisher_public)
211+
self.test_page7 = self.reload(p7.publisher_public)
212+
self.test_page8 = self.reload(p8.publisher_public)
213+
self.test_page9 = self.reload(p9.publisher_public)
214+
self.test_page10 = self.reload(p10.publisher_public)
172215

173216
def strip_rendered(self, content):
174217
return content.strip().replace(u"\n", u"")
@@ -559,7 +602,8 @@ def test_inherit_placeholder(self):
559602
self.assertEqual(r, u'|' + self.test_data5['text_main'] + '|' + self.test_data6['text_sub'])
560603

561604
def test_inherit_placeholder_with_cache(self):
562-
expected = u'|' + self.test_data5['text_main'] + '|' + self.test_data6['text_sub']
605+
expected_6 = u'|' + self.test_data5['text_main'] + '|' + self.test_data6['text_sub']
606+
expected_7 = u'|' + self.test_data5['text_main'] + '|'
563607
# Render the top-most page
564608
# This will cache its contents
565609
self.render(self.test_page)
@@ -568,7 +612,22 @@ def test_inherit_placeholder_with_cache(self):
568612
self.render(self.test_page5)
569613
# Render the target page
570614
# This should use the cached parent page content
571-
self.assertEqual(self.render(self.test_page6), expected)
615+
self.assertEqual(self.render(self.test_page6), expected_6)
616+
self.assertEqual(self.render(self.test_page7), expected_7)
617+
618+
self.render(self.test_page9)
619+
# This should use the cached parent page content
620+
self.assertEqual(self.render(self.test_page10), u'|<p>Ultimate fallback</p>|')
621+
622+
def test_inherit_placeholder_with_or(self):
623+
# Tests that the "or" statement used in a {% placeholder %}
624+
# declaration is used as the last fallback when inheritance
625+
# fails to find content.
626+
expected_8 = u'|' + self.test_data5['text_main'] + '|'
627+
self.assertEqual(self.render(self.test_page8), expected_8)
628+
629+
expected_10 = u'|<p>Ultimate fallback</p>|'
630+
self.assertEqual(self.render(self.test_page10), expected_10)
572631

573632
def test_inherit_placeholder_override(self):
574633
# Tests that the user can override the inherited content

‎cms/tests/test_templatetags.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,16 @@ class FakeRequest(object):
253253
current_page = page
254254
GET = {'language': 'en'}
255255

256+
self.assertObjectDoesNotExist(page.placeholders.all(), slot='col_right')
256257
context = self.get_context(page=page)
257258
content_renderer = context['cms_content_renderer']
258-
placeholder = content_renderer._get_page_placeholder(context, page, 'col_right')
259-
page.placeholders.get(slot='col_right')
260-
self.assertEqual(placeholder.slot, 'col_right')
259+
content_renderer.render_page_placeholder(
260+
'col_right',
261+
context,
262+
inherit=False,
263+
page=page,
264+
)
265+
self.assertObjectExist(page.placeholders.all(), slot='col_right')
261266

262267
def test_render_plugin_toolbar_config(self):
263268
"""

0 commit comments

Comments
 (0)
0