瀏覽代碼

Address review: extract map URL helpers to ui/utils.py, document suppression

- Move _build_coords_url() to netbox/ui/utils.py as build_coords_url(),
  and extract the {lat}/{lon} detection into a companion is_coordinate_map_url()
  helper, so both AddressAttr and GPSCoordinatesAttr share one implementation
- Update AddressAttr.get_context() and GPSCoordinatesAttr.render() to
  import and call the helpers from ui/utils
- Update test_ui.py to import build_coords_url from netbox.ui.utils
- Add a note to the MAPS_URL config parameter description that
  address-based map links are disabled when coordinate placeholders
  ({lat}/{lon}) are present, so admins see this in the UI

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Brian Tiemann 16 小時之前
父節點
當前提交
33dfdef640
共有 4 個文件被更改,包括 38 次插入21 次删除
  1. 2 1
      netbox/netbox/config/parameters.py
  2. 9 8
      netbox/netbox/tests/test_ui.py
  3. 4 12
      netbox/netbox/ui/attrs.py
  4. 23 0
      netbox/netbox/ui/utils.py

+ 2 - 1
netbox/netbox/config/parameters.py

@@ -231,7 +231,8 @@ PARAMS = (
         default='https://maps.google.com/?q=',
         description=_(
             "URL for mapping geographic locations. For GPS coordinates, include {lat} and/or {lon} placeholders, "
-            "or omit them to append coordinates as a comma-separated pair."
+            "or omit them to append coordinates as a comma-separated pair. "
+            "Note: when {lat} or {lon} placeholders are present, address-based map links will be disabled."
         )
     ),
 

+ 9 - 8
netbox/netbox/tests/test_ui.py

@@ -16,6 +16,7 @@ from dcim.choices import InterfaceTypeChoices
 from dcim.models import Interface, Site
 from netbox.ui import attrs
 from netbox.ui.panels import ObjectsTablePanel
+from netbox.ui.utils import build_coords_url
 from users.models import ObjectPermission, User
 from utilities.testing import create_test_device
 from vpn.choices import (
@@ -441,11 +442,11 @@ class GPSCoordinatesAttrTestCase(TestCase):
         self.assertEqual(attr.render(obj, {'name': 'coordinates'}), attr.placeholder)
 
     def test_build_coords_url_legacy_prefix(self):
-        url = attrs._build_coords_url('https://maps.google.com/?q=', 48.858, 2.294)
+        url = build_coords_url('https://maps.google.com/?q=', 48.858, 2.294)
         self.assertEqual(url, 'https://maps.google.com/?q=48.858,2.294')
 
     def test_build_coords_url_lat_lon_placeholders(self):
-        url = attrs._build_coords_url(
+        url = build_coords_url(
             'https://www.openstreetmap.org/?mlat={lat}&mlon={lon}#map=16/{lat}/{lon}',
             48.858,
             2.294,
@@ -453,21 +454,21 @@ class GPSCoordinatesAttrTestCase(TestCase):
         self.assertEqual(url, 'https://www.openstreetmap.org/?mlat=48.858&mlon=2.294#map=16/48.858/2.294')
 
     def test_build_coords_url_lat_placeholder_only(self):
-        url = attrs._build_coords_url('https://example.com/?lat={lat}', 48.858, 2.294)
+        url = build_coords_url('https://example.com/?lat={lat}', 48.858, 2.294)
         self.assertEqual(url, 'https://example.com/?lat=48.858')
 
     def test_build_coords_url_lon_placeholder_only(self):
-        url = attrs._build_coords_url('https://example.com/?lon={lon}', 48.858, 2.294)
+        url = build_coords_url('https://example.com/?lon={lon}', 48.858, 2.294)
         self.assertEqual(url, 'https://example.com/?lon=2.294')
 
     def test_build_coords_url_unknown_placeholder_falls_back_to_legacy(self):
         # URL with only an unknown placeholder (no {lat}/{lon}) → legacy append
-        url = attrs._build_coords_url('https://example.com/?q={unknown}', 48.858, 2.294)
+        url = build_coords_url('https://example.com/?q={unknown}', 48.858, 2.294)
         self.assertEqual(url, 'https://example.com/?q={unknown}48.858,2.294')
 
     def test_build_coords_url_known_and_unknown_placeholder(self):
         # {lat} is substituted; unknown key is left as a literal placeholder
-        url = attrs._build_coords_url(
+        url = build_coords_url(
             'https://example.com/?lat={lat}&layer={layer}', 48.858, 2.294
         )
         self.assertEqual(url, 'https://example.com/?lat=48.858&layer={layer}')
@@ -475,7 +476,7 @@ class GPSCoordinatesAttrTestCase(TestCase):
     def test_build_coords_url_decimal_values_no_locale_separator(self):
         # Decimal field values must format with '.' as the decimal separator regardless of locale;
         # a locale-style comma separator would produce e.g. '48,858258' and break the URL
-        url = attrs._build_coords_url(
+        url = build_coords_url(
             'https://maps.google.com/?q=',
             Decimal('48.858258'),
             Decimal('2.294498'),
@@ -483,7 +484,7 @@ class GPSCoordinatesAttrTestCase(TestCase):
         self.assertEqual(url, 'https://maps.google.com/?q=48.858258,2.294498')
 
     def test_build_coords_url_decimal_with_placeholders_no_locale_separator(self):
-        url = attrs._build_coords_url(
+        url = build_coords_url(
             'https://www.openstreetmap.org/?mlat={lat}&mlon={lon}',
             Decimal('48.858258'),
             Decimal('2.294498'),

+ 4 - 12
netbox/netbox/ui/attrs.py

@@ -3,6 +3,7 @@ from django.utils.safestring import mark_safe
 from django.utils.translation import gettext_lazy as _
 
 from netbox.config import get_config
+from netbox.ui.utils import build_coords_url, is_coordinate_map_url
 from utilities.data import resolve_attr_path
 
 __all__ = (
@@ -457,15 +458,6 @@ class GenericForeignKeyAttr(ObjectAttribute):
         }
 
 
-def _build_coords_url(map_url, latitude, longitude):
-    """Build a GPS map URL, substituting {lat}/{lon} placeholders or appending as a comma-separated pair."""
-    lat_str = str(latitude)
-    lon_str = str(longitude)
-    if '{lat}' in map_url or '{lon}' in map_url:
-        return map_url.replace('{lat}', lat_str).replace('{lon}', lon_str)
-    return f'{map_url}{lat_str},{lon_str}'
-
-
 class AddressAttr(MapURLMixin, ObjectAttribute):
     """
     A physical or mailing address.
@@ -482,8 +474,8 @@ class AddressAttr(MapURLMixin, ObjectAttribute):
 
     def get_context(self, obj, attr, value, context):
         map_url = self.map_url
-        # A GPS-format MAPS_URL (containing {lat}/{lon}) cannot be used for address rendering
-        if map_url and ('{lat}' in map_url or '{lon}' in map_url):
+        # A coordinate-format MAPS_URL (containing {lat}/{lon}) cannot be used for address rendering
+        if map_url and is_coordinate_map_url(map_url):
             map_url = None
         return {
             'map_url': map_url,
@@ -515,7 +507,7 @@ class GPSCoordinatesAttr(MapURLMixin, ObjectAttribute):
             return self.placeholder
         map_url = self.map_url
         if map_url:
-            map_url = _build_coords_url(map_url, latitude, longitude)
+            map_url = build_coords_url(map_url, latitude, longitude)
         return render_to_string(self.template_name, {
             'name': context['name'],
             'latitude': latitude,

+ 23 - 0
netbox/netbox/ui/utils.py

@@ -0,0 +1,23 @@
+__all__ = (
+    'build_coords_url',
+    'is_coordinate_map_url',
+)
+
+
+def is_coordinate_map_url(url):
+    """Return True if the URL contains GPS coordinate placeholders ({lat} or {lon})."""
+    return '{lat}' in url or '{lon}' in url
+
+
+def build_coords_url(map_url, latitude, longitude):
+    """
+    Build a GPS map URL from a base URL and coordinate values.
+
+    If the URL contains {lat} or {lon} placeholders they are substituted directly;
+    otherwise the coordinates are appended as a comma-separated pair.
+    """
+    lat_str = str(latitude)
+    lon_str = str(longitude)
+    if is_coordinate_map_url(map_url):
+        return map_url.replace('{lat}', lat_str).replace('{lon}', lon_str)
+    return f'{map_url}{lat_str},{lon_str}'