Opened 9 years ago

Last modified 8 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.py minimally for the above to work.
  • Tests not added for get_updated_model() nor get_updated_models() because the changes are small enough to be considered as a refactoring.
  • Both the tests and contrib.auth have been modified: any call to save(commit=False) for the purpose of obtaining a populated model has been replaced by get_updated_model(). Tests still pass, I'm confident it's a successful refactor.
  • docs/customizing.txt has 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.

Change History (0)

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