Opened 14 years ago
Closed 14 years ago
#15011 closed (invalid)
An improvement of GenericForeignKey which behaves badly in some a bit uncommon but possible situations
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Contrib apps | Version: | 1.2 |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The fk_field associated with GenericForeignKey field gets updated upon assigning an object to GFK field and initialization of the parent model (via kwargs). In my opinion it should also get updated when the parent model is saved.
Suppose we have a model (A) with GFK field. We create an instance (b) of some model (B) and assign it to the GFK field of an instance of model A (a). If a wasn't saved prior to the assigning then the fk_field will be None when saving this instance of A. If we don't have NOT NULL (null=True) constraints on the fk_field then the a will be saved with fk_field=NULL thus failing silently (I don't consider this to be a proper nor logical behaviour.
My proposition is to add a hook to pre_save signal of the parent model in which we update the fk_field if not set. If the the GFK field has an object assigned already but it isn't saved yet then we throw an exception.
I am attaching my improved version of the GFK class (via subclassing) - it should be trivial to merge if you deem it worthy.
Regards,
Filip Sobalski
Attachments (1)
Change History (3)
by , 14 years ago
Attachment: | generic.py added |
---|
comment:1 by , 14 years ago
comment:2 by , 14 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
Firstly - please upload changes as diffs, not blocks of code that we have to guess where to integrate.
Secondly - test cases aren't optional. If you want use to take you seriously, you need to provide programatic test cases integrated into Django's own test suite.
Lastly, the change you propose would make GFKs inconsistent with FKs.
Of course it should be 'b':
If b wasn't saved prior to the assigning then the fk_field will be None when saving this instance of A.