From 0f53bfdc19f3b1691c0e055aac6c4a347cc041f9 Mon Sep 17 00:00:00 2001 From: Arthur Date: Fri, 8 Aug 2025 14:53:48 -0700 Subject: [PATCH] #20048 review feedback --- netbox/extras/dashboard/widgets.py | 6 +- netbox/extras/tests/test_dashboard.py | 2 +- netbox/netbox/tables/columns.py | 2 +- .../extras/dashboard/widgets/objectlist.html | 2 +- netbox/utilities/templatetags/helpers.py | 109 +++++++++--------- netbox/utilities/views.py | 10 +- 6 files changed, 67 insertions(+), 64 deletions(-) diff --git a/netbox/extras/dashboard/widgets.py b/netbox/extras/dashboard/widgets.py index 92034b521..2497e9c80 100644 --- a/netbox/extras/dashboard/widgets.py +++ b/netbox/extras/dashboard/widgets.py @@ -11,7 +11,7 @@ from django.conf import settings from django.core.cache import cache from django.db.models import Model from django.template.loader import render_to_string -from django.urls import NoReverseMatch, resolve, reverse +from django.urls import NoReverseMatch, resolve from django.utils.translation import gettext as _ from core.models import ObjectType @@ -53,9 +53,9 @@ def object_list_widget_supports_model(model: Model) -> bool: """ def can_resolve_model_list_view(model: Model) -> bool: try: - reverse(get_action_url(model, action='list')) + get_action_url(model, action='list') return True - except Exception: + except NoReverseMatch: return False tests = [ diff --git a/netbox/extras/tests/test_dashboard.py b/netbox/extras/tests/test_dashboard.py index b12d7a167..fb94710c7 100644 --- a/netbox/extras/tests/test_dashboard.py +++ b/netbox/extras/tests/test_dashboard.py @@ -45,4 +45,4 @@ class ObjectListWidgetTests(TestCase): mock_request = Request() widget = ObjectListWidget(id='2829fd9b-5dee-4c9a-81f2-5bd84c350a27', **config) rendered = widget.render(mock_request) - self.assertTrue('Unable to load content. Invalid list viewname for:' in rendered) + self.assertTrue('Unable to load content. Could not resolve list URL for:' in rendered) diff --git a/netbox/netbox/tables/columns.py b/netbox/netbox/tables/columns.py index cf5c11188..84722512b 100644 --- a/netbox/netbox/tables/columns.py +++ b/netbox/netbox/tables/columns.py @@ -285,7 +285,7 @@ class ActionsColumn(tables.Column): for idx, (action, attrs) in enumerate(self.actions.items()): permission = get_permission_for_model(model, attrs.permission) if attrs.permission is None or user.has_perm(permission): - url = get_action_url(record, action=action, kwargs={'pk': record.pk}) + url = get_action_url(model, action=action, kwargs={'pk': record.pk}) # Render a separate button if a) only one action exists, or b) if split_actions is True if len(self.actions) == 1 or (self.split_actions and idx == 0): diff --git a/netbox/templates/extras/dashboard/widgets/objectlist.html b/netbox/templates/extras/dashboard/widgets/objectlist.html index 20f4fe4b1..ca31ac2a0 100644 --- a/netbox/templates/extras/dashboard/widgets/objectlist.html +++ b/netbox/templates/extras/dashboard/widgets/objectlist.html @@ -7,6 +7,6 @@ {% else %}
- {% trans "Unable to load content. Invalid list viewname for:" %} {{ model_name }} + {% trans "Unable to load content. Could not resolve list URL for:" %} {{ model_name }}
{% endif %} diff --git a/netbox/utilities/templatetags/helpers.py b/netbox/utilities/templatetags/helpers.py index 00df1e829..8ea78ffa6 100644 --- a/netbox/utilities/templatetags/helpers.py +++ b/netbox/utilities/templatetags/helpers.py @@ -12,6 +12,7 @@ from utilities.views import get_viewname, get_action_url from netbox.settings import DISK_BASE_UNIT, RAM_BASE_UNIT __all__ = ( + 'action_url', 'applied_filters', 'as_range', 'divide', @@ -64,6 +65,59 @@ def validated_viewname(model, action): return None +class ActionURLNode(template.Node): + """Template node for the {% action_url %} template tag.""" + + child_nodelists = () + + def __init__(self, model, action, kwargs, asvar=None): + self.model = model + self.action = action + self.kwargs = kwargs + self.asvar = asvar + + def __repr__(self): + return ( + f"<{self.__class__.__qualname__} " + f"model='{self.model}' " + f"action='{self.action}' " + f"kwargs={repr(self.kwargs)} " + f"as={repr(self.asvar)}>" + ) + + def render(self, context): + """ + Render the action URL node. + + Args: + context: The template context + + Returns: + The resolved URL or empty string if using 'as' syntax + + Raises: + NoReverseMatch: If the URL cannot be resolved and not using 'as' syntax + """ + # Resolve model and kwargs from context + model = self.model.resolve(context) + kwargs = {k: v.resolve(context) for k, v in self.kwargs.items()} + + # Get the action URL using the utility function + try: + url = get_action_url(model, action=self.action, kwargs=kwargs) + except NoReverseMatch: + if self.asvar is None: + raise + url = "" + + # Handle variable assignment or return escaped URL + if self.asvar: + context[self.asvar] = url + return "" + + return conditional_escape(url) if context.autoescape else url + + @register.tag def action_url(parser, token): """ @@ -82,7 +136,7 @@ def action_url(parser, token): {% action_url model "action_name" pk=object.pk as variable_name %} - The first argument is a model instance. The second argument is the action name. + The first argument is a model or instance. The second argument is the action name. Additional keyword arguments can be passed for URL parameters. For example, if you have a Device model and want to link to its edit action:: @@ -99,59 +153,6 @@ def action_url(parser, token): {% action_url device "edit" as edit_url %} """ - - class ActionURLNode(template.Node): - """Template node for the {% action_url %} template tag.""" - - child_nodelists = () - - def __init__(self, model, action, kwargs, asvar=None): - self.model = model - self.action = action - self.kwargs = kwargs - self.asvar = asvar - - def __repr__(self): - return ( - f"<{self.__class__.__qualname__} " - f"model='{self.model}' " - f"action='{self.action}' " - f"kwargs={repr(self.kwargs)} " - f"as={repr(self.asvar)}>" - ) - - def render(self, context): - """ - Render the action URL node. - - Args: - context: The template context - - Returns: - The resolved URL or empty string if using 'as' syntax - - Raises: - NoReverseMatch: If the URL cannot be resolved and not using 'as' syntax - """ - # Resolve model and kwargs from context - model = self.model.resolve(context) - kwargs = {k: v.resolve(context) for k, v in self.kwargs.items()} - - # Get the action URL using the utility function - try: - url = get_action_url(model, action=self.action, kwargs=kwargs) - except NoReverseMatch: - if self.asvar is None: - raise - url = "" - - # Handle variable assignment or return escaped URL - if self.asvar: - context[self.asvar] = url - return "" - - return conditional_escape(url) if context.autoescape else url - # Parse the token contents bits = token.split_contents() if len(bits) < 3: diff --git a/netbox/utilities/views.py b/netbox/utilities/views.py index 98bddd860..fbf6e1d81 100644 --- a/netbox/utilities/views.py +++ b/netbox/utilities/views.py @@ -20,6 +20,7 @@ __all__ = ( 'GetReturnURLMixin', 'ObjectPermissionRequiredMixin', 'ViewTab', + 'get_action_url', 'get_viewname', 'register_model_view', ) @@ -284,12 +285,13 @@ def get_viewname(model, action=None, rest_api=False): def get_action_url(model, action=None, rest_api=False, kwargs=None): """ - Return the url for the given model and action, if valid. + Return the URL for the given model and action, if valid; otherwise raise NoReverseMatch. + Will defer to _get_action_url() on the model if it exists. - :param model: The model or instance to which the view applies + :param model: The model or instance to which the URL belongs :param action: A string indicating the desired action (if any); e.g. "add" or "list" - :param rest_api: A boolean indicating whether this is a REST API view - :param kwargs: A dictionary of keyword arguments for the view to include when registering its URL path (optional). + :param rest_api: A boolean indicating whether this is a REST API action + :param kwargs: A dictionary of keyword arguments for the view to include when resolving its URL path (optional) """ if hasattr(model, '_get_action_url'): return model._get_action_url(action, rest_api, kwargs)