Add components and inserting them before existing components may break dependencies [#1509686]

Thanks so much for this fantastic module.

I just found that certain components were not displaying properly when (a) there was a hierarchy of dependent components of at least two levels and (b) an item set high in the hierarchy was added after the other components. Given this scenario:

1. Add components A and B
2. Add component C and move it between A and B
3. Make C dependent on A and B dependent on C
4. B is triggered correctly, but C is not

I traced it to webform_conditional_module line 89:

$sql = "SELECT * FROM {webform_component} where nid = %d";

needs to become:

$sql = "SELECT * FROM {webform_component} where nid = %d ORDER BY weight";

The reason is that the default return order is by cid, which means the C component's dependencies are not evaluated (because it is already in the array of fields to be sent to Javascript).

Hope that helps.

Comment	File	Size	Author
#7	webform_conditional_1509686_cbd_2014-02-21.patch	1.69 KB	sdsheridan

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

smithwib CreditAttribution: smithwib commented 1 April 2012 at 11:28

One followup on this -- this does not properly address fieldsets, which must be sorted by the fieldset THEN then item. Not sure how best to resolve -- this would take care of one possible level of fieldsets (not multiple embedded):

$sql = "SELECT c.* FROM {webform_component} c LEFT JOIN [webform_component} p ON c.pid=p.cid"
      ." WHERE c.nid = %d ORDER BY CASE c.pid WHEN 0 THEN c.weight ELSE p.weight END, c.weight";

Comment #2

smithwib CreditAttribution: smithwib commented 22 April 2012 at 16:06

Scrap my above solution - does not work for multiple levels of nesting. Instead, I'll offer some other thoughts:

- The problem above appears to be resolved by letting it go through adding items to the jsfields array even if it has been added previously, because it may have been added as a dependent, but not necessarily an item that has dependents if it appears after its dependent. I made a few edits to webform_conditional_form_alter and it seems to help:

  $cid = $row['cid'];
  $formkey = $row['form_key'];
  if ($field = _webform_conditional_get_field($form['submitted'], $cid)) { // && !isset($js_fields[$formkey])) {

and later:

if (!is_array($form_element['#after_build']) || !in_array("webform_conditional_element_after_build",$form_element['#after_build']))
  $form_element['#after_build'][] = "webform_conditional_element_after_build";
if ($row['mandatory'] && (!is_array($form_element['#pre_render']) || !in_array("webform_elment_pre_render_add_required",$form_element['#pre_render']))){
  $form_element['#pre_render'][] = "webform_elment_pre_render_add_required";

- Also, when creating a fieldset, it doesn't always get "rewritten" with the "webform_conditional" tags instead of the standard conditional tags. Saving twice generally fixes this, but I'll see if I can pinpoint where that's happening.

Thanks again!

Comment #3

tedbow

he/him

English

Ithaca, NY, USA

CreditAttribution: tedbow commented 23 April 2012 at 00:24

I'll see if I can pinpoint where that's happening.

@smithwib Thank you for looking into this

Comment #4

todea CreditAttribution: todea commented 22 May 2013 at 14:53

~~I also have this issue. It is impossible to correct without completely rebuilding the form and the conditional fields.~~ Scratch that, my conditional fields were not working due to this code, which i was using to hide colons...

var lab1 = $('#webform-client-form-2106 label');
lab1.each(function() { $(this).html($(this).html().replace(":", "")); });

Comment #5

sdsheridan

he/him/his

CreditAttribution: sdsheridan commented 21 February 2014 at 22:56

Issue summary:

View changes

Took me a while to figure this one out! I was getting validation errors that a required field wasn't present even when it was hidden. The problem has been identified above, in that the rows from the SELECT are coming back not in the order they should be.

I believe the solution is to create a recursive function that returns them in the correct fashion, as follows:

/**
 * Helper function to get all webform components in the correct order for webfrom_conditional_form_alter().
 * @param int $nid The nid of the webform node.
 * @param int $parent Internal - the cid of the parent, used in the recursive calls.
 * @return array The correctly ordered array of components.
 */
function webform_conditional_get_ordered_components($nid, $parent = 0) {
  $rows = array();
  $result = db_query('SELECT * FROM {webform_component} WHERE nid=%d AND pid=%d ORDER BY weight', $nid, $parent);
  while ( $row = db_fetch_array($result) ) {
    $rows[$row['cid']] = $row;
    $rows = $rows + webform_conditional_get_ordered_components($nid, $row['cid']);
  }
  return $rows;
}
/*
 * Implements hook_form_alter().
* Edit Webform
* Add Javascript to describe dependent fields
* Add validation of hidden conditional fields
*/
function webform_conditional_form_alter(&$form, $form_state, $form_id) {
  //todo: change to look only at beginning of string
  static $called_forms = array();
  if (strstr($form_id, 'webform_client_form_') && isset($form['submitted'])) {
    if (isset($form['#webform_conditional_js'])) {
      return;
    }
    $nid = $form['details']['nid']['#value'];
    if (in_array($nid, $called_forms)) {
      //return;
    }
    else {
      $called_forms[] = $nid;
    }
//    $sql = "SELECT * FROM {webform_component} where nid = %d";  // <== replace all this with a call to the function
//    $result = db_query($sql, $nid);
//    while ($row = db_fetch_array($result)) {
//      $rows[$row['cid']] = $row;
//    }
    $rows = webform_conditional_get_ordered_components($nid); // <== our recursive function
    foreach ( $rows as $row) {
      //see if field is on this page
...

The call to webform_conditional_get_ordered_components now retrieves the components in the correct order as they appear on the form, and and the hook_form_alter needs them. This fixed my problem. Would be good if this got into the next release.

Shawn