Closes #19968: Use multiple selection lists for the assignment of object types when editing a permission (#19991)

* Closes #19968: Use  multiple selection lists for the assignment of object types when editing a permission

* Remove errant logging statements

* Defer compilation of choices for object_types

* Fix test data

netbox/project-static/src/buttons/moveOptions.ts

@@ -1,5 +1,20 @@
 import { getElements } from '../util';
 
+/**
+ * Move selected options from one select element to another.
+ *
+ * @param source Select Element
+ * @param target Select Element
+ */
+function moveOption(source: HTMLSelectElement, target: HTMLSelectElement): void {
+  for (const option of Array.from(source.options)) {
+    if (option.selected) {
+      target.appendChild(option.cloneNode(true));
+      option.remove();
+    }
+  }
+}
+
 /**
  * Move selected options of a select element up in order.
  *
@@ -39,23 +54,35 @@ function moveOptionDown(element: HTMLSelectElement): void {
 }
 
 /**
- * Initialize move up/down buttons.
+ * Initialize select/move buttons.
  */
 export function initMoveButtons(): void {
-  for (const button of getElements<HTMLButtonElement>('#move-option-up')) {
+  // Move selected option(s) between lists
+  for (const button of getElements<HTMLButtonElement>('.move-option')) {
+    const source = button.getAttribute('data-source');
+    const target = button.getAttribute('data-target');
+    const source_select = document.getElementById(`id_${source}`) as HTMLSelectElement;
+    const target_select = document.getElementById(`id_${target}`) as HTMLSelectElement;
+    if (source_select !== null && target_select !== null) {
+      button.addEventListener('click', () => moveOption(source_select, target_select));
+    }
+  }
+
+  // Move selected option(s) up in current list
+  for (const button of getElements<HTMLButtonElement>('.move-option-up')) {
     const target = button.getAttribute('data-target');
-    if (target !== null) {
-      for (const select of getElements<HTMLSelectElement>(`#${target}`)) {
-        button.addEventListener('click', () => moveOptionUp(select));
-      }
+    const target_select = document.getElementById(`id_${target}`) as HTMLSelectElement;
+    if (target_select !== null) {
+      button.addEventListener('click', () => moveOptionUp(target_select));
     }
   }
-  for (const button of getElements<HTMLButtonElement>('#move-option-down')) {
+
+  // Move selected option(s) down in current list
+  for (const button of getElements<HTMLButtonElement>('.move-option-down')) {
     const target = button.getAttribute('data-target');
-    if (target !== null) {
-      for (const select of getElements<HTMLSelectElement>(`#${target}`)) {
-        button.addEventListener('click', () => moveOptionDown(select));
-      }
+    const target_select = document.getElementById(`id_${target}`) as HTMLSelectElement;
+    if (target_select !== null) {
+      button.addEventListener('click', () => moveOptionDown(target_select));
     }
   }
 }

netbox/templates/extras/tableconfig_edit.html

@@ -36,10 +36,10 @@
       <div class="col-5 text-center">
         <label class="form-label">{{ form.columns.label }}</label>
         {{ form.columns }}
-        <a tabindex="0" class="btn btn-primary btn-sm mt-2" id="move-option-up" data-target="id_columns">
+        <a tabindex="0" class="btn btn-primary btn-sm mt-2 move-option-up" data-target="columns">
           <i class="mdi mdi-arrow-up-bold"></i> {% trans "Move Up" %}
         </a>
-        <a tabindex="0" class="btn btn-primary btn-sm mt-2" id="move-option-down" data-target="id_columns">
+        <a tabindex="0" class="btn btn-primary btn-sm mt-2 move-option-down" data-target="columns">
           <i class="mdi mdi-arrow-down-bold"></i> {% trans "Move Down" %}
         </a>
       </div>

netbox/users/forms/model_forms.py

@@ -15,7 +15,7 @@ from users.models import *
 from utilities.data import flatten_dict
 from utilities.forms.fields import ContentTypeMultipleChoiceField, DynamicModelMultipleChoiceField
 from utilities.forms.rendering import FieldSet
-from utilities.forms.widgets import DateTimePicker
+from utilities.forms.widgets import DateTimePicker, SplitMultiSelectWidget
 from utilities.permissions import qs_filter_from_constraints
 
 __all__ = (
@@ -272,12 +272,21 @@ class GroupForm(forms.ModelForm):
         return instance
 
 
+def get_object_types_choices():
+    return [
+        (ot.pk, str(ot))
+        for ot in ObjectType.objects.filter(OBJECTPERMISSION_OBJECT_TYPES).order_by('app_label', 'model')
+    ]
+
+
 class ObjectPermissionForm(forms.ModelForm):
     object_types = ContentTypeMultipleChoiceField(
         label=_('Object types'),
         queryset=ObjectType.objects.all(),
-        limit_choices_to=OBJECTPERMISSION_OBJECT_TYPES,
-        widget=forms.SelectMultiple(attrs={'size': 6})
+        widget=SplitMultiSelectWidget(
+            choices=get_object_types_choices
+        ),
+        help_text=_('Select the types of objects to which the permission will appy.')
     )
     can_view = forms.BooleanField(
         required=False

netbox/users/tests/test_views.py

@@ -180,7 +180,7 @@ class ObjectPermissionTestCase(
         cls.form_data = {
             'name': 'Permission X',
             'description': 'A new permission',
-            'object_types': [object_type.pk],
+            'object_types_1': [object_type.pk],  # SplitMultiSelectWidget requires _1 suffix on field name
             'actions': 'view,edit,delete',
         }
 

netbox/utilities/forms/widgets/select.py

@@ -8,6 +8,7 @@ __all__ = (
     'ColorSelect',
     'HTMXSelect',
     'SelectWithPK',
+    'SplitMultiSelectWidget',
 )
 
 
@@ -63,3 +64,79 @@ class SelectWithPK(forms.Select):
     Include the primary key of each option in the option label (e.g. "Router7 (4721)").
     """
     option_template_name = 'widgets/select_option_with_pk.html'
+
+
+class AvailableOptions(forms.SelectMultiple):
+    """
+    Renders a <select multiple=true> including only choices that have been selected. (For unbound fields, this list
+    will be empty.) Employed by SplitMultiSelectWidget.
+    """
+    def optgroups(self, name, value, attrs=None):
+        self.choices = [
+            choice for choice in self.choices if str(choice[0]) not in value
+        ]
+        value = []  # Clear selected choices
+        return super().optgroups(name, value, attrs)
+
+    def get_context(self, name, value, attrs):
+        context = super().get_context(name, value, attrs)
+
+        # This widget should never require a selection
+        context['widget']['attrs']['required'] = False
+
+        return context
+
+
+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):
+        self.choices = [
+            choice for choice in self.choices if str(choice[0]) in value
+        ]
+        value = []  # Clear selected choices
+        return super().optgroups(name, value, attrs)
+
+
+class SplitMultiSelectWidget(forms.MultiWidget):
+    """
+    Renders two <select multiple=true> widgets side-by-side: one listing available choices, the other listing selected
+    choices. Options are selected by moving them from the left column to the right.
+
+    Args:
+        ordering: If true, the selected choices list will include controls to reorder items within the list. This should
+                  be enabled only if the order of the selected choices is significant.
+    """
+    template_name = 'widgets/splitmultiselect.html'
+
+    def __init__(self, choices, attrs=None, ordering=False):
+        widgets = [
+            AvailableOptions(
+                attrs={'size': 8},
+                choices=choices
+            ),
+            SelectedOptions(
+                attrs={'size': 8, 'class': 'select-all'},
+                choices=choices
+            ),
+        ]
+
+        super().__init__(widgets, attrs)
+
+        self.ordering = ordering
+
+    def get_context(self, name, value, attrs):
+        # Replicate value for each multi-select widget
+        # Django bug? See django/forms/widgets.py L985
+        value = [value, value]
+
+        # Include ordering boolean in widget context
+        context = super().get_context(name, value, attrs)
+        context['widget']['ordering'] = self.ordering
+        return context
+
+    def value_from_datadict(self, data, files, name):
+        # Return only the choices from the SelectedOptions widget
+        return super().value_from_datadict(data, files, name)[1]

netbox/utilities/templates/helpers/table_config_form.html

@@ -27,11 +27,11 @@
           <div class="col-5 text-center">
             {{ form.columns.label }}
             {{ form.columns }}
-            <a tabindex="0" class="btn btn-primary btn-sm mt-2" id="move-option-up" data-target="id_columns">
-                <i class="mdi mdi-arrow-up-bold"></i> {% trans "Move Up" %}
+            <a tabindex="0" class="btn btn-primary btn-sm mt-2 move-option-up" data-target="columns">
+              <i class="mdi mdi-arrow-up-bold"></i> {% trans "Move Up" %}
             </a>
-            <a tabindex="0" class="btn btn-primary btn-sm mt-2" id="move-option-down" data-target="id_columns">
-                <i class="mdi mdi-arrow-down-bold"></i> {% trans "Move Down" %}
+            <a tabindex="0" class="btn btn-primary btn-sm mt-2 move-option-down" data-target="columns">
+              <i class="mdi mdi-arrow-down-bold"></i> {% trans "Move Down" %}
             </a>
           </div>
         </div>

netbox/utilities/templates/widgets/splitmultiselect.html

@@ -0,0 +1,31 @@
+{% load i18n %}
+<div class="field-group">
+  <div class="row">
+    <div class="col-5 text-center">
+      <label class="form-label mb-1">{% trans "Available" %}</label>
+      {% include "django/forms/widgets/select.html" with widget=widget.subwidgets.0 %}
+    </div>
+    <div class="col-2 d-flex align-items-center">
+      <div>
+        <a tabindex="0" class="btn btn-success btn-sm w-100 my-2 move-option" data-source="{{ widget.name }}_0" data-target="{{ widget.name }}_1">
+          <i class="mdi mdi-arrow-right-bold"></i> {% trans "Add" %}
+        </a>
+        <a tabindex="0" class="btn btn-danger btn-sm w-100 my-2 move-option" data-source="{{ widget.name }}_1" data-target="{{ widget.name }}_0">
+          <i class="mdi mdi-arrow-left-bold"></i> {% trans "Remove" %}
+        </a>
+      </div>
+    </div>
+    <div class="col-5 text-center">
+      <label class="form-label mb-1">{% trans "Selected" %}</label>
+      {% include "django/forms/widgets/select.html" with widget=widget.subwidgets.1 %}
+      {% if widget.ordering %}
+        <a tabindex="0" class="btn btn-primary btn-sm mt-2 move-option-up" data-target="{{ widget.name }}_1">
+          <i class="mdi mdi-arrow-up-bold"></i> {% trans "Move Up" %}
+        </a>
+        <a tabindex="0" class="btn btn-primary btn-sm mt-2 move-option-down" data-target="{{ widget.name }}_1">
+          <i class="mdi mdi-arrow-down-bold"></i> {% trans "Move Down" %}
+        </a>
+      {% endif %}
+    </div>
+  </div>
+</div>