[Django] #30231: "verbose_name" is ignored in FilteredSelectMultiple widget for new objects
Django
------------------------------------------+------------------------
Reporter: FlorianSchuler | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 1 |
------------------------------------------+------------------------
When using the FilteredSelectMultiple widget (filter_horizontal) in a
TabularInline view on a ModelAdmin page the "verbose_name" of a
ManyToManyField is ignored for newly added objects via the "add another"
button. It simply displays the field name for those elements. The
FilteredSelectMultiple widgets of initially loaded / already existing
objects is displayed correctly with the use of the "verbose_name" of the
regarding field.
It seems like a small JS Bug, because when checking out the generated HTML
source code, the "select"-element has always the correct "data-field-name"
attribute which fits to the "verbose_name" (for the newly added objects
too). But when the FilteredSelectMultiple widget of newly added objects is
initialized the generation of the heading and the help texts seems to
ignore this "data-field-name" attribute / the "verbose_name" and instead
uses simply the field name.
See attached screenshot for explanation.
--
Ticket URL: <https://code.djangoproject.com/ticket/30231>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: FlorianSchuler | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
Changes (by FlorianSchuler):
* Attachment "screenshot.jpg" added.
Screenshot which shows wrong display of field verbose_name
Django
Reporter: FlorianSchuler | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
Changes (by FlorianSchuler):
* cc: FlorianSchuler (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:1>
Django
Reporter: Florian Schuler | Owner: Ahisahar
| Pretel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
Changes (by Ahisahar Pretel):
* owner: nobody => Ahisahar Pretel
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:2>
Django
Reporter: Florian Schuler | Owner: Ahisahar
| Pretel
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
Comment (by Florian Schuler):
Hey Ahisahar, were you already able to reproduce the bug on your side?
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:3>
Django
Reporter: Florian Schuler | Owner: Ahisahar
| Pretel
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
Comment (by Ahisahar Pretel):
Hello @Florian Schuler, actually not. It happens in admin.TabularInline
view or in the admin modelview once that we have a form with
FilteredSelectMultiple?
Thank you
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:4>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: Ahisahar
| Pretel
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
* easy: 1 => 0
Comment:
I reproduced at 9681e968ebdcd58cac99c1e60f0a6932abd4e5c9 with this patch
on the tutorial:
{{{ #!diff
diff --git a/polls/admin.py b/polls/admin.py
index f1bf573..0a184ab 100644
--- a/polls/admin.py
+++ b/polls/admin.py
@@ -6,6 +6,7 @@ from .models import Choice, Question
class ChoiceInline(admin.TabularInline):
model = Choice
extra = 3
+ filter_horizontal = ['groups']
class QuestionAdmin(admin.ModelAdmin):
diff --git a/polls/models.py b/polls/models.py
index 38ae549..b0756e3 100644
--- a/polls/models.py
+++ b/polls/models.py
@@ -4,6 +4,7 @@ import datetime
from six import python_2_unicode_compatible
+from django.contrib.auth.models import Group
from django.db import models
from django.utils import timezone
@@ -29,6 +30,7 @@ class Choice(models.Model):
question = models.ForeignKey(Question, on_delete=models.CASCADE)
choice_text = models.CharField(max_length=200)
votes = models.IntegerField(default=0)
+ groups = models.ForeignKey(Group, blank=True, verbose_name='test')
def __str__(self):
return self.choice_text
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:5>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: Ahisahar
| Pretel
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Florian Schuler):
Replying to [comment:4 Ahisahar Pretel]:
> Hello @Florian Schuler, actually not. It happens in admin.TabularInline
view or in the admin modelview once that we have a form with
FilteredSelectMultiple?
>
> Thank you
Like Tim Graham just confirmed it happens in admin.TabularInline view,
when adding a field to "filter_horizontal". In addition I already tried to
overwrite get_formset method of the regarding admin.TabularInline and
added FilteredSelectMultiple widget manually with verbose_name redefined
on the widget instead, but with the same result. So the bug occurs with
manually added FilteredSelectMultiple widgets too.
@Tim Graham thank you for the confirmation of the bug and clarifying the
title of the ticket! Of course it is important that this only happens to
inlines added via the "add another" button.
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:6>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: Ahisahar
| Pretel
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Ahisahar Pretel):
I should probably let it to someone who has more experience right @Florian
Schuler @Tim Graham? This is my first contribution
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:7>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: Ahisahar
| Pretel
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Florian Schuler):
Replying to [comment:7 Ahisahar Pretel]:
> I should probably let it to someone who has more experience right
@Florian Schuler @Tim Graham? This is my first contribution
Hey @Ahisahar Pretel and @Tim Graham, I just found the origin of this
wrong behaviour and fixed it in my local Django system (2.1.7)!
Like already noted in my bug report it seems like a small logic fault in a
javascript file. In the file which is responsible for initializing the
FilteredSelectMultiple widget at page load
(django/contrib/admin/static/admin/js/SelectFilter2.js) there is the
correct usage of calling the SelectFilter.init() method at the very end
(line 244 to 250):
{{{
window.addEventListener('load', function(e) {
$('select.selectfilter,
select.selectfilterstacked').each(function() {
var $el = $(this),
data = $el.data();
SelectFilter.init($el.attr('id'), data.fieldName,
parseInt(data.isStacked, 10));
});
});
}}}
So second parameter has to be the data attribute fieldName of the HTML
element, which represents the verbose_name if one is given for the field.
In the js file which is responsible for triggering the initialization of
newly added FilteredSelectMultiple Widgets after clicking the "add
another" button (django/contrib/admin/static/admin/js/inlines.js) this
parameter isn't set correctly.
**For tabular inlines:**
Current function:
{{{
var updateSelectFilter = function() {
// If any SelectFilter widgets are a part of the new form,
// instantiate a new SelectFilter instance for it.
if (typeof SelectFilter !== 'undefined') {
$('.selectfilter').each(function(index, value) {
var namearr = value.name.split('-');
SelectFilter.init(value.id, namearr[namearr.length -
1], false);
});
$('.selectfilterstacked').each(function(index, value) {
var namearr = value.name.split('-');
SelectFilter.init(value.id, namearr[namearr.length -
1], true);
});
}
};
}}}
So it gets the field name out of the "name" attribute which is the real
field name, not the verbose name which could be easily taken with
"$(this).data('fieldName')"
Corrected function should be:
{{{
var updateSelectFilter = function() {
// If any SelectFilter widgets are a part of the new form,
// instantiate a new SelectFilter instance for it.
if (typeof SelectFilter !== 'undefined') {
$('.selectfilter').each(function(index, value) {
SelectFilter.init(value.id, $(this).data('fieldName'),
false);
});
$('.selectfilterstacked').each(function(index, value) {
SelectFilter.init(value.id, $(this).data('fieldName'),
true);
});
}
};
}}}
**The same for Stacked inline:**
Current function:
{{{
var updateSelectFilter = function() {
// If any SelectFilter widgets were added, instantiate a new
instance.
if (typeof SelectFilter !== "undefined") {
$(".selectfilter").each(function(index, value) {
var namearr = value.name.split('-');
SelectFilter.init(value.id, namearr[namearr.length -
1], false);
});
$(".selectfilterstacked").each(function(index, value) {
var namearr = value.name.split('-');
SelectFilter.init(value.id, namearr[namearr.length -
1], true);
});
}
};
}}}
Corrected function should be:
{{{
var updateSelectFilter = function() {
// If any SelectFilter widgets were added, instantiate a new
instance.
if (typeof SelectFilter !== "undefined") {
$(".selectfilter").each(function(index, value) {
SelectFilter.init(value.id, $(this).data('fieldName'),
false);
});
$(".selectfilterstacked").each(function(index, value) {
SelectFilter.init(value.id, $(this).data('fieldName'),
true);
});
}
};
}}}
At least on my side everything is working correctly with these changes.
Maybe one of you could recheck my changes and test it on a clean project
if everything is still working like expected and maybe it would also be a
good idea to search for more SelectFilter.init() calls at other places in
the code?! Don't know if it is anywhere else wrongly used.
It would also be good to check if the data attribute fieldName is
guaranteed to be set, but because it is used in this way in the
SelectFilter2.js file i would think so... But i still wonder why it was
wrongly implemented in the inlines.js file (maybe there was a change
someday at the generation of the selectfilter elements and the fieldName
attribute was added later and no one thought about changing it in
inlines.js file too?!).
@Ahisahar Pretel or @Tim Graham could any of you take care of checking
this out and submitting a patch?
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:8>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
| Schuler
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: Ahisahar Pretel => Florian Schuler
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:9>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: Florian
| Schuler
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Florian Schuler):
Just submitted a patch with the changes i already mentioned above and made
a pull request.
https://github.com/django/django/pull/11076
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:10>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: Florian
| Schuler
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Florian Schuler):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:11>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: Florian
| Schuler
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_tests: 0 => 1
Comment:
Tests (probably using selenium in this case) are also required.
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:12>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: Florian
| Schuler
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Florian Schuler):
Replying to [comment:12 Tim Graham]:
> Tests (probably using selenium in this case) are also required.
Hey Tim, I don't know what tests could be useful in this case of UI bug
where only a wrong name was passed to display (it's a very trivial bugfix,
simply fetching the name from fieldName attribute instead). I don't have
any experience in writing tests using selenium for Django UI and also
don't have any time for reading into it and setting up a testing
environment at the moment. So if there really should be tests needed,
someone else needs to take care of it.
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:13>
Django
inlines created with "Add another" button
Reporter: Florian Schuler | Owner: (none)
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
---------------------------------+------------------------------------
Changes (by Tim Graham):
* status: assigned => new
* owner: Florian Schuler => (none)
Comment:
Okay, I'll deassign the ticket then. Even trivial bug fixes require tests
to prevent regressions.
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:14>
Django
inlines created with "Add another" button
---------------------------------+------------------------------------
Reporter: Florian Schuler | Owner: (none)
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
---------------------------------+------------------------------------
Comment (by Florian Schuler):
Replying to [comment:14 Tim Graham]:
> Okay, I'll deassign the ticket then. Even trivial bug fixes require
tests to prevent regressions.
Hey Tim, Okay!
Just noticed that the owner field of this ticket is now empty. Shouldn't
it be "nobody" like the other new tickets so that other users can query
for unassigned tickets by searching for owner "nobody"?
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:15>
Django
inlines created with "Add another" button
Reporter: Florian Schuler | Owner:
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: (none) => TeresaPartidaS
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:16>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: Teresa
| Partida
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Teresa Partida):
Hi! I submitted a new patch with the changes Florian made and the Selenium
test
https://github.com/django/django/pull/11282
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:17>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: Teresa
| Partida
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* cc: Carlton Gibson (added)
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:18>
Django
inlines created with "Add another" button
Reporter: Florian Schuler | Owner: David Smith
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
---------------------------------+---------------------------------------
Changes (by David Smith):
* owner: Teresa Partida => David Smith
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:19>
Django
inlines created with "Add another" button
Reporter: Florian Schuler | Owner: David
| Smith
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:20>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: David
| Smith
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"45bef6706ae7436b5c2c503018b21ff70b2e6728" 45bef670]:
{{{
#!CommitTicketReference repository=""
revision="45bef6706ae7436b5c2c503018b21ff70b2e6728"
Fixed #30231 -- Fixed admin filter horizontal/vertical verbose_name
generation.
Co-authored-by: David Smith <smi...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:21>
Django
inlines created with "Add another" button
-------------------------------------+-------------------------------------
Reporter: Florian Schuler | Owner: David
| Smith
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"96a50934007f4a9a14c76a08be5bbd3800b4b66c" 96a50934]:
{{{
#!CommitTicketReference repository=""
revision="96a50934007f4a9a14c76a08be5bbd3800b4b66c"
Refs #30231 -- Fixed SeleniumTests.test_inlines_verbose_name with headless
mode.
Horizontal scrollbar doesn't appear with the headless mode on small
windows, that's why window.scrollTo() is not an option for these tests
even after fixing #32459.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30231#comment:22>