#4667 closed (fixed)
[newforms-admin] add edit_inline support for generic relations
Reported by: | Owned by: | Brian Rosner | |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Keywords: | nfa-someday edit_inline generic content_type sprintsept14 ep2008 | |
Cc: | paltman@…, simon@…, semente@…, peschler@…, prufrocks@…, smcoll@…, dev@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
I have a model that looks like (working example)
from django.db import models from django.contrib.contenttypes import generic from django.contrib.contenttypes.models import ContentType class Dependency( models.Model ): target_ct = models.ForeignKey( ContentType, related_name='dependency_for_set' ) target_id = models.IntegerField() target = generic.GenericForeignKey( 'target_ct', 'target_id' ) source_ct = models.ForeignKey( ContentType, related_name='dependent_on_set' ) source_id = models.IntegerField() source = generic.GenericForeignKey( 'source_ct', 'source_id' )
and I want to be able to edit it inline with other models, but the current implementations only support edit_inline for foreign key relations. With the attached patch I can do something like this:
from django.contrib import admin class SomeModel( models.Model ): text = models.TextField() class SomeModelOptions( admin.ModelAdmin ): inlines = [ admin.TabularInline( Dependency, name='source_ct:source_id', formset=generic.GenericInlineFormset ), ] admin.site.register( SomeModel , SomeModelOptions )
The patch is very rough, but working (tested adding and editing).
I have some questions for the authors (I will post it to django-dev):
- InlineFormset.rel_name is not really applicable in this case, what would be best to use as a prefix (the value in tha patch -
'aaa'
is far from ideal ;) ) - I moved some of the functionality to FormSet (add_fk), to allow for custom binding between objects and use
content_type_field:object_id_field
as the foreign key name to pass that information. I feel that its not the most elegant solution, could anybody help me out here with some ideas?
btw. Great work, Joseph Kocherhans, your implementation of edit_inline really made this easy for me, thanks.
Attachments (10)
Change History (38)
by , 17 years ago
Attachment: | newforms-admin-5519-generic-edit-inline.patch added |
---|
comment:1 by , 17 years ago
Just a minor style note: the preferred style for Django patches is to do function(arg)
, not function( arg )
, following the lead of PEP 8.
comment:2 by , 17 years ago
Patch needs improvement: | set |
---|
comment:3 by , 17 years ago
Cc: | added |
---|
comment:4 by , 17 years ago
Owner: | changed from | to
---|
by , 17 years ago
Attachment: | newforms-admin-5519-generic-edit-inline-sprint14sep.patch added |
---|
version matching django @6158
comment:5 by , 17 years ago
So first of all, the name='object_id:content_type'
syntax is just too dirty. It seems like we should be able to provide *just* the name of the GenericForeignKey
field, then use its ct_field
and fk_field
attributes to get what we need. If that isn't possible, I'd much rather use 2 different arguments, something like ct_field
and fk_field
.
Also, I *think* we could use the existing FormSet.add_fields
hook instead of adding a new add_fk
method like in the patch.
Keep in mind that I may be missing something here. These are my initial impressions, but they may not end up working in practice. I haven't worked all the way through it.
comment:6 by , 17 years ago
Keywords: | sprintsept14 added |
---|
comment:7 by , 17 years ago
Replying to jkocherhans:
So first of all, the
name='object_id:content_type'
syntax is just too dirty. It seems like we should be able to provide *just* the name of theGenericForeignKey
field, then use itsct_field
andfk_field
attributes to get what we need. If that isn't possible, I'd much rather use 2 different arguments, something likect_field
andfk_field
.
I dont want that because I often don't have GenericForeignKey
defined - it is a modular system and I do not want to modify every model that can use the generic relation. so for that I would have to override the InlineModelAdmin
as well to provide two options - ct_name
and id_name
, but that's certainly an option...
Also, I *think* we could use the existing
FormSet.add_fields
hook instead of adding a newadd_fk
method like in the patch.
The reason for this is that the patch existed before that hooks were added, I just did a small update to match the current version... I will have a look at that
Keep in mind that I may be missing something here. These are my initial impressions, but they may not end up working in practice. I haven't worked all the way through it.
comment:8 by , 17 years ago
Triage Stage: | Unreviewed → Accepted |
---|
by , 17 years ago
Attachment: | newforms-admin-6426-generic-edit-inline.patch added |
---|
Matching django version 6426
by , 17 years ago
Attachment: | newforms-admin-6477-generic-edit-inline.patch added |
---|
Matching django version 6477
comment:9 by , 17 years ago
I moved all the code except few things (enable formset
overriding and something to prevent circular imports) out of admin
and directly into contenttypes.generic
This will enable me to specify my generic inlines like
class ListingInlineOptions( generic.GenericTabularInline ): model = Listing extra = 2 ct_field_name = 'target_ct' id_field_name = 'target_id'
The downside is that I am duplicating some code in contenttypes.generic
There are few more things to do:
- Find better names than
id_field_name
andct_field_name
- Enable this to work with autodetection and/or
GenericForeignKey
- clean-up the code in
contenttypes.generic
(maybe refactor admin a bit to enable getting rid of some boilerplate code) - Enjoy :)
by , 17 years ago
Attachment: | 4667-added-custom-formset.diff added |
---|
new two-part patch for django @6657 (first part)
by , 17 years ago
Attachment: | 4667-generic_edit_inline.diff added |
---|
new two-part patch for django @6657 (second part)
comment:10 by , 17 years ago
I split the patch into two parts, one handling the custom formset
overrides, the second actually implementing the feature.
comment:11 by , 17 years ago
Keywords: | nfa-someday added |
---|
This functionality is not critical before the merge to trunk. Tagging with nfa-someday.
comment:12 by , 17 years ago
Cc: | added |
---|
comment:13 by , 16 years ago
Cc: | added |
---|
comment:14 by , 16 years ago
Cc: | added |
---|
comment:15 by , 16 years ago
Cc: | added |
---|
comment:16 by , 16 years ago
Cc: | added; removed |
---|
by , 16 years ago
Attachment: | 4667-generic-edit-inline.patch added |
---|
Cleaned up previous patch to work with existing code base (r7771).
comment:17 by , 16 years ago
Cc: | added |
---|
comment:18 by , 16 years ago
The latest patch has the code in django/contrib/admin/options.py
which is a wrong place for the code. It should have a nice home in
django/contrib/contenttypes
. Not sure where but I am thinking either
inlines.py
or
admin.py
.
by , 16 years ago
Attachment: | 4667-against-7875.patch added |
---|
comment:19 by , 16 years ago
Keywords: | ep2008 added |
---|
by , 16 years ago
Attachment: | 4667-against-7875.2.patch added |
---|
Patch modified - save_as_new argument missing in init
comment:20 by , 16 years ago
Modified constructor (added save_as_new argument), patch 4667-against-7875.2.patch works well on rev. 7877, all tests passed.
Please change your admin options if using example at the top of this page:
class SomeInline( generic.GenericTabularInline ): model = Dependency ct_field_name = 'source_ct' id_field_name = 'target_id' class SomeModelOptions( admin.ModelAdmin ): inlines = [ SomeInline ]
by , 16 years ago
Attachment: | 4667-against-7875.3.patch added |
---|
can_delete and can_order update for the patch
comment:21 by , 16 years ago
in my last attachment I just added same default values for can_delete
and can_order
like there are in django.newforms.models.inlineformset_factory
comment:22 by , 16 years ago
[source:django/branches/newforms-admin/django/newforms/models.py@7899#L450 django.newforms.models.inlineformset_factory]
comment:23 by , 16 years ago
Cc: | added |
---|
comment:24 by , 16 years ago
Version: | newforms-admin → SVN |
---|
comment:25 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:26 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:27 by , 16 years ago
Cc: | removed |
---|
rough version of the patch