This module allows you to make blocks out of taxonomy vocabularies. It works similar like the Menu block module, using Drupal's core block functionalities and building on that. The output of a Taxonomy menu block is a (nested) unordered list of term names, linked to that term's detail page(taxonomy/term/tid).

This is handy if your taxonomy tree is the backbone structure of your site, for example in a webshop where you have a taxonomy of categories with your product displays linked to those categories.

The difference between this module and the already existing Taxonomy menu:

  • This module does not make use of menus: no need to regenerate your menu with each update of your taxonomy
  • Performance: this is the biggest advantage of this module and the reason of its creation. Because of smart caching and a different approach Taxonomy menu block is very fast and performant, something that cannot be said about Taxonomy menu. Think one query for one block versus one query for each taxonomy term.

Functionalities:

  • Ability to alter rendered data through hooks, see .api.php file
  • Active trail is followed, even on nodes attached to a certain term
  • See the readme.txt for more info

Project page: http://drupal.org/sandbox/paulvb/1630054
Git repository: git clone http://git.drupal.org/sandbox/paulvb/1630054.git taxonomy_menu_block

Reviews of other projects
http://drupal.org/node/1946966#comment-7246196
http://drupal.org/node/1891548#comment-7243498
http://drupal.org/node/1896820#comment-7240370

CommentFileSizeAuthor
tmb.jpg69.17 KBAnonymous (not verified)

Comments

PA robot’s picture

PA robot commented
Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://ventral.org/pareview/httpgitdrupalorgsandboxpaulvb1630054git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

Fixed all the warnings but this one: "Missing function doc comment" => is because these functions are offloaded to another file, the function doc comment can be found there.

vlad.pavlovic’s picture

vlad.pavlovic commented

Is the primary difference from Taxonomy menu module that it does not use the menu system, but instead generates a block with a list? Just wondering.

From the automated review, I think that you should still ahve the Function doc included where the validator expects it. If nothing else, have 'Implements hook_block_info().', etc. for each of the function.

Manual Review:
Excellent job with your inline documentation, code is nice and easy to follow.

In taxonomy_menu_block_save_block (taxonomy_menu_block_settings.inc) you serialize the variable before you use variable_set, variable_set will automatically take care of serialization.

Conversely, in taxonomy_menu_block_get_config (taxonomy_menu_block.module), you are forced to unserialize this same variable. Just wondering why you are bothering?

Otherwise from what I can tell, the module code looks good.

Anonymous’s picture

Anonymous (not verified) commented

Hello Vlad,

Thank you very much for the review. I added the function doc comments and removed the serialize, I didn't know variable_get had this function built-in.

To answer your question: the main difference is indeed that my module doesn't make use of menus, allowing it to be way more performant. With a vocabulary of only a handful of terms you will probably never notice the difference, but when you have, for example, a large webshop with a few hundreds of categories Taxonomy menu is just not good enough. It's too slow.
Also, you don't need to regenerate your menu each time you make an adjustment to your taxonomy. Makes it just a little bit easier to sell to a client.

Thanks again for the review.

parwan005’s picture

parwan005 commented
Status: Needs review » Needs work

Can you please explain how this module is different from http://drupal.org/project/taxonomy_menu.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

The difference between this module and Taxonomy Menu is the approach. This module does not make use of Drupal's menu system.

As a result, it avoids some of the pitfalls of standard menus, such as the active trail: if you don't use the Main Menu that is present by default, Drupal will not keep the active trail. TMB does. It even keeps the active trail for nodes attached to a certain term, out of the box. If you want to achieve the same effect with Taxonomy menu you'll need an extra module: Taxonomy menu trails.
Another thing that you can't do out of the box with Drupal's menus is only display the menu up to a certain level, or display dynamic menus (show the items under the currently active item). For this you need the Menu block module. TMB has these options out of the box.

Also, by copying all the terms to a menu you are creating two places where you can adjust your menu: in your vocabulary and in your menu. This is a) difficult to explain to a client you are selling a website to and b) can give weird results. I've had situations where I create a custom menu item in my taxonomy menu, update my vocabulary, and my new menu item doesn't show anymore.

Taxonomy menu offers some great functionality, but it comes at a cost: it is often not reliable and causes a great strain on performance. It's all fine and dandy for small vocabularies/menus but bigger taxonomies cause a heavy load on the system due to the high number of db queries (for example, one menu link needs at least two queries). TMB runs one query per page, once it's been cached.

TMB is way simpler in setup and as a result has great performance and no unexpected results. It also offers some functionalities that aren't present in Taxonomy menu and where you'd normally need extra modules for.

Anonymous’s picture

Anonymous (not verified) commented
Issue tags: +PAreview: review bonus

PAReview: review bonus

sd46’s picture

sd46 commented

Hi, I've installed and had a look through your module and found a couple of minor things.

1) In the .module file on line 15 you do a module_load_include() within a generic hook_form_alter(). You should probably check for the correct forms before loading the include rather than loading it for every form.

2) In the .module file on line 16 you use a return when hook_form_alter() isn't expecting a return value.

3) You should probably include some validation on the add block page to make sure the user doesn't enter a parent level higher than the depth of the tree.

4) In the taxonomy_menu_block_form_options() function you add JavaScript to the form to show and hide form elements, you could optionally use Drupal form #states instead.

sd46’s picture

sd46 commented
Issue summary: View changes

Add reviews of other projects

nickgundry’s picture

nickgundry commented

I did a manual review of the code. This is a really useful module. The code looks good and is well documented. The only thing I saw when I also ran it through the Drupal code standards was this warning. Nice job on the module, very useful

FILE: ...public/sites/all/modules/taxonomy_menu_block/taxonomy_menu_block.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
9 | WARNING | There must be no blank line following an inline comment
--------------------------------------------------------------------------------

arnoldbird’s picture

arnoldbird commented

In your original project application post you should change the git command to...

git clone http://git.drupal.org/sandbox/paulvb/1630054.git taxonomy_menu_block

arnoldbird’s picture

arnoldbird commented
Issue summary: View changes

change branch

Anonymous’s picture

Anonymous (not verified) commented

Thanks for the reviews guys!

@FlakMonkey46
1 & 2 are very good points, fixed those.
3 & 4 are in the pipeline for the coming days as well

@nickgundry
fixed

@arnoldbird
fixed

Thanks again!

arnoldbird’s picture

arnoldbird commented
Status: Needs review » Needs work

@samspinoy

I agree with you that your module does not duplicate taxonomy_menu. It's a different approach, as you say.

Aside from the issues that others have pointed out, I have these thoughts...

Why do you use this file name: taxonomy_menu_block.settings.inc
Many of the things in that file don't seem to be related to settings. Not a major issue.

You have some functions in your module file that call functions in your settings file. In some cases the functions you are calling are quite short, and off-loading the functionality to another file arguably makes your code less readable rather than more readable. Something to ponder.

In _taxonomy_menu_block_form_alter(), you are using submit handlers to clear the cache. What if these entities are being programmatically modified or deleted by other modules? You could end up with unpredictable results. Consider instead clearing your cache by implementing hooks like hook_entity_presave() and hook_entity_delete(). There is still a chance that a module might edit the taxonomy data with db_query, which still won't clear your cache, but I'd say you are better off with the entity hooks than if you assume that data will only be changed when forms are submitted.

Some of what I'm saying is a matter of opinion, but based on FlakMonkey46's comments I will set this to needs work.

arnoldbird’s picture

arnoldbird commented

I am now getting this notice:

Notice: Undefined index: vid in taxonomy_menu_block_build() (line 141 of /var/www/morse/public_html/sites/all/modules/taxonomy_menu_block/taxonomy_menu_block.module).
    Notice: Undefined index: vid in taxonomy_menu_block_build() (line 144 of /var/www/morse/public_html/sites/all/modules/taxonomy_menu_block/taxonomy_menu_block.module).
    Notice: Undefined index: parent in taxonomy_menu_block_build() (line 150 of /var/www/morse/public_html/sites/all/modules/taxonomy_menu_block/taxonomy_menu_block.module).
    Notice: Undefined index: parent in taxonomy_menu_block_build() (line 158 of /var/www/morse/public_html/sites/all/modules/taxonomy_menu_block/taxonomy_menu_block.module).
    Notice: Undefined index: parent in taxonomy_menu_block_build() (line 163 of /var/www/morse/public_html/sites/all/modules/taxonomy_menu_block/taxonomy_menu_block.module).
    Notice: Undefined index: depth in taxonomy_menu_block_build() (line 214 of /var/www/morse/public_html/sites/all/modules/taxonomy_menu_block/taxonomy_menu_block.module).
    Notice: Undefined index: depth in taxonomy_menu_block_build() (line 217 of /var/www/morse/public_html/sites/all/modules/taxonomy_menu_block/taxonomy_menu_block.module).

This problem seemed to begin when I uninstalled the taxonomy_menu module. For a little while I had both your module and the taxonomy_menu module installed. The problem goes away temporarily if I disable your module, and returns when I enable your module. Clearing the cache doesn't help. Uninstalling and then re-installing your module made the notices go away permanently.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Needs work

Thank you for the thorough review arnoldbird.

In reply:

The offloading of functions is to keep all my back-end functions & logic in one place, such as submit & validation functions etc. That being said, hook_block_info was indeed in the wrong place and I did re-arrange some code.

Your point about the clearing cache is very valid, I re-worked that part with the hooks you suggested.

As for the errors, it seems the Taxonomy Menu module runs this code in hook_uninstall:

// Delete variables
  $query = db_select('variable', 'v')->fields('v')->execute();
  $variables = $query->fetchAll();
  foreach ($variables as $variable) {
    if (strpos($variable->name, 'taxonomy_menu') !== FALSE) {
      variable_del($variable->name);
    }
  }

Since the name of my module is quite similar my variables get deleted too, generating errors. Correct me if I'm wrong but I consider this to be an error on the side of the Taxonomy Menu module. It should do a more correct check than a strpos. Any thoughts?

I have also added some form validations as Flakmonkey suggested.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

Forgot to change status.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Assigned: Unassigned » cweagans
Status: Needs work » Reviewed & tested by the community
Issue tags: -PAreview: review bonus

manual review:

  1. Notice: Undefined property: stdClass::$vid in taxonomy_menu_block_entity_presave() (line 550 of /home/s1534c77966ab5f7/www/sites/default/modules/1630054/taxonomy_menu_block.module). When adding a new vocabulary. I think you want hook_entity_update() instead?
  2. Please add the differences to taxonomy_menu to the project page.

But otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to cweagans as he might have time to take a final look at this.

Anonymous’s picture

Anonymous (not verified) commented

Bug fixed, thanks for the heads up.

Also added difference to Taxonomy Menu on the project page. Will write better documentation in a few days.

I don't know if this is important or not but I thought I'd better mention it: even though this module is 100% written by me the sandbox is not actually mine but a friend's. Don't know if that matters in the final approval process, with the promoting sandboxes to full projects and stuff.

Thanks!

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

Well, who would I be giving vetted git access to then?

samspinoy, if I'm giving you vetted git access, please have paulvb chime in here giving me permission to change the project ownership to your user account.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Postponed (maintainer needs more info) » Reviewed & tested by the community

@cweagans: the project owner does not matter, co-maintainers can also promote the project. They only need sufficient permissions on the maintainers tab, and samspinoy can get that from the project node author.

Anonymous’s picture

Anonymous (not verified) commented

I'm a bit confused here... do I need to do anything in particular? I have access to paul's sandbox, to commit code and change the project description etc. Will that suffice?

Also, if I get git access and promote the project, will that make paul co-maintainer automatically?

Or how do we go from here?

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
Status: Reviewed & tested by the community » Fixed

Sorry for the confusion. I mistakenly believed that a user needed to own the project node that they were using to get git vetted access. Given klausi's reply...

Thanks for your contribution, samspinoy!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

arnoldbird’s picture

arnoldbird commented

Correct me if I'm wrong but I consider this to be an error on the side of the Taxonomy Menu module. It should do a more correct check than a strpos. Any thoughts?

Yes I'd say that's right. It's bound to happen again, as other people who try out your module might also try out Taxonomy Menu. You might submit a patch to the Taxonomy Menu module.

Anonymous’s picture

Anonymous (not verified) commented

Awesome. Thanks a lot for the reviews and suggestions guys!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Change clone repository