[Django] #19536: in ModelAdmin disabling has_add_permission results in not showing any object-tools
Django
object-tools
--------------------------------------+--------------------
Reporter: a.fazeli@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 1 | UI/UX: 0
--------------------------------------+--------------------
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
--
Ticket URL: <https://code.djangoproject.com/ticket/19536>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
object-tools
Reporter: a.fazeli@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 1
* version: 1.4 => master
* needs_docs: => 0
* needs_tests: => 1
* stage: Unreviewed => Accepted
Comment:
Makes sense. Please provide your attachment as patch/diff format.
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:1>
Django
object-tools
Reporter: a.fazeli@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by ramiro):
The relevant code is:
{{{
#!application/x-jinja
{% 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:2>
Django
object-tools
Reporter: a.fazeli@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by arash77):
How about this solution:
{{{
#!application/x-jinja
{% 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:3>
Django
object-tools
Reporter: a.fazeli@… | Owner: ikks
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by ikks):
* status: new => assigned
* owner: nobody => ikks
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:4>
Django
object-tools
Reporter: a.fazeli@… | Owner: ikks
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by ikks):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:5>
Django
object-tools
Reporter: a.fazeli@… | Owner: ikks
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by timo):
Left some comments on the [https://github.com/django/django/pull/1623 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:6>
Django
object-tools
Reporter: a.fazeli@… | Owner: ikks
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
--------------------------------------+------------------------------------
Changes (by timo):
* easy: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:7>
Django
object-tools
Reporter: a.fazeli@… | Owner: ikks
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by slurms):
Added a new PR: https://github.com/django/django/pull/5573.
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:8>
Django
object-tools
Reporter: a.fazeli@… | Owner: ikks
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by knbk):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
The patch looks good to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:9>
Django
object-tools
Reporter: a.fazeli@… | Owner: ikks
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
Left some minor comments.
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:10>
Django
object-tools
Reporter: a.fazeli@… | Owner: ikks
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ff19df9c2d6ad80bb9177e8190176e9c3716705f" ff19df9]:
{{{
#!CommitTicketReference repository=""
revision="ff19df9c2d6ad80bb9177e8190176e9c3716705f"
Fixed #19536 -- Included object-tools when ModelAdmin.has_add_permission()
is False.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:11>
Django
object-tools
Reporter: a.fazeli@… | Owner: ikks
Type: Cleanup/optimization | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"e73fb2c9e1ecddf0016ea553350270c855d1de71" e73fb2c]:
{{{
#!CommitTicketReference repository=""
revision="e73fb2c9e1ecddf0016ea553350270c855d1de71"
Refs #19536 -- Tweaked test assertion to avoid collision with CSRF token.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19536#comment:12>