#20048 review feedback

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 = [

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)

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):

netbox/templates/extras/dashboard/widgets/objectlist.html

@@ -7,6 +7,6 @@
   </div>
 {% else %}
   <div class="text-danger text-center">
-    <i class="mdi mdi-alert"></i> {% trans "Unable to load content. Invalid list viewname for:" %} <span class="font-monospace">{{ model_name }}</span>
+    <i class="mdi mdi-alert"></i> {% trans "Unable to load content. Could not resolve list URL for:" %} <span class="font-monospace">{{ model_name }}</span>
   </div>
 {% endif %}

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:

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)