Opened 8 years ago

Closed 8 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 Changed 8 years ago by Tim Graham

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 Changed 8 years ago by arcturusannamalai

Owner: changed from nobody to arcturusannamalai
Status: newassigned

Attempt Django bug fix.

comment:4 in reply to:  1 Changed 8 years ago by arcturusannamalai

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 Changed 8 years ago by Tim Graham

Needs tests: set

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

comment:6 in reply to:  5 Changed 8 years ago by arcturusannamalai

Replying to timgraham:
I will update it. Thanks.

comment:7 Changed 8 years ago by Berker Peksag

Needs tests: unset
Patch needs improvement: set

comment:8 Changed 8 years ago by arcturusannamalai

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

comment:9 Changed 8 years ago by arcturusannamalai

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

comment:10 Changed 8 years ago by Claude Paroz

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 Changed 8 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:12 Changed 8 years ago by Tim Graham <timograham@…>

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