-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
limits vlans on interface tables #17662
Conversation
netbox/dcim/tables/template_code.py
Outdated
@@ -56,9 +56,13 @@ | |||
|
|||
INTERFACE_TAGGED_VLANS = """ | |||
{% if record.mode == 'tagged' %} | |||
{% if value.count > 3 %} | |||
<a href="{{ record.get_absolute_url }}#vlans">{{ value.count }}</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of the interface view, let's link to the VLAN list with a filter for the current interface. This will be consistent with the way we currently handle assigned IP addresses (see INTERFACE_IPADDRESSES
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was also my initial plan, but VLANs doesn't seem to have interface filter like IP-address do.
So ipam/vlans/?(vm)interface_id=XXX needs to be implemented first for this to work.
That's why I implemented it with "vlan" anchor on interface page instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right. I've just opened FR #17669 to add filters for this. I'll see if I can knock that out real quick today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the pull request to support #17669
Just my two cents on this, as I know it's being implemented the same way as for IP addresses. For IP addresses, having just a number instead of a list of IPs is somewhat intuitive as to what is being conveyed, but in this context, I have a concern that having just a number may be misinterpreted as representing an actual VID and not a count that needs to be clicked on for additional details. |
@sleepinggenius2 I've changed it to "5 VLANs" - not just "5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To ensure that VLAN IDs are rendered correctly when exporting e.g. to CSV, we also need to add a method to BaseInterfaceTable (similar to value_ip_addresses
). This should work:
def value_tagged_vlans(self, value):
return ",".join([str(obj.vid) for obj in value.all()])
I used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @thordreier!
* limits vlans on interface tables * limits vlans on interface tables * limits vlans on interface tables * limits vlans on interface tables
* limits vlans on interface tables * limits vlans on interface tables * limits vlans on interface tables * limits vlans on interface tables
Fixes: #17655
Limit VLANs displayed on interface tables