Node count wrong when node access modules enabled

Hi

I think that this is a real issue.
I have a case where advanced access rules have been applied to nodes and the count cannot handle that. If I am an admin , I can see every nodes; if I am a user with lower privileges, I may only be able to see a few nodes. The count includes all nodes as if I could view all of them regardless my level of privileges.

Because each user in the site may havedifferent access privileges, the post count in the menu should be different for everybody (same issue as previous post). I don't know how this one can be solved because the menu is cached and the label seems to be the same for everybody.

Those access rules are defined using hook_access_grants and hook_node_access_records, so by PHP code. It cannot be solved with mysql queries. This means that the _taxonomy_menu_term_count function has to be re-written using the hook_node_access to increment the count.

If anyone has an idea on how to solve that issue, that would be great.

Minhtao

There's not a solution really, unless you want to rebuild the menu on EVERY page load for every user role, which you don't.

This is a pretty good argument actually for completely getting rid of the node counts.

db_rewrite_sql() which calls i18ntaxonomy_db_rewrite_sql() also steps in the way when using the i18ntaxonomy module (from i18n modules package). All nodes which do not belong to the current active language don't get counted.

How about this approach? Let node access grants check permissions as menu links do.

Only adding node_access to the query wont help. Since the node count is saved to the menu link itself, all users will see the amount of nodes that the user who generated the menu has access to. To get this working, the node count needs to be determined for each user. This can be achieved using hook_translated_menu_link_alter(), adding the count when loading the menu item instead of saving it to the database.

Attached patch moves the node counting and hiding of empty links to taxonomy_menu_translated_menu_link_alter. In order for the tests to pass, I also had to change assertMenuLink to use menu_link_load, instead of fetching the values from the database.

I also removed the translation handling from taxonomy_menu_translated_menu_link_alter, as it could never be called anyway (['options']['alter'] was never set for the links). Additionally, translating menu links "on the fly" needs all terms to be loaded each request, so this wasn't a very elegant solution.

By counting the terms in taxonomy_menu_translated_menu_link_alter, the need for taxonomy_menu_node_insert, taxonomy_menu_node_update, taxonomy_menu_node_presave and taxonomy_menu_node_delete is eliminated, so those are also removed.

As a vaguely related issue, I also added recursive counting and associated tests while at it.

@jgullstr Patch in comment #7 applies without problems to current 2.x-dev version.

Encountered the same issue with access control modules.

After patching "drush updb" has nothing to do and flushing caches is not enough to refresh counts. It is necessary to force the taxonomy menu to rebuild by going to the vocabulary edit settings, check "Rebuild the menu on submit" and save. After menu is rebult go back and uncheck the rebuild setting and save. It again rebuilds the menu one more time that is not necessary.

After this everything is correct and adding a term to new content updates the count.

Only new issue with this patch is terms with no content now show count of zero including parent terms that have no content. It was more cosmetically pleasing to not show zero counts. Less cluttered look.

Thanks for patch which seems to be working. I have not done a code review on patch.

The patch in #7 needs a rebase against 7.x-2.x.

Attached update. Recursion is avoided in this one by using taxonomy_get_tree to fetch children and minimize amount of db queries.

Still, a single db query is executed per term during count using this approach. The term counts could maybe be cached per user for better performance?

@ismeez There's a setting for hiding empty terms. Have you enabled it?

This is going to need a reroll as the taxonomy_menu.admin.inc file has been removed.

@jgullstr Hiding empty terms in the edit vocabulary > structure options is not the same. It was better before when a count of zero was not displayed beside the term in the menu. Same issue in latest patch in #10.

[edit]My mistake, didn't have latest dev release. Didn't have any problem applying patch to dev release today, taxonomy_menu.admin.inc file is still present.

@jenlampton the taxonomy_menu.admin.inc has been removed from the 1.x branch but not the 2.x branch. This helps to explain our experience that the patch in #10 does still apply to current 2.x-dev.

This is a critical issue for cases where node access modules are used.

Patch in comment #10 applies to taxonomy_menu-7.x-2.x-dev and works as expected with organic groups.

Rolling the patch for the 1.x branch looks to be quite involved.

It's a shame that the 2.x branch is not supported while the 1.x branch is, see #3090445: Taxonomy_menu 7.x-2.x reported as "Not supported" on Available updates.

Attached is a patch for the 2.x-dev branch identical to that in comment #10 only with a fixed typo.