Address PR feedback

netbox/project-static/styles/transitional/_forms.scss

@@ -39,7 +39,10 @@ select[multiple] {
     position: sticky;
     top: 0;
     background-color: var(--bs-body-bg);
+    font-style: normal;
     font-weight: bold;
-    padding: 0.25rem 0.5rem;
+  }
+  option {
+    padding-left: 0.5rem;
   }
 }

netbox/users/forms/model_forms.py

@@ -1,6 +1,8 @@
 import json
+from collections import defaultdict
 
 from django import forms
+from django.apps import apps
 from django.conf import settings
 from django.contrib.auth import password_validation
 from django.contrib.postgres.forms import SimpleArrayField
@@ -21,6 +23,7 @@ from utilities.forms.fields import (
     DynamicModelMultipleChoiceField,
     JSONField,
 )
+from utilities.string import title
 from utilities.forms.rendering import FieldSet
 from utilities.forms.widgets import DateTimePicker, SplitMultiSelectWidget
 from utilities.permissions import qs_filter_from_constraints
@@ -287,37 +290,20 @@ def get_object_types_choices():
     Generate choices for object types grouped by app label using optgroups.
     Returns nested structure: [(app_label, [(id, model_name), ...]), ...]
     """
-    from django.apps import apps
+    app_label_map = {
-
+        app_config.label: app_config.verbose_name
-    choices = []
+        for app_config in apps.get_app_configs()
-    current_app = None
+    }
-    current_group = []
+    choices_by_app = defaultdict(list)
 
     for ot in ObjectType.objects.filter(OBJECTPERMISSION_OBJECT_TYPES).order_by('app_label', 'model'):
-        # Get verbose app label (e.g., "NetBox Branching" instead of "netbox_branching")
+        app_label = app_label_map.get(ot.app_label, ot.app_label)
-        try:
-            app_config = apps.get_app_config(ot.app_label)
-            app_label = app_config.verbose_name
-        except LookupError:
-            app_label = ot.app_label
 
-        # Start new optgroup when app changes
-        if current_app != app_label:
-            if current_group:
-                choices.append((current_app, current_group))
-            current_app = app_label
-            current_group = []
-
-        # Add model to current group using just the model's verbose name
         model_class = ot.model_class()
         model_name = model_class._meta.verbose_name if model_class else ot.model
-        current_group.append((ot.pk, model_name.title()))
+        choices_by_app[app_label].append((ot.pk, title(model_name)))
 
-    # Add final group
+    return list(choices_by_app.items())
-    if current_group:
-        choices.append((current_app, current_group))
-
-    return choices
 
 
 class ObjectPermissionForm(forms.ModelForm):

netbox/utilities/forms/widgets/select.py

@@ -66,31 +66,46 @@ class SelectWithPK(forms.Select):
     option_template_name = 'widgets/select_option_with_pk.html'
 
 
-class AvailableOptions(forms.SelectMultiple):
+class SelectMultipleBase(forms.SelectMultiple):
     """
-    Renders a <select multiple=true> including only choices that have been selected. (For unbound fields, this list
+    Base class for select widgets that filter choices based on selected values.
-    will be empty.) Employed by SplitMultiSelectWidget.
+    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)):  # optgroup
-            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]) 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:
+            else:  # option, e.g. flat choice
-                # This is a flat choice: (id, name)
+                if (str(choice[0]) in value) == include_selected:
-                if str(choice[0]) not 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)
+        option['attrs']['title'] = label  # Add title attribute to show full text on hover
+        return option
+
+
+class AvailableOptions(SelectMultipleBase):
+    """
+    Renders a <select multiple=true> including only choices that have been selected. (For unbound fields, this list
+    will be empty.) Employed by SplitMultiSelectWidget.
+    """
+
     def get_context(self, name, value, attrs):
         context = super().get_context(name, value, attrs)
 
@@ -99,43 +114,13 @@ class AvailableOptions(forms.SelectMultiple):
 
         return context
 
-    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
 
-
+class SelectedOptions(SelectMultipleBase):
-class SelectedOptions(forms.SelectMultiple):
     """
     Renders a <select multiple=true> 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):
+    include_selected = True
-        # 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
 
 
 class SplitMultiSelectWidget(forms.MultiWidget):

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')])