Skip to content

Commit 8654653

Browse files
committed
refactor(forms): Simplify fieldset and template usage
Reorganizes form fieldsets to introduce TabbedGroups for better clarity and structure. Removes redundant template files by leveraging default NetBox rendering behavior, streamlining the codebase and reducing maintenance overhead. Fixes #239
1 parent 404ad38 commit 8654653

File tree

4 files changed

+104
-197
lines changed

4 files changed

+104
-197
lines changed

netbox_acls/forms/models.py

Lines changed: 103 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
"""
22
Defines each django model's GUI form to add or edit objects for each django model.
33
"""
4-
from django.utils.translation import gettext_lazy as _
4+
55
from dcim.models import Device, Interface, Region, Site, SiteGroup, VirtualChassis
66
from django.contrib.contenttypes.models import ContentType
77
from django.core.exceptions import ValidationError
88
from django.utils.safestring import mark_safe
9+
from django.utils.translation import gettext_lazy as _
910
from ipam.models import Prefix
1011
from netbox.forms import NetBoxModelForm
11-
from utilities.forms.rendering import FieldSet
1212
from utilities.forms.fields import CommentField, DynamicModelChoiceField
13+
from utilities.forms.rendering import FieldSet, TabbedGroups
1314
from virtualization.models import (
1415
Cluster,
1516
ClusterGroup,
@@ -117,14 +118,41 @@ class AccessListForm(NetBoxModelForm):
117118
"cluster_group_id": "$cluster_group",
118119
},
119120
)
120-
121121
comments = CommentField()
122+
122123
fieldsets = (
123-
FieldSet('region', 'site_group', 'site', 'virtual_machine', 'virtual_chassis', 'device', name=_('Assignment')),
124-
FieldSet('name', 'type', 'default_action', name=_('Access List')),
125-
FieldSet('comments', 'tags', name=_('')),
124+
FieldSet(
125+
"name",
126+
"type",
127+
"default_action",
128+
"tags",
129+
name=_("Access List Details"),
130+
),
131+
FieldSet(
132+
TabbedGroups(
133+
FieldSet(
134+
"region",
135+
"site_group",
136+
"site",
137+
"device",
138+
name=_("Device"),
139+
),
140+
FieldSet(
141+
"virtual_chassis",
142+
name=_("Virtual Chassis"),
143+
),
144+
FieldSet(
145+
"cluster_type",
146+
"cluster_group",
147+
"cluster",
148+
"virtual_machine",
149+
name=_("Virtual Machine"),
150+
),
151+
),
152+
name=_("Host Assignment"),
153+
),
126154
)
127-
155+
128156
class Meta:
129157
model = AccessList
130158
fields = (
@@ -145,7 +173,7 @@ class Meta:
145173
"default_action": "The default behavior of the ACL.",
146174
"name": "The name uniqueness per device is case insensitive.",
147175
"type": mark_safe(
148-
"<b>*Note:</b> CANNOT be changed if ACL Rules are assoicated to this Access List.",
176+
"<b>*Note:</b> CANNOT be changed if ACL Rules are associated to this Access List.",
149177
),
150178
}
151179

@@ -200,14 +228,22 @@ def clean(self):
200228
# Check if more than one host type selected.
201229
if (device and virtual_chassis) or (device and virtual_machine) or (virtual_chassis and virtual_machine):
202230
raise ValidationError(
203-
{"__all__": "Access Lists must be assigned to one host at a time. Either a device, virtual chassis or virtual machine."},
231+
{
232+
"__all__": (
233+
"Access Lists must be assigned to one host at a time. Either a device, virtual chassis or "
234+
"virtual machine."
235+
)
236+
},
204237
)
205238

206239
# Check if no hosts selected.
207240
if not device and not virtual_chassis and not virtual_machine:
208-
raise ValidationError({"__all__": "Access Lists must be assigned to a device, virtual chassis or virtual machine."})
241+
raise ValidationError(
242+
{"__all__": "Access Lists must be assigned to a device, virtual chassis or virtual machine."}
243+
)
209244

210245
existing_acls = None
246+
host_type = None
211247
if device:
212248
host_type = "device"
213249
existing_acls = AccessList.objects.filter(name=name, device=device).exists()
@@ -233,7 +269,9 @@ def clean(self):
233269
def save(self, *args, **kwargs):
234270
# Set assigned object
235271
self.instance.assigned_object = (
236-
self.cleaned_data.get("device") or self.cleaned_data.get("virtual_chassis") or self.cleaned_data.get("virtual_machine")
272+
self.cleaned_data.get("device")
273+
or self.cleaned_data.get("virtual_chassis")
274+
or self.cleaned_data.get("virtual_machine")
237275
)
238276

239277
return super().save(*args, **kwargs)
@@ -292,10 +330,29 @@ class ACLInterfaceAssignmentForm(NetBoxModelForm):
292330
),
293331
)
294332
comments = CommentField()
333+
295334
fieldsets = (
296-
FieldSet('device', 'interface', 'virtual_machine', 'vminterface', name=_('Assignment')),
297-
FieldSet('access_list', 'direction', name=_('Access List Details')),
298-
FieldSet('comments', 'tags', name=_('')),
335+
FieldSet(
336+
"access_list",
337+
"direction",
338+
"tags",
339+
name=_("Access List Details"),
340+
),
341+
FieldSet(
342+
TabbedGroups(
343+
FieldSet(
344+
"device",
345+
"interface",
346+
name=_("Device"),
347+
),
348+
FieldSet(
349+
"virtual_machine",
350+
"vminterface",
351+
name=_("Virtual Machine"),
352+
),
353+
),
354+
name=_("Interface Assignment"),
355+
),
299356
)
300357

301358
def __init__(self, *args, **kwargs):
@@ -453,9 +510,21 @@ class ACLStandardRuleForm(NetBoxModelForm):
453510
)
454511

455512
fieldsets = (
456-
FieldSet("access_list", "description", "tags", name=_('Access List Details')),
457-
FieldSet("index", "action", "remark", "source_prefix", name=_('Rule Definition'))
513+
FieldSet(
514+
"access_list",
515+
"description",
516+
"tags",
517+
name=_("Access List Details"),
518+
),
519+
FieldSet(
520+
"index",
521+
"action",
522+
"remark",
523+
"source_prefix",
524+
name=_("Rule Definition"),
525+
),
458526
)
527+
459528
class Meta:
460529
model = ACLStandardRule
461530
fields = (
@@ -468,9 +537,6 @@ class Meta:
468537
"description",
469538
)
470539

471-
472-
473-
474540
help_texts = {
475541
"index": help_text_acl_rule_index,
476542
"action": help_text_acl_action,
@@ -540,9 +606,25 @@ class ACLExtendedRuleForm(NetBoxModelForm):
540606
label="Destination Prefix",
541607
)
542608
fieldsets = (
543-
FieldSet("access_list", "description", "tags", name=_('Access List Details')),
544-
FieldSet("index", "action", "remark", "source_prefix", "source_ports", "destination_prefix", "destination_ports", "protocol", name=_('Rule Definition'))
609+
FieldSet(
610+
"access_list",
611+
"description",
612+
"tags",
613+
name=_("Access List Details"),
614+
),
615+
FieldSet(
616+
"index",
617+
"action",
618+
"remark",
619+
"source_prefix",
620+
"source_ports",
621+
"destination_prefix",
622+
"destination_ports",
623+
"protocol",
624+
name=_("Rule Definition"),
625+
),
545626
)
627+
546628
class Meta:
547629
model = ACLExtendedRule
548630
fields = (

netbox_acls/templates/netbox_acls/accesslist_edit.html

Lines changed: 0 additions & 97 deletions
This file was deleted.

netbox_acls/templates/netbox_acls/aclinterfaceassignment_edit.html

Lines changed: 0 additions & 76 deletions
This file was deleted.

0 commit comments

Comments
 (0)