Skip to content

Commit ed33dbd

Browse files
authored
Merge pull request #98 from opensafely-core/evansd/missing-template-vars
Be strict about missing template variables
2 parents 8eae78d + 0c5e251 commit ed33dbd

File tree

2 files changed

+71
-2
lines changed

2 files changed

+71
-2
lines changed

airlock/settings.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@
1010
https://docs.djangoproject.com/en/5.0/ref/settings/
1111
"""
1212

13+
import logging
1314
import os
15+
import warnings
1416
from pathlib import Path
1517

1618
import django.dispatch
@@ -247,3 +249,70 @@ def run_connection_init_queries(*, connection, **kwargs):
247249

248250
# Temporary UI featureflag
249251
TREE = True
252+
253+
254+
class MissingVariableErrorFilter(logging.Filter):
255+
"""
256+
Convert "missing template variable" log messages into warnings, whose presence will
257+
trip our zero warnings enforcement in test runs.
258+
259+
Heavily inspired by Adam Johnson's work here:
260+
https://adamj.eu/tech/2022/03/30/how-to-make-django-error-for-undefined-template-variables/
261+
"""
262+
263+
ignored_prefixes = (
264+
# Some internal Django templates rely on the silent missing variable behaviour
265+
"admin/",
266+
"auth/",
267+
"django/",
268+
# As does our internal component library
269+
"_components",
270+
"_partials",
271+
)
272+
273+
def filter(self, record): # pragma: no cover
274+
if record.msg.startswith("Exception while resolving variable "):
275+
template_name = record.args[1]
276+
if (
277+
not template_name.startswith(self.ignored_prefixes)
278+
# This shows up when rendering Django's internal error pages
279+
and template_name != "unknown"
280+
):
281+
# Use `warn_explicit` to raise the warning at whatever location the
282+
# original log message was raised
283+
warnings.warn_explicit(
284+
record.getMessage(),
285+
UserWarning,
286+
record.pathname,
287+
record.lineno,
288+
)
289+
# Remove from log output
290+
return False
291+
292+
293+
# NOTE: This is the default minimal logging config from Django's docs. It is not
294+
# intended to be the final word in how logging should be configured in Airlock.
295+
LOGGING = {
296+
"version": 1,
297+
"disable_existing_loggers": False,
298+
"handlers": {
299+
"console": {
300+
"class": "logging.StreamHandler",
301+
},
302+
},
303+
"root": {
304+
"handlers": ["console"],
305+
"level": "WARNING",
306+
},
307+
"filters": {
308+
"missing_variable_error": {
309+
"()": "{0.__module__}.{0.__name__}".format(MissingVariableErrorFilter),
310+
},
311+
},
312+
"loggers": {
313+
"django.template": {
314+
"level": "DEBUG",
315+
"filters": ["missing_variable_error"],
316+
},
317+
},
318+
}

airlock/templates/base.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
{% block extra_meta %}{% endblock %}
1414

15-
<script type="module" nonce="{{ request.csp_nonce }}">
15+
<script type="module">
1616
document.documentElement.classList.remove("no-js");
1717
document.documentElement.classList.add("js");
1818

@@ -79,7 +79,7 @@
7979
</a>
8080
</li>
8181
{% endfor %}
82-
{% if not request.user.is_authenticated %}
82+
{% if not request.user or not request.user.is_authenticated %}
8383
<li>
8484
<a
8585
class="

0 commit comments

Comments
 (0)