Support for a term with multiple parents

I'm dealing with this problem right now. I gather that the module is built under the premise that you need one menu item for one term in every vocabulary.

Take the _taxonomy_menu_get_mlid function for example.

function _taxonomy_menu_get_mlid($tid, $vid) {
  return db_result(db_query('SELECT mlid FROM {taxonomy_menu} WHERE tid = %d AND vid = %d', $tid, $vid));
}

This function assumes that every term has only one menu link assigned to it, and it looks like it's pretty fundamental to the module. It'll take some rewriting to get the whole thing to work on multiple parents per term.

I'll probably have to take care of this to finish my project anyway so if anyone's interested I`ll post a hacked version of this when I`m done (dont expect it to be too professional or stable though).

Apart from this issue, this is really a wonderful module, especially when combined with NAT.

Thanks a lot!

That is a correct assumption. It's being worked on as part of V3. It's not a complete rewrite (like V1 to V2) but close.

http://drupal.org/node/432048

Webel. My new daughter has taken priority over my VOLUNTEER work. The wonderful part of the Drupal community is how the member's give back. I think this is a great opportunity for you to give back in the form of a patch.

I'm happy to commit the patch as I see it as a very useful feature.

Cheers,
Neil

Im working on it webel, and it's far from easy. Please don't put your faith in what will ultimately end up as baddly hacked code. It might not even work on other people's sites. I'd love it if people were to benefit from my amateur level tinkerings, but I can't guarantee it.

Well, if anyone's desperate enough , here's three files you need to replace in the taxonomy module. I haven't tested it properly. I dont even want to know what happens if you mark down a term as its own child.

I had to change the way the path was being created because obviously each term has to have a different path for each of its menu item links if you want the right menu item to be marked as active. I had to add a function or two as well for term\menu parent searching purposes.

Nevermind. I wont go into too many details. If indytechcook or webel or anyone else for the matter is interested in how this works (or is supposed to work anyway) I'd be happy to discuss the changes I've made. Like I said before, this is just the minimum I need to make my site work so dont expect uh .... anything at all.

here, this might fix it

Right.
I decided to stop tiptoeing around this issue and just rewrite entire sections. So I axed a couple of them and removed about 200 lines of code which I found redundant. I added a few database query functions and modified one of the existing ones. Seems to be working ok now....

It gives out this 'duplicate-entry" warning message whenever I'm adding new rows to the taxonomy_menu table for reasons I dont understand at the moment but as far as I can see it doesnt damage anything.

give it a spin. this time I packed the whole module so you have to overwrite everything.

@webel, try disable, uninstalling (admin/build/modules/uninstall) then reenable module. This will whip out all prior existence of the module (menu items and such)

@whodies I'll be the first person to tell you that the code wasn't perfect. Optimization was on my list. Being that you have put a ton of hard work on this, would you like to be part of the taxonomy menu team? When I commited your changes, of course I would give you credit, but this way you could also have the commit.

I've taken a quick look and the only comment I have is that some of the code you took out because it looked unnecessary was put in there for a reason.

For example, I had taxonomy_menu_taxonomy that same way at one time using and ran into issue because taxonomy_get_term pulls from a cache. So when you update the menu item, it will pulls the previous data.

Otherwise, pretty good work. I'll like this features and I'll work it into a release.

@Webel
This is probably the last version I'm posting. I did my best to take care of the bugs I could reproduce. Unless I run into any of these problems in the future, its out of my hands.

@Indytechcook
I added a few small modifications and documented the new functions (I think the documentation for some of the existing functions I've changed is no longer valid).

About your example concerning the taxonomy hook, take a closer look and you'll see that taxonomy_get_term is being used only if the $op is 'delete' , otherwise its just the array given with hook converted into an object.

This is still a relatively half assed attempt on my part so I'm sure I screwed up a few things. I can't see any of them at the moment besides that duplicate-entry error. I've done very little testing though , so who knows ?

As for your offer, I am both flattered and compeletly unwilling to commit to it. I'm in enough trouble with my boss as it is for having taken the time to tackle this issue.

Again, thank you very much for developing this module in the first place, I appreciate it.

@whodies
your help will always be accepted :) Cheers

@webel , im sorry the rebuild didnt work out for you. It seems to work well enough for me, since the last post I didnt have to change anything. I suggest you look at least at the original module before you rebuild it completly, that's what I wanted to do at first but after some inspection I realised that the module doesnt need a complete reboot, most of the major changes were in the handler function, and rest was just comsetics. I hope you work it out one way or another.

@webel: Yes, there is an uninstall for this module. It doesn't show up until you deselect "Taxonomy Menu" in the module list and apply the changes.

@whodies: Thank you so much, you saved my life! You fix works perfectly.

Here is an error I am getting when creating new nodes. I do not think it actually creates any problems (i.e., I am able to create the node regardless), but I decided to post it any way.

    * user warning: Duplicate entry '1076-96' for key 1 query: INSERT INTO latin_taxonomy_menu (mlid, tid, vid) VALUES (1076, 96, 7) in \sites\all\modules\taxonomy_menu\taxonomy_menu.database.inc on line 22.
    * user warning: Duplicate entry '1073-83' for key 1 query: INSERT INTO latin_taxonomy_menu (mlid, tid, vid) VALUES (1073, 83, 7) in \sites\all\modules\taxonomy_menu\taxonomy_menu.database.inc on line 22.
    * user warning: Duplicate entry '1042-50' for key 1 query: INSERT INTO latin_taxonomy_menu (mlid, tid, vid) VALUES (1042, 50, 5) in \sites\all\modules\taxonomy_menu\taxonomy_menu.database.inc on line 22.
    * user warning: Duplicate entry '1032-32' for key 1 query: INSERT INTO latin_taxonomy_menu (mlid, tid, vid) VALUES (1032, 32, 5) in \sites\all\modules\taxonomy_menu\taxonomy_menu.database.inc on line 22.
    * user warning: Duplicate entry '1017-13' for key 1 query: INSERT INTO latin_taxonomy_menu (mlid, tid, vid) VALUES (1017, 13, 5) in \sites\all\modules\taxonomy_menu\taxonomy_menu.database.inc on line 22.
    * user warning: Duplicate entry '1057-106' for key 1 query: INSERT INTO latin_taxonomy_menu (mlid, tid, vid) VALUES (1057, 106, 7) in \sites\all\modules\taxonomy_menu\taxonomy_menu.database.inc on line 22.

I've already mentioned this issue a few comments back. Indeed, it seems to be pretty harmless, but I'd be happy to get rid of it if I only knew why it was happening.

@indytechcook, this changes been applied in 2.4-beta1 version?

No it did not. There were to many changes to be in an incremental release. I have used part for the next version.

delete my - error post - sorry!

After futhur review, this will not be included in the V2 release. There are a large amount of changes that would need to occur to get the node count updates working correctly. This will be part of the V3 release.

Moving to V3.

Hi I looked at taxonomy_menu_4.zip and edited taxonomy_menu.module
from

  //save the menu item
  
  if ($mlid = menu_link_save($item)) {
    //if inserting a new menu item then insert a record into the table
    
    if (!is_null($item['mlid'])) {
      _taxonomy_menu_insert_menu_item($mlid, $item['tid'], $item['vid']);
    }
    return $mlid;
  }

to

  //save the menu item
  
  if ($mlid = menu_link_save($item)) {
    //if inserting a new menu item then insert a record into the table
    $existing_item = FALSE;
    if (isset($item['mlid'])) {
      $existing_item = db_fetch_array(db_query("SELECT * FROM {taxonomy_menu} WHERE mlid = %d", $item['mlid']));
     
      if (!isset($existing_item['mlid'])) {
        _taxonomy_menu_insert_menu_item($mlid, $item['tid'], $item['vid']);
      }
    }
    return $mlid;
  }

after this change there are no warnings.

Marinex

i will set this patch on my working site - if i take bug - will send comment in this issue

Subscribing, greetings, Martijn

Move back to 2. Need some one to review and set to RTBC

Someone please create a patch against DRUPAL-6--2 branch so this can be reviewed and committed.

Subscribe

I really need this feature and would be ready to test on this
http://www.controlchainmanagement.org/node/993

but I cannot figure out what to do from this thread.

Install
http://drupal.org/files/issues/taxonomy_menu_3.zip
or http://drupal.org/files/issues/taxonomy_menu_4.zip + Modify according #43
or go 6.x-3.x-dev

???

@jvieille,

It looks like http://drupal.org/files/issues/taxonomy_menu_4.zip + Modify according #43 might be the best bet.

6.x-3.x is a dead end right now.

We still need a patch against the DRUPAL--6-2 branch.

Fantastic!
It worked on the spot (just needed to rebuilding the menu)

Many thanks for this great improvement - hoping it could be committed soon

path from http://drupal.org/node/498182#comment-1772248 working corrctly on my site with multyparent taxonomy.

Is this committed in the last dev?

Multi-parent terms are a standard core taxonomy feature, it really has to be supported in Taxonomy menu.

Thanks again for this great module

This is still a great change. I took a deep look at the code and like the changes. I think it might actually make the code more efficient overall.

Can't RTBC this for 2 reason.
1. It's not a patch. It's a MAJOR change and needs a patch so others can test.
2. @webel is still having issue

good to find this thread :)
is it already commited to dev version ? I tried it but still have a problem with that.

SUBSCRIBE

Here is an initial patch against the DRUPAL-6--2 branch. This is a MAJOR change and needs very heavy testing before it's committed.

My initial tests turned up one bug so far: When a new term is added that is a child term of a term with multiple parents, it isn't added correctly. I'm not sure if this is a bug in the menu system or how it's being saved. I just really wanted to get some code up here. Screenshot: http://skitch.com/indytechcook/dwn68/1.1-taxmenu.local The term 2.111 has parents 2.0 and 1.0. Term 2.112 has a parent of 2.111.

I decided to wait until patch will be commited and by now uninstalled this module and installed again the version from here - http://drupal.org/node/498182#comment-1772248 - works well. I recommend to use it until official version of taxonomy menu will render menu links correct also.
THX

Hi!

I've been working a little with this issue and I've created a patch (against 6.x-2.x-dev 2010-Aug-26) that "seems" to work with multiple parents.

Please, check it (but not on production sites).

Attached patches, the whole module tar file and screenshot of taxonomy with multiple parents.

Regards.

Whoa, marking this needs review. Thanks for the patches, I'll check this out in a bit.

I need this feature yesterday... From reading through this thread I think the final version that webel put out along with a mod to fix db errors still had issues? Can anyone point me to a module I can use until this feature is added, or can you give me a patch for now? This isn't included in current dev is it? Help!

Thanks for your frantic post. My limited focus is on the D7 version of taxonomy menu, as 7 is soon to be released.

Drupal is a community of volunteers, so rather then telling me how badly you need this, it would be helpful if you tested out the patches in #64 and reported back here. http://drupal.org/patch/apply

I'm looking forward to hearing the results of your testing.

Well, sorry for the "frantic" post but I was looking at posts from end of last year saying that this would be put into 6, and now into 7. We can't move to 7 for a good long time. Anyway, I know how to apply patches... this thread still had someone saying there were issues with what people were coming up here, so I was hoping someone had a final confirmed mod for this, or a recommendation on a module I could use in the meantime. The final upload someone put on this thread was to patch a dev version. Patching a stable version is worrisome enough for a production site. Anyway, thanks. At least I know nothing official is going to be done for 6.

I didn't say anything about this not going into 6. I'm all for it to go into 6.x. The way you can speed this process up is to help out.

I'm sorry that your client or whomever needs this feature ASAP but your client isn't paying me to work on their site. They are paying you, presumably.

I've got a pretty small group of people who DO contribute to this project. miguel_angel is one of them and the patches in #64 need review. Please, don't tell me how badly you need this feature and then make excuses when I ask you to spend a little time improving it.

This issue sounds very interesting, subscribing...

Cheers

Hi,

I've tried the module as posted in #64, cleaned caches and ran updates but it is not working for me.

In my situation I have a taxonomy as followed;

- A1-level-1
-- A1.1-level-2
--- A1.1.1-level-3
--- A1.1.2-level-3
-- A1.2-level-2
--- A1.2.1-level-3
--- A1.2.2-level-3
-- A1.3-level-2
--- A1.3.1-level-3
--- A1.3.2-level-3
- B1-level-1
-- A1.1-level-2
--- A1.1.1-level-3
--- A1.1.2-level-3
- C1-level-1
-- A1.1-level-2
--- A1.1.1-level-3
--- A1.1.2-level-3
- D1-level-1
-- A1.1-level-2
--- A1.1.1-level-3
--- A1.1.2-level-3

As you can see I've set the parents of A1.1-level-2 to be multiple and attached it to every top level term. The outcome is completely fubar and I can't really figure out any logic behind it. Both 2nd as 3rd level terms have become 1st level menu items in some case but not all. These are not always terms with multiple parents.

Even after removing the multiple parents and making it a normal hierarchical taxonomy everything is messed up. Even after using the 'Select to rebuild the menu on submit' option. Reverting the module to it's orginal state and rebuilding the menu again fixes it. But ofcouse only without multiple parent terms.

Have I forgotten something? Am not using any of the settings in the taxonomy menu fieldset other than assigning it to a menu.

Cheers

Hi again,

I've also tried the patch in #62. Had to patch by hand cause it wouldn't work against latest release for some reason.

After changing the db call on line #61 and added {} around table names it kinda works.

db_query('DELETE {taxonomy_menu}.* , {menu_links}.* FROM {taxonomy_menu} LEFT JOIN {menu_links} ON {taxonomy_menu}.mlid={menu_links}.mlid WHERE {taxonomy_menu}.vid = %d', $vid);

What it does is it does show 2nd level terms with multiple parents beneath the various parents. Yet the 3rd level terms are duplicated in their original position.

- A1-level-1
-- A1.1-level-2
--- A1.1.1-level-3
--- A1.1.1-level-3
--- A1.1.2-level-3
--- A1.1.2-level-3
- B1-level-1
-- A1.1-level-2
- C1-level-1
- D1-level-1

Can anyone confirm this?

Cheers,
Bartezz

Am currently testing the solution from #64 and seems to work OK for the site I am working on. I will try and duplicate the errors reported by others on this thread and report back...

@Bartezz I have tested your taxonomy hierarchy against #64 and found no problem... until I rebuilt the menu.

In general the solution from #64 works as expected, however rebuilding the menu is problematic. Rebuilding the menu works fine for level 1 child terms, and fails completely for any child terms level 2 or deeper. The typical outcome of a menu rebuild in this case is the level 2 or deeper child terms are added to the top level of the menu.

Having absolutely no knowledge of what is happening at code level, my initial suspicion is that the menu rebuild deletes all the menu items and starts by adding the menu items for child terms first, before the menu items for the parent terms even exist...

I have found further problems when deleting terms from the taxonomy that have child terms, some menu items that should be deleted are not removed. I say some because the behaviour is pretty haphazard and I can't establish any consistent pattern for this behaviour.

However, I believe once the rebuild function is working correctly this issue could be resolved by rebuilding the menu after deleting terms.

One more issue, the menu structure is not updated to reflect any change in the parent term settings for a taxonomy term. This is for all terms that have parent terms, irrespective of the number of parents. Currently this is fixable by rebuilding the menu (but only for level 1 child terms as I mentioned in #75).

Some body know any alternatives of this module for making multi parents hierarchical menu from vocabulary?

Subscribing :)

The attached patch should fix the problem where menu links are left behind when deleting the generated menu for a multi-parent taxonomy. (whether from setting the Menu Location to Disabled, or when deleting it for a rebuild). The problem is that the unpatched _taxonomy_menu_delete_all deletes one menu link per tid, and for multi-parent situations, there are more than one per tid.

Sorry about this and the last comment. The patch I posted (next comment) didn't include the patch itself, and in trying fix it, I managed to post 2 extra messages. I can't delete comments, so editing it down seemed appropriate.

This is the patch we're using to get the menu rebuild for a multi-parent taxonomy menu to work. The technique I'm using will only work for a rebuild - after adding or changing terms, you'll have to do a rebuild. At this point, we consider this usable.

This code depends on the fact that the rebuild uses the output of taxonomy_get_tree, which puts items out in the order that you want to build the menu, with depth set appropriately. At any point, the parent to an item is the previous item that has a depth one less than the current item. The key to having the menu build correctly is to get the plid (the parent's mlid) set correctly when passing a link to menu_link_save(this happens in _taxonomy_menu_save). To do this I've added a depth field to the taxonomy_menu table, and pass the depth from the term into it. During a rebuild, you can find the appropriate plid for an item by getting the mlid of the last taxonomy_menu record in the current vocabulary that has a depth one less than the depth of the current item.

This alternate code for setting the plid is only used during a rebuild (by setting the drupal variable taxonomy_menu_rebuild_menu to 1 during the rebuild) if the vocabulary's hierarchy field is set to 2 (multi-hierarchy). The latter condition requires that you also have the patch from #1197198: Editing Vocabulary changes a Multiple Hierarchy Term list to be marked as a Flat list applied, as otherwise the hierarchy field gets reset to 0(flat), until they get that into core. You'll also want to have the patch in #80 installed, so that all of the items in the menu will be cleared out before the rebuild.

Installing this patch requires running a database update (http://<sitename>/update.php or 'drush updb').

I haven't yet looked at the patch in #64, so I'm not yet sure how this would fit in with what's done there. I'm not even sure if this is what is needed for a permanent fix, but I thought that it would be helpful for those trying to get something useful going until the permanent fix is completed.

The patch link in #83 is not complete. Anyway I got the patch to download by completing the url manually.

Only now did I realize that this patch is for version 6. I am having same problems with the D7 version of the module.

This is a re-upload of the patch in 83. I don't know why the link was bad.

I may look at an equivalent patch for D7 when I have time. We are at the moment pretty much still in D6.

Given the popularity of this module, it's pretty amazing that no one has addressed this issue in the span of over 2 years. I'm obviously overlooking some complexities inherent to this module, but to me this seems pretty straight forward. Of course, my use case only involves rebuilding the menu in its entirety - not each time a term is modified.

Given the lack of support for multiple parents in this module, and my reluctance to dive into the innards of another module, I simply wrote a custom function to handle this for me. I'm posting it here in case it is of any use to anyone:

function catalog_menu_rebuild() {
  $menuArr = array();
  $vocabID = 23;
  $topMLID = 17345;
  
  // Clear out the existing menu entries
  db_query("delete from {menu_links} where menu_name like 'primary-links' and module like 'catalog'");
  
  foreach(taxonomy_get_tree($vocabID) as $term) {
    // Create a clean url friendly version of the term
    $termName = preg_replace("/[^a-zA-Z0-9]/", '-', $term->name);

    // Add menu entry for each of term's parents
    foreach($term->parents as $parentTID) {
      $menuItem = array();
      
      // Determine where (or if) to link the new menu item.  If we find a
      // parent term that we haven't encountered yet, we skip it as it is most
      // likely further down the tree.
      if (empty($parentTID)) {
        $menuItem['plid'] = $topMLID;
      } else if (isset($menuArr[$parentTID])) {
        $menuItem['plid'] = $menuArr[$parentTID];
      } else {
        continue;
      }
            
      // Populate the remainder of the menu item
      $menuItem['module']      = 'catalog';
      $menuItem['menu_name']   = 'primary-links';
      $menuItem['weight']      = $term->weight;
      $menuItem['link_title']  = $term->name;
      $menuItem['link_path']   = "network-hardware/search/$termName";
      $menuItem['router_path'] = "network-hardware/search/$termName";
      $menuItem['expanded']    = 1;
      
      // Save the new menu item and store the new mlid
      $menuArr[$term->tid] = menu_link_save($menuItem);
    }
  }  
}

subscribing

Also need this.
subscribing.

Going to give #87 a look. thanks.

Link to duplicate issue. http://drupal.org/node/468174

So it seems to me that until this feature is added to taxonomy menu module the best work around (at least for my scenario) is to use taxonomy menu module to build a menu containing all your terms from a vocab, then use Menu Clone module to make a clone of it, and modify the menu accordingly. From there use Nice menu module to create menu blocks from your menus. You can also use OM Maximenu module for more menu block options...
You'll have to re-clone your menus to add new terms, and then update the menu source in the blocks provided by Nice Menus.
Hope this is helpful for someone.

Thanks. Does it also work with D7?

I would suggest looking at this module

http://drupal.org/project/taxonomyblocks

No more trouble with everything : based on Views (dynamic), no Menu (complex reconstruction / resync needed)

Same problem:

Advanced Taxonomy Blocks is designed to work with single hierarchy taxonomies, which is to say that if terms have more than one parent I don't know how it will display (I have never tried it).

ATB works perfectly with multiparental terms.

Thanks but it is only for D6. :-(

Moving to 7.x-2.x, won't happen in 6.x. Closing other duplicates.

Here's a patch for the latest D7 version.

All the tests pass, but I haven't written any tests for the multi parent features yet.

Please do some testing, tell me how to break it, and I'll fix any issues.

#98: taxonomy_menu-multiple_parents-498182-98.patch queued for re-testing.

Oh, great. I was afraid of this. Re-rolling this is going to be so much fun...

Thank you for your work firebird, I know the dev version is changing quickly and there will obviously be re-rolls of any patches, and we are sorry for that. Maybe you could take a look at http://drupal.org/node/711070#merging, it could help you for re-rolling the patches. Anyway, this is great that all these old and long-awaited feature issues are eventually on their way to be solved, thanks again !

Ok, here we go. The merging actually worked quite well, thanks for the tip.

This should pass all the tests, and it also passes a couple easy tests I tested manually, namely creating menus from taxonomy trees with the following structures:

A
-C
B
-C

and

C
A
-C
B

Still needs tests for the multi-parent cases.

Just to let you know so you don't feel like waiting, I won't be able to really check this for something like a week. But hopefully someone else will !

The code looks good. I still need to look it over functionally.

I suggest installing PHP_CodeSniffer and the coder module and run your code through it once and fix up the things it suggests.

Few things noted below.

+++ b/taxonomy_menu.batch.incundefined
+++ b/taxonomy_menu.batch.incundefined
@@ -54,7 +54,7 @@ function _taxonomy_menu_save_menu_links_process($terms, $menu_name, &$context) {

+++ b/taxonomy_menu.installundefined
@@ -119,3 +119,11 @@ function taxonomy_menu_update_7200() {
+  db_drop_primary_key('taxonomy_menu');

Maybe not a PK but this table should probably have a key. Also, if you're dropping the PK you need to drop it in the hook_schema() as well.

+++ b/taxonomy_menu.moduleundefined
@@ -148,24 +150,68 @@ function taxonomy_menu_menu_link_prepare($term, $menu_name) {
+      $mlid = $mlids[$index];      ¶
+      $menu_link = menu_link_load($mlid);
+
+      if ($parent_mlids) {
+        // TODO this might cause an bug as we're ignoring all other possible mlids.
+        $parent_mlid = reset($parent_mlids);
+        $menu_link['plid'] = $parent_mlid;      ¶
+      }

trailing white space

+++ b/taxonomy_menu.moduleundefined
@@ -224,20 +280,7 @@ function taxonomy_menu_menu_link_prepare($term, $menu_name) {
+     ¶
   return $menu_link;

white space

I've commited this 429b67d, which makes the prepare function much more readable. I suppose this should help with this patch but you'll have to merge again, sorry about that...

I think that commit introduces a bug.

This line in taxonomy_menu_menu_link_prepare():

$menu_link['hidden'] = ($hide_term && isset($nodes_count) && $nodes_count > 0) ? 0 : 1;

..causes all links to be hidden if the hide_term option isn't set.

oh, indeed... Tracking the issue in #1895882: All links are hidden if the hide_term option isn't set

Introduced bug is fixed in latest dev.

Cool, thanks. Just a short progress report:

Done:
- I've got the basic functionality working the way I want it
- I've added some multi-parent terms in the test-vocabulary
- The code is passing all the tests in "Configuration options" and "Vocabulary interface"
TODO:
- There are some fails left in "CRUD functions"
- Merge to latest dev.

I might get this done tomorrow, and if not that, then early next week.

Awesome, thank you ! Also, in the meantime, I've commited long-awaited custom paths feature.

I did check the code with phpcs, but there were so many errors in the original code that I didn't feel like picking my own from between those. And it's not really good practice to combine cleanup commits with code changes, so I figured I'd just leave it for now.

I have now fixed at least most of my whitespace errors, and I wouldn't mind doing a whole separate commit to fix the coding standard problems. I'd just like to get the actual code committed before I need to re-roll it again...

Changes to this patch:

- Again, all tests should pass
- I've removed the PK from schema_install
- I've also added a replacement key for the removed PK. The replacement index is added in the update hook
- The tests have been modified to take into account the terms with multiple parents. One of the tests I had to comment out, since the drag-and-drop re-ordering of menu items doesn't work in vocabularies with terms with multiple parents.
- The custom path tests also work.

Forgot to include the latest changes in the patch.

I'll just give some thoughts as I haven't really review/tested this, just looked at the code. Overall code looks good, but there are 2 things that I don't feel very comfortable with:
- The code duplicate in "prepare" function
- All the loops that were added

I know this is probably needed because of the way the module works atm, but the same question always pops up in my head: should we prepare all the taxonomy terms first and then batch save their respective menu links instead of doing the prepare/save process one term at a time like it is atm... I think this would simplify the whole process and remove at least a few loops here.

Yes, I actually already optimized that duplicate code away once, but then I had to re-write that part after the function changed. I'll do the same re-structuring again.

Regarding the loops, I'm afraid there's little that can be done, apart re-writing a lot of the module. It's just that when we're dealing with multiple terms, we just need to go through all the parent terms and their menu links.

I'm sure there are ways to optimize the code, but at this time I'm not prepared to spend a whole lot more time on this. I need this feature for a customer project, and there's plenty of other work to be done in addition to this.

I'll re-roll the patch with the duplicate code removed in the near future.

Okay let's try to make this as clean as possible and try to optimise the duplicate code a little bit more. I'll check the tests and if all goes smoothly, I think we can commit this. It would be great that a few people review it also before commiting. Thanks for your hard work firebird !

Ok, here's another version with the duplicate code moved to a helper function.

Very nice ! I'm really sorry but I can't test/commit anything because I'm moving house atm...

Anyone in this issue queue up to help testing and reviewing this patch ? That would be great.

Here's a new re-roll that applies to the current dev 2.x.

This is my last re-roll, as I can't justify spending more time and money on this. I'm just going to take the current dev version patched with the latest patch into production use and hope for the best.

Hopefully the patch gets committed at some point.

Don't worry firebird, you did some great work and it will be committed. But we need at least someone to confirm that it works flawlessly because this is a major feature. And I'll re-roll the patch for you if needed.
As you will use this in production, maybe you can give us feedback in the next weeks or so to confirm that you did not find any problems.

...soooooo, I suppose those of us who are stuck with PHP 5.2 are boned, eh?

subbing.

Why would that be dooley ?

Marked #1196898: 2 Parents Lead to Stray Menu Items as a duplicate:

If a term has 2 parents ... ,then one instance of the term menu is created as a stray item at the root level of the menu. Disabling the taxonomy menu will not remove the stray item and it has to be removed manually from the menu_links table

#122: taxonomy_menu-multiple_parent_support-498182-122.patch queued for re-testing.

#85: fix_multi_parent_rebuild-498182-81.patch queued for re-testing.

Deleted my last comment

Fairly quickly rolled patch (incorporating much of firebird's work (thanks btw!)) to address a few issues around the menu generation:
- Children (and grandchildren) of poly terms are generally created with only one ancestry. So in the following (overly simplified) example:
---- Products (tid: 1)
-------- Internet (tid: 3)
-------- Computers (tid: 4)
---- Services (tid: 2)
-------- Internet (tid: 3)
-------- Computers (tid: 4)

A menu link would be created for Internet, but it might only have a plid (& p1/2/3 depending on depth) matching tid1. Thus navigating in through Services would result in the link not showing.

This patch runs after the menu rebuild batch script to then generate missing menu links.
It was quickly hammered together, thus has the following limitations:
- As it stands, it will only traverse down to the grandchildren of a term with multiple parents (no further), but could easily be adapted if it's of any use to anyone.
- Only tested/applied against 2.0-alpha2.

Please let me know if it's of any use to anyone so I can put a tidier version together.

The patch in #131 works for me with 7.x-2.0-alpha2, (I had to manually empty the menu though before regenerating)

Having issue with this. Patch #131 works to a degree.

If a term has multiple parents and on of those is also a poly not all the link get created e.g.

- Science (tid: 1)
-- Publications (tid:2)
--- Earth (tid: 3)
-- Books (tid: 4)
--- Sea (tid: 5)
--- Earth (tid: 3)
- Photography (tid: 6)
-- Books (tid: 4)
--- Sea (tid: 5)
--- Earth (tid: 3)

In this case Earth is a ploy under books and publications, and books is a poly under science and photography. In this case looking at the code it loops though twice for books as it only has two parents, this means the link will appear under books in both science and photography, but the link under publications in not created. I narrowed it down to the fact the code only sees two parents so only loops twice (around line 155 in taxonomy_menu.module), but as seen three links are required so mean that one is missed.

If tried a few things but not getting any where with this, I can get the sub menus to create properly but this then breaks the root as it adds sub categories that then appear on the root menu.

Any suggestions or pointers to help with this?

Thanks

7.x-2.0-alpha2
#131 works for me too

#131 works for me too with 7.x-2.0-alpha2

But I have a notice when the menu is refreshing :

Notice : Undefined variable: poly_terms dans _taxonomy_menu_save_menu_links_finished() (ligne 103 dans /www/sites/all/modules/taxonomy_menu/taxonomy_menu.batch.inc).
Warning : Invalid argument supplied for foreach() dans _taxonomy_menu_save_menu_links_finished() (ligne 103 dans /sites/all/modules/taxonomy_menu/taxonomy_menu.batch.inc).

But it works, the menu with multiple parents terms is generated correctly.

This still seems to be an issue with the Drupal 8 version: if you make a taxonomy menu block, terms with multiple parents only show in the tree under their first parent. (Please correct me if I am wrong!)

Has anyone made a version of the patch for the D8 version of Taxonomy Menu?

Hi, I do have the same problem that #137. (Drupal 8). Any fix or new thread to be followed? This one was 11 month old? Cheers and thanks for the great work!

Patch for multiple parents support.

Also a little tweaking: delete all taxonomy menu links in single db transaction instead of useless looping.