Opened 9 years ago

Closed 8 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
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: UI/UX:

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@…> 9 years ago.
patch
save_instance.patch (1.6 KB) - added by SmileyChris 8 years ago.
save_instance.2.patch (1.4 KB) - added by SmileyChris 8 years ago.

Download all attachments as: .zip

Change History (9)

Changed 9 years ago by Andrew Durdin <adurdin@…>

patch

comment:1 Changed 9 years ago by mir@…

  • Needs documentation set
  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

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

comment:2 Changed 8 years ago by SmileyChris

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

comment:3 Changed 8 years ago by SmileyChris

  • Needs documentation unset

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

Changed 8 years ago by SmileyChris

Changed 8 years ago by SmileyChris

comment:4 Changed 8 years ago by SmileyChris

  • Triage Stage changed from Accepted to Ready 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 Changed 8 years ago by anonymous

  • Cc alex@… added

comment:6 Changed 8 years ago by mtredinnick

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

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