From b1ffa9a9d78b0c2c5ad6ed5a1d84e380d5cfd010 Mon Sep 17 00:00:00 2001 From: seanhelvey Date: Fri, 13 Dec 2024 11:56:53 -0800 Subject: Fixed #13883 -- Rendered named choice groups with in FilteredSelectMultiple. This patch adds support for s in FilteredSelectMultiple widgets. When a popup returns a new object, if the source field contains optgroup choices, the optgroup is now also included in the response data. Additionally, this adds error handling for invalid source_model parameters to prevent crashes and display user-friendly error messages instead. Co-authored-by: Michael McLarnon --- tests/admin_views/admin.py | 33 +++++++++++- tests/admin_views/forms.py | 63 +++++++++++++++++++++++ tests/admin_views/tests.py | 122 ++++++++++++++++++++++++++++++++++++++++++++- tests/admin_views/urls.py | 3 ++ 4 files changed, 219 insertions(+), 2 deletions(-) (limited to 'tests/admin_views') diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index 69570a8062..64e0e23229 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -18,7 +18,12 @@ from django.utils.html import format_html from django.utils.safestring import mark_safe from django.views.decorators.common import no_append_slash -from .forms import MediaActionForm +from .forms import ( + MediaActionForm, + SectionFormWithDynamicOptgroups, + SectionFormWithObjectOptgroups, + SectionFormWithOptgroups, +) from .models import ( Actor, AdminOrderedAdminMethod, @@ -1345,6 +1350,32 @@ site2.register(Language) site7 = admin.AdminSite(name="admin7") site7.register(Article, ArticleAdmin2) site7.register(Section) + + +# Admin for testing optgroup in popup response +class SectionAdminWithOptgroups(admin.ModelAdmin): + form = SectionFormWithOptgroups + + +class SectionAdminWithObjectOptgroups(admin.ModelAdmin): + form = SectionFormWithObjectOptgroups + + +class SectionAdminWithDynamicOptgroups(admin.ModelAdmin): + form = SectionFormWithDynamicOptgroups + + +site11 = admin.AdminSite(name="admin11") +site11.register(Article, ArticleAdmin2) +site11.register(Section, SectionAdminWithOptgroups) + +site12 = admin.AdminSite(name="admin12") +site12.register(Article, ArticleAdmin2) +site12.register(Section, SectionAdminWithObjectOptgroups) + +site13 = admin.AdminSite(name="admin13") +site13.register(Article, ArticleAdmin2) +site13.register(Section, SectionAdminWithDynamicOptgroups) site7.register(PrePopulatedPost, PrePopulatedPostReadOnlyAdmin) site7.register( Pizza, diff --git a/tests/admin_views/forms.py b/tests/admin_views/forms.py index 3a3566c10f..f15a5b3ac1 100644 --- a/tests/admin_views/forms.py +++ b/tests/admin_views/forms.py @@ -1,7 +1,10 @@ +from django import forms from django.contrib.admin.forms import AdminAuthenticationForm, AdminPasswordChangeForm from django.contrib.admin.helpers import ActionForm from django.core.exceptions import ValidationError +from .models import Section + class CustomAdminAuthenticationForm(AdminAuthenticationForm): class Media: @@ -23,3 +26,63 @@ class CustomAdminPasswordChangeForm(AdminPasswordChangeForm): class MediaActionForm(ActionForm): class Media: js = ["path/to/media.js"] + + +class SectionFormWithOptgroups(forms.ModelForm): + articles = forms.ChoiceField( + choices=[ + ("Published", [("1", "Test Article")]), + ("Draft", [("2", "Other Article")]), + ], + required=False, + ) + + class Meta: + model = Section + fields = ["name", "articles"] + + +class SectionFormWithObjectOptgroups(forms.ModelForm): + """Form with model instances as optgroup keys (tests str() conversion).""" + + articles = forms.ChoiceField(required=False) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # Use Section instances as optgroup keys + sections = Section.objects.all()[:2] + if sections: + self.fields["articles"].choices = [ + (sections[0], [("1", "Article 1")]), + ( + sections[1] if len(sections) > 1 else sections[0], + [("2", "Article 2")], + ), + ] + + class Meta: + model = Section + fields = ["name", "articles"] + + +class SectionFormWithDynamicOptgroups(forms.ModelForm): + """ + Form where the field with optgroups is added dynamically in __init__. + This tests that the implementation doesn't rely on accessing the + uninstantiated form class's _meta or fields, which wouldn't work here. + """ + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # Dynamically add a field with optgroups after instantiation. + self.fields["articles"] = forms.ChoiceField( + choices=[ + ("Category A", [("1", "Item 1"), ("2", "Item 2")]), + ("Category B", [("3", "Item 3"), ("4", "Item 4")]), + ], + required=False, + ) + + class Meta: + model = Section + fields = ["name"] diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index f7eaad659e..3377a6d441 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -11,7 +11,7 @@ from django.contrib import admin from django.contrib.admin import AdminSite, ModelAdmin from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME from django.contrib.admin.models import ADDITION, DELETION, LogEntry -from django.contrib.admin.options import TO_FIELD_VAR +from django.contrib.admin.options import SOURCE_MODEL_VAR, TO_FIELD_VAR from django.contrib.admin.templatetags.admin_urls import add_preserved_filters from django.contrib.admin.tests import AdminSeleniumTestCase from django.contrib.admin.utils import quote @@ -468,6 +468,126 @@ class AdminViewBasicTest(AdminViewBasicTestCase): response = self.client.post(reverse("admin:admin_views_article_add"), post_data) self.assertContains(response, "title with a new\\nline") + def test_popup_add_POST_with_valid_source_model(self): + """ + Popup add with a valid source_model returns a successful response. + """ + post_data = { + IS_POPUP_VAR: "1", + SOURCE_MODEL_VAR: "admin_views.section", + "title": "Test Article", + "content": "some content", + "date_0": "2010-09-10", + "date_1": "14:55:39", + } + response = self.client.post(reverse("admin:admin_views_article_add"), post_data) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "data-popup-response") + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 0) + + def test_popup_add_POST_with_optgroups(self): + """ + Popup add with source_model containing optgroup choices includes + the optgroup in the response. + """ + post_data = { + IS_POPUP_VAR: "1", + SOURCE_MODEL_VAR: "admin_views.section", + "title": "Test Article", + "content": "some content", + "date_0": "2010-09-10", + "date_1": "14:55:39", + } + response = self.client.post( + reverse("admin11:admin_views_article_add"), post_data + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, ""optgroup": "Published"") + + def test_popup_add_POST_without_optgroups(self): + """ + Popup add where source_model form exists but doesn't have the field + should work without crashing. + """ + post_data = { + IS_POPUP_VAR: "1", + SOURCE_MODEL_VAR: "admin_views.section", + "title": "Test Article 2", + "content": "some content", + "date_0": "2010-09-10", + "date_1": "14:55:39", + } + # Use regular admin (not admin11) where Section doesn't have optgroups. + response = self.client.post(reverse("admin:admin_views_article_add"), post_data) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "data-popup-response") + self.assertNotContains(response, ""optgroup"") + + def test_popup_add_POST_with_object_optgroups(self): + """ + Popup add with source_model containing optgroups where the optgroup + keys are model instances (not strings) still serialize to strings. + """ + post_data = { + IS_POPUP_VAR: "1", + SOURCE_MODEL_VAR: "admin_views.section", + "title": "Article 1", + "content": "some content", + "date_0": "2010-09-10", + "date_1": "14:55:39", + } + response = self.client.post( + reverse("admin12:admin_views_article_add"), post_data + ) + self.assertEqual(response.status_code, 200) + # Check that optgroup is in the response with str() of Section instance + # The form uses Section.objects.all()[:2] which includes cls.s1 + # ("Test section") as the first optgroup key (HTML encoded). + self.assertContains(response, ""optgroup": "Test section"") + + def test_popup_add_POST_with_dynamic_optgroups(self): + """ + Popup add with source_model where optgroup field is added dynamically + in __init__. This ensures the implementation doesn't rely on accessing + the uninstantiated form class's _meta or fields, but instead properly + instantiates the form with get_form(request)() to access field info. + """ + post_data = { + IS_POPUP_VAR: "1", + SOURCE_MODEL_VAR: "admin_views.section", + "title": "Item 1", + "content": "some content", + "date_0": "2010-09-10", + "date_1": "14:55:39", + } + response = self.client.post( + reverse("admin13:admin_views_article_add"), post_data + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, ""optgroup": "Category A"") + + def test_popup_add_POST_with_invalid_source_model(self): + """ + Popup add with an invalid source_model (non-existent app/model) + shows an error message instead of crashing. + """ + post_data = { + IS_POPUP_VAR: "1", + SOURCE_MODEL_VAR: "admin_views.nonexistent", + "title": "Test Article", + "content": "some content", + "date_0": "2010-09-10", + "date_1": "14:55:39", + } + response = self.client.post(reverse("admin:admin_views_article_add"), post_data) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "data-popup-response") + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertIn("admin_views.nonexistent", str(messages[0])) + self.assertIn("could not be found", str(messages[0])) + def test_basic_edit_POST(self): """ A smoke test to ensure POST on edit_view works. diff --git a/tests/admin_views/urls.py b/tests/admin_views/urls.py index c1e673d811..3c43b8721d 100644 --- a/tests/admin_views/urls.py +++ b/tests/admin_views/urls.py @@ -32,6 +32,9 @@ urlpatterns = [ ), path("test_admin/admin9/", admin.site9.urls), path("test_admin/admin10/", admin.site10.urls), + path("test_admin/admin11/", admin.site11.urls), + path("test_admin/admin12/", admin.site12.urls), + path("test_admin/admin13/", admin.site13.urls), path("test_admin/has_permission_admin/", custom_has_permission_admin.site.urls), path("test_admin/autocomplete_admin/", autocomplete_site.urls), # Shares the admin URL prefix. -- cgit v1.3