Opened 4 years ago

Closed 9 months ago

Last modified 8 months ago

#19536 closed Cleanup/optimization (fixed)

in ModelAdmin disabling has_add_permission results in not showing any object-tools

Reported by: a.fazeli@… Owned by: Igor Támara
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

in change_list.html when has_add_permission is false all the object-tools (buttons) are not shown, even if you have implemented some custom tools. This permission check should only be for the add button item.

I tweaked the code (very small change) that will fix this problem.

My proposal for this matter is attached to this ticket

Attachments (1)

change_list_patch.html (483 bytes) - added by a.fazeli@… 4 years ago.
patch for ModelAdmin has_add_permission disabling all the object-tools-items

Download all attachments as: .zip

Change History (13)

Changed 4 years ago by a.fazeli@…

Attachment: change_list_patch.html added

patch for ModelAdmin has_add_permission disabling all the object-tools-items

comment:1 Changed 4 years ago by Claude Paroz

Needs documentation: unset
Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Version: 1.4master

Makes sense. Please provide your attachment as patch/diff format.

comment:2 Changed 4 years ago by Ramiro Morales

The relevant code is:

{% block object-tools %}
  {% if has_add_permission %}
    <ul class="object-tools">
      {% block object-tools-items %}
        <li>
          <a href="{% url cl.opts|admin_urlname:'add' %}{% if is_popup %}?_popup=1{% endif %}" class="addlink">
            {% blocktrans with cl.opts.verbose_name as name %}Add {{ name }}{% endblocktrans %}
          </a>
        </li>
      {% endblock %}
    </ul>
  {% endif %}
{% endblock %}

The extension strategy provided to developers wishing to implement custom tools is to override the whole object-tools block. The modification proposed by the OP would generate an empty UL element if the current user has no add permission for the normal case (no customization of the template). IMHO we need a better solution or simply leave thing as the are. Consider me -1 on the proposal in its current form.

comment:3 Changed 3 years ago by arash77

How about this solution:

{% block object-tools %}
    <ul class="object-tools">
        <li style="display: none;"></li>
        {% block object-tools-items %}
            {% if has_add_permission %}
            <li>
                <a href="{% url cl.opts|admin_urlname:'add' %}{% if is_popup %}?_popup=1{% endif %}" class="addlink">
                    {% blocktrans with cl.opts.verbose_name as name %}Add {{ name }}{% endblocktrans %}
                </a>
            </li>
            {% endif %}
        {% endblock %}
    </ul>
{% endblock %}

This way an empty UL will not be created. Of-course the first LI can also be configured through relative CSS class.

comment:4 Changed 3 years ago by Igor Támara

Owner: changed from nobody to Igor Támara
Status: newassigned

comment:5 Changed 3 years ago by Igor Támara

a pull request has been done from https://github.com/ikks/django/tree/ticket_19536 , added a test to make sure it works as expected.

comment:6 Changed 3 years ago by Tim Graham

Left some comments on the PR.

comment:7 Changed 3 years ago by Tim Graham

Easy pickings: unset

comment:8 Changed 11 months ago by Nick Sandford

comment:9 Changed 11 months ago by Marten Kenbeek

Needs tests: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

The patch looks good to me.

comment:10 Changed 11 months ago by Tim Graham

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

Left some minor comments.

comment:11 Changed 9 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In ff19df9:

Fixed #19536 -- Included object-tools when ModelAdmin.has_add_permission() is False.

comment:12 Changed 8 months ago by Tim Graham <timograham@…>

In e73fb2c:

Refs #19536 -- Tweaked test assertion to avoid collision with CSRF token.

Note: See TracTickets for help on using tickets.
Back to Top