Issue #1266 sbo__relationship field infraspecific nomenclature support

[dsenalik]

Bug Fix

Issue #1266

Description

Changes in this pull request allow the sbo__relationship tripal field to support infraspecific nomenclature for organisms, which was added with chado v1.3

For the organism name, preference is given first to the abbreviation in the chado.organism table. If no abbreviation is defined, and we have chado >=1.3, use the chado_get_organism_scientific_name() function to get the organism name. Fallback is to the original genus + species method of constructing the name.

I also removed an extra space introduced into the relationship clause when the verb from get_rel_verb() is a null string by padding in that function only when there is a verb returned. This only appears visibly in web services.

Testing?

Need chado 1.3, and one or more organisms with subspecies or other infraspecific name, and a relationship in the organism_relationship table, e.g.

[dsenalik]

I am trying to understand why testing is failing and so far can't figure it out.

[dsenalik]

Ooooh that was an insidious hard-to-spot bug that the testing caught, but only after the first PHP8 bug was fixed. All should be good now!

[dsenalik]

PHP8, empty tripal3 test site, adding a new organism, when select an empty ontology (i.e. no terms, project_relationship on an empty tripal3 site), I see this error from the schema__publication field, noting it so I don't forget Warning: Undefined array key "accession" in schema__publication_widget->form() (line 69 of /var/www/prova/drupal-7.92/sites/all/modules/tripal/tripal_chado/includes/TripalFields/schema__publication/schema__publication_widget.inc).

Fixed with https://github.com/tripal/tripal/pull/1267/commits/ab2ac5f2c28c2f6785164376a9eef8aebda343de

[dsenalik]

The "weird bug" in https://github.com/tripal/tripal/pull/1267#pullrequestreview-1201494455 is a pre-existing bug. Most relationships are on content types named with just schema__name. But others, such as organism, use something else, and schema__name does not exist. This is currently used in sbo__relationship_table_formatter.inc to determine if the current content page is subject or it is object, and for organisms this is undefined and fails so the page is always "subject". Under PHP8 you also get warnings that help track this down. Warning: Undefined property: TripalEntity::$schema__name in sbo__relationship_table_formatter->view() (line 183 of /var/www/prova/drupal-7.92/sites/all/modules/tripal/tripal_chado/includes/TripalFields/sbo__relationship_table/sbo__relationship_table_formatter.inc).

The offending section of code is:

      // Determine if the subject or object is the current entity.
      $is_subject = TRUE;
      if ($object_name == $entity->schema__name['und'][0]['value']) {
        $is_subject = FALSE;
      }

I can make this check more generic and work on all content types with this simple fix:

      if ($entity->chado_record_id == $item['chado-'.$entity->chado_table.'_relationship__object_id']) {

[dsenalik]

New issues noted in the "Relationship Table by Type" version:

The grammar of the cv term is not formatted as nicely as in the "Relationship Statements", e.g. The following are Is Individual Of Population this Organism because it uses these settings

[dsenalik]

To finish off review responses, I can see infraspecific name in organism add page so @laceysanderson can you please try that again?

The validation error may have been the same one I fixed in the chado_linker__contact field in pull #1319 so I will incorporate that fix here too for testing https://github.com/tripal/tripal/pull/1267/commits/5949490bdd9eb03e2a3895fe80c12e2d798469d4 and remember that the order of preference for the organism name is: abbreviation → chado_get_organism_scientific_name() → Genus+species so if you filled out abbreviation while creating the new organism, that is what needs to go in the box.

[dsenalik]

The state of this pull request leaves organism relationships only fully working for Genus + species, infraspecific nomenclature only works if the Ajax callback sets the [id: xx] organism_id value. To look up an organism by name e.g. Tripalus domesticus subsp. chadoii is not easily done in just SQL because of the type abbreviations (for which there are api functions). I will propose a new api function for this lookup, and can then implement full infraspecific support for this field later.

I created Issue #1327 for this.

[dsenalik]

@laceysanderson this "caveat" you noted is almost certainly due to the lack of infraspecific support in one spot, where only genus and species are queried in the organism table. I can replicate exactly what you saw. I will be able to easily fix this if pull #1328 is approved.

[laceysanderson]

Great, I thought that might be the case :-) Can we add a validation error to this PR for that one case just so it doesn't create an erroneous relationship?

[dsenalik]

I have added this with https://github.com/tripal/tripal/pull/1267/commits/fa3e0a938da20bea8c0b606415866b4cfc1b695e and it now prevents this from happening Note that I did not have to check for multiple matches, that is already caught elsewhere.

[dsenalik]

I am putting this here so I remember next year :yawning_face: If you use the relationship field to link two derived content types, e.g. Genome Assembly which is derived from Analysis, then the underlying type is displayed, and both subject and object are clickable links. For example