#6002 closed (fixed)
Better saving in newforms-admin ModelAdmin
| Reported by: | Owned by: | Jacob | |
|---|---|---|---|
| Component: | contrib.admin | Version: | dev |
| Severity: | Keywords: | ||
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description
There are two very long methods in django.contrib.admin.options.ModelAdmin - save_add and save_change. They do three different things - they save objects, they log what happened and they redirect. So I propose to divide them.
Method add_view would call save_add for saving, log_add for logging and redirect_after_save for redirection. Method change_view would call save_change for saving, log_change for logging and redirect_after_save for redirection.
Methods save_add and save_change would be useful for complex saving which depends on forms and inline formsets together. (It solves #4507.)
Methods log_add and log_change could be used for example for something as AuditModelAdmin or NoLogAdmin.
Method redirect_after_save would be something as render_change_form - technical code common for adding and changing, which is not interesting very much.
I feel that now redirection is quite broken, it does not check permissions correctly. My patch fixes it too.
Attachments (5)
Change History (17)
by , 18 years ago
| Attachment: | 00-admin-better-saving.diff added |
|---|
by , 18 years ago
| Attachment: | 00-admin-better-saving.2.diff added |
|---|
by , 18 years ago
| Attachment: | 00-admin-better-saving.3.diff added |
|---|
new version - after autoescape merge
comment:1 by , 18 years ago
| Keywords: | nfa-blocker added |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
This should be solved with the use of ModelForm once it is integrated into ModelAdmin.
by , 18 years ago
| Attachment: | 00-admin-better-saving.4.diff added |
|---|
comment:2 by , 18 years ago
Just some little addition: Add a post_url parameter to the proposed redirect_after_save method:
def redirect_after_save(self, request, new_obj, add=False, change=False, post_url="../")
Then use this to redirect when succesfully saved.
Rationale:
Consider a Menu model which contains MenuEntry models. In the admin I want to edit the Menus, MenuEntries should only be accessible through the admin view of the Menu. If a MenuEntry was edited I want to redirect to the Menu it is contained in, rather than to the MenuEntry overview. To achieve this I need to overwrite save_change (in newforms-admin trunk) or redirect_after_save (in the patch version) and duplicate the whole code of these functions. With the additional post_url parameter a simple wrapper would suffice.
comment:3 by , 17 years ago
| milestone: | → 1.0 alpha |
|---|
by , 17 years ago
| Attachment: | 00-admin-better-saving.5.diff added |
|---|
Patch cleanly applies to revision 7875
comment:4 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:5 by , 17 years ago
| Keywords: | nfa-blocker removed |
|---|---|
| milestone: | 1.0 alpha → 1.0 beta |
This actually really isn't critical for a merge. I am going to drop the nfa-blocker and bump to 1.0 beta. I will look at it later.
comment:6 by , 17 years ago
comment:7 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Patch needs improvement: | set |
| Status: | assigned → new |
| Version: | newforms-admin → SVN |
comment:8 by , 17 years ago
| Status: | new → assigned |
|---|
comment:9 by , 17 years ago
comment:10 by , 17 years ago
comment:11 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
[8266] should have said "fixed" instead of "refs"; this is done now.
new version - after autoescape merge