Opened 4 years ago

Closed 4 years ago

#18051 closed Cleanup/optimization (fixed)

flatten_fieldsets error

Reported by: Riccardo Di Virgilio Owned by: mateusgondim
Component: contrib.admin Version: 1.4
Severity: Normal Keywords:
Cc: nick.sandford@… 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

hi when flattening fieldsets django checks if a fieldset has a line

if type(field) == tuple:

i've got a lot of errors because i often generate fieldsets from code, and sometime i must use list to declare lines.

maybe to avoid list problems it's better to change this line of code to

if type(field) in (tuple, list):

Attachments (2)

ticket18051.diff (568 bytes) - added by mateusgondim 4 years ago.
I followed the above suggestion and replaced that line with the isinstance checking for list and tuples
ticket18051-2.diff (2.1 KB) - added by mateusgondim 4 years ago.
Added tests.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 4 years ago by Claude Paroz

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

It seems that the pattern isinstance(field, (list, tuple)) is widely used in Django.

comment:2 Changed 4 years ago by mateusgondim

Owner: changed from nobody to mateusgondim
Status: newassigned

Changed 4 years ago by mateusgondim

Attachment: ticket18051.diff added

I followed the above suggestion and replaced that line with the isinstance checking for list and tuples

comment:3 Changed 4 years ago by mateusgondim

Has patch: set

comment:4 Changed 4 years ago by Julien Phalip

Easy pickings: set
Needs tests: set

The patch looks good. Could you add some tests for it? Thanks!

Changed 4 years ago by mateusgondim

Attachment: ticket18051-2.diff added

Added tests.

comment:5 Changed 4 years ago by Nick Sandford

Cc: nick.sandford@… added
Needs tests: unset
Triage Stage: AcceptedReady for checkin

Patch with tests looks good.

comment:6 Changed 4 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: assignedclosed

In 013db6ba85fb880bd1f9a5ad2e91dc5c1efe197c:

Fixed #18051 -- Allowed admin fieldsets to contain lists

Thanks Ricardo di Virgilio for the report, Mateus Gondim for the
patch and Nick Sandford for the review.

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