Opened 7 months ago
Closed 6 months ago
#36295 closed Bug (fixed)
Unable to Override GenericForeignKey in Inherited Abstract Class
| Reported by: | Gagan Deep | Owned by: | Ahmed Nassar |
|---|---|---|---|
| Component: | contrib.contenttypes | Version: | 5.2 |
| Severity: | Normal | Keywords: | |
| Cc: | Gagan Deep | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Attempting to override a GenericForeignKey field in an inherited abstract model does not work as expected. According to the Django documentation, it should be possible to hide a field inherited from an abstract model by setting it to None.
However, this does not seem to work for GenericForeignKey.
I have created a Django project for quick testing.
Steps to replicate:
- Define an abstract model with a GenericForeignKey field.
- Create another abstract model that inherits from the first and attempt to override the GenericForeignKey field by setting it to None.
- Define a concrete model that inherits from the second abstract model.
Code Example
from django.contrib.contenttypes.fields import GenericForeignKey
from django.contrib.contenttypes.models import ContentType
from django.db import models
class AbstractBaseModel(models.Model):
content_type = models.ForeignKey(
ContentType, on_delete=models.SET_NULL, null=True, blank=True
)
object_id = models.PositiveIntegerField(null=True, blank=True)
related_object = GenericForeignKey("content_type", "object_id")
description = models.TextField(null=True, blank=True)
class Meta:
abstract = True
class AbstractDerivedModel(AbstractBaseModel):
related_object = None # Override GenericForeignKey
description = None
class Meta:
abstract = True
class ConcreteEntity(AbstractDerivedModel):
name = models.CharField(max_length=255)
class Meta:
abstract = False
Expected Behavior:
The related_object field in AbstractDerivedModel should override the GenericForeignKey in AbstractBaseModel, making it effectively absent in ConcreteEntity.
Actual Behavior:
Django still considers the GenericForeignKey field from AbstractBaseModel when defining ConcreteEntity, leading to unexpected behavior.
Additional Notes:
- Overriding standard model fields with
Noneworks as expected, butGenericForeignKeydoes not follow the same behavior (descriptionfield in the above example). - There is no explicit mention in the documentation that GenericForeignKey is exempt from the field-hiding mechanism. I checked the following pages:
Change History (13)
comment:1 by , 7 months ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 7 months ago
| Component: | Database layer (models, ORM) → contrib.contenttypes |
|---|
comment:3 by , 7 months ago
Hello Sarah! Can I get some pointers how can I approach to solve this issue? I would like to attempt a fix.
comment:4 by , 7 months ago
[Ignore this fix, unit tests were failing. My bad, for rushing it]
This is my first time working with the Django source code. After reviewing the codebase, I believe the fix below might address the issue. However, I'm unsure whether this is the expected behavior that requires a documentation update or if it's an actual bug.
File db/models/base.py -- line 348
if field.column is not None: # the exact fix
field = copy.deepcopy(field)
if not base._meta.abstract:
field.mti_inherited = True
new_class.add_to_class(field.name, field)
Apologies, but I'm unsure how to present this from the codebase. It would be helpful if someone could guide me on how to share the diff, similar to the comments above. Thanks!
comment:5 by , 7 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:6 by , 7 months ago
| Has patch: | set |
|---|
comment:7 by , 6 months ago
The PR looks fine to me. Just waiting for some minor, optional adjustments before _ready for checkin_.
comment:8 by , 6 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
comment:10 by , 6 months ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Ready for checkin → Accepted |
comment:11 by , 6 months ago
| Patch needs improvement: | unset |
|---|
comment:12 by , 6 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Thank you!
Using the models as defined above, I was able to replicate
tests/generic_relations/tests.py