From 6e5d9762be25820779ead2a99b4ce42dbb82a807 Mon Sep 17 00:00:00 2001 From: seanhelvey Date: Fri, 13 Dec 2024 11:56:53 -0800 Subject: [PATCH] Fixed #13883 -- Added optgroups for SelectBox in Admin (updated) --- AUTHORS | 1 + django/contrib/admin/options.py | 43 ++++++++++++++++--- .../admin/static/admin/js/SelectBox.js | 37 ++++++++++------ .../admin/js/admin/RelatedObjectLookups.js | 7 +-- .../admin/static/admin/js/popup_response.js | 2 +- .../admin/templates/admin/change_form.html | 1 + django/contrib/admin/views/main.py | 2 + django/contrib/admin/widgets.py | 10 ++++- js_tests/admin/SelectBox.test.js | 13 ++++++ js_tests/admin/SelectFilter2.test.js | 42 +++++++++--------- tests/admin_widgets/tests.py | 39 +++++++++-------- 11 files changed, 134 insertions(+), 63 deletions(-) diff --git a/AUTHORS b/AUTHORS index 5e7bca67f5..94b94458ee 100644 --- a/AUTHORS +++ b/AUTHORS @@ -952,6 +952,7 @@ answer newbie questions, and generally made Django that much better: Scott Pashley scott@staplefish.com Sean Brant + Sean Helvey Sebastian Hillig Sebastian Spiegel Segyo Myung diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 6c202c8e61..d62d656144 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -8,6 +8,7 @@ from urllib.parse import quote as urlquote from urllib.parse import urlsplit from django import forms +from django.apps import apps from django.conf import settings from django.contrib import messages from django.contrib.admin import helpers, widgets @@ -71,6 +72,7 @@ from django.views.decorators.csrf import csrf_protect from django.views.generic import RedirectView IS_POPUP_VAR = "_popup" +SOURCE_MODEL_VAR = "_source_model" TO_FIELD_VAR = "_to_field" IS_FACETS_VAR = "_facets" @@ -1342,6 +1344,7 @@ class ModelAdmin(BaseModelAdmin): "save_on_top": self.save_on_top, "to_field_var": TO_FIELD_VAR, "is_popup_var": IS_POPUP_VAR, + "source_model_var": SOURCE_MODEL_VAR, "app_label": app_label, } ) @@ -1398,12 +1401,39 @@ class ModelAdmin(BaseModelAdmin): else: attr = obj._meta.pk.attname value = obj.serializable_value(attr) - popup_response_data = json.dumps( - { - "value": str(value), - "obj": str(obj), - } - ) + popup_response = { + "value": str(value), + "obj": str(obj), + } + + # Find the optgroup for the new item, if available + source_model_name = request.POST.get(SOURCE_MODEL_VAR) + + if source_model_name: + if "." not in source_model_name: + raise ValueError(f"Invalid model format: {source_model_name}") + app_label, model_name = source_model_name.split(".", 1) + try: + source_model = apps.get_model(app_label, model_name) + except LookupError: + raise LookupError(f"No installed app/model: {source_model_name}") + form_class = self.admin_site._registry[source_model].form + if ( + hasattr(form_class, "_meta") + and hasattr(form_class._meta, "fields") + and form_class._meta.fields is not None + and self.opts.verbose_name_plural in form_class._meta.fields + ): + field_choices = ( + form_class().fields[self.opts.verbose_name_plural].choices + ) + + for optgroup_label, optgroup_choices in field_choices: + for choice_value, choice_display in optgroup_choices: + if choice_display == str(obj): + popup_response["optgroup"] = optgroup_label + + popup_response_data = json.dumps(popup_response) return TemplateResponse( request, self.popup_response_template @@ -1913,6 +1943,7 @@ class ModelAdmin(BaseModelAdmin): "object_id": object_id, "original": obj, "is_popup": IS_POPUP_VAR in request.POST or IS_POPUP_VAR in request.GET, + "source_model": request.GET.get(SOURCE_MODEL_VAR), "to_field": to_field, "media": media, "inline_admin_formsets": inline_formsets, diff --git a/django/contrib/admin/static/admin/js/SelectBox.js b/django/contrib/admin/static/admin/js/SelectBox.js index 3db4ec7fa6..eca56c18db 100644 --- a/django/contrib/admin/static/admin/js/SelectBox.js +++ b/django/contrib/admin/static/admin/js/SelectBox.js @@ -1,5 +1,6 @@ 'use strict'; { + const getOptionGroupName = (option) => option.parentElement.label; const SelectBox = { cache: {}, init: function(id) { @@ -7,20 +8,29 @@ SelectBox.cache[id] = []; const cache = SelectBox.cache[id]; for (const node of box.options) { - cache.push({value: node.value, text: node.text, displayed: 1}); + const group = getOptionGroupName(node); + cache.push({group, value: node.value, text: node.text, displayed: 1}); } + SelectBox.sort(id); }, redisplay: function(id) { // Repopulate HTML select box from cache const box = document.getElementById(id); const scroll_value_from_top = box.scrollTop; box.innerHTML = ''; - for (const node of SelectBox.cache[id]) { - if (node.displayed) { - const new_option = new Option(node.text, node.value, false, false); - // Shows a tooltip when hovering over the option - new_option.title = node.text; - box.appendChild(new_option); + let node = box; + let group = null; + for (const option of SelectBox.cache[id]) { + if (option.group && option.group !== group && option.displayed) { + group = option.group; + node = document.createElement('optgroup'); + node.setAttribute('label', option.group); + box.appendChild(node); + } + if (option.displayed) { + const new_option = new Option(option.text, option.value, false, false); + new_option.title = option.text; + node.appendChild(new_option); } } box.scrollTop = scroll_value_from_top; @@ -57,7 +67,8 @@ cache.splice(delete_index, 1); }, add_to_cache: function(id, option) { - SelectBox.cache[id].push({value: option.value, text: option.text, displayed: 1}); + SelectBox.cache[id].push({group: option.group, value: option.value, text: option.text, displayed: 1}); + SelectBox.sort(id); }, cache_contains: function(id, value) { // Check if an item is contained in the cache @@ -73,7 +84,8 @@ for (const option of from_box.options) { const option_value = option.value; if (option.selected && SelectBox.cache_contains(from, option_value)) { - SelectBox.add_to_cache(to, {value: option_value, text: option.text, displayed: 1}); + const group = getOptionGroupName(option); + SelectBox.add_to_cache(to, {group, value: option_value, text: option.text, displayed: 1}); SelectBox.delete_from_cache(from, option_value); } } @@ -85,7 +97,8 @@ for (const option of from_box.options) { const option_value = option.value; if (SelectBox.cache_contains(from, option_value)) { - SelectBox.add_to_cache(to, {value: option_value, text: option.text, displayed: 1}); + const group = getOptionGroupName(option); + SelectBox.add_to_cache(to, {group, value: option_value, text: option.text, displayed: 1}); SelectBox.delete_from_cache(from, option_value); } } @@ -94,8 +107,8 @@ }, sort: function(id) { SelectBox.cache[id].sort(function(a, b) { - a = a.text.toLowerCase(); - b = b.text.toLowerCase(); + a = (a.group && a.group.toLowerCase() || '') + a.text.toLowerCase(); + b = (b.group && b.group.toLowerCase() || '') + b.text.toLowerCase(); if (a > b) { return 1; } diff --git a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js index 1fc03c6232..09a6e20d29 100644 --- a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js +++ b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js @@ -123,7 +123,7 @@ }); } - function dismissAddRelatedObjectPopup(win, newId, newRepr) { + function dismissAddRelatedObjectPopup(win, newId, newRepr, optgroup) { const name = removePopupIndex(win.name); const elem = document.getElementById(name); if (elem) { @@ -143,8 +143,9 @@ } else { const toId = name + "_to"; const toElem = document.getElementById(toId); - const o = new Option(newRepr, newId); - SelectBox.add_to_cache(toId, o); + const newOption = new Option(newRepr, newId); + newOption.group = optgroup; + SelectBox.add_to_cache(toId, newOption); SelectBox.redisplay(toId); if (toElem && toElem.nodeName.toUpperCase() === 'SELECT') { const skipIds = [name + "_from"]; diff --git a/django/contrib/admin/static/admin/js/popup_response.js b/django/contrib/admin/static/admin/js/popup_response.js index fecf0f4798..bdd93a6eb5 100644 --- a/django/contrib/admin/static/admin/js/popup_response.js +++ b/django/contrib/admin/static/admin/js/popup_response.js @@ -9,7 +9,7 @@ opener.dismissDeleteRelatedObjectPopup(window, initData.value); break; default: - opener.dismissAddRelatedObjectPopup(window, initData.value, initData.obj); + opener.dismissAddRelatedObjectPopup(window, initData.value, initData.obj, initData.optgroup); break; } } diff --git a/django/contrib/admin/templates/admin/change_form.html b/django/contrib/admin/templates/admin/change_form.html index f6edffb4d4..7116f1b8b8 100644 --- a/django/contrib/admin/templates/admin/change_form.html +++ b/django/contrib/admin/templates/admin/change_form.html @@ -38,6 +38,7 @@
{% if is_popup %}{% endif %} {% if to_field %}{% endif %} +{% if source_model %}{% endif %} {% if save_on_top %}{% block submit_buttons_top %}{% submit_row %}{% endblock %}{% endif %} {% if errors %}

diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 8c9118808e..40652f7100 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -11,6 +11,7 @@ from django.contrib.admin.exceptions import ( from django.contrib.admin.options import ( IS_FACETS_VAR, IS_POPUP_VAR, + SOURCE_MODEL_VAR, TO_FIELD_VAR, IncorrectLookupParameters, ShowFacets, @@ -49,6 +50,7 @@ IGNORED_PARAMS = ( SEARCH_VAR, IS_FACETS_VAR, IS_POPUP_VAR, + SOURCE_MODEL_VAR, TO_FIELD_VAR, ) diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index 81b57f33aa..c420707ca6 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -332,16 +332,24 @@ class RelatedFieldWidgetWrapper(forms.Widget): ) def get_context(self, name, value, attrs): - from django.contrib.admin.views.main import IS_POPUP_VAR, TO_FIELD_VAR + from django.contrib.admin.views.main import ( + IS_POPUP_VAR, + SOURCE_MODEL_VAR, + TO_FIELD_VAR, + ) rel_opts = self.rel.model._meta info = (rel_opts.app_label, rel_opts.model_name) related_field_name = self.rel.get_related_field().name + app_label = self.rel.field.model._meta.app_label + model_name = self.rel.field.model._meta.model_name + url_params = "&".join( "%s=%s" % param for param in [ (TO_FIELD_VAR, related_field_name), (IS_POPUP_VAR, 1), + (SOURCE_MODEL_VAR, f"{app_label}.{model_name}"), ] ) context = { diff --git a/js_tests/admin/SelectBox.test.js b/js_tests/admin/SelectBox.test.js index 7d127b5d59..0856385875 100644 --- a/js_tests/admin/SelectBox.test.js +++ b/js_tests/admin/SelectBox.test.js @@ -45,3 +45,16 @@ QUnit.test('preserve scroll position', function(assert) { assert.equal(toSelectBox.options.length, selectedOptions.length); assert.notEqual(fromSelectBox.scrollTop, 0); }); + +QUnit.test('retain optgroups', function(assert) { + const $ = django.jQuery; + $('').appendTo('#qunit-fixture'); + const grp = $('').appendTo('#id'); + $('').appendTo(grp); + $('').appendTo('#id'); + $('').appendTo('#id'); + SelectBox.init('id'); + SelectBox.redisplay('id'); + assert.equal($('#id option').length, 2); + assert.equal($('#id optgroup').length, 1); +}); \ No newline at end of file diff --git a/js_tests/admin/SelectFilter2.test.js b/js_tests/admin/SelectFilter2.test.js index 9a020d2c53..ed454d3894 100644 --- a/js_tests/admin/SelectFilter2.test.js +++ b/js_tests/admin/SelectFilter2.test.js @@ -44,9 +44,9 @@ QUnit.test('init', function(assert) { QUnit.test('filtering available options', function(assert) { const $ = django.jQuery; $('

').appendTo('#qunit-fixture'); - $('').appendTo('#select'); - $('').appendTo('#select'); - $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); SelectFilter.init('select', 'items', 0); assert.equal($('#select_from option').length, 3); assert.equal($('#select_to option').length, 0); @@ -58,7 +58,7 @@ QUnit.test('filtering available options', function(assert) { setTimeout(() => { assert.equal($('#select_from option').length, 2); assert.equal($('#select_to option').length, 0); - assert.equal($('#select_from option')[0].value, '1'); + assert.equal($('#select_from option')[0].value, '2'); assert.equal($('#select_from option')[1].value, '3'); done(); }); @@ -67,9 +67,9 @@ QUnit.test('filtering available options', function(assert) { QUnit.test('filtering selected options', function(assert) { const $ = django.jQuery; $('
').appendTo('#qunit-fixture'); - $('').appendTo('#select'); - $('').appendTo('#select'); - $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); SelectFilter.init('select', 'items', 0); assert.equal($('#select_from option').length, 0); assert.equal($('#select_to option').length, 3); @@ -81,7 +81,7 @@ QUnit.test('filtering selected options', function(assert) { setTimeout(() => { assert.equal($('#select_from option').length, 0); assert.equal($('#select_to option').length, 2); - assert.equal($('#select_to option')[0].value, '1'); + assert.equal($('#select_to option')[0].value, '2'); assert.equal($('#select_to option')[1].value, '3'); done(); }); @@ -90,9 +90,9 @@ QUnit.test('filtering selected options', function(assert) { QUnit.test('filtering available options to nothing', function(assert) { const $ = django.jQuery; $('
').appendTo('#qunit-fixture'); - $('').appendTo('#select'); - $('').appendTo('#select'); - $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); SelectFilter.init('select', 'items', 0); assert.equal($('#select_from option').length, 3); assert.equal($('#select_to option').length, 0); @@ -111,9 +111,9 @@ QUnit.test('filtering available options to nothing', function(assert) { QUnit.test('filtering selected options to nothing', function(assert) { const $ = django.jQuery; $('
').appendTo('#qunit-fixture'); - $('').appendTo('#select'); - $('').appendTo('#select'); - $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); SelectFilter.init('select', 'items', 0); assert.equal($('#select_from option').length, 0); assert.equal($('#select_to option').length, 3); @@ -132,9 +132,9 @@ QUnit.test('filtering selected options to nothing', function(assert) { QUnit.test('selecting option', function(assert) { const $ = django.jQuery; $('
').appendTo('#qunit-fixture'); - $('').appendTo('#select'); - $('').appendTo('#select'); - $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); SelectFilter.init('select', 'items', 0); assert.equal($('#select_from option').length, 3); assert.equal($('#select_to option').length, 0); @@ -154,13 +154,13 @@ QUnit.test('selecting option', function(assert) { QUnit.test('deselecting option', function(assert) { const $ = django.jQuery; $('
').appendTo('#qunit-fixture'); - $('').appendTo('#select'); - $('').appendTo('#select'); - $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); + $('').appendTo('#select'); SelectFilter.init('select', 'items', 0); assert.equal($('#select_from option').length, 2); assert.equal($('#select_to option').length, 1); - assert.equal($('#select_to option')[0].value, '1'); + assert.equal($('#select_to option')[0].value, '3'); // move back to the left const done_left = assert.async(); $('#select_to')[0].selectedIndex = 0; diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index 7588c2cc32..abe7fdc531 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -957,6 +957,7 @@ class RelatedFieldWidgetWrapperTests(SimpleTestCase): self.assertIn(" + href="/admin_widgets/releaseevent/add/?_to_field=album&_popup=1&_source_model=admin_widgets.videostream">
@@ -1354,14 +1355,14 @@ class HorizontalVerticalFilterSeleniumTests(AdminWidgetSeleniumTestCase): self.assertSelectOptions( to_box, [ - str(self.lisa.id), - str(self.peter.id), str(self.arthur.id), str(self.bob.id), str(self.cliff.id), str(self.jason.id), str(self.jenny.id), str(self.john.id), + str(self.lisa.id), + str(self.peter.id), ], ) self.assertButtonsDisabled( @@ -1387,14 +1388,14 @@ class HorizontalVerticalFilterSeleniumTests(AdminWidgetSeleniumTestCase): self.assertSelectOptions( from_box, [ - str(self.lisa.id), - str(self.peter.id), str(self.arthur.id), str(self.bob.id), str(self.cliff.id), str(self.jason.id), str(self.jenny.id), str(self.john.id), + str(self.lisa.id), + str(self.peter.id), ], ) self.assertSelectOptions(to_box, []) @@ -1443,19 +1444,19 @@ class HorizontalVerticalFilterSeleniumTests(AdminWidgetSeleniumTestCase): self.assertSelectOptions( from_box, [ - str(self.peter.id), str(self.arthur.id), str(self.cliff.id), str(self.jenny.id), + str(self.peter.id), ], ) self.assertSelectOptions( to_box, [ - str(self.lisa.id), str(self.bob.id), str(self.jason.id), str(self.john.id), + str(self.lisa.id), ], ) @@ -1492,12 +1493,12 @@ class HorizontalVerticalFilterSeleniumTests(AdminWidgetSeleniumTestCase): self.assertSelectOptions( from_box, [ - str(self.peter.id), str(self.arthur.id), + str(self.bob.id), str(self.cliff.id), str(self.jenny.id), str(self.lisa.id), - str(self.bob.id), + str(self.peter.id), ], ) self.assertSelectOptions(to_box, [str(self.jason.id), str(self.john.id)]) @@ -1510,19 +1511,19 @@ class HorizontalVerticalFilterSeleniumTests(AdminWidgetSeleniumTestCase): self.assertSelectOptions( from_box, [ - str(self.peter.id), + str(self.bob.id), str(self.jenny.id), str(self.lisa.id), - str(self.bob.id), + str(self.peter.id), ], ) self.assertSelectOptions( to_box, [ - str(self.jason.id), - str(self.john.id), str(self.arthur.id), str(self.cliff.id), + str(self.jason.id), + str(self.john.id), ], ) @@ -1532,9 +1533,9 @@ class HorizontalVerticalFilterSeleniumTests(AdminWidgetSeleniumTestCase): # Confirm they're selected after clicking inactive buttons: ticket # #26575 - self.assertSelectedOptions(from_box, [str(self.peter.id), str(self.lisa.id)]) + self.assertSelectedOptions(from_box, [str(self.lisa.id), str(self.peter.id)]) self.selenium.find_element(By.ID, remove_button).click() - self.assertSelectedOptions(from_box, [str(self.peter.id), str(self.lisa.id)]) + self.assertSelectedOptions(from_box, [str(self.lisa.id), str(self.peter.id)]) # Unselect the options ------------------------------------------------ self.deselect_option(from_box, str(self.peter.id)) @@ -1661,9 +1662,9 @@ class HorizontalVerticalFilterSeleniumTests(AdminWidgetSeleniumTestCase): self.assertSelectOptions( to_box, [ + str(self.jason.id), str(self.lisa.id), str(self.peter.id), - str(self.jason.id), ], ) @@ -1673,7 +1674,7 @@ class HorizontalVerticalFilterSeleniumTests(AdminWidgetSeleniumTestCase): from_box, [str(self.arthur.id), str(self.lisa.id)] ) self.assertSelectOptions( - to_box, [str(self.peter.id), str(self.jason.id)] + to_box, [str(self.jason.id), str(self.peter.id)] ) input.send_keys([Keys.BACK_SPACE]) # Clear text box @@ -1689,7 +1690,7 @@ class HorizontalVerticalFilterSeleniumTests(AdminWidgetSeleniumTestCase): ], ) self.assertSelectOptions( - to_box, [str(self.peter.id), str(self.jason.id)] + to_box, [str(self.jason.id), str(self.peter.id)] ) # Pressing enter on a filtered option sends it properly to @@ -1700,7 +1701,7 @@ class HorizontalVerticalFilterSeleniumTests(AdminWidgetSeleniumTestCase): self.assertSelectOptions(from_box, [str(self.jason.id)]) input.send_keys([Keys.ENTER]) self.assertSelectOptions( - to_box, [str(self.peter.id), str(self.jason.id)] + to_box, [str(self.jason.id), str(self.peter.id)] ) input.send_keys([Keys.BACK_SPACE, Keys.BACK_SPACE])