Improve 'administer content' permission description

Shall I look into this.

Possibly reposition Administer Content row.

There is no reason to do this.

This comment is not necessarily an endorsement of this issue.

Fixed html

+++ b/core/modules/node/node.permissions.yml
@@ -8,7 +8,7 @@ administer content types:
+  description: 'Change ownership, date/time, promote or make sticky and edit past revisions for content in all content types. Does not include view, edit or delete.'

promote or make sticky

Its not one or the other, both are possible

date/time

Of what? (creation date) Why is this worthy of mention?

Also, preferably no slashes

Does not include view, edit or delete.

Typically we only mention what a permission does, not what is does not do.

@wturell, changes are good. Patch supplies well. Changing to RTBC.

Thanks everyone for improving the text of this key permission. Accurately communicating what powerful permissions like this do is very important.

I'm not sure this is the best change. I find the last sentence actually confusing. In general our overall pattern should be to shorten or remove permission descriptions rather than lengthen them, because we should make the label say everything the user needs to know about the permission if possible. That may not be possible in this case because it is a complex permission.

Looking over the introduced changes with a word diff, this patch:

1. Makes "Change ownership" first
2. Moves "Promote" later. I guess that makes sense since it is not by itself the most important permission.
3. Adds "date/time". The meaning of that is unclear.
4. Changes "edit revisions" to "edit past revisions". While I see the goal of this change, it is actually implying that the user can't edit future revisions, and I believe they can with this permission and core or contrib modules that allow that. Does the permission also allow changing which revision is the published one? The summary says "You can't access /node/1/revisions with Administer Content alone" so maybe not. Not sure about that change.
5. Changes "across content types" to "for content in all content types". That's probably better, although it is more words.
6. Removes "perform other tasks". I can see the goal in removing this because it's vague. However, it looks like maybe this was what was replaced with "date/time", to give the user a complete picture of what the permission could do, maybe? The problem with that is that other modules may also use this permission to allow content administrators to do things with their module, that won't be listed here. I agree "perform other tasks" is not really that helpful though.
7. Removes the serial (Oxford) comma. We should put it back; it's in our content guidelines. :)
8. Finally, the added sentence "Does not include view, edit or delete." That's kind of confusing and I wouldn't necessarily know what it means. I think this is say that the user who can administer all these things can do so only if they already have access to the content.

So, maybe a better improvement would be to have something like:

Change metadata like author, published time, 'promote' and 'make sticky', etc. for content the user may access in any content type, and edit other revisions.

That's just a rough draft; it can probably be better. I might not be following all the content standards there for the descriptions. I just wanted to show how we could explain the limitations on the permission more clearly.

In general, a lot of usability work is done on these UI descriptions, and many of them have undergone prior usability review. So, it's good to get usability review before changing them. However, this issue does not seem to be attached to any meta issue for permission descriptions that are being updated to get a high-level picture on this work. Can we create one and attach these issues to it?

Finally, I'm adding a couple of past related issues.

So, let's look more at how we could improve this text, and look at past improvements made to it and related permissions as well as future improvements that are proposed for other related permissions. Then, with that larger picture, we can come up with an improved description and get usability review to make sure we are achieving our goal.

Thanks everyone for your careful attention to detail here!

@wturrell, can you suggest the changes to be issued as of now?

@wturell, made new changes. Please review

uploading interdiff here.

@wturell,

Could you please describe the changes required as of now ?

@wturrel thanks for raising this issue, I agree especially that "perform other tasks across all content types" is too vague, it could mean anything, and the only way to know what the consequences are is to tediously experiment. I've been using Drupal for about 10 years and I still sometimes struggle with that permission setting.

I guess this should target the latest dev release. And un-assigning the issue since there is nothing done since 2017 =)

I've been using Drupal for about 10 years and I still sometimes struggle with that permission setting.

+1

Figuring out what "administer nodes" gives access exactly is almost impossible to tell. My gut says both core and contrib modules are using it incorrectly. Probably we should get rid of these AIO permissions and only support granural ones which makes more clear that who can access to what and why.

I've reviewed the patch in #23, and this is the updated recommendation:

Change ownership, published time, promote, make sticky and edit past revisions of content, for all content types. Does not include view, edit or delete permissions to users without prior access to the content.

This is a big improvement and I support this change.

I am not sure if "edit past revisions of content" should be tied to this permission. Probably revisioning system should have it own permission for handling access to past/draft revisions.

Although I do agree that the description in #23 describes much better what this permission is currently used/should be used for.

Thanks for working on this. Node access control has always confused people and maybe we can help fix that.

1. +++ b/core/modules/node/node.permissions.yml
   @@ -8,7 +8,7 @@ administer content types:
   -  description: 'Promote, change ownership, edit revisions, and perform other tasks across all content types.'
   +  description: 'Change ownership, published time, promote, make sticky and edit past revisions of content, for all content types. Does not include view, edit or delete permissions to users without prior access to the content.'
   

   The first sentence is lacking parallelism, and is therefore very confusing.

   "Change ownership" is a verb with an object. "published time" is a noun. "Promote" is again a verb. "Make sticky and edit past revisions" is not a single concept, yet is written as such. (There's at least a missing serial comma after "sticky"). The second sentence is also unclear -- I don't understand it, and I nominally am a node access system maintainer. ;)

2. +++ b/core/modules/node/node.permissions.yml
   @@ -20,10 +20,10 @@ view all revisions:
   -  description: 'Role requires permission <em>view revisions</em> and <em>edit rights</em> for nodes in question or <em>administer nodes</em>.'
   +  description: 'Role requires permission to <em>view revisions</em> and <em>edit</em> permission for nodes in question or <em>administer nodes</em>.'
   ...
   -  description: 'Role requires permission to <em>view revisions</em> and <em>delete rights</em> for nodes in question or <em>administer nodes</em>.'
   +  description: 'Role requires permission to <em>view revisions</em> and <em>delete</em> permission for nodes in question or <em>administer nodes</em>.'
   

   This also is missing parallel constructions. At a minimum, the second instance of the word "permission" in each case is redundant. It says currently "Role requires permission... to delete permission".

Also, it looks like my feedback in #19 wasn't really ever addressed. Please review both before continuing work on this issue. It should also have a usability review once the obvious problems have been fixed, as I stated back then. :)

1. I interpret the first sentence as "Change all, those, things". But agreed that the mix of nouns and actions is confusing, but it's also hard to make nouns for promotedness and stickyness.

I think I mentioned that elsewhere but IMHO, the main reason that it's so hard to describe is that the permission is a mess. It simply covers all the left-overs that weren't access content overview nor bypass node access, back when those two permissions were split away from administer nodes.

If this would be a simple fix then we could do this and then later split it off, but maybe we should actually start by adding dedicated permissions for some of the things it covers, that would make it easier to describe then eventually, although IMHO the goal should eventually be to deprecate the permission entirely. We could start with the revision part. There's already ongoing activity to improve node revision access handling.

Applied patch #44 . Now the description seems more meaningful.
Attaching screenshots
Before patch:

After patch:

RTBC

Thank you to everyone for getting this to RTBC.

I briefly read the issue and noticed that the feedback from xjm in #19 and #41 has not been addressed.

The IS needs an update, the remaining task states that the wording needs to be agreed on which based on the feedback mentioned above that has not happened. This issue requires screenshots so the latest before and after screenshot should be in the Issue Summary. There may be other items in the IS that need updating, I only skimmed it. Adding tag.

Because of the above, setting back to NW.

Applied patch #46 applied successfully
After patch description has been changed in node.permission.yml file
Thanks for the patch

Still relevant, #19 and #41 still need actioning. We also need an MR here.

After reading the current description in Drupal 11 I still didn't understand completely what the "administer nodes" permission does. After examining \Drupal\node\NodeAccessControlHandler::checkFieldAccess i did. It still felt strange to me that this is single permission covers all node types. Shouldn't it be split in a permission per node type as is done for the view/edit/etc permissions?