Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visibility of when opposite rack face is occupied #6961

Closed
candlerb opened this issue Aug 14, 2021 · 8 comments
Closed

Visibility of when opposite rack face is occupied #6961

candlerb opened this issue Aug 14, 2021 · 8 comments
Labels
pending closure Requires immediate attention to avoid being closed for inactivity status: under review Further discussion is needed to determine this issue's scope and/or implementation type: feature Introduction of new functionality to the application

Comments

@candlerb
Copy link
Contributor

NetBox version

v2.11.11 and v3.0-beta2

Feature type

Change to existing functionality

Proposed functionality

This is regarding rack elevations.

Currently, when a half-depth device is installed in the front face, the rear face doesn't show any indication of this. I would like a way to be able to observe, visually, when the opposite face is occupied.

If the front is occupied with with a full-depth device, the rear shows either the rear image, or a red striped pattern with the device name - and that's fine.

What I want is if the front is occupied with a half-depth device, at least something is visible. This could be a different kind of hatching, for example.

Use case

To be able to distinguish visually between:

  • a rack unit which is completely unoccupied
  • a rack unit which has the opposite face occupied (and therefore can only take a half-depth device on this side)

This in turn helps easily identify which spaces are usable for a full-depth device.

Database changes

None

External dependencies

None

@candlerb candlerb added the type: feature Introduction of new functionality to the application label Aug 14, 2021
@candlerb
Copy link
Contributor Author

candlerb commented Aug 14, 2021

There is a CSS class "occupied", and it might be intended for this purpose, but I cannot see how it can ever be activated. Looking at the source code:

            elif device:   <<< (1)
                # Devices which the user does not have permission to view are rendered only as unavailable space
                drawing.add(drawing.rect(start_cordinates, end_cordinates, class_='blocked'))
            else:
                # Draw shallow devices, reservations, or empty units
                class_ = 'slot'
                reservation = reserved_units.get(unit["id"])
                if device:    <<< (2)
                    class_ += ' occupied'

I cannot see how the "if device" at (2) can ever be true, because if it were, the branch at (1) would have been taken.

(I linked to the Netbox v3 beta code, but the same code is in v2.11.11 in netbox/dcim/elevations.py)

@candlerb
Copy link
Contributor Author

candlerb commented Aug 15, 2021

If the intention is that class="occupied" is used when the opposite face is occupied, then this proof-of-concept makes it work that way:

--- a/netbox/dcim/svg.py
+++ b/netbox/dcim/svg.py
@@ -173,7 +173,7 @@ class RackElevationSVG:
         unit_cursor = 0
         for u in elevation:
             o = other[unit_cursor]
-            if not u['device'] and o['device'] and o['device'].device_type.is_full_depth:
+            if not u['device'] and o['device']:
                 u['device'] = o['device']
                 u['height'] = 1
             unit_cursor += u.get('height', 1)
@@ -218,7 +218,7 @@ class RackElevationSVG:
                 self._draw_device_front(drawing, device, start_cordinates, end_cordinates, text_cordinates)
             elif device and device.device_type.is_full_depth and device.pk in self.permitted_device_ids:
                 self._draw_device_rear(drawing, device, start_cordinates, end_cordinates, text_cordinates)
-            elif device:
+            elif device and device.face == face:
                 # Devices which the user does not have permission to view are rendered only as unavailable space
                 drawing.add(drawing.rect(start_cordinates, end_cordinates, class_='blocked'))
             else:

image

I won't provide that as a PR until it has been confirmed what the intended purpose of "occupied" is.

EDIT: if a unit is both "occupied" and "reserved", both classes are applied. In my browser (Chrome) it renders as if only "occupied" - and oddly, it doesn't make a difference if I swap the classes around. I wonder if it would be better to render such cases as "reserved", as it's still possible to reserve space for a half-depth device when the other face is occupied. That would be an easy change in the Python, or it's probably possible in the CSS somehow.

@jeremystretch
Copy link
Member

Sounds like a duplicate of #6156?

@candlerb
Copy link
Contributor Author

candlerb commented Aug 16, 2021

Yes I guess so. However in this case I've actually dug into the code :-)

Do you know what the intended purpose of class="occupied" was? Even though it can never be applied by the current code.

Looking through history, it appears to have been added originally in 31e65a0:

Closes #4940: Added an occupied field to rack unit representations for rack elevation views

But the description in that issue sounds more like the use of "blocked" rather than "occupied":

a need to convey that a particular rack unit is occupied without revealing the specific device occupying it

EDIT: earlier than that, I find 1ec191d ("initial cleanup of rack elevations")

You need to trace the renames through:

  • netbox/dcim/models.py
  • netbox/dcim/models/__init__.py
  • netbox/dcim/elevations.py
  • netbox/dcim/svg.py (v3 only)

@candlerb
Copy link
Contributor Author

candlerb commented Sep 7, 2021

@jeremystretch, any comment regarding:

  1. what the original intended purpose of class="occupied" was?
  2. whether it's acceptable to use it now to mean "opposite rack face is populated", given that it's never used at the moment?

@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Sep 28, 2021
@candlerb
Copy link
Contributor Author

candlerb commented Nov 22, 2021

I would like to add a second feature to this: if a device is half-depth, and it has a rear image, then show the rear image when the opposite face is unoccupied.

At the moment the opposite face shows completely blank, and this original FR is to show some indication that the opposite side was in use. I think that if the device has a rear image, then that's the image that should be used (possibly greyed out, or with some other way to indicate that it's not full depth)

At the moment, as far as I can tell, the rear image is never used for a half-depth device (except when viewing the Device Type of course)

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Jan 22, 2022
@netbox-community netbox-community deleted a comment from candlerb Feb 28, 2022
@github-actions
Copy link

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the core maintainers may elect to reopen this issue at a later date if deemed necessary.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pending closure Requires immediate attention to avoid being closed for inactivity status: under review Further discussion is needed to determine this issue's scope and/or implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

2 participants