From 306cfeeebb9de99dd4330502d3012e7e32fa783d Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 19 Jul 2023 10:09:57 -0400 Subject: [PATCH] Misc cleanup --- netbox/extras/api/nested_serializers.py | 2 +- netbox/extras/api/serializers.py | 8 ++-- netbox/extras/filtersets.py | 6 +++ netbox/extras/forms/filtersets.py | 2 +- .../migrations/0096_customfieldchoiceset.py | 2 +- netbox/extras/models/customfields.py | 16 +++++--- netbox/extras/tables/tables.py | 8 ++-- netbox/extras/tests/test_api.py | 2 +- netbox/extras/tests/test_filtersets.py | 40 ++++++++++++++++--- netbox/netbox/tables/columns.py | 11 +++++ .../extras/customfieldchoiceset.html | 2 +- 11 files changed, 77 insertions(+), 22 deletions(-) diff --git a/netbox/extras/api/nested_serializers.py b/netbox/extras/api/nested_serializers.py index 294b32290..a97c630d2 100644 --- a/netbox/extras/api/nested_serializers.py +++ b/netbox/extras/api/nested_serializers.py @@ -40,7 +40,7 @@ class NestedCustomFieldChoiceSetSerializer(WritableNestedSerializer): class Meta: model = models.CustomFieldChoiceSet - fields = ['id', 'url', 'display', 'name'] + fields = ['id', 'url', 'display', 'name', 'choices_count'] class NestedCustomLinkSerializer(WritableNestedSerializer): diff --git a/netbox/extras/api/serializers.py b/netbox/extras/api/serializers.py index fa6ce1545..fea7582c0 100644 --- a/netbox/extras/api/serializers.py +++ b/netbox/extras/api/serializers.py @@ -103,8 +103,8 @@ class CustomFieldSerializer(ValidatedModelSerializer): fields = [ 'id', 'url', 'display', 'content_types', 'type', 'object_type', 'data_type', 'name', 'label', 'group_name', 'description', 'required', 'search_weight', 'filter_logic', 'ui_visibility', 'is_cloneable', 'default', - 'weight', 'validation_minimum', 'validation_maximum', 'validation_regex', 'choice_set', 'choices', - 'created', 'last_updated', + 'weight', 'validation_minimum', 'validation_maximum', 'validation_regex', 'choice_set', 'created', + 'last_updated', ] def validate_type(self, value): @@ -135,8 +135,8 @@ class CustomFieldChoiceSetSerializer(ValidatedModelSerializer): class Meta: model = CustomFieldChoiceSet fields = [ - 'id', 'url', 'display', 'name', 'description', 'extra_choices', 'order_alphabetically', 'created', - 'last_updated', + 'id', 'url', 'display', 'name', 'description', 'extra_choices', 'order_alphabetically', 'choices_count', + 'created', 'last_updated', ] diff --git a/netbox/extras/filtersets.py b/netbox/extras/filtersets.py index dd34c9ae4..42277d219 100644 --- a/netbox/extras/filtersets.py +++ b/netbox/extras/filtersets.py @@ -78,6 +78,11 @@ class CustomFieldFilterSet(BaseFilterSet): choice_set_id = django_filters.ModelMultipleChoiceFilter( queryset=CustomFieldChoiceSet.objects.all() ) + choice_set = django_filters.ModelMultipleChoiceFilter( + field_name='choice_set__name', + queryset=CustomFieldChoiceSet.objects.all(), + to_field_name='name' + ) class Meta: model = CustomField @@ -122,6 +127,7 @@ class CustomFieldChoiceSetFilterSet(BaseFilterSet): ) def filter_by_choice(self, queryset, name, value): + # TODO: Support case-insensitive matching return queryset.filter(extra_choices__overlap=value) diff --git a/netbox/extras/forms/filtersets.py b/netbox/extras/forms/filtersets.py index adf3d12a4..26b4d9a41 100644 --- a/netbox/extras/forms/filtersets.py +++ b/netbox/extras/forms/filtersets.py @@ -94,7 +94,7 @@ class CustomFieldChoiceSetFilterForm(SavedFiltersMixin, FilterForm): class CustomLinkFilterForm(SavedFiltersMixin, FilterForm): fieldsets = ( (None, ('q', 'filter_id')), - ('Attributes', ('content_types', 'enabled', 'new_window', 'weight')), + (_('Attributes'), ('content_types', 'enabled', 'new_window', 'weight')), ) content_types = ContentTypeMultipleChoiceField( queryset=ContentType.objects.filter(FeatureQuery('custom_links').get_query()), diff --git a/netbox/extras/migrations/0096_customfieldchoiceset.py b/netbox/extras/migrations/0096_customfieldchoiceset.py index a1c02d994..dea6f02fc 100644 --- a/netbox/extras/migrations/0096_customfieldchoiceset.py +++ b/netbox/extras/migrations/0096_customfieldchoiceset.py @@ -52,7 +52,7 @@ class Migration(migrations.Migration): migrations.AddField( model_name='customfield', name='choice_set', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='custom_fields', to='extras.customfieldchoiceset'), + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='choices_for', to='extras.customfieldchoiceset'), ), migrations.RunPython( code=create_choice_sets, diff --git a/netbox/extras/models/customfields.py b/netbox/extras/models/customfields.py index 91804521c..bdb600c88 100644 --- a/netbox/extras/models/customfields.py +++ b/netbox/extras/models/customfields.py @@ -162,7 +162,7 @@ class CustomField(CloningMixin, ExportTemplatesMixin, ChangeLoggedModel): choice_set = models.ForeignKey( to='CustomFieldChoiceSet', on_delete=models.PROTECT, - related_name='custom_fields', + related_name='choices_for', blank=True, null=True ) @@ -286,18 +286,18 @@ class CustomField(CloningMixin, ExportTemplatesMixin, ChangeLoggedModel): 'validation_regex': "Regular expression validation is supported only for text and URL fields" }) - # Choice set must be set on selection fields + # Choice set must be set on selection fields, and *only* on selection fields if self.type in ( CustomFieldTypeChoices.TYPE_SELECT, CustomFieldTypeChoices.TYPE_MULTISELECT ): if not self.choice_set: raise ValidationError({ - 'choice_set': "Selection fields must define a set of choices." + 'choice_set': "Selection fields must specify a set of choices." }) elif self.choice_set: raise ValidationError({ - 'choice_set': "Choices may be set only for selection fields." + 'choice_set': "Choices may be set only on selection fields." }) # A selection field's default (if any) must be present in its available choices @@ -633,7 +633,7 @@ class CustomField(CloningMixin, ExportTemplatesMixin, ChangeLoggedModel): raise ValidationError("Required field cannot be empty.") -class CustomFieldChoiceSet(ChangeLoggedModel): +class CustomFieldChoiceSet(CloningMixin, ExportTemplatesMixin, ChangeLoggedModel): """ Represents a set of choices available for choice and multi-choice custom fields. """ @@ -654,6 +654,8 @@ class CustomFieldChoiceSet(ChangeLoggedModel): help_text=_('Choices are automatically ordered alphabetically on save') ) + clone_fields = ('extra_choices', 'order_alphabetically') + class Meta: ordering = ('name',) @@ -667,6 +669,10 @@ class CustomFieldChoiceSet(ChangeLoggedModel): def choices(self): return self.extra_choices + @property + def choices_count(self): + return len(self.choices) + def save(self, *args, **kwargs): # Sort choices if alphabetical ordering is enforced diff --git a/netbox/extras/tables/tables.py b/netbox/extras/tables/tables.py index 268f2319d..e5e722398 100644 --- a/netbox/extras/tables/tables.py +++ b/netbox/extras/tables/tables.py @@ -5,7 +5,7 @@ from django.conf import settings from django.utils.translation import gettext as _ from extras.models import * -from netbox.tables import ArrayColumn, NetBoxTable, columns +from netbox.tables import NetBoxTable, columns from .template_code import * __all__ = ( @@ -66,7 +66,8 @@ class CustomFieldTable(NetBoxTable): required = columns.BooleanColumn() ui_visibility = columns.ChoiceFieldColumn(verbose_name="UI visibility") description = columns.MarkdownColumn() - choices = ArrayColumn( + choices = columns.ArrayColumn( + max_items=10, orderable=False, verbose_name=_('Choices') ) @@ -86,7 +87,8 @@ class CustomFieldChoiceSetTable(NetBoxTable): name = tables.Column( linkify=True ) - choices = ArrayColumn( + choices = columns.ArrayColumn( + max_items=10, accessor=tables.A('extra_choices'), orderable=False, verbose_name=_('Choices') diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index 3230d1da0..922b45240 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -135,7 +135,7 @@ class CustomFieldTest(APIViewTestCases.APIViewTestCase): class CustomFieldChoiceSetTest(APIViewTestCases.APIViewTestCase): model = CustomFieldChoiceSet - brief_fields = ['display', 'id', 'name', 'url'] + brief_fields = ['choices_count', 'display', 'id', 'name', 'url'] create_data = [ { 'name': 'Choice Set 4', diff --git a/netbox/extras/tests/test_filtersets.py b/netbox/extras/tests/test_filtersets.py index 91a842bd5..c558a0467 100644 --- a/netbox/extras/tests/test_filtersets.py +++ b/netbox/extras/tests/test_filtersets.py @@ -27,7 +27,11 @@ class CustomFieldTestCase(TestCase, BaseFilterSetTests): @classmethod def setUpTestData(cls): - content_types = ContentType.objects.filter(model__in=['site', 'rack', 'device']) + choice_sets = ( + CustomFieldChoiceSet(name='Choice Set 1', extra_choices=['A', 'B', 'C']), + CustomFieldChoiceSet(name='Choice Set 2', extra_choices=['D', 'E', 'F']), + ) + CustomFieldChoiceSet.objects.bulk_create(choice_sets) custom_fields = ( CustomField( @@ -54,11 +58,31 @@ class CustomFieldTestCase(TestCase, BaseFilterSetTests): filter_logic=CustomFieldFilterLogicChoices.FILTER_DISABLED, ui_visibility=CustomFieldVisibilityChoices.VISIBILITY_HIDDEN ), + CustomField( + name='Custom Field 4', + type=CustomFieldTypeChoices.TYPE_SELECT, + required=False, + weight=400, + filter_logic=CustomFieldFilterLogicChoices.FILTER_DISABLED, + ui_visibility=CustomFieldVisibilityChoices.VISIBILITY_HIDDEN, + choice_set=choice_sets[0] + ), + CustomField( + name='Custom Field 5', + type=CustomFieldTypeChoices.TYPE_MULTISELECT, + required=False, + weight=500, + filter_logic=CustomFieldFilterLogicChoices.FILTER_DISABLED, + ui_visibility=CustomFieldVisibilityChoices.VISIBILITY_HIDDEN, + choice_set=choice_sets[1] + ), ) CustomField.objects.bulk_create(custom_fields) - custom_fields[0].content_types.add(content_types[0]) - custom_fields[1].content_types.add(content_types[1]) - custom_fields[2].content_types.add(content_types[2]) + custom_fields[0].content_types.add(ContentType.objects.get_by_natural_key('dcim', 'site')) + custom_fields[1].content_types.add(ContentType.objects.get_by_natural_key('dcim', 'rack')) + custom_fields[2].content_types.add(ContentType.objects.get_by_natural_key('dcim', 'device')) + custom_fields[3].content_types.add(ContentType.objects.get_by_natural_key('dcim', 'device')) + custom_fields[4].content_types.add(ContentType.objects.get_by_natural_key('dcim', 'device')) def test_name(self): params = {'name': ['Custom Field 1', 'Custom Field 2']} @@ -67,7 +91,7 @@ class CustomFieldTestCase(TestCase, BaseFilterSetTests): def test_content_types(self): params = {'content_types': 'dcim.site'} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) - params = {'content_type_id': [ContentType.objects.get_for_model(Site).pk]} + params = {'content_type_id': [ContentType.objects.get_by_natural_key('dcim', 'site').pk]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) def test_required(self): @@ -86,6 +110,12 @@ class CustomFieldTestCase(TestCase, BaseFilterSetTests): params = {'ui_visibility': CustomFieldVisibilityChoices.VISIBILITY_READ_WRITE} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) + def test_choice_set(self): + params = {'choice_set': ['Choice Set 1', 'Choice Set 2']} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + params = {'choice_set_id': CustomFieldChoiceSet.objects.values_list('pk', flat=True)} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + class CustomFieldChoiceSetTestCase(TestCase, BaseFilterSetTests): queryset = CustomFieldChoiceSet.objects.all() diff --git a/netbox/netbox/tables/columns.py b/netbox/netbox/tables/columns.py index 05dd396f4..1f698f396 100644 --- a/netbox/netbox/tables/columns.py +++ b/netbox/netbox/tables/columns.py @@ -598,5 +598,16 @@ class ArrayColumn(tables.Column): """ List array items as a comma-separated list. """ + def __init__(self, *args, max_items=None, **kwargs): + self.max_items = max_items + super().__init__(*args, **kwargs) + def render(self, value): + if self.max_items: + # Limit the returned items to the specified maximum number + omitted = len(value) - self.max_items + value = value[:self.max_items - 1] + if omitted > 0: + value.append(f'({omitted} more)') + return ', '.join(value) diff --git a/netbox/templates/extras/customfieldchoiceset.html b/netbox/templates/extras/customfieldchoiceset.html index 796d268a9..25c95729e 100644 --- a/netbox/templates/extras/customfieldchoiceset.html +++ b/netbox/templates/extras/customfieldchoiceset.html @@ -29,7 +29,7 @@ Used by