Skip to content

Commit 57c8c8b

Browse files
Karolis Ryselisjacobtylerwalls
authored andcommitted
Fixed #36984 -- Made inline formset error messages respect delete_confirmation_max_display.
1 parent a2348c8 commit 57c8c8b

10 files changed

Lines changed: 150 additions & 34 deletions

File tree

AUTHORS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,7 @@ answer newbie questions, and generally made Django that much better:
600600
Karderio <karderio@gmail.com>
601601
Karen Tracey <kmtracey@gmail.com>
602602
Karol Sikora <elektrrrus@gmail.com>
603+
Karolis Ryselis <karolis.ryselis@gmail.com>
603604
Kasey Steinhauer <kstein257@gmail.com>
604605
Kasun Herath <kasunh01@gmail.com>
605606
Katherine “Kati” Michel <kthrnmichel@gmail.com>

django/contrib/admin/checks.py

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ def check(self, admin_obj, **kwargs):
190190
*self._check_view_on_site_url(admin_obj),
191191
*self._check_ordering(admin_obj),
192192
*self._check_readonly_fields(admin_obj),
193+
*self._check_delete_confirmation_max_display(admin_obj),
193194
]
194195

195196
def _check_autocomplete_fields(self, obj):
@@ -824,6 +825,25 @@ def _check_readonly_fields_item(self, obj, field_name, label):
824825
else:
825826
return []
826827

828+
def _check_delete_confirmation_max_display(self, obj):
829+
"""Check that delete_confirmation_max_display is
830+
a non-negative integer or None."""
831+
832+
if obj.delete_confirmation_max_display is None:
833+
return []
834+
if (
835+
not isinstance(obj.delete_confirmation_max_display, int)
836+
or obj.delete_confirmation_max_display < 0
837+
):
838+
return must_be(
839+
"a non-negative integer or None",
840+
option="delete_confirmation_max_display",
841+
obj=obj,
842+
id="admin.E041",
843+
)
844+
else:
845+
return []
846+
827847

828848
class ModelAdminChecks(BaseModelAdminChecks):
829849
def check(self, admin_obj, **kwargs):
@@ -842,7 +862,6 @@ def check(self, admin_obj, **kwargs):
842862
*self._check_search_fields(admin_obj),
843863
*self._check_date_hierarchy(admin_obj),
844864
*self._check_actions(admin_obj),
845-
*self._check_delete_confirmation_max_display(admin_obj),
846865
]
847866

848867
def _check_save_as(self, obj):
@@ -1093,25 +1112,6 @@ def _check_list_select_related(self, obj):
10931112
else:
10941113
return []
10951114

1096-
def _check_delete_confirmation_max_display(self, obj):
1097-
"""Check that delete_confirmation_max_display is
1098-
a non-negative integer or None."""
1099-
1100-
if obj.delete_confirmation_max_display is None:
1101-
return []
1102-
if (
1103-
not isinstance(obj.delete_confirmation_max_display, int)
1104-
or obj.delete_confirmation_max_display < 0
1105-
):
1106-
return must_be(
1107-
"a non-negative integer or None",
1108-
option="delete_confirmation_max_display",
1109-
obj=obj,
1110-
id="admin.E131",
1111-
)
1112-
else:
1113-
return []
1114-
11151115
def _check_list_per_page(self, obj):
11161116
"""Check that list_per_page is an integer."""
11171117

django/contrib/admin/options.py

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import copy
22
import enum
3+
import itertools
34
import json
45
import re
6+
import sys
57
import warnings
68
from collections.abc import Callable
79
from dataclasses import dataclass
@@ -191,6 +193,7 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass):
191193
view_on_site = True
192194
show_full_result_count = True
193195
checks_class = BaseModelAdminChecks
196+
delete_confirmation_max_display = None
194197

195198
def check(self, **kwargs):
196199
return self.checks_class().check(self, **kwargs)
@@ -704,7 +707,6 @@ class ModelAdmin(BaseModelAdmin):
704707
add_form_template = None
705708
change_form_template = None
706709
change_list_template = None
707-
delete_confirmation_max_display = None
708710
delete_confirmation_template = None
709711
delete_selected_confirmation_template = None
710712
object_history_template = None
@@ -2755,6 +2757,11 @@ def get_formset(self, request, obj=None, **kwargs):
27552757
base_model_form = defaults["form"]
27562758
can_change = self.has_change_permission(request, obj) if request else True
27572759
can_add = self.has_add_permission(request, obj) if request else True
2760+
delete_confirmation_max_display = (
2761+
self.delete_confirmation_max_display
2762+
if self.delete_confirmation_max_display
2763+
else sys.maxsize
2764+
)
27582765

27592766
class DeleteProtectedModelForm(base_model_form):
27602767
def hand_clean_DELETE(self):
@@ -2771,7 +2778,10 @@ def hand_clean_DELETE(self):
27712778
collector.collect([self.instance])
27722779
if collector.protected:
27732780
objs = []
2774-
for p in collector.protected:
2781+
protected = itertools.islice(
2782+
collector.protected, delete_confirmation_max_display
2783+
)
2784+
for p in protected:
27752785
objs.append(
27762786
# Translators: Model verbose name and instance
27772787
# representation, suitable to be an item in a
@@ -2782,8 +2792,26 @@ def hand_clean_DELETE(self):
27822792
params = {
27832793
"class_name": self._meta.model._meta.verbose_name,
27842794
"instance": self.instance,
2785-
"related_objects": get_text_list(objs, _("and")),
27862795
}
2796+
remaining_object_count = (
2797+
len(collector.protected) - delete_confirmation_max_display
2798+
)
2799+
if remaining_object_count > 0:
2800+
# Translators: This string is used as a separator
2801+
# between list elements
2802+
related = (
2803+
_(", ").join(str(i) for i in objs)
2804+
+ _(", ")
2805+
+ ngettext(
2806+
"…and %(count)d more object.",
2807+
"…and %(count)d more objects.",
2808+
remaining_object_count,
2809+
)
2810+
% {"count": remaining_object_count}
2811+
)
2812+
else:
2813+
related = get_text_list(objs, _("and"))
2814+
params["related_objects"] = related
27872815
msg = _(
27882816
"Deleting %(class_name)s %(instance)s would require "
27892817
"deleting the following protected related objects: "

docs/ref/checks.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,8 @@ with the admin site:
759759
referenced by ``<modeladmin>.autocomplete_fields``.
760760
* **admin.E040**: ``<modeladmin>`` must define ``search_fields``, because
761761
it's referenced by ``<other_modeladmin>.autocomplete_fields``.
762+
* **admin.E041**: The value of ``delete_confirmation_max_display`` must be a
763+
non-negative integer or ``None``.
762764

763765
``ModelAdmin``
764766
~~~~~~~~~~~~~~
@@ -819,8 +821,6 @@ with the admin site:
819821
method for the ``<action>`` action.
820822
* **admin.E130**: ``__name__`` attributes of actions defined in
821823
``<modeladmin>`` must be unique. Name ``<name>`` is not unique.
822-
* **admin.E131**: The value of ``delete_confirmation_max_display`` must be a
823-
non-negative integer or ``None``.
824824

825825
``InlineModelAdmin``
826826
~~~~~~~~~~~~~~~~~~~~

docs/ref/contrib/admin/index.txt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,8 +1403,10 @@ default templates used by the :class:`ModelAdmin` views:
14031403
relationship hierarchy. This is purely a display setting and does not
14041404
affect the total number of objects retrieved from the database.
14051405

1406-
This applies to both :meth:`delete_view` and the ``delete_selected``
1407-
action. By default, this is ``None`` (no truncation).
1406+
This applies to :meth:`delete_view` and the ``delete_selected``
1407+
action if specified for ``ModelAdmin``, and to protected deletion
1408+
validation messages if specified for ``InlineModelAdmin``. By default,
1409+
this is ``None`` (no truncation).
14081410

14091411
.. attribute:: ModelAdmin.object_history_template
14101412

docs/releases/6.1.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,8 @@ Minor features
143143

144144
* The :attr:`~django.contrib.admin.ModelAdmin.delete_confirmation_max_display`
145145
option allows customizing how many objects are displayed on admin delete
146-
confirmation pages before the remainder is truncated. The default is
147-
``None`` (no truncation).
146+
confirmation pages and inline protected deletion errors before the remainder
147+
is truncated. The default is ``None`` (no truncation).
148148

149149
* In order to improve accessibility of the admin change forms:
150150

tests/admin_inlines/admin.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ def call_me(self, obj):
296296
class ChapterInline(admin.TabularInline):
297297
model = Chapter
298298
readonly_fields = ["call_me"]
299+
delete_confirmation_max_display = 3
299300

300301
def call_me(self, obj):
301302
return "Callable in ChapterInline"

tests/admin_inlines/models.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,7 @@ class Chapter(models.Model):
247247

248248

249249
class FootNote(models.Model):
250-
"""
251-
Model added for ticket 19838
252-
"""
250+
"""Model for models.PROTECT."""
253251

254252
chapter = models.ForeignKey(Chapter, models.PROTECT)
255253
note = models.CharField(max_length=40)

tests/admin_inlines/tests.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,6 +830,76 @@ def test_inlines_based_on_model_state(self):
830830
parent.refresh_from_db()
831831
self.assertIs(parent.show_inlines, True)
832832

833+
def test_delete_protected_message_limits_number_of_objects_displayed(self):
834+
# admin limits the number of displayed objects to 2, so we create
835+
# 5 footnotes.
836+
novel = Novel.objects.create()
837+
chapter = Chapter.objects.create(novel=novel)
838+
footnotes = [FootNote(chapter=chapter) for i in range(5)]
839+
FootNote.objects.bulk_create(footnotes)
840+
841+
response = self.client.post(
842+
reverse("admin:admin_inlines_novel_change", args=(novel.pk,)),
843+
data={
844+
"show_inlines": "on",
845+
"chapter_set-TOTAL_FORMS": "1",
846+
"chapter_set-INITIAL_FORMS": "1",
847+
"chapter_set-MAX_NUM_FORMS": "1000",
848+
"chapter_set-MIN_NUM_FORMS": "0",
849+
"chapter_set-0-id": chapter.id,
850+
"chapter_set-0-name": chapter.name,
851+
"chapter_set-0-novel": novel.id,
852+
"chapter_set-0-DELETE": "on",
853+
},
854+
)
855+
self.assertEqual(response.status_code, 200)
856+
inline_formset = response.context_data["inline_admin_formsets"][0]
857+
self.assertEqual(1, len(inline_formset.non_form_errors()))
858+
error_message = inline_formset.non_form_errors()[0]
859+
self.assertTrue(
860+
error_message.startswith(
861+
f"Deleting chapter Chapter object ({chapter.pk}) would "
862+
"require deleting the following protected related objects:"
863+
),
864+
error_message,
865+
)
866+
self.assertEqual(error_message.count("FootNote"), 3, error_message)
867+
self.assertTrue(error_message.endswith("…and 2 more objects."), error_message)
868+
869+
def test_delete_protected_message_does_not_limit_small_amount_of_objects(self):
870+
novel = Novel.objects.create()
871+
chapter = Chapter.objects.create(novel=novel)
872+
footnotes = [FootNote(chapter=chapter) for i in range(3)]
873+
FootNote.objects.bulk_create(footnotes)
874+
875+
response = self.client.post(
876+
reverse("admin:admin_inlines_novel_change", args=(novel.pk,)),
877+
data={
878+
"show_inlines": "on",
879+
"chapter_set-TOTAL_FORMS": "1",
880+
"chapter_set-INITIAL_FORMS": "1",
881+
"chapter_set-MAX_NUM_FORMS": "1000",
882+
"chapter_set-MIN_NUM_FORMS": "0",
883+
"chapter_set-0-id": chapter.id,
884+
"chapter_set-0-name": chapter.name,
885+
"chapter_set-0-novel": novel.id,
886+
"chapter_set-0-DELETE": "on",
887+
},
888+
)
889+
self.assertEqual(response.status_code, 200)
890+
inline_formset = response.context_data["inline_admin_formsets"][0]
891+
self.assertEqual(1, len(inline_formset.non_form_errors()))
892+
error_message = inline_formset.non_form_errors()[0]
893+
self.assertTrue(
894+
error_message.startswith(
895+
f"Deleting chapter Chapter object ({chapter.pk}) would require "
896+
"deleting the following protected related objects:"
897+
),
898+
error_message,
899+
)
900+
self.assertEqual(error_message.count("FootNote object"), 3)
901+
self.assertNotIn("more", error_message)
902+
833903

834904
@override_settings(ROOT_URLCONF="admin_inlines.urls")
835905
class TestInlineMedia(TestDataMixin, TestCase):

tests/modeladmin/test_checks.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,7 +1033,7 @@ class TestModelAdmin(ModelAdmin):
10331033
"'delete_confirmation_max_display'"
10341034
" must be a non-negative integer or None."
10351035
),
1036-
"admin.E131",
1036+
"admin.E041",
10371037
)
10381038

10391039
def test_negative_integer(self):
@@ -1048,7 +1048,7 @@ class TestModelAdmin(ModelAdmin):
10481048
"'delete_confirmation_max_display'"
10491049
" must be a non-negative integer or None."
10501050
),
1051-
"admin.E131",
1051+
"admin.E041",
10521052
)
10531053

10541054
def test_valid_case(self):
@@ -1063,6 +1063,22 @@ class TestModelAdmin(ModelAdmin):
10631063

10641064
self.assertIsValid(TestModelAdmin, ValidationTestModel)
10651065

1066+
def test_inline_not_integer(self):
1067+
class TestInlineModelAdmin(TabularInline):
1068+
delete_confirmation_max_display = "goodbye"
1069+
model = ValidationTestInlineModel
1070+
1071+
self.assertIsInvalid(
1072+
TestInlineModelAdmin,
1073+
ValidationTestModel,
1074+
(
1075+
"The value of "
1076+
"'delete_confirmation_max_display'"
1077+
" must be a non-negative integer or None."
1078+
),
1079+
"admin.E041",
1080+
)
1081+
10661082

10671083
class SearchFieldsCheckTests(CheckTestCase):
10681084
def test_not_iterable(self):

0 commit comments

Comments
 (0)