Code

Opened 4 years ago

Closed 12 months ago

#13708 closed New feature (duplicate)

Improve ModelAdmin.save_model() [PATCH]

Reported by: sebastian_noack Owned by: nobody
Component: contrib.admin Version: 1.2
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The ModelAdmin class provides a save_model() method, which can be overriden easily to add some pre- or post-save hooks to the admin. This is a good think and essential for some custom admins. But this API seems to be incomplete.


  1. There is no delete_model() method. If you can change the behaviour of saving a model instance though the admin why not doing the same for deleting an instance?
  1. You can do anything in overriden save_model() methods, even skipping the save operation. Ususually custom form validation, is the way to go, if you want prevent the user from saving the data under some circumstences. But there are use cases, where you need a pre-save hook, which is only called after the form was validated successful, just before the model instance is saved into the database. This hook might fail for some reasons, and you don't want to save the instance in this case. You can implement it this way already, but the code calling save_model() does not care whether the model instance was saved succesful or not, right now. So an admin.LogEntry and an auth.Message object, claiming that the object was added/changed succesdful is created, doesn't matter if it was saved or not. This can be handled by letting save_model() and delete_model() return a boolean indicating, whether the instance was really saved or deleted.

Attachments (1)

admin_improve_save_model.patch (5.5 KB) - added by sebastian_noack 4 years ago.

Download all attachments as: .zip

Change History (8)

Changed 4 years ago by sebastian_noack

comment:1 Changed 4 years ago by jacob

  • Needs documentation set
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 4 years ago by sebastian_noack

You have set "Patch needs improvement". So please comment, what's wrong with the patch?

comment:3 Changed 4 years ago by anonymous

Hi, you might wanna take a look at #11108 too. You could steal some of the docs there… Though I am not sure yet whether I like that boolean flag or not…

comment:4 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:6 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:7 Changed 12 months ago by timo

  • Resolution set to duplicate
  • Status changed from new to closed

The delete_model hook was added in #11108. I don't think point 2 is worthwhile as it'll be backwards incompatible.

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.