Opened 10 years ago
Last modified 9 years ago
#25227 closed Cleanup/optimization
Add utility method `get_updated_model()` to `ModelForm` — at Initial Version
| Reported by: | Javier Candeira | Owned by: | nobody |
|---|---|---|---|
| Component: | Documentation | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Add utility method get_updated_model() to ModelForm and, additionally, add utility method get_updated_models() to FormSet.
Rationale:
While doing the djangogirls tutorial, I noticed that ModelForm.save(commit=False) is given to newcomers as a reasonable way to acquire a form's populated model. This is an antipattern for several reasons:
- It's counterintuitive. To a newcomer, it's the same as
save(save=no). - It's a leaky abstraction. Understanding it requires understanding that the save method does two things: a) prepare the model, and b) save it to the database.
- It doesn't name its effect or return well. It returns a model, not a form.
All these problems are addressed in the current patch. Changes:
- Implement
ModelForm.get_updated_model() - Implement
FormSet.get_updated_models() - Refactor
models.pyminimally for the above to work. - Tests not added for
get_updated_model()norget_updated_models()because the changes are small enough to be considered as a refactoring. - Both the tests and
contrib.authhave been modified: any call tosave(commit=False)for the purpose of obtaining a populated model has been replaced byget_updated_model(). Tests still pass, I'm confident it's a successful refactor. docs/customizing.txthas also been modified to showcase the new method.
Notes:
Uses of ModelForm.save(commit=False) in the codebase have been left alone wherever it was used for its side effects and not for its return value.