Remove Usage of deprecated function taxonomy_*

Updating to needs review so that the patch is tested

Created a new patch.

Coupled CR and created follow-up issue to remove the functions: #2465463: Remove deprecated taxonomy_* functions..

.

Some stuff to get started:

1. +++ b/core/modules/forum/src/Form/ForumForm.php
   @@ -128,8 +128,7 @@ protected function actions(array $form, FormStateInterface $form_state) {
   -    // @todo Inject a taxonomy service when one exists.
   -    $parents = taxonomy_term_load_parents($tid);
   +    $parents = $this->entityManager->getStorage('taxonomy_term')->loadParents($tid);
   

   This method has enough occurrences of getStorage() that you might just call it once and reference the manager. Like:

   $taxonomy_storage = $this->entityManager->getStorage('taxonomy_term');
   $taxonomy_storage->doStuffWithTheStorageManager();
   

2. +++ b/core/modules/taxonomy/src/TermForm.php
   @@ -34,13 +34,14 @@ public function form(array $form, FormStateInterface $form_state) {
   -    // taxonomy_get_tree and taxonomy_term_load_parents may contain large
   +    // \Drupal\taxonomy\TermStorage::loadTree() and
   +    // Drupal\taxonomy\TermStorageController::loadParents() may contain large
   ...
   -      $children = taxonomy_get_tree($vocabulary->id(), $term->id());
   

   Needs leading \ before the second class name.

3. +++ b/core/modules/taxonomy/src/TermForm.php
   @@ -34,13 +34,14 @@ public function form(array $form, FormStateInterface $form_state) {
        if (!$this->config('taxonomy.settings')->get('override_selector')) {
   -      $parent = array_keys(taxonomy_term_load_parents($term->id()));
   -      $children = taxonomy_get_tree($vocabulary->id(), $term->id());
   +      $parent = array_keys($this->entityManager->getStorage('taxonomy_term')->loadParents($term->id()));
   +      $children = $this->entityManager->getStorage('taxonomy_term')->loadTree($vocabulary->id(), $term->id());
   

   Consolidate the calls to getManager() like in #1 above.

4. +++ b/core/modules/taxonomy/src/Tests/TermTest.php
   @@ -438,8 +438,9 @@ function testTermMultipleParentsInterface() {
   -    // not added by taxonomy_term_load_parents().
   -    $parents = taxonomy_term_load_parents($term->id());
   +    // not added by
   +    // \Drupal::entityManager()->getStorage('taxonomy_term')->loadParents().
   +    $parents = \Drupal::entityManager()->getStorage('taxonomy_term')->loadParents($term->id());
   

   Test classes descending from TestBase have their own dependency-injected container for the mocked Drupal under test.

   So instead of saying \Drupal::entityManager() you should say $this->container->get('entity.manager')

5. +++ b/core/modules/taxonomy/taxonomy.tokens.inc
   @@ -133,7 +133,7 @@ function taxonomy_tokens($type, $tokens, array $data = array(), array $options =
   -          if ($parents = taxonomy_term_load_parents($term->id())) {
   +          if ($parents = \Drupal::entityManager()->getStorage('taxonomy_term')->loadParents($term->id())) {
                $parent = array_pop($parents);
                $replacements[$original] = SafeMarkup::checkPlain($parent->getName());
              }
   @@ -146,7 +146,7 @@ function taxonomy_tokens($type, $tokens, array $data = array(), array $options =
   
   @@ -146,7 +146,7 @@ function taxonomy_tokens($type, $tokens, array $data = array(), array $options =
          $replacements += $token_service->generate('vocabulary', $vocabulary_tokens, array('vocabulary' => $vocabulary), $options);
        }
    
   -    if (($vocabulary_tokens = $token_service->findWithPrefix($tokens, 'parent')) && $parents = taxonomy_term_load_parents($term->id())) {
   +    if (($vocabulary_tokens = $token_service->findWithPrefix($tokens, 'parent')) && $parents = \Drupal::entityManager()->getStorage('taxonomy_term')->loadParents($term->id())) {
   

   For both of these, and for the other call to getStorage() down below it, grab a reference to the storage manager at the top of the function, outside the foreach() loop, and then just call the methods on that inside the loop.

Thanks!

Note: this is kind of a duplicate of this one: #2405365: Remove usage of taxonomy_term_load_parents()

working on it

Trying

Trying.

Hi tadityar,

Thank you for your patch.

I think the patch of AjitS (#16) is almost there, but there are still some failing tests in TermTest.php.

I haven't test it, but i think it's because the loadParents(), loadChildren() and loadTree() methods are directly called on the entitymanager instead of on the taxonomy storage.

So

$this->entityManager->loadParents();

should be changed to :

$this->entityManager->getStorage('taxonomy_term')->loadParents();

updated IS

@JeroenT You are right. That was the fatal error!
Correcting the error for now. Will collect the taxonomy storage in a single variable in the next patch.

Back to needs work as per #13.
Will update the patch soon.

Moved the taxonomy storage to variables wherever possible. IMO, the priority of the issue is not minor, as getting the storage at 10 different places, unnecessarily, might have performance issues.

2 more places to fix + docs error

1. +++ b/core/modules/taxonomy/src/TermForm.php
   @@ -34,13 +35,14 @@ public function form(array $form, FormStateInterface $form_state) {
   +    // \Drupal\taxonomy\TermStorage::loadTree() and
   +    // \Drupal\taxonomy\TermStorageController::loadParents() may contain large
   

   I'd suggest to use interface here \Drupal\taxonomy\TermStorageInterface that defines this methods

   not controller that could be swapped and there's no such thing *TSController*

2. +++ b/core/modules/taxonomy/src/Tests/TaxonomyTermIndentationTest.php
   @@ -56,7 +56,7 @@ function testTermIndentation() {
   +    $parents = \Drupal::entityManager()->getStorage('taxonomy_term')->loadParents($term2->id());
   
   @@ -73,7 +73,7 @@ function testTermIndentation() {
   +    $parents = \Drupal::entityManager()->getStorage('taxonomy_term')->loadParents($term2->id());
   

   Better to get the storage to separate variable here too

3. +++ b/core/modules/taxonomy/src/Tests/TermKernelTest.php
   @@ -113,11 +113,11 @@ public function testTaxonomyVocabularyTree() {
   +    $tree = \Drupal::entityManager()->getStorage('taxonomy_term')->loadTree($vocabulary->id(), $term[1]->id(), 1);
   ...
   +    $tree = \Drupal::entityManager()->getStorage('taxonomy_term')->loadTree($vocabulary->id());
   

   the same

Is this correct?

yep, thanx

Great work @tadityar -- this is looking good! Two points of feedback:

1. +++ b/core/modules/migrate_drupal/src/Tests/d6/MigrateTaxonomyTermTest.php
   @@ -99,7 +99,7 @@ public function testTaxonomyTerms() {
   +        foreach (\Drupal::entityManager()->getStorage('taxonomy_term')->loadParents($tid) as $parent) {
   
   +++ b/core/modules/node/src/Tests/NodeAccessPagerTest.php
   @@ -78,7 +78,7 @@ public function testForumPager() {
   +    $tree = \Drupal::entityManager()->getStorage('taxonomy_term')->loadTree($vid, 0, 1);
   
   +++ b/core/modules/taxonomy/src/Tests/TermTest.php
   @@ -232,7 +237,7 @@ function testNodeTermCreationAndDeletion() {
   +    $tree = $this->container->get('entity.manager')->getStorage('taxonomy_term')->loadTree($this->vocabulary->id());
   

   Any particular reason we're getting the entity manager from the available $this->container in some simpletests, but using the static method wrapper for the service in others? Not a big deal either way.

2. +++ b/core/modules/taxonomy/src/TermForm.php
   @@ -34,13 +35,14 @@ public function form(array $form, FormStateInterface $form_state) {
   +    // \Drupal\taxonomy\TermStorageInterface::loadTree() and
   +    // \Drupal\taxonomy\TermStorageInterface::loadParents() may contain large
   
   +++ b/core/modules/taxonomy/src/Tests/TermTest.php
   @@ -438,8 +445,9 @@ function testTermMultipleParentsInterface() {
   +    // not added by
   +    // $this->container->get('entity.manager')->getStorage('taxonomy_term')->loadParents().
   

   In code comments, we should reference the default implementation of the service (class and method name), like the first hunk I've highlighted here does. So let's change the second hunk to do so as well.

Closed #2405365: Remove usage of taxonomy_term_load_parents() as a duplicate of this issue per #14. Thanks @Mile23.

Speaking to #28.1, there is this issue which should provide some clarity, but doesn't: #2087751: [policy, no patch] Stop using $this->container in web tests

Aha, that's what I was thinking of. Thanks @Mile23.

Thank you for the reviews :)

Should this issue get postponed until something has been decided in #2087751: [policy, no patch] Stop using $this->container in web tests then? Or should I just revise the comments and leave the code as is?

@tadityar, I think it's fine to leave them as you have in the patch, even though it's not quite consistent. If the policy is decided one way or another, there will be a whole lot to clean up. So I think it's just the second point, to reference the class in the docs. Thanks!

Created straight reroll.

Patch attached.

Addressed #28.2 as suggested by xjm. Patch attached.

Looks good to me

My IDE couldn't find usages of taxonomy_term_load_parents(), taxonomy_term_load_children(), or taxonomy_get_tree() after applying the patch.

Thanks all! I confirmed that all three of these functions are deprecated for removal in 8.0.0 and that #2505851: Remove deprecated function taxonomy_* already has an attached change record for them. I also confirmed that no usages or references remain:

[mandelbrot:maintainer | Sun 17:58:47] $ grep -r "taxonomy_term_load_parents" *
core/modules/taxonomy/taxonomy.module:function taxonomy_term_load_parents($tid) {
[mandelbrot:maintainer | Sun 17:59:50] $ grep -r "taxonomy_term_load_children" * 
core/modules/taxonomy/taxonomy.module:function taxonomy_term_load_children($tid) {
[mandelbrot:maintainer | Sun 17:59:56] $ grep -r "taxonomy_get_tree" *
core/modules/taxonomy/taxonomy.module:function taxonomy_get_tree($vid, $parent = 0, $max_depth = NULL, $load_entities = FALSE) {

This issue is a prioritized change as per https://www.drupal.org/core/beta-changes and its benefits outweigh any disruption. Committed and pushed to 8.0.x.