Opened 14 years ago

Closed 14 years ago

#13772 closed (wontfix)

'exists' parameter for pre_save signal

Reported by: George Sakkis Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2
Severity: Keywords:
Cc: George Sakkis Triage Stage: Design decision needed
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As Model.save() is called both for inserts and updates, it would sometimes be useful for pre_save listeners to know whether the instance exists or not. This information is sent with post_save but by then it may be too late (e.g. under some conditions you may want to prevent an insertion from happening).

Attachments (2)

pre_save.patch (6.0 KB ) - added by George Sakkis 14 years ago.
Patch against r13861
13772.patch (6.9 KB ) - added by George Sakkis 14 years ago.
Patch against r13975

Download all attachments as: .zip

Change History (11)

comment:1 by George Sakkis, 14 years ago

Has patch: set
Needs documentation: set

Patch (including updated tests) attached; needs docs, will add if accepted.

comment:2 by George Sakkis, 14 years ago

Cc: george.sakkis@… added
Component: UncategorizedDatabase layer (models, ORM)
milestone: 1.3

comment:3 by Preston Holmes, 14 years ago

Resolution: worksforme
Status: newclosed
Triage Stage: UnreviewedAccepted

The presave signal sends the instance of the object being saved. An unsaved instance will not yet have a pk, while an instance being updated will already have a pk.

Checking for a primary key on the instance will tell you whether the instance exists or not.

def my_callback(sender, instance=None, **kwargs):
    print "presave signal"
    if not instance.pk:
        print "not yet saved"
    else:
        print "exists, updating"

comment:4 by George Sakkis, 14 years ago

Resolution: worksforme
Status: closedreopened
Triage Stage: AcceptedUnreviewed

Sorry, this works only if the pk field is AutoIncrement and the pk has not been set explicitly beforehand (e.g. with obj = MyModel(pk=42, ...) or obj.pk = 42).

comment:5 by Erin Kelly, 14 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

The patch moves the sending of the signal to after the recursive save_base call. This means that when the signal is sent, any parent models will have already been saved, which is undesirable. The patch should keep the signal where it is relative to the recursion, if at all possible.

by George Sakkis, 14 years ago

Attachment: pre_save.patch added

Patch against r13861

in reply to:  5 comment:6 by George Sakkis, 14 years ago

Patch needs improvement: unset

The patch moves the sending of the signal to after the recursive save_base call. This means that when the signal is sent, any parent models will have already been saved, which is undesirable. The patch should keep the signal where it is relative to the recursion, if at all possible.

Updated according to suggestion and brought up to date with trunk (r13861).

by George Sakkis, 14 years ago

Attachment: 13772.patch added

Patch against r13975

comment:7 by George Sakkis, 14 years ago

Cc: George Sakkis added; george.sakkis@… removed
Triage Stage: AcceptedReady for checkin

Extended and migrated the doctests into proper unit tests.

comment:8 by Łukasz Rekucki, 14 years ago

Triage Stage: Ready for checkinDesign decision needed

This ticket's history is a mess right now. You shouldn't mark your own patches as RFC. For starters, it still needs documentation. After that, they need to be reviewed by someone else.

Also, based on comment from Jeremy Dunck on django-dev, I'm not so sure we can just do this without cost. I'm going to push this back to DDN.

comment:9 by George Sakkis, 14 years ago

milestone: 1.3
Resolution: wontfix
Status: reopenedclosed

Withdrawn after two negative and zero positive votes at http://groups.google.com/group/django-developers/browse_frm/thread/0c0dec77aea5d83a.

Note: See TracTickets for help on using tickets.
Back to Top