#25227: Add utility method `get_updated_model()` to `ModelForm`
-------------------------------------+-------------------------------------
Reporter: candeira | Owner: candeira
Type: | Status: assigned
Cleanup/optimization |
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by candeira:
Old 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.
New 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.
The PR at https://github.com/DjangoGirls/tutorial/pull/450 depends on the
present patch. Its changeset there explains the rationale for this
addition better than anything else.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25227#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.485f5d3f71b524b3f45bb9ab79aae46b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
