Browse Source

Restructured chado node api (properties, dbxrefs, relationships) to deal with a Drupal bug causing the triggering button to be incorectly determined. Essentially there is now a common ajax callback and common remove/add validate/submit functions that determine the button actually triggered using and then call the correct functions. This change is completely backwards compatible as no outward facing function names/parameters were changed.

Lacey Sanderson 10 years ago
parent
commit
5204a98770

+ 199 - 0
tripal_core/api/tripal_core.chado_nodes.api.inc

@@ -133,6 +133,205 @@ function chado_node_get_base_table($content_type, $module = FALSE) {
 
 }
 
+/** 
+ * @section
+ * Common Functionality for Properties, Dbxrefs and relationships chado node API
+ */
+
+/**
+ * Validate the Triggering element from a node form.
+ * 
+ * We are going to inspect the post to determine what PHP knows is the triggering
+ * element and if it doesn't agree with Drupal then we are actually going to
+ * change it in Drupal.
+ * 
+ * This fixes an obscure bug triggered when a property is added and then
+ * a relationship removed, Drupal thinks the first property remove button was
+ * clicked and instead removes a property (not a relationship) and renders the new
+ * property table in the relationship table page space.
+ * 
+ * NOTE: Many Drupal issues state that this problem is solved if the #name
+ * of the button is unique (which it is in our case) but we are still experiencing
+ * incorrectly determined triggering elements so we need to handle it ourselves.
+ */
+function chado_validate_node_form_triggering_element($form, &$form_state) {
+  
+  // We are going to inspect the post to determine what PHP knows is the triggering
+  // element and if it doesn't agree with Drupal then we are actually going to
+  // change it in Drupal.
+  if ($_POST['_triggering_element_name'] != $form_state['triggering_element']['#name']) {
+    $form_state['triggering_element']['#name'] = $_POST['_triggering_element_name'];
+  }
+  
+}
+
+/**
+ * Validate Adding Subtables entries from the node forms.
+ * Supported subtables: Properties, Relationships, Additional DBxrefs.
+ *
+ * @param array $form
+ * @param array $form_state
+ */
+function chado_add_node_form_subtables_add_button_validate($form, &$form_state) {
+
+  // Based on triggering element call the correct validation function
+  // ASUMPTION #1: each of the buttons must have property, dbxref or relationship
+  // as the first part of the #name to uniquely identify the subsection.
+  if (preg_match('/^([a-z]+).*/', $form_state['triggering_element']['#name'], $matches)) {
+    $subsection = $matches[1];
+
+    switch($subsection) {
+      case 'properties':
+        chado_add_node_form_properties_add_button_validate($form, $form_state);
+        break;
+      case 'dbxrefs':
+        chado_add_node_form_dbxrefs_add_button_validate($form, $form_state);
+        break;
+      case 'relationships':
+        chado_add_node_form_relationships_add_button_validate($form, $form_state);
+        break;
+    }
+  }
+}
+
+/**
+ * Add subtable entries to the node forms.
+ * Supported subtables: Properties, Relationships, Additional DBxrefs.
+ *
+ * @param array $form
+ * @param array $form_state
+ */
+function chado_add_node_form_subtables_add_button_submit($form, &$form_state) {
+
+  // Based on triggering element call the correct submit function
+  // ASUMPTION #1: each of the buttons must have properties, dbxrefs or relationships
+  // as the first part of the #name to uniquely identify the subsection.
+  if (preg_match('/^([a-z]+).*/', $form_state['triggering_element']['#name'], $matches)) {
+    $subsection = $matches[1];
+
+     switch($subsection) {
+      case 'properties':
+        chado_add_node_form_properties_add_button_submit($form, $form_state);
+        break;
+      case 'dbxrefs':
+        chado_add_node_form_dbxrefs_add_button_submit($form, $form_state);
+        break;
+      case 'relationships':
+        chado_add_node_form_relationships_add_button_submit($form, $form_state);
+        break;
+    }
+  }
+  
+  // This is needed to ensure the form builder function is called for the node
+  // form in order for any of these changes to be seen.
+  $form_state['rebuild'] = TRUE;
+}
+
+/**
+ * Validate Removing Subtables entries from the node forms.
+ * Supported subtables: Properties, Relationships, Additional DBxrefs.
+ * 
+ * Since Removing isn't associated with any user input the only thing we
+ * need to validate is that Drupal has determined the triggering element correctly.
+ * That said, we will call each subtables associated validate function just incase
+ * there is some case-specific validation we do not know of or have not anticipated.
+ * 
+ * @param array $form
+ * @param array $form_state
+ */
+function chado_add_node_form_subtables_remove_button_validate($form, &$form_state) {
+ 
+  // We need to validate the trigerring element since Drupal has known
+  // issues determining this correctly when there are multiple buttons
+  // with the same label.
+  chado_validate_node_form_triggering_element($form, $form_state);
+  
+  // Based on triggering element call the correct validation function
+  // ASUMPTION #1: each of the buttons must have property, dbxref or relationship
+  // as the first part of the #name to uniquely identify the subsection.
+  if (preg_match('/^([a-z]+).*/', $form_state['triggering_element']['#name'], $matches)) {
+    $subsection = $matches[1];
+    
+      switch($subsection) {
+      case 'properties':
+        chado_add_node_form_properties_remove_button_validate($form, $form_state);
+        break;
+      case 'dbxrefs':
+        chado_add_node_form_dbxrefs_remove_button_validate($form, $form_state);
+        break;
+      case 'relationships':
+        chado_add_node_form_relationships_remove_button_validate($form, $form_state);
+        break;
+    }
+  }
+}
+
+/**
+ * Remove subtable entries to the node forms.
+ * Supported subtables: Properties, Relationships, Additional DBxrefs.
+ *
+ * @param array $form
+ * @param array $form_state
+ */
+function chado_add_node_form_subtables_remove_button_submit($form, &$form_state) {
+
+  // Based on triggering element call the correct submit function
+  // ASUMPTION #1: each of the buttons must have properties, dbxrefs or relationships
+  // as the first part of the #name to uniquely identify the subsection.
+  if (preg_match('/^([a-z]+).*/', $form_state['triggering_element']['#name'], $matches)) {
+    $subsection = $matches[1];
+
+    switch($subsection) {
+      case 'properties':
+        chado_add_node_form_properties_remove_button_submit($form, $form_state);
+        break;
+      case 'dbxrefs':
+        chado_add_node_form_dbxrefs_remove_button_submit($form, $form_state);
+        break;
+      case 'relationships':
+        chado_add_node_form_relationships_remove_button_submit($form, $form_state);
+        break;
+    }
+  }
+
+  // This is needed to ensure the form builder function is called for the node
+  // form in order for any of these changes to be seen.
+  $form_state['rebuild'] = TRUE;
+}
+
+/**
+ * Ajax function which returns the section of the form to be re-rendered
+ * for either the properties, dbxref or relationship sub-sections.
+ *
+ * @ingroup tripal_core
+ */
+function chado_add_node_form_subtable_ajax_update($form, &$form_state) {
+  
+  // We need to validate the trigerring element since Drupal has known
+  // issues determining this correctly when there are multiple buttons
+  // with the same label.
+  chado_validate_node_form_triggering_element($form, $form_state);
+  
+  // Based on triggering element render the correct part of the form.
+  // ASUMPTION: each of the buttons must have property, dbxref or relationship
+  // as the first part of the #name to uniquely identify the subsection.
+  if (preg_match('/^([a-z]+).*/', $form_state['triggering_element']['#name'], $matches)) {
+    $subsection = $matches[1];
+    
+    switch($subsection) {
+      case 'properties':
+        return $form['properties']['property_table'];
+        break;
+      case 'dbxrefs':
+        return $form['addtl_dbxrefs']['dbxref_table'];
+        break;
+      case 'relationships':
+        return $form['relationships']['relationship_table'];
+        break;
+    }
+  }
+}
+
 /**
  * @section
  * Sync Form

+ 12 - 28
tripal_core/api/tripal_core.chado_nodes.dbxrefs.api.inc

@@ -298,9 +298,9 @@ function chado_add_node_form_dbxrefs(&$form, &$form_state, $details) {
       $form['addtl_dbxrefs']['dbxref_table'][$dbxref->db_id][$version]['dbxref_action'] = array(
         '#type' => 'submit',
         '#value' => t('Remove'),
-        '#name' => "dbxref_remove-".$dbxref->db_id.'-'.$version,
+        '#name' => "dbxrefs_remove-".$dbxref->db_id.'-'.$version,
         '#ajax' => array(
-          'callback' => "chado_add_node_form_dbxrefs_ajax_update",
+          'callback' => "chado_add_node_form_subtable_ajax_update",
           'wrapper' => 'tripal-generic-edit-addtl_dbxrefs-table',
           'effect'   => 'fade',
           'method'   => 'replace',
@@ -313,8 +313,8 @@ function chado_add_node_form_dbxrefs(&$form, &$form_state, $details) {
         // from the chado_additional_dbxrefs array. In order to keep validate errors
         // from the node form validate and Drupal required errors for non-dbxref fields
         // preventing the user from removing dbxrefs we set the #limit_validation_errors below
-        '#validate' => array('chado_add_node_form_dbxrefs_remove_button_validate'),
-        '#submit' => array('chado_add_node_form_dbxrefs_remove_button_submit'),
+        '#validate' => array('chado_add_node_form_subtables_add_button_validate'),
+        '#submit' => array('chado_add_node_form_subtables_remove_button_submit'),
         // Limit the validation of the form upon clicking this button to the dbxref_table tree
         // No other fields will be validated (ie: no fields from the main form or any other api
         // added form).
@@ -352,9 +352,9 @@ function chado_add_node_form_dbxrefs(&$form, &$form_state, $details) {
   $form['addtl_dbxrefs']['dbxref_table']['new']['dbxref_action'] = array(
     '#type' => 'submit',
     '#value' => t('Add'),
-    '#name' => "dbxref-add",
+    '#name' => "dbxrefs-add",
     '#ajax' => array(
-      'callback' => "chado_add_node_form_dbxrefs_ajax_update",
+      'callback' => "chado_add_node_form_subtable_ajax_update",
       'wrapper' => 'tripal-generic-edit-addtl_dbxrefs-table',
       'effect'   => 'fade',
       'method'   => 'replace',
@@ -367,8 +367,8 @@ function chado_add_node_form_dbxrefs(&$form, &$form_state, $details) {
     // array. In order to keep validate errors from the node form validate and Drupal
     // required errors for non-dbxref fields preventing the user from adding dbxrefs we
     // set the #limit_validation_errors below
-    '#validate' => array('chado_add_node_form_dbxrefs_add_button_validate'),
-    '#submit' => array('chado_add_node_form_dbxrefs_add_button_submit'),
+    '#validate' => array('chado_add_node_form_subtables_add_button_validate'),
+    '#submit' => array('chado_add_node_form_subtables_add_button_submit'),
     // Limit the validation of the form upon clicking this button to the dbxref_table tree
     // No other fields will be validated (ie: no fields from the main form or any other api
     // added form).
@@ -386,7 +386,7 @@ function chado_add_node_form_dbxrefs(&$form, &$form_state, $details) {
  * @ingroup tripal_core
  */
 function chado_add_node_form_dbxrefs_add_button_validate($form, &$form_state) {
-
+  
   // Ensure the db_id is supplied & Valid
   $db = chado_select_record(
     'db',
@@ -414,7 +414,7 @@ function chado_add_node_form_dbxrefs_add_button_validate($form, &$form_state) {
  *
  * @ingroup tripal_core
  */
-function chado_add_node_form_dbxrefs_add_button_submit(&$form, &$form_state) {
+function chado_add_node_form_dbxrefs_add_button_submit($form, &$form_state) {
 
   // if the chado_additional_dbxrefs array is not set then this is the first time modifying the
   // dbxref table. this means we need to include all the dbxrefs from the db
@@ -440,20 +440,15 @@ function chado_add_node_form_dbxrefs_add_button_submit(&$form, &$form_state) {
   unset($form_state['input']['dbxref_table']['new']['db_name']);
   unset($form_state['input']['dbxref_table']['new']['dbxref_version']);
   unset($form_state['input']['dbxref_table']['new']['dbxref_accession']);
-
-  $form_state['rebuild'] = TRUE;
 }
 
 /**
- * There is no user input for the remove buttons so there is no need to validate
- * However, both a submit & validate need to be specified so this is just a placeholder
- *
  * Called by the many remove buttons in chado_add_node_form_dbxrefs
  *
  * @ingroup tripal_core
  */
 function chado_add_node_form_dbxrefs_remove_button_validate($form, $form_state) {
-  // No Validation needed for remove
+  // No validation needed.
 }
 
 /**
@@ -471,23 +466,12 @@ function chado_add_node_form_dbxrefs_remove_button_submit(&$form, &$form_state)
   }
 
   // remove the specified dbxref from the form dbxref table
-  if(preg_match('/dbxref_remove-([^-]+-[^-]+)/',$form_state['triggering_element']['#name'],$match)) {
+  if(preg_match('/dbxrefs_remove-([^-]+-[^-]+)/',$form_state['triggering_element']['#name'],$match)) {
     $key = $match[1];
     if (array_key_exists($key, $form_state['chado_additional_dbxrefs'])) {
       unset($form_state['chado_additional_dbxrefs'][$key]);
     }
   }
-
-  $form_state['rebuild'] = TRUE;
-}
-
-/**
- * Ajax function which returns the section of the form to be re-rendered
- *
- * @ingroup tripal_core
- */
-function chado_add_node_form_dbxrefs_ajax_update($form, $form_state) {
-  return $form['addtl_dbxrefs']['dbxref_table'];
 }
 
 /**

+ 14 - 28
tripal_core/api/tripal_core.chado_nodes.properties.api.inc

@@ -503,9 +503,9 @@ function chado_add_node_form_properties(&$form, &$form_state, $details) {
       $form['properties']['property_table'][$property->type_id][$property->rank]['property_action'] = array(
         '#type' => 'submit',
         '#value' => t('Remove'),
-        '#name' => "property_remove-".$property->type_id.'-'.$property->rank,
+        '#name' => "properties_remove-".$property->type_id.'-'.$property->rank,
         '#ajax' => array(
-          'callback' => "chado_add_node_form_properties_ajax_update",
+          'callback' => "chado_add_node_form_subtable_ajax_update",
           'wrapper' => 'tripal-generic-edit-properties-table',
           'effect'   => 'fade',
           'method'   => 'replace',
@@ -518,8 +518,8 @@ function chado_add_node_form_properties(&$form, &$form_state, $details) {
         // from the chado_properties array. In order to keep validate errors
         // from the node form validate and Drupal required errors for non-property fields
         // preventing the user from removing properties we set the #limit_validation_errors below
-        '#validate' => array('chado_add_node_form_properties_remove_button_validate'),
-        '#submit' => array('chado_add_node_form_properties_remove_button_submit'),
+        '#validate' => array('chado_add_node_form_subtables_remove_button_validate'),
+        '#submit' => array('chado_add_node_form_subtables_remove_button_submit'),
         // Limit the validation of the form upon clicking this button to the property_table tree
         // No other fields will be validated (ie: no fields from the main form or any other api
         // added form).
@@ -570,9 +570,9 @@ function chado_add_node_form_properties(&$form, &$form_state, $details) {
   $form['properties']['property_table']['new']['property_action'] = array(
     '#type' => 'submit',
     '#value' => t('Add'),
-    '#name' => "property-add",
+    '#name' => "properties-add",
     '#ajax' => array(
-      'callback' => "chado_add_node_form_properties_ajax_update",
+      'callback' => "chado_add_node_form_subtable_ajax_update",
       'wrapper' => 'tripal-generic-edit-properties-table',
       'effect'   => 'fade',
       'method'   => 'replace',
@@ -585,8 +585,8 @@ function chado_add_node_form_properties(&$form, &$form_state, $details) {
     // array. In order to keep validate errors from the node form validate and Drupal
     // required errors for non-property fields preventing the user from adding properties we
     // set the #limit_validation_errors below
-    '#validate' => array('chado_update_node_form_properties_add_button_validate'),
-    '#submit' => array('chado_add_node_form_properties_add_button_submit'),
+    '#validate' => array('chado_add_node_form_subtables_add_button_validate'),
+    '#submit' => array('chado_add_node_form_subtables_add_button_submit'),
     // Limit the validation of the form upon clicking this button to the property_table tree
     // No other fields will be validated (ie: no fields from the main form or any other api
     // added form).
@@ -602,8 +602,8 @@ function chado_add_node_form_properties(&$form, &$form_state, $details) {
  *
  * @ingroup tripal_core
  */
-function chado_update_node_form_properties_add_button_validate($form, &$form_state) {
-
+function chado_add_node_form_properties_add_button_validate($form, &$form_state) {
+  
   // Ensure the type_id is supplied & Valid
   $cvterm = chado_select_record(
     'cvterm',
@@ -632,7 +632,7 @@ function chado_update_node_form_properties_add_button_validate($form, &$form_sta
  *
  * @ingroup tripal_core
  */
-function chado_add_node_form_properties_add_button_submit(&$form, &$form_state) {
+function chado_add_node_form_properties_add_button_submit($form, &$form_state) {
 
   $details = unserialize($form_state['values']['property_table']['details']);
 
@@ -688,19 +688,15 @@ function chado_add_node_form_properties_add_button_submit(&$form, &$form_state)
   unset($form_state['input']['property_table']['new']['definition']);
   unset($form_state['input']['property_table']['new']['value']);
 
-  $form_state['rebuild'] = TRUE;
 }
 
 /**
- * There is no user input for the remove buttons so there is no need to validate
- * However, both a submit & validate need to be specified so this is just a placeholder
- *
  * Called by the many remove buttons in chado_add_node_form_properties
  *
  * @ingroup tripal_core
  */
-function chado_add_node_form_properties_remove_button_validate($form, $form_state) {
-  // No Validation needed for remove
+function chado_add_node_form_properties_remove_button_validate($form, &$form_state) {
+  // No validation needed.
 }
 
 /**
@@ -718,27 +714,17 @@ function chado_add_node_form_properties_remove_button_submit(&$form, &$form_stat
   }
 
   // remove the specified property from the form property table
-  if(preg_match('/property_remove-([^-]+-[^-]+)/',$form_state['triggering_element']['#name'],$match)) {
+  if(preg_match('/properties_remove-([^-]+-[^-]+)/',$form_state['triggering_element']['#name'],$match)) {
     $key = $match[1];
     if (array_key_exists($key, $form_state['chado_properties'])) {
       unset($form_state['chado_properties'][$key]);
     }
   }
-
-  $form_state['rebuild'] = TRUE;
 }
 
 function chado_add_node_form_properties_ajax_desc($form, $form_state) {
   return $form['properties']['property_table']['new']['type'];
 }
-/**
- * Ajax function which returns the section of the form to be re-rendered
- *
- * @ingroup tripal_core
- */
-function chado_add_node_form_properties_ajax_update($form, $form_state) {
-  return $form['properties']['property_table'];
-}
 
 /**
  * Creates an array in form_state containing the existing properties. This array is

+ 13 - 33
tripal_core/api/tripal_core.chado_nodes.relationships.api.inc

@@ -435,9 +435,9 @@ function chado_add_node_form_relationships(&$form, &$form_state, $details) {
       $form['relationships']['relationship_table'][$relationship->type_id][$rank]['rel_action'] = array(
         '#type' => 'submit',
         '#value' => t('Remove'),
-        '#name' => "rel_remove-".$relationship->type_id.'-'.$rank,
+        '#name' => "relationships_remove-".$relationship->type_id.'-'.$rank,
         '#ajax' => array(
-          'callback' => 'chado_add_node_form_relationships_ajax_update',
+          'callback' => 'chado_add_node_form_subtable_ajax_update',
           'wrapper' => 'tripal-generic-edit-relationships-table',
           'effect'   => 'fade',
           'method'   => 'replace',
@@ -450,8 +450,8 @@ function chado_add_node_form_relationships(&$form, &$form_state, $details) {
         // from the chado_relationships array. In order to keep validate errors
         // from the node form validate and Drupal required errors for non-relationship fields
         // preventing the user from removing relationships we set the #limit_validation_errors below
-        '#validate' => array('chado_add_node_form_relationships_form_remove_button_validate'),
-        '#submit' => array('chado_add_node_form_relationships_remove_button_submit'),
+        '#validate' => array('chado_add_node_form_subtables_remove_button_validate'),
+        '#submit' => array('chado_add_node_form_subtables_remove_button_submit'),
         // Limit the validation of the form upon clicking this button to the relationship_table tree
         // No other fields will be validated (ie: no fields from the main form or any other api
         // added form).
@@ -495,9 +495,9 @@ function chado_add_node_form_relationships(&$form, &$form_state, $details) {
   $form['relationships']['relationship_table']['new']['rel_action'] = array(
     '#type' => 'submit',
     '#value' => t('Add'),
-    '#name' => 'rel-add',
+    '#name' => 'relationships-add',
     '#ajax' => array(
-      'callback' => 'chado_add_node_form_relationships_ajax_update',
+      'callback' => 'chado_add_node_form_subtable_ajax_update',
       'wrapper' => 'tripal-generic-edit-relationships-table',
       'effect'   => 'fade',
       'method'   => 'replace',
@@ -509,8 +509,8 @@ function chado_add_node_form_relationships(&$form, &$form_state, $details) {
     // array. In order to keep validate errors from the node form validate and Drupal
     // required errors for non-relationship fields preventing the user from adding relationships we
     // set the #limit_validation_errors below
-    '#validate' => array('chado_add_node_form_relationships_add_button_validate'),
-    '#submit' => array('chado_add_node_form_relationships_add_button_submit'),
+    '#validate' => array('chado_add_node_form_subtables_add_button_validate'),
+    '#submit' => array('chado_add_node_form_subtables_add_button_submit'),
     // Limit the validation of the form upon clicking this button to the relationship_table tree
     // No other fields will be validated (ie: no fields from the main form or any other api
     // added form).
@@ -530,7 +530,7 @@ function chado_add_node_form_relationships(&$form, &$form_state, $details) {
 function chado_add_node_form_relationships_add_button_validate($form, &$form_state) {
 
   $details = unserialize($form_state['values']['relationship_table']['details']);
-
+  
   // First deal with autocomplete fields.
   // Extract the base_id assuming '(###) NAME FIELD'.
   if (!empty($form_state['values']['relationship_table']['new']['subject_name'])) {
@@ -655,7 +655,7 @@ function chado_add_node_form_relationships_add_button_validate($form, &$form_sta
  *
  * @ingroup tripal_core
  */
-function chado_add_node_form_relationships_add_button_submit(&$form, &$form_state) {
+function chado_add_node_form_relationships_add_button_submit($form, &$form_state) {
 
 
   $details = unserialize($form_state['values']['relationship_table']['details']);
@@ -704,22 +704,15 @@ function chado_add_node_form_relationships_add_button_submit(&$form, &$form_stat
   // we don't want the new element to pick up the values from the previous element so wipe them out
   unset($form_state['input']['relationship_table']['new']['type_id']);
   unset($form_state['input']['relationship_table']['new']['type_name']);
-
-  // This is needed to ensure the form builder function is called to
-  // rebuild the form on ajax requests.
-  $form_state['rebuild'] = TRUE;
 }
 
 /**
- * There is no user input for the remove buttons so there is no need to validate
- * However, both a submit & validate need to be specified so this is just a placeholder
- *
  * Called by the many remove buttons in chado_add_node_form_relationships
  *
  * @ingroup tripal_core
  */
-function chado_add_node_form_relationships_form_remove_button_validate($form, $form_state) {
-  // No Validation needed for remove
+function chado_add_node_form_relationships_remove_button_validate($form, &$form_state) {
+  // No validation needed.
 }
 
 /**
@@ -735,25 +728,12 @@ function chado_add_node_form_relationships_remove_button_submit(&$form, &$form_s
   chado_add_node_form_relationships_create_relationship_formstate_array($form, $form_state);
 
   // remove the specified relationship from the form relationship table
-  if(preg_match('/rel_remove-([^-]+-[^-]+)/',$form_state['triggering_element']['#name'],$match)) {
+  if(preg_match('/relationships_remove-([^-]+-[^-]+)/',$form_state['triggering_element']['#name'],$match)) {
     $key = $match[1];
     if (array_key_exists($key, $form_state['chado_relationships'])) {
       unset($form_state['chado_relationships'][$key]);
     }
   }
-
-  // This is needed to ensure the form builder function is called to
-  // rebuild the form on ajax requests.
-  $form_state['rebuild'] = TRUE;
-}
-
-/**
- * Ajax function which returns the section of the form to be re-rendered
- *
- * @ingroup tripal_core
- */
-function chado_add_node_form_relationships_ajax_update($form, $form_state) {
-  return $form['relationships']['relationship_table'];
 }
 
 /**