multiple values nodereference field
iko - March 2, 2009 - 11:39
| Project: | Popups: Add and Reference |
| Version: | 6.x-2.0-alpha1 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | sirkitree |
| Status: | needs work |
Description
Hi !
I have a nodereference field allowing (unlimited) multiple value. When I create a new node, it is referenced twice (please see the attached screenshot) ; moreover, I can't add other items (clicking the "add another item" button has no result).
Config : Drupal 6.10, module version 6x-1.0-rc1, Firefox 3 (the problem also occurs in IE 7)... tell me if you need more ?
Thanks,
| Attachment | Size |
|---|---|
| bug1popupaddreference.jpg | 18.86 KB |
#1
I have added this to the list of known limitations.
#2
A fix for this has been checked into dev, and will be released in RC3.
#3
thanks !
#4
This fix is working for me except for when "unlimited" is selected. Creating a new node, on an autocomplete nodereference field the "add another item" button will not create anything beyond the 2 initial fields. Disabling Popups: Add and Reference module returns this field to it's expected behavior, so I believe this module is the culprit.
#5
Drat. This is why it is good to open separate issue for each bug. I saw and fixed one part of this issue, but missed the other part.
#6
Attached patch will create a link for each instance of the nodereference.
I think this is the first step required so that we can have accurate counts when we want to:
1) make sure that the node we created through the popup populates the nodereference field after creation
2) return a link after any subsequest request for more nodereference fields
#7
Can this patch also be combined with #397614: Append to #prefix and #suffix instead of overwrite ?
#8
Sure! Here's a re-roll.
#9
hrm. should we mark the other as duplicate in this case? I'm sure we do not want to keep track of this in multiple places.
#10
Sorry, this patch is against the latest alpha. marking as such.
#11
I marked it as a duplicate. I suppose the proper way would have been to see if either patch got in, and then to re-roll the other accordingly, but I thought the other patch seemed small and straightforward enough to merge into here...
#12
I agree. I just hope starbow does ;)
#13
OK, so in order for (2) above to work a change is necessary to cck's AHAH response callback. @see: #416074: drupal_alter() form instead of form_element in AHAH callback for the patch. After these two patches are applied, you will now get the following:
#14
OK, here's an update and why this is marked as 'needs work'.
As soon as you click 'Add another item' the Popups.originalSettings is overwritten and Popups.originalSettings.popups.originalPath is now equal to "content/js_add_more/story/field_story_photo" because this is the path that is called for the AHAH callback.
This makes so that when you click the 'Add Photo' popups link, the $_GET request's destination is incorrect:
So that after you add a node within the popup, you get this:
Not too sure how to resolve this just yet, but finding the problem is 80%, right? :-/
#15
would something like #360081: Pass Settings Locally to JS Behaviors fix this? will test monday... time to go home /whew
#16
Ok, I will need to look at this again when I have some more time.
#17
regarding #13 above:
We probably do not need to patch cck in order to do what we need here. Instead we need to keep in mind that when an AHAH callback is invoked, the only thing that is passed back is just the form element, not the whole form, and make sure that our form_alter() can work with that. In order to do that, we need to detect on an elemental basis against our variable_set() like so:
<?phpfunction popups_reference_form_alter(&$form, $form_state, $form_id) {
if ($form_id == 'content_field_edit_form' && $form['#field']['type'] == 'nodereference') {
...
}
elseif ((isset($form['type']) && $form['type']['#value'] .'_node_form' == $form_id) ||
variable_get('popups_reference_show_add_link_'. current(array_keys($form)), FALSE) == TRUE) {
// In the case of AHAH we need to remember that the $form variable is simply the element
// and not the whole form. Hence we check the key against our variable_set().
// Add the "Add New: Node Type" links.
...
}
}
?>
Here is a re-roll of the entire patch so far.
#18
I noticed you rolled in the patch from #308589: Autofill base form NodeReference field on popup close here too. With this patch looking like it might take a little more time with reviewing and discussion before it gets in, I'm going to un-dupe #397614: Append to #prefix and #suffix instead of overwrite and see if it can be committed separately. All these patches should be their own issues, I was mistaken to ask it be rolled into here and apologize for that...it ends up making it tougher for review and commit, especially when dealing with the different branches of this module.
#19
The part I mentioned in #14 is actually an issue for popups module rather than popups_reference, though this does pretty much hinge upon that working. Should I cross post there starbow?
#20
I'm also seeing the problem described in #14 which makes it impossible to have a popups: add and reference field working on the same form as a field that has an "Add Another Item" button.
#21
I was just reading an article today from Trellon and think it might solve the problem by using a different approach altogether: http://www.trellon.com/content/blog/adding-new-fields-to-file-uploads
I'll let you know how ti goes after doing some testing.
EDIT: this isn't going to help us. The problem is within the javascript, not the form_alter().... /me keeps digging
#22
subscribing.
#23
subscribed
#24
subscribe
#25
Here is how I got this working.
I actually patched cck to detect popups when it does it's Drupal.settings change. This is a HACK and is not a definitive solution - however until we really get to the root of this problem, I submit this to you so that you can a) use it and get on with your life or b) see what's going on to give you some ideas as to how we can solve this in a more fashionable way.
#26
Thank you sirkitree. I tried messing around with this for a bit and couldn't get it working with a legit solution or a hack, so I'll give this a try since I am already hacking CCK in order to implement combo fields and will continue looking into a more maintainable fix.
#27
hi sirkitree (love the name btw):
what version are you patching against with #25?
i patched against Content Construction Kit (CCK) 6.x-2.2 and am using Popups: Add and Reference 6.x-2.0-alpha1 and Popups API (Ajax Dialogs) 6.x-2.0-alpha5 and found no change in the behavior.
#28
The hack in #25 is against cck 2.2. Save the patch file to your cck module directory and run it from there. It should be applied after the patch in #17 (which is against the version of popups_reference marked in the issue) - I know - it's getting messy now, sorry :( but as I said it's a temporary hack.
#29
after applying patch #17 that temporary fix works. thanks!
#30
I've applied patches #17 and #25 (in that order) and "Add Another Item" still gives me the following error popup (and does not create a new auto-complete line:
An error occurred.
/content/js_add_more/tree/field_maintenance
{ "status": true, "data": "\x3ctable id=\"field_maintenance_values\" class=\"content-...
Also,
after saving the node creation popup generated by click "Add New: Add ContentTypeXXX", the autocomplete field is not filled with the new node.
Content Construction Kit (CCK) 6.x-2.2
Popups: Add and Reference 6.x-2.0-alpha1
Popups API (Ajax Dialogs) 6.x-2.0-alpha5
any thoughts?
#31
Has anyone done anything of a hack or fix for 6.x-1.0? The same bug manifests and since people like me still need a production version and 6.x-2.0 is not officially supported (either here or in the Popups API) I'm looking for a 6.x-1.0 fix.
#32
I'm still working on it, but for the 6.x-1.0, I've discovered that a small fix to lines 95 and 96 of popups_reference.module seem to fix the problem with not being able to add more fields:
<?php$form[$key]['#prefix'] = '<div id="'. $wrapper_id .'">' . $form[$key]['#prefix'];
$form[$key]['#suffix'] = $form[$key]['#suffix'] .'<div>Add New: ' . implode(', ', $links) .'</div></div>';
?>
Basically the problem is that before those two lines were over-writing the #prefix and #suffix that the ahah add another field needed to target properly. I've just started testing, but this seems to have fixed the problem so far.
#33
Here's a patch file for making that change for the 6.x-1.0 branch:
#34
@jcfiala: There is an issue already for that fix #397614: Append to #prefix and #suffix instead of overwrite and also sirkitree had included that in his patch in comment #17 above. Maybe it is because I also have other patches applied, but this did not appear to fix the problem for me.
#35
@jcfiala: Thanks for patch, but didn't work ... let's carry on in other issue cYu pointed out:
http://drupal.org/node/397614#comment-1606556
(Ps - other bug, we'll get off your lawn! sorry!)
#36
subscribing... would absolutely love for this to work w/ unlimited value fields
#37
subscribing.. is someone still working on that?
#38
Subscribe. I will test.
#39
Don't want to hack cck (again.. )
So needed a different way
much better to hack popups!
Long story short, give pop ups add popup an alter, then alter the original path that is getting changed incorrectly.
to pops
<?phpdrupal_alter('popups_add_popups_settings',$settings);
?>
(making patches atm are blah due to multi other pages to get inline references to work and my damn windows line breaks, sorry)
context end of popups_add_popup
<?phpdrupal_alter('popups_add_popups_settings',$settings);
+ drupal_add_js( $settings, 'setting' );
$added = TRUE;
?>
now for popups_reference (using patched with $#17)
One 2 new functions
<?phpfunction popups_reference_original_url($in_url='') {
static $url;
if ($in_url) $url = $in_url;
return $url;
}
?>
added call to
<?phpfunction _popups_reference_links($field, $src_type, $wrapper_id, $widget_type) {
+ popups_reference_original_url('node/add/'.str_replace('_','-',$src_type));
?>
then a implementation of the earlier declared alter
<?phpfunction popups_reference_popups_add_popups_settings_alter(&$settings){
if (strpos($settings['popups']['originalPath'],'content/js_add_more')!==false && popups_reference_original_url()) $settings['popups']['originalPath'] = popups_reference_original_url();
}
?>
Haven't tested too much, and it's 6 am so, uh, yeah.
#40
I was bit tired last night
It'd be much similair if popups_add_popups took in an optional overide value for original path, lol XD.
Also tried overiding the javascript, but the array merge recursive turns two values into a multi value array which has.. amusing results. Couldn't figure out how to "fix" it later on in the JS, so no luck on that.
#41
Bitten by this double since I had the field returning various popups links, so made a topic in popups since I think the only good way to solve it is in there. http://drupal.org/node/531578
#42
Subscribed
#43
Subscribed