From 5dc4a934233710cf68b434b3d91e9f1afb0b8de0 Mon Sep 17 00:00:00 2001 From: Alastair Porter Date: Thu, 9 Jul 2026 13:32:11 +0200 Subject: [PATCH 1/3] Require keywords for optional arguments in templatetags This makes it easier to understand what a parameter to the templatetag means, and prevents issues from needing to update all call sites if the tag parameters change. Update bw_paginator/bw_user_avatar/bw_tag, which are our most used tags --- DEVELOPERS.md | 16 +++++++++ general/templatetags/bw_templatetags.py | 6 ++-- general/templatetags/tests.py | 35 ++++++++++++++++++-- templates/accounts/account.html | 4 +-- templates/accounts/attribution.html | 2 +- templates/accounts/display_user.html | 8 ++--- templates/accounts/edit.html | 2 +- templates/accounts/manage_sounds.html | 4 +-- templates/accounts/modal_comments.html | 2 +- templates/accounts/modal_downloads.html | 4 +-- templates/accounts/modal_follow.html | 6 ++-- templates/bookmarks/bookmarks.html | 2 +- templates/collections/collection.html | 4 +-- templates/donations/donation_list.html | 2 +- templates/forum/display_forum.html | 2 +- templates/forum/display_post.html | 2 +- templates/forum/display_thread.html | 6 ++-- templates/forum/hot_threads.html | 2 +- templates/forum/new_thread.html | 2 +- templates/forum/reply.html | 2 +- templates/forum/thread.html | 2 +- templates/forum/threads.html | 2 +- templates/front.html | 2 +- templates/messages/message.html | 4 +-- templates/messages/messages_list.html | 2 +- templates/moderation/assign_sounds.html | 2 +- templates/moderation/assigned.html | 2 +- templates/moderation/modal_annotations.html | 2 +- templates/moderation/modal_pending.html | 2 +- templates/moderation/modal_tardy.html | 2 +- templates/molecules/navbar_user_section.html | 2 +- templates/search/facet.html | 2 +- templates/search/search.html | 2 +- templates/search/search_forum.html | 2 +- templates/sounds/display_pack.html | 4 +-- templates/sounds/display_sound.html | 2 +- templates/sounds/modal_downloaders.html | 2 +- templates/sounds/modal_similar_sounds.html | 2 +- templates/sounds/pack.html | 4 +-- templates/sounds/sound.html | 6 ++-- templates/tags/tag_cloud.html | 2 +- 41 files changed, 106 insertions(+), 59 deletions(-) diff --git a/DEVELOPERS.md b/DEVELOPERS.md index 54a4364e19..bd368d6b96 100644 --- a/DEVELOPERS.md +++ b/DEVELOPERS.md @@ -64,6 +64,22 @@ that were made, and allow feedback if necessary ## Specific notes +### Template tag optional arguments + +Optional arguments in template tags must always be keyword-only: + +```py +@register.inclusion_tag("atoms/avatar.html") +def bw_user_avatar(avatar_url, username, *, size=40, extra_class=""): +``` + +and templates need to specify the keywords in order to change the default: + +```html +{% bw_user_avatar url username size=32 %} +``` + + ### Custom Django permissions If there is a need for defining custom permissions we should define them in the corresponding model's `Meta` class diff --git a/general/templatetags/bw_templatetags.py b/general/templatetags/bw_templatetags.py index 21790a9025..77724d6b86 100644 --- a/general/templatetags/bw_templatetags.py +++ b/general/templatetags/bw_templatetags.py @@ -42,7 +42,7 @@ def bw_icon(name, class_name=""): @register.inclusion_tag("atoms/tag.html") -def bw_tag(tag_name, size=1, class_name="", url=None, weight=None): +def bw_tag(tag_name, *, size=1, class_name="", url=None, weight=None): """ Displays a BW tag with the given name """ @@ -66,7 +66,7 @@ def bw_tag(tag_name, size=1, class_name="", url=None, weight=None): @register.inclusion_tag("atoms/avatar.html") -def bw_user_avatar(avatar_url, username, size=40, extra_class=""): +def bw_user_avatar(avatar_url, username, *, size=40, extra_class=""): """ Displays a BW user avatar or no avatar if user has none We check if user has custom avatar by checking if the given avatar URL contains the filename of the default @@ -179,7 +179,7 @@ def bw_generic_stars(context, rating_0_10): @register.inclusion_tag("molecules/paginator.html", takes_context=True) -def bw_paginator(context, page, request, anchor="", non_grouped_number_of_results=-1, max_pages=None): +def bw_paginator(context, page, request, *, anchor="", non_grouped_number_of_results=-1, max_pages=None): """ Adds pagination context variables for use in displaying first, adjacent and last page links in addition to those created by the object_list generic diff --git a/general/templatetags/tests.py b/general/templatetags/tests.py index dfc0cf0e13..731a69e6b4 100644 --- a/general/templatetags/tests.py +++ b/general/templatetags/tests.py @@ -18,15 +18,16 @@ # See AUTHORS file. # +import pytest from django.core.paginator import Paginator -from django.template import Context, Template +from django.template import Context, Template, TemplateSyntaxError from django.test import RequestFactory def _render(num_pages, current_page, max_pages_expr): page = Paginator(range(num_pages * 15), 15).page(current_page) request = RequestFactory().get("/search/?q=wind") - tpl = Template("{% load bw_templatetags %}{% bw_paginator page request 'sound' -1 " + max_pages_expr + " %}") + tpl = Template("{% load bw_templatetags %}{% bw_paginator page request anchor='sound' " + max_pages_expr + " %}") return tpl.render(Context({"page": page, "request": request})) @@ -53,3 +54,33 @@ def test_next_arrow_hidden_at_capped_last_page(): def test_next_arrow_shown_below_cap(): html = _render(num_pages=700, current_page=50, max_pages_expr="max_pages=100") assert 'title="Next Page"' in html + + +def test_paginator_rejects_positional_optional(): + # Optionals are keyword-only (the `*` in the signature), so passing one positionally + # is a compile-time error rather than a silent rebind. bw_paginator has takes_context=True, + # exercising the most intricate parse_bits path. + with pytest.raises(TemplateSyntaxError): + Template("{% load bw_templatetags %}{% bw_paginator page request 'sound' %}") + + +def test_user_avatar_accepts_keyword_optionals(): + tpl = Template( + "{% load bw_templatetags %}" + '{% bw_user_avatar "https://example.com/a.png" "alice" size=64 extra_class="new-posts-notification" %}' + ) + html = tpl.render(Context({})) + assert "width:64px" in html + assert "new-posts-notification" in html + + +def test_tag_accepts_keyword_optionals(): + tpl = Template( + "{% load bw_templatetags %}" + "{% bw_tag \"windy\" size=3 class_name='h-spacing-1' url='/browse/tags/windy/' weight=1.0 %}" + ) + html = tpl.render(Context({})) + assert "windy" in html + assert "padding-3" in html + assert "h-spacing-1" in html + assert 'href="/browse/tags/windy/"' in html diff --git a/templates/accounts/account.html b/templates/accounts/account.html index dd67c0ba68..9ef03ae0ab 100644 --- a/templates/accounts/account.html +++ b/templates/accounts/account.html @@ -15,7 +15,7 @@