Skip to content

Commit

Permalink
Explicitly test whether render / render_basic will accept a 'context'…
Browse files Browse the repository at this point in the history
… kwarg

This avoids unexpected behaviour when the method legitimately accepts a context
kwarg, but happens to throw an unrelated TypeError - in this situation, the final
output (or error diagnostics) will behave as if the context was never passed,
making debugging difficult. See #2786 (comment)
  • Loading branch information
gasman committed Jul 5, 2016
1 parent d852016 commit 60851f4
Showing 1 changed file with 19 additions and 14 deletions.
33 changes: 19 additions & 14 deletions wagtail/wagtailcore/blocks/base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import absolute_import, unicode_literals

import collections
import inspect
import warnings
from importlib import import_module

Expand Down Expand Up @@ -226,13 +227,13 @@ def render(self, value)
In Wagtail 1.8, when support for context-less `render` methods is dropped,
this method will be deleted (and calls to it replaced with a direct call to `render`).
"""
try:
return self.render(value, context=context)
except TypeError:
# retry without the 'context' kwarg
result = self.render(value)
argspec = inspect.getargspec(self.render)

# if this is successful, the block needs updating for Wagtail >=1.6 -
if ('context' in argspec.args) or argspec.keywords is not None:
# this render method can receive a 'context' kwarg, so we're good
return self.render(value, context=context)
else:
# this render method needs updating for Wagtail >=1.6 -
# output a deprecation warning

# find the specific parent class that defines `render` by stepping through the MRO,
Expand All @@ -247,7 +248,9 @@ def render(self, value)
"keyword argument" % class_with_render_method,
category=RemovedInWagtail18Warning
)
return result

# fall back on a call to 'render' without the context kwarg
return self.render(value)

def render(self, value, context=None):
"""
Expand Down Expand Up @@ -277,13 +280,13 @@ def render_basic(self, value)
In Wagtail 1.8, when support for context-less `render_basic` methods is dropped,
this method will be deleted (and calls to it replaced with a direct call to `render_basic`).
"""
try:
return self.render_basic(value, context=context)
except TypeError:
# retry without the 'context' kwarg
result = self.render_basic(value)
argspec = inspect.getargspec(self.render_basic)

# if this is successful, the block needs updating for Wagtail >=1.6 -
if ('context' in argspec.args) or argspec.keywords is not None:
# this render_basic method can receive a 'context' kwarg, so we're good
return self.render_basic(value, context=context)
else:
# this render_basic method needs updating for Wagtail >=1.6 -
# output a deprecation warning

# find the specific parent class that defines `render_basic` by stepping through the MRO,
Expand All @@ -298,7 +301,9 @@ def render_basic(self, value)
"keyword argument" % class_with_render_basic_method,
category=RemovedInWagtail18Warning
)
return result

# fall back on a call to 'render_basic' without the context kwarg
return self.render_basic(value)

def render_basic(self, value, context=None):
"""
Expand Down

0 comments on commit 60851f4

Please sign in to comment.