save_instance raises KeyError when form_from_{model,instance} is supplied a formfield_callback that does not return a formfield

[Matias Hermanrud Fjeld <mhf@…>]

I have a custom user class:

class User(model.Model):
  first_name = models.CharField(maxlength=50)
  last_name = models.CharField(maxlength=50)
  [snip]
  company = models.ForeignKey(Company)

Users should be able to change all fields except company. Attached is a patch implementing an exclude-keyword to django.newforms.models.form_for_{model,instance}.

[Matias Hermanrud Fjeld <mhf@…>]

diff against r4607.

[Matias Hermanrud Fjeld <mhf@…>]

If/when this is approved by the devs, I'll write tests and documentation.

(I hope this is the right way to proceed. If not, I apologize.)

[James Bennett]

I'm extremely wary of this -- in the old manipulator system we used follow, and it's been the bane of many a debugger (see #999 and all its related tickets, for example, which took far too long to finally nail down). And where writing a manipulator was pretty tedious, writing a custom form in newforms to grant fine-grained control over fields and behavior is pretty easy... I'm tempted to vote for form_for_model and form_for_instance just being fairly "dumb" default forms, and recommending custom forms when custom behavior is desired.

[Matias Hermanrud Fjeld <mhf@…>]

diff against r4741

[Matias Hermanrud Fjeld <mhf@…>]

I've attached a new patch, newforms_save_instance.diff. This changes django.newforms.models.save_instance to only update fields that have corresponding data in clean_data. With this applied, I can write:

form_for_model(User, formfield_callback=lambda f, **kwargs: f.name in ('company', 'someotherfield') and f.formfield(kwargs**))

It seems to me that adrian intended the the formfield callback to be able to keep fields out of the form by returning a value evaluating to false. From django/newforms/models.py, lines 97-99:

        formfield = formfield_callback(f, initial=current_value)
        if formfield:
            field_list.append((f.name, formfield))

The problem is that if there is no field in the form corresponding to a field in the model, save_instance raises a KeyError. This patch fixes this, but would break code that depends on this behavior.

Maybe adrian could comment on how he intended this to work?

[Matias Hermanrud Fjeld <mhf@…>]

I have found a better way for writing this. More code, but more flexible:

class MyForm(forms.Form):
    field1 = MyModel._meta.get_field('field1').formfield()
    field2 = MyModel._meta.get_field('field2').formfield()

    def save(self, **kwargs):
        instance = MyModel(**dict(self.clean_data, **kwargs))
        instance.save()
        return instance

Then views can do:

form = MyForm(request.POST)
if form.is_valid():
  instance = form.save(user=request.User)
  ...

[Matias Hermanrud Fjeld <mhf@…>]

Ubernostrum: You are absolutely right, form_for_{model,instance} should be just that, nothing more. I'm closing this, but it would be nice to update the newforms-documentation with something like the example above.