[Django] #36498: Inline button to add first / only related instance should say "Add ...", not "Add another ..."

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."
-------------------------------------+-------------------------------------
Reporter: Denis Washington | Type: Bug
Status: new | Component:
| contrib.admin
Version: 5.2 | Severity: Normal
Keywords: inline | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------
In the default `InlineModelAdmin` templates, the button to add a related
model instance always says "Add another X", even when

- there is no other related instance (e.g., if `extra=0` or the last
related instance was deleted)
- if it's not even possible to add multiple related instances (e.g.,
`OneToOneField`)

In these cases, I would expect the button to say "Add X" (without
"another") instead.
--
Ticket URL: <https://code.djangoproject.com/ticket/36498>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."

--------------------------------------+------------------------------------
Reporter: Denis Washington | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted

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

--------------------------------------+------------------------------------
Changes (by Antoliny):

* stage: Unreviewed => Accepted
* type: Bug => Cleanup/optimization

Comment:

I agree with this proposal as well.
When `extra=0` is set and there are no related objects connected via
Inline, I feel that the button to add an InlineForm should say "Add
{model_name}", as that is the more appropriate expression.
As you mentioned, this is especially true in cases like a `OneToOneField`,
where only a single object can be added.
--
Ticket URL: <https://code.djangoproject.com/ticket/36498#comment:1>

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."
--------------------------------------+------------------------------------
Reporter: Denis Washington | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
--------------------------------------+------------------------------------
Changes (by Antoliny):

* cc: Antoliny (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/36498#comment:2>

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."

-------------------------------------+-------------------------------------
Reporter: Denis Washington | Owner: Dorian
Type: | Adams
Cleanup/optimization | Status: assigned

Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1

-------------------------------------+-------------------------------------
Changes (by Dorian Adams):

* owner: (none) => Dorian Adams
* status: new => assigned

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

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."
-------------------------------------+-------------------------------------
Reporter: Denis Washington | Owner: Dorian
Type: | Adams
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by Dorian Adams):

Should the "add" label be based on the actual database contents or on the
user’s visible context? Specifically, when a user has add permission but
lacks change permission on an inline model, existing instances may exist
but won't be visible to the user.

(Is it ok to pose this question here)
--
Ticket URL: <https://code.djangoproject.com/ticket/36498#comment:4>

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."
-------------------------------------+-------------------------------------
Reporter: Denis Washington | Owner: Dorian
Type: | Adams
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by Antoliny):

Replying to [comment:4 Dorian Adams]:

> Should the "add" label be based on the actual database contents or on
the user’s visible context? Specifically, when a user has add permission
but lacks change permission on an inline model, existing instances may
exist but won't be visible to the user.
>
> (Is it ok to pose this question here)

I believe it should be possible to base it on the actual database contents
without requiring additional queries! (Of course, I could be wrong.)

In the Django admin inline, the text for adding an inline form is
determined by the inline_formset_data method of the InlineAdminFormSet
class.
And if you look at the `__iter__` magic method, you'll see that the query
is already executed there.(To populate the objects linked to the
InlineForm)
so I think we can simply reuse the result of that query like this...

{{{#!diff
diff --git a/django/contrib/admin/helpers.py
b/django/contrib/admin/helpers.py
index 969167f0e2..8586c20a1f 100644
--- a/django/contrib/admin/helpers.py
+++ b/django/contrib/admin/helpers.py
@@ -419,6 +419,7 @@ class InlineAdminFormSet:

def inline_formset_data(self):
verbose_name = self.opts.verbose_name
+ has_object = any(inlineadminform.original is not None for
inlineadminform in self)
return json.dumps(
{
"name": "#%s" % self.formset.prefix,

}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36498#comment:5>

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."
-------------------------------------+-------------------------------------
Reporter: Denis Washington | Owner: Dorian
Type: | Adams
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

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

Changes (by Dorian Adams):

* has_patch: 0 => 1

Comment:

PR: https://github.com/django/django/pull/19666
--
Ticket URL: <https://code.djangoproject.com/ticket/36498#comment:6>

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."
-------------------------------------+-------------------------------------
Reporter: Denis Washington | Owner: Dorian
Type: | Adams
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by Dorian Adams):

Replying to [comment:5 Antoliny]:

Thanks for the additional info. It looks like a DB query may be necessary,
but only when the user has add permission and lacks change permission.
From what I can gather, this is due to the permission-based filtering
applied to `formset.get_queryset()` within the `__iter__` method of
`InlineAdminFormSet`.

From `InlineModelAdmin`:

{{{
def get_queryset(self, request):
queryset = super().get_queryset(request)
if not self.has_view_or_change_permission(request):
queryset = queryset.none()
return queryset
}}}

I opened a PR with a potential workaround that relies on what's available
in the database across all scenarios. The query only runs when necessary.
However, the potential downside is that this would reveal the existence of
records to users without view permission.
--
Ticket URL: <https://code.djangoproject.com/ticket/36498#comment:7>

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."
-------------------------------------+-------------------------------------
Reporter: Denis Washington | Owner: Dorian
Type: | Adams
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

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

Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

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

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."
-------------------------------------+-------------------------------------

Reporter: Denis Washington | Owner: Dani
Type: | Fornons

Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------

Changes (by Dani Fornons):

* owner: Dorian Adams => Dani Fornons

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

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."
-------------------------------------+-------------------------------------
Reporter: Denis Washington | Owner: Dani
Type: | Fornons
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by Dani Fornons):

I've Changed inline admin button text to use "Add" instead of "Add
another" and updated the test
PR: https://github.com/django/django/pull/20084
--
Ticket URL: <https://code.djangoproject.com/ticket/36498#comment:10>

[Django]

#36498: Inline button to add first / only related instance should say "Add ...",
not "Add another ..."
-------------------------------------+-------------------------------------
Reporter: Denis Washington | Owner: Dani
Type: | Fornons
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: inline | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

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

Changes (by Dani Fornons):

* needs_better_patch: 1 => 0

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