Repository: netbox-community/netbox
Pull request: #10033 disable Add a Termination button if 2 terminations on L2VPN P2P
<!--
    Thank you for your interest in contributing to NetBox! Please note
    that our contribution policy requires that a feature request or bug
    report be opened for approval prior to filing a pull request. This
    helps avoid wasting time and effort on something that we might not
    be able to accept.

    Please indicate the relevant feature request or bug report below.
    IF YOUR PULL REQUEST DOES NOT REFERENCE AN ACCEPTED BUG REPORT OR
    FEATURE REQUEST, IT WILL BE MARKED AS INVALID AND CLOSED.
-->
### Fixes: #10033 
<!--
    Please include a summary of the proposed changes below.
-->
Disables the "Add a Termination" button if there are already 2 terminations for an L2VPN P2P

Problem statement:
Disable "Add termination" button for P2P L2VPNs which already have 2 terminations
### 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

Hints from issue discussion:
Ref discussion: https://github.com/netbox-community/netbox/discussions/9727#discussioncomment-3409332

Commit message: #10033 disable Add a Termination button if 2 terminations on L2VPN P2P

Patch to review:
diff --git a/netbox/ipam/models/l2vpn.py b/netbox/ipam/models/l2vpn.py
index 0e948b18ec9..db6f47924be 100644
--- a/netbox/ipam/models/l2vpn.py
+++ b/netbox/ipam/models/l2vpn.py
@@ -67,6 +67,12 @@ def __str__(self):
     def get_absolute_url(self):
         return reverse('ipam:l2vpn', args=[self.pk])
 
+    def can_add_termination(self):
+        if self.type in L2VPNTypeChoices.P2P and self.terminations.count() >= 2:
+            return False
+        else:
+            return True
+
 
 class L2VPNTermination(NetBoxModel):
     l2vpn = models.ForeignKey(
diff --git a/netbox/templates/ipam/l2vpn.html b/netbox/templates/ipam/l2vpn.html
index 44a1da81861..32013400b3c 100644
--- a/netbox/templates/ipam/l2vpn.html
+++ b/netbox/templates/ipam/l2vpn.html
@@ -59,7 +59,7 @@ <h5 class="card-header">Terminations</h5>
       </div>
       {% if perms.ipam.add_l2vpntermination %}
         <div class="card-footer text-end noprint">
-          <a href="{% url 'ipam:l2vpntermination_add' %}?l2vpn={{ object.pk }}&return_url={{ object.get_absolute_url }}" class="btn btn-primary btn-sm">
+          <a href="{% url 'ipam:l2vpntermination_add' %}?l2vpn={{ object.pk }}&return_url={{ object.get_absolute_url }}" class="btn btn-primary btn-sm{% if not object.can_add_termination %} disabled{% endif %}"{% if not object.can_add_termination %} aria-disabled="true"{% endif %}>
             <i class="mdi mdi-plus-thick" aria-hidden="true"></i> Add a Termination
           </a>
         </div>


Code review comments:
diff_hunk: @@ -59,7 +59,7 @@ <h5 class="card-header">Terminations</h5>
       </div>
       {% if perms.ipam.add_l2vpntermination %}
         <div class="card-footer text-end noprint">
-          <a href="{% url 'ipam:l2vpntermination_add' %}?l2vpn={{ object.pk }}&return_url={{ object.get_absolute_url }}" class="btn btn-primary btn-sm">
+          <a href="{% url 'ipam:l2vpntermination_add' %}?l2vpn={{ object.pk }}&return_url={{ object.get_absolute_url }}" class="btn btn-primary btn-sm{% if not object.can_add_termination %} disabled{% endif %}"{% if not object.can_add_termination %} aria-disabled="true"{% endif %}>
path: netbox/templates/ipam/l2vpn.html
text: @user1:
Can we simplify this a bit to avoid repeating the `{% if %}` block? I know the HTML is a little ugly but it eliminates the second evaluation. Alternatively, we could repeat the entire button element for better readability (once enable and once disabled).

```suggestion
          <a href="{% url 'ipam:l2vpntermination_add' %}?l2vpn={{ object.pk }}&return_url={{ object.get_absolute_url }}" class="btn btn-primary btn-sm{% if not object.can_add_termination %} disabled" aria-disabled="true{% endif %}">
```