Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#13095 closed (fixed)

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

Reported by: hvdklauw Owned by: carljm
Component: Forms Version: master
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: UI/UX:

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 hvdklauw 4 years ago.
Bug in inlineformset_factory patch
ticket13095_r13294.diff (3.6 KB) - added by devinj 4 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 aldaran 4 years ago.
for the current trunk 13307
13095-formfield_callback-r13517.diff (7.5 KB) - added by vung 4 years ago.
Use None as default value for formfield_callback (patch from #13633+tests)

Download all attachments as: .zip

Change History (21)

Changed 4 years ago by hvdklauw

Bug in inlineformset_factory patch

comment:1 Changed 4 years ago by kmtracey

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset

comment:2 Changed 4 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 4 years ago by russellm

  • milestone 1.2 deleted

Not critical for 1.2

Changed 4 years ago by devinj

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

comment:4 Changed 4 years ago by devinj

  • Needs tests unset
  • Summary changed from inlineformset_factory formfield_callback lamba function missing **kwargs to modelform_factory, modelformset_factory, inlineformset_factory formfield_callback lamba function missing **kwargs

comment:5 Changed 4 years ago by hvdklauw

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.

Changed 4 years ago by aldaran

for the current trunk 13307

comment:6 Changed 4 years ago by aldaran

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 Changed 4 years ago by russellm

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 Changed 4 years ago by shauncutts

  • Cc shaun@… added

comment:9 Changed 4 years ago by russellm

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

comment:10 follow-up: Changed 4 years ago by 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?

comment:11 in reply to: ↑ 10 Changed 4 years ago by jk

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.

Changed 4 years ago by vung

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

comment:12 Changed 4 years ago by vung

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 Changed 4 years ago by frans

  • Cc francois@… added

comment:14 Changed 4 years ago by carljm

  • Keywords sprintSep2010 added
  • Owner changed from nobody to carljm
  • Summary changed from modelform_factory, modelformset_factory, inlineformset_factory formfield_callback lamba function missing **kwargs to modelform_factory, modelformset_factory, inlineformset_factory formfield_callback lambda function missing **kwargs
  • Triage Stage changed from Accepted to Ready 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 Changed 4 years ago by jbronn

  • Resolution set to fixed
  • Status changed from new to closed

(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 Changed 4 years ago by jbronn

(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 Changed 3 years ago by simon29

See related tickets #14572 and #14574

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.