Improve hook_node_grants() docs around effect of published status of nodes.

Good point... There is a clearer discussion in http://api.drupal.org/api/drupal/modules%21node%21node.module/group/node...

The code of http://api.drupal.org/api/drupal/modules%21node%21node.module/function/n... also shows the process, and how it relates to $node->status. This only applies to single-node checks though, not listings, and it is also true that hook_node_grants() doesn't distinguish between published and unpublished. So probably the hook_node_grants() docs just need to clarify that one sentence noted in the original issue report here. Probably a good Novice project...

To me the docs on hook_node_grants() are pretty clear but please review the following revised paragraph and see if it makes better sense and clarity for updating the docs:

A node access module may implement as many realms as necessary to properly define the access privileges for the nodes. Note that in cases where single-node checks are performed there is a private case of enabling the user to view his own unpublished node if the permission 'view own unpublished content' has been set for that user, in which case node access checks will stop there and hook_node_grants() will not be called. Otherwise, the node access system makes no distinction between published and unpublished nodes. It is the module's responsibility to provide appropriate realms to limit access to unpublished content.

How's that?

That looks pretty good, except I'm not sure what a "private case" means, and the second sentence runs on for quite a while.

But maybe the right thing to do would be to link to the page that completely describes the node access process instead of partially duplicating what that page says? Which would be:
http://api.drupal.org/api/drupal/modules!node!node.module/group/node_acc...
And perhaps that page needs some additional text about the case described here?

Hello, I have attempted to revise the paragraph on node access rights. Please review the paragraph to see if it is explained clearly.

A node access module may implement as many realms as necessary to properly define the access privileges for the nodes. The distinction between published and unpublished nodes is clarified by node_access. Please see this link for more detailed information on the process of determining access rights for a node. https://api.drupal.org/api/drupal/modules!node!node.module/group/node_ac... It is the module's responsibility to provide appropriate realms to limit access to unpublished content.

What does "node_access" mean in this sentence:
The distinction between published and unpublished nodes is clarified by node_access.
I'm confused.

Also as a note, in documentation to make a link to a topic, use something like

See @link node_access the Node Access topic @endlink for more information.

This would be easier to check in the form of a patch.

Thank you for your review. This is my second attempt at the paragraph. I removed node_access from my previous attempt.

A node access module may implement as many realms as necessary to properly define the access privileges for the nodes. Note that in cases where single-node checks are performed, there is an option of enabling the user to view his own unpublished node. See @link node_access the Node Access topic @endlink for more information. The node access system makes no distinction between published and unpublished nodes. It is the module's responsibility to provide appropriate realms to limit access to unpublished content.

And as per your comment #5 I added additional text about the case described above.

If all modules ignore the access request, then the node_access table is used to determine access. All node access modules are queried using hook_node_grants() to assemble a list of "grant IDs" for the user. This list is compared against the table. If any row contains the node ID in question (or 0, which stands for "all nodes"), one of the grant IDs returned, and a value of TRUE for the operation in question, then access is granted. Note that this table is a list of grants; any matching row is sufficient to grant access to the node. In cases where users are able to view their own unpublished node, the permission 'view own unpublished content' has been set for that user and node access checks will stop there and hook_node_grants() will not be called.

Please make a patch.

Here's the patch. Just copied from finnydobson and unsplit an infinitive.

Thanks @ottlik, I have made some slight changes:

• wrapped the lines so none go over 80 characters
• removed trailing whitespace

All text is the same.

Thanks for the patches! I think this needs some work:

a)

 ... Note that in cases where
+ * single-node checks are performed, there is an option of enabling the user to
+ * view his own unpublished node.

This sentence needs some attention. We don't normally say that we "enable the user to" do things, and where is this option anyway? Also please do not use "his" or other gender-specific language in documentation.

b)

See @link node_access the Node Access topic
+ * @endlink for more information.

@link ... @endlink all needs to be on one line.

c) Generally... I am not finding this new content very easy to follow, in the context of the rest of the documentation that is already there. It seems to wander between the single-node-access check and the grant system. It needs to be more systematic.

Assembled the input from @jhodgdon and merged with patch.

Thanks for reviving this issue! I think this still needs some work though... and this is probably not a novice issue really!

1. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,25 @@
   + * A node access module may implement as many realms as necessary to define the
   + * access privileges for the nodes properly. In cases where single-node checks ¶
   + * are performed, there is an option to the user to view their own unpublished nodes.
   + * See @link node_access and @endlink the Node Access topics for more information.
   + * The node access system makes no distinction between published and unpublished ¶
   + * nodes. It is the module's responsibility to provide appropriate realms to limit ¶
   + * access to unpublished content.
   + *
   

   This paragraph has several lines that run over 80 characters, and several that end with an extra space.

   There is also some awkward wording in the sentence about single-node checks, and again it shouldn't refer to "an option to the user". This is programmer documentation, not user documentation, so talk about what a programmer can do in a module and how to do it, not what a user can do in the UI.

   The See sentence also needs some attention. You might need to look up how to do @link:
   https://www.drupal.org/node/1354#link

2. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,25 @@
   + * access to the node. In cases where users are able to view their own
   + * unpublished node, the permission 'view own unpublished content' has been set
   + * for that user and node access checks will stop there and hook_node_grants()
   + * will not be called.
   

   Too many "and" in this sentence.

   Also, ... this is unclear. The first part of this paragraph says that the node_access table and hook_node_grants() will be called. I think this is a bypass of that whole process, but that is not made clear by this sentence. Perhaps it belongs in its own paragraph, and perhaps it belongs before this paragraph about node_access table and hook_node_grants()?

I tried to give my better in this patch.

Sorry, but this is still not OK. The @link formatting is still wrong, and not everything pointed out in #15 is fixed. All of the wording in there about users viewing their own unpublished content is still very very wrong and/or confusing.

I have made some improvements

Thanks for the new patch but this is still missing the point. And it still has similar problems to before....

1. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,25 @@
   + * are performed, there is an option to the user to view their own unpublished nodes.
   

   This line goes over 80 characters.

2. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,25 @@
   + * See also @link node_access, Node Access topics for more information. @endlink ¶
   

   This line has an extra space at the end.

3. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,25 @@
   + * set for that user and node access checks will stop there and hook_node_grants()
   

   This line goes over 80 characters.

1. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,25 @@
   + * access privileges for the nodes properly. In cases where single-node checks
   + * are performed, there is an option to the user to view their own unpublished nodes.
   

   This sentence about single-node checks still doesn't fit with this paragraph and I am not sure what it means.

   Either fix it so it is right and move to its own paragraph, or (preferably) remove it.

   I really don't know where this came from or why it was added.

2. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,25 @@
   + * See also @link node_access, Node Access topics for more information. @endlink ¶
   

   This @link is still wrong. Please read the documentation about how to do @link that I referenced in previous reviews.

   Also it is only 1 topic, not "topics".

3. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,25 @@
   + * If all modules ignore the access request, then the node_access table is used
   

   This is a paragraph copied out of context from the node access topic. It makes no sense here.

I have made the changes. Please review my patch.

Please ignore the previous on and review this patch.

Most of what I said in the previous several reviews has still not been addressed. When a patch is made that addresses *all* of the problems, please attach it to this issue and set it to Needs review. If you cannot address all the problems, ether get help from the Core Mentoring group, post a question to the issue, or find another issue to work with. Making a patch that only addresses a small part of the problems that were identified is not helpful.

Also when you make a new patch, make an interdiff file.

Thanks!

1. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,22 @@
   + * access system makes no distinction between published and unpublished nodes. ¶
   

   Blank Spaces. Please configure your IDE for not doing this

2. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,22 @@
   + * It is the module's responsibility to provide appropriate realms to limit ¶
   

   Blank Spaces. Please configure your IDE for not doing this

3. +++ b/core/modules/node/node.api.php
   @@ -29,11 +29,22 @@
   + * All node access modules are queried using hook_node_grants() to assemble a ¶
   + * list of "grant IDs" for the user. This list is compared against the table.
   + * If any row contains the node ID in question (or 0, which stands for ¶
   ...
   + * of grants; any matching row is sufficient to grant access to the node. ¶
   + * In cases where users are able to view their own unpublished node, the ¶
   

   Blank Spaces. Please configure your IDE for not doing this

Here is an updated patch which fixed the white space issues.

Discussed in a documentation triage meeting with myself and ambermatz. Nether of us are familiar with the node grant system but we do like the additional documentation to explain the system. We also noticed a few things

The new second paragraph 'the table' is referred to without defining what table is being used.
"realms" is kind of confusing here. It should have a definition.
Needs a review from someone familiar with node grants.
Can be unassigned since it hasn't been worked on it a few years.

I have updated the IS.

Attached patch against Drupal 10.1.x

#43 was not addressed.

Starting March 2023, simple rerolls, rebases, or merges will no longer receive issue credit. Only rerolls that address a merge conflict will be credited, and the merge conflict that was resolved must be documented in the text of an issue comment.

Example
error: patch failed: core/modules/system/tests/modules/twig_theme_test/twig_theme_test.module:77
error: core/modules/system/tests/modules/twig_theme_test/twig_theme_test.module: patch does not apply

To receive credit for contributing to this issue, assist with other outstanding tasks or unaddressed feedback.
See the issue credit guidelines for more information.