Problem/Motivation

The work being done in #731724: Convert comment settings into a field to make them work with CMI and non-node entities will result in that comments can be used on any entity just by creating a comment field in the same way as adding a text field. It will also be possible to tailor each comment form used with other fields, making them perfectly adapted for each entity type used on.

But, there is one very big bottleneck that will severely block the usefulness of this new flexibility. That is that permissions are still globally set for all comment forms throughout the same site. This means that it isn't possible to:

  • Allow anonymous commenting on blog posts
  • Only allow comments for logged in users on articles
  • That comments to product must be approved before being visible

To take it further. A site might even need to have different commenting permissions for two different blog content types on the same site.

This permission control isn't just limited to the entity type itself. It should also be possible to override on individual entity items too. For example:

  • A blog post offering a giveaway for the right answer could then set the comments to that post to be approved before publishing.
  • The writer want to disable comments to a post about a sensitive topic.

This kind of detailed configuration simply isn't possible with global permissions.

Implementing the proposed solution below will not only make the comment module a lot more flexible, it will also give site-owners much more control over their site and who can do what and where.

Proposed resolution

First:
comment_permissions.png

  • Remove the global comment viewing/posting permissions completely.
  • Split the "Administer comments and comment settings " into two:
    • Global comment administration
    • Configure comment settings

Then move all the comment viewing/posting permissions to the field configuration. Permissions needed for:

  • View
  • Post
  • Reply (new)
  • Skip approval
  • Approve comments (new)
  • Edit own
  • Edit all (new)

Anonymous, authenticated roles will be statically available. All other roles can be manually added to the field. If not added, the permissions for the authenticated is used unless the two new global comment permissions kicks in.

It should also be possible to set the default permissions in the global comment settings. Including adding more roles that then automatically are used for new comment fields.

There will also be a new setting for allowing overriding certain settings on individual items. Such as described in the examples above.

When enabled options are needed for:

  • What options can be overridden
    • Turn on/off skip approval
    • Disable/Enable commenting
    • Close/open commenting
  • Who can do it
    • Item author (overrides the need of the "Approve comments" perm)
    • Roles with the "Approve comments" perm

When for example disabling commenting or turning on approval, a custom, overrideable, message should be displayed instead. For example "Comments to this post has been disabled due to ....".

Here too, the global comment admin permission overrides all custom settings.

Remaining tasks

  • Create mockups with proposed UI changes
  • Implement the changes

User interface changes

TBD

API changes

TBD

#10426: Make comment permission more granular. Assigned to: killes@www.drop.org

Files: 
CommentFileSizeAuthor
#18 comment-permissions.wip_.patch13.1 KBlarowlan
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/comment/comment.module.
[ View ]
comment_permissions.png5.3 KBtsvenson

Comments

People working on this may be interested in http://drupal.org/node/1903124 and http://drupal.org/node/1903128.

An (apparently) similar discussion to the one reported in the OP happened in #1807776: Support both simple and editorial workflows for translating entities.

@plach This awesome change! Is there any suggestions/ideas about UI for that? Following #1807776: Support both simple and editorial workflows for translating entities I found that UI part is planned for contrib to not mess a permissions page.

I we make #731724-291: Convert comment settings into a field to make them work with CMI and non-node entities real so this permissions will be very useful. But I not sure exactly how to apply them to comment-field on per entity/bundle basis

Sort of related #366416: Field Permissions
I think if that gets in this may be obsolete, but I don't consider that likely at this point.

Category:task» feature
Priority:Major» Normal

I don't see anything in the OP that you can do in core right now, so don't understand why this is a major task. It sounds like a feature request to me, and most likely one for D9.

Assigned:Unassigned» andypost
Category:feature» task
Status:Active» Postponed

@tsvenson once we have agreement on settings for comment form. We can continue here.
Anyway having this global settings currently enough, contrib have field permission and only posting an action that questionable.

I see no reason to introduce this permissions in global scope.

Global
View comments - granularity defined by field
Skip approval - same as now, detailed about anonymous in Form settings

Administer - skip entity access and allows to configure forms.
Approve comments (new) - great idea!

Entity Access
Edit own - entity level
Edit all (new) - entity level
Post comments - global + granularity defined by entity

Reply (new) - useless, It's just a link in formatter (when user allowed to post for the entity type)

Here's a new way to extend access for fields introduced in #1883152: Field level access for EntityNG

Why do we need to introduce a new concept for this? If we don't fix generic field level access for D8 we should not build a crippled comment-specific field-level access solution here. Instead, just provide dynamic permissions for each comment field instance in use. If you want/need a different permission model for commenting on nodes vs. commenting on users just make the two two separate comment setting fields. Done.

@fubhy: Say what? As things are now both those comment settings fields with be governed by the global comment permissions as described in the OP.

From the issue summary:

Then move all the comment viewing/posting permissions to the field configuration. Permissions needed for:

I disagree with that. We should not split up access permissions to fields and global unless done generically. Let's not do this specifially for comments. If we can solve field level access for D8 generally fine, let's do it, if we can't let's not introduce a crippled subset of that functionality specifically for comment module.

Permissions should be controlled via the global permission configuration page. If that is your intent, both the issue summary and title are not correct.

However, if you actually plan to move permission configuration into the field instance configuration (as the issue summary and title suggest) you are introducing a huge comment-module specific UX wtf.

If we can solve field level access for D8 generally fine, let's do it, if we can't let's not introduce a crippled subset of that functionality specifically for comment module.

I would like to turn that around and say that - As long as we have global permissions for things, such as fields, we are severely crippling the usefulness and flexibility of those features. It forces sites to make a global permission compromise instead of being able to tailor it exactly to how they need it for each individual feature on the site.

This crippling is painfully exposed in this case.

Also see my Drupal Limitations and Bottlenecks and Don’t Make Drupal the New ‘Microsoft Office’! posts that goes into much more detail about this.

I would like to turn that around and say that - As long as we have global permissions for things, such as fields, we are severely crippling the usefulness and flexibility of those features. It forces sites to make a global permission compromise instead of being able to tailor it exactly to how they need it for each individual feature on the site.

I am not saying we should not allow per-comment-field-instance permissions. But we should NOT move them into the field settings. We should keep them on the global permission page. Otherwise people will have serious issues trying to find out where they can set permissions for what. Let's keep it in one spot. That's all.

How about making them available in both places?

From a UX perspective it would make a massive improvement, and avoid pogo-sticking, if I would be able to get a full overview of both the configuration and the permissions at field level too.

Then on the big permissions page the same are shown, grouped per field and also with a convenient link to the field settings page too.

There's global permissions anyway - for comment entity (for 80% of sites enough)
I think better to implement permissions per-comment form as "bundle".

If someone will need a per-field settings - it's ok with contrib module a-la field_permissions for comment-field.
Permissions on field-instance level seems absurd but could be implemented with custom module.
Another case is per-entity permissions or more granular for example the workflow module when commenting should be enabled in some states or roles.

The related discussion about access to field/field-item #2018707: Do we still need to support hook_field_prepare_translation() ?

Status:Postponed» Active

Assigned:andypost» larowlan

grabbing this one, not sure if you're actively working on it, if so, please assign back and I'll hold-off.

Status:Active» Needs review
StatusFileSize
new13.1 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/comment/comment.module.
[ View ]

here's a wip, still lots of calls to user_access|hasPermission(access comments|post comments|skip comment approval|edit own comments) to update to the new method as follows:
before

<?php
$account
->hasPermission('access comments');
// Legacy format.
user_access('access comments');
?>

after
<?php
$manager
= \Drupal::service('comment.manager');
$field = $manager->loadField($field_name, $entity_type);
$manager->checkPermission('access comments', $field, $account);
?>

But before that thoughts on approach.
Offline for a few weeks so feel free to keep kicking this along.

The last submitted patch, comment-permissions.wip_.patch, failed testing.

Issue summary:View changes

related

I'm wondering if we might be better to refactor comment permission to make one set of permissions per comment bundle regardless.
For sites upgrading from D7 with stacks of different node-types (which map to bundles on upgrade) this would mean a lot of permissions, but no more than node spits out. And if they're not using said bundle, they can remove the field.
Thoughts?