Просмотр исходного кода

Address code review feedback

- Expand WeightAttr else-branch ternary into readable if/else
- Fix display_weight tag: use 'if weight is None' instead of 'if not weight'
  to correctly handle weight=0
- Fix display_weight tag: apply lb/lbs singular/plural logic consistently
  with WeightAttr (was always returning 'lbs')
- Deduplicate unit sets: import _IMPERIAL_WEIGHT/_METRIC_WEIGHT/
  _IMPERIAL_DISTANCE/_METRIC_DISTANCE from attrs.py into helpers.py
  instead of re-declaring them inline
- Add trailing newline to both total_weight.html templates

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Brian Tiemann 2 дней назад
Родитель
Сommit
5489ae9f5f

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

@@ -603,7 +603,10 @@ class WeightAttr(ObjectAttribute):
             display_unit = 'lb' if display_value == 1 else 'lbs'
         else:
             display_value = weight
-            display_unit = 'lb' if (unit == 'lb' and display_value == 1) else ('lbs' if unit == 'lb' else unit)
+            if unit == 'lb':
+                display_unit = 'lb' if display_value == 1 else 'lbs'
+            else:
+                display_unit = unit
 
         return render_to_string(self.template_name, {
             'name': context['name'],

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

@@ -7,4 +7,4 @@
 {% 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 %}
+{% endwith %}

+ 1 - 1
netbox/templates/dcim/rack/attrs/total_weight.html

@@ -7,4 +7,4 @@
 {% 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 %}
+{% endwith %}

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

@@ -10,6 +10,7 @@ from django.utils.translation import gettext_lazy as _
 
 from core.models import ObjectType
 from netbox.settings import DISK_BASE_UNIT, RAM_BASE_UNIT
+from netbox.ui.attrs import _IMPERIAL_DISTANCE, _IMPERIAL_WEIGHT, _METRIC_DISTANCE, _METRIC_WEIGHT
 from utilities.forms import TableConfigForm, get_selected_values
 from utilities.forms.mixins import FORM_FIELD_LOOKUPS
 from utilities.views import get_action_url, get_viewname
@@ -339,16 +340,16 @@ def display_weight(context, weight, weight_unit, abs_weight):
     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 not weight:
+    if weight is None:
         return ''
     system = (context.get('preferences') or {}).get('ui.measurement_system') or ''
-    _IMPERIAL = {'lb', 'oz'}
-    _METRIC = {'kg', 'g'}
-    if system == 'metric' and weight_unit in _IMPERIAL and abs_weight:
+    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 and abs_weight:
-        return f'{round(abs_weight / 453.592, 2):g} lbs'
-    return f'{weight:g} {weight_unit}'
+    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}'
 
 
 @register.simple_tag(takes_context=True)
@@ -361,14 +362,12 @@ def display_distance(context, distance, distance_unit, abs_distance):
     if distance is None:
         return ''
     system = (context.get('preferences') or {}).get('ui.measurement_system') or ''
-    _IMPERIAL = {'mi', 'ft'}
-    _METRIC = {'km', 'm'}
-    if system == 'metric' and distance_unit in _IMPERIAL and abs_distance is not None:
+    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 and abs_distance is not None:
+    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'