Opened 17 years ago
Last modified 13 years ago
#5450 closed
save_m2m should raise an exception for unsaved models — at Version 3
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
object = form.save(commit=False) object.foofield = 'something' object.save() form.save_m2m() # would save to a unsaved instance form.save_m2m(object) # would be better
(See below for more discussion -JKM)
Change History (4)
by , 17 years ago
Attachment: | save_m2m_with_new_instance.diff added |
---|
comment:2 by , 17 years ago
I can see the problem you're trying to avoid, but this isn't the right approach. The m2m data from a form should only ever be used with a very specific instance - allowing users to specify an arbitrary instance isn't the way to solve the problem. Plus, the following would still be possible:
object = form.save(commit=False) object.foofield = 'something' form.save_m2m(object)
which is just as invalid.
The better solution here would be to put in a check at the start of save_m2m to see if the object has been saved. A simple pk check would probably suffice, raising an exception if the object isn't saved.
comment:3 by , 17 years ago
Description: | modified (diff) |
---|---|
Summary: | Save_m2m in forms created by form for model should take a instance argument. → save_m2m should raise an exception for unsaved models |
Triage Stage: | Unreviewed → Accepted |
Agreed with Russell -- save_m2m() is very specific to that instance; better behavior would be to raise an error if the object doesn't have a pk. I'm coopting this ticket to handle that issue instead.