Opened 8 years ago

Closed 8 years ago

#3922 closed (fixed)

formfield_callback in form_for_model also changes refering model

Reported by: Jure Cuhalev <gandalf@…> Owned by: PhiR
Component: Forms Version: master
Severity: Keywords: form_for_model
Cc: gandalf@…, cgrady@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Using formfield_callback with newforms form.form_for_model will also change the model you are refering to.

def CategoryCallback(f, **kwargs):
    if not "cat_" in f.name:
      f.name = 'cat_'+f.name
    return f.formfield(**kwargs)

>>> w = Writer(name='Bob Woodward')
>>> Article(headline='Test article', pub_date=datetime.date(1988, 1, 4), writer=w, article='Hello.')
<Article: Test article>

>>> ArticleForm = form_for_model(Article, formfield_callback=CategoryCallback)
>>> f = ArticleForm(auto_id=False)
>>> print f
<tr><th>Headline:</th><td><input type="text" name="cat_headline" maxlength="50" /></td></tr>
<tr><th>Pub date:</th><td><input type="text" name="cat_pub_date" /></td></tr>
<tr><th>Writer:</th><td><select name="cat_writer">
<option value="" selected="selected">---------</option>
</select></td></tr>
<tr><th>Article:</th><td><textarea name="cat_article"></textarea></td></tr>
<tr><th>Categories:</th><td><select multiple="multiple" name="cat_categories">
</select><br /> Hold down "Control", or "Command" on a Mac, to select more than one.</td></tr>
>>> w = Writer(name='Bob Woodward')

but then doing this one (same one as in the beginning):

>>> Article(headline='Test article', pub_date=datetime.date(1988, 1, 4), writer=w, article='Hello.')

fails with:

    Traceback (most recent call last):
      File "/Users/gandalf/django/django_src/django/test/doctest.py", line 1243, in __run
        compileflags, 1) in test.globs
      File "<doctest modeltests.model_forms_callback.models.__test__.API_TESTS[16]>", line 1, in ?
        Article(headline='Test article', pub_date=datetime.date(1988, 1, 4), writer=w, article='Hello.')
      File "/Users/gandalf/django/django_src/django/db/models/base.py", line 166, in __init__
        raise TypeError, "'%s' is an invalid keyword argument for this function" % kwargs.keys()[0]
    TypeError: 'writer' is an invalid keyword argument for this function

I have attached more comprehensive test derived from modeltests/model_form. I belive that form_for_model with callback should not affect the actual model. It is also not clear if by doing my approach .save() should still work normally or is up to the developer to rename fields back to the way they were originally defined.

Attachments (2)

models.py (3.4 KB) - added by Jure Cuhalev <gandalf@…> 8 years ago.
Test for described formfield_callback problem
3922.patch (704 bytes) - added by PhiR 8 years ago.
patch to newforms.txt to mention this problem

Download all attachments as: .zip

Change History (7)

Changed 8 years ago by Jure Cuhalev <gandalf@…>

Test for described formfield_callback problem

comment:1 Changed 8 years ago by Collin Grady <cgrady@…>

  • Cc cgrady@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Doesn't the prefix argument to a Form do the same thing, but without the side-effects? :)

Editing the model fields to produce changes in the form output seems a bit backwards - perhaps a custom base form with init overrided would be better than trying to do certain modifications in the callback?

comment:2 Changed 8 years ago by Jure Cuhalev <gandalf@…>

Yes, it probably would. But using it in this "wrong" way still shouldn't break the model in a running project.

comment:3 Changed 8 years ago by PhiR

There are no object/class permissions in python, there is no way to "fix" this. The documentation should mention that the argument passed to the callback is the actual class field and should not be treated as a temporary variable you can modify.

Changed 8 years ago by PhiR

patch to newforms.txt to mention this problem

comment:4 Changed 8 years ago by PhiR

  • Has patch set
  • Keywords form_for_model added; for_for_model removed
  • Owner changed from nobody to PhiR
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

comment:5 Changed 8 years ago by mtredinnick

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

(In [6148]) Fixed #3922 -- Added a warning that fields passed to form_for_model callback function should not be modified. It's bad for your health. Thanks, Philippe Raoult.

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