Bulk edit of models with M2M fields can clear the M2M field

[Urth]

NetBox version

v3.5.4 and tested on develop

Python version

3.10

Steps to Reproduce

1. Create an interface with one or more tagged_vlans.
2. Use bulk edit on the interface to change something unrelated like the speed.
3. Observe the changelog with the tagged_vlans being removed.

Expected Behavior

The untagged_vlans should remain unchanged if none are selected and set null is unchecked.

Observed Behavior

The untagged_vlans are removed from the interface.

Caused by https://github.com/netbox-community/netbox/blob/develop/netbox/netbox/views/generic/bulk_views.py#L554-L555
In 41c9248 the truthy check on the m2m queryset was removed. Django querysets evaluate to False if the queryset is empty.

[netopsab]

Hello,

I'm facing this issue too... Also and for clarification, you mention the untagged_vlan field (excepted / observed behavior section). However, as you mentioned, this issue occurs with m2m fields, so with tagged_vlans (not untagged_vlan).

My proposal to fix this issue :

--- a/netbox/netbox/views/generic/bulk_views.py
+++ b/netbox/netbox/views/generic/bulk_views.py
@@ -551,7 +551,7 @@ class BulkEditView(GetReturnURLMixin, BaseMultiObjectView):
             for name, m2m_field in m2m_fields.items():
                 if name in form.nullable_fields and name in nullified_fields:
                     getattr(obj, name).clear()
-                else:
+                elif name in form.changed_data:
                     getattr(obj, name).set(form.cleaned_data[name])

[abhi1693]

@netopsab Would you like to submit a pr?

[netopsab]

why not ... :)

[Urth]

The change proposed by @netopsab does not fix the problem. The bulk edit form always includes m2m fields in the cleaned_data. Otherwise the old code would trigger a KeyError. The problem is that the cleaned_data is an empty queryset when nothing is selected. My PR checks if the queryset is non-empty like the code before 41c9248 was applied.

[DanSheps]

The change proposed by @netopsab does not fix the problem. The bulk edit form always includes m2m fields in the cleaned_data. Otherwise the old code would trigger a KeyError. The problem is that the cleaned_data is an empty queryset when nothing is selected. My PR checks if the queryset is non-empty like the code before 41c9248 was applied.

@netopsab is checking against changed_data not cleaned_data.

His method is consistent with our rchecking in other areas. Both approaches are equally valid.