Code

Opened 4 years ago

Closed 4 years ago

#12149 closed (invalid)

pre_save is not called before the overridden save() method on a model

Reported by: siddhi Owned by: nobody
Component: Uncategorized Version: 1.1
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by kmtracey)

If I have a model where I override the save() method, then the pre_save signal is not sent before the save method is called.

Example:

class MyModel(models.Model):
    name = models.CharField(max_length=20)

    def save(self, force_insert=False, force_update=False):
        if self.name == "dont_save":
            return
        super(Project, self).save(force_insert, force_delete)
def presave_handler(sender, instance, **kwargs):
    instance.name = "dont_save"

signals.pre_save.connect(presave_handler, sender=MyModel, dispatch_uid="abc")

In the above case, the flow goes like this

  1. call overridden save method
  2. check the condition in save method (condition is false)
  3. call super
  4. call pre_save
  5. set name to "dont_save"
  6. object saved to database with name = "dont_save"

This is rather unintuitive that the pre_save gets called in the middle of the save method. Also, any processing done in the pre_save cannot be handled in the save method as the flow has gone to the super class by then.

The expected flow should be like this

  1. call overridden save method
  2. call pre_save
  3. set name to "dont_save"
  4. execution enters save method
  5. check condition in overridden save method (condition is true)
  6. return without saving

Attachments (0)

Change History (3)

comment:1 follow-up: Changed 4 years ago by kmtracey

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

(Fixed formatting. Note you've got to put a space before the 1. in your lists to get them to format properly.)

The signals doc: http://docs.djangoproject.com/en/dev/ref/signals/#module-django.db.models.signals

notes that if you override save() you must call the parent class method in order for the signals to be sent. That makes it pretty clear the parent class code is what is going to send the signals. It isn't clear to me how you expect a signal to get sent at step 2 here:

  1. call overridden save method
  2. call pre_save
  3. set name to "dont_save".

At that point execution is in your own code, how is Django supposed to cause a signal to be sent?

comment:2 in reply to: ↑ 1 Changed 4 years ago by siddhi

  • Resolution invalid deleted
  • Status changed from closed to reopened

Replying to kmtracey:

It isn't clear to me how you expect a signal to get sent at step 2 here:

  1. call overridden save method
  2. call pre_save
  3. set name to "dont_save".


At that point execution is in your own code, how is Django supposed to cause a signal to be sent?

As currently implemented, the signal can't be sent. But it should. That is the bug :) Perhaps steps 1 and 2 should be exchanged in that flow.

The design for sending the signals needs to be changed. Maybe the metaclass should automatically decorate the save method to send the signal before and after, instead of sending it within the parent save(). The right design and alternatives can be discussed.

As it stands, you cant use the changes from the pre_save within the custom save method. By the time pre_save is called execution is already out of the custom save and in the superclass. This is very illogical. You would expect a signal called pre_save would be triggered before the save.

comment:3 Changed 4 years ago by Alex

  • Resolution set to invalid
  • Status changed from reopened to closed

The pattern you want is simply not possible with code, you need to manually send the signal (or some custom signal).

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.