diff --git a/CHANGELOG.md b/CHANGELOG.md index 6216395e..48d24a9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Add support for Django 5.0 ([#241](https://github.com/torchbox/django-pattern-library/pull/241)) +### Changed + +- From Django >= 4.0, calls to `Node.render()` must always return a string, but this app previously allowed non-string values to be passed in the `default_html` parameter to `override_tag`. Passing a non-string now raises a `TypeError` when using Django >= 4.0, and raises a warning for older versions ([issue #211](https://github.com/torchbox/django-pattern-library/issues/211)). + ## [1.1.0](https://github.com/torchbox/django-pattern-library/releases/tag/v1.1.0) - 2023-10-25 ### Added diff --git a/pattern_library/monkey_utils.py b/pattern_library/monkey_utils.py index b502b96a..12eeb3e8 100644 --- a/pattern_library/monkey_utils.py +++ b/pattern_library/monkey_utils.py @@ -1,5 +1,7 @@ +import inspect import logging -from typing import Optional +import typing +import warnings import django from django.template.library import SimpleNode @@ -11,7 +13,9 @@ def override_tag( - register: django.template.Library, name: str, default_html: Optional[str] = None + register: django.template.Library, + name: str, + default_html: typing.Optional[typing.Any] = UNSPECIFIED, ): """ An utility that helps you override original tags for use in your pattern library. @@ -79,6 +83,23 @@ def node_render(context): # See https://github.com/torchbox/django-pattern-library/issues/166. return str(result) elif default_html is not UNSPECIFIED: + # Ensure default_html is a string. + if not isinstance(default_html, str): + # Save the caller for the override tag in case it's needed for error reporting. + trace = inspect.stack()[1] + if django.VERSION < (4, 0): + warnings.warn( + "default_html argument to override_tag should be a string to ensure compatibility " + 'with Django >= 4.0 (line %s in "%s")' + % (trace.lineno, trace.filename), + Warning, + ) + else: + raise TypeError( + 'default_html argument to override_tag must be a string (line %s in "%s")' + % (trace.lineno, trace.filename) + ) + # Render provided default; # if no stub data supplied. return default_html diff --git a/tests/templates/patterns/atoms/tags_test_atom/invalid_tags_test_atom.html.fail b/tests/templates/patterns/atoms/tags_test_atom/invalid_tags_test_atom.html.fail new file mode 100644 index 00000000..b88ab0a1 --- /dev/null +++ b/tests/templates/patterns/atoms/tags_test_atom/invalid_tags_test_atom.html.fail @@ -0,0 +1,5 @@ +{% load test_tags_invalid %} + +MARMA{% default_html_tag_invalid empty_string %}LADE01 +MARMA{% default_html_tag_invalid none %}LADE02 +MARMA{% default_html_tag_invalid dict %}LADE03 diff --git a/tests/templatetags/test_tags_invalid.py b/tests/templatetags/test_tags_invalid.py new file mode 100644 index 00000000..9d6ba7cd --- /dev/null +++ b/tests/templatetags/test_tags_invalid.py @@ -0,0 +1,15 @@ +from django import template + +from pattern_library.monkey_utils import override_tag + +register = template.Library() + + +@register.simple_tag() +def default_html_tag_invalid(arg=None): + "Just raise an exception, never do anything" + raise Exception("default_tag raised an exception") + + +# Test overriding tag with a default_html that's not valid in Django >= 4.0 +override_tag(register, "default_html_tag_invalid", default_html=[1, 2, 3]) diff --git a/tests/tests/test_tags.py b/tests/tests/test_tags.py index ee88b900..6d34f0a1 100644 --- a/tests/tests/test_tags.py +++ b/tests/tests/test_tags.py @@ -1,4 +1,9 @@ -from django.test import SimpleTestCase +from unittest.mock import patch + +from django.shortcuts import render +from django.test import RequestFactory, SimpleTestCase + +from pattern_library import get_pattern_context_var_name from .utils import reverse @@ -43,3 +48,54 @@ def test_falsey_default_html_overide(self): self.assertContains(response, "POTATO1") self.assertContains(response, "POTANoneTO2") self.assertContains(response, "POTA0TO3") + + +class TagsTestFailCase(SimpleTestCase): + def test_bad_default_html_warning(self): + """ + Test that the library raises a warning when passing a non-string `default_html` argument to `override_tag` + in Django < 4.0 + """ + with patch("django.VERSION", (3, 2, 0, "final", 0)): + with self.assertWarns(Warning) as cm: + template_name = ( + "patterns/atoms/tags_test_atom/invalid_tags_test_atom.html.fail" + ) + request = RequestFactory().get("/") + + # Rendering the template with a non-string `default_html` argument will cause Django >= 4 to raise + # a `TypeError`, which we need to catch and ignore in order to check that the warning is raised + try: + render( + request, + template_name, + context={get_pattern_context_var_name(): True}, + ) + except TypeError: + pass + + self.assertIn( + "default_html argument to override_tag should be a string to ensure compatibility with Django", + str(cm.warnings[0]), + ) + + def test_bad_default_html_error(self): + """ + Test that the library raises a TypeError when passing a non-string `default_html` argument to `override_tag` + in Django >= 4.0 + """ + with patch("django.VERSION", (4, 2, 0, "final", 0)): + with self.assertRaises(TypeError) as cm: + template_name = ( + "patterns/atoms/tags_test_atom/invalid_tags_test_atom.html.fail" + ) + request = RequestFactory().get("/") + render( + request, + template_name, + context={get_pattern_context_var_name(): True}, + ) + self.assertIn( + "default_html argument to override_tag must be a string", + str(cm.exception), + )