generic inlines break javascript inline add button

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Make a ModelAdmin with a generic inline
2. Go to that ModelAdmin's add or change view
(3. Any other inlines on the same ModelAdmin will also be broken)

What is the expected output? What do you see instead?
I should be able to add inline forms. I can't because the add button doesn't do 
anything, because the js errors out before it gets that far. The error is:
link.attr("href") is undefined
jquery.grp_related_generic.js
 :: Line 57 ( var spliturl = link.attr('href').split('/');)

The issue here is that "link" is set to elem.next(), and elem is set to the 
TOTAL_FORMS input for the formset, while elem.next() is the INITIAL_FORMS 
input. Since inputs don't have hrefs, the current code will always fail.

What version of the product are you using? On what operating system?
r1399 on MacOS X (10.6.4) with Firefox 3.6.3/Safari 5.0.1/Chrome 5.0.342.9 beta

Please provide any additional information below.

Original issue reported on code.google.com by Stephen....@gmail.com on 10 Jan 2011 at 4:55

[GoogleCodeExporter]

and again ... can´t reproduce (this being the third ticket I´m not sure your 
setup is ok). I just did console.log(elem) on line 56 and console.log(link) on 
line 58 and TOTAL_FORMS is not showing up. hmmm.

http://code.google.com/p/django-grappelli/source/browse/trunk/grappelli/template
s/admin/change_form.html?r=1399#36
... I´m not sure how TOTAL_FORMS can be initialized with looking for 
input[name*='object_id']

sorry. any further details?

Original comment by sehmaschine on 10 Jan 2011 at 7:18

[GoogleCodeExporter]

In generic forms, all of the management form inputs are named with both 
content_type and object_id for a generic inline formset. i.e. 
input#id_philo-attribute-entity_content_type-entity_object_id-TOTAL_FORMS.

The specific code used to generate this prefix can be found at 
http://code.djangoproject.com/browser/django/trunk/django/contrib/contenttypes/g
eneric.py#L320.

The default values for ct_field.name and ct_fk_field.name are object_id and 
content_type, as defined at 
http://code.djangoproject.com/browser/django/trunk/django/contrib/contenttypes/g
eneric.py#L26.

Original comment by Stephen....@gmail.com on 13 Jan 2011 at 2:47

[GoogleCodeExporter]

I've attached a diff of the fix I've applied locally for this problem... 
essentially, everywhere you've got input[name*='object_id'], change it to 
input[name*='object_id'][name$='id'].
I don't know if this is how you'll ultimately want to implement it, though. It 
feels a bit hackish.

Original comment by Stephen....@gmail.com on 14 Jan 2011 at 4:17

Attachments:

• generic_fix.diff

[GoogleCodeExporter]

Your patch solves the add button problem for me. However in my setup the 
content_type selector doesn't get a onchange event registered to render the 
lookuplink.

Original comment by defaultw...@gmail.com on 14 Jan 2011 at 9:34

[GoogleCodeExporter]

Original comment by sehmaschine on 14 Jan 2011 at 9:42

• Changed state: Accepted

[GoogleCodeExporter]

from withing the javascript console it looks like its the 
closest('div[class*="object_id"]') that doesn't match.

Original comment by defaultw...@gmail.com on 14 Jan 2011 at 9:58

[GoogleCodeExporter]

see rev1401: I´ve uploaded a fix for the initial issue here (which is the 
lookups for object_id). please let me know if this works.

@defaultw...: if there´s a problem with the events not being registered, 
please open another ticket. thanks.

note: I´m not entirely happy with the lookups right now (fk, m2m, generic). 
it´d be a lot cleaner using a custom widget or at least a specific 
widget-class (like .vForeignKeyRelatedLookup). unfortunately, custom widgets 
for raw-id-fields are painstaking. overriding a widget-class only is not 
possible as far as I know.

Original comment by sehmaschine on 14 Jan 2011 at 2:53

[GoogleCodeExporter]

Yeah, that seems to solve the problem I was having with generic inlines being 
targeted, while retaining the generic lookup - which I'm only now realizing is 
the point of that function. :p no wonder I wasn't explaining the problem well. 
I'm somewhat concerned that any vIntegerField named 'object_id' would be 
targeted, while any generic lookup named without object_id would be ignored... 
a widget class would be better indeed. Unfortunately, I don't think that the 
admin has a concept of handling generic foreign keys at all... the closest I 
can find is this: 
https://github.com/alex/django/blob/generic-foreign-key-widget/django/contrib/ad
min/widgets.py
But that's pretty irrelevant since you'd only be able to get the admin to use 
the new class by overriding at least ModelAdmin.formfield_for_dbfield, which 
would be not fun.

Original comment by Stephen....@gmail.com on 14 Jan 2011 at 5:00

[GoogleCodeExporter]

found another way (although slightly more complex):

defining the related lookups within ModelAdmin (TabularAdmin, ...), we could 
add precise lookups:

fk_related_lookup_fields = ["fk_field_1","fk_field_2",]
m2m_related_lookup_fields = ["m2m_field_1"]
generic_related_lookup_fields = 
[["content_type","object_id"],["content_type_2","object_id_2"]]

the code (e.g. with change_form.html) would change from:

$("input.vIntegerField[name*='object_id']").grp_related_generic({lookup_url:"{% 
url grp_related_lookup %}"});

to something like:

var generic_related_lookup_fields = {{ 
adminform.model_admin.generic_related_lookup_fields|safe }};
$.each(generic_related_lookup_fields, function() {
    $("input[name$='" + this[1] + "']").grp_related_generic({content_type:this[0], lookup_url:"{% url grp_related_lookup %}"});
});

usually, I´m not happy with adding modeladmin-options for grappelli, but this 
might be an exception.

@stephen: the formfield_for_dbfield (resp. formfield_for_foreignkey and 
formfield_for_manytomany) does not handle generic relations.

please let me know what you think. 

thanks,
patrick

Original comment by sehmaschine on 17 Jan 2011 at 10:11

[GoogleCodeExporter]

re: formfield_for_dbfield. With some hacks, it could be made to handle generic 
relations, is more what I was trying to get at :p But that would be so ugly.

Using attributes on the model admin does seem appropriate... but also somewhat 
redundant. The fk and m2m fields are always raw id fields, already, right? 
Ideally, the raw_id_fields attribute could be used for GFKs as well, and one 
could just loop through that attribute to initialize everything. However, this 
would probably need adjustments to the ModelAdmin validation, which I'm not 
sure is even possible and which would definitely be a little ugly.

In any case, fk and m2m fields are probably already listed in raw_id_fields, 
and looking for their widget classes seems to work fine. Perhaps just have an 
attribute for generics, like:

raw_generic_fields = [("content_type", "object_id")]

?

Original comment by Stephen....@gmail.com on 19 Jan 2011 at 3:09

[GoogleCodeExporter]

fk- and m2m-field are NOT always raw-id-fields.

Original comment by sehmaschine on 19 Jan 2011 at 3:12

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Clarification: The fk and m2m fields that are going to be caught by the current 
javascript and converted to grappelli related lookup fields are always in 
raw_id_fields.

If you intentionally want to separate raw_id_fields from related lookup fields, 
then the attributes you suggested would make sense - though it might be more 
compact in some ways to use a dict:

related_lookup_fields = {
    'fk': ['field'],
    'm2m': ['other'],
    'generic': [['apples', 'oranges']]
}

Original comment by Stephen....@gmail.com on 19 Jan 2011 at 3:53

[GoogleCodeExporter]

good idea.

I think I prefer to seperate raw_id_fields from related_lookup_fields. problem 
is, that something like "raw_generic_fields" is strange (because generic-fields 
are always raw_generic_fields, no matter what).

Original comment by sehmaschine on 19 Jan 2011 at 3:56

[GoogleCodeExporter]

Original comment by sehmaschine on 2 Feb 2011 at 12:35

• Changed state: Started

[GoogleCodeExporter]

this issue should be solved with r1402, see
http://code.google.com/p/django-grappelli/source/detail?r=1402

please let me know if it works for you. I´ve tested with tabular- and 
stacked-inlines as well as generic-inlines. It also works on a changelist (with 
list_editables).

it´s been a bit more complex as expected since I´ve changed the 
jquery-lookups in order to look for the exact IDs of dom-elements.

you need to add related_lookup_fields to your admin-definition:

    related_lookup_fields = {
        'fk': ['fk', 'user'],
        'm2m': ['m2m'],
        'generic': [['rel_ct','rel_id']],
    }

of course, the fk- and m2m-fields need to be raw-id-fields as well.

Original comment by sehmaschine on 3 Feb 2011 at 3:06

[GoogleCodeExporter]

Yep. Seems to be working fine for me - which is to say, I'm not getting any js 
errors, and the problems I was having with generic inlines are no longer 
present. I haven't tried using the related_lookup_fields yet. I kind of wish 
that I didn't have to do that each time I wanted to use raw_id_fields... but 
ah, well. :-)

Original comment by Stephen....@gmail.com on 3 Feb 2011 at 3:29

[GoogleCodeExporter]

yes, I´d love to remove the lookups as soon as possible (hopefully it´ll be 
integrated with the next django-version). the js is overly complex and it 
should be implemented using widgets.

Original comment by sehmaschine on 3 Feb 2011 at 3:34

[GoogleCodeExporter]

should be solved with 2.3.1+

Original comment by sehmaschine on 16 Feb 2011 at 3:03

• Changed state: Fixed