From 6dd45c557c42a35342d9d8f6842b835420fd8f9c Mon Sep 17 00:00:00 2001 From: Ben Dickinson Date: Thu, 24 Sep 2020 17:16:17 +0100 Subject: [PATCH] Fix broken page/fragment detection --- .github/PULL_REQUEST_TEMPLATE.md | 1 + CHANGELOG.md | 6 ++ README.md | 72 ++++++++++++++++--- pattern_library/__init__.py | 27 ++++--- pattern_library/loader_tags.py | 14 ++-- pattern_library/utils.py | 26 +++++++ pattern_library/views.py | 25 +++++-- tests/settings.py | 1 + .../non-patterns/variable_include.html | 1 + .../patterns/atoms/test_extends/base.html | 1 + .../patterns/atoms/test_extends/extended.html | 3 + .../patterns/atoms/test_extends/extended.yaml | 2 + .../atoms/test_includes/test_includes.html | 1 + .../atoms/test_includes/test_includes.yaml | 3 + tests/templates/patterns/base.html | 9 ++- tests/templates/patterns/base_page.html | 3 + .../patterns/pages/test_page/test_page.html | 3 + .../patterns/pages/test_page/test_page.yaml | 11 +++ tests/tests/test_utils.py | 35 +++++++++ tests/tests/test_views.py | 37 ++++++++++ tox.ini | 3 +- 21 files changed, 250 insertions(+), 34 deletions(-) create mode 100644 tests/templates/non-patterns/variable_include.html create mode 100644 tests/templates/patterns/atoms/test_extends/base.html create mode 100644 tests/templates/patterns/atoms/test_extends/extended.html create mode 100644 tests/templates/patterns/atoms/test_extends/extended.yaml create mode 100644 tests/templates/patterns/base_page.html create mode 100644 tests/templates/patterns/pages/test_page/test_page.html create mode 100644 tests/templates/patterns/pages/test_page/test_page.yaml create mode 100644 tests/tests/test_utils.py diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 55b84438..f02f29f0 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -13,3 +13,4 @@ Fixes # (issue) - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes +- [ ] I have added an appropriate CHANGELOG entry diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a93ec34..0d90beda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ### Fixed +- Pages and fragments are now handled correctly again ([#119](https://github.com/torchbox/django-pattern-library/issues/119)) - PyPI package metadata now uses absolute URLs to GitHub ([#120](https://github.com/torchbox/django-pattern-library/issues/120)). ## [0.2.9] - 2020-07-29 @@ -57,3 +58,8 @@ ### Added - Compatibility with Django 2.2 + +[0.2.9]: https://github.com/torchbox/django-pattern-library/releases/tag/v0.2.9 +[0.2.8]: https://github.com/torchbox/django-pattern-library/releases/tag/v0.2.8 +[0.2.5]: https://github.com/torchbox/django-pattern-library/releases/tag/v0.2.5 +[0.2.4]: https://github.com/torchbox/django-pattern-library/releases/tag/v0.2.4 diff --git a/README.md b/README.md index 4669e680..33baef00 100644 --- a/README.md +++ b/README.md @@ -2,14 +2,13 @@ [![PyPI](https://img.shields.io/pypi/v/django-pattern-library.svg)](https://pypi.org/project/django-pattern-library/) [![PyPI downloads](https://img.shields.io/pypi/dm/django-pattern-library.svg)](https://pypi.org/project/django-pattern-library/) [![Travis](https://travis-ci.com/torchbox/django-pattern-library.svg?branch=master)](https://travis-ci.com/torchbox/django-pattern-library) [![Total alerts](https://img.shields.io/lgtm/alerts/g/torchbox/django-pattern-library.svg?logo=lgtm&logoWidth=18)](https://lgtm.com/projects/g/torchbox/django-pattern-library/alerts/) -A module for Django that helps you to build pattern libraries and follow the -[Atomic design](http://bradfrost.com/blog/post/atomic-web-design/) methodology. +A module for Django that helps you to build pattern libraries. ![Screenshot of the pattern library UI, with navigation, pattern rendering, and configuration](https://raw.githubusercontent.com/torchbox/django-pattern-library/master/.github/pattern-library-screenshot.webp) ## Documentation -Documentation is located in GitHub in [`docs/`](https://github.com/torchbox/django-pattern-library/tree/master/docs). +Documentation is located on GitHub in [`docs/`](https://github.com/torchbox/django-pattern-library/tree/master/docs). ## Objective @@ -29,9 +28,38 @@ To learn more about how this package can be used, have a look at our Wagtail Spa [![Reusable UI components: A journey from React to Wagtail](https://raw.githubusercontent.com/torchbox/django-pattern-library/master/.github/pattern-library-talk-youtube.webp)](https://www.youtube.com/watch?v=isrOufI7TKc) +## Concepts +To understand how `django-pattern-library` works, the following concepts are important. + +### Patterns +Any template that is displayed by the pattern library is referred to as a pattern. Patterns are divided into two categories: fragments and pages. + +### Fragments +A fragment is a pattern whose markup does not include all of the resources (typically CSS and Javascript) for it to be displayed correctly on its own. This is typical for reusable component templates which depend on global stylesheets or Javascript bundles to render and behave correctly. + +To enable them to be correctly displayed in the pattern library, `django-pattern-library` will inject the rendered markup of fragments into the **pattern base template** specified by `PATTERN_LIBRARY['PATTERN_BASE_TEMPLATE_NAME']`. + +This template should include references to any required static files. The rendered markup of fragments will be available in the `pattern_library_rendered_pattern` context variable (see the tests for [an example](https://github.com/torchbox/django-pattern-library/blob/master/tests/templates/patterns/base.html)). + +### Pages +In contrast to fragments, pages are patterns that include everything they need to be displayed correctly in their markup. Pages are defined by `PATTERN_LIBRARY['BASE_TEMPLATE_NAMES']`. + +Any template in that list — or that extends a template in that list — is considered a page and will be displayed as-is when rendered in the pattern library. + +It is common practice for page templates to extend the pattern base template to avoid duplicate references to stylesheets and Javascript bundles. Again, [an example](https://github.com/torchbox/django-pattern-library/blob/master/tests/templates/patterns/base_page.html) of this can be seen in the tests. + ## How to install -In your Django settings, add `pattern_library` into your `INSTALLED_APPS`, and `pattern_library.loader_tags` into the `TEMPLATES` setting. For example: +First install the library: + +```sh +pip install django-pattern-library +# ... or... +poetry add django-pattern-library +``` + + +Then, in your Django settings, add `pattern_library` into your `INSTALLED_APPS`, and `pattern_library.loader_tags` to `OPTIONS['builtins']` into the `TEMPLATES` setting. For example: ```python INSTALLED_APPS = [ @@ -58,18 +86,42 @@ TEMPLATES = [ ] ``` -Note that this module only supports the Django template backend out of the box. +Note that this module only supports the Django template backend. -Set the `PATTERN_LIBRARY_TEMPLATE_DIR` setting to point to a template directory with your patterns: +### Settings + +Next, set the `PATTERN_LIBRARY` setting. Here's an example showing the defaults: ```python -PATTERN_LIBRARY_TEMPLATE_DIR = os.path.join(BASE_DIR, 'project_styleguide', 'templates') +PATTERN_LIBRARY = { + # PATTERN_BASE_TEMPLATE_NAME is the template that fragments will be wrapped with. + # It should include any required CSS and JS and output + # `pattern_library_rendered_pattern` from context. + 'PATTERN_BASE_TEMPLATE_NAME': 'patterns/base.html', + # Any template in BASE_TEMPLATE_NAMES or any template that extends a template in + # BASE_TEMPLATE_NAMES is a "page" and will be rendered as-is without being wrapped. + 'BASE_TEMPLATE_NAMES': ['patterns/base_page.html'], + 'TEMPLATE_SUFFIX': '.html', + # SECTIONS controls the groups of templates that appear in the navigation. The keys + # are the group titles and the values are lists of template name prefixes that will + # be searched to populate the groups. + 'SECTIONS': ( + ('atoms', ['patterns/atoms']), + ('molecules', ['patterns/molecules']), + ('organisms', ['patterns/organisms']), + ('templates', ['patterns/templates']), + ('pages', ['patterns/pages']), + ), +} + ``` -Note that `PATTERN_LIBRARY_TEMPLATE_DIR` must be available for -[template loaders](https://docs.djangoproject.com/en/1.11/ref/templates/api/#loader-types). +Note that the templates in your `PATTERN_LIBRARY` settings must be available to your project's +[template loaders](https://docs.djangoproject.com/en/3.1/ref/templates/api/#loader-types). + +### URLs -Include `pattern_library.urls` into your `urlpatterns`. Here's an example `urls.py`: +Include `pattern_library.urls` in your `urlpatterns`. Here's an example `urls.py`: ```python from django.apps import apps diff --git a/pattern_library/__init__.py b/pattern_library/__init__.py index d6bbf060..994f877c 100644 --- a/pattern_library/__init__.py +++ b/pattern_library/__init__.py @@ -1,10 +1,17 @@ - - default_app_config = 'pattern_library.apps.PatternLibraryAppConfig' DEFAULT_SETTINGS = { - 'BASE_TEMPLATE_NAME': 'patterns/base.html', + # PATTERN_BASE_TEMPLATE_NAME is the template that fragments will be wrapped with. + # It should include any required CSS and JS and output + # `pattern_library_rendered_pattern` from context. + 'PATTERN_BASE_TEMPLATE_NAME': 'patterns/base.html', + # Any template in BASE_TEMPLATE_NAMES or any template that extends a template in + # BASE_TEMPLATE_NAMES is a "page" and will be rendered as-is without being wrapped. + 'BASE_TEMPLATE_NAMES': ['patterns/base_page.html'], 'TEMPLATE_SUFFIX': '.html', + # SECTIONS controls the groups of templates that appear in the navigation. The keys + # are the group titles and the value are lists of template name prefixes that will + # be searched to populate the groups. 'SECTIONS': ( ('atoms', ['patterns/atoms']), ('molecules', ['patterns/molecules']), @@ -15,25 +22,27 @@ } -def get_from_settings(attr): +def get_setting(attr): from django.conf import settings - library_settings = DEFAULT_SETTINGS.copy() library_settings.update(getattr(settings, 'PATTERN_LIBRARY', {})) - return library_settings.get(attr) def get_pattern_template_suffix(): - return get_from_settings('TEMPLATE_SUFFIX') + return get_setting('TEMPLATE_SUFFIX') def get_pattern_base_template_name(): - return get_from_settings('BASE_TEMPLATE_NAME') + return get_setting('PATTERN_BASE_TEMPLATE_NAME') + + +def get_base_template_names(): + return get_setting('BASE_TEMPLATE_NAMES') def get_sections(): - return get_from_settings('SECTIONS') + return get_setting('SECTIONS') def get_pattern_context_var_name(): diff --git a/pattern_library/loader_tags.py b/pattern_library/loader_tags.py index 6b898e69..b74b6fb7 100644 --- a/pattern_library/loader_tags.py +++ b/pattern_library/loader_tags.py @@ -17,7 +17,8 @@ class ExtendsNode(DjangoExtendsNode): """ def render(self, context): if is_pattern_library_context(context): - parent_context = get_pattern_context(self.parent_name.var) + parent_name = self.parent_name.resolve(context) + parent_context = get_pattern_context(parent_name) if parent_context: # We want parent_context to appear later in the lookup process # than context of the actual template. @@ -59,7 +60,8 @@ class IncludeNode(DjangoIncludeNode): """ def render(self, context): if is_pattern_library_context(context): - pattern_context = get_pattern_context(self.template.var) + template = self.template.resolve(context) + pattern_context = get_pattern_context(template) extra_context = {name: var.resolve(context) for name, var in self.extra_context.items()} if self.isolated_context: @@ -89,8 +91,8 @@ def render(self, context): @register.tag('extends') def do_extends(parser, token): """ - Copy if Django's built-in {% extends ... %} tag that uses the custom - ExtendsNode to allow us to load dump data for pattern library. + A copy of Django's built-in {% extends ... %} tag that uses our custom + ExtendsNode to allow us to load dummy context for the pattern library. """ bits = token.split_contents() if len(bits) != 2: @@ -106,8 +108,8 @@ def do_extends(parser, token): @register.tag('include') def do_include(parser, token): """ - Copy if Django's built-in {% include ... %} tag that uses the custom - IncludeNode to allow us to load dump data for pattern library. + A copy of Django's built-in {% include ... %} tag that uses our custom + IncludeNode to allow us to load dummy context for the pattern library. """ bits = token.split_contents() if len(bits) < 2: diff --git a/pattern_library/utils.py b/pattern_library/utils.py index e139b7b9..4a51762f 100644 --- a/pattern_library/utils.py +++ b/pattern_library/utils.py @@ -3,7 +3,9 @@ import re from django.template import TemplateDoesNotExist +from django.template.context import Context from django.template.loader import get_template, render_to_string +from django.template.loader_tags import ExtendsNode from django.template.loaders.app_directories import get_app_template_dirs from django.utils.safestring import mark_safe @@ -189,3 +191,27 @@ def render_pattern(request, template_name, allow_non_patterns=False): context = get_pattern_context(template_name) context[get_pattern_context_var_name()] = True return render_to_string(template_name, request=request, context=context) + + +def get_template_ancestors(template_name, context=None, ancestors=None): + """ + Returns a list of template names, starting with provided name + and followed by the names of any templates that extends until + the most extended template is reached. + """ + if ancestors is None: + ancestors = [template_name] + + if context is None: + context = Context() + + pattern_template = get_template(template_name) + + for node in pattern_template.template.nodelist: + if isinstance(node, ExtendsNode): + parent_template_name = node.parent_name.resolve(context) + ancestors.append(parent_template_name) + get_template_ancestors(parent_template_name, context=context, ancestors=ancestors) + break + + return ancestors diff --git a/pattern_library/views.py b/pattern_library/views.py index b0adf022..d6448cb6 100644 --- a/pattern_library/views.py +++ b/pattern_library/views.py @@ -1,17 +1,20 @@ -from django.http import Http404 +from django.http import Http404, HttpResponse from django.template.loader import get_template from django.utils.decorators import method_decorator from django.utils.html import escape from django.views.decorators.clickjacking import xframe_options_sameorigin from django.views.generic.base import TemplateView -from pattern_library import get_pattern_base_template_name +from pattern_library import ( + get_base_template_names, get_pattern_base_template_name +) from pattern_library.exceptions import ( PatternLibraryEmpty, TemplateIsNotPattern ) from pattern_library.utils import ( - get_pattern_config, get_pattern_config_str, get_pattern_markdown, - get_pattern_templates, get_sections, is_pattern, render_pattern + get_pattern_config, get_pattern_config_str, get_pattern_context, + get_pattern_markdown, get_pattern_templates, get_sections, + get_template_ancestors, is_pattern, render_pattern ) @@ -74,12 +77,20 @@ class RenderPatternView(TemplateView): @method_decorator(xframe_options_sameorigin) def get(self, request, pattern_template_name=None): + pattern_template_ancestors = get_template_ancestors( + pattern_template_name, + context=get_pattern_context(self.kwargs['pattern_template_name']), + ) + pattern_is_fragment = set(pattern_template_ancestors).isdisjoint(set(get_base_template_names())) + try: rendered_pattern = render_pattern(request, pattern_template_name) except TemplateIsNotPattern: raise Http404 - context = self.get_context_data() - context['pattern_library_rendered_pattern'] = rendered_pattern + if pattern_is_fragment: + context = self.get_context_data() + context['pattern_library_rendered_pattern'] = rendered_pattern + return self.render_to_response(context) - return self.render_to_response(context) + return HttpResponse(rendered_pattern) diff --git a/tests/settings.py b/tests/settings.py index e6607b96..b25ef535 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -24,6 +24,7 @@ 'SECTIONS': [ ('atoms', ['patterns/atoms']), ('molecules', ['patterns/molecules']), + ('pages', ['patterns/pages']), ], } diff --git a/tests/templates/non-patterns/variable_include.html b/tests/templates/non-patterns/variable_include.html new file mode 100644 index 00000000..be595c6b --- /dev/null +++ b/tests/templates/non-patterns/variable_include.html @@ -0,0 +1 @@ +included content from variable diff --git a/tests/templates/patterns/atoms/test_extends/base.html b/tests/templates/patterns/atoms/test_extends/base.html new file mode 100644 index 00000000..43c5ba6b --- /dev/null +++ b/tests/templates/patterns/atoms/test_extends/base.html @@ -0,0 +1 @@ +{% block content %}base content{% endblock %} diff --git a/tests/templates/patterns/atoms/test_extends/extended.html b/tests/templates/patterns/atoms/test_extends/extended.html new file mode 100644 index 00000000..4316cf39 --- /dev/null +++ b/tests/templates/patterns/atoms/test_extends/extended.html @@ -0,0 +1,3 @@ +{% extends parent_template_name %} + +{% block content %}{{ block.super }} - extended content{% endblock %} diff --git a/tests/templates/patterns/atoms/test_extends/extended.yaml b/tests/templates/patterns/atoms/test_extends/extended.yaml new file mode 100644 index 00000000..8c2e3283 --- /dev/null +++ b/tests/templates/patterns/atoms/test_extends/extended.yaml @@ -0,0 +1,2 @@ +context: + parent_template_name: patterns/atoms/test_extends/base.html diff --git a/tests/templates/patterns/atoms/test_includes/test_includes.html b/tests/templates/patterns/atoms/test_includes/test_includes.html index 859af903..f21a94a3 100644 --- a/tests/templates/patterns/atoms/test_includes/test_includes.html +++ b/tests/templates/patterns/atoms/test_includes/test_includes.html @@ -1,5 +1,6 @@ {% load test_tags %} {% include 'non-patterns/include.html' %} +{% include variable_include %} {% error_tag include %} diff --git a/tests/templates/patterns/atoms/test_includes/test_includes.yaml b/tests/templates/patterns/atoms/test_includes/test_includes.yaml index 4be920d4..757b9209 100644 --- a/tests/templates/patterns/atoms/test_includes/test_includes.yaml +++ b/tests/templates/patterns/atoms/test_includes/test_includes.yaml @@ -1,3 +1,6 @@ +context: + variable_include: non-patterns/variable_include.html + tags: error_tag: include: diff --git a/tests/templates/patterns/base.html b/tests/templates/patterns/base.html index f2f049d8..453fb887 100644 --- a/tests/templates/patterns/base.html +++ b/tests/templates/patterns/base.html @@ -1 +1,8 @@ -{% block content %}{{ pattern_library_rendered_pattern }}{% endblock %} + + + {% block title %}Fragment{% endblock %} + + + {% block content %}{{ pattern_library_rendered_pattern }}{% endblock %} + + diff --git a/tests/templates/patterns/base_page.html b/tests/templates/patterns/base_page.html new file mode 100644 index 00000000..52a1cfe5 --- /dev/null +++ b/tests/templates/patterns/base_page.html @@ -0,0 +1,3 @@ +{% extends 'patterns/base.html' %} + +{% block title %}Page{% endblock %} diff --git a/tests/templates/patterns/pages/test_page/test_page.html b/tests/templates/patterns/pages/test_page/test_page.html new file mode 100644 index 00000000..5264f100 --- /dev/null +++ b/tests/templates/patterns/pages/test_page/test_page.html @@ -0,0 +1,3 @@ +{% extends 'patterns/base_page.html' %} + +{% block content %}{{ page.body }}{% endblock %} diff --git a/tests/templates/patterns/pages/test_page/test_page.yaml b/tests/templates/patterns/pages/test_page/test_page.yaml new file mode 100644 index 00000000..9b8ba8fa --- /dev/null +++ b/tests/templates/patterns/pages/test_page/test_page.yaml @@ -0,0 +1,11 @@ +context: + page: + body: > +

+ Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod + tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, + quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo + consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse + cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat + non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. +

diff --git a/tests/tests/test_utils.py b/tests/tests/test_utils.py new file mode 100644 index 00000000..81b937ef --- /dev/null +++ b/tests/tests/test_utils.py @@ -0,0 +1,35 @@ +from django.test import SimpleTestCase + +from pattern_library.utils import get_template_ancestors + + +class TestGetTemplateAncestors(SimpleTestCase): + def test_page(self): + self.assertEqual( + get_template_ancestors('patterns/pages/test_page/test_page.html'), + [ + 'patterns/pages/test_page/test_page.html', + 'patterns/base_page.html', + 'patterns/base.html', + ], + ) + + def test_fragment(self): + self.assertEqual( + get_template_ancestors('patterns/atoms/test_atom/test_atom.html'), + [ + 'patterns/atoms/test_atom/test_atom.html', + ], + ) + + def test_parent_template_from_variable(self): + self.assertEqual( + get_template_ancestors( + 'patterns/atoms/test_extends/extended.html', + context={'parent_template_name': 'patterns/base.html'}, + ), + [ + 'patterns/atoms/test_extends/extended.html', + 'patterns/base.html', + ], + ) diff --git a/tests/tests/test_views.py b/tests/tests/test_views.py index 802d0721..471259e9 100644 --- a/tests/tests/test_views.py +++ b/tests/tests/test_views.py @@ -71,3 +71,40 @@ def test_includes(self): self.assertEqual(render_response.status_code, 200) self.assertContains(render_response, 'SHOWME') self.assertNotContains(render_response, 'HIDEME') + self.assertContains(render_response, 'included content from variable') + + def test_page(self): + test_page_render_url = reverse( + 'pattern_library:render_pattern', + kwargs={'pattern_template_name': "patterns/pages/test_page/test_page.html"}, + ) + response = self.client.get(test_page_render_url) + + self.assertContains(response, 'Page') + + def test_fragments(self): + for template_name in [ + 'patterns/atoms/test_atom/test_atom.html', + 'patterns/molecules/test_molecule/test_molecule.html', + ]: + with self.subTest(template_name=template_name): + self.assertContains( + self.client.get( + reverse( + 'pattern_library:render_pattern', + kwargs={'pattern_template_name': template_name}, + ), + ), + 'Fragment', + ) + + def test_fragment_extended_from_variable(self): + self.assertContains( + self.client.get( + reverse( + 'pattern_library:render_pattern', + kwargs={'pattern_template_name': "patterns/atoms/test_extends/extended.html"}, + ), + ), + 'base content - extended content', + ) diff --git a/tox.ini b/tox.ini index 14fc6539..6154dac5 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py{36,37,38}-dj{22,30,31}, lint +envlist = py{36,37,38}-dj{22,30,31,master}, lint skipsdist = true [testenv] @@ -13,6 +13,7 @@ deps = dj22: Django>=2.2,<2.3 dj30: Django>=3.0,<3.1 dj31: Django>=3.1,<3.2 + djmaster: https://github.com/django/django/archive/master.zip [testenv:lint] commands =