New base table third party setting + Drupal 10.2 fixes

[dsenalik]

New Feature + Bug Fix

Closes #1440

Closes #1791

Closes #1818

Tripal Version: 4.x

Description

Because they are so intertwined, this PR does several things:

1. Create a third party setting so that fields can finally know what their base table should be (Thanks to @spficklin for this genius discovery).
2. Introduces an is_compatible() function that fields will implement a way to know what fields can be added to a particular content type.
3. Fix the form for adding fields through the GUI to support the new Drupal 10.2 subform.
   To quote @laceysanderson : "In Drupal 10.2 the UI for adding custom fields has changed quite a lot.
   This is happening because in Drupal 10.2 the field settings and field storage settings have each become subforms and then been merged into a single form page. This means that where we originally were given the full form state, now we are given a subformstate which only has the values specific to our subform in it."
   Drupal reference: https://www.drupal.org/node/3386675
   This PR will replace Fixes for the new Drupal 10.2 subform #1795
4. Edit (spficklin): Also adds the YAML, library entities and CSS for grouping Tripal and Tripal Chado fields in the new 10.2 Drupal field grid layout.
5. For a new content type defined through the GUI, saves the base table selected for the first Chado field added to that content type as the base table for all subsequent Chado fields (Issue Provide a way to set the base table for a newly defined content type #1818)

Testing?

Test on BOTH Drupal 10.1 and 10.2:

1. Test a base column form element by adding a Chado Text Field Type, e.g. to project
   
   You might notice that under 10.2 you can also see the description, so I took the opportunity to make the distinction between Text and String clear (I always seem to forget which is which)
   The new third party setting will allow the field to know that the base table is project, so the selector should be pre-populated and grayed out. The "Table Column" should be populated with the only valid choice, "Description"
   
   Select any CV Term - this is a two-form process under 10.1, but now all on the same form in 10.2
   
   Save and verify the field looks good.
   Go back to view with Edit and make sure everything is still there

2. Test the property form by adding a Chado Property to e.g. pub. The base table selector will again be grayed out.
   Use whatever CV term you like

3. Test the linking table form element by adding a Chado Contact field to e.g. project.
   The CV term should not need to be entered for this one.

4. Test the linking table form element by adding an Analysis field to e.g. project.
   In this case there is no default CV term so you will need to specify one.

5. Test the Chado Database Cross Reference field

6. Remove the original "Description", "Contact", and "Dbxref" (Database reference annotation) fields, we are going to test the ones we just added.

7. Create some content. First create a "contact", and then an "analysis".

8. Create a project and use this contact, fill in a description, your property, your analysis, and any dbxref value.
   

9. Other fields to test: Chado Type Reference (ChadoAdditionalTypeDefault) (e.g. on organism, pub, contact, or protocol)

10. Chado Synonym (on any feature table derived content, e.g. gene)

11. Chado Unit (on featuremap types only, Genetic Map or Physical Map)

12. Try to add a field that is wrong for the content type, e.g. put the Chado Unit field on a publication. Or consult the big linker table Expanding Tripal 4 Fields documentation tripal_doc#32 (comment) and find a blank cell, e.g. try to put the Feature field on a Study. (Sometimes there is an obscure linking table e.g. Contact on an Analysis through the quantification table, but that is not by default a content type)

[laceysanderson]

To fix the schema incomplete failures you will want to add the third party setting to the schema. Since the content type is defined in the Tripal module and this is chado specific you need to use the tripal_chado_config_schema_info_alter hook in the tripal_chado.module file to alter the schema.

• For future reference, an example of the error was

45) Drupal\Tests\tripal_chado\Functional\api\SchemaAPITest::testGetBaseTables with data set #0
   ('1.3', array('organism', 'feature', 'stock', 'project', 'analysis', 'phylotree'))
  Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for 
  tripal.tripalentitytype_collection.expression_chado with the following errors: 
  tripal.tripalentitytype_collection.expression_chado:content_types.0.settings.chado_base_table missing schema, 
  tripal.tripalentitytype_collection.expression_chado:content_types.1.settings.chado_base_table missing schema, 
  tripal.tripalentitytype_collection.expression_chado:content_types.2.settings.chado_base_table missing schema

[dsenalik]

I have implemented the "Quick Fix" with commit 01c081f and isCompatible() will now return true if the base table is not defined.
You can now add Chado fields to a new content type.

When I did this, I could proceed, and discovered that the ajax was broken under Drupal 10.2.
So that was fixed with commit b0dfc9d

A few fields are still broken when adding to a newly defined content type, but as Lacey said, there is a lot in this PR already so I will create a new issue for this, issue #1817

And probably a separate new issue would be to provide a way to define the base table when creating a new content type, this will make a site admin's job easier as it doesn't have to be selected over and over for each new field added. Issue #1818

[spficklin]

When content types are created through the UI we currently don't have a way to set the base table for the content type.

I think we should fix this. If we don't require that a field's base table matches the one for the content type then you could get a bunch of fields referencing differnet tables, which will then cause problems if some one wants to "check for new fields".

[spficklin]

Perhaps the solution is that if the contnet type doesn't have a base table set, then when the first Chado field is added then it will set the base table for the content type. Then all others added later must match the base table of the content type.

Also , I think our implementations of the isCompatible() function is getting too repetitive and we're forcing developers to always remember to code the check for a missing base table in the content type. If they forget to code it then their fields won't work like we want. I think that check should be handled outside of the field and the isCompatible() for each field should only be responsible for making sure the base table on the content type is compatible with the field (true if so, false if not). Perhaps we only call isCompatible() if a base table is present in the content type.

[spficklin]

AS per additional conversation with @laceysanderson on this last comment (on Slack) i'm working on the fix.

[spficklin]

Okay, I've added a fix that sets the base table on a content type automatically when the first Chado field is added.

However, I'm really confused by the code that adjust the base table drop down. @dsenalik I think you need to look at it to see what's wrong. Here's the problem.

1. I create a new content type manually through the GUI
2. I add a new "Chado Contact" field and set the base table to be "library". This should then set the base table for the content type to be 'library'.
3. Add a new "Chado Property" field to the same content type. But even though it knows the base table is library for this content the base table drop down is empty.

[dsenalik]

On Drupal 10.2 I created a library content type. First field I used Chado String Type and the uniquename column. Then I added a Chado Contact Type and the base table was preselected and I could see the one possible linking method.

The fix I made was for the error
Warning: Undefined variable $base_table in Drupal\tripal_chado\TripalField\ChadoFieldItemBase->storageSettingsForm() (line 120 of modules/contrib/tripal/tripal_chado/src/TripalField/ChadoFieldItemBase.php).
It bothers me that there are so many variables for base table, probably a close analysis could remove at least one, but for now the fix in commit 9b202e8 works.

I will build a 10.1 docker to test there too

[dsenalik]

You get this error under Drupal 10.1
TypeError: array_key_exists(): Argument #2 ($array) must be of type array, null given in tripal_chado_form_alter() (line 230 of modules/contrib/tripal/tripal_chado/tripal_chado.module).

Your new hook unfortunately needs to know how to handle both drupal versions it seems. The fix is not as easy as I hoped. In tripal_chado_form_alter() the form ID is different depending on Drupal version.

  // Under Drupal 10.1 we want 'field_storage_config_edit_form', the storage form
  // Under Drupal 10.2 we want 'field_config_edit_form', storage is a subform within this form
  if ($form_id == 'field_config_edit_form') {

so I could do this
if (($form_id == 'field_config_edit_form') or ($form_id == 'field_storage_config_edit_form')) {

but then there is the check for entity type, which we cannot do from field_storage_config_edit_form in Drupal 10.1 because that is back in the field_config_edit_form

    $entity_type = $form['#entity'];
    if ($entity_type->getEntityTypeId() == 'tripal_entity') {

If I just take out the entity type check it works under Drupal 10.1.

    $entity_type = $form['#entity'] ?? NULL;
    if (!$entity_type or ($entity_type->getEntityTypeId() == 'tripal_entity')) {

But I don't understand whether we need that or not.
I'm sort of stuck here @spficklin HELP!

[spficklin]

I think we don't need the check for tripal entity, because the check for the base_table in the storage settings should be sufficien.t

[spficklin]

Thanks for doing all that cleanup work. It's unfortunate that the form_id's change as well as the form structure :-/

[dsenalik]

Thanks! If that's the case, I just pushed changes a4bc925 which should fix it for Drupal 10.1
I used null coalescing ?? to reduce the number of array_key_exists() 😼
Edit: I forgot I changed to using form state instead of form, as is done elsewhere.

[laceysanderson]

Ok, I retested and everything looks great except for a small hiccup on D10.0 regarding unsupported fields (see more below).

Task	D10.2/PHP8.3	D10.1 (Doug)	D10.0/PHP8.2
Add Chado Text field to existing content type	✅		✅
Add Chado Property field to existing content type	✅		✅
Add Chado Analysis field to existing content type	✅		✅
Unable to add unsupported field to content type	✅	❌	❌
Add Chado Text field to new content type	✅		✅
Add Chado Dbxref field to a new content type	✅		✅
Unable to add unsupported field to content type	✅	❌	❌

D10.0 ONLY, Unsupported fields bug

On Drupal 10.0 ONLY, if you try to add an unsupported field you will experience the following:

• You do get an error that the field is unsupported
• When you go back to the field listing though the unsupported field was added anyway and is in the listing 👿
• When you try to edit the unsupported field, again you get the message about it being unsupported and cannot set any details
• If you try to create content of that type, it will now fail as chado storage sees that unsupported field and adds it to the verification code 🙈

For example, I added a sequence checksum field on the existing project content type. When creating a project I now get errors about not having an organism, type, etc 😭

[dsenalik]

For Drupal ≤ 10.1 this is the point at which the field comes into existence, this is the form prior to where we check compatibility. Ideally, if the field is detected as not compatible in the next form, we would then have to also remove the field.

 Schema |              Name              | Type  |    Owner    
--------+--------------------------------+-------+-------------
 public | tripal_entity__field_noooooooo | table | drupaladmin

[dsenalik]

I just pushed a commit that adds a function to "clean up" when an incompatible field is added under Drupal ≤10.1 😀
$this->removeIncompatibleField($bundle, $machine_name);
It's not particularly complicated, so this may be the simplest fix.

[laceysanderson]

Works perfectly now! I just tested again on Drupal 10.2 and Drupal 10.1. I specifically tested one unsupported field and one supported field on an existing content type.

Thanks @spficklin and @dsenalik for all the work on this one!!! ❤️