carltongibson / django-template-partials

Reusable named inline partials for the Django Template Language.
MIT License
444 stars 16 forks source link

TemplateProxy needs to pass through the `source` property of a template. #3

Closed vsajip closed 1 year ago

vsajip commented 1 year ago

I'm hitting an error when trying to use partials with Slippers, where I have a template components.html which contains partials for buttons, etc. and I'm telling Slippers the template name for the my_cool_button component is 'components.html#my_cool_button'. It falls over because Slippers expects a template to have the source property, but TemplateProxy doesn't.

  File "/disk2/vinay/projects/slippers/slippers/templatetags/slippers.py", line 103, in render
    source_front_matter = extract_template_parts(template.source)[0]
AttributeError: 'TemplateProxy' object has no attribute 'source'

I added this code to TemplateProxy to make the problem go away:

    @property
    def source(self):
        template = self.origin.loader.get_template(self.origin.template_name)
        return template.source

I don't mind submitting a PR, if you think the above is the right fix, but I should then add a test, and it's not clear to me how to exercise the unit tests on your project, as you have no GitHub actions set up and I couldn't quite tell from the documentation here.

carltongibson commented 1 year ago

Hi @vsajip, seems reasonable yes.

To run the tests you can use just test, assuming you have Just installed, or run the command from the justfile, if not.

If you'd like to add a GitHub Actions workflow that would be good too. To begin I'd test Django 4.2 and Python 3.10+

Thanks for the input!

vsajip commented 1 year ago

OK, thanks. Next question - what's the most natural way to cause a TemplateProxy to be created and returned? I did

        engine = engines["django"]
        template = engine.get_template("example.html#test-partial")

but that gets me a Template and not a TemplateProxy.

carltongibson commented 1 year ago

but that gets me a Template and not a TemplateProxy.

I'm not expecting that. Please put what you have in a PR so I can look at it. Thanks.

alkelaun commented 1 year ago

I think you want to do:

def test_template_source(self):
        engine = engines["django"]
        template = engine.get_template("example.html#test-partial")
        self.assertIsInstance(template.template, TemplateProxy)
        self.assertTrue(hasattr(template.template, 'source'))

This code passed the tests when I ran them.

Why: There are two defined Template classes in the 'template' portion of django source code that I found.

  1. django/template/base.py

  2. django/template/backends/django.py

When I checked to the type of the "Template" being returned, I expected it to be from 1. It was not. Then I checked to see if it was of the type of the second. It was.

I noticed that the '__init__' function of the second Template class requires a 'template' to be passed and sets it to self.template. here.

I hope this helps.

John

alkelaun commented 1 year ago

To prove to myself:

from django.template import base
from django.template.backends import django
from template_partials.templatetags.partials import TemplateProxy

# testing both the TemplateProxy and the django.Template

def test_template_source(self):
        engine = engines["django"]
        template = engine.get_template("example.html#test-partial")
        self.assertIsInstance(template.template, TemplateProxy)
        self.assertTrue(hasattr(template.template, 'source'))
        self.assertIsInstance(template, django.Template)

# making sure if it's a partial that it doesn't use the TemplateProxy and it uses the django builtin

 def test_template_source_regular(self):
        engine = engines["django"]
        template = engine.get_template("example.html")
        self.assertTrue(hasattr(template.template,'source'))
        self.assertIsInstance(template.template, base.Template)
        self.assertIsInstance(template, django.Template)
vsajip commented 1 year ago

Hmmm .. for me, the above fails, this is what works for me:

    def test_template_source(self):
        engine = engines["django"]
        template = engine.get_template("example.html#test-partial")
        self.assertIsInstance(template, django.template.backends.django.Template)
        self.assertIsInstance(template.template, TemplateProxy)
        self.assertTrue(hasattr(template.template, "source"))

        # Regular template
        template = engine.get_template("example.html")
        self.assertIsInstance(template, django.template.backends.django.Template)
        self.assertIsInstance(template.template, base.Template)
        self.assertTrue(hasattr(template.template,"source"))
alkelaun commented 1 year ago

It was probably from django.template.backends import django that caused some errors. Below I have your code and my code working in the same test.

I had some problems running tests and I had to move the code from src/template_partials up a directory to get the test to run (otherwise it couldn't import template_partials) and wouldn't run the tests.

My imports now look like:

from django.template import engines, TemplateDoesNotExist
from django.test import TestCase
from django.template import base, backends
import django # added to get the second set of tests running.

And the code works:

    def test_template_source(self):
        engine = engines["django"]
        template = engine.get_template("example.html#test-partial")
        self.assertIsInstance(template.template, TemplateProxy)
        self.assertTrue(hasattr(template.template, 'source'))
        self.assertIsInstance(template, backends.django.Template)
             # moved from django.Template to banckends.django.Template

    def test_template_source_regular(self):
        engine = engines["django"]
        template = engine.get_template("example.html")
        self.assertTrue(hasattr(template.template,'source'))
        self.assertIsInstance(template.template, base.Template)
        self.assertIsInstance(template, backends.django.Template)
                 # moved from django.Template to banckends.django.Template

    def test_template_source2(self):
        engine = engines["django"]
        template = engine.get_template("example.html#test-partial")
        self.assertIsInstance(template, django.template.backends.django.Template)
        self.assertIsInstance(template.template, TemplateProxy)
        self.assertTrue(hasattr(template.template, "source"))

        # Regular template
        template = engine.get_template("example.html")
        self.assertIsInstance(template, django.template.backends.django.Template)
        self.assertIsInstance(template.template, base.Template)
        self.assertTrue(hasattr(template.template,"source"))
vsajip commented 1 year ago

I had to move the code from src/template_partials up a directory to get the test to run (otherwise it couldn't import template_partials) and wouldn't run the tests.

See #4 - the answer is to add both src and . to the PYTHONPATH. Easier to do in the shell or via .env file, as django-admin --pythonpath only allows you to add one directory.

alkelaun commented 1 year ago

See #4 - the answer is to add both src and . to the PYTHONPATH. Easier to do in the shell or via .env file, as django-admin --pythonpath only allows you to add one directory.

Thank you! That makes sense because when I added --pythonpath src, then it couldn't find tests.

carltongibson commented 1 year ago

Fixed in #5