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

Support VLAN in Import Prefixes, Add VLAN column to Prefixes page #204

Closed
wants to merge 2 commits into from

Conversation

Gelob
Copy link
Contributor

@Gelob Gelob commented Jul 6, 2016

Addresses #42

This commit adds support for including the VLAN Name when importing prefixes. This also adds the VLAN Display Name to the prefixes view. I've changed the size on the site and role choice fields in in order to save some space because most people will have more VLANs than sites or roles.

The import will throw an error if you have a VLAN with the same VID and name. Its not super descriptive but if anyone has suggestions on how to pull the VLAN Name out of the exception information please let me know.

Screenshot of above described error and other changes to the page's examples
screen shot 2016-07-06 at 2 12 46 am

Screenshot of new view on Prefixes page
screen shot 2016-07-06 at 2 13 35 am

Gelob added 2 commits July 6, 2016 02:59
catch MultipleObjectsReturned Error & Cleanup

Fix VLAN Filter on Prefixes page
@jeremystretch
Copy link
Member

Any reason you opted to use the VLAN name rather than the VID? Doesn't really make a difference but I'd figure the VID might be more convenient.

@jeremystretch jeremystretch added the status: accepted This issue has been accepted for implementation label Jul 6, 2016
@Gelob
Copy link
Contributor Author

Gelob commented Jul 6, 2016

No real reason, It was based off of the fact that in the prefixes view I would show the vlan display_name and I wanted both to be the same. I figured it was also more likely that you would have multiple VIDs in the same site with different names than you would have multiple VIDs with the same name.

@jeremystretch
Copy link
Member

jeremystretch commented Jul 7, 2016

Wondering if we should hold off on this until #111 is implemented. We might be able to squeeze both into v1.2.

@Gelob
Copy link
Contributor Author

Gelob commented Jul 8, 2016

I think its worthwhile to wait and then modify as needed. One thing I constantly found out when researching how to catch the MultipleObjectsReturned exception is that we should try avoid having any duplicate data in backend if possible. If we don't its going to become more of an issue

@jeremystretch jeremystretch added pr/hold and removed status: accepted This issue has been accepted for implementation labels Jul 8, 2016
@jeremystretch
Copy link
Member

jeremystretch commented Jul 12, 2016

FYI #42 and #111 have been tagged for v1.3.

@jeremystretch
Copy link
Member

Ended up implementing VLAN assignment for prefix import as part of v1.3.0.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants