Future patch to remove taxonomy_term_hierarchy

See https://www.drupal.org/node/2936675

This module will break because it is interacting directly with entity tables via SQL instead of using the methods on TermStorage handler.

The core issue has been committed to 8.6.x and is being considered for backport to 8.5.x

I wanted to help file a patch for this, but the first bit of code I wanted to update was this:

  //remove root level entry
  db_delete('taxonomy_term_hierarchy')
    ->condition('tid', $term->tid)
    ->condition('parent', 0)
    ->execute();

  taxonomy_term_save($term); //executes hooks and clears caches

… but taxonomy_term_save() doesn't actually exist in D8…

AFAICT 100% of the code in taxonomy_manager.admin.inc is just there to be ported, it's not executed. The commit log also supports this:

wim.leers at acquia in ~/Work/d8/modules/taxonomy_manager on 8.x-1.x*
$ git lg taxonomy_manager.admin.inc
* 9ae22d7 - Fixed path alias generation for terms translated via the entity translation module. (3 years, 5 months ago) <Matthias Hutterer>
* 842a3dc - Entity translation and title field integration. (4 years, 2 months ago) <Matthias Hutterer>
* 22ce9c4 - (tag: 7.x-1.0) Fixed coding style. (4 years, 8 months ago) <Matthias Hutterer>

Zero commits touching this file since the release of Drupal 8. The last commit was on Sep 2, 2014, more than a year before Drupal 8 was released.

Yet taxonomy_manager.admin.inc has 8 of the 12 occurrences of the string taxonomy_term_hierarchy:

This means rather than 12 places to update, there are only 4. Working on it!

This fixes 2 occurrences of taxonomy_term_hierarchy to use the actual Taxonomy API, with minimal changes. These occurrences are definitely being used in the actual code.

The next use, in \Drupal\taxonomy_manager\Element\TaxonomyManagerTree::isRoot(), is only called by \Drupal\taxonomy_manager\Element\TaxonomyManagerTree::getFirstPath(), which in turn is only called by \Drupal\taxonomy_manager\Element\TaxonomyManagerTree::processTree(), which in turn is only called if $element['#terms_to_expand'] is set, which is never set by the module itself, only by potential custom code.

If I do set #terms_to_expand to something, then \Drupal\taxonomy_manager\Element\TaxonomyManagerTree::isRoot() is called and … results in a fatal PHP error because the code is wrong:

  public static function isRoot($tid) {

    $database = \Drupal::database();
    $query = $database->select('taxonomy_term_hierarchy', 'h');
    $query->fields('h', 'tid');
    $query->condition('h.parent', 0);
    $query->condition('h.tid', $tid);
    $result = $query->countQuery()->execute()->fetchField();

This results in the following PHP fatal error:

The website encountered an unexpected error. Please try again later.
TypeError: Argument 2 passed to Drupal\Core\Database\Query\Select::fields() must be of the type array, string given, called in /Users/wim.leers/Work/d8/modules/taxonomy_manager/src/Element/TaxonomyManagerTree.php on line 243 in Drupal\Core\Database\Query\Select->fields() (line 555 of core/lib/Drupal/Core/Database/Query/Select.php).
Drupal\Core\Database\Query\Select->fields('h', 'tid') (Line: 243)
Drupal\taxonomy_manager\Element\TaxonomyManagerTree::isRoot('3') (Line: 180)
Drupal\taxonomy_manager\Element\TaxonomyManagerTree::getFirstPath(4, Array) (Line: 42)
Drupal\taxonomy_manager\Element\TaxonomyManagerTree::processTree(Array, Object, Array)
call_user_func_array(Array, Array) (Line: 993)
Drupal\Core\Form\FormBuilder->doBuildForm('taxonomy_manager.vocabulary_terms_form', Array, Object) (Line: 1056)
Drupal\Core\Form\FormBuilder->doBuildForm('taxonomy_manager.vocabulary_terms_form', Array, Object) (Line: 1056)
Drupal\Core\Form\FormBuilder->doBuildForm('taxonomy_manager.vocabulary_terms_form', Array, Object) (Line: 1056)
Drupal\Core\Form\FormBuilder->doBuildForm('taxonomy_manager.vocabulary_terms_form', Array, Object) (Line: 557)
Drupal\Core\Form\FormBuilder->processForm('taxonomy_manager.vocabulary_terms_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm('taxonomy_manager.vocabulary_terms_form', Object) (Line: 74)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
call_user_func_array(Object, Array) (Line: 153)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 657)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Despite #7's explanation about how this is completely broken in HEAD, the intent of the code is clear, and I can update it to use the proper API and fix the fatal error in doing so!

The fourth and final occurrence of taxonomy_term_hierarchy cannot be updated to use TermStorageInterface, because it also uses a pager. TermStorage doesn't support paging.

But … that's what entity queries are for! Entity queries do support paging. But … entity queries don't yet support the parent field, because it uses custom storage, as described in #2599450: Drupal\Core\Entity\Query\QueryException: 'parent' not found in Drupal\Core\Entity\Query\Sql\Tables->ensureEntityTable. But that's exactly what #2543726: Make $term->parent behave like any other entity reference field, to fix REST and Migrate support and de-customize its Views integration is fixing!

So, let's be a bit creative. Let's use an entity query, but catch the exception that will be thrown on sites that don't yet have #2599450: Drupal\Core\Entity\Query\QueryException: 'parent' not found in Drupal\Core\Entity\Query\Sql\Tables->ensureEntityTable. If that exception is caught, we just execute HEAD's code (which uses the taxonomy_term_hierarchy table). In the future, we'll just be able to remove that code!

Note that the patch in #9 makes the module work with both Drupal 8 HEAD (without #2543726) and with #2543726: Make $term->parent behave like any other entity reference field, to fix REST and Migrate support and de-customize its Views integration applied!

@Wim-Leers

This is superb work, thanks so much. I applied for maintainer status late last year in attempt get the 3-4 patches in our build system committed and to start getting the module in a reasonable shape for 8. I don't have a lot of time, but the primary maintainer isn't running 8.x yet and isn't focused on it. So I'll fill in where I can.

As you say, it does appear MUCH of the code is 'awaiting a D8 port', many functions avoid using OOP, and some files in the tree are no longer even used in D8 (#2792761: taxonomy_vocabulary_load and taxonomy_vocabulary_load have been deprecated).

It's really great to get this cleanup started in such a thorough way..

@JacobSanford: wow, I did not expect this to get committed so fast! And thank you for the kind words :)

Porting a module can be hard, especially one that has seemingly been minimally maintained for a while: rather than lots of straightforward changes, it means much more complex changes are necessary, and that can definitely be challenging! I'm glad that I was able to lend you a hand in this very small way — hopefully it makes you feel a bit more optimistic about the rest :)