Working through bugs in the Chado field storage and field submit

tripal/includes/TripalField.inc

@@ -406,16 +406,12 @@ class TripalField {
   /**
    * Performs extra commands when the entity form is submitted.
    *
-   * This function corresponds to the hook_field_submit()
-   * function of the Drupal Field API.
-   *
    * Drupal typically does not provide a submit hook for fields.  The
    * TripalField provides one to allow for behind-the-scenes actions to
    * occur.   This function should never be used for updates, deletes or
    * inserts into the storage backend. Rather, the appropriate Field Storage
-   * implementation will take care of that. An example where this function
-   * may be useful would be to set values in the $items array using values
-   * of the other.
+   * implementation will take care of that. Use this function instead to
+   * to change the $items array before insertion into the database.
    *
    *  @param $entity_type
    *    The type of $entity.

tripal/includes/TripalTerm.inc

@@ -26,7 +26,7 @@ class TripalTerm extends Entity {
       if (function_exists($function)) {
         $term_details = $function($vocab->vocabulary, $this->accession);
         if ($term_details) {
-          $this->details = $term_details;
+//          $this->details = $term_details;
           if ($term_details and $term_details['definition']) {
             $this->definition = $term_details['definition'];
             $this->url = $term_details['url'];

tripal/includes/tripal.fields.inc

@@ -374,7 +374,7 @@ function tripal_field_validate($entity_type, $entity, $field, $instance,
 /**
  * Implements hook_field_submit()
  *
- * This is a TripalEntity specific hook.
+ * This is a hook called by the TripalEntityUIController class.
  */
 function tripal_field_submit($entity_type, $entity, $field, $instance,
     $langcode, &$items, $form, &$form_state) {

tripal_chado/api/modules/tripal_chado.cv.api.inc

@@ -144,51 +144,52 @@ function tripal_get_cv_select_options() {
  * Retrieves a chado controlled vocabulary term variable
  *
  * @param $identifier
- *   An array with the key stating what the identifier is. Supported keys (only one of the
- *   following unique keys are required):
- *    - cvterm_id: the chado cv.cvterm_id primary key
- *    - name: the chado cvterm.name field (assume unique)
- *   There are also some specially handled keys. They are:
- *    - cv_id:  an integer indicating the cv_id or an array with 'name' => the name of the cv.
- *    - synonym: an array with 'name' => the name of the synonym of the cvterm you want
- *        returned; 'cv_id' => the cv_id of the synonym; 'cv_name' => the name of the cv
- *        of the synonym
- *    - property: An array/object describing the property to select records for. It
- *      should at least have either a type_name (if unique across cvs) or type_id. Other
- *      supported keys include: cv_id/cv_name (of the type), value and rank
+ *   An array apropriate for use with the chado_generate_var for uniquely
+ *   identifying a cvterm record. Alternativley, there are also some specially
+ *   handled keys. They are:
+ *    - cv_id:  an integer indicating the cv_id or an array with 'name' => the
+ *      name of the cv.
+ *    - synonym: an array with 'name' => the name of the synonym of the cvterm
+ *      you want returned; 'cv_id' => the cv_id of the synonym; 'cv_name' =>
+ *      the name of the cv of the synonym
+ *    - property: An array/object describing the property to select records
+ *      for. It should at least have either a type_name (if unique across cvs)
+ *      or type_id. Other supported keys include: cv_id/cv_name (of the type),
+ *      value and rank
  * @param $options
  *   An array of options. Supported keys include:
- *     - Any keys supported by chado_generate_var(). See that function definition for
- *       additional details.
+ *     - Any keys supported by chado_generate_var(). See that function
+ *       definition for additional details.
  *
- * NOTE: the $identifier parameter can really be any array similar to $values passed into
- *   chado_select_record(). It should fully specify the cvterm record to be returned.
+ * NOTE: the $identifier parameter can really be any array similar to $values
+ *   passed into chado_select_record(). It should fully specify the cvterm
+ *   record to be returned.
  *
  * @return
- *   If unique values were passed in as an identifier then an object describing the cvterm
- *   will be returned (will be a chado variable from chado_generate_var()). Otherwise,
- *   FALSE will be returned.
+ *   If unique values were passed in as an identifier then an object describing
+ *   the cvterm will be returned (will be a chado variable from
+ *   chado_generate_var()). Otherwise, FALSE will be returned.
  *
- * @ingroup tripal_chado_api
+ * @ingroup tripal_cv_api
  */
 function tripal_get_cvterm($identifiers, $options = array()) {
 
   // Set Defaults
   if (!isset($options['include_fk'])) {
     // Tells chado_generate_var to only get the cv
-    //$options['include_fk'] = array('cv_id' => TRUE);
+    $options['include_fk'] = array('cv_id' => TRUE);
   }
 
   // Error Checking of parameters
   if (!is_array($identifiers)) {
-    tripal_report_error('tripal_chado_api', TRIPAL_ERROR,
+    tripal_report_error('tripal_cv_api', TRIPAL_ERROR,
       "tripal_get_cvterm: The identifier passed in is expected to be an array with the key
         matching a column name in the cvterm table (ie: cvterm_id or name). You passed in %identifier.",
       array('%identifier'=> print_r($identifiers, TRUE))
     );
   }
   elseif (empty($identifiers)) {
-    tripal_report_error('tripal_chado_api', TRIPAL_ERROR,
+    tripal_report_error('tripal_cv_api', TRIPAL_ERROR,
       "tripal_get_cvterm: You did not pass in anything to identify the cvterm you want. The identifier
         is expected to be an array with the key matching a column name in the cvterm table
         (ie: cvterm_id or name). You passed in %identifier.",
@@ -224,7 +225,6 @@ function tripal_get_cvterm($identifiers, $options = array()) {
   }
 
   // If one of the identifiers is property then use chado_get_record_with_property()
-  $cvterm = NULL;
   if (isset($identifiers['property'])) {
     $property = $identifiers['property'];
     unset($identifiers['property']);
@@ -234,19 +234,17 @@ function tripal_get_cvterm($identifiers, $options = array()) {
       $options
     );
   }
+
   // Else we have a simple case and we can just use chado_generate_var to get the cvterm
   else {
     // Try to get the cvterm
     $cvterm = chado_generate_var('cvterm', $identifiers, $options);
-    if ($cvterm) {
-      $cvterm = chado_expand_var($cvterm, 'field', 'cvterm.definition');
-    }
   }
 
   // Ensure the cvterm is singular. If it's an array then it is not singular
   if (is_array($cvterm)) {
     tripal_report_error(
-      'tripal_chado_api',
+      'tripal_cv_api',
       TRIPAL_ERROR,
       "tripal_get_cvterm: The identifiers you passed in were not unique. You passed in %identifier.",
       array(
@@ -258,7 +256,7 @@ function tripal_get_cvterm($identifiers, $options = array()) {
   // Report an error if $cvterm is FALSE since then chado_generate_var has failed
   elseif ($cvterm === FALSE) {
     tripal_report_error(
-      'tripal_chado_api',
+      'tripal_cv_api',
       TRIPAL_ERROR,
       "tripal_get_cvterm: chado_generate_var() failed to return a cvterm based on the identifiers
         you passed in. You should check that your identifiers are correct, as well as, look
@@ -275,6 +273,7 @@ function tripal_get_cvterm($identifiers, $options = array()) {
   }
 
 }
+
 /**
  * Create an options array to be used in a form element
  *   which provides a list of all chado cvterms

tripal_chado/api/modules/tripal_chado.db.api.inc

@@ -179,11 +179,9 @@ function tripal_get_db_select_options() {
  * @endcode
  *
  * @param $identifier
- *   An array with the key stating what the identifier is. Supported keys (only one of the
- *   following unique keys is required):
- *    - dbxref_id: the chado dbxref.dbxref_id primary key
- *    - accession: the chado dbxref.accession field (assume unique)
- *   There are also some specially handled keys. They are:
+ *   An array apropriate for use with the chado_generate_var for uniquely
+ *   identifying a dbxref record. Alternatively, there are also some specially
+ *   handled keys. They are:
  *    - property: An array/object describing the property to select records for. It
  *      should at least have either a type_name (if unique across cvs) or type_id. Other
  *      supported keys include: cv_id/cv_name (of the type), value and rank
@@ -200,7 +198,7 @@ function tripal_get_db_select_options() {
  *   will be returned (will be a chado variable from chado_generate_var()). Otherwise,
  *   FALSE will be returned.
  *
- * @ingroup tripal_chado_api
+ * @ingroup tripal_db_api
  */
 function tripal_get_dbxref($identifiers, $options = array()) {
 
@@ -212,26 +210,18 @@ function tripal_get_dbxref($identifiers, $options = array()) {
 
   // Error Checking of parameters
   if (!is_array($identifiers)) {
-    tripal_report_error(
-      'tripal_chado_api',
-      TRIPAL_ERROR,
+    tripal_report_error('tripal_db_api', TRIPAL_ERROR,
       "tripal_get_dbxref: The identifier passed in is expected to be an array with the key
         matching a column name in the dbxref table (ie: dbxref_id or name). You passed in %identifier.",
-      array(
-        '%identifier'=> print_r($identifiers, TRUE)
-      )
+      array('%identifier'=> print_r($identifiers, TRUE))
     );
   }
   elseif (empty($identifiers)) {
-    tripal_report_error(
-      'tripal_chado_api',
-      TRIPAL_ERROR,
+    tripal_report_error('tripal_db_api', TRIPAL_ERROR,
       "tripal_get_dbxref: You did not pass in anything to identify the dbxref you want. The identifier
         is expected to be an array with the key matching a column name in the dbxref table
         (ie: dbxref_id or name). You passed in %identifier.",
-      array(
-        '%identifier'=> print_r($identifiers, TRUE)
-      )
+      array('%identifier'=> print_r($identifiers, TRUE))
     );
   }
 
@@ -253,7 +243,7 @@ function tripal_get_dbxref($identifiers, $options = array()) {
 
   // Ensure the dbxref is singular. If it's an array then it is not singular
   if (is_array($dbxref)) {
-    tripal_report_error('tripal_chado_api', TRIPAL_ERROR,
+    tripal_report_error('tripal_db_api', TRIPAL_ERROR,
       "tripal_get_dbxref: The identifiers you passed in were not unique. You passed in %identifier.",
       array('%identifier'=> print_r($identifiers, TRUE))
     );
@@ -262,7 +252,7 @@ function tripal_get_dbxref($identifiers, $options = array()) {
   // Report an error if $dbxref is FALSE since then chado_generate_var has failed
   elseif ($dbxref === FALSE) {
     tripal_report_error(
-      'tripal_chado_api',
+      'tripal_db_api',
       TRIPAL_ERROR,
       "tripal_get_dbxref: chado_generate_var() failed to return a dbxref based on the identifiers
         you passed in. You should check that your identifiers are correct, as well as, look

tripal_chado/api/tripal_chado.query.api.inc

@@ -1215,11 +1215,12 @@ function chado_select_record($table, $columns, $values, $options = NULL) {
 
   // Get the table description.
   $table_desc = chado_get_schema($table);
-  if (empty($table_desc)) {
+  if (!is_array($table_desc)) {
     tripal_report_error('tripal_chado', TRIPAL_WARNING,
       'chado_insert_record; There is no table description for !table_name',
       array('!table_name' => $table), array('print' => $print_errors)
     );
+    return FALSE;
   }
 
   $select = '';

tripal_chado/includes/TripalFields/chado_linker__pub.inc

@@ -80,7 +80,6 @@ class chado_linker__pub extends TripalField {
     // array.  This happens when editing an existing record.
     if (count($items) > 0 and array_key_exists($delta, $items)) {
       $record_id = $items[$delta][$table_name . '__' . $pkey];
-      $fkey_value = $items[$delta][$table_name . '__' . $fkey];
       $pub_id = $items[$delta][$table_name . '__pub_id'];
       $title = $items[$delta][$table_name . '--pub__uniquename'];
     }
@@ -97,7 +96,6 @@ class chado_linker__pub extends TripalField {
 
     $widget['#table_name'] = $table_name;
     $widget['#fkey_field'] = $fkey;
-//    $widget['#element_validate'] = array('chado_linker__pub_widget_validate');
     $widget['#theme'] = 'chado_linker__pub_widget';
     $widget['#prefix'] =  "<span id='$table_name-$delta'>";
     $widget['#suffix'] =  "</span>";
@@ -122,7 +120,7 @@ class chado_linker__pub extends TripalField {
 
     $widget[$table_name . '--pub__uniquename'] = array(
       '#type' => 'textfield',
-      '#title' => t('Publication ID'),
+      '#title' => t('Publication'),
       '#default_value' => $title,
       '#autocomplete_path' => 'admin/tripal/storage/chado/auto_name/pub',
       '#ajax' => array(
@@ -135,46 +133,49 @@ class chado_linker__pub extends TripalField {
     );
   }
 
+  /**
+   * @see TripalField::widgetFormSubmit()
+   */
   public function widgetFormSubmit($entity_type, $entity, $langcode, &$items, $form, &$form_state) {
 
-    $field_name = $element['#field_name'];
-    $delta = $element['#delta'];
-    $table_name = $element['#table_name'];
-    $fkey = $element['#fkey_field'];
-    
+    // Get the FK column that links to the base table.
+    $table_name = $this->field['settings']['chado_table'];
+    $base_table = $this->field['settings']['base_table'];
+    $schema = chado_get_schema($table_name);
+    $pkey = $schema['primary key'][0];
+    $fkeys = array_values($schema['foreign keys'][$base_table]['columns']);
+    $fkey = $fkeys[0];
+
     // If the form ID is field_ui_field_edit_form, then the user is editing the
     // field's values in the manage fields form of Drupal.  We don't want
     // to validate it as if it were being used in a data entry form.
     if ($form_state['build_info']['form_id'] =='field_ui_field_edit_form') {
       return;
     }
-    
+
     // Get the field values.
-    //   $fkey_value = tripal_chado_get_field_form_values($table_name, $form_state, $delta, $table_name . '__' . $fkey);
-    //   $pub_id = tripal_chado_get_field_form_values($table_name, $form_state, $delta, $table_name . '__pub_id');
-    //   $uname = tripal_chado_get_field_form_values($table_name, $form_state, $delta, $table_name . '--pub__uniquename');
-    
-    // If the user provided a uniquename then we want to set the
-    // foreign key value to be the chado_record_idd
-    if ($uname) {
-    
-      // Get the pub. If one with the same name and type is already present
-      // then use that. Otherwise, insert a new one.
-      if (!$pub_id) {
+    foreach ($items as $delta => $values) {
+      $fkey_value = $values['value'];
+      $pub_id = $values[$table_name . '__pub_id'];
+      $uname = $values[$table_name . '--pub__uniquename'];
+
+      // If the user provided a uniquename then we want to set the foreign key
+      // value to be the chado_record_id
+      if ($uname and !$pub_id) {
         $pub = chado_generate_var('pub', array('uniquename' => $uname));
-        // Set the pub_id and FK value
-        tripal_chado_set_field_form_values($field_name, $form_state, $pub->pub_id, $delta, $table_name . '__pub_id');
-        $fkey_value = $element['#entity']->chado_record_id;
-        tripal_chado_set_field_form_values($field_name, $form_state, $fkey_value, $delta, $table_name . '__' . $fkey);
+        $items[$delta][$table_name . '__pub_id'] = $pub->pub_id;
+      }
+
+      // In the widgetFrom function we automatically add the foreign key
+      // record.  But if the user did not provide a publication we want to take
+      // it out so that the Chado field_storage infrastructure won't try to
+      // write a record.
+      if (!$uname and !$pub_id) {
+        $items[$delta][$table_name . '__' . $fkey] = '';
       }
-    
-    }
-    else {
-      // If the $syn_name is not set, then remove the linker FK value to the base table.
-      tripal_chado_set_field_form_values($field_name, $form_state, '', $delta, $table_name . '__' . $fkey);
-      tripal_chado_set_field_form_values($field_name, $form_state, '', $delta, $table_name . '__pub_id');
     }
   }
+
   /**
    * @see TripalField::load()
    */

tripal_chado/includes/TripalFields/chado_linker__relationship.inc

@@ -552,7 +552,7 @@ class chado_linker__relationship extends TripalField {
    * @see TripalField::submit()
    */
   public function widgetFormSubmit($entity_type, $entity, $langcode, &$items, $form, &$form_state) {
-
+return;
     $field_name = $this->field['field_name'];
     $field_type = $this->field['type'];
     $field_table = $this->field['settings']['chado_table'];
@@ -565,16 +565,16 @@ class chado_linker__relationship extends TripalField {
     $fkeys = $schema['foreign keys'];
 
     foreach ($items as $delta => $item) {
+      $type_name = array_key_exists('type_name', $item) ? $item['type_name'] : '';
       $subject_id = $item[$field_table . '__subject_id'];
       $object_id = $item[ $field_table . '__object_id'];
       $type_id = $item[$field_table . '__type_id'];
-      $type_id = isset($item['type_id']) ? $type_id : $item['type_id'];
-      $type_name = $item['type_name'];
+
       $subject_name = $item['subject_name'];
       $object_name = $item['object_name'];
 
       // If the row is empty then skip this one, there's nothing to validate.
-      if (!$type_id and !$type_name and !$subject_name and !$object_name) {
+      if (!($type_id or !$type_name) and !$subject_name and !$object_name) {
         continue;
       }
 

tripal_chado/includes/tripal_chado.field_storage.inc

@@ -27,7 +27,10 @@ function tripal_chado_field_storage_write($entity_type, $entity, $op, $fields) {
   $bundle = tripal_load_bundle_entity(array('name' => $entity->bundle));
   $term = entity_load('TripalTerm', array('id' => $entity->term_id));
   $term = reset($term);
-  $cvterm = $term->details['cvterm'];
+
+  // Convert the Tripal term entity into the appropriate record in Chado.
+  $dbxref = tripal_get_dbxref(array('accession' => $term->accession, 'db_id' => array('name' => $term->vocab->vocabulary)));
+  $cvterm = tripal_get_cvterm(array('dbxref_id' => $dbxref->dbxref_id));
 
   // Get the base table, type field and record_id from the entity.
   $base_table = $entity->chado_table;
@@ -39,7 +42,7 @@ function tripal_chado_field_storage_write($entity_type, $entity, $op, $fields) {
 
   // Convert the fields into a key/value list of fields and their values.
   $field_vals = tripal_chado_field_storage_write_merge_fields($fields, $entity_type, $entity);
-
+dpm($field_vals);
   // First, write the record for the base table.  If we have a record id then
   // this is an upate and we need to set the primary key.  If not, then this
   // is an insert and we need to set the type_id if the table supports it.
@@ -82,7 +85,7 @@ function tripal_chado_field_storage_write($entity_type, $entity, $op, $fields) {
 }
 
 /**
- * Write (inserts/oupdates) a nested array of values for a table.
+ * Write (inserts/updates) a nested array of values for a table.
  *
  * The $values array is of the same format used by chado_insert_record() and
  * chado_update_record().  However, both of those methods will use any nested
@@ -107,6 +110,8 @@ function tripal_chado_field_storage_write_table($table_name, $values) {
    $schema = chado_get_schema($table_name);
    $fkeys = $schema['foreign keys'];
    $pkey = $schema['primary key'][0];
+   dpm(array($table_name, $values));
+
 
    // Before inserting or updating this table, recurse if there are any
    // nested FK array values.
@@ -145,7 +150,7 @@ function tripal_chado_field_storage_write_table($table_name, $values) {
 
    // If the primary key column has a value then this will be an udpate,
    // otherwise it's an insert.
-   if (!array_key_exists($pkey, $values) or !$values[$pkey]) {
+   if (!array_key_exists($pkey, $values) or !$values[$pkey] or !isset($values[$pkey])) {
      // Before inserting, we want to make sure the record does not
      // already exist.  Using the unique constraint check for a matching record.
      $options = array('is_duplicate' => TRUE);
@@ -155,8 +160,11 @@ function tripal_chado_field_storage_write_table($table_name, $values) {
        return $record[0]->$pkey;
      }
 
-     // Insert the values array as a new record in the table.
-     $record = chado_insert_record($table_name, $values);
+     // Insert the values array as a new record in the table but remove the
+     // pkey as it should be set.
+     $new_vals = $values;
+     unset($new_vals[$pkey]);
+     $record = chado_insert_record($table_name, $new_vals);
      if ($record === FALSE) {
        throw new Exception('Could not insert Chado record into table: "' . $table_name . '".');
      }
@@ -337,9 +345,9 @@ function tripal_chado_field_storage_write_merge_fields($fields, $entity_type, $e
 
           // Is this a nested FK field? If so break it down into a sub array.
           if (preg_match('/^(.*?)--(.*?)$/', $column_name, $matches)) {
-            $parent_item_name = $matches[1];
-            $sub_item_name = $matches[2];
-            $sub_item = tripal_chado_field_storage_expand_field($sub_item_name, $value);
+            $linker_table = $matches[1];
+            $base_table = $matches[2];
+            $sub_item = tripal_chado_field_storage_expand_field($base_table, $value);
             if (count(array_keys($sub_item))) {
               // If we've already encountered this table and column then we've
               // already seen the numeric FK value or we've already added a
@@ -347,11 +355,11 @@ function tripal_chado_field_storage_write_merge_fields($fields, $entity_type, $e
               // so we can add the details.
               if (!array_key_exists($table_name, $new_fields) or
                   !array_key_exists($delta, $new_fields[$table_name]) or
-                  !array_key_exists($parent_item_name, $new_fields[$table_name][$delta]) or
-                  !is_array($new_fields[$table_name][$delta][$parent_item_name])) {
-                $new_fields[$table_name][$delta][$parent_item_name] = array();
+                  !array_key_exists($linker_table, $new_fields[$table_name][$delta]) or
+                  !is_array($new_fields[$table_name][$delta][$linker_table])) {
+                $new_fields[$table_name][$delta][$linker_table] = array();
               }
-              $new_fields[$table_name][$delta][$parent_item_name] += $sub_item;
+              $new_fields[$table_name][$delta][$linker_table] += $sub_item;
             }
           }
           else {
@@ -383,9 +391,7 @@ function tripal_chado_field_storage_write_merge_fields($fields, $entity_type, $e
 }
 
 /**
- * Recurses through a field's item name breaking it into a nested array.
- *
- *
+ * Recurses through a field's items breaking it into a nested array.
  */
 function tripal_chado_field_storage_expand_field($item_name, $value) {