From e5b49e25b9153c53f9551dda065d7a0bc28c266f Mon Sep 17 00:00:00 2001 From: hellerve Date: Sun, 8 Dec 2019 18:14:59 +0100 Subject: [PATCH] dcim api: add feedback from @jeremystretch to rack elevations api --- netbox/dcim/api/serializers.py | 2 +- netbox/dcim/api/views.py | 100 ++++++------------------------- netbox/dcim/constants.py | 55 +++++++++++++++++ netbox/dcim/models.py | 2 +- netbox/dcim/tests/test_models.py | 4 +- netbox/templates/dcim/rack.html | 4 +- 6 files changed, 80 insertions(+), 87 deletions(-) diff --git a/netbox/dcim/api/serializers.py b/netbox/dcim/api/serializers.py index 3c4170373..84a08107b 100644 --- a/netbox/dcim/api/serializers.py +++ b/netbox/dcim/api/serializers.py @@ -157,7 +157,7 @@ class RackUnitSerializer(serializers.Serializer): """ id = serializers.IntegerField(read_only=True) name = serializers.CharField(read_only=True) - face = serializers.IntegerField(read_only=True) + face = ChoiceField(choices=DeviceFaceChoices, required=False, allow_null=True) device = NestedDeviceSerializer(read_only=True) diff --git a/netbox/dcim/api/views.py b/netbox/dcim/api/views.py index d66bc020b..22246ae2d 100644 --- a/netbox/dcim/api/views.py +++ b/netbox/dcim/api/views.py @@ -15,7 +15,7 @@ from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet, ViewSet from circuits.models import Circuit -from dcim import filters +from dcim import constants, filters from dcim.models import ( Cable, ConsolePort, ConsolePortTemplate, ConsoleServerPort, ConsoleServerPortTemplate, Device, DeviceBay, DeviceBayTemplate, DeviceRole, DeviceType, FrontPort, FrontPortTemplate, Interface, InterfaceTemplate, @@ -30,9 +30,7 @@ from ipam.models import Prefix, VLAN from utilities.api import ( get_serializer_for_model, IsAuthenticatedOrLoginNotRequired, FieldChoicesViewSet, ModelViewSet, ServiceUnavailable, ) -from utilities.utils import get_subquery -# XXX: should this be moved to a util function so that we don’t have to import templatetags? -from utilities.templatetags.helpers import fgcolor +from utilities.utils import get_subquery, foreground_color from virtualization.models import VirtualMachine from . import serializers from .exceptions import MissingFilterException @@ -205,61 +203,6 @@ class RackViewSet(CustomFieldModelViewSet): return self.get_paginated_response(rack_units.data) -RACK_ELEVATION_STYLE = """ -* { - font-family: 'Helvetica Neue'; - font-size: 13px; -} -rect { - box-sizing: border-box; -} -text { - text-anchor: middle; - dominant-baseline: middle; -} -.rack { - background-color: #f0f0f0; - fill: none; - stroke: black; - stroke-width: 3px; -} -.slot { - fill: #f7f7f7; - stroke: #a0a0a0; -} -.slot:hover { - fill: #fff; -} -.slot+.add-device { - fill: none; -} -.slot:hover+.add-device { - fill: blue; -} -.reserved { - fill: url(#reserved); -} -.reserved:hover { - fill: url(#reserved); -} -.occupied { - fill: url(#occupied); -} -.occupied:hover { - fill: url(#occupied); -} -.blocked { - fill: url(#blocked); -} -.blocked:hover { - fill: url(#blocked); -} -.blocked:hover+.add-device { - fill: none; -} -""" - - class RackElevationViewSet(ViewSet): queryset = Rack.objects.prefetch_related( 'devices' @@ -280,7 +223,7 @@ class RackElevationViewSet(ViewSet): drawing = svgwrite.Drawing(size=(width, height)) # add the stylesheet - drawing.defs.add(drawing.style(RACK_ELEVATION_STYLE)) + drawing.defs.add(drawing.style(constants.RACK_ELEVATION_STYLE)) # add gradients self._add_gradient(drawing, 'reserved', '#c7c7ff') @@ -296,12 +239,9 @@ class RackElevationViewSet(ViewSet): reverse('dcim:device', kwargs={'pk': device.pk}), fill='black' ) ) - link.add( - drawing.rect(start, end, fill='#{}'.format(color)) - ) - link.add( - drawing.text(device.name, insert=text, fill=fgcolor(color)) - ) + link.add(drawing.rect(start, end, fill='#{}'.format(color))) + hex_color = '#{}'.format(foreground_color(color)) + link.add(drawing.text(device.name, insert=text, fill=hex_color)) def _draw_device_rear(self, drawing, device, start, end, text): drawing.add(drawing.rect(start, end, class_="blocked")) @@ -317,14 +257,14 @@ class RackElevationViewSet(ViewSet): link.add(drawing.rect(start, end, class_=class_)) link.add(drawing.text("add device", insert=text, class_='add-device')) - def _draw_elevations(self, rack, elevation, reserved, face_id, width, slot_height): - drawing = self._setup_drawing(width, slot_height * rack.u_height) + def _draw_elevations(self, rack, elevation, reserved, face_id, width, u_height): + drawing = self._setup_drawing(width, u_height * rack.u_height) i = 0 for u in elevation: device = u['device'] height = u['height'] - start_y = i * slot_height - end_y = slot_height * height + start_y = i * u_height + end_y = u_height * height start = (0, start_y) end = (width, end_y) text = (width / 2, start_y + end_y / 2) @@ -342,7 +282,7 @@ class RackElevationViewSet(ViewSet): rack, drawing, start, end, text, u["id"], face_id, class_ ) i += height - drawing.add(drawing.rect((0, 0), (width, rack.u_height * slot_height), class_='rack')) + drawing.add(drawing.rect((0, 0), (width, rack.u_height * u_height), class_='rack')) return drawing def _get_elevation(self, rack): @@ -366,28 +306,26 @@ class RackElevationViewSet(ViewSet): rack = get_object_or_404(Rack, pk=pk) face_id = request.GET.get('face', '0') - if face_id not in ['0', '1']: - return HttpResponseBadRequest('side should either be "0" or "1".') - # this is safe because of the validation above - face_id = int(face_id) + if face_id not in ['front', 'rear']: + return HttpResponseBadRequest('face should either be "front" or "rear".') - width = request.GET.get('width', '230') + width = request.GET.get('u_width', '230') try: width = int(width) except ValueError: - return HttpResponseBadRequest('width must be numeric.') + return HttpResponseBadRequest('u_width must be numeric.') - slot_height = request.GET.get('slot_height', '20') + u_height = request.GET.get('u_height', '20') try: - slot_height = int(slot_height) + u_height = int(u_height) except ValueError: - return HttpResponseBadRequest('slot_height must be numeric.') + return HttpResponseBadRequest('u_height must be numeric.') elevation = self._get_elevation(rack) reserved = rack.get_reserved_units().keys() - drawing = self._draw_elevations(rack, elevation, reserved, face_id, width, slot_height) + drawing = self._draw_elevations(rack, elevation, reserved, face_id, width, u_height) return HttpResponse(drawing.tostring(), content_type='image/svg+xml') diff --git a/netbox/dcim/constants.py b/netbox/dcim/constants.py index 8dacd68f5..a33682f51 100644 --- a/netbox/dcim/constants.py +++ b/netbox/dcim/constants.py @@ -55,3 +55,58 @@ COMPATIBLE_TERMINATION_TYPES = { 'rearport': ['consoleport', 'consoleserverport', 'interface', 'frontport', 'rearport', 'circuittermination'], 'circuittermination': ['interface', 'frontport', 'rearport'], } + + +RACK_ELEVATION_STYLE = """ +* { + font-family: 'Helvetica Neue'; + font-size: 13px; +} +rect { + box-sizing: border-box; +} +text { + text-anchor: middle; + dominant-baseline: middle; +} +.rack { + background-color: #f0f0f0; + fill: none; + stroke: black; + stroke-width: 3px; +} +.slot { + fill: #f7f7f7; + stroke: #a0a0a0; +} +.slot:hover { + fill: #fff; +} +.slot+.add-device { + fill: none; +} +.slot:hover+.add-device { + fill: blue; +} +.reserved { + fill: url(#reserved); +} +.reserved:hover { + fill: url(#reserved); +} +.occupied { + fill: url(#occupied); +} +.occupied:hover { + fill: url(#occupied); +} +.blocked { + fill: url(#blocked); +} +.blocked:hover { + fill: url(#blocked); +} +.blocked:hover+.add-device { + fill: none; +} +""" diff --git a/netbox/dcim/models.py b/netbox/dcim/models.py index 0a78f862f..244e61e54 100644 --- a/netbox/dcim/models.py +++ b/netbox/dcim/models.py @@ -670,7 +670,7 @@ class Rack(ChangeLoggedModel, CustomFieldModel): def get_status_class(self): return self.STATUS_CLASS_MAP.get(self.status) - def get_rack_units(self, face=Device.FaceChoices.FACE_FRONT, exclude=None): + def get_rack_units(self, face=DeviceFaceChoices.FACE_FRONT, exclude=None): """ Return a list of rack units as dictionaries. Example: {'device': None, 'face': 0, 'id': 48, 'name': 'U48'} Each key 'device' is either a Device or None. By default, multi-U devices are repeated for each U they occupy. diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index 634ccb3ca..9d7772804 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -125,14 +125,14 @@ class RackTestCase(TestCase): self.assertEqual(list(self.rack.units), list(reversed(range(1, 43)))) # Validate inventory (front face) - rack1_inventory_front = self.rack.get_rack_units(face=RACK_FACE_FRONT) + rack1_inventory_front = self.rack.get_rack_units(face=DeviceFaceChoices.FACE_FRONT) self.assertEqual(rack1_inventory_front[-10]['device'], device1) del(rack1_inventory_front[-10]) for u in rack1_inventory_front: self.assertIsNone(u['device']) # Validate inventory (rear face) - rack1_inventory_rear = self.rack.get_rack_units(face=RACK_FACE_REAR) + rack1_inventory_rear = self.rack.get_rack_units(face=DeviceFaceChoices.FACE_REAR) self.assertEqual(rack1_inventory_rear[-10]['device'], device1) del(rack1_inventory_rear[-10]) for u in rack1_inventory_rear: diff --git a/netbox/templates/dcim/rack.html b/netbox/templates/dcim/rack.html index e30ef19b6..0cc261a27 100644 --- a/netbox/templates/dcim/rack.html +++ b/netbox/templates/dcim/rack.html @@ -317,13 +317,13 @@

Front

- {% include 'dcim/inc/rack_elevation.html' with face=0 %} + {% include 'dcim/inc/rack_elevation.html' with face='front' %}

Rear

- {% include 'dcim/inc/rack_elevation.html' with face=1 %} + {% include 'dcim/inc/rack_elevation.html' with face='rear' %}