소스 검색

Refactor measurement display: eliminate duplication and fix review issues

- Rename private constants (_IMPERIAL_WEIGHT etc.) to public names
- Extract compute_weight_display() / compute_distance_display() shared
  helpers in attrs.py; have both WeightAttr/DistanceAttr and the
  display_weight/display_distance template tags call them instead of
  duplicating the conversion math
- Fix inconsistent null check: use `is not None` for abs_weight
  (previously used truthy check) to match abs_distance behaviour
- Consolidate duplicate dcim/rack/attrs/total_weight.html and
  dcim/device/attrs/total_weight.html into a single shared template
  at dcim/attrs/total_weight.html; update both panel references
- Add display_weight and display_distance to helpers.py __all__
- Add comments to WeightMixin.abs_weight / DistanceMixin.abs_distance
  explaining why the public properties exist

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Brian Tiemann 1 일 전
부모
커밋
0928a23ab2

+ 2 - 2
netbox/dcim/ui/panels.py

@@ -62,7 +62,7 @@ class RackPanel(panels.ObjectAttributesPanel):
 class RackWeightPanel(panels.ObjectAttributesPanel):
 class RackWeightPanel(panels.ObjectAttributesPanel):
     weight = attrs.WeightAttr('weight')
     weight = attrs.WeightAttr('weight')
     max_weight = attrs.WeightAttr('max_weight', label=_('Maximum weight'))
     max_weight = attrs.WeightAttr('max_weight', label=_('Maximum weight'))
-    total_weight = attrs.TemplatedAttr('total_weight', template_name='dcim/rack/attrs/total_weight.html')
+    total_weight = attrs.TemplatedAttr('total_weight', template_name='dcim/attrs/total_weight.html')
 
 
 
 
 class RackRolePanel(panels.OrganizationalObjectPanel):
 class RackRolePanel(panels.OrganizationalObjectPanel):
@@ -137,7 +137,7 @@ class DeviceDeviceTypePanel(panels.ObjectAttributesPanel):
 class DeviceDimensionsPanel(panels.ObjectAttributesPanel):
 class DeviceDimensionsPanel(panels.ObjectAttributesPanel):
     title = _('Dimensions')
     title = _('Dimensions')
 
 
-    total_weight = attrs.TemplatedAttr('total_weight', template_name='dcim/device/attrs/total_weight.html')
+    total_weight = attrs.TemplatedAttr('total_weight', template_name='dcim/attrs/total_weight.html')
 
 
 
 
 class DeviceRolePanel(panels.NestedGroupObjectPanel):
 class DeviceRolePanel(panels.NestedGroupObjectPanel):

+ 2 - 0
netbox/netbox/models/mixins.py

@@ -53,6 +53,7 @@ class WeightMixin(models.Model):
 
 
     @property
     @property
     def abs_weight(self):
     def abs_weight(self):
+        # Public alias for _abs_weight; Django templates cannot access underscore-prefixed attributes.
         return self._abs_weight
         return self._abs_weight
 
 
     def save(self, *args, **kwargs):
     def save(self, *args, **kwargs):
@@ -101,6 +102,7 @@ class DistanceMixin(models.Model):
 
 
     @property
     @property
     def abs_distance(self):
     def abs_distance(self):
+        # Public alias for _abs_distance; Django templates cannot access underscore-prefixed attributes.
         return self._abs_distance
         return self._abs_distance
 
 
     def save(self, *args, **kwargs):
     def save(self, *args, **kwargs):

+ 45 - 39
netbox/netbox/ui/attrs.py

@@ -565,10 +565,45 @@ class UtilizationAttr(ObjectAttribute):
     template_name = 'ui/attrs/utilization.html'
     template_name = 'ui/attrs/utilization.html'
 
 
 
 
-_IMPERIAL_WEIGHT = {'lb', 'oz'}
-_METRIC_WEIGHT = {'kg', 'g'}
-_IMPERIAL_DISTANCE = {'mi', 'ft'}
-_METRIC_DISTANCE = {'km', 'm'}
+IMPERIAL_WEIGHT = {'lb', 'oz'}
+METRIC_WEIGHT = {'kg', 'g'}
+IMPERIAL_DISTANCE = {'mi', 'ft'}
+METRIC_DISTANCE = {'km', 'm'}
+
+
+def compute_weight_display(weight, weight_unit, abs_weight, system):
+    """
+    Return (display_value, display_unit) for a weight, respecting the user's measurement system.
+    abs_weight is in grams (from WeightMixin._abs_weight).
+    oz and g pass through unchanged since there is no cross-system equivalent.
+    """
+    if system == 'metric' and weight_unit in IMPERIAL_WEIGHT and abs_weight is not None:
+        return round(abs_weight / 1000, 2), 'kg'
+    if system == 'imperial' and weight_unit in METRIC_WEIGHT and abs_weight is not None:
+        lbs = round(abs_weight / 453.592, 2)
+        return lbs, 'lb' if lbs == 1 else 'lbs'
+    if weight_unit == 'lb':
+        return weight, 'lb' if weight == 1 else 'lbs'
+    return weight, weight_unit
+
+
+def compute_distance_display(distance, distance_unit, abs_distance, system):
+    """
+    Return (display_value, display_unit) for a distance, respecting the user's measurement system.
+    abs_distance is in metres (from DistanceMixin._abs_distance).
+    Distances < 1 km are shown in metres; < 1 mi are shown in feet.
+    """
+    if system == 'metric' and distance_unit in IMPERIAL_DISTANCE and abs_distance is not None:
+        abs_m = float(abs_distance)
+        if abs_m >= 1000:
+            return round(abs_m / 1000, 2), 'km'
+        return round(abs_m, 2), 'm'
+    if system == 'imperial' and distance_unit in METRIC_DISTANCE and abs_distance is not None:
+        abs_m = float(abs_distance)
+        if abs_m >= 1609.344:
+            return round(abs_m / 1609.344, 2), 'mi'
+        return round(abs_m / 0.3048, 2), 'ft'
+    return distance, distance_unit
 
 
 
 
 class WeightAttr(ObjectAttribute):
 class WeightAttr(ObjectAttribute):
@@ -577,7 +612,8 @@ class WeightAttr(ObjectAttribute):
 
 
     Parameters:
     Parameters:
         unit_attr (str): Name of the field holding the weight unit (default: 'weight_unit')
         unit_attr (str): Name of the field holding the weight unit (default: 'weight_unit')
-        abs_attr (str): Name of the field holding the absolute weight in grams (default: '_abs_weight')
+        abs_attr (str): The internal _abs_weight field name on WeightMixin (stored in grams).
+            Accessed via Python — not subject to Django's template underscore restriction.
     """
     """
     template_name = 'ui/attrs/numeric.html'
     template_name = 'ui/attrs/numeric.html'
 
 
@@ -594,19 +630,7 @@ class WeightAttr(ObjectAttribute):
         system = (context.get('preferences') or {}).get('ui.measurement_system') or ''
         system = (context.get('preferences') or {}).get('ui.measurement_system') or ''
         unit = resolve_attr_path(obj, self.unit_attr)
         unit = resolve_attr_path(obj, self.unit_attr)
         abs_weight = resolve_attr_path(obj, self.abs_attr)
         abs_weight = resolve_attr_path(obj, self.abs_attr)
-
-        if system == 'metric' and unit in _IMPERIAL_WEIGHT and abs_weight:
-            display_value = round(abs_weight / 1000, 2)
-            display_unit = 'kg'
-        elif system == 'imperial' and unit in _METRIC_WEIGHT and abs_weight:
-            display_value = round(abs_weight / 453.592, 2)
-            display_unit = 'lb' if display_value == 1 else 'lbs'
-        else:
-            display_value = weight
-            if unit == 'lb':
-                display_unit = 'lb' if display_value == 1 else 'lbs'
-            else:
-                display_unit = unit
+        display_value, display_unit = compute_weight_display(weight, unit, abs_weight, system)
 
 
         return render_to_string(self.template_name, {
         return render_to_string(self.template_name, {
             'name': context['name'],
             'name': context['name'],
@@ -621,7 +645,8 @@ class DistanceAttr(ObjectAttribute):
 
 
     Parameters:
     Parameters:
         unit_attr (str): Name of the field holding the distance unit (default: 'distance_unit')
         unit_attr (str): Name of the field holding the distance unit (default: 'distance_unit')
-        abs_attr (str): Name of the field holding the absolute distance in meters (default: '_abs_distance')
+        abs_attr (str): The internal _abs_distance field name on DistanceMixin (stored in metres).
+            Accessed via Python — not subject to Django's template underscore restriction.
     """
     """
     template_name = 'ui/attrs/numeric.html'
     template_name = 'ui/attrs/numeric.html'
 
 
@@ -638,26 +663,7 @@ class DistanceAttr(ObjectAttribute):
         system = (context.get('preferences') or {}).get('ui.measurement_system') or ''
         system = (context.get('preferences') or {}).get('ui.measurement_system') or ''
         unit = resolve_attr_path(obj, self.unit_attr)
         unit = resolve_attr_path(obj, self.unit_attr)
         abs_distance = resolve_attr_path(obj, self.abs_attr)
         abs_distance = resolve_attr_path(obj, self.abs_attr)
-
-        if system == 'metric' and unit in _IMPERIAL_DISTANCE and abs_distance is not None:
-            abs_m = float(abs_distance)
-            if abs_m >= 1000:
-                display_value = round(abs_m / 1000, 2)
-                display_unit = 'km'
-            else:
-                display_value = round(abs_m, 2)
-                display_unit = 'm'
-        elif system == 'imperial' and unit in _METRIC_DISTANCE and abs_distance is not None:
-            abs_m = float(abs_distance)
-            if abs_m >= 1609.344:
-                display_value = round(abs_m / 1609.344, 2)
-                display_unit = 'mi'
-            else:
-                display_value = round(abs_m / 0.3048, 2)
-                display_unit = 'ft'
-        else:
-            display_value = distance
-            display_unit = unit  # 'km', 'm', 'mi', 'ft' are standard abbreviations
+        display_value, display_unit = compute_distance_display(distance, unit, abs_distance, system)
 
 
         return render_to_string(self.template_name, {
         return render_to_string(self.template_name, {
             'name': context['name'],
             'name': context['name'],

+ 1 - 1
netbox/templates/dcim/device/attrs/total_weight.html → netbox/templates/dcim/attrs/total_weight.html

@@ -7,4 +7,4 @@
 {% else %}
 {% else %}
 {% with formatted=value|floatformat %}{% with lbs=value|kg_to_pounds|floatformat %}{{ formatted }} {% trans "Kilogram" %}{{ formatted|pluralize }} ({{ lbs }} {% trans "Pound" %}{{ lbs|pluralize }}){% endwith %}{% endwith %}
 {% with formatted=value|floatformat %}{% with lbs=value|kg_to_pounds|floatformat %}{{ formatted }} {% trans "Kilogram" %}{{ formatted|pluralize }} ({{ lbs }} {% trans "Pound" %}{{ lbs|pluralize }}){% endwith %}{% endwith %}
 {% endif %}
 {% endif %}
-{% endwith %}
+{% endwith %}

+ 0 - 10
netbox/templates/dcim/rack/attrs/total_weight.html

@@ -1,10 +0,0 @@
-{% load helpers i18n %}
-{% with system=preferences|get_key:"ui.measurement_system" %}
-{% if system == "imperial" %}
-{% with lbs=value|kg_to_pounds|floatformat %}{{ lbs }} {% trans "Pound" %}{{ lbs|pluralize }}{% endwith %}
-{% elif system == "metric" %}
-{% with formatted=value|floatformat %}{{ formatted }} {% trans "Kilogram" %}{{ formatted|pluralize }}{% endwith %}
-{% else %}
-{% with formatted=value|floatformat %}{% with lbs=value|kg_to_pounds|floatformat %}{{ formatted }} {% trans "Kilogram" %}{{ formatted|pluralize }} ({{ lbs }} {% trans "Pound" %}{{ lbs|pluralize }}){% endwith %}{% endwith %}
-{% endif %}
-{% endwith %}

+ 10 - 23
netbox/utilities/templatetags/helpers.py

@@ -10,7 +10,10 @@ from django.utils.translation import gettext_lazy as _
 
 
 from core.models import ObjectType
 from core.models import ObjectType
 from netbox.settings import DISK_BASE_UNIT, RAM_BASE_UNIT
 from netbox.settings import DISK_BASE_UNIT, RAM_BASE_UNIT
-from netbox.ui.attrs import _IMPERIAL_DISTANCE, _IMPERIAL_WEIGHT, _METRIC_DISTANCE, _METRIC_WEIGHT
+from netbox.ui.attrs import (
+    compute_distance_display,
+    compute_weight_display,
+)
 from utilities.forms import TableConfigForm, get_selected_values
 from utilities.forms import TableConfigForm, get_selected_values
 from utilities.forms.mixins import FORM_FIELD_LOOKUPS
 from utilities.forms.mixins import FORM_FIELD_LOOKUPS
 from utilities.views import get_action_url, get_viewname
 from utilities.views import get_action_url, get_viewname
@@ -19,6 +22,8 @@ __all__ = (
     'action_url',
     'action_url',
     'applied_filters',
     'applied_filters',
     'as_range',
     'as_range',
+    'display_distance',
+    'display_weight',
     'divide',
     'divide',
     'get_item',
     'get_item',
     'get_key',
     'get_key',
@@ -337,42 +342,24 @@ def kg_to_pounds(n):
 def display_weight(context, weight, weight_unit, abs_weight):
 def display_weight(context, weight, weight_unit, abs_weight):
     """
     """
     Render a weight value respecting the user's ui.measurement_system preference.
     Render a weight value respecting the user's ui.measurement_system preference.
-    When the stored unit conflicts with the preferred system, converts via abs_weight (grams).
-    Falls back to the stored value/unit when no conversion is needed.
     """
     """
     if weight is None:
     if weight is None:
         return ''
         return ''
     system = (context.get('preferences') or {}).get('ui.measurement_system') or ''
     system = (context.get('preferences') or {}).get('ui.measurement_system') or ''
-    if system == 'metric' and weight_unit in _IMPERIAL_WEIGHT and abs_weight:
-        return f'{round(abs_weight / 1000, 2):g} kg'
-    if system == 'imperial' and weight_unit in _METRIC_WEIGHT and abs_weight:
-        lbs = round(abs_weight / 453.592, 2)
-        return f'{lbs:g} {"lb" if lbs == 1 else "lbs"}'
-    unit = 'lb' if (weight_unit == 'lb' and weight == 1) else ('lbs' if weight_unit == 'lb' else weight_unit)
-    return f'{weight:g} {unit}'
+    value, unit = compute_weight_display(weight, weight_unit, abs_weight, system)
+    return f'{value:g} {unit}'
 
 
 
 
 @register.simple_tag(takes_context=True)
 @register.simple_tag(takes_context=True)
 def display_distance(context, distance, distance_unit, abs_distance):
 def display_distance(context, distance, distance_unit, abs_distance):
     """
     """
     Render a distance value respecting the user's ui.measurement_system preference.
     Render a distance value respecting the user's ui.measurement_system preference.
-    When the stored unit conflicts with the preferred system, converts via abs_distance (meters).
-    Falls back to the stored value/unit when no conversion is needed.
     """
     """
     if distance is None:
     if distance is None:
         return ''
         return ''
     system = (context.get('preferences') or {}).get('ui.measurement_system') or ''
     system = (context.get('preferences') or {}).get('ui.measurement_system') or ''
-    if system == 'metric' and distance_unit in _IMPERIAL_DISTANCE and abs_distance is not None:
-        abs_m = float(abs_distance)
-        if abs_m >= 1000:
-            return f'{round(abs_m / 1000, 2):g} km'
-        return f'{round(abs_m, 2):g} m'
-    if system == 'imperial' and distance_unit in _METRIC_DISTANCE and abs_distance is not None:
-        abs_m = float(abs_distance)
-        if abs_m >= 1609.344:
-            return f'{round(abs_m / 1609.344, 2):g} mi'
-        return f'{round(abs_m / 0.3048, 2):g} ft'
-    return f'{distance:g} {distance_unit}'
+    value, unit = compute_distance_display(distance, distance_unit, abs_distance, system)
+    return f'{value:g} {unit}'
 
 
 
 
 @register.filter("startswith")
 @register.filter("startswith")