Opened 9 years ago

Closed 9 years ago

#24089 closed Cleanup/optimization (fixed)

Misleading error raised during system check related to ModelAdmin

Reported by: okrutny Owned by: arcturusannamalai
Component: contrib.admin Version: 1.7
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

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.

Change History (12)

comment:1 by Tim Graham, 9 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted

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.

comment:2 by arcturusannamalai, 9 years ago

Owner: changed from nobody to arcturusannamalai
Status: newassigned

Attempt Django bug fix.

in reply to:  1 comment:4 by arcturusannamalai, 9 years ago

Replying to 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?

comment:5 by Tim Graham, 9 years ago

Needs tests: set

Please add a test case, check your against using our patch review checklist, and then create a pull request.

in reply to:  5 comment:6 by arcturusannamalai, 9 years ago

Replying to timgraham:
I will update it. Thanks.

comment:7 by Berker Peksag, 9 years ago

Needs tests: unset
Patch needs improvement: set

comment:8 by arcturusannamalai, 9 years ago

I added a test, https://github.com/django/django/pull/3876/commits and sent a pull request.

comment:9 by arcturusannamalai, 9 years ago

Patch needs improvement: unset
Resolution: fixed
Status: assignedclosed
Triage Stage: AcceptedReady for checkin

comment:10 by Claude Paroz, 9 years ago

Resolution: fixed
Status: closednew
Triage Stage: Ready for checkinAccepted

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.

comment:11 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: newclosed

In b75c707943e159b80c179c538721406bbfb8b120:

Fixed #24089 -- Added check for when ModelAdmin.fieldsets[1]fields isn't a list/tuple.

Note: See TracTickets for help on using tickets.
Back to Top