[Django] #36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
-------------------------------------+-------------------------------------
Reporter: Antoliny | Type:
| Cleanup/optimization
Status: new | Component:
| contrib.admin
Version: 5.1 | Severity: Normal
Keywords: TabularInline | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
When using a foreign key reverse reference model with TabularInline in
ModelAdmin, if the corresponding object does not exist, the 'DELETE' text
seems unnecessary.
I feel that the 'DELETE' text helps users remove an object when the
TabularInline object exists.
However, if the object doesn't exist, the 'DELETE' text seems to lose its
purpose.
[[Image(tabular_inline_ex.png)]]
--
Ticket URL: <https://code.djangoproject.com/ticket/36067>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
-------------------------------------+-------------------------------------

Reporter: Antoliny | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:

Keywords: TabularInline | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Antoliny):

* Attachment "tabular_inline_ex.png" added.

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
-------------------------------------+-------------------------------------

Reporter: Antoliny | Owner: Antoliny
Type: | Status: assigned

Cleanup/optimization |
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Antoliny):

* owner: (none) => Antoliny
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:1>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist

--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Natalia Bidart):

* stage: Unreviewed => Accepted
* version: 5.1 => dev

Comment:

Hello Antoliny, thank you for taking the time to create this report! I
agree with you, the "Delete?" legend is confusing and unnecessary.
--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:2>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by Antoliny):

I explored several approaches to address this issue.
I initially wanted to solve it using conditions directly in the template,
but there was no suitable attribute available to determine whether each
Formset contains objects.

As a solution, I decided to add a self.has_object attribute to the
InlineAdminFormSet, assigning it the return value of
self.formset.get_queryset().exists(). This allows the presence of the
"DELETE?" text to be determined based on whether objects exist.

However, the downside of this approach is that an attribute is being added
solely for the purpose of removing the "DELETE?" text. If you have a
better suggestion, I would greatly appreciate your advice.
{{{#!diff
diff --git a/django/contrib/admin/helpers.py
b/django/contrib/admin/helpers.py
index 51450d1d9e..97a9f764f1 100644
--- a/django/contrib/admin/helpers.py
+++ b/django/contrib/admin/helpers.py
@@ -337,6 +337,7 @@ class InlineAdminFormSet:
self.has_change_permission = has_change_permission
self.has_delete_permission = has_delete_permission
self.has_view_permission = has_view_permission
+ self.has_object = self.formset.get_queryset().exists()

def __iter__(self):
if self.has_change_permission:
diff --git a/django/contrib/admin/templates/admin/edit_inline/tabular.html
b/django/contrib/admin/templates/admin/edit_inline/tabular.html
index 7acfda7bd1..e1c7d993b5 100644
--- a/django/contrib/admin/templates/admin/edit_inline/tabular.html
+++ b/django/contrib/admin/templates/admin/edit_inline/tabular.html
@@ -23,7 +23,7 @@
{% if field.help_text %}<img src="{% static "admin/img/icon-
unknown.svg" %}" class="help help-tooltip" width="10" height="10" alt="({{
field.help_text|striptags }})" title="{{ field.help_text|striptags }}">{%
endif %}
</th>
{% endfor %}
- {% if inline_admin_formset.formset.can_delete and
inline_admin_formset.has_delete_permission %}<th>{% translate "Delete?"
%}</th>{% endif %}
+ {% if inline_admin_formset.formset.can_delete and
inline_admin_formset.has_delete_permission and
inline_admin_formset.has_object %}<th>{% translate "Delete?" %}</th>{%
endif %}
</tr></thead>

<tbody>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:3>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by Natalia Bidart):

Replying to [comment:3 Antoliny]:

> As a solution, I decided to add a self.has_object attribute to the
InlineAdminFormSet, assigning it the return value of
self.formset.get_queryset().exists(). This allows the presence of the
"DELETE?" text to be determined based on whether objects exist.
>
> However, the downside of this approach is that an attribute is being
added solely for the purpose of removing the "DELETE?" text. If you have a
better suggestion, I would greatly appreciate your advice.

I share your concern with your initial diff. There is one extra query
being issued which feels unnecessary. I played a bit with this and settled
on having something like this, which nicely reduces logic in the template
for calculating the header and checkbox row, and it also uses the existing
information in the formset:

{{{#!diff
diff --git a/django/contrib/admin/helpers.py
b/django/contrib/admin/helpers.py

index 51450d1d9e..3e33855394 100644
--- a/django/contrib/admin/helpers.py
+++ b/django/contrib/admin/helpers.py
@@ -460,6 +460,14 @@ class InlineAdminFormSet:
def total_form_count(self):
return self.formset.total_form_count

+ @property
+ def can_delete(self):
+ return (
+ self.formset.can_delete
+ and self.has_delete_permission
+ and any(inlineadminform.original is not None for
inlineadminform in self)
+ )
+
@property
def media(self):
media = self.opts.media + self.formset.media
diff --git a/django/contrib/admin/templates/admin/edit_inline/tabular.html
b/django/contrib/admin/templates/admin/edit_inline/tabular.html
index 7acfda7bd1..ebbe3dfe25 100644

--- a/django/contrib/admin/templates/admin/edit_inline/tabular.html
+++ b/django/contrib/admin/templates/admin/edit_inline/tabular.html
@@ -23,7 +23,7 @@
{% if field.help_text %}<img src="{% static "admin/img/icon-
unknown.svg" %}" class="help help-tooltip" width="10" height="10" alt="({{
field.help_text|striptags }})" title="{{ field.help_text|striptags }}">{%
endif %}
</th>
{% endfor %}
- {% if inline_admin_formset.formset.can_delete and
inline_admin_formset.has_delete_permission %}<th>{% translate "Delete?"
%}</th>{% endif %}

+ {% if inline_admin_formset.can_delete %}<th>{% translate "Delete?"

%}</th>{% endif %}
</tr></thead>

<tbody>

@@ -58,7 +58,7 @@
{% endfor %}
{% endfor %}

{% endfor %}
- {% if inline_admin_formset.formset.can_delete and
inline_admin_formset.has_delete_permission %}

+ {% if inline_admin_formset.can_delete %}
<td class="delete">{% if inline_admin_form.original %}{{
inline_admin_form.deletion_field.field }}{% endif %}</td>
{% endif %}
</tr>
}}}

Antoliny, what do you think? This would need a few tests added to existing
admin suite.
--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:4>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by Antoliny):

Replying to [comment:4 Natalia Bidart]:

> I share your concern with your initial diff. There is one extra query
being issued which feels unnecessary. I played a bit with this and settled
on having something like this, which nicely reduces logic in the template
for calculating the header and checkbox row, and it also uses the existing

information in the formset: ...

That's correct. My suggestion added unnecessary queries. I think it's
great that this can be resolved through repetition.
(`any(inlineadminform.original is not None for inlineadminform in self)`)
Also, the part about simplifying the logic in the template is very
beneficial. Thank you so much for your excellent suggestion.
--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:5>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Changes (by Antoliny):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:6>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by Antoliny):

[https://github.com/django/django/pull/19015 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:7>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Changes (by Natalia Bidart):

* needs_better_patch: 0 => 1
* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:8>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Changes (by Antoliny):

* needs_better_patch: 1 => 0
* needs_tests: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:9>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Antoliny):

* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:10>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Antoliny):

* needs_tests: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:11>

[Django]

#36067: Ambiguity of the 'DELETE' text when a TabularInline object does not exist
--------------------------------------+------------------------------------
Reporter: Antoliny | Owner: Antoliny
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: TabularInline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Antoliny):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36067#comment:12>