[Django] #24089: Misleading error raised during system check related to ModelAdmin
Django
--------------------------------------+--------------------
Reporter: okrutny | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
When comma is forgoten when defining fieldsets on ModelAdmin, wrong error
is raised:
ERRORS:
<class 'fund_profiles.admin.OrganizationAdmin'>: (admin.E012) There are
duplicate field(s) in 'fieldsets[0][1]'.
Code to reproduce:
{{{
class OrganizationAdmin(admin.ModelAdmin):
list_display = ('name','email',
'accepted_ads','accepted_rules','accepted_handling','date_created','date_modified')
list_filter = ('verified','accepted_ads',)
fieldsets = (
(_('administration'),
{
'fields': ('removed')
}),
)
}}}
Adding comma to tuple after 'removed' fixes issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/24089>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: okrutny | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by timgraham):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Looking at the `_check_fields()` method in `contrib/admin/checks.py`, it
looks like we fail to check if the nested fields inside a fieldset are a
list or tuple. We can likely reuse the existing `'admin.E008'` code.
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:1>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by arcturusannamalai):
* owner: nobody => arcturusannamalai
* status: new => assigned
Comment:
Attempt Django bug fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:2>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by arcturusannamalai):
* has_patch: 0 => 1
Comment:
Patch :
https://github.com/arcturusannamalai/django/compare/ticket_24089?expand=1
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:3>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by arcturusannamalai):
Replying to [comment:1 timgraham]:
> Looking at the `_check_fields()` method in `contrib/admin/checks.py`, it
looks like we fail to check if the nested fields inside a fieldset are a
list or tuple. We can likely reuse the existing `'admin.E008'` code.
Can you please review my change, Tim?
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:4>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 0 => 1
Comment:
Please add a test case, check your against using our
[https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
/submitting-patches/#patch-review-checklist patch review checklist], and
then create a pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:5>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by arcturusannamalai):
Replying to [comment:5 timgraham]:
I will update it. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:6>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:7>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by arcturusannamalai):
I added a test, https://github.com/django/django/pull/3876/commits and
sent a pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:8>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Component: contrib.admin | Version: 1.7
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* needs_better_patch: 1 => 0
* resolution: => fixed
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:9>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Component: contrib.admin | Version: 1.7
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: fixed =>
* stage: Ready for checkin => Accepted
Comment:
Ticket is only closed when the fix is committed. And `Ready for checkin`
should never be set by the patch author, but by a reviewer.
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:10>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Component: contrib.admin | Version: 1.7
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:11>
Django
Reporter: okrutny | Owner:
Type: | arcturusannamalai
Component: contrib.admin | Version: 1.7
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"b75c707943e159b80c179c538721406bbfb8b120"]:
{{{
#!CommitTicketReference repository=""
revision="b75c707943e159b80c179c538721406bbfb8b120"
Fixed #24089 -- Added check for when ModelAdmin.fieldsets[1]['fields']
isn't a list/tuple.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24089#comment:12>