From 8d4001ef332f703e32c60e944c6fb8fc9cb1fdec Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 7 Dec 2023 13:14:01 -0800 Subject: [PATCH 1/5] 14467 change ChoiceField separator from comma to colon --- netbox/extras/forms/model_forms.py | 6 +++--- netbox/utilities/forms/widgets/misc.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/netbox/extras/forms/model_forms.py b/netbox/extras/forms/model_forms.py index 83a3464201d..a3f88f43e86 100644 --- a/netbox/extras/forms/model_forms.py +++ b/netbox/extras/forms/model_forms.py @@ -95,8 +95,8 @@ class CustomFieldChoiceSetForm(BootstrapMixin, forms.ModelForm): required=False, help_text=mark_safe(_( 'Enter one choice per line. An optional label may be specified for each choice by appending it with a ' - 'comma. Example:' - ) + ' choice1,First Choice') + 'colon. Example:' + ) + ' choice1:First Choice') ) class Meta: @@ -107,7 +107,7 @@ def clean_extra_choices(self): data = [] for line in self.cleaned_data['extra_choices'].splitlines(): try: - value, label = line.split(',', maxsplit=1) + value, label = line.split(':', maxsplit=1) except ValueError: value, label = line, line data.append((value, label)) diff --git a/netbox/utilities/forms/widgets/misc.py b/netbox/utilities/forms/widgets/misc.py index 307031bd882..158b0e67e9a 100644 --- a/netbox/utilities/forms/widgets/misc.py +++ b/netbox/utilities/forms/widgets/misc.py @@ -65,5 +65,5 @@ def format_value(self, value): if not value: return None if type(value) is list: - return '\n'.join([f'{k},{v}' for k, v in value]) + return '\n'.join([f'{k}:{v}' for k, v in value]) return value From 010721ed073187bfc536a770716682d7f70dcb0b Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 7 Dec 2023 13:42:11 -0800 Subject: [PATCH 2/5] 14467 fix test --- netbox/extras/tests/test_views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/netbox/extras/tests/test_views.py b/netbox/extras/tests/test_views.py index e034abff53b..944ae865c53 100644 --- a/netbox/extras/tests/test_views.py +++ b/netbox/extras/tests/test_views.py @@ -76,6 +76,7 @@ def setUpTestData(cls): class CustomFieldChoiceSetTestCase(ViewTestCases.PrimaryObjectViewTestCase): model = CustomFieldChoiceSet + validation_excluded_fields = ('extra_choices',) @classmethod def setUpTestData(cls): @@ -98,7 +99,7 @@ def setUpTestData(cls): cls.form_data = { 'name': 'Choice Set X', - 'extra_choices': '\n'.join(['X1,Choice 1', 'X2,Choice 2', 'X3,Choice 3']) + 'extra_choices': '\n'.join(['X1:Choice 1', 'X2:Choice 2', 'X3:Choice 3']) } cls.csv_data = ( From df82f782be445f28cdbf2d2569bb1e00013467d5 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 7 Dec 2023 14:26:55 -0800 Subject: [PATCH 3/5] 14467 fix test --- netbox/extras/tests/test_views.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/netbox/extras/tests/test_views.py b/netbox/extras/tests/test_views.py index 944ae865c53..f7bde889ec3 100644 --- a/netbox/extras/tests/test_views.py +++ b/netbox/extras/tests/test_views.py @@ -120,6 +120,31 @@ def setUpTestData(cls): 'description': 'New description', } + # These are here as extra_choices field splits on colon, but is returned + # from DB as comma separated. So we exclude them in validation_excluded_fields + # but test they are actually set correctly here. + def _check_extra_choices_data(self): + instance = self._get_queryset().last() + form_data = self.form_data + form_data['extra_choices'] = form_data['extra_choices'].replace(':', ',') + self.assertInstanceEqual(instance, form_data) + + def test_create_object_with_constrained_permission(self): + super().test_create_object_with_constrained_permission() + self._check_extra_choices_data() + + def test_create_object_with_permission(self): + super().test_create_object_with_permission() + self._check_extra_choices_data() + + def test_edit_object_with_constrained_permission(self): + super().test_edit_object_with_constrained_permission() + self._check_extra_choices_data() + + def test_edit_object_with_permission(self): + super().test_edit_object_with_permission() + self._check_extra_choices_data() + class CustomLinkTestCase(ViewTestCases.PrimaryObjectViewTestCase): model = CustomLink From 855c32a635b83fee9aaece0006ddb630cee3777d Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 11 Dec 2023 10:59:47 -0800 Subject: [PATCH 4/5] 14467 use regex for colon detection --- netbox/extras/forms/model_forms.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/netbox/extras/forms/model_forms.py b/netbox/extras/forms/model_forms.py index a3f88f43e86..4e4a6e0de88 100644 --- a/netbox/extras/forms/model_forms.py +++ b/netbox/extras/forms/model_forms.py @@ -1,4 +1,5 @@ import json +import re from django import forms from django.conf import settings @@ -107,7 +108,7 @@ def clean_extra_choices(self): data = [] for line in self.cleaned_data['extra_choices'].splitlines(): try: - value, label = line.split(':', maxsplit=1) + value, label = re.split(r'(? Date: Mon, 11 Dec 2023 11:10:41 -0800 Subject: [PATCH 5/5] 14467 update tests --- netbox/extras/tests/test_views.py | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/netbox/extras/tests/test_views.py b/netbox/extras/tests/test_views.py index f7bde889ec3..49725d3fb82 100644 --- a/netbox/extras/tests/test_views.py +++ b/netbox/extras/tests/test_views.py @@ -76,7 +76,6 @@ def setUpTestData(cls): class CustomFieldChoiceSetTestCase(ViewTestCases.PrimaryObjectViewTestCase): model = CustomFieldChoiceSet - validation_excluded_fields = ('extra_choices',) @classmethod def setUpTestData(cls): @@ -120,30 +119,12 @@ def setUpTestData(cls): 'description': 'New description', } - # These are here as extra_choices field splits on colon, but is returned - # from DB as comma separated. So we exclude them in validation_excluded_fields - # but test they are actually set correctly here. - def _check_extra_choices_data(self): - instance = self._get_queryset().last() - form_data = self.form_data - form_data['extra_choices'] = form_data['extra_choices'].replace(':', ',') - self.assertInstanceEqual(instance, form_data) - - def test_create_object_with_constrained_permission(self): - super().test_create_object_with_constrained_permission() - self._check_extra_choices_data() - - def test_create_object_with_permission(self): - super().test_create_object_with_permission() - self._check_extra_choices_data() - - def test_edit_object_with_constrained_permission(self): - super().test_edit_object_with_constrained_permission() - self._check_extra_choices_data() - - def test_edit_object_with_permission(self): - super().test_edit_object_with_permission() - self._check_extra_choices_data() + # This is here as extra_choices field splits on colon, but is returned + # from DB as comma separated. + def assertInstanceEqual(self, instance, data, exclude=None, api=False): + if 'extra_choices' in data: + data['extra_choices'] = data['extra_choices'].replace(':', ',') + return super().assertInstanceEqual(instance, data, exclude, api) class CustomLinkTestCase(ViewTestCases.PrimaryObjectViewTestCase):