API page: http://api.drupal.org/api/drupal/modules--node--node.module/group/node_a...

Description of how node access rights works is incomplete. It is not mentioned that one have to use hook_query_TAG_alter() to alter node listings.

Current paragraph:
In node listings, the process above is followed except that hook_node_access() is not called on each node for performance reasons and for proper functioning of the pager system. When adding a node listing to your module, be sure to use a dynamic query created by db_select() and add a tag of "node_access" to ensure that only nodes to which the user has access are retrieved.

Something like the following should be appended: Modules implementing hook_node_access() will then be able to use hook_query_TAG_alter() on queries tagged with "node_access".

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Title: Documentation problem with Node access rights » Node access rights doc should mention how to use query tags
Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7

Sounds like a reasonable idea, though actually it's a different system, so I think the text to be added is not as simple as suggested...

Anyway, should be patched in 8.x, then backported to 7.x.

mdupont’s picture

While it's a different system, isn't the whole concept of adding a query tag to be able to alter the query through hook_query_TAG_alter()? If so, then by mentioning a query tag in the documentation a hint should be given on how to use it. The query tag itself is from a different system, and there is already a reference to db_select().

What about simply adding @see hook_query_TAG_alter() or change the paragraph to:

When adding a node listing to your module, be sure to use a dynamic query created by db_select() and add a tag of "node_access". Using hook_query_TAG_alter() on this tag will ensure that only nodes to which the user has access are retrieved.

mdupont’s picture

Status: Active » Needs review
mdupont’s picture

Patch attached

jhodgdon’s picture

Status: Needs review » Needs work

Actually, the main purpose of adding a query tag to a query in module X is so that a site's other modules that handle node access are respected in module X's query. I think the doc you added implies that a module that adds the tag also needs to add a hook_query_TAG_alter, which isn't the case. Can it be reworded?

mdupont’s picture

Status: Needs work » Needs review
FileSize
918 bytes

Right, here is a reworded attempt.

jhodgdon’s picture

That looks much better. The only thing I would suggest is changing "It" to either "This" or "That" at the beginning of the sentence you added.

mdupont’s picture

Changed to "this". Every opportunity to improve my English is good :-)

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me, thanks! 8.x/7.x please.

mdupont’s picture

Note : the patch also applies cleanly against 7.x-dev.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x and 7.x. Thanks!

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