validation.py backwards incompatibility for fieldsets

[rebus_]

Hi,

This is my first bug report so please don't shoot me right away.

In revision 11744 [1] code was added in
django/contrib/admin/validation.py which is supposed to improve the
error message for M2M fields. It seems to me that this change has
broken ability to display multiple fields on the same line in
administration.


For example lets take this Model and AdminModel:

class InlineFieldsets(models.Model):
a = models.CharField(max_length=100)
b = models.CharField(max_length=100)
c = models.CharField(max_length=100)

class InlineFieldsetsAdmin(admin.ModelAdmin):
fieldsets = (
('Header 1', {'fields': ('a',('b', 'c'))}),
)

This will now fail horribly. Once the loop in
django/contrib/admin/validation.py@222 gets to the ('b', 'c') fields
subset it will pass ('b', 'c') as `field` argument to
check_formfield() function which will raise

ImproperlyConfigured: 'InlineFieldsetsAdmin.fieldsets[0][1]['fields']'
refers to field '('b', 'c')' that is missing from the form.


Instead, i think fieldset should first be flattened then go through
the loop, call check_formfield() and check for M2M fields etc. like
so:

flattened_fieldsets = flatten_fieldsets(cls.fieldsets)
for field in flattened_fieldsets:
check_formfield(cls, model, opts, "fieldsets[%d][1]['fields']" % idx, field)
<some M2M checking code>

instead the way it does it now:
for field in fieldset[1]['fields']:
check_formfield(cls, model, opts, "fieldsets[%d][1]['fields']" % idx, field)
<some M2M checking code>
flattened_fieldsets = flatten_fieldsets(cls.fieldsets)

Patch is attached, i can open up a new ticket if you want.
I have ran runtests.py with this patch applied and everything passes.
But then again i am not python nor django guru so i can't be sure
everything is ok.
I have also added regression tests for this case. Hope this patch can help.

[1] http://code.djangoproject.com/changeset/11744

--
Davor Lučić

[Russell Keith-Magee]

On Thu, Nov 19, 2009 at 10:35 PM, rebus_ <r.da...@gmail.com> wrote:
> Hi,
>
> This is my first bug report so please don't shoot me right away.
>
> In revision 11744 [1] code was added in
> django/contrib/admin/validation.py which is supposed to improve the
> error message for M2M fields. It seems to me that this change has
> broken ability to display multiple fields on the same line in
> administration.

Thanks for the report! You've definitely found a problem, and your
test case made it easy to narrow it down. Your patch wasn't quite
right - it resulted in an incorrect error message for a different edge
case (the unrelated error message that your testcase patch modified),
but it was enough to get me to the right patch.

I've (hopefully) fixed the problem in r11752.

Thanks again,
Russ Magee %-)

[rebus_]

2009/11/19 Russell Keith-Magee <freakb...@gmail.com>:

I was thinking of similar patch, but couldn't decided which way to go.
I guessed flatten_fieldsets would be more appropriate as it didn't
introduced yet another loop, but you're right. This is much better way
to handle it since the other testcase still passes as it should.

Thanks for fixing this.

--
Davor Lučić