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

Tripal content types: full support for TripalTerms #317

Closed laceysanderson closed 1 year ago

laceysanderson commented 1 year ago

This PR adds full support for Tripal Terms to Tripal Content Types. This was started by @spficklin with #244 in the Chado Prepare and with previous implementation of TripalTerms.

How to Test

  1. Build a new version of the Tripal Docker. This is needed to ensure the prepare step is run fresh on a clean site. Simply running prepare on an existing site may not be sufficient as the content types already exist.
git clone https://github.com/tripal/t4d8.git t4d8-256
cd t4d8-256
git checkout tv4g1-issue256-tripalContentType-termsAttached
docker build --tag=tripalproject/tripaldocker:t4d8-256 --build-arg drupalversion='9.4.x-dev' ./

The image should build without error. This shows that prepare can still be run + create content types without throwing exceptions.

  1. Now create a container to do your testing in.
docker run --publish=9000:80 --name=t4d8-256 -tid --volume=`pwd`:/var/www/drupal9/web/modules/contrib/tripal tripalproject/tripaldocker:t4d8-256
docker exec t4d8-256 service postgresql restart

All further steps take place in this container through the web browser: localhost

  1. Go to the Tripal Content Types listing (Admin > Tripal > Page Structure) and check that all content types are created and that the term column is filled out. This shows the prepare attached terms properly and that the list builder is able to retrieve them.

    Screen Shot 2022-12-26 at 1 03 46 PM
  2. Edit any content type and check that the term is fully described at the top of the page and the autocomplete is filled out and disabled. Save the content type again to make sure it doesn't change the term or throw and error on update.

    Screen Shot 2022-12-26 at 1 04 11 PM
  3. Now create a new content type by clicking "Add Tripal Content Type" at the top of the Tripal Page Structure page. Fill in all the required fields. Confirm the controlled vocabulary term autocomplete pulls up values. Try the following and ensure you get validation errors:

Screen Shot 2022-12-26 at 1 04 11 PM
  1. Finally fill out the form in the above step correctly and click save. Check the new content type is in the list with the correct term associated with it.
Screen Shot 2022-12-26 at 1 12 04 PM

PR Summary of Changes

✅ Pass term object to create and it saves IDSpace and Accession + sets help text based on term definition if not already set ✅ The content type ID/Name (i.e. bio_data\d+) is not automatically created when the content type is saved ✅ If not set already, the category, hide_empty_field and ajax_field are set with sensible details on save. ✅ Label, help text, term IDSpace + Accession are now checked/required on save (exception thrown). ✅ There is a new setTerm() method to allow you to set the term details with a term object rather then separately. ✅ Upgraded edit/create tripalentitytype form to show the attached term if set. ✅ Updated some tests which didn't meet the new requirements for content types. ✅ Edit/create form now has the same term autocomplete that field settings do. It is set via form state on create and via the term on update. It's disabled on update. ✅ Added to the validate of the edit/create for. It now validates the following:

✅ The term is added to the entity during the submit ✅ The content type listing is updated to display the term ✅ Fixed bug in the entity where it set the ID Space in setTermAccession() instead of the accession ✅ Fixed a bug where you couldn't save an existing content type because it tried to re-assign the id.

spficklin commented 1 year ago

This is great @laceysanderson and so nice to have working! I tested on my end and all worked as expected.

spficklin commented 1 year ago

I went ahead and merged in the 9.x-4.x branch as it was a bit out of date. Looks like only updates to the tripaldocker were included.

spficklin commented 1 year ago

This PR also addresses issue #256 right?

laceysanderson commented 1 year ago

Yes, #256 is linked in the sidebar under development and will be automatically closed once this is merged :-)

codeclimate[bot] commented 1 year ago

Code Climate has analyzed commit e27054b2 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

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

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

View more on Code Climate.