Opened 16 years ago
Closed 11 years ago
#7551 closed Bug (fixed)
GenericForeignKey fails if initialized to None during model init
Reported by: | Sam Bull | Owned by: | Bouke Haarsma |
---|---|---|---|
Component: | contrib.contenttypes | Version: | dev |
Severity: | Normal | Keywords: | contenttypes genericforeignkey |
Cc: | sbull@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Supposing you have a model with a GenericForeignKey...
class Example(models.Model): name = models.CharField() content_type = models.ForeignKey(ContentType, null=True, blank=True) object_id = models.PositiveIntegerField(null=True, blank=True) obj = GenericForeignKey()
If you initialize like this, everything is fine:
ex = Example(name='foobar')
But if you do this...
ex = Example(name='foobar', obj=None)
You get an exciting error:
Impossible arguments to GFK.get_content_type!
It looks like the GFK tries to initialize the content_type and object_id fields if its attribute is one of the ones passed to the init. In fact, it should follow the example of get and only initialize those fields if the argument passed to it is non-None.
Attachments (2)
Change History (17)
by , 16 years ago
Attachment: | fix_for_content_type_bug.diff added |
---|
comment:1 by , 16 years ago
Cc: | added |
---|
Gah, I realized my ct and obj_id fields in the example won't work with the GFK field unless I tell it what they're called. This is a bug in my example, my original code properly identifies the content type and object id fields to the GFK.
comment:2 by , 16 years ago
milestone: | → 1.0 |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 16 years ago
Owner: | changed from | to
---|
comment:4 by , 16 years ago
Owner: | removed |
---|
comment:5 by , 16 years ago
milestone: | 1.0 → post-1.0 |
---|
This feels like enough of an edge case (knowingly passing None
when you could just leave it out of the argument list to the constructor) that it can wait a bit.
comment:6 by , 16 years ago
I've tried to provide a better patch.
I agree that this is an unusual situation, but it can cause hard to debug errors. The problem is that you might be passing in an object via the model's constructor and not know if it's None or not.
Null GFK fields are allowed, so passing in a value that might be None should be allowed as well.
I do agree that this may not be needed for 1.0, but I wanted to highlight the problems this bug causes.
comment:8 by , 14 years ago
Component: | Contrib apps → contrib.contenttypes |
---|
comment:9 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:12 by , 12 years ago
Description: | modified (diff) |
---|
comment:14 by , 11 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
I've improved on this patch by also checking if None is allowed as the foreign key. Previous patches just ignored None, but that doesn't seem right as sometimes None is really invalid, for example when the GFK is non-optional. See my PR: https://github.com/django/django/pull/1753.
comment:15 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
GFK-init patch