Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#13095 closed (fixed)

modelform_factory, modelformset_factory, inlineformset_factory formfield_callback lambda function missing **kwargs

Reported by: Harro Owned by: Carl Meyer
Component: Forms Version: dev
Severity: Keywords: inlineformset_factory sprintSep2010
Cc: shaun@…, francois@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using the inlineformset_factory with a custom modelform class which has the widgets dict defined in the meta class for one of the fields I ran into the following error.

<lambda>() got an unexpected keyword argument 'widget'

Some debugging later I discovered the formfield_callback definition for the inlineformset_factory is missing the kwargs argument.

Attachments (4)

ticket13095_r12761.diff (636 bytes ) - added by Harro 14 years ago.
Bug in inlineformset_factory patch
ticket13095_r13294.diff (3.6 KB ) - added by devinj 14 years ago.
Updated patch, also fixes modelform_factory and modelformset_factory. Adds tests. Removes encoding problems of previous attempt.
ticket13095_r13307.diff (3.6 KB ) - added by Simone 14 years ago.
for the current trunk 13307
13095-formfield_callback-r13517.diff (7.5 KB ) - added by vung 14 years ago.
Use None as default value for formfield_callback (patch from #13633+tests)

Download all attachments as: .zip

Change History (21)

by Harro, 14 years ago

Attachment: ticket13095_r12761.diff added

Bug in inlineformset_factory patch

comment:1 by Karen Tracey, 14 years ago

Needs tests: set

comment:2 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

comment:3 by Russell Keith-Magee, 14 years ago

milestone: 1.2

Not critical for 1.2

by devinj, 14 years ago

Attachment: ticket13095_r13294.diff added

Updated patch, also fixes modelform_factory and modelformset_factory. Adds tests. Removes encoding problems of previous attempt.

comment:4 by devinj, 14 years ago

Needs tests: unset
Summary: inlineformset_factory formfield_callback lamba function missing **kwargsmodelform_factory, modelformset_factory, inlineformset_factory formfield_callback lamba function missing **kwargs

comment:5 by Harro, 14 years ago

I've tested devinj's patch, the regression test fails without the fix, it succeeds with the fix.

From my own code I can confirm that the patch works.

by Simone, 14 years ago

Attachment: ticket13095_r13307.diff added

for the current trunk 13307

comment:6 by Simone, 14 years ago

Ok is not critical for 1.2,

but which is the best solution to have a workaround?
We cannot use the Meta widgets = {} in the form...
Ok, we can override the init method and override the widget.

comment:7 by Russell Keith-Magee, 14 years ago

I'll probably end up fixing this as part of #13633, since there will be some overlap (especially when it comes to adding tests)

comment:8 by Shaun Cutts, 14 years ago

Cc: shaun@… added

comment:9 by Russell Keith-Magee, 14 years ago

#13633 has been closed wontfix, so this problem needs to be solved on its own. @kenth provided another patch on that ticket.

comment:10 by jk, 14 years ago

I get an error when I try to apply the patch

patch --dry-run -p1 < ~/Downloads/ticket13095_r12761.diff 
patching file forms/models.py
Hunk #1 FAILED at 797.
1 out of 1 hunk FAILED -- saving rejects to file forms/models.py.rej

Any ideas?

in reply to:  10 comment:11 by jk, 14 years ago

Replying to jk:

I get an error when I try to apply the patch

patch --dry-run -p1 < ~/Downloads/ticket13095_r12761.diff 
patching file forms/models.py
Hunk #1 FAILED at 797.
1 out of 1 hunk FAILED -- saving rejects to file forms/models.py.rej

Any ideas?

My apologies.
I tried applying the patch "ticket13095_r13307.diff" and it worked. It did complain about the missing test files, which I skipped.

by vung, 14 years ago

Use None as default value for formfield_callback (patch from #13633+tests)

comment:12 by vung, 14 years ago

Having None as default for formfield_callback seems better to me, so I added some tests to the patch provided by @kenth in #13633.

The patch is a bit different, though. If formfield_callback is None, then formfield() is called directly instead of creating a lambda that calls it.

Note that not all tests fail before the patch; some of them are there to ensure that changing the defaults to None doesn't change current behaviour.

comment:13 by frans, 14 years ago

Cc: francois@… added

comment:14 by Carl Meyer, 14 years ago

Keywords: sprintSep2010 added
Owner: changed from nobody to Carl Meyer
Summary: modelform_factory, modelformset_factory, inlineformset_factory formfield_callback lamba function missing **kwargsmodelform_factory, modelformset_factory, inlineformset_factory formfield_callback lambda function missing **kwargs
Triage Stage: AcceptedReady for checkin

Verified that the latest patch (from vung) applies cleanly, fixes the reported problem, improves code maintainability (the same non-trivial default should not be repeated several different places, or we're asking for this bug to come back for a repeat visit), and adds useful test coverage (I verified that messing with any of the formfield_callback defaults now causes at least one test failure).

Entire test suite passes with the patch (tested only on sqlite, but it's not an ORM patch).

Marking ready-for-checkin. (Feel free to slap me if this is overstepping things).

comment:15 by jbronn, 14 years ago

Resolution: fixed
Status: newclosed

(In [13730]) Fixed #13095 -- formfield_callback keyword argument is now more sane and works with widgets defined in ModelForm.Meta.widgets. Thanks, hvdklauw for bug report, vung for initial patch, and carljm for review.

comment:16 by jbronn, 14 years ago

(In [13731]) [1.2.X] Fixed #13095 -- formfield_callback keyword argument is now more sane and works with widgets defined in ModelForm.Meta.widgets. Thanks, hvdklauw for bug report, vung for initial patch, and carljm for review. Backport of r13730 from trunk.

comment:17 by Simon Litchfield, 14 years ago

See related tickets #14572 and #14574

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