Repository: netbox-community/netbox
Pull request: Adds region hierarchy in templates
<!--
    Thank you for your interest in contributing to NetBox! Please note that
    our contribution policy requires that a feature request or bug report be
    approved and assigned prior to opening a pull request. This helps avoid
    waste time and effort on a proposed change that we might not be able to
    accept.

    IF YOUR PULL REQUEST DOES NOT REFERENCE AN ISSUE WHICH HAS BEEN ASSIGNED
    TO YOU, IT WILL BE CLOSED AUTOMATICALLY.

    Please specify your assigned issue number on the line below.
-->
### Fixes: #13735 

<!--
    Please include a summary of the proposed changes below.
-->


Problem statement:
Show full Region hierarchy on all relevant object views
### NetBox version

v3.6.1

### Feature type

Change to existing functionality

### Proposed functionality

There seems to be some inconsistency with the way that the Region information is displayed in different object views. For a Device, there is a separate Region row shown in the UI with the full Region path. But, for Circuit Termination, Prefix, Rack, Rack Reservation, and VLAN (this is everything I could find with `grep -F 'site.region|linkify'`, so maybe I missed something), only the leaf Region is shown on the Site row in the respective object view. My proposal would be to align the other models' object views to the way that Device does it, but I'm also fine with the regions staying on the Site line, as long as they show the full path, and ideally are consistent across all the object views.

I also noticed that models like Power Panels and Virtualization Clusters show the Site and no Region information. I'm not sure how to gather an exhaustive list for that scenario, but I feel like it should be consistent as well. Maybe there's a better way to DRY the site/location/rack and related site groups and regions into a separate template for consistency (understanding that location/rack may not be relevant to all models that support site)?

### Use case

We are trying to implement a Region hierarchy that follows a pattern like `State / City` (which I think is fairly common) and it seems redundant to have to include the state as part of the city name, since the Region model only requires the name to be unique to its parent. There is also inconsistency today between how different models represent the same data in the UI, so that creates some visual dissonance. There are other areas of the UI that currently don't handle nested models very well, like table columns and dropdowns, but that's a separate issue.

### Database changes

None

### External dependencies

None

Hints from issue discussion:

Commit message: adds display region tag #13735

Patch to review:
diff --git a/netbox/dcim/templatetags/__init__.py b/netbox/dcim/templatetags/__init__.py
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/netbox/dcim/templatetags/display_region.py b/netbox/dcim/templatetags/display_region.py
new file mode 100644
index 00000000000..933d6ecbe79
--- /dev/null
+++ b/netbox/dcim/templatetags/display_region.py
@@ -0,0 +1,37 @@
+from django import template
+from django.utils.safestring import mark_safe
+
+register = template.Library()
+
+
+def _display_site(obj):
+    """
+    Render a link to the site of an object.
+    """
+    site = getattr(obj, 'site', None)
+    return mark_safe('<a href="{}">{}</a>'.format(site.get_absolute_url(), site)) if site else None
+
+
+@register.simple_tag(takes_context=True)
+def display_region(context, obj, include_site=False):
+    """
+    Renders hierarchical region data for a given object, optionally including the site.
+    """
+    # Retrieve the region or site information
+    region = getattr(obj, 'region', None) or getattr(obj.site, 'region', None) if hasattr(obj, 'site') else None
+    site_link = _display_site(obj) if include_site else None
+
+    # Return a placeholder if no region or site is found
+    if not region and not site_link:
+        return mark_safe('&mdash;')
+
+    # Build the region links if the region is available
+    region_links = ' / '.join(
+        '<a href="{}">{}</a>'.format(context['request'].build_absolute_uri(reg.get_absolute_url()), reg)
+        for reg in region.get_ancestors(include_self=True)
+    ) if region else ''
+
+    # Concatenate region and site links
+    links = ' / '.join(filter(None, [region_links, site_link]))
+
+    return mark_safe(links)
diff --git a/netbox/templates/dcim/device.html b/netbox/templates/dcim/device.html
index 5fa6a3314a3..22aeb20eda5 100644
--- a/netbox/templates/dcim/device.html
+++ b/netbox/templates/dcim/device.html
@@ -5,6 +5,7 @@
 {% load helpers %}
 {% load plugins %}
 {% load i18n %}
+{% load display_region %}
 
 {% block content %}
     <div class="row">
@@ -16,14 +17,7 @@ <h5 class="card-header">{% trans "Device" %}</h5>
                         <tr>
                             <th scope="row">{% trans "Region" %}</th>
                             <td>
-                                {% if object.site.region %}
-                                    {% for region in object.site.region.get_ancestors %}
-                                        {{ region|linkify }} /
-                                    {% endfor %}
-                                    {{ object.site.region|linkify }}
-                                {% else %}
-                                    {{ ''|placeholder }}
-                                {% endif %}
+                                {% display_region object %}
                             </td>
                         </tr>
                         <tr>
diff --git a/netbox/templates/dcim/rack.html b/netbox/templates/dcim/rack.html
index 671c7ab2eb2..1a3f4e4ef50 100644
--- a/netbox/templates/dcim/rack.html
+++ b/netbox/templates/dcim/rack.html
@@ -4,6 +4,7 @@
 {% load static %}
 {% load plugins %}
 {% load i18n %}
+{% load display_region %}
 
 {% block content %}
   <div class="row">
@@ -17,10 +18,7 @@ <h5 class="card-header">
                     <tr>
                         <th scope="row">{% trans "Site" %}</th>
                         <td>
-                            {% if object.site.region %}
-                                {{ object.site.region|linkify }} /
-                            {% endif %}
-                            {{ object.site|linkify }}
+                            {% display_region object include_site=True %}
                         </td>
                     </tr>
                     <tr>
diff --git a/netbox/templates/dcim/rackreservation.html b/netbox/templates/dcim/rackreservation.html
index 8edb75f324f..ccd64a63ea0 100644
--- a/netbox/templates/dcim/rackreservation.html
+++ b/netbox/templates/dcim/rackreservation.html
@@ -4,6 +4,7 @@
 {% load static %}
 {% load plugins %}
 {% load i18n %}
+{% load display_region %}
 
 {% block breadcrumbs %}
   {{ block.super }}
@@ -24,10 +25,7 @@ <h5 class="card-header">
                         <tr>
                             <th scope="row">{% trans "Site" %}</th>
                             <td>
-                                {% if rack.site.region %}
-                                    {{ rack.site.region|linkify }} /
-                                {% endif %}
-                                {{ rack.site|linkify }}
+                                {% display_region rack include_site=True %}
                             </td>
                         </tr>
                         <tr>
diff --git a/netbox/templates/dcim/site.html b/netbox/templates/dcim/site.html
index 7f43a0ab33a..e4b93956140 100644
--- a/netbox/templates/dcim/site.html
+++ b/netbox/templates/dcim/site.html
@@ -3,6 +3,7 @@
 {% load plugins %}
 {% load tz %}
 {% load i18n %}
+{% load display_region %}
 
 {% block breadcrumbs %}
   {{ block.super }}
@@ -29,14 +30,7 @@ <h5 class="card-header">{% trans "Site" %}</h5>
           <tr>
             <th scope="row">{% trans "Region" %}</th>
             <td>
-              {% if object.region %}
-                {% for region in object.region.get_ancestors %}
-                  {{ region|linkify }} /
-                {% endfor %}
-                {{ object.region|linkify }}
-              {% else %}
-                {{ ''|placeholder }}
-              {% endif %}
+              {% display_region object %}
             </td>
           </tr>
           <tr>
diff --git a/netbox/templates/ipam/prefix.html b/netbox/templates/ipam/prefix.html
index 71b240cedef..399200da33d 100644
--- a/netbox/templates/ipam/prefix.html
+++ b/netbox/templates/ipam/prefix.html
@@ -3,6 +3,7 @@
 {% load helpers %}
 {% load plugins %}
 {% load i18n %}
+{% load display_region %}
 
 {% block content %}
 <div class="row">
@@ -47,14 +48,7 @@ <h5 class="card-header">{% trans "Prefix" %}</h5>
           <tr>
             <th scope="row">{% trans "Site" %}</th>
             <td>
-              {% if object.site %}
-                {% if object.site.region %}
-                  {{ object.site.region|linkify }} /
-                {% endif %}
-                {{ object.site|linkify }}
-              {% else %}
-                {{ ''|placeholder }}
-              {% endif %}
+              {% display_region object include_site=True %}
             </td>
           </tr>
           <tr>
diff --git a/netbox/templates/ipam/vlan.html b/netbox/templates/ipam/vlan.html
index 4ca045d4b4d..af70e2fadd9 100644
--- a/netbox/templates/ipam/vlan.html
+++ b/netbox/templates/ipam/vlan.html
@@ -3,6 +3,7 @@
 {% load render_table from django_tables2 %}
 {% load plugins %}
 {% load i18n %}
+{% load display_region %}
 
 {% block content %}
     <div class="row">
@@ -16,14 +17,7 @@ <h5 class="card-header">
                         <tr>
                             <th scope="row">{% trans "Site" %}</th>
                             <td>
-                                {% if object.site %}
-                                    {% if object.site.region %}
-                                        {{ object.site.region|linkify }} /
-                                    {% endif %}
-                                    {{ object.site|linkify }}
-                                {% else %}
-                                    {{ ''|placeholder }}
-                                {% endif %}
+                                {% display_region object include_site=True %}
                             </td>
                         </tr>
                         <tr>


Code review comments:
diff_hunk: @@ -0,0 +1,37 @@
+from django import template
+from django.utils.safestring import mark_safe
+
+register = template.Library()
+
+
+def _display_site(obj):
+    """
+    Render a link to the site of an object.
+    """
+    site = getattr(obj, 'site', None)
+    return mark_safe('<a href="{}">{}</a>'.format(site.get_absolute_url(), site)) if site else None
+
+
+@register.simple_tag(takes_context=True)
+def display_region(context, obj, include_site=False):
path: netbox/dcim/templatetags/display_region.py
text: @user1:
IMO we should omit any reference to a site, and limit this tag to only being concerned with rendering the region hierarchy. For example, something like `{% region_tree region %}` might render "US > North Carolina > Raleigh" for the Raleigh region.

In fact, it might be useful to turn this into a general-purpose tag to render other MPTT objects in the same manner.

@author:
To keep the current scope limited, I'll update the relevant UI and add a simple region tree tag. We can make it generic as a part of a separate FR.

@user1:
I agree on limiting the implementation scope for this FR, but we should give it a generic name now to avoid having to update it later. How about something like `nested_tree`?