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

Closes: #15239 - Allow adding/removing tagged VLANs in bulk editing of Interfaces #17524

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Closes: #15239 - Allow adding/removing tagged VLANs in bulk editing of Interfaces #17524

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e66695b
Allow adding/removing tagged VLANs in bulk editing of Interfaces
bctiemann Sep 17, 2024
c4aac4e
Move vlan/interface-specific field operations to an overrideable method
bctiemann Sep 19, 2024
6fa295b
Ensure interfaces are MODE_TAGGED before adding/removing tagged vlans
bctiemann Sep 23, 2024
8fe1b74
Add docstring for generic extra_object_field_operations
bctiemann Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 38 additions & 16 deletions netbox/dcim/forms/bulk_edit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1404,18 +1404,25 @@ class InterfaceBulkEditForm(
parent = DynamicModelChoiceField(
label=_('Parent'),
queryset=Interface.objects.all(),
required=False
required=False,
query_params={
'virtual_chassis_member_id': '$device',
}
)
bridge = DynamicModelChoiceField(
label=_('Bridge'),
queryset=Interface.objects.all(),
required=False
required=False,
query_params={
'virtual_chassis_member_id': '$device',
}
)
lag = DynamicModelChoiceField(
queryset=Interface.objects.all(),
required=False,
query_params={
'type': 'lag',
'virtual_chassis_member_id': '$device',
},
label=_('LAG')
)
Expand Down Expand Up @@ -1472,6 +1479,7 @@ class InterfaceBulkEditForm(
required=False,
query_params={
'group_id': '$vlan_group',
'available_on_device': '$device',
},
label=_('Untagged VLAN')
)
Expand All @@ -1480,9 +1488,28 @@ class InterfaceBulkEditForm(
required=False,
query_params={
'group_id': '$vlan_group',
'available_on_device': '$device',
},
label=_('Tagged VLANs')
)
add_tagged_vlans = DynamicModelMultipleChoiceField(
label=_('Add tagged VLANs'),
queryset=VLAN.objects.all(),
required=False,
query_params={
'group_id': '$vlan_group',
'available_on_device': '$device',
},
)
remove_tagged_vlans = DynamicModelMultipleChoiceField(
label=_('Remove tagged VLANs'),
queryset=VLAN.objects.all(),
required=False,
query_params={
'group_id': '$vlan_group',
'available_on_device': '$device',
}
)
vrf = DynamicModelChoiceField(
queryset=VRF.objects.all(),
required=False,
Expand All @@ -1509,7 +1536,7 @@ class InterfaceBulkEditForm(
FieldSet('vdcs', 'mtu', 'tx_power', 'enabled', 'mgmt_only', 'mark_connected', name=_('Operation')),
FieldSet('poe_mode', 'poe_type', name=_('PoE')),
FieldSet('parent', 'bridge', 'lag', name=_('Related Interfaces')),
FieldSet('mode', 'vlan_group', 'untagged_vlan', 'tagged_vlans', name=_('802.1Q Switching')),
FieldSet('mode', 'vlan_group', 'untagged_vlan', 'tagged_vlans', 'add_tagged_vlans', 'remove_tagged_vlans', name=_('802.1Q Switching')),
FieldSet(
'rf_role', 'rf_channel', 'rf_channel_frequency', 'rf_channel_width', 'wireless_lan_group', 'wireless_lans',
name=_('Wireless')
Expand All @@ -1523,19 +1550,7 @@ class InterfaceBulkEditForm(

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if self.device_id:
device = Device.objects.filter(pk=self.device_id).first()

# Restrict parent/bridge/LAG interface assignment by device
self.fields['parent'].widget.add_query_param('virtual_chassis_member_id', device.pk)
self.fields['bridge'].widget.add_query_param('virtual_chassis_member_id', device.pk)
self.fields['lag'].widget.add_query_param('virtual_chassis_member_id', device.pk)

# Limit VLAN choices by device
self.fields['untagged_vlan'].widget.add_query_param('available_on_device', device.pk)
self.fields['tagged_vlans'].widget.add_query_param('available_on_device', device.pk)

else:
if not self.device_id:
# See #4523
if 'pk' in self.initial:
site = None
Expand All @@ -1559,6 +1574,13 @@ def __init__(self, *args, **kwargs):
'site_id', [site.pk, settings.FILTERS_NULL_CHOICE_VALUE]
)

self.fields['add_tagged_vlans'].widget.add_query_param(
'site_id', [site.pk, settings.FILTERS_NULL_CHOICE_VALUE]
)
self.fields['remove_tagged_vlans'].widget.add_query_param(
'site_id', [site.pk, settings.FILTERS_NULL_CHOICE_VALUE]
)

self.fields['parent'].choices = ()
self.fields['parent'].widget.attrs['disabled'] = True
self.fields['bridge'].choices = ()
Expand Down
7 changes: 7 additions & 0 deletions netbox/dcim/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2616,6 +2616,13 @@ class InterfaceBulkEditView(generic.BulkEditView):
table = tables.InterfaceTable
form = forms.InterfaceBulkEditForm

def extra_object_field_operations(self, form, obj):
# Add/remove tagged VLANs
if form.cleaned_data.get('add_tagged_vlans', None):
obj.tagged_vlans.add(*form.cleaned_data['add_tagged_vlans'])
if form.cleaned_data.get('remove_tagged_vlans', None):
obj.tagged_vlans.remove(*form.cleaned_data['remove_tagged_vlans'])

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to validate the interface mode for these interfaces when bulk editing, to prevent #15924 and similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DanSheps I would think this change is all we need, right? We don't need to throw an error if the mode is wrong, we can just silently skip interfaces that aren't in the right mode?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think for this it is a valid approach.


class InterfaceBulkRenameView(generic.BulkRenameView):
queryset = Interface.objects.all()
Expand Down
5 changes: 5 additions & 0 deletions netbox/netbox/views/generic/bulk_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,9 @@ class BulkEditView(GetReturnURLMixin, BaseMultiObjectView):
def get_required_permission(self):
return get_permission_for_model(self.queryset.model, 'change')

def extra_object_field_operations(self, form, obj):
bctiemann marked this conversation as resolved.
Show resolved Hide resolved
pass

def _update_objects(self, form, request):
custom_fields = getattr(form, 'custom_fields', {})
standard_fields = [
Expand Down Expand Up @@ -615,6 +618,8 @@ def _update_objects(self, form, request):
if form.cleaned_data.get('remove_tags', None):
obj.tags.remove(*form.cleaned_data['remove_tags'])

self.extra_object_field_operations(form, obj)

# Rebuild the tree for MPTT models
if issubclass(self.queryset.model, MPTTModel):
self.queryset.model.objects.rebuild()
Expand Down