Merge branch 'main' into feature

netbox/extras/events.py

@@ -119,7 +119,9 @@ def process_event_rules(event_rules, object_type, event_type, data, username=Non
             if snapshots:
                 params["snapshots"] = snapshots
             if request:
-                params["request"] = copy_safe_request(request)
+                # Exclude FILES - webhooks don't need uploaded files,
+                # which can cause pickle errors with Pillow.
+                params["request"] = copy_safe_request(request, include_files=False)
 
             # Enqueue the task
             rq_queue.enqueue(

netbox/extras/forms/model_forms.py

@@ -189,22 +189,22 @@ class CustomFieldChoiceSetForm(ChangelogMessageMixin, OwnerMixin, forms.ModelFor
         # if standardize these, we can simplify this code
 
         # Convert extra_choices Array Field from model to CharField for form
-        if 'extra_choices' in self.initial and self.initial['extra_choices']:
-            extra_choices = self.initial['extra_choices']
+        if extra_choices := self.initial.get('extra_choices', None):
             if isinstance(extra_choices, str):
                 extra_choices = [extra_choices]
-            choices = ""
+            choices = []
             for choice in extra_choices:
                 # Setup choices in Add Another use case
                 if isinstance(choice, str):
                     choice_str = ":".join(choice.replace("'", "").replace(" ", "")[1:-1].split(","))
-                    choices += choice_str + "\n"
+                    choices.append(choice_str)
                 # Setup choices in Edit use case
                 elif isinstance(choice, list):
-                    choice_str = ":".join(choice)
-                    choices += choice_str + "\n"
+                    value = choice[0].replace(':', '\\:')
+                    label = choice[1].replace(':', '\\:')
+                    choices.append(f'{value}:{label}')
 
-            self.initial['extra_choices'] = choices
+            self.initial['extra_choices'] = '\n'.join(choices)
 
     def clean_extra_choices(self):
         data = []

netbox/extras/models/customfields.py

@@ -450,7 +450,14 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
             return model.objects.filter(pk__in=value)
         return value
 
-    def to_form_field(self, set_initial=True, enforce_required=True, enforce_visibility=True, for_csv_import=False):
+    def to_form_field(
+        self,
+        set_initial=True,
+        enforce_required=True,
+        enforce_visibility=True,
+        for_csv_import=False,
+        for_filterset_form=False,
+    ):
         """
         Return a form field suitable for setting a CustomField's value for an object.
 
@@ -458,6 +465,7 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
         enforce_required: Honor the value of CustomField.required. Set to False for filtering/bulk editing.
         enforce_visibility: Honor the value of CustomField.ui_visible. Set to False for filtering.
         for_csv_import: Return a form field suitable for bulk import of objects in CSV format.
+        for_filterset_form: Return a form field suitable for use in a FilterSet form.
         """
         initial = self.default if set_initial else None
         required = self.required if enforce_required else False
@@ -520,7 +528,7 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
                     field_class = CSVMultipleChoiceField
                 field = field_class(choices=choices, required=required, initial=initial)
             else:
-                if self.type == CustomFieldTypeChoices.TYPE_SELECT:
+                if self.type == CustomFieldTypeChoices.TYPE_SELECT and not for_filterset_form:
                     field_class = DynamicChoiceField
                     widget_class = APISelect
                 else:
@@ -871,6 +879,16 @@ class CustomFieldChoiceSet(CloningMixin, ExportTemplatesMixin, OwnerMixin, Chang
         if not self.base_choices and not self.extra_choices:
             raise ValidationError(_("Must define base or extra choices."))
 
+        # Check for duplicate values in extra_choices
+        choice_values = [c[0] for c in self.extra_choices] if self.extra_choices else []
+        if len(set(choice_values)) != len(choice_values):
+            # At least one duplicate value is present. Find the first one and raise an error.
+            _seen = []
+            for value in choice_values:
+                if value in _seen:
+                    raise ValidationError(_("Duplicate value '{value}' found in extra choices.").format(value=value))
+                _seen.append(value)
+
         # Check whether any choices have been removed. If so, check whether any of the removed
         # choices are still set in custom field data for any object.
         original_choices = set([

netbox/extras/tests/test_customfields.py

@@ -1506,19 +1506,18 @@ class CustomFieldModelTest(TestCase):
 
     def test_invalid_data(self):
         """
-        Setting custom field data for a non-applicable (or non-existent) CustomField should raise a ValidationError.
+        Any invalid or stale custom field data should be removed from the instance.
         """
         site = Site(name='Test Site', slug='test-site')
 
         # Set custom field data
         site.custom_field_data['foo'] = 'abc'
         site.custom_field_data['bar'] = 'def'
-        with self.assertRaises(ValidationError):
-            site.clean()
-
-        del site.custom_field_data['bar']
         site.clean()
 
+        self.assertIn('foo', site.custom_field_data)
+        self.assertNotIn('bar', site.custom_field_data)
+
     def test_missing_required_field(self):
         """
         Check that a ValidationError is raised if any required custom fields are not present.

netbox/extras/tests/test_forms.py

@@ -5,6 +5,7 @@ from dcim.forms import SiteForm
 from dcim.models import Site
 from extras.choices import CustomFieldTypeChoices
 from extras.forms import SavedFilterForm
+from extras.forms.model_forms import CustomFieldChoiceSetForm
 from extras.models import CustomField, CustomFieldChoiceSet
 
 
@@ -90,6 +91,31 @@ class CustomFieldModelFormTest(TestCase):
             self.assertIsNone(instance.custom_field_data[field_type])
 
 
+class CustomFieldChoiceSetFormTest(TestCase):
+
+    def test_escaped_colons_preserved_on_edit(self):
+        choice_set = CustomFieldChoiceSet.objects.create(
+            name='Test Choice Set',
+            extra_choices=[['foo:bar', 'label'], ['value', 'label:with:colons']]
+        )
+
+        form = CustomFieldChoiceSetForm(instance=choice_set)
+        initial_choices = form.initial['extra_choices']
+
+        # colons are re-escaped
+        self.assertEqual(initial_choices, 'foo\\:bar:label\nvalue:label\\:with\\:colons')
+
+        form = CustomFieldChoiceSetForm(
+            {'name': choice_set.name, 'extra_choices': initial_choices},
+            instance=choice_set
+        )
+        self.assertTrue(form.is_valid())
+        updated = form.save()
+
+        # cleaned extra choices are correct, which does actually mean a list of tuples
+        self.assertEqual(updated.extra_choices, [('foo:bar', 'label'), ('value', 'label:with:colons')])
+
+
 class SavedFilterFormTest(TestCase):
 
     def test_basic_submit(self):