#16115 closed New feature (fixed)
New hook to allow change objects after all inline formsets have been saved
Reported by: | Igor Sobreira | Owned by: | Jannis Leidel |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Today we have save_model()
and save_formset()
in ModelAdmin
, but there is no default place where we can change objects after the object and all related formsets have been saved.
You can override response_change()
and response_add()
, but I don't think that's a good place to deal with model objects, since the purpose of these methods is to send messages to user and return the correct HttpResponse
object.
I'm attaching a patch with the proposed hook, I think it's better to have a method that wraps save_model()
and save_formet()
than create an empty method in the end of add_view()
and change_view()
. But it's just a suggestion.
I can write docs if you like the idea.
Attachments (5)
Change History (19)
by , 13 years ago
Attachment: | new_hook_save_model_and_relations.diff added |
---|
comment:1 by , 13 years ago
Version: | 1.3 → SVN |
---|
comment:2 by , 13 years ago
Type: | Cleanup/optimization → New feature |
---|
comment:3 by , 13 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Triage Stage: | Unreviewed → Accepted |
comment:4 by , 13 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
by , 13 years ago
Attachment: | new_hook_save_model_and_related_with_tests.diff added |
---|
comment:5 by , 13 years ago
Thanks for the feedback julien. I agree with you that the method name is too long, specially because it has an "and". I'm not very good on giving names. What about "save_all", or "save_all_objects"?
I've added a new patch with tests, there is a unit test mocking external calls to make sure it calls the correct methods, and a more high level test, to make sure I can do what I want, that is edit all related objects on add_view and change_view.
I'll work on some docs for it later.
by , 13 years ago
Attachment: | new_hook_save_model_and_relations_with_tests_and_docs.diff added |
---|
comment:6 by , 13 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
by , 13 years ago
Attachment: | 16115.modeladmin-save-related.diff added |
---|
comment:7 by , 13 years ago
Thank you for your patch. I've made a number of modifications to it:
- Renamed the hook to
save_related()
. - Took the call to
save_model()
out of that hook so that it focuses solely on form/formset saving business. - Removed the test that was in
regressiontests.modeladmin
as it was testing internal implementation details, which is best to avoid doing. The other test verifies the external behaviour, which is enough here. - Tweaked the doc and added release notes.
Please take a look and see whether this addresses your original request. Thanks.
comment:8 by , 13 years ago
Great julien, your changes make a lot of sense. I was really in doubt about that unit test since i havent's seen other like that. Thanks!
comment:9 by , 13 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
julien, i'm reassigning the ticket to you, I think you pretty much finished it. (actually I don't have permission :)
comment:10 by , 13 years ago
Owner: | removed |
---|
Igors, no need to assign it to me ;)
I too feel that the work is pretty much done, but since I came up with a new API in the latest patch I'd prefer to get at least another person to cast their eyes on it before RFC'ing this ticket, just in case we missed something.
by , 13 years ago
Attachment: | 16115.modeladmin-save-related.2.diff added |
---|
Patch updated to latest trunk
comment:11 by , 13 years ago
Triage Stage: | Accepted → Ready for checkin |
---|---|
UI/UX: | unset |
I applied and reviewed the patch at r16452. It looks good to me. Updating to RFC.
comment:13 by , 13 years ago
Thanks guys, save_related is a great thing I've been waiting for a long time. But it seems there's something missing. Assuming that the inlines are part of the object, let's say for example, that I've edited the object and the inlines through the admin page and hit the "save" button. now I want to do some processing on the object itself after the inlines were saved. calling save_model won't help, as the inlines haven't been saved yet. wouldn't it be very useful to also pass "obj" to save_realted arguments?
comment:14 by , 13 years ago
@nivhaa: You should be able to access the main object via form.instance
from inside save_related()
.
This makes sense and was also requested in #16016 (closed as dupe). "save_model_and_relations" feels quite verbose but I can't think of a better name right this minute. It'd be useful if you could provide tests and doc so this ticket can be moved along.