tripal / t4d8

This is a temporary repository for Drupal 9 upgrade for Tripal v4. It is meant to house issues related to the upgrade.
GNU General Public License v2.0
1 stars 10 forks source link

Type Field #303

Closed spficklin closed 1 year ago

spficklin commented 1 year ago

Issues Addressed

Issue #298 Issue #129

Description

This PR adds a new field for managing additional types. There are three types of types that this field supports. This is a required addition prior to Alpha 1 release. The field specifically supports managing "types" in three different ways:

  1. All content types have a type. If the record has an additional type that is different from the content type then this field will support it. Examples include: organism.type_id , pub.type_id, contact.type_id, 'featuremap.unittype_id`, etc.
  2. A type that is the same as the content type and saves to a table in Chado that has a type_id field, but it will always be the same as the entity type. The user should never change it. Examples include all of the feature types (e.g. gene, mRNA) and stock types.
  3. A type that is the same as the content type but saves to a property table in Chado. For example, this happens for some records stored in the analysisprop table. Example content types include 'Genome assembly', 'Genome annotation' etc.

Changes

The following changes were made to support this field:

  1. I realized that the Drupal\tripal\TripalField\Interfaces\TripalFieldItemInterface::tripalValuesTemplate() function did not have easy access to the field definitions. So, I added an argument to pass it in. I thought I needed it for this field so I added it but it turned out I didn't. However, I left it as is in th eevent that in the future someone will need the field definition to set anything in the value (e.g. the property key). I updated the tripalValuesTemplate function for all fields to match.
  2. I added a new 'cache' setting to property types in their 'storage_plugin_settings' section. Previously, we would only cache the record_id property whenever a field was saved. This way we don't duplicate the data that is stored in Chado also inside of the Drupal schema. However, when working with properties that have values in linker tables, we also need to store the linker record ID too. This happens in the 3rd case above, where the type for the field is stored in the analysisprop table. I changed the Drupal\tripal\Entity\TripalEntity::preSave so that it would cache any field whose storage_plugin_settings had a cache value set to TRUE. I also changed the Drupal\tripal\TripalField\TripalFieldItemBase::defaultTripalTypes() function to set the record_id property to have a cache value of TRUE. Now, any field that has cache set to TRUE will get cached by Drupal.
  3. I added a new 'fixed_value' setting to property types in their 'storage_plugin_settings' section. If any properties have a fixed_value then the widget is supposed to honor that and not let the user change the value. This is what allows the content types tied to the type_id fields of Chado tables to stay fixed with the content type's type.
  4. The tripal_chado/config/install/tripal_chado.chado_term_mapping.core_mapping.yml file was missing mappings for the analysisprop table. I added those.
  5. I added a new type of action for Chado properties in fields: the link action. Previously we had store, join, replace and function. The link action is what tells the ChadoStorage class that the field is managing a linked record (e.g. analysisprop). You can see where this gets used in the schema__additional_type field on line 83. For this field, the property is managing the the primary key (e.g. analysisprop_id), but is linked to the analysis table via the analysis_id field.
  6. I updated the ChadoPrepare task to add new type fields to all content types that use them.

Testing

  1. Start with a completely fresh installation of Drupal/Tripal.
  2. Install chado
  3. Prepare Chado
  4. Create entities for each of the three types:
    • Create an organism record with an intrapsecific type. You should be able to create this without any errors. You should be able to change the type as often as you like.
    • Create a gene or mRNA record. You will not be able to change the type. But you can edit/update as you like without it changing.
    • Create a Genome assembly entity. You will not be able to change the type. But you can edit/update as you like. There should also be a corresponding record in the analysisprop table.
    • Create a normal analysis to compare. It also gets saved to the analysis table but does not have a type so it does not add a record in the analysisprop table.

This to Note

  1. I have not written any function testing yet. I wanted to get this posted so folks could at least test it. It can wait to be merged until we have functional test.
  2. When creating an mRNA or gene page the field for the length of the sequence requires a value or it won't let you save.... that's a bug for a different day.
laceysanderson commented 1 year ago

Why did the validation in ChadoStorage get removed in the last commits? We had talked about removing automated testing but these are further then that and were working before -weren't they? They definitely were in the property field. I wonder if this is a merge conflict mistake @spficklin?

codeclimate[bot] commented 1 year ago

Code Climate has analyzed commit b9443481 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 59.9% (50% is the threshold).

This pull request will bring the total coverage in the repository to 34.3% (0.3% change).

View more on Code Climate.