Rename duplicated terms in tcontact.obo

[dsenalik]

Bug Fix

Closes #1906

Tripal Version: 4.x

Description

This renames the two duplicated terms to be unique, and renames a third term for consistency. See more details in linked issue #1906
Also, this file had DOS style line endings, so converted this. See commit eb58f33 to more clearly see the actual changes.

Testing?

Build a docker
Add a new contact
Enter "%Departmen" for the type, the autocomplete should offer

Change to "%Organizatio"

[laceysanderson]

I think it would also be good to also change/add a definition to make it clear that these are specifically related to the contact (i.e. properties). The other terms with the same name are intended to be the type of contact.

Affiliated Department: The department of the institution or organization that the contact is affiliated with.

Affiliated Institution: The employer (e.g. company, university, corporation) that the contact is affiliated with.

Affiliated Organization: The formal group of people the contact is affiliated with. Organizations differ from institutions by being more focused on people coming together for a specific purpose; where as, institutions usually refer to physical locations of a particular corporation.

[spficklin]

Have we always been able to use the % character in an autocomplete? Does that get fed into the SQL query? If so, that really makes me nervous. I think that characters is benign but can you give it others and hack the query?

[dsenalik]

That's a very good question @spficklin that I had not thought about.
You could do this in Tripal 3 also

The good news is that the database abstraction layer will handle sanitization for a condition() method as long as the user input is not used to select the operation -
Writing secure code for Drupal / Use the database abstraction layer to avoid SQL injection attacks
There is an example there of how to prevent use of the wildcard character, but because it is highly useful for the autocomplete we would not want to do that.

Our code, where $name comes from the autocomplete input, is

    if (array_key_exists('exact', $options) and $options['exact'] === True) {
      $query1->condition('CVT.name', $name, '=');
    }
    else {
      $query1->condition('CVT.name', $name . '%', 'LIKE');
    }

[laceysanderson]

I think based on the code that it is safe to continue allowing this for the autocompletes :-)

The updated definitions look perfect @dsenalik <3
Do we also want an update hook to change these term names in existing chado instances?

[dsenalik]

I did not think we would need an update hook, because if we do the Chado prepare step when migrating, it will change these terms.

[laceysanderson]

😮 So you are right -no need for an update hook for this one.

I finally found the time to test this. I created a docker on the 4.x branch and queried to confirm that I had the old version of the department term:

2956	tripal_contact	Department
2963	tripal_contact	Department(TCONTACT:0000016)

Then I switched to this branch and ran the prepare again. The prepare ran again without an error. Then I queried the terms again.

2956	tripal_contact	Department
2963	tripal_contact	Affiliated Department