Problem/Motivation

There is a node access documentation topic, but it does not include the various classes and interfaces that interact with node access, only the procedural code.

Proposed resolution

Add the appropriate @ingroup to classes and interfaces in the node module that play a role in node access.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because there is no functional bug, just some extra digging needed.
Issue priority Normal; it's a fairly small issue, but it does make the documentation topic incomplete
Unfrozen changes Unfrozen because it only changes documentation.

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Create a patch Instructions
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards Instructions

User interface changes

API changes

CommentFileSizeAuthor
#9 interdiff_2473123_6-9.txt512 bytesjacobsanford
#9 node_access_documentation-2473123-9.patch3.79 KBjacobsanford
#6 interdiff-4-6.txt2.42 KBsander.devos
#6 node_access_documentation-2473123-6.patch4.29 KBsander.devos
#4 node_access_documentation-2473123-4.patch1006 bytessander.devos

Comments

xjm’s picture

xjm
she/her
Primary language English
commented
Issue summary: View changes

Adding beta evaluation.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

The only class I see mentioned in the text is NodeAccessControlHandler, which implements NodeAccessControlHandlerInterface...

Are these the only two that need @ingroup to be included here?

sander.devos’s picture

sander.devos commented
Assigned: Unassigned » sander.devos

I'm going to try and work on this issue.

sander.devos’s picture

sander.devos commented
Status: Active » Needs review
StatusFileSize
new node_access_documentation-2473123-4.patch1006 bytes

My first ever patch. Hope you all like it!

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs review » Needs work

Thanks @sander.devos!

There are a couple more classes and interfaces we need to add. See, for example, NodeGrantDatabaseStorageInterface. Take a look inside the class at the dependencies to get an idea. See the related issues around #2461049: Node module permissions are broken if hook_node_grants is implemented for more ideas as well.

sander.devos’s picture

sander.devos commented
Status: Needs work » Needs review
StatusFileSize
new node_access_documentation-2473123-6.patch4.29 KB
new interdiff-4-6.txt2.42 KB

I just applied more changes. I think I got most of the classes that interact with node access.

I also found that BookNodeIsRemovableAccessCheck makes use of node access. But is in another module. Do I have to add @ingroup node_access here as well or not?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: sander.devos » xjm
Status: Needs review » Reviewed & tested by the community

I don't think that the class in the Book module belongs in the node access topic... the list looks good to me. Provisionally setting to RTBC and assigning to xjm to decide whether everything is complete now.

xjm’s picture

xjm
she/her
Primary language English
commented
Assigned: xjm » Unassigned
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/node/src/NodeTypeAccessControlHandler.php
+++ b/core/modules/node/src/NodeTypeAccessControlHandler.php
@@ -16,6 +16,7 @@
  * Defines the access control handler for the node type entity type.
  *
  * @see \Drupal\node\Entity\NodeType
+ * @ingroup node_access
  */
 class NodeTypeAccessControlHandler extends EntityAccessControlHandler {

So this one I think should not be in the group since it is about access control for creating content types, not for nodes.

Looks good to me other than that!

jacobsanford’s picture

jacobsanford
Primary language English
Location Fredericton 🇨🇦
commented
Assigned: Unassigned » jacobsanford
Status: Needs work » Needs review
StatusFileSize
new node_access_documentation-2473123-9.patch3.79 KB
new interdiff_2473123_6-9.txt512 bytes

Update as requested in #8.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Looks like this fixes #8, so should be RTBC now. Thanks!

xjm’s picture

xjm
she/her
Primary language English
commented
Assigned: jacobsanford » Unassigned
Status: Reviewed & tested by the community » Fixed

This issue only changes documentation, so per https://www.drupal.org/core/beta-changes, this can be completed any time during the Drupal 8 beta phase. Committed and pushed to 8.0.x. Thanks!

  • xjm committed c085079 on 8.0.x 
    Issue #2473123 by sander.devos, JacobSanford, xjm, jhodgdon: Add node...

Status: Fixed » Closed (fixed)

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