From caae4f34f6b0e2ea0073a13a8399d19352e75535 Mon Sep 17 00:00:00 2001 From: Marcelo Galigniana Date: Mon, 29 Aug 2022 23:19:37 -0300 Subject: [PATCH] Fixed #12090 -- Show admin actions on the edit pages too --- django/contrib/admin/options.py | 83 +++++++++---- .../contrib/admin/static/admin/css/base.css | 78 +++++++++++++ .../admin/static/admin/css/changelists.css | 62 ---------- .../admin/templates/admin/change_form.html | 11 +- .../contrib/admin/templatetags/admin_urls.py | 29 +++++ tests/admin_views/admin.py | 8 +- tests/admin_views/test_actions.py | 109 ++++++++++++++++++ 7 files changed, 290 insertions(+), 90 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index e8760c2931..678d664b0d 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -251,7 +251,7 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass): def get_field_queryset(self, db, db_field, request): """ If the ModelAdmin specifies ordering, the queryset should respect that - ordering. Otherwise don't specify the queryset, let the field decide + ordering. Otherwise don't specify the queryset, let the field decide (return None in that case). """ try: @@ -1086,7 +1086,7 @@ class ModelAdmin(BaseModelAdmin): def get_action_choices(self, request, default_choices=models.BLANK_CHOICE_DASH): """ - Return a list of choices for use in a form object. Each choice is a + Return a list of choices for use in a form object. Each choice is a tuple (name, description). """ choices = [] + default_choices @@ -1098,7 +1098,7 @@ class ModelAdmin(BaseModelAdmin): def get_action(self, action): """ Return a given action from a parameter, which can either be a callable, - or the name of a method on the ModelAdmin. Return is a tuple of + or the name of a method on the ModelAdmin. Return is a tuple of (callable, name, description). """ # If the action is a callable, just use it. @@ -1606,11 +1606,13 @@ class ModelAdmin(BaseModelAdmin): """ return self._response_post_save(request, obj) - def response_action(self, request, queryset): + def response_action(self, request, queryset, change): """ Handle an admin action. This is called if a request is POSTed to the - changelist; it returns an HttpResponse if the action was handled, and - None otherwise. + changelist or changeform; it returns an HttpResponse if the action was + handled, and None otherwise. + `change` is True if it's called from change_view; False if comes from + changelist_view or add_view. """ # There can be multiple action forms on the page (at the top @@ -1641,25 +1643,26 @@ class ModelAdmin(BaseModelAdmin): # If the form's valid we can handle the action. if action_form.is_valid(): action = action_form.cleaned_data["action"] - select_across = action_form.cleaned_data["select_across"] func = self.get_actions(request)[action][0] + if not change: + select_across = action_form.cleaned_data["select_across"] + # Get the list of selected PKs. If nothing's selected, we can't + # perform an action on it, so bail. Except we want to perform + # the action explicitly on all objects. + selected = request.POST.getlist(helpers.ACTION_CHECKBOX_NAME) + if not selected and not select_across: + # Reminder that something needs to be selected or nothing will + # happen + msg = _( + "Items must be selected in order to perform " + "actions on them. No items have been changed." + ) + self.message_user(request, msg, messages.WARNING) + return None - # Get the list of selected PKs. If nothing's selected, we can't - # perform an action on it, so bail. Except we want to perform - # the action explicitly on all objects. - selected = request.POST.getlist(helpers.ACTION_CHECKBOX_NAME) - if not selected and not select_across: - # Reminder that something needs to be selected or nothing will happen - msg = _( - "Items must be selected in order to perform " - "actions on them. No items have been changed." - ) - self.message_user(request, msg, messages.WARNING) - return None - - if not select_across: - # Perform the action only on the selected objects - queryset = queryset.filter(pk__in=selected) + if not select_across: + # Perform the action only on the selected objects + queryset = queryset.filter(pk__in=selected) response = func(self, request, queryset) @@ -1856,7 +1859,23 @@ class ModelAdmin(BaseModelAdmin): ModelForm = self.get_form( request, obj, change=not add, fields=flatten_fieldsets(fieldsets) ) + + actions = self.get_actions(request) if request.method == "POST": + if actions and request.POST.get("action", ""): + action_failed = False + response = self.response_action(request, obj, change=not add) + if response: + return response + else: + action_failed = True + + if action_failed: + # Redirect back to the changelist page to avoid resubmitting the + # form if the user refreshes the browser or uses the "No, take + # me back" button on the action confirmation page. + return HttpResponseRedirect(request.get_full_path()) + form = ModelForm(request.POST, request.FILES, instance=obj) formsets, inline_instances = self._create_formsets( request, @@ -1925,6 +1944,19 @@ class ModelAdmin(BaseModelAdmin): title = _("Change %s") else: title = _("View %s") + + # Build the action form and populate it with available actions. + if actions and not add: + action_form = self.action_form(auto_id=None) + action_choices = self.get_action_choices(request) + # Remove "delete" action; change view already has a button for + # that action. + action_choices.pop(1) + action_form.fields["action"].choices = action_choices + media += action_form.media + else: + action_form = None + context = { **self.admin_site.each_context(request), "title": title % self.opts.verbose_name, @@ -1935,6 +1967,7 @@ class ModelAdmin(BaseModelAdmin): "is_popup": IS_POPUP_VAR in request.POST or IS_POPUP_VAR in request.GET, "to_field": to_field, "media": media, + "action_form": action_form, "inline_admin_formsets": inline_formsets, "errors": helpers.AdminErrorList(form, formsets), "preserved_filters": self.get_preserved_filters(request), @@ -2033,7 +2066,7 @@ class ModelAdmin(BaseModelAdmin): ): if selected: response = self.response_action( - request, queryset=cl.get_queryset(request) + request, queryset=cl.get_queryset(request), change=False ) if response: return response @@ -2057,7 +2090,7 @@ class ModelAdmin(BaseModelAdmin): ): if selected: response = self.response_action( - request, queryset=cl.get_queryset(request) + request, queryset=cl.get_queryset(request), change=False ) if response: return response diff --git a/django/contrib/admin/static/admin/css/base.css b/django/contrib/admin/static/admin/css/base.css index 769195af13..97a09ec31f 100644 --- a/django/contrib/admin/static/admin/css/base.css +++ b/django/contrib/admin/static/admin/css/base.css @@ -1176,3 +1176,81 @@ a.deletelink:focus, a.deletelink:hover { color: var(--body-fg); background-color: var(--body-bg); } + +/* ACTIONS */ + +#changelist .actions, +#changeform .actions { + padding: 10px; + background: var(--body-bg); + border-top: none; + border-bottom: none; + line-height: 1.5rem; + color: var(--body-quiet-color); + width: 100%; +} + +#changelist .actions span.all, +#changelist .actions span.action-counter, +#changelist .actions span.clear, +#changelist .actions span.question + +#changeform .actions span.all, +#changeform .actions span.action-counter, +#changeform .actions span.clear, +#changeform .actions span.question { + font-size: 0.8125rem; + margin: 0 0.5em; +} + +#changelist .actions:last-child, +#changeform .actions:last-child { + border-bottom: none; +} + +#changelist .actions select, +#changeform .actions select { + vertical-align: top; + height: 1.5rem; + color: var(--body-fg); + border: 1px solid var(--border-color); + border-radius: 4px; + font-size: 0.875rem; + padding: 0 0 0 4px; + margin: 0; + margin-left: 10px; +} + +#changelist .actions select:focus, +#changeform .actions select:focus { + border-color: var(--body-quiet-color); +} + +#changelist .actions label, +#changeform .actions label { + display: inline-block; + vertical-align: middle; + font-size: 0.8125rem; +} + +#changelist .actions .button, +#changeform .actions .button { + font-size: 0.8125rem; + border: 1px solid var(--border-color); + border-radius: 4px; + background: var(--body-bg); + box-shadow: 0 -15px 20px -10px rgba(0, 0, 0, 0.15) inset; + cursor: pointer; + height: 1.5rem; + line-height: 1; + padding: 4px 8px; + margin: 0; + color: var(--body-fg); +} + +#changelist .actions .button:focus, +#changelist .actions .button:hover, +#changeform .actions .button:focus, +#changeform .actions .button:hover { + border-color: var(--body-quiet-color); +} diff --git a/django/contrib/admin/static/admin/css/changelists.css b/django/contrib/admin/static/admin/css/changelists.css index 005b7768c8..f4dc2ddb52 100644 --- a/django/contrib/admin/static/admin/css/changelists.css +++ b/django/contrib/admin/static/admin/css/changelists.css @@ -279,65 +279,3 @@ background-color: SelectedItem; } } - -#changelist .actions { - padding: 10px; - background: var(--body-bg); - border-top: none; - border-bottom: none; - line-height: 1.5rem; - color: var(--body-quiet-color); - width: 100%; -} - -#changelist .actions span.all, -#changelist .actions span.action-counter, -#changelist .actions span.clear, -#changelist .actions span.question { - font-size: 0.8125rem; - margin: 0 0.5em; -} - -#changelist .actions:last-child { - border-bottom: none; -} - -#changelist .actions select { - vertical-align: top; - height: 1.5rem; - color: var(--body-fg); - border: 1px solid var(--border-color); - border-radius: 4px; - font-size: 0.875rem; - padding: 0 0 0 4px; - margin: 0; - margin-left: 10px; -} - -#changelist .actions select:focus { - border-color: var(--body-quiet-color); -} - -#changelist .actions label { - display: inline-block; - vertical-align: middle; - font-size: 0.8125rem; -} - -#changelist .actions .button { - font-size: 0.8125rem; - border: 1px solid var(--border-color); - border-radius: 4px; - background: var(--body-bg); - box-shadow: 0 -15px 20px -10px rgba(0, 0, 0, 0.15) inset; - cursor: pointer; - height: 1.5rem; - line-height: 1; - padding: 4px 8px; - margin: 0; - color: var(--body-fg); -} - -#changelist .actions .button:focus, #changelist .actions .button:hover { - border-color: var(--body-quiet-color); -} diff --git a/django/contrib/admin/templates/admin/change_form.html b/django/contrib/admin/templates/admin/change_form.html index 31ff5d6c10..43dc9fef3c 100644 --- a/django/contrib/admin/templates/admin/change_form.html +++ b/django/contrib/admin/templates/admin/change_form.html @@ -6,7 +6,13 @@ {{ media }} {% endblock %} -{% block extrastyle %}{{ block.super }}{% endblock %} +{% block extrastyle %} + {{ block.super }} + + {% if action_form %} + + {% endif %} +{% endblock %} {% block coltype %}colM{% endblock %} @@ -34,7 +40,8 @@ {% endif %} {% endblock %}
{% csrf_token %}{% block form_top %}{% endblock %} -
+
+{% if action_form %}{% admin_actions %}{% endif %} {% if is_popup %}{% endif %} {% if to_field %}{% endif %} {% if save_on_top %}{% block submit_buttons_top %}{% submit_row %}{% endblock %}{% endif %} diff --git a/django/contrib/admin/templatetags/admin_urls.py b/django/contrib/admin/templatetags/admin_urls.py index 176e7a49ed..c1f9e10752 100644 --- a/django/contrib/admin/templatetags/admin_urls.py +++ b/django/contrib/admin/templatetags/admin_urls.py @@ -5,6 +5,8 @@ from django.contrib.admin.utils import quote from django.urls import Resolver404, get_script_prefix, resolve from django.utils.http import urlencode +from .base import InclusionAdminNode + register = template.Library() @@ -68,3 +70,30 @@ def add_preserved_filters(context, url, popup=False, to_field=None): parsed_url[3] = urlencode(merged_qs) return urlunsplit(parsed_url) + + +def admin_actions(context): + """ + Track the number of times the action field has been rendered on the page, + so we know which value to use. + """ + context["action_index"] = context.get("action_index", -1) + 1 + return context + + +@register.tag(name="admin_actions") +def admin_actions_tag(parser, token): + return InclusionAdminNode( + parser, token, func=admin_actions, template_name="actions.html" + ) + + +@register.tag(name="change_list_object_tools") +def change_list_object_tools_tag(parser, token): + """Display the row of change list object tools.""" + return InclusionAdminNode( + parser, + token, + func=lambda context: context, + template_name="change_list_object_tools.html", + ) diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index 0ea64d594a..ec623ec82b 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -413,7 +413,13 @@ def redirect_to(modeladmin, request, selected): @admin.action(description="Download subscription") def download(modeladmin, request, selected): - buf = StringIO("This is the content of the file") + from django.db.models.query import QuerySet + + if isinstance(selected, QuerySet): + buf = StringIO("This is the content of the file") + else: + buf = StringIO(f"This is the content of the file written by {selected.name}") + return StreamingHttpResponse(FileWrapper(buf)) diff --git a/tests/admin_views/test_actions.py b/tests/admin_views/test_actions.py index 8e1fc144e4..8bbfeb7a37 100644 --- a/tests/admin_views/test_actions.py +++ b/tests/admin_views/test_actions.py @@ -544,3 +544,112 @@ class AdminActionsPermissionTests(TestCase): reverse("admin:admin_views_subscriber_changelist"), delete_confirmation_data ) self.assertEqual(response.status_code, 403) + + +@override_settings(ROOT_URLCONF="admin_views.urls") +class AdminDetailActionsTest(TestCase): + @classmethod + def setUpTestData(cls): + cls.superuser = User.objects.create_superuser( + username="super", password="secret", email="super@example.com" + ) + cls.s1 = ExternalSubscriber.objects.create( + name="John Doe", email="john@example.org" + ) + + def setUp(self): + self.client.force_login(self.superuser) + + def test_available_detail_actions(self): + """ + - Action 1 has not description. + - Action 2 has custom description. + - Action 3 ("Detail download") is not displayed because user does not + have permission. + - "Delete" action is not displayed in change view because already + exists a button for it. + """ + + response = self.client.get( + reverse("admin:admin_views_externalsubscriber_change", args=[self.s1.pk]) + ) + + self.assertContains( + response, + """