starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-04-10 11:23:58 +02:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Simplify ObjectPermission form and remove custom widgets

Browse Source 
Replace the dynamic UI with standard BooleanField checkboxes for each
registered action. No custom widgets, no JavaScript, no template
changes.

- Remove RegisteredActionsWidget, ObjectTypeSplitMultiSelectWidget,
  and registeredActions.ts
- Use dynamic BooleanFields for registered actions (renders identically
  to CRUD checkboxes)
- Move action-resolution logic from panel to ObjectPermission model
- Remove object-type cross-validation from form clean()
- Remove unused get_action_model_map utility
This commit is contained in:
Jason Novinger
2026-04-07 10:11:28 -05:00
parent 2e19ee6961
commit 1ec9b5b135
14 changed files with 110 additions and 345 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
netbox/project-static/dist/netbox.js vendored
View File

File diff suppressed because one or more lines are too long

8
netbox/project-static/dist/netbox.js.map vendored
View File

File diff suppressed because one or more lines are too long

9
netbox/project-static/src/forms/index.ts
View File

@@ -1,17 +1,10 @@
import { initClearField } from './clearField';
import { initFormElements } from './elements';
import { initFilterModifiers } from './filterModifiers';
import { initRegisteredActions } from './registeredActions';
import { initSpeedSelector } from './speedSelector';
export function initForms(): void {
for (const func of [
initFormElements,
initSpeedSelector,
initFilterModifiers,
initClearField,
initRegisteredActions,
]) {
for (const func of [initFormElements, initSpeedSelector, initFilterModifiers, initClearField]) {
func();
}
}

62
netbox/project-static/src/forms/registeredActions.ts
View File

@@ -1,62 +0,0 @@
import { getElements } from '../util';
/**
* Enable/disable registered action checkboxes based on selected object_types.
*/
export function initRegisteredActions(): void {
const selectedList = document.querySelector<HTMLSelectElement>(
'select[data-object-types-selected]',
);
if (!selectedList) {
return;
}
const actionCheckboxes = Array.from(
document.querySelectorAll<HTMLInputElement>('input[type="checkbox"][data-models]'),
);
if (actionCheckboxes.length === 0) {
return;
}
function updateState(): void {
const selectedModels = new Set<string>();
// Get model keys from selected options
for (const option of Array.from(selectedList!.options)) {
const modelKey = option.dataset.modelKey;
if (modelKey) {
selectedModels.add(modelKey);
}
}
// Enable a checkbox if any of its supported models is selected
for (const checkbox of actionCheckboxes) {
const modelKeys = (checkbox.dataset.models ?? '').split(',').filter(Boolean);
const enabled = modelKeys.some(m => selectedModels.has(m));
checkbox.disabled = !enabled;
if (!enabled) {
checkbox.checked = false;
}
checkbox.style.opacity = enabled ? '' : '0.75';
// Fade the label text when disabled
const label = checkbox.nextElementSibling as HTMLElement | null;
if (label) {
label.style.opacity = enabled ? '' : '0.5';
}
}
}
// Initial update
updateState();
// Listen to move button clicks
for (const btn of getElements<HTMLButtonElement>('.move-option')) {
btn.addEventListener('click', () => {
// Wait for DOM update
setTimeout(updateState, 0);
});
}
}

110
netbox/users/forms/model_forms.py
View File

@@ -26,8 +26,8 @@ from utilities.forms.fields import (
JSONField,
)
from utilities.forms.rendering import FieldSet
from utilities.forms.widgets import DateTimePicker, ObjectTypeSplitMultiSelectWidget, RegisteredActionsWidget
from utilities.permissions import get_action_model_map, qs_filter_from_constraints
from utilities.forms.widgets import DateTimePicker, SplitMultiSelectWidget
from utilities.permissions import qs_filter_from_constraints
from utilities.string import title
__all__ = (
@@ -326,7 +326,7 @@ class ObjectPermissionForm(forms.ModelForm):
object_types = ContentTypeMultipleChoiceField(
label=_('Object types'),
queryset=ObjectType.objects.all(),
widget=ObjectTypeSplitMultiSelectWidget(
widget=SplitMultiSelectWidget(
choices=get_object_types_choices
),
help_text=_('Select the types of objects to which the permission will apply.')
@@ -343,11 +343,6 @@ class ObjectPermissionForm(forms.ModelForm):
can_delete = forms.BooleanField(
required=False
)
registered_actions = forms.MultipleChoiceField(
required=False,
widget=RegisteredActionsWidget(),
label=_('Registered actions'),
)
actions = SimpleArrayField(
label=_('Additional actions'),
base_field=forms.CharField(),
@@ -378,7 +373,7 @@ class ObjectPermissionForm(forms.ModelForm):
FieldSet('name', 'description', 'enabled'),
FieldSet('object_types', name=_('Objects')),
FieldSet(
'can_view', 'can_add', 'can_change', 'can_delete', 'registered_actions', 'actions',
'can_view', 'can_add', 'can_change', 'can_delete', 'actions',
name=_('Actions')
),
FieldSet('groups', 'users', name=_('Assignment')),
@@ -394,24 +389,38 @@ class ObjectPermissionForm(forms.ModelForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Build PK to model key mapping for object_types widget
pk_to_model_key = {
ot.pk: f'{ot.app_label}.{ot.model}'
for ot in ObjectType.objects.filter(OBJECTPERMISSION_OBJECT_TYPES)
}
self.fields['object_types'].widget.set_model_key_map(pk_to_model_key)
# Configure registered_actions widget and field choices (flat, deduplicated by name)
model_actions = dict(registry['model_actions'])
self.fields['registered_actions'].widget.set_model_actions(model_actions)
# Build dynamic BooleanFields for registered actions (deduplicated by name)
seen = set()
choices = []
for actions in model_actions.values():
registered_action_names = []
for actions in registry['model_actions'].values():
for action in actions:
if action.name not in seen:
choices.append((action.name, action.name))
registered_action_names.append(action.name)
seen.add(action.name)
self.fields['registered_actions'].choices = choices
action_field_names = []
for action_name in registered_action_names:
field_name = f'action_{action_name}'
self.fields[field_name] = forms.BooleanField(
required=False,
label=action_name,
)
action_field_names.append(field_name)
# Rebuild the Actions fieldset to include dynamic fields
if action_field_names:
self.fieldsets = (
FieldSet('name', 'description', 'enabled'),
FieldSet('object_types', name=_('Objects')),
FieldSet(
'can_view', 'can_add', 'can_change', 'can_delete',
*action_field_names,
'actions',
name=_('Actions')
),
FieldSet('groups', 'users', name=_('Assignment')),
FieldSet('constraints', name=_('Constraints')),
)
# Make the actions field optional since the form uses it only for non-CRUD actions
self.fields['actions'].required = False
@@ -431,23 +440,14 @@ class ObjectPermissionForm(forms.ModelForm):
self.fields[f'can_{action}'].initial = True
remaining_actions.remove(action)
# Pre-select registered actions: action is checked if it's in instance.actions
# AND at least one assigned object type supports it
selected_registered = []
consumed_actions = set()
for ct in self.instance.object_types.all():
model_key = f'{ct.app_label}.{ct.model}'
if model_key in model_actions:
for ma in model_actions[model_key]:
if ma.name in remaining_actions and ma.name not in consumed_actions:
selected_registered.append(ma.name)
consumed_actions.add(ma.name)
self.fields['registered_actions'].initial = selected_registered
# Pre-select registered action checkboxes
for action_name in registered_action_names:
if action_name in remaining_actions:
self.fields[f'action_{action_name}'].initial = True
remaining_actions.remove(action_name)
# Remaining actions go to the additional actions field
self.initial['actions'] = [
a for a in remaining_actions if a not in consumed_actions
]
self.initial['actions'] = remaining_actions
# Populate initial data for a new ObjectPermission
elif self.initial:
@@ -461,6 +461,11 @@ class ObjectPermissionForm(forms.ModelForm):
if action in cloned_actions:
self.fields[f'can_{action}'].initial = True
self.initial['actions'].remove(action)
# Pre-select registered action checkboxes from cloned data
for action_name in registered_action_names:
if action_name in cloned_actions:
self.fields[f'action_{action_name}'].initial = True
self.initial['actions'].remove(action_name)
# Convert data delivered via initial data to JSON data
if 'constraints' in self.initial:
if type(self.initial['constraints']) is str:
@@ -470,37 +475,20 @@ class ObjectPermissionForm(forms.ModelForm):
super().clean()
object_types = self.cleaned_data.get('object_types', [])
registered_actions = self.cleaned_data.get('registered_actions', [])
constraints = self.cleaned_data.get('constraints')
# Build set of selected model keys for validation
selected_models = {f'{ct.app_label}.{ct.model}' for ct in object_types}
# Build map of action_name -> set of model_keys that support it
action_model_keys = get_action_model_map(dict(registry['model_actions']))
# Validate each selected action is supported by at least one selected object type
errors = []
# Merge all actions: registered action checkboxes, CRUD checkboxes, and additional
final_actions = []
for action_name in registered_actions:
supported_models = action_model_keys.get(action_name, set())
if not supported_models & selected_models:
errors.append(
_('Action "{action}" is not supported by any of the selected object types.').format(
action=action_name
)
)
elif action_name not in final_actions:
final_actions.append(action_name)
if errors:
raise forms.ValidationError({'registered_actions': errors})
for key, value in self.cleaned_data.items():
if key.startswith('action_') and value:
action_name = key[7:]
if action_name not in final_actions:
final_actions.append(action_name)
# Append any of the selected CRUD checkboxes to the actions list
for action in RESERVED_ACTIONS:
if self.cleaned_data.get(f'can_{action}') and action not in final_actions:
final_actions.append(action)
# Add additional/manual actions
if additional_actions := self.cleaned_data.get('actions'):
for action in additional_actions:
if action not in final_actions:

24
netbox/users/models/permissions.py
View File

@@ -4,6 +4,8 @@ from django.urls import reverse
from django.utils.translation import gettext_lazy as _
from netbox.models.features import CloningMixin
from netbox.registry import registry
from users.constants import RESERVED_ACTIONS
from utilities.querysets import RestrictedQuerySet
__all__ = (
@@ -82,5 +84,27 @@ class ObjectPermission(CloningMixin, models.Model):
return [self.constraints]
return self.constraints
def get_registered_actions(self):
"""
Return a list of (action_name, is_enabled, model_keys_csv) tuples for all
registered actions, indicating which are enabled on this permission.
"""
enabled_actions = set(self.actions) - set(RESERVED_ACTIONS)
seen = []
seen_set = set()
action_models = {}
for model_key, model_actions in registry['model_actions'].items():
for action in model_actions:
if action.name not in seen_set:
seen.append(action.name)
seen_set.add(action.name)
action_models.setdefault(action.name, []).append(model_key)
return [
(action, action in enabled_actions, ', '.join(sorted(action_models[action])))
for action in seen
]
def get_absolute_url(self):
return reverse('users:objectpermission', args=[self.pk])

22
netbox/users/ui/panels.py
View File

@@ -1,8 +1,6 @@
from django.utils.translation import gettext_lazy as _
from netbox.registry import registry
from netbox.ui import actions, attrs, panels
from users.constants import RESERVED_ACTIONS
class TokenPanel(panels.ObjectAttributesPanel):
@@ -61,28 +59,10 @@ class ObjectPermissionActionsPanel(panels.ObjectPanel):
(_('Delete'), 'delete' in obj.actions),
]
enabled_actions = set(obj.actions) - set(RESERVED_ACTIONS)
# Collect all registered actions from the full registry, deduplicating by name.
seen = []
seen_set = set()
action_models = {}
for model_key, model_actions in registry['model_actions'].items():
for action in model_actions:
if action.name not in seen_set:
seen.append(action.name)
seen_set.add(action.name)
action_models.setdefault(action.name, []).append(model_key)
registered_display = [
(action, action in enabled_actions, ', '.join(sorted(action_models[action])))
for action in seen
]
return {
**super().get_context(context),
'crud_actions': crud_actions,
'registered_actions': registered_display,
'registered_actions': obj.get_registered_actions(),
}

1
netbox/utilities/forms/widgets/__init__.py
View File

@@ -1,4 +1,3 @@
from .actions import *
from .apiselect import *
from .datetime import *
from .misc import *

47
netbox/utilities/forms/widgets/actions.py
View File

@@ -1,47 +0,0 @@
from django import forms
from utilities.permissions import get_action_model_map
__all__ = (
'RegisteredActionsWidget',
)
class RegisteredActionsWidget(forms.CheckboxSelectMultiple):
"""
Widget for registered model actions. Renders each action as an individual checkbox
row styled identically to the CRUD checkboxes, with a data-models attribute so JS
can enable/disable based on the currently selected object types.
"""
template_name = 'widgets/registered_actions.html'
def __init__(self, *args, model_actions=None, **kwargs):
super().__init__(*args, **kwargs)
self.model_actions = model_actions or {}
self._action_model_keys = {}
self._action_help_text = {}
self._build_maps()
def _build_maps(self):
self._action_model_keys = get_action_model_map(self.model_actions)
self._action_help_text = {}
for actions in self.model_actions.values():
for action in actions:
if action.name not in self._action_help_text:
self._action_help_text[action.name] = action.help_text
def set_model_actions(self, model_actions):
self.model_actions = model_actions
self._build_maps()
def create_option(self, name, value, label, selected, index, subindex=None, attrs=None):
"""
Inject model_keys (comma-separated model keys that support this action) and
help_text into the option dict. The template uses model_keys for the data-models
attribute, which JS reads to enable/disable checkboxes based on selected object types.
"""
option = super().create_option(name, value, label, selected, index, subindex=subindex, attrs=attrs)
action_name = str(value)
option['model_keys'] = ','.join(sorted(self._action_model_keys.get(action_name, set())))
option['help_text'] = self._action_help_text.get(action_name, '')
return option

51
netbox/utilities/forms/widgets/select.py
View File

@@ -9,7 +9,6 @@ __all__ = (
'ClearableSelect',
'ColorSelect',
'HTMXSelect',
'ObjectTypeSplitMultiSelectWidget',
'SelectWithPK',
'SplitMultiSelectWidget',
)
@@ -183,53 +182,3 @@ class SplitMultiSelectWidget(forms.MultiWidget):
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]
#
# ObjectType-specific widgets for ObjectPermissionForm
#
class ObjectTypeSelectMultiple(SelectMultipleBase):
"""
SelectMultiple that adds data-model-key attribute to options for JS targeting.
"""
pk_to_model_key = None
def create_option(self, name, value, label, selected, index, subindex=None, attrs=None):
option = super().create_option(name, value, label, selected, index, subindex, attrs)
if self.pk_to_model_key:
model_key = self.pk_to_model_key.get(value) or self.pk_to_model_key.get(str(value))
if model_key:
option['attrs']['data-model-key'] = model_key
return option
class ObjectTypeAvailableOptions(ObjectTypeSelectMultiple):
include_selected = False
def get_context(self, name, value, attrs):
context = super().get_context(name, value, attrs)
context['widget']['attrs']['required'] = False
return context
class ObjectTypeSelectedOptions(ObjectTypeSelectMultiple):
include_selected = True
def get_context(self, name, value, attrs):
context = super().get_context(name, value, attrs)
context['widget']['attrs']['data-object-types-selected'] = 'true'
return context
class ObjectTypeSplitMultiSelectWidget(SplitMultiSelectWidget):
"""
SplitMultiSelectWidget that adds data-model-key attributes to options.
Used by ObjectPermissionForm to enable JS show/hide of custom actions.
"""
available_widget_class = ObjectTypeAvailableOptions
selected_widget_class = ObjectTypeSelectedOptions
def set_model_key_map(self, pk_to_model_key):
for widget in self.widgets:
widget.pk_to_model_key = pk_to_model_key

18
netbox/utilities/permissions.py
View File

@@ -10,7 +10,6 @@ from users.constants import CONSTRAINT_TOKEN_USER, RESERVED_ACTIONS
__all__ = (
'ModelAction',
'get_action_model_map',
'get_permission_for_model',
'permission_is_exempt',
'qs_filter_from_constraints',
@@ -63,23 +62,6 @@ def register_model_actions(model: type[Model], actions: list[ModelAction | str])
registry['model_actions'][label].add(action)
def get_action_model_map(model_actions):
"""
Build a mapping of action name to the set of model keys that support it.
Args:
model_actions: Dict of {model_key: [ModelAction, ...]} from the registry
Returns:
Dict of {action_name: {model_key, ...}}
"""
mapping = {}
for model_key, actions in model_actions.items():
for action in actions:
mapping.setdefault(action.name, set()).add(model_key)
return mapping
def get_permission_for_model(model, action):
"""
Resolve the named permission for a given model (or instance) and action (e.g. view or add).

14
netbox/utilities/templates/form_helpers/render_field.html
View File

@@ -2,19 +2,6 @@
{% load helpers %}
{% load i18n %}
{# RegisteredActionsWidget emits its own row markup per option #}
{% if field|widget_type == 'registeredactionswidget' %}
{{ field }}
{% if field.errors %}
<div class="row mb-3">
<div class="col offset-3">
<div class="form-text text-danger">
{% for error in field.errors %}{{ error }}{% if not forloop.last %}<br />{% endif %}{% endfor %}
</div>
</div>
</div>
{% endif %}
{% else %}
<div class="row mb-3{% if field.errors %} has-errors{% endif %}">
{# Render the field label (if any), except for checkboxes #}
@@ -83,4 +70,3 @@
</div>
</div>
{% endif %}

19
netbox/utilities/templates/widgets/registered_actions.html
View File

@@ -1,19 +0,0 @@
{# optgroups is Django's CheckboxSelectMultiple context: list of (group_name, options, index) tuples #}
{% for group_name, options, index in widget.optgroups %}{% for option in options %}
<div class="row mb-3">
<div class="col offset-3">
<div class="form-check mb-0">
<input type="checkbox"
class="form-check-input"
name="{{ option.name }}"
value="{{ option.value }}"
id="{{ option.attrs.id }}"
data-models="{{ option.model_keys }}"
{% if option.selected %}checked{% endif %}>
<label class="form-check-label" for="{{ option.attrs.id }}">
{{ option.label }}{% if option.help_text %} <small class="text-muted ms-1">{{ option.help_text }}</small>{% endif %}
</label>
</div>
</div>
</div>
{% endfor %}{% endfor %}

62
netbox/utilities/tests/test_permissions.py
View File

@@ -98,21 +98,14 @@ class RegisterModelActionsTest(TestCase):
with self.assertRaises(ValueError):
register_model_actions(Site, [ModelAction('')])
def test_flat_choices_no_duplicates(self):
"""Same action name registered for multiple models should appear once in flat choices."""
def test_no_duplicate_action_fields(self):
"""Same action name registered for multiple models should produce only one form field."""
register_model_actions(Device, [ModelAction('render_config')])
register_model_actions(VirtualMachine, [ModelAction('render_config')])
device_ct = ObjectType.objects.get_for_model(Device)
form = ObjectPermissionForm(data={
'name': 'test',
'object_types_0': [],
'object_types_1': [device_ct.pk],
'can_view': True,
})
choices = form.fields['registered_actions'].choices
names = [name for name, label in choices]
self.assertEqual(names.count('render_config'), 1)
form = ObjectPermissionForm()
action_fields = [k for k in form.fields if k.startswith('action_')]
self.assertEqual(action_fields.count('action_render_config'), 1)
class ObjectPermissionFormTest(TestCase):
@@ -126,7 +119,7 @@ class ObjectPermissionFormTest(TestCase):
registry['model_actions'].update(self._original_actions)
def test_shared_action_preselection(self):
"""Editing a permission with render_config should pre-select 'render_config' (plain name)."""
"""Editing a permission with render_config should pre-select the action_render_config checkbox."""
register_model_actions(Device, [ModelAction('render_config')])
register_model_actions(VirtualMachine, [ModelAction('render_config')])
@@ -141,13 +134,7 @@ class ObjectPermissionFormTest(TestCase):
form = ObjectPermissionForm(instance=permission)
initial = form.fields['registered_actions'].initial
self.assertIn('render_config', initial)
# Should not use the old model-prefixed format
self.assertNotIn('dcim.device.render_config', initial)
self.assertNotIn('virtualization.virtualmachine.render_config', initial)
# Should only appear once despite being registered for two models
self.assertEqual(initial.count('render_config'), 1)
self.assertTrue(form.fields['action_render_config'].initial)
# Should not leak into the additional actions field
self.assertEqual(form.initial['actions'], [])
@@ -155,7 +142,7 @@ class ObjectPermissionFormTest(TestCase):
permission.delete()
def test_clean_accepts_valid_registered_action(self):
"""clean() should accept a plain action name when the matching object type is selected."""
"""clean() should include checked registered action in saved actions."""
register_model_actions(Device, [ModelAction('render_config')])
device_ct = ObjectType.objects.get_for_model(Device)
@@ -163,25 +150,30 @@ class ObjectPermissionFormTest(TestCase):
'name': 'test perm',
'object_types_0': [],
'object_types_1': [device_ct.pk],
'registered_actions': ['render_config'],
'action_render_config': True,
'can_view': True,
'actions': '',
})
self.assertTrue(form.is_valid(), form.errors)
self.assertIn('render_config', form.cleaned_data['actions'])
def test_clean_rejects_action_without_matching_object_type(self):
"""clean() should reject a registered action when no matching object type is selected."""
def test_get_registered_actions(self):
"""ObjectPermission.get_registered_actions() should return tuples of (name, is_enabled, models)."""
register_model_actions(Device, [ModelAction('render_config')])
register_model_actions(VirtualMachine, [ModelAction('render_config')])
site_ct = ObjectType.objects.get_for_model(Site)
form = ObjectPermissionForm(data={
'name': 'test perm',
'object_types_0': [],
'object_types_1': [site_ct.pk],
'registered_actions': ['render_config'],
'can_view': True,
'actions': '',
})
self.assertFalse(form.is_valid())
self.assertIn('registered_actions', form.errors)
device_ct = ObjectType.objects.get_for_model(Device)
permission = ObjectPermission.objects.create(
name='Test Registered Actions',
actions=['view', 'render_config'],
)
permission.object_types.set([device_ct])
registered = permission.get_registered_actions()
self.assertEqual(len(registered), 1)
action_name, is_enabled, models_csv = registered[0]
self.assertEqual(action_name, 'render_config')
self.assertTrue(is_enabled)
permission.delete()