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,

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

starbow’s picture

I have added this to the list of known limitations.

starbow’s picture

Status: Active » Fixed

A fix for this has been checked into dev, and will be released in RC3.

iko’s picture

thanks !

cYu’s picture

Status: Fixed » Active

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.

starbow’s picture

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.

sirkitree’s picture

Assigned: Unassigned » sirkitree
FileSize
1.82 KB

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

cYu’s picture

Status: Active » Needs work
sirkitree’s picture

Status: Needs work » Active
FileSize
1.91 KB

Sure! Here's a re-roll.

sirkitree’s picture

Status: Active » Needs work

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.

sirkitree’s picture

Version: 6.x-1.0-rc1 » 6.x-2.0-alpha1

Sorry, this patch is against the latest alpha. marking as such.

cYu’s picture

Status: Needs work » Needs review

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...

sirkitree’s picture

I agree. I just hope starbow does ;)

sirkitree’s picture

Status: Needs review » Needs work

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:
Create Story | Community
Create Story | Community

sirkitree’s picture

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:
Create Story | drupal

So that after you add a node within the popup, you get this:
Create Story | drupal

Not too sure how to resolve this just yet, but finding the problem is 80%, right? :-/

sirkitree’s picture

would something like #360081: Pass Settings Locally to JS Behaviors fix this? will test monday... time to go home /whew

starbow’s picture

Ok, I will need to look at this again when I have some more time.

sirkitree’s picture

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:

function 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.

cYu’s picture

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.

sirkitree’s picture

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?

cYu’s picture

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.

sirkitree’s picture

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

quinn-2’s picture

subscribing.

russbollesjr’s picture

subscribed

ccshannon’s picture

subscribe

sirkitree’s picture

FileSize
1.6 KB

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.

cYu’s picture

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.

acouch’s picture

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.

sirkitree’s picture

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.

acouch’s picture

after applying patch #17 that temporary fix works. thanks!

russbollesjr’s picture

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?

greg.harvey’s picture

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.

jcfiala’s picture

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:

      $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.

jcfiala’s picture

Here's a patch file for making that change for the 6.x-1.0 branch:

cYu’s picture

@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.

greg.harvey’s picture

@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!)

jrefano’s picture

subscribing... would absolutely love for this to work w/ unlimited value fields

webnotwar’s picture

subscribing.. is someone still working on that?

bomarmonk’s picture

Subscribe. I will test.

hefox’s picture

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

 drupal_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

    drupal_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

function popups_reference_original_url($in_url='') {
   static $url;
   if ($in_url) $url = $in_url;
   return $url;
}

added call to


function _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

function 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.

hefox’s picture

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.

hefox’s picture

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

ISPTraderChris’s picture

Subscribed

pixlkat’s picture

Subscribed

udig’s picture

Subscribed

FiNeX’s picture

Subscribing

momper’s picture

subscribed

manimal’s picture

I'm subscribing to this, but it looks like its a dead issue now.

Did anyone get this to work? I applied patch from #17, but it isn't filling in the node reference field with the newly created node. Is this the expected behavior? I add a new node, and then have to reference the newly created node manually. Seems like I'm having to do 2 steps for an advertised 1 step.

I'm using the 2.0-alpha1.