#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
-------------------------------------+-------------------------------------
Comment (by candeira):
Replying to [comment:4 akaariai]:
> These methods seem like a good addition. The save(do_not_save=True) API
isn't the best one in Django...
I'm glad I'm not the only one to see that.
> However we do need dedicated tests for the new methods as they are part
of public API. Also, we need dedicated documentation for the new methods.
Right. I'll get to both immediately. I thought the documentation would be
picked up from the docstrings. I'll ask on IRC where to add the
documentation for the new method if I can't find where.
> I don't see what side effects save(commit=False) has when compared to
get_updated_model[s]()?
First of all, thanks for your questions, which have made me check the
repo again, and find required changes for this ticket which I had missed.
Also, I'm kind of new to Django, so I may be wrong here... but I don't
think I am.
Two reasons I think there are side effects:
1. The code in the tests calls `save(commit=False)` in two different ways,
which makes me think that whoever wrote that code knows there are two
reasons to call it.
{{{
./src/django$ rgrep "save(commit=False)"
(...)
tests/auth_tests/test_forms.py: form.save(commit=False)
tests/auth_tests/test_forms.py: form.save(commit=False)
tests/auth_tests/test_forms.py: form.save(commit=False)
tests/auth_tests/test_forms.py: form.save(commit=False)
tests/model_forms/tests.py: price = form.save(commit=False)
tests/model_forms/tests.py: c1 = f.save(commit=False)
tests/model_forms/tests.py: new_art = f.save(commit=False)
tests/model_forms/tests.py: doc = form.save(commit=False)
tests/model_forms/tests.py: doc = form.save(commit=False)
tests/model_forms/tests.py: doc = form.save(commit=False)
(...)
}}}
Sometimes the method is called for its return value (an instance of
`django.model`), and some times it's not assigned to anything, which makes
me think that the author of the code knows there are side effects in play.
2. From reading the code while writing the patch, I remember that, for
instance, a ModelForm has an associated Model instance, which gets
updated, e.g., with a save_m2m method that knows what information it
should commit when `form.save()` or the associated model's instance's
`save()` is called with the default `commit=True`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25227#comment:6>
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.56e87ff84a4464d6c368f336ffcaf54f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
