#19774 closed Cleanup/optimization (fixed)
contentypes generic module has core functionality plus admins-specific one
Reported by: | Ramiro Morales | Owned by: | Ramiro Morales |
---|---|---|---|
Component: | contrib.contenttypes | Version: | dev |
Severity: | Normal | Keywords: | dependency generic contenttypes |
Cc: | loic@…, Simon Charette | 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 (last modified by )
(Copied and adapted from https://code.djangoproject.com/ticket/16368#comment:16)
Consider a project with a foo
and a bar
apps listed on INSTALLED_APPS
:
foo
uses generic foreign keys, with the followingmodels.py
:from django.db import models from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes import generic class TaggedItem(models.Model): tag = models.SlugField() content_type = models.ForeignKey(ContentType) object_id = models.PositiveIntegerField() content_object = generic.GenericForeignKey()
bar
has aSite
model:from django.db import models class Site(models.Model): name = models.CharField(... # ...
- Neither the admin nor sites Django apps are being used.
This causes the user's Site model to be overridden and masked by Django sites framework's one.
This is because the django/contrib/contenttypes/generic.py
module contain both the definitions of the model-related generic stuff (GenericForeignKey, etc.) AND the admin app-related specialized inlines (GenericInlineModelAdmin, GenericStackedInline, GenericTabularInline.)
Maybe it's time we move the latter ones from django.contrib.contenttypes.generic
to, say, a new generic_admin
(or admin_tools
?) on django.contrib.contenttypes
?. Of course this would need a deprecation process.
Change History (15)
comment:1 by , 12 years ago
Keywords: | dependency generic contenttypes added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
comment:2 by , 12 years ago
Description: | modified (diff) |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:3 by , 11 years ago
Cc: | added |
---|
comment:4 by , 11 years ago
comment:5 by , 11 years ago
Cc: | added |
---|
comment:6 by , 11 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Patch needs improvement: | set |
@ramiro, I created a PR missing release notes and documentation adjustment. I'll try to work on it in the new few days but if you want to pick it up from here feel free to do it.
comment:7 by , 11 years ago
Patch needs improvement: | unset |
---|
I adjusted the documentation and added a release note. The PR should be ready for review.
comment:8 by , 11 years ago
Needs documentation: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Looks good to me, I'm pretty happy with the forms.py
, fields.py
, admin.py
split up.
comment:9 by , 11 years ago
Looks good to me; good idea to remove everything from that module and just deprecate the module, that's a lot simpler than the previous approach.
I won't claim to have reviewed all the minor changes to docs and tests in detail, but I assume if you've run the tests and they are warning-free that that was all done correctly. Looks good to me!
comment:11 by , 11 years ago
Simon,
I had some unfinished work in that direction and was a bit uncomfortable about still having the formset definition in the new fields module. Good call on creating forms.py
Please go ahead, my review didn't find anything.
comment:12 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
As discussed on IRC with ramiro and loic splitting the
generic
module intofields
andadmin
and deprecating the former should fix the issue.I couldn't reproduce the
Site
cloaking with you described example against 1.6 and master. The only way I managed to reproduce is by having asites
app, just like it's described in #16368.