From 3fe366b4707421c268a0674eda66b25c15066144 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Mon, 8 Dec 2025 22:13:07 -0600 Subject: [PATCH] Address PR feedback --- netbox/project-static/dist/netbox.css | Bin 557176 -> 557216 bytes .../styles/transitional/_forms.scss | 5 +- netbox/users/forms/model_forms.py | 36 +++------ netbox/utilities/forms/widgets/select.py | 71 +++++++----------- netbox/utilities/tests/test_forms.py | 69 +++++++++++++++++ 5 files changed, 112 insertions(+), 69 deletions(-) diff --git a/netbox/project-static/dist/netbox.css b/netbox/project-static/dist/netbox.css index bda703cbe90bac077c913ca44c43f2ee60053896..001c8d1eae936dc23b4948a59151c575004775d0 100644 GIT binary patch delta 69 zcmey-ptPV-siB3jg{g(Pg{6hHg{_5s3&*3>sjN(rGUf&bwZ*A9smUeLxurQJnFTqi Zu?qPGC7Jno)zc3;vWskIPU9$*0svT07(oC4 delta 77 zcmZ3`sPv;jsiB3jg{g(Pg{6hHg{_5s3&*2WmGacg^o$ZKa{~kGg2a@R%)E3fJtNbi e)LaEU5Lv5NkeEI_@f?%fWW6b(+Y{3`?4 including only choices that have been selected. (For unbound fields, this list - will be empty.) Employed by SplitMultiSelectWidget. + Base class for select widgets that filter choices based on selected values. + Subclasses should set `include_selected` to control filtering behavior. """ + include_selected = False + def optgroups(self, name, value, attrs=None): - # Handle both flat choices and optgroup choices filtered_choices = [] + include_selected = self.include_selected + for choice in self.choices: - # Check if this is an optgroup (nested tuple) or flat choice - if isinstance(choice[1], (list, tuple)): - # This is an optgroup: (group_label, [(id, name), ...]) + if isinstance(choice[1], (list, tuple)): # optgroup group_label, group_choices = choice - filtered_group = [c for c in group_choices if str(c[0]) not in value] + filtered_group = [ + c for c in group_choices if (str(c[0]) in value) == include_selected + ] + if filtered_group: # Only include optgroup if it has choices left filtered_choices.append((group_label, filtered_group)) - else: - # This is a flat choice: (id, name) - if str(choice[0]) not in value: + else: # option, e.g. flat choice + if (str(choice[0]) in value) == include_selected: filtered_choices.append(choice) self.choices = filtered_choices value = [] # Clear selected choices return super().optgroups(name, value, attrs) + def create_option(self, name, value, label, selected, index, subindex=None, attrs=None): + option = super().create_option(name, value, label, selected, index, subindex, attrs) + option['attrs']['title'] = label # Add title attribute to show full text on hover + return option + + +class AvailableOptions(SelectMultipleBase): + """ + Renders a including only choices that have _not_ been selected. (For unbound fields, this will include _all_ choices.) Employed by SplitMultiSelectWidget. """ - def optgroups(self, name, value, attrs=None): - # Handle both flat choices and optgroup choices - filtered_choices = [] - for choice in self.choices: - # Check if this is an optgroup (nested tuple) or flat choice - if isinstance(choice[1], (list, tuple)): - # This is an optgroup: (group_label, [(id, name), ...]) - group_label, group_choices = choice - filtered_group = [c for c in group_choices if str(c[0]) in value] - if filtered_group: # Only include optgroup if it has choices left - filtered_choices.append((group_label, filtered_group)) - else: - # This is a flat choice: (id, name) - if str(choice[0]) in value: - filtered_choices.append(choice) - - self.choices = filtered_choices - value = [] # Clear selected choices - return super().optgroups(name, value, attrs) - - def create_option(self, name, value, label, selected, index, subindex=None, attrs=None): - option = super().create_option(name, value, label, selected, index, subindex, attrs) - # Add title attribute to show full text on hover - option['attrs']['title'] = label - return option + include_selected = True class SplitMultiSelectWidget(forms.MultiWidget): diff --git a/netbox/utilities/tests/test_forms.py b/netbox/utilities/tests/test_forms.py index cc3ebbb71..6e523c271 100644 --- a/netbox/utilities/tests/test_forms.py +++ b/netbox/utilities/tests/test_forms.py @@ -7,6 +7,7 @@ from utilities.forms.bulk_import import BulkImportForm from utilities.forms.fields.csv import CSVSelectWidget from utilities.forms.forms import BulkRenameForm from utilities.forms.utils import get_field_value, expand_alphanumeric_pattern, expand_ipaddress_pattern +from utilities.forms.widgets.select import AvailableOptions, SelectedOptions class ExpandIPAddress(TestCase): @@ -481,3 +482,71 @@ class CSVSelectWidgetTest(TestCase): widget = CSVSelectWidget() data = {'test_field': 'valid_value'} self.assertFalse(widget.value_omitted_from_data(data, {}, 'test_field')) + + +class SelectMultipleWidgetTest(TestCase): + """ + Validate filtering behavior of AvailableOptions and SelectedOptions widgets. + """ + + def test_available_options_flat_choices(self): + """AvailableOptions should exclude selected values from flat choices""" + widget = AvailableOptions(choices=[ + (1, 'Option 1'), + (2, 'Option 2'), + (3, 'Option 3'), + ]) + widget.optgroups('test', ['2'], None) + + self.assertEqual(len(widget.choices), 2) + self.assertEqual(widget.choices[0], (1, 'Option 1')) + self.assertEqual(widget.choices[1], (3, 'Option 3')) + + def test_available_options_optgroups(self): + """AvailableOptions should exclude selected values from optgroups""" + widget = AvailableOptions(choices=[ + ('Group A', [(1, 'Option 1'), (2, 'Option 2')]), + ('Group B', [(3, 'Option 3'), (4, 'Option 4')]), + ]) + + # Select options 2 and 3 + widget.optgroups('test', ['2', '3'], None) + + # Should have 2 groups with filtered choices + self.assertEqual(len(widget.choices), 2) + self.assertEqual(widget.choices[0][0], 'Group A') + self.assertEqual(widget.choices[0][1], [(1, 'Option 1')]) + self.assertEqual(widget.choices[1][0], 'Group B') + self.assertEqual(widget.choices[1][1], [(4, 'Option 4')]) + + def test_selected_options_flat_choices(self): + """SelectedOptions should include only selected values from flat choices""" + widget = SelectedOptions(choices=[ + (1, 'Option 1'), + (2, 'Option 2'), + (3, 'Option 3'), + ]) + + # Select option 2 + widget.optgroups('test', ['2'], None) + + # Should only have option 2 + self.assertEqual(len(widget.choices), 1) + self.assertEqual(widget.choices[0], (2, 'Option 2')) + + def test_selected_options_optgroups(self): + """SelectedOptions should include only selected values from optgroups""" + widget = SelectedOptions(choices=[ + ('Group A', [(1, 'Option 1'), (2, 'Option 2')]), + ('Group B', [(3, 'Option 3'), (4, 'Option 4')]), + ]) + + # Select options 2 and 3 + widget.optgroups('test', ['2', '3'], None) + + # Should have 2 groups with only selected choices + self.assertEqual(len(widget.choices), 2) + self.assertEqual(widget.choices[0][0], 'Group A') + self.assertEqual(widget.choices[0][1], [(2, 'Option 2')]) + self.assertEqual(widget.choices[1][0], 'Group B') + self.assertEqual(widget.choices[1][1], [(3, 'Option 3')])