prymitive / bootstrap-breadcrumbs

Django template tags for easy breadcrumbs using twitter bootstrap css classes or custom template
django-bootstrap-breadcrumbs.readthedocs.org
MIT License
93 stars 48 forks source link

Django 1.9 template render warning fix #27

Closed antwan closed 7 years ago

antwan commented 7 years ago

This PR fixes a Django 1.9 warning when rendering template using a RequestContext instead of a dict. As per the [deprecation plan](https://docs.djangoproject.com/en/1.9/_modules/django/template/backends/django/|deprecation plan) since 1.8, it is recommanded to send a dictionnary context instead of a Context or RequestContext object when rendering templates. A warning is raised on Django 1.9, but it won't (immediately) break on Django 1.10 thus making a future error silent.

Tested with Django 1.5 to 1.10.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling da3766cf6ea830e15112f8efc7306c71c4a7999d on Antwan86:master into 6ff5563bf1918bfa66d857a4377ec3b588423557 on prymitive:master.

prymitive commented 7 years ago

Thanks for the patch, it does pass all tests but it overrides context object and I'm not 100% sure it wouldn't break anything. My concern is that context might have additional keys used for template rendering as so replacing it with a static 2 element dict will make those keys unavailable. A safer approach would be to convert context object to a dict and inject breadcrumb keys. Please take a look at https://github.com/prymitive/bootstrap-breadcrumbs/commit/f2e6b17afa41cf7c7498baa67c09dd85191894bb and if that works for you update this PR. Thanks again.

antwan commented 7 years ago

Given the templates used, even if the context had additional keys (it's actually empty), they wouldn't be used anyway.

I'll update the PR though.

prymitive commented 7 years ago

Fixed in 17b8879643ddfff6b5568a7f05b84b27380a8cec