Opened 17 years ago

Closed 17 years ago

#3258 closed enhancement (fixed)

[patch] Make newforms.models.save_instance work with a subset of model fields

Reported by: adurdin@… Owned by: Adrian Holovaty
Component: Forms Version:
Severity: normal Keywords:
Cc: alex@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The save_instance() function in newforms.models expects clean_data to have a key/value pair for every field in the model. This needlessly limits one from using a form to update only part of a model.

The attached patch adds a check that the field name is in the clean_data dict before modifying an instance's value for that field.

Attachments (3)

savesubset.diff (540 bytes ) - added by Andrew Durdin <adurdin@…> 17 years ago.
patch
save_instance.patch (1.6 KB ) - added by Chris Beaven 17 years ago.
save_instance.2.patch (1.4 KB ) - added by Chris Beaven 17 years ago.

Download all attachments as: .zip

Change History (9)

by Andrew Durdin <adurdin@…>, 17 years ago

Attachment: savesubset.diff added

patch

comment:1 by mir@…, 17 years ago

Needs documentation: set
Needs tests: set
Triage Stage: UnreviewedAccepted

Thanks, Andrew. I'm not sure, but I *think* that a sentence should be added in the docs about this.

comment:2 by Chris Beaven, 17 years ago

This is much better than the current behaviour. save_instance shouldn't require all of the fields in the form.

comment:3 by Chris Beaven, 17 years ago

Needs documentation: unset

save_instance isn't documented anyway, so this ticket doesn't specifically need documentation yet.

by Chris Beaven, 17 years ago

Attachment: save_instance.patch added

by Chris Beaven, 17 years ago

Attachment: save_instance.2.patch added

comment:4 by Chris Beaven, 17 years ago

Triage Stage: AcceptedReady for checkin

I'm not sure that this specific ticket needs tests either since there are no existing tests for save_instance. I'm going to promote to checkin for Adrian to have a look at anyway.

comment:5 by anonymous, 17 years ago

Cc: alex@… added

comment:6 by Malcolm Tredinnick, 17 years ago

Resolution: fixed
Status: newclosed

Looks like this is the same as #3635. I committed a patch for the latter in [4878].

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