Skip to content

Loading…

When using the menu tag wrong (eg give a page as 'namespace') the menu ta #935

Merged
merged 3 commits into from

4 participants

@ojii

When using the menu tag wrong (eg give a page as 'namespace') the menu tag should fail fast (or ignore the value). This fix prevents some infinite loops in some cases.

Jonas Obrist added some commits
Jonas Obrist When using the menu tag wrong (eg give a page as 'namespace') the men…
…u tag should fail fast (or ignore the value). This fix prevents some infinite loops in some cases.
33a759d
Jonas Obrist Merge branch 'develop' of github.com:divio/django-cms into failfast-o…
…n-invalid-menu-usage
246e6dc
@sankroh

Thanks for fixing this so quick!

@chrisglass

LGTM.

Shouldn't this be backported to classy? Seems to be a useful case for other projects too... And while we're at it, maybe a tag to enforce integer arguments would be useful too? :) That's besides the scope for this pull, but would be interesting in classy I assume.

@ojii

There already is one for ints in classytags: https://github.com/ojii/django-classy-tags/blob/master/classytags/arguments.py#L89

And I agree, the StringArgument might be useful.

@chrisglass

Waiting for classy tags release to pypi, then we will bump the requirements in setup.py and use classy's then version of StrictStringValue instead (or whatever it's called).

Then we merge :)

@ojii

The classytags update will probably happen tonight. This means the b2/rc1 release of the CMS is re-scheduled to Saturday.

@ojii

alright fixed it in classytags

@fivethreeo fivethreeo merged commit 16086e1 into divio:develop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 10, 2011
  1. When using the menu tag wrong (eg give a page as 'namespace') the men…

    Jonas Obrist committed
    …u tag should fail fast (or ignore the value). This fix prevents some infinite loops in some cases.
  2. Merge branch 'develop' of github.com:divio/django-cms into failfast-o…

    Jonas Obrist committed
    …n-invalid-menu-usage
Commits on Aug 11, 2011
  1. Moved to newer classytags

    Jonas Obrist committed
Showing with 15 additions and 7 deletions.
  1. +9 −1 cms/tests/menu.py
  2. +1 −1 docs/getting_started/installation.rst
  3. +4 −4 menus/templatetags/menu_tags.py
  4. +1 −1 setup.py
View
10 cms/tests/menu.py
@@ -13,7 +13,7 @@
from django.contrib.auth.models import AnonymousUser, User, Permission, Group
from django.contrib.contenttypes.models import ContentType
from django.contrib.sites.models import Site
-from django.template import Template
+from django.template import Template, TemplateSyntaxError
from menus.base import NavigationNode
from menus.menu_pool import menu_pool, _build_nodes_inner_for_one_menu
from menus.utils import mark_descendants, find_selected, cut_levels
@@ -71,6 +71,14 @@ def get_level(self, num):
def get_all_pages(self):
return Page.objects.all()
+ def test_menu_failfast_on_invalid_usage(self):
+ context = self.get_context()
+ context['child'] = self.get_page(1)
+ # test standard show_menu
+ with SettingsOverride(DEBUG=True, TEMPLATE_DEBUG=True):
+ tpl = Template("{% load menu_tags %}{% show_menu 0 0 0 0 'menu/menu.html' child %}")
+ self.assertRaises(TemplateSyntaxError, tpl.render, context)
+
def test_basic_cms_menu(self):
self.assertEqual(len(menu_pool.menus), 1)
response = self.client.get(self.get_pages_root()) # path = '/'
View
2 docs/getting_started/installation.rst
@@ -13,7 +13,7 @@ Requirements
* `Django`_ 1.2.5 (or a 1.3.x release).
* `South`_ 0.7.2 or higher
* `PIL`_ 1.1.6 or higher
-* `django-classy-tags`_ 0.3.2 or higher
+* `django-classy-tags`_ 0.3.4.1 or higher
* `django-mptt`_ 0.4.2 or higher
* `django-sekizai`_ 0.4.2 or higher
* An installed and working instance of one of the databases listed in the
View
8 menus/templatetags/menu_tags.py
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*-
-from classytags.arguments import IntegerArgument, Argument
+from classytags.arguments import IntegerArgument, Argument, StringArgument
from classytags.core import Options
from classytags.helpers import InclusionTag
from django import template
@@ -104,9 +104,9 @@ class ShowMenu(InclusionTag):
IntegerArgument('to_level', default=100, required=False),
IntegerArgument('extra_inactive', default=0, required=False),
IntegerArgument('extra_active', default=1000, required=False),
- Argument('template', default='menu/menu.html', required=False),
- Argument('namespace', default=None, required=False),
- Argument('root_id', default=None, required=False),
+ StringArgument('template', default='menu/menu.html', required=False),
+ StringArgument('namespace', default=None, required=False),
+ StringArgument('root_id', default=None, required=False),
Argument('next_page', default=None, required=False),
)
View
2 setup.py
@@ -29,7 +29,7 @@
classifiers=CLASSIFIERS,
install_requires=[
'Django>=1.2.5',
- 'django-classy-tags>=0.3.3',
+ 'django-classy-tags>=0.3.4.1',
'south>=0.7.2',
'django-mptt>=0.4.2',
'django-sekizai>=0.4.2',
Something went wrong with that request. Please try again.