taxonomy_term_load_parents_all() doesn't work correctly with multiple hierarchy terms

Very straightforward. Patch is attached below.

Example vocabulary was created according to the original post. This command was executed pre- and post-patch:

var_dump(taxonomy_get_parents_all(14));

Pre-patch:

array(3) {
  [0]=>
  object(stdClass)#28 (5) {
    ["tid"]=>
    string(2) "14"
    ["vid"]=>
    string(1) "3"
    ["name"]=>
    string(5) "Event"
    ["description"]=>
    string(0) ""
    ["weight"]=>
    string(1) "0"
  }
  [1]=>
  object(stdClass)#27 (5) {
    ["tid"]=>
    string(2) "11"
    ["vid"]=>
    string(1) "3"
    ["name"]=>
    string(8) "Politics"
    ["description"]=>
    string(0) ""
    ["weight"]=>
    string(1) "0"
  }
  [2]=>
  object(stdClass)#26 (5) {
    ["tid"]=>
    string(2) "13"
    ["vid"]=>
    string(1) "3"
    ["name"]=>
    string(6) "Tennis"
    ["description"]=>
    string(0) ""
    ["weight"]=>
    string(1) "0"
  }
}

Post-patch:

array(4) {
  [0]=>
  object(stdClass)#23 (5) {
    ["tid"]=>
    string(2) "14"
    ["vid"]=>
    string(1) "3"
    ["name"]=>
    string(5) "Event"
    ["description"]=>
    string(0) ""
    ["weight"]=>
    string(1) "0"
  }
  [1]=>
  object(stdClass)#24 (5) {
    ["tid"]=>
    string(2) "11"
    ["vid"]=>
    string(1) "3"
    ["name"]=>
    string(8) "Politics"
    ["description"]=>
    string(0) ""
    ["weight"]=>
    string(1) "0"
  }
  [2]=>
  object(stdClass)#25 (5) {
    ["tid"]=>
    string(2) "13"
    ["vid"]=>
    string(1) "3"
    ["name"]=>
    string(6) "Tennis"
    ["description"]=>
    string(0) ""
    ["weight"]=>
    string(1) "0"
  }
  [3]=>
  object(stdClass)#26 (5) {
    ["tid"]=>
    string(2) "12"
    ["vid"]=>
    string(1) "3"
    ["name"]=>
    string(6) "Sports"
    ["description"]=>
    string(0) ""
    ["weight"]=>
    string(1) "0"
  }
}

As you said in the other issue, taxonomy breadcrumbs need to work around the multiple hierarchies. This patch doesn't fix that, it only deals with the bug above.

Subscribing. This would need to be backported to Drupal 5 as well.

Subscribing. This bug appeared to me when using the hierarchical_select module.

I have a new copy of 7.x-dev. I used the patch. It worked good. Example vocabulary was created according to the original post. I saw what Arancaytar saw.

SimpleTest taxonomy modul:
62 passes, 0 fails, 0 exceptions

pp

This still applies and looks good, but IMO we need a test for it.

Rolled in an E_ALL fix from #351669: taxonomy_get_parents_all() is not E_ALL compliant which also needs backporting to Drupal 6. Also added a test which covers both bugs..

Here it is without an infinite loop, but in this case the (very basic) test fails.
I'm more interested in getting #344019: New implementation for database tree parsing ( taxonomy / comment ) in than trying to fix these recursive queries.

Split the tests off to #352479: Tests for taxonomy_get_parents_all()

Also demoting from critical.

.

Hi,

This patch is not working correct. I need it to get breadcrumbs correct for weblinks on D6: http://drupal.org/node/735874#comment-2690350

I tried the latest patch and got WSOD.
I added the D7 functions taxonomy_term_load and taxonomy_term_load_multiple, but still WSOD.

Edit: Will implement this module: http://drupal.org/project/taxidermy help?

Thanks a lot in advance for working on this!
greetings,
Martijn

+++ modules/taxonomy/taxonomy.module	29 Dec 2008 13:33:51 -0000
@@ -767,11 +767,13 @@ function taxonomy_get_parents($tid, $key
-  if ($tid) {
-    $parents[] = taxonomy_term_load($tid);
+  if ($term = taxonomy_term_load($tid)) {
+    $parents[] = $term;

This part got committed in #346156: term delete fails because of misnamed function, so this function might actually be working now. Lets try out the tests from #352479: Tests for taxonomy_get_parents_all().

Rolled a new patch with a different way of searching. Also included the test as it is the only way of telling whether the function actually works correct.

Last patch wasn't correct, changed some details.

I confirmed that the bug is still present in 8.x HEAD, and that this patch solves the issue.

The code looks sound to me, just two small remarks:

• A re-roll is needed, since the most recent patch no longer applies cleanly to HEAD
• No space between ! and isset().
  

  +++ b/core/modules/taxonomy/taxonomy.module
  @@ -565,11 +565,18 @@ function taxonomy_term_load_parents_all($tid) {
  +          if (! isset($parents[$new_parent->id()])) {
  

• Re-rolled to keep up with HEAD
• Removed the space between ! and isset()
• Added an additional test.

24: 217676-24-taxonomy-load-all-ancestors.patch queued for re-testing.

The same test-only patch that fails in #20 is now passing in #24. Since all terms share the same weight, the order in which parents for a given term are returned is defined by the term names (see TermStorageController::loadParents()). Since weight of terms plays a role in whether or the result from taxonomy_term_load_parents_all() is correct, this causes the tests to fail in only half of the test runs.

Attached is a new test-only patch, which now forces weight on $term[1] to trigger the bug, and a new complete patch (fix + tests)

27: 217676-27-taxonomy-load-all-ancestors.patch queued for re-testing.

Seems like this function taxonomy_term_load_parents_all() has been rewritten

function taxonomy_term_load_parents_all($tid) {
  return \Drupal::entityManager()->getStorage('taxonomy_term')->loadAllParents($tid);
}

Pointing everything to core/modules/taxonomy/src/TermStorage.php and public function loadAllParents($tid) {

This will need to be re-rolled with someone understanding TermStorage.php.

proper re-roll, still does not get how to work with "cycled" terms

+++ b/core/modules/taxonomy/src/Tests/TermUnitTest.php
@@ -58,6 +58,12 @@ function testTaxonomyVocabularyTree() {
+    // taxonomy_term_load_parents_all().

@@ -98,5 +104,21 @@ function testTaxonomyVocabularyTree() {
+    $parents = taxonomy_term_load_parents($term[3]->id());
...
+    $ancestors = taxonomy_term_load_parents_all($term[2]->id());
...
+    $ancestors = taxonomy_term_load_parents_all($term[3]->id());

We can update this.

Fixed

interdiff

Thanks

+1 thanks

Test bot fluke.

Committed 99876e6 and pushed to 8.0.x. Thanks!

This is a reroll of #26 for D7.

Any update when this is going to be committed to 7.x?

Re-posting patch from 2 years ago to trigger the bot.