From 17e5184a113ca2fee1970b137a883770cd3b68d3 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Tue, 9 Dec 2025 07:43:29 -0600 Subject: [PATCH] Fixes #20759: Group object types by app in permission form (#20931) * Fixes #20759: Group object types by app in permission form Modified the ObjectPermissionForm to use optgroups for organizing object types by application. This shortens the display names (e.g., "permission" instead of "Authentication and Authorization | permission") while maintaining clear organization through visual grouping. Changes: - Updated get_object_types_choices() to return nested optgroup structure - Enhanced AvailableOptions and SelectedOptions widgets to handle optgroups - Modified TypeScript moveOptions to preserve optgroup structure - Added hover text showing full model names - Styled optgroups with bold, padded labels * Address PR feedback --- netbox/project-static/dist/netbox.css | Bin 557056 -> 557216 bytes netbox/project-static/dist/netbox.js | Bin 383019 -> 383471 bytes netbox/project-static/dist/netbox.js.map | Bin 1774920 -> 1777461 bytes .../project-static/src/buttons/moveOptions.ts | 63 +++++++++++++--- .../styles/transitional/_forms.scss | 14 ++++ netbox/users/forms/model_forms.py | 25 ++++++- netbox/utilities/forms/widgets/select.py | 51 +++++++++---- netbox/utilities/tests/test_forms.py | 69 ++++++++++++++++++ 8 files changed, 193 insertions(+), 29 deletions(-) diff --git a/netbox/project-static/dist/netbox.css b/netbox/project-static/dist/netbox.css index 19bcf14d617c56f793b8320976b2c078ac0b7736..001c8d1eae936dc23b4948a59151c575004775d0 100644 GIT binary patch delta 128 zcmZo@P+HKa)X>7%!qmdt!qURp!q&pRg`+D~qBu1tHMt}@w=}0DvmhrmRw2KjB)urV zbo#|qc8STGc8GH&73(JDr&Q`DrBDC&pHV@^+`ym~o370Kyz1!(9oa>g3lh_(C!S-H Po2)lQbo;?n4tps82WT)K delta 45 zcmZ3`sMOG))X>7%!qmdt!qURp!q&pRg`+E#MXw++eR|?KCb`LaQ$)8Xrg7Ly0RUci B4&nd+ diff --git a/netbox/project-static/dist/netbox.js b/netbox/project-static/dist/netbox.js index 095469011d2bef21a3decdf5e887cdd258b4bd2a..32c552b74ee4a72f81af0ecf58eca8504f283f44 100644 GIT binary patch delta 2733 zcmaJ@eNYtV8K2#E7r{jE0|i0l)9Ws`te|F8&K1E^cjZ&mByfYUaPQ&TpfJuB4ri2nslswKUyvt*yM7B8q7yEAE(3;8i>RnL_BgB=_t$M$p_r(nM>nm~Wu<`_>X@(h?_AGF&j(G>!TK)D0=rc|c=5kxoUR zdGh)U>P1Si+%u_{DKKB3ML8yeKCX~j2;0`uxzxXG2{_IJ1}n}*S<@()99vCE5Pf$w zwI*H;MTN}V4#j$^hWgqDU9q0}J@}?JP;&sWC9Bk$3%QEaf=wk53Y$-={xCK;QwZ=v zrKE^h*Mqtt)JlpqEYiF307h9Yz0pSZMjgEjGTf=7*TqBC>SlT_ShqLRhry}POCN%v zBY)gUXH&{RFP-O9q^hQkxY-m{!_Zm+oFc4RQ&lO3RNcT_CC~GM5*5lMu3S*ax?Z{# zqF;l!G=(o(>#3~wsP&SkR#566XbSmi%&nr2hM6aa`si*D`Mc;DfNRw*`szYZvrf?a zKpa0ouS!;=a#8a_W36H^WNN{H;ZaD&NgJY7dXl#6w(gvwH&0M_X-T;l)V)~GS4h`s z8r~-IyVLYcR*6Ki6!M4DbU#>vXY3;&u`~2e5R1>!v+@*&gIgnI6$mAw7BH}GmSSH} z$Ixww=+kt=Tn2;ewVSneoTYsWDEY_xQIR#9LIIkRt(Oo=rfH>>?E9KtY<-H_3 zjz=xousg?z_OWo*7Yon|P^T3l7zEj>nu|1$WYT;zg`A&<++c&}J>Y7+x(Kz|K3Ohw zltnU^O|nW+s`brMlx~|Rm!d|fOAI&40YTj8dte=QqY+>>>+o8X1d~PHu0vZuUFbm@ zXH0+`2KJ9$WJn{Kr6|+7xCy;$1K8b;)R5QAMPhwSb`Gsm8XL4>xVpREKYGJ%-vGT6&^__3L(Nv1t4@D?xk<5mzZaMDCm zZK+gSF_2SsRrT7HHC5}Xv&RB-Nh8Pm=xHt?D3$2C+AjL^U@M2kX8Q^t5f3*K@N$N< z#CT*Hhr<|6@R-efyek-qa6z*A31<32n4T6r7!GCgLeS>V64TVRrm%^*Y`@xs{n>)f z3s_V`Asq0I-Z&S8I}ef>-DqZ96#EUFNz%K~tQGK0&~a<94RgHk=w=)a?EzxZj{_~{ z2EbEbo9{3fX%#jUO?~Iobl$1r~=FhyAe=` zY#r}MNj8sP9sq9$P1}Rs2k((RXmKidSG|sAS@IDSXNQ6^0((*n3QxEB$rk{ADi_;S zze~qvSPx`|fqd(R&?TGWoo}J89FT|pjIIDPknT^>B~WWVLpksTBW+WdFG1CknWy3X zCGX}i!=P@n7cM-olRsL5@|&zk#siI zDxJw<_&Qm?oXze`1X(zrJ<>$UE7;E-pyY$s_OmwX4-c|O>>KVo#A4=to?s|6ng{a^hisuib}JhXCA@Y4 zGC0Jhke?5+HaFiGVjpK{858j+n`R{)WfvrXb>(~Pdi$UboMeY>05C<#WXm~rAw0*< zv3u{CEbn<1hEKNseSuA)$qQEsJi6_Yn3{W?A1Z>&_Oj?MO7D{XnkPazBW%!p8JI@Q>{(0Xak~rbiL=eMk3&$|Z3|!*f7=6;qD15x!zMagJi0}Rht}DUlIUFb zK7#ey*hrL2qN204Q46Rx+S`ND6CLZdB=B%&O9gzL9W{>;)fN=R*#4(+YW%H^>tMHM zYX>)hL<$RItJ~Xy_NeBY-=YSM_I8pe*sfc^?$sk#Krec9Ek;WGtL|bDvVYrhzbZ;G1>xKk!Yzbs2WLb89vvY1*Ga1df@urlGqaHcmw6 zAsTZ=25N`o>D!rTFCwOEGCGKWzMh4wU|>GxKr><4oHGsmn<2z;9%3+)oXAW=3O)S{ zlEM1HGid1uB^;A1`Rjjmt%3|8}mRkjRVbn^do1_e3IEL(oQ~g-aJz+ru6`qS~rOQItqbs*r7! z5~0htb2VW77T8T9V#zXhb)DN&C%bDTQum`o=(i8ms&;VD^YqvbZWBP!PA(JTTDX(D zJQKv6vs@Rz`)9d@sYI@{Yd%P<*&YfTS|}KC6PkUFg=m(a<4l&;{SUacDMXZKSK33m zPt}VE-O$g$ZKA*H=OzoJy*-D}tNmOYn9zr;1T^s>w+*25BQC3eSgp3Da!#={*RBO4 zs&164{*bOhwsY-%O^+D!ppktnS#!%roc|X{+4njwF`q&>$RWji8RJxrBh9qy9#?99 zA>ixml=?^DhFn;kT8O&@sP)BSya2>B2Zlya%th0%21v(E$K&bwr?Cq-_}vMr=FwTW zh3Vut@$Ut!*m7x3IZijfn}ahL{X{ul4g1oF3+Dk)7oGxap9|lH`DS)6!(*Vc=&4%# zJcu*hxG6IQd_>g1@Ii((YAMGS^I|Lh0}J4#HoP3nZ?@rYp^xdOF`N&8)?*oAm<2>-PoI*#Luz@Zbc`IGE!`Gat`D(x+vfKykEs2;RLVQ^%b z_+vJo?wrCK@&H4h;Y-lfbpGe~V-VAB;5?YVv}6MR4Twc){BzKIbn8=mABYRAe1kJn z(dGH7^T+D8vg#_jE!Ph3AieA1(|LFy6?((NFA;`$dfUT0X=^<{3n{d>o-YIVx}MjP zpnLN)A3amg!<%XD_40*m0H12$S&M$(z|R4;w2>c|3|Ou68oQM582lP`mkOTer6j-= z8~GPAI3-BG9^%u??+@}v9l##Cz>kCn*z6hL53x&Ae3_rT{|Ybczse`m zV(R;yz6_X_a0L7~JHU_OtX2#1Sw3tQ6;(uMP?SVC}j6lHlk63laP!fdBvi diff --git a/netbox/project-static/dist/netbox.js.map b/netbox/project-static/dist/netbox.js.map index 2429b2c4c0175f17448f1609497a625855c3ea96..6ea877d79c37b002590fe0accd1b40896fc82b9b 100644 GIT binary patch delta 3133 zcmb_e&2JM&6jvaSgbk3EINL-`89*XQS)8VTXoNyFUN6p<;{+#7Q?7Hi9 znouP7V8pRKb)<6Xg+n=1Jy1ox6fV#J2RI=01nR$0sStT}9o3!H#_F!>?rKxDx!O|Qb6Y9iJE)PO$4bmwwVXV0i+iYkdyE(FDgX5L_Y=Lqa?8Y8L>b@p^_jz+;#UbJkT=f=Bk2!v*ANS4tgt@}V(9jU+$xKeC&Sz#Od-}*XUj{aW z0FrZ-vp`&r&_im?bhiogN27DJ+m-vlUKQYiu$^x`ivjmM&Rx*w;~&pm5s= zB3o1De{b%1f5Z(6+0$1?&r6!cc!~Mc_dRRA?6ck;8`hd_$cZOehv8%H~N5%sB zBEV<3`Mg0Lgu15PRTg%a?*@1x&^0n7|Dgl!-dB$E-=8V5emDv`({X)FvaT~VVit=G z8fU@sSkAZCk+~PUVtneS1gFm~k z93&cnyIh3)w%mt#x9lvKp2bSM`+@QczxAiG$y=@}tvkreH?%t{w7fAe8&)m1T$0rC zpB^f)kB^gq3$Ia|6!#hL)>gZVHv!?c2>`Ta9DAK0-#A%9qp%6Yh#`J~krGY?Nd)PI zI>{jgGoZw5$hL1g_hN>G4kfJUZZzTi0dM^ALxu3i@x~5S)?UoQx_tXyL2^59d!X$7 zU#2&J=}#N@$xY?#YpCvgi9?62;Ut6>M4AtG%L3qO#A=<&n=y(8Z;j0wl!!?r5^){~ zqv*El!2FKb)ur4gXj~_R*#(_kw9OooQ$ZF$XlO!NQh~RSKZrMe5DUz;ARVDuSD)*b z9x1R|VeN&r57vHIZLr>f^)9Rfu-ak8HZDEt$lj=&(zO(&BZg*Bn$fb9&S*L{oP;() zXJ;gkOiy*x-u#gk5*Z*!7=Ur;+MwvIrl?8aG0JQeissK{*$h1k203LeOI&k1<44{FJ->iL9FpTu3} zES+O|vcFCzM{7c;&pzNUiPBk&af%l&#oC)hZL$XMw$u|np75D<#Xsp(-Wi~G0E-8) zvMUK*veaYkKK6s49_|S!_{nmD|7xkRZIZR*R|)=?rS`W#0APF;6!$Kv$(CZ|@$;ou zcw9@C5}YopN1Er+%*-eFoh9|`8=D=P5cSO>P7^n*v9V^rt^-Z_>hSez%POq*V0CU> zTkfh9PRkb>3_9^TJFJnz;)nS7!>6quI(Q;JEPr^~)&HTi#k24wCkJNziF|MHV^6!f bKB`G;V;pr|@x~F;6d$88yxlihR&cAM8ke*cjBg_2yv-}3UQmOJ+$~tQ zIa|mV?))CCIL9x}*@ph87(lmCcQY}3Llt3gd375}v`T~|+9cW~4oe)7=#c1?=#uEJ zu5O1D&q5Vu5|h$2$x=265eEntbTa;R(T5iHE7l)0yhgptEGE<7Zi6l-bPr!E< zFZ3n%A1tDZlpKT8F765@)hL>Vn=bBsu)uuDzHw~kQ#ECR{0Ws(iIXaE#n<~yRiUYC z{&i$bTisoU)yI{z-cgz)DP2-aGX>;}cyJ)6beSm~OHc6+wGcL`dl`6F#F3F%-&(R& zUM6Ce>dr^CFX3_#BqbC4MXSWJV3hEwU{0+eYeBArW5MaV@wC8^j(-$0@KoS|;J8v) zQ_wE(<^NM8^#7594+8fDM$F|MfhBq*j#hVsTsW+J?Tv&Noib1B*CysH+5N=mSI=QNK diff --git a/netbox/project-static/src/buttons/moveOptions.ts b/netbox/project-static/src/buttons/moveOptions.ts index a98bf732a..2b7e39a16 100644 --- a/netbox/project-static/src/buttons/moveOptions.ts +++ b/netbox/project-static/src/buttons/moveOptions.ts @@ -1,7 +1,7 @@ import { getElements } from '../util'; /** - * Move selected options from one select element to another. + * Move selected options from one select element to another, preserving optgroup structure. * * @param source Select Element * @param target Select Element @@ -9,14 +9,42 @@ import { getElements } from '../util'; function moveOption(source: HTMLSelectElement, target: HTMLSelectElement): void { for (const option of Array.from(source.options)) { if (option.selected) { - target.appendChild(option.cloneNode(true)); + // Check if option is inside an optgroup + const parentOptgroup = option.parentElement as HTMLElement; + + if (parentOptgroup.tagName === 'OPTGROUP') { + // Find or create matching optgroup in target + const groupLabel = parentOptgroup.getAttribute('label'); + let targetOptgroup = Array.from(target.children).find( + child => child.tagName === 'OPTGROUP' && child.getAttribute('label') === groupLabel, + ) as HTMLOptGroupElement; + + if (!targetOptgroup) { + // Create new optgroup in target + targetOptgroup = document.createElement('optgroup'); + targetOptgroup.setAttribute('label', groupLabel!); + target.appendChild(targetOptgroup); + } + + // Move option to target optgroup + targetOptgroup.appendChild(option.cloneNode(true)); + } else { + // Option is not in an optgroup, append directly + target.appendChild(option.cloneNode(true)); + } + option.remove(); + + // Clean up empty optgroups in source + if (parentOptgroup.tagName === 'OPTGROUP' && parentOptgroup.children.length === 0) { + parentOptgroup.remove(); + } } } } /** - * Move selected options of a select element up in order. + * Move selected options of a select element up in order, respecting optgroup boundaries. * * Adapted from: * @see https://www.tomred.net/css-html-js/reorder-option-elements-of-an-html-select.html @@ -27,14 +55,21 @@ function moveOptionUp(element: HTMLSelectElement): void { for (let i = 1; i < options.length; i++) { const option = options[i]; if (option.selected) { - element.removeChild(option); - element.insertBefore(option, element.options[i - 1]); + const parent = option.parentElement as HTMLElement; + const previousOption = element.options[i - 1]; + const previousParent = previousOption.parentElement as HTMLElement; + + // Only move if previous option is in the same parent (optgroup or select) + if (parent === previousParent) { + parent.removeChild(option); + parent.insertBefore(option, previousOption); + } } } } /** - * Move selected options of a select element down in order. + * Move selected options of a select element down in order, respecting optgroup boundaries. * * Adapted from: * @see https://www.tomred.net/css-html-js/reorder-option-elements-of-an-html-select.html @@ -43,12 +78,18 @@ function moveOptionUp(element: HTMLSelectElement): void { function moveOptionDown(element: HTMLSelectElement): void { const options = Array.from(element.options); for (let i = options.length - 2; i >= 0; i--) { - let option = options[i]; + const option = options[i]; if (option.selected) { - let next = element.options[i + 1]; - option = element.removeChild(option); - next = element.replaceChild(option, next); - element.insertBefore(next, option); + const parent = option.parentElement as HTMLElement; + const nextOption = element.options[i + 1]; + const nextParent = nextOption.parentElement as HTMLElement; + + // Only move if next option is in the same parent (optgroup or select) + if (parent === nextParent) { + const optionClone = parent.removeChild(option); + const nextClone = parent.replaceChild(optionClone, nextOption); + parent.insertBefore(nextClone, optionClone); + } } } } diff --git a/netbox/project-static/styles/transitional/_forms.scss b/netbox/project-static/styles/transitional/_forms.scss index 147b11b97..668f60e2a 100644 --- a/netbox/project-static/styles/transitional/_forms.scss +++ b/netbox/project-static/styles/transitional/_forms.scss @@ -32,3 +32,17 @@ form.object-edit { border: 1px solid $red; } } + +// Make optgroup labels sticky when scrolling through select elements +select[multiple] { + optgroup { + position: sticky; + top: 0; + background-color: var(--bs-body-bg); + font-style: normal; + font-weight: bold; + } + option { + padding-left: 0.5rem; + } +} diff --git a/netbox/users/forms/model_forms.py b/netbox/users/forms/model_forms.py index 25db67ea8..d5f5bbbc0 100644 --- a/netbox/users/forms/model_forms.py +++ b/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 @@ -283,10 +286,24 @@ class GroupForm(forms.ModelForm): def get_object_types_choices(): - return [ - (ot.pk, str(ot)) - for ot in ObjectType.objects.filter(OBJECTPERMISSION_OBJECT_TYPES).order_by('app_label', 'model') - ] + """ + Generate choices for object types grouped by app label using optgroups. + Returns nested structure: [(app_label, [(id, model_name), ...]), ...] + """ + app_label_map = { + app_config.label: app_config.verbose_name + for app_config in apps.get_app_configs() + } + choices_by_app = defaultdict(list) + + for ot in ObjectType.objects.filter(OBJECTPERMISSION_OBJECT_TYPES).order_by('app_label', 'model'): + app_label = app_label_map.get(ot.app_label, ot.app_label) + + model_class = ot.model_class() + model_name = model_class._meta.verbose_name if model_class else ot.model + choices_by_app[app_label].append((ot.pk, title(model_name))) + + return list(choices_by_app.items()) class ObjectPermissionForm(forms.ModelForm): diff --git a/netbox/utilities/forms/widgets/select.py b/netbox/utilities/forms/widgets/select.py index 7f4e9c87f..b3197a922 100644 --- a/netbox/utilities/forms/widgets/select.py +++ b/netbox/utilities/forms/widgets/select.py @@ -66,17 +66,45 @@ class SelectWithPK(forms.Select): option_template_name = 'widgets/select_option_with_pk.html' -class AvailableOptions(forms.SelectMultiple): +class SelectMultipleBase(forms.SelectMultiple): + """ + 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): + filtered_choices = [] + include_selected = self.include_selected + + for choice in self.choices: + if isinstance(choice[1], (list, tuple)): # optgroup + group_label, group_choices = choice + 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: # 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): - self.choices = [ - choice for choice in self.choices if str(choice[0]) in value - ] - value = [] # Clear selected choices - return super().optgroups(name, value, attrs) + 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')])