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.

Sign up for GitHub

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

Jump to bottom

Disable "Add termination" button for P2P L2VPNs which already have 2 terminations #10033

Closed
candlerb opened this issue Aug 16, 2022 · 1 comment · Fixed by #10115
Closed

Disable "Add termination" button for P2P L2VPNs which already have 2 terminations #10033

candlerb opened this issue Aug 16, 2022 · 1 comment · Fixed by #10115
Assignees
@arthanson
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@candlerb
Copy link
Contributor

NetBox version

v3.3-beta2

Feature type

Change to existing functionality

Proposed functionality

Netbox enforces at model level that a P2P L2VPN can only have two terminations.

However in the GUI, if you are viewing a P2P L2VPN e.g. /ipam/l2vpns/1/, and it already has 2 terminations, then clicking "Add Termination" still takes you to the Add Termination page, allowing you to fill in termination information. It's only when you try to save it that you're told you can't have more than 2 terminations.

I propose that either you should get an error immediately on clicking the Add Termination button, or that the button should be greyed out (with a tooltip saying that you cannot have more than 2 terminations on a P2P L2VPN)

Use case

User experience: not prompting the user to enter data which you know in advance will be rejected.

Database changes

None

External dependencies

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

Ref discussion: #9727 (reply in thread)

@jeremystretch jeremystretch added beta Concerns a bug/feature in a beta release and removed beta Concerns a bug/feature in a beta release labels Aug 17, 2022
@DanSheps DanSheps added the status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation label Aug 17, 2022
@arthanson arthanson self-assigned this Aug 23, 2022
arthanson added a commit to arthanson/netbox that referenced this issue Aug 23, 2022
@arthanson
netbox-community#10033 disable Add a Termination button if 2 terminat…
374abe5 
…ions on L2VPN P2P
@arthanson arthanson mentioned this issue Aug 23, 2022
Merged
arthanson added a commit to arthanson/netbox that referenced this issue Aug 23, 2022
@arthanson
netbox-community#10033 changes from code review
439cf1a
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Aug 24, 2022
jeremystretch added a commit that referenced this issue Aug 24, 2022
@jeremystretch
Merge pull request #10115 from arthanson/art-10033
6a7c56d 
#10033 disable Add a Termination button if 2 terminations on L2VPN P2P
jeremystretch added a commit that referenced this issue Aug 24, 2022
@jeremystretch
Changelog for #10033, #10037, #10094
18d5576
@jeremystretch jeremystretch mentioned this issue Aug 25, 2022
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@arthanson arthanson

Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

#10033 disable Add a Termination button if 2 terminations on L2VPN P2P arthanson/netbox
4 participants
@candlerb @arthanson @DanSheps @jeremystretch