Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#6002 closed (fixed)

Better saving in newforms-admin ModelAdmin

Reported by: Petr Marhoun <petr.marhoun@…> Owned by: jacob
Component: contrib.admin Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

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)

00-admin-better-saving.diff (11.0 KB) - added by Petr Marhoun <petr.marhoun@…> 6 years ago.
00-admin-better-saving.2.diff (11.1 KB) - added by Petr Marhoun <petr.marhoun@…> 6 years ago.
new version - after autoescape merge
00-admin-better-saving.3.diff (11.1 KB) - added by Petr Marhoun <petr.marhoun@…> 6 years ago.
new version - after autoescape merge
00-admin-better-saving.4.diff (11.1 KB) - added by Petr Marhoun <petr.marhoun@…> 6 years ago.
00-admin-better-saving.5.diff (12.0 KB) - added by shnur 6 years ago.
Patch cleanly applies to revision 7875

Download all attachments as: .zip

Change History (17)

Changed 6 years ago by Petr Marhoun <petr.marhoun@…>

Changed 6 years ago by Petr Marhoun <petr.marhoun@…>

new version - after autoescape merge

Changed 6 years ago by Petr Marhoun <petr.marhoun@…>

new version - after autoescape merge

comment:1 Changed 6 years ago by brosner

  • Keywords nfa-blocker added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

This should be solved with the use of ModelForm once it is integrated into ModelAdmin.

Changed 6 years ago by Petr Marhoun <petr.marhoun@…>

comment:2 Changed 6 years ago by peschler

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 Changed 6 years ago by garcia_marc

  • milestone set to 1.0 alpha

Changed 6 years ago by shnur

Patch cleanly applies to revision 7875

comment:4 Changed 6 years ago by shnur

  • Owner changed from nobody to shnur
  • Status changed from new to assigned

comment:5 Changed 6 years ago by brosner

  • Keywords nfa-blocker removed
  • milestone changed from 1.0 alpha to 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 Changed 6 years ago by jacob

#6406 and #8005 were marked as dups; they're not quite the same problem, but the problems there should be solved by a general refactoring of saving. Both have some interesting code, though.

comment:7 Changed 6 years ago by jacob

  • Owner changed from shnur to jacob
  • Patch needs improvement set
  • Status changed from assigned to new
  • Version changed from newforms-admin to SVN

comment:8 Changed 6 years ago by jacob

  • Status changed from new to assigned

comment:9 Changed 6 years ago by jacob

(In [8265]) Broke the admin's use of LogEntry and user messages out into callbacks on ModelAdmin. Refs #6002.

comment:10 Changed 6 years ago by jacob

(In [8266]) Added ModelAdmin.save_model() and ModelAdmin.save_formset() methods to allow for easier modification of objects/inlines at admin-save time. Refs #6002.

comment:11 Changed 6 years ago by jacob

  • Resolution set to fixed
  • Status changed from assigned to closed

[8266] should have said "fixed" instead of "refs"; this is done now.

comment:12 Changed 3 years ago by jacob

  • milestone 1.0 beta deleted

Milestone 1.0 beta deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.