Changes between Version 10 and Version 11 of Ticket #25227


Ignore:
Timestamp:
Aug 10, 2015, 6:04:37 AM (9 years ago)
Author:
Javier Candeira
Comment:

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #25227

    • Property Summary Add utility method `get_updated_model()` to `ModelForm` → Remove need for `ModelForm.save(commit=False)`

  • Ticket #25227 – Description

    v10 v11  
    1 Add utility method get_updated_model() to ModelForm
    2 
    3 Additionally, add utility method get_updated_models() to FormSet
    4 
    5 Rationale:
    6 
    71While 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:
    82
    93  - It's counterintuitive. To a newcomer, it's the same as ``save(save=no)``.
    10   - 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.
     4  - It's a leaky abstraction. Understanding it requires understanding that the `save()` method does two things: a) return the model, and b) save it to the database.
    115  - It doesn't name its effect or return well.
    126
    13 All these problems are addressed in the current patch. Changes:
     7The first two issues are addressed in the current patch. About the third, the mailing list discussion convinced me to avoid the `.get_updated_model()` name. Instead, `apply()` was used.
    148
    15  - Implement ModelForm.get_updated_model()
    16  - Implement FormSet.get_updated_models()
    17  - Refactor ModelForm.save() and FormSet.save() to allow the above.
    18  - 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.
    19 - New tests have been added for the new methods in different contexts (from a ModelForm, from a FormSet, etc).
     9Changes:
     10
     11 - Implement `ModelForm.apply()` as alternative to `.save(commit=False)`
     12 - Implement `FormSet.apply()` as alternative to `.save(commit=False)`
     13 - Refactor `ModelForm.save()` and `FormSet.save()` to allow the above.
     14 - New tests have been added for the new methods in different contexts (from a ModelForm, from a FormSet, etc).
    2015 - documentation has also been modified to showcase the new methods.
    2116
    2217Notes:
    2318
    24 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.
     19Uses of either `.save(commit=False)` in the codebase have been left alone wherever it was used for its side effects (the return value was not used).
    2520
    2621The Djangogirls tutorial has a PR that depends on the changes in the present one:
Back to Top