Skip to content

Expand Edit Interface 802.1Q VLAN form search query #3840

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

Closed
tyler-8 opened this issue Jan 3, 2020 · 9 comments · Fixed by #3925
Closed

Expand Edit Interface 802.1Q VLAN form search query #3840

tyler-8 opened this issue Jan 3, 2020 · 9 comments · Fixed by #3925
Assignees
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@tyler-8
Copy link
Contributor

tyler-8 commented Jan 3, 2020

Environment

  • Python version: 3.6.8
  • NetBox version: 2.6.7

Proposed Functionality

When using the GUI Select2 fields for adding untagged/tagged VLANs to a device/VM interface, expand the search parameters to also search by site and VLAN group. Currently it appears that only the VLAN Name and VID are used. The dropdown currently displays the sites in the dropdown list, but they're not part of the search parameters.

Use Case

When you have a cookiecutter design for a particular type of site, the VIDs and VLAN names are repeated identically, and added to their respective sites and VLAN groups, if applicable.

When you go to add those VLANs to an interface and search for vlan 5 for example, you may get dozens or hundreds of results because that VID and name are repeated lots of times. This makes it nearly impossible to add the VLAN through the GUI.

A potential workaround with the current state is to make all the VLAN names unique by adding the site name to the VLAN name, but this seems repetitive and less flexible, should the site name ever change for example.

A cursory glance makes this appear to be an issue with the REST API (and the q param) but there is also a site filter param that could be used in this case.

Database Changes

N/A

External Dependencies

N/A

@stale
Copy link

stale bot commented Jan 19, 2020

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.

@stale stale bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Jan 19, 2020
@tyler-8
Copy link
Contributor Author

tyler-8 commented Jan 20, 2020

This is being worked on, stalebot!

@stale stale bot removed the pending closure Requires immediate attention to avoid being closed for inactivity label Jan 20, 2020
@hSaria
Copy link
Contributor

hSaria commented Jan 20, 2020 via email

@jeremystretch
Copy link
Member

This is being worked on, stalebot!

Per the contributing guide:

Be sure to open an issue before starting work on a pull request, and discuss your idea with the NetBox maintainers before beginning work. This will help prevent wasting time on something that might we might not be able to implement. When suggesting a new feature, also make sure it won't conflict with any work that's already in progress.

and

Any pull request which does not relate to an accepted issue will be closed.

While community contributions are always appreciated, it is crucial that development effort remains focused on priorities. We introduced stalebot for a reason: to automatically close issues that are either not of interest (as indicated by the lack of discussion) or that we do not have the resources to explore currently.

Please wait to begin any work on an issue until after a maintainer has marked it as accepted, and understand that every issue that gets submitted has a time cost associated with it, as do pull requests. The mere presence of a pull request has no bearing on whether an issue gets accepted, as we cannot allow arbitrary engagement to drive long-term prioritization. We discourage contributors from expending the effort prematurely to avoid a situation where they perceive their time as wasted.

@DanSheps
Copy link
Member

I think one thing we need to make sure to capture in this, and I haven't looked at the PR yet to see if it is captured, is that when you search by VID, you need to search the global, the global groups, and the site specific vlan and vlan groups.

I do think this is something we should look at doing and it was on the todo list, however the filtering javascript would need more love then I could dedicate to it to get it working the way it needs to. If hSaria is willing to own this, I don't see any problem accepting it.

@hSaria
Copy link
Contributor

hSaria commented Jan 24, 2020

@DanSheps Yep, that's captured in the PR.

... the global, the global groups, and the site specific vlan and vlan groups.

Simply limiting the VLANs to the global ones site_id=null and the site-specific ones site_id={site_pk} will achieve this.

There's an explanation in #3589 (comment), but here's the summary on why it works:

  • a VLAN can be in a global VLAN group only if it has no site, and
  • a VLAN can be in a site VLAN group only it it is in the same site.

Stated differently

  • a VLAN in a site cannot be in a global VLAN group, and
  • a VLAN not in a site cannot be in a site VLAN group.

@hSaria
Copy link
Contributor

hSaria commented Jan 27, 2020

If hSaria is willing to own this, I don't see any problem accepting it.

@DanSheps, sure. The PR already adjusts how the data-additional-query-param-* is handled to allow multiple values (i.e. calling add_additional_query_param multiple times with the same name but different values will simply add to a list, instead of overriding it). If there's anything else you guys think needs to be done, I'd be happy to help where I can.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application labels Jan 30, 2020
@jeremystretch jeremystretch added status: blocked Another issue or external requirement is preventing implementation and removed status: accepted This issue has been accepted for implementation labels Jan 30, 2020
@jeremystretch
Copy link
Member

Marking this as blocked until we have a chance to look at consolidating the interface forms (#4057). Will keep the PR open.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: blocked Another issue or external requirement is preventing implementation labels Feb 14, 2020
@jeremystretch
Copy link
Member

Unblocking since it might actually be easier to tackle #4057 after these changes are made.

jeremystretch added a commit that referenced this issue Feb 14, 2020
Fixes #3840: Only show valid interface VLAN choices
jeremystretch added a commit that referenced this issue Feb 14, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants