From 766b57670e892bd2a6747778c054b20e9ac7f37a Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 27 Jul 2023 13:07:38 -0400 Subject: [PATCH] Change extra_choices back to a nested ArrayField to preserve choice ordering --- netbox/extras/filtersets.py | 2 +- netbox/extras/forms/bulk_import.py | 5 ++++ netbox/extras/forms/model_forms.py | 15 ++++++++-- .../migrations/0096_customfieldchoiceset.py | 4 +-- netbox/extras/models/customfields.py | 20 ++++++++----- netbox/extras/tables/tables.py | 7 +++-- netbox/extras/tests/test_api.py | 24 ++++++++++++--- netbox/extras/tests/test_changelog.py | 4 +-- netbox/extras/tests/test_customfields.py | 30 +++++++++++-------- netbox/extras/tests/test_forms.py | 2 +- netbox/extras/tests/test_views.py | 26 +++++++++------- .../extras/customfieldchoiceset.html | 2 +- netbox/utilities/forms/widgets/misc.py | 4 ++- netbox/utilities/testing/base.py | 11 +++++-- 14 files changed, 105 insertions(+), 51 deletions(-) diff --git a/netbox/extras/filtersets.py b/netbox/extras/filtersets.py index 4458469f3..80fe2d253 100644 --- a/netbox/extras/filtersets.py +++ b/netbox/extras/filtersets.py @@ -128,7 +128,7 @@ class CustomFieldChoiceSetFilterSet(BaseFilterSet): def filter_by_choice(self, queryset, name, value): # TODO: Support case-insensitive matching - return queryset.filter(extra_choices__has_any_keys=value) + return queryset.filter(extra_choices__overlap=value) class CustomLinkFilterSet(BaseFilterSet): diff --git a/netbox/extras/forms/bulk_import.py b/netbox/extras/forms/bulk_import.py index 9f490ae73..e91ce7e93 100644 --- a/netbox/extras/forms/bulk_import.py +++ b/netbox/extras/forms/bulk_import.py @@ -68,6 +68,11 @@ class CustomFieldChoiceSetImportForm(CSVModelForm): required=False, help_text=_('The base set of predefined choices to use (if any)') ) + extra_choices = SimpleArrayField( + base_field=forms.CharField(), + required=False, + help_text=_('Comma-separated list of field choices') + ) class Meta: model = CustomFieldChoiceSet diff --git a/netbox/extras/forms/model_forms.py b/netbox/extras/forms/model_forms.py index d5b132110..35700967c 100644 --- a/netbox/extras/forms/model_forms.py +++ b/netbox/extras/forms/model_forms.py @@ -19,6 +19,7 @@ from utilities.forms.fields import ( CommentField, ContentTypeChoiceField, ContentTypeMultipleChoiceField, DynamicModelChoiceField, DynamicModelMultipleChoiceField, JSONField, SlugField, ) +from utilities.forms.widgets import ChoicesWidget from virtualization.models import Cluster, ClusterGroup, ClusterType @@ -84,14 +85,24 @@ class CustomFieldForm(BootstrapMixin, forms.ModelForm): class CustomFieldChoiceSetForm(BootstrapMixin, forms.ModelForm): - extra_choices = forms.JSONField( - required=False + extra_choices = forms.CharField( + widget=ChoicesWidget(), ) class Meta: model = CustomFieldChoiceSet fields = ('name', 'description', 'base_choices', 'extra_choices', 'order_alphabetically') + def clean_extra_choices(self): + data = [] + for line in self.cleaned_data['extra_choices'].splitlines(): + try: + value, label = line.split(',', maxsplit=1) + except ValueError: + value, label = line, line + data.append((value, label)) + return data + class CustomLinkForm(BootstrapMixin, forms.ModelForm): content_types = ContentTypeMultipleChoiceField( diff --git a/netbox/extras/migrations/0096_customfieldchoiceset.py b/netbox/extras/migrations/0096_customfieldchoiceset.py index a11a32438..1984e17f8 100644 --- a/netbox/extras/migrations/0096_customfieldchoiceset.py +++ b/netbox/extras/migrations/0096_customfieldchoiceset.py @@ -19,7 +19,7 @@ def create_choice_sets(apps, schema_editor): for cf in choice_fields: choiceset = CustomFieldChoiceSet.objects.create( name=f'{cf.name} Choices', - extra_choices=dict(zip(cf.choices, cf.choices)) # Convert list to key:val dict + extra_choices=tuple(zip(cf.choices, cf.choices)) # Convert list to tuple of two-tuples ) cf.choice_set = choiceset @@ -43,7 +43,7 @@ class Migration(migrations.Migration): ('name', models.CharField(max_length=100, unique=True)), ('description', models.CharField(blank=True, max_length=200)), ('base_choices', models.CharField(blank=True, max_length=50)), - ('extra_choices', models.JSONField(blank=True, default=dict, null=True)), + ('extra_choices', django.contrib.postgres.fields.ArrayField(base_field=django.contrib.postgres.fields.ArrayField(base_field=models.CharField(max_length=100), size=2), blank=True, null=True, size=None)), ('order_alphabetically', models.BooleanField(default=False)), ], options={ diff --git a/netbox/extras/models/customfields.py b/netbox/extras/models/customfields.py index 0ff218327..28bda6fbf 100644 --- a/netbox/extras/models/customfields.py +++ b/netbox/extras/models/customfields.py @@ -6,6 +6,7 @@ import django_filters from django import forms from django.conf import settings from django.contrib.contenttypes.models import ContentType +from django.contrib.postgres.fields import ArrayField from django.core.validators import RegexValidator, ValidationError from django.db import models from django.urls import reverse @@ -657,8 +658,11 @@ class CustomFieldChoiceSet(CloningMixin, ExportTemplatesMixin, ChangeLoggedModel blank=True, help_text=_('Base set of predefined choices (optional)') ) - extra_choices = models.JSONField( - default=dict, + extra_choices = ArrayField( + ArrayField( + base_field=models.CharField(max_length=100), + size=2 + ), blank=True, null=True ) @@ -684,14 +688,14 @@ class CustomFieldChoiceSet(CloningMixin, ExportTemplatesMixin, ChangeLoggedModel Returns a concatenation of the base and extra choices. """ if not hasattr(self, '_choices'): - self._choices = {} + self._choices = [] if self.base_choices: - self._choices.update(dict(CHOICE_SETS.get(self.base_choices))) + self._choices.extend(CHOICE_SETS.get(self.base_choices)) if self.extra_choices: - self._choices.update(self.extra_choices) + self._choices.extend(self.extra_choices) if self.order_alphabetically: - self._choices = dict(sorted(self._choices.items())) - return list(self._choices.items()) + self._choices = sorted(self._choices, key=lambda x: x[0]) + return self._choices @property def choices_count(self): @@ -705,6 +709,6 @@ class CustomFieldChoiceSet(CloningMixin, ExportTemplatesMixin, ChangeLoggedModel # Sort choices if alphabetical ordering is enforced if self.order_alphabetically: - self.extra_choices = dict(sorted(self.extra_choices.items())) + self.extra_choices = sorted(self.extra_choices, key=lambda x: x[0]) return super().save(*args, **kwargs) diff --git a/netbox/extras/tables/tables.py b/netbox/extras/tables/tables.py index ce69d7f57..0216e5e80 100644 --- a/netbox/extras/tables/tables.py +++ b/netbox/extras/tables/tables.py @@ -66,6 +66,9 @@ class CustomFieldTable(NetBoxTable): required = columns.BooleanColumn() ui_visibility = columns.ChoiceFieldColumn(verbose_name="UI visibility") description = columns.MarkdownColumn() + choice_set = tables.Column( + linkify=True + ) choices = columns.ChoicesColumn( max_items=10, orderable=False @@ -76,8 +79,8 @@ class CustomFieldTable(NetBoxTable): model = CustomField fields = ( 'pk', 'id', 'name', 'content_types', 'label', 'type', 'group_name', 'required', 'default', 'description', - 'search_weight', 'filter_logic', 'ui_visibility', 'is_cloneable', 'weight', 'choices', 'created', - 'last_updated', + 'search_weight', 'filter_logic', 'ui_visibility', 'is_cloneable', 'weight', 'choice_set', 'choices', + 'created', 'last_updated', ) default_columns = ('pk', 'name', 'content_types', 'label', 'group_name', 'type', 'required', 'description') diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index 922b45240..ad50d3562 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -139,15 +139,27 @@ class CustomFieldChoiceSetTest(APIViewTestCases.APIViewTestCase): create_data = [ { 'name': 'Choice Set 4', - 'extra_choices': ['4A', '4B', '4C'], + 'extra_choices': [ + ['4A', 'Choice 1'], + ['4B', 'Choice 2'], + ['4C', 'Choice 3'], + ], }, { 'name': 'Choice Set 5', - 'extra_choices': ['5A', '5B', '5C'], + 'extra_choices': [ + ['5A', 'Choice 1'], + ['5B', 'Choice 2'], + ['5C', 'Choice 3'], + ], }, { 'name': 'Choice Set 6', - 'extra_choices': ['6A', '6B', '6C'], + 'extra_choices': [ + ['6A', 'Choice 1'], + ['6B', 'Choice 2'], + ['6C', 'Choice 3'], + ], }, ] bulk_update_data = { @@ -155,7 +167,11 @@ class CustomFieldChoiceSetTest(APIViewTestCases.APIViewTestCase): } update_data = { 'name': 'Choice Set X', - 'extra_choices': ['X1', 'X2', 'X3'], + 'extra_choices': [ + ['X1', 'Choice 1'], + ['X2', 'Choice 2'], + ['X3', 'Choice 3'], + ], 'description': 'New description', } diff --git a/netbox/extras/tests/test_changelog.py b/netbox/extras/tests/test_changelog.py index 95b981b78..34fd72b2b 100644 --- a/netbox/extras/tests/test_changelog.py +++ b/netbox/extras/tests/test_changelog.py @@ -18,7 +18,7 @@ class ChangeLogViewTest(ModelViewTestCase): def setUpTestData(cls): choice_set = CustomFieldChoiceSet.objects.create( name='Choice Set 1', - extra_choices={'bar': 'Bar', 'foo': 'Foo'} + extra_choices=(('foo', 'Foo'), ('bar', 'Bar')) ) # Create a custom field on the Site model @@ -226,7 +226,7 @@ class ChangeLogAPITest(APITestCase): # Create a select custom field on the Site model choice_set = CustomFieldChoiceSet.objects.create( name='Choice Set 1', - extra_choices={'bar': 'Bar', 'foo': 'Foo'} + extra_choices=(('foo', 'Foo'), ('bar', 'Bar')) ) cf_select = CustomField( type=CustomFieldTypeChoices.TYPE_SELECT, diff --git a/netbox/extras/tests/test_customfields.py b/netbox/extras/tests/test_customfields.py index 4bfb5063d..019aef235 100644 --- a/netbox/extras/tests/test_customfields.py +++ b/netbox/extras/tests/test_customfields.py @@ -269,11 +269,11 @@ class CustomFieldTest(TestCase): self.assertIsNone(instance.custom_field_data.get(cf.name)) def test_select_field(self): - CHOICES = { - 'a': 'Option A', - 'b': 'Option B', - 'c': 'Option C', - } + CHOICES = ( + ('a', 'Option A'), + ('b', 'Option B'), + ('c', 'Option C'), + ) value = 'a' # Create a set of custom field choices @@ -306,11 +306,11 @@ class CustomFieldTest(TestCase): self.assertIsNone(instance.custom_field_data.get(cf.name)) def test_multiselect_field(self): - CHOICES = { - 'a': 'Option A', - 'b': 'Option B', - 'c': 'Option C', - } + CHOICES = ( + ('a', 'Option A'), + ('b', 'Option B'), + ('c', 'Option C'), + ) value = ['a', 'b'] # Create a set of custom field choices @@ -461,7 +461,7 @@ class CustomFieldAPITest(APITestCase): # Create a set of custom field choices choice_set = CustomFieldChoiceSet.objects.create( name='Custom Field Choice Set 1', - extra_choices={'foo': 'Foo', 'bar': 'Bar', 'baz': 'Baz'} + extra_choices=(('foo', 'Foo'), ('bar', 'Bar'), ('baz', 'Baz')) ) custom_fields = ( @@ -1049,7 +1049,11 @@ class CustomFieldImportTest(TestCase): # Create a set of custom field choices choice_set = CustomFieldChoiceSet.objects.create( name='Custom Field Choice Set 1', - extra_choices={'a': 'Option A', 'b': 'Option B', 'c': 'Option C'} + extra_choices=( + ('a', 'Option A'), + ('b', 'Option B'), + ('c', 'Option C'), + ) ) custom_fields = ( @@ -1229,7 +1233,7 @@ class CustomFieldModelFilterTest(TestCase): choice_set = CustomFieldChoiceSet.objects.create( name='Custom Field Choice Set 1', - extra_choices={'A': 'A', 'B': 'B', 'C': 'C', 'X': 'X'} + extra_choices=(('a', 'A'), ('b', 'B'), ('c', 'C'), ('x', 'X')) ) # Integer filtering diff --git a/netbox/extras/tests/test_forms.py b/netbox/extras/tests/test_forms.py index 98e4a1b77..9c22bf83c 100644 --- a/netbox/extras/tests/test_forms.py +++ b/netbox/extras/tests/test_forms.py @@ -15,7 +15,7 @@ class CustomFieldModelFormTest(TestCase): obj_type = ContentType.objects.get_for_model(Site) choice_set = CustomFieldChoiceSet.objects.create( name='Choice Set 1', - extra_choices={'a': 'A', 'b': 'B', 'c': 'C'} + extra_choices=(('a', 'A'), ('b', 'B'), ('c', 'C')) ) cf_text = CustomField.objects.create(name='text', type=CustomFieldTypeChoices.TYPE_TEXT) diff --git a/netbox/extras/tests/test_views.py b/netbox/extras/tests/test_views.py index 0caecbbc3..f1b081cfc 100644 --- a/netbox/extras/tests/test_views.py +++ b/netbox/extras/tests/test_views.py @@ -24,7 +24,11 @@ class CustomFieldTestCase(ViewTestCases.PrimaryObjectViewTestCase): site_ct = ContentType.objects.get_for_model(Site) CustomFieldChoiceSet.objects.create( name='Choice Set 1', - extra_choices={'A': 'A', 'B': 'B', 'C': 'C'} + extra_choices=( + ('A', 'A'), + ('B', 'B'), + ('C', 'C'), + ) ) custom_fields = ( @@ -79,36 +83,36 @@ class CustomFieldChoiceSetTestCase(ViewTestCases.PrimaryObjectViewTestCase): choice_sets = ( CustomFieldChoiceSet( name='Choice Set 1', - extra_choices={'A1': 'Choice 1', 'A2': 'Choice 2', 'A3': 'Choice 3'} + extra_choices=(('A1', 'Choice 1'), ('A2', 'Choice 2'), ('A3', 'Choice 3')) ), CustomFieldChoiceSet( name='Choice Set 2', - extra_choices={'B1': 'Choice 1', 'B2': 'Choice 2', 'B3': 'Choice 3'} + extra_choices=(('B1', 'Choice 1'), ('B2', 'Choice 2'), ('B3', 'Choice 3')) ), CustomFieldChoiceSet( name='Choice Set 3', - extra_choices={'C1': 'Choice 1', 'C2': 'Choice 2', 'C3': 'Choice 3'} + extra_choices=(('C1', 'Choice 1'), ('C2', 'Choice 2'), ('C3', 'Choice 3')) ), ) CustomFieldChoiceSet.objects.bulk_create(choice_sets) cls.form_data = { 'name': 'Choice Set X', - 'extra_choices': json.dumps({'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 = ( 'name,extra_choices', - 'Choice Set 4,"{""D1"": ""Choice 1"", ""D2"": ""Choice 2"", ""D3"": ""Choice 3""}"', - 'Choice Set 5,"{""E1"": ""Choice 1"", ""E2"": ""Choice 2"", ""E3"": ""Choice 3""}"', - 'Choice Set 6,"{""F1"": ""Choice 1"", ""F2"": ""Choice 2"", ""F3"": ""Choice 3""}"', + 'Choice Set 4,"D1,D2,D3"', + 'Choice Set 5,"E1,E2,E3"', + 'Choice Set 6,"F1,F2,F3"', ) cls.csv_update_data = ( 'id,extra_choices', - f'{choice_sets[0].pk},"{{""a"": ""A"", ""b"": ""B"", ""c"": ""C""}}"', - f'{choice_sets[1].pk},"{{""a"": ""A"", ""b"": ""B"", ""c"": ""C""}}"', - f'{choice_sets[2].pk},"{{""a"": ""A"", ""b"": ""B"", ""c"": ""C""}}"', + f'{choice_sets[0].pk},"A,B,C"', + f'{choice_sets[1].pk},"A,B,C"', + f'{choice_sets[2].pk},"A,B,C"', ) cls.bulk_edit_data = { diff --git a/netbox/templates/extras/customfieldchoiceset.html b/netbox/templates/extras/customfieldchoiceset.html index 2f42e405e..f1e5ae7d6 100644 --- a/netbox/templates/extras/customfieldchoiceset.html +++ b/netbox/templates/extras/customfieldchoiceset.html @@ -55,7 +55,7 @@ Label - {% for value, label in object.choices.items %} + {% for value, label in object.choices %} {{ value }} {{ label }} diff --git a/netbox/utilities/forms/widgets/misc.py b/netbox/utilities/forms/widgets/misc.py index c4d77fc9a..307031bd8 100644 --- a/netbox/utilities/forms/widgets/misc.py +++ b/netbox/utilities/forms/widgets/misc.py @@ -64,4 +64,6 @@ class ChoicesWidget(forms.Textarea): def format_value(self, value): if not value: return None - return '\n'.join([f'{k},{v}' for k, v in value.items()]) + if type(value) is list: + return '\n'.join([f'{k},{v}' for k, v in value]) + return value diff --git a/netbox/utilities/testing/base.py b/netbox/utilities/testing/base.py index 76a9fac06..aa2093a9a 100644 --- a/netbox/utilities/testing/base.py +++ b/netbox/utilities/testing/base.py @@ -129,13 +129,18 @@ class ModelTestCase(TestCase): model_dict[key] = str(value) else: + field = instance._meta.get_field(key) # Convert ArrayFields to CSV strings - if type(instance._meta.get_field(key)) is ArrayField: - model_dict[key] = ','.join([str(v) for v in value]) + if type(field) is ArrayField: + if type(field.base_field) is ArrayField: + # Handle nested arrays (e.g. choice sets) + model_dict[key] = '\n'.join([f'{k},{v}' for k, v in value]) + else: + model_dict[key] = ','.join([str(v) for v in value]) # JSON - if type(instance._meta.get_field(key)) is JSONField and value is not None: + if type(field) is JSONField and value is not None: model_dict[key] = json.dumps(value) return model_dict