Add a shortcut to unapprove a comment for consistency and usability

I think that makes sense, to achieve better consistency.

Maybe it also would be a good idea to add support for contextual links, to make it even more consistent. That of course would only be added it the contextual links module is installed.

Thinking more about it, I don't know if I like the AJAX thing though. That is not a pattern core uses for that type of functionality.

Here is a re-roll of the patch in #4. It fixes some spacing issues (see below) and adds test to assert the link's functionality.

+++ b/modules/comment/comment.pages.inc
@@ -117,3 +117,22 @@ function comment_approve($cid) {
   return MENU_NOT_FOUND;
 }
+/**
+ * Menu callback; unpublish specified comment.
+ *
+ * @param $cid
+ *   A comment identifier.
+ */
+function comment_unapprove($cid) {

Should always be blank line between functions.

+++ b/modules/comment/comment.pages.inc
@@ -117,3 +117,22 @@ function comment_approve($cid) {
+  return MENU_NOT_FOUND;
+}
\ No newline at end of file

Should always be blank line at end of file.

20 days to next Drupal core point release.

Hmm, a tab space seemed to sneak in there somehow. New patch.

I was searching for this solution as well, that's why I wrote my custom module for D7 (Views integration provided). Thank's dixon, I took some tips from your patch;)
Also there are my patches for D7 comment.module and Views, see the link above to find references.

http://drupal.org/node/1546758

Screenshot please.

The order of those links - never made sense to me, but other than that its RTBC from a UX standpoint.

Looks good to me.

Sorry. My English dictionary (LEO) does not know the term "[to] unapprove".

There's "unapproved" (adjective), but that's not a verb, and seems to be rarely used either.

I'm OK with "disapprove", but want to point out that the entire "approve"-based terminology we're using here is completely misleading and inappropriate. As I just mentioned in #1616324: Add view display for admin/content/comment/approval:

The separate "Approval" page makes little sense in general. Even less, if you take into account that the entire "approval" state for comments is nothing else than published and unpublished in reality. Unpublishing a comment unexpectedly means to "disapprove" it, and makes it re-appear in the list of comments to approve (moderate). That's completely nuts and I've no idea why core chose to use a terminology that implies (moderation) business logic that doesn't exist.

I'd like to see a follow-up issue that renames both of those links into "publish" (ex. approve) and "unpublish" (ex. disapprove), because it is highly confusing for users to see already approved comments re-appearing in the approval queue after unpublishing them.

Updated #22 to use drupal_container().

I would wonder what disapprove does. Sun suggests in #19:

> I'd like to see a follow-up issue that renames both of those links into "publish" (ex. approve) and "unpublish"
> (ex. disapprove), because it is highly confusing for users to see already approved comments re-appearing
> in the approval queue after unpublishing them.

This issue seems RTBC again. The follow-up would be a nice improvement too.

I'm not sure I understand why we want to debate the names in a follow-up issue when it can be done now.

I think "publish" and "unpublish" make a lot more sense.

I wouldn't mind either. Lets see if we can incorporate those changes then.

Here's a patch that changes any comment approved/unapproved language to published/unpublished.

This meant changing the name of the "Skip comment approval" permission. The only decent name I came up with was "Automatically publish comments," but I don't like it. Every other permission name describes an action that a user is allowed/forbidden to take. "Automatically publish comments" doesn't match that pattern and could be confusing.

Thanks, @dcam! :)

Hm. I actually do not think we need to adjust the permission name/label in any way. The permission focuses on the interaction of a user that is posting a comment, and in that process/interaction, we're actually talking about an "approval" of a posted but initially unpublished comment.

What do you think?

Tested patch #30. Clicking on the new "unpublish" link does not unpublish the comment (comment still shows up in the "published comments" list (in admin/content/comment).

Also, using admin/content/comment to unpublish the comment does result in it being unpublished, but the now-unpublished comment does not show up in the unpublished comments list - so the list is empty, but the "Unpublished comments" counter shows a value of 1 (see attached screenshot).

changing status to "needs work"

Submitting patch after modifying #26 by changing the label and path as commented on #28
@attached(screenshot - comment-1266306-35.png, publish-unpublish-comment-1266306-35.png).

Submitting the modified test case, which were failing in the previous post.

I just tested patch #37 and it works as expected. Ready to RTBC?

I realize #19 only talked about using "Publish/Unpublish" terminology in the comment links, but I think if we don't standardize the terminology overall then we'll just be revisiting the issue in the future.

Before installing the patch, I created a comment and "approved" it.

After installing the patch in the latest code base, the "Unpublish" link appears, but it doesn't change anything when you click on it. If there is a new comment, the "Publish" link is likewise ineffective.

I wonder if something changed in related code?

I tried the patch from #37, but it failed to apply, both using git apply, and using the 'patch -p1' command.

I worked through the patch and applied it manually, and it worked as described above. I can now unpubish and publish comments from the commented node.

In response to #40 - the patch changed menu callbacks so I had to clear the cache before the 'unpublish' link would work.

I've re-rolled the patch, which seems to apply ok for me now.

I hope this helps. This has come out of a Drupal Ladder (http://drupalladder.org/) sprint, so if I've made a mistake, sorry!

#41: unpublished-comment-1266306-41.patch queued for re-testing.

#41: unpublished-comment-1266306-41.patch queued for re-testing.

#41 Doesn't pass the test. I have rewrite the code for the newest 8.x-dev version.

In other hand, I noticed that the internal paths for the administration items are using the Approval names instead the Published names. Also, not all the code has been updated. For example, the tests and some other paths,

Needs re-role. We're going to work on it.

yep. the most recent patch does not apply.

@techmsi did you have work on this you wanted to post? or any questions?

http://drupal.org/patch/reroll are the instructions, and I'm just going to add the tag for needs reroll.

If someone rerolls this and posts a new patch, just remove the needs reroll tag then. :)

working on a re-roll...

This task was larger than expected, please review!

I would say overall that looks quite reasonable @benjf! I guess per #28 that renaming unapprove to unpublish (and same with approve) is a part of this issue. Tagging for an issue summary update.

+++ b/core/modules/comment/comment.views.inc
@@ -229,17 +229,17 @@ function comment_views_data() {
+        'published-not-publlished' => array(t('Published'), t('Not Published')),

Small typo here: published-not-published.

thanks! fixed the typo

Status

rerolls dont usually need interdiff, but the change from 51 to 54 is a good case for an interdiff

For instructions on creating an interdiff, see https://drupal.org/documentation/git/interdiff | Microbranching workflow: http://xjm.drupalgardens.com/blog/interdiffs-how-make-them-and-why-they-...

Thanks! I only sort of understand interdiff's, but I'll read through it again.

Meanwhile, here is a new patch with the typo from #51 fixed.

Patch no longer applies and needs a reroll.

1. +++ b/core/modules/comment/comment.module
   @@ -896,12 +901,20 @@ function comment_links(Comment $comment, EntityInterface $node) {
   +        'query' => array('token' => drupal_get_token("comment/{$comment->id()}/unpublish")),
   

   drupal_get_token is deprecated

2. +++ b/core/modules/comment/lib/Drupal/comment/CommentAccessController.php
   @@ -35,10 +35,13 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A
   +        return user_access('administer comments', $account);
   

   user_access id Deprecated

Re-rolling the patch from #60.

+++ b/core/modules/comment/lib/Drupal/comment/Controller/CommentController.php
@@ -87,13 +87,13 @@ public static function create(ContainerInterface $container) {
+    if (!isset($token) || !drupal_valid_token($token, 'comment/' . $comment->id() . '/publish')) {

@@ -108,6 +108,31 @@ public function commentApprove(Request $request, CommentInterface $comment) {
+    if (!isset($token) || !drupal_valid_token($token, 'comment/' . $comment->id() . '/unpublish')) {

we can use $this->csrfToken->validate().

Don't know where the tag came from.

Patch #63 no longer applies.

37: unpublished-comment-1266306-37.patch queued for re-testing.

An updated patch that renames the administration paths, test methods, access operations, etc and adds an "unpublish link" Views field for consistency but continues to leave the user-facing text alone per #32.

This could be split into multiple issues but I've included a single patch per #28.

Rerolled the patch (CommentAccessControl.php changed to CommentAccessControlHandler.php) so it applies cleanly again. Cleared caches and it works as advertised:

* comment creation is unaffected
* comment "publish" link is visible with the new URL, comment/x/publish
* uses the newly renamed comment.publish route
* comment "unpublish" link is visible with URL comment/x/unpublish
* it does actually publish and unpublish.

This is a big patch though, so it could use some more eyeballs.

I thought it might be nice to actually attach the updated patch lol

Reviewing this now

Initial roll gave 7 rejects.
Re-rolled the patch. It gave 7 conflict messages, which where all resolved manually.
Needs a new review.
Removed Novice tag as it's quite a big patch.

This patch failed testbot because it couldn't install...

* CommentStorageInterface has public method updateEntityStatistics, so CommentStorage (which extends CommentStorageInterface) has to have that method, too. I added it as an empty function, because comments don't get to have comments on them. Not 100% sure if this is the right thing to do.

* It also calls undefined method ConfigEntityType::isFieldable a couple of times in comment.views.inc. This was removed in #2100343: Remove 'fieldable' key in entity definitions in favour of 'field_ui_base_route', so I replaced it with $entity_type->isSubclassOf('\Drupal\Core\Entity\ContentEntityInterface') as recommended in the change record (https://www.drupal.org/node/2346455).

It installs locally now. Submitting for testbot to give it a try.

Re-submitting because testbot is dying on too many MySQL connections... that's a testbot problem AFAIK. Someone correct me if I'm wrong!

Yay! Real test failures! :D I'll work on these...

So now it failed because of change record https://www.drupal.org/node/2337377 from Sept 17, introducing \Drupal\Core\Access\AccessResultInterface . I've got that addressed in a local copy, still working on testing errors in CommentAnonymousTest and CommentAdminTest. It looks like more issues with the same change. Will post with progress once I get it testing green!

Failed patching because of updates to CommentViewBuilder from #2348547: CommentViewBuilder should use static where possible, subclassing is currently too painful.

This patch has gotten enormous, and entails a renaming of the action from "approve" to "publish". That means that any time a commit affects Comment module, it risks breaking the patch. :|

@droplet - thanks for the re-test!

Can you post an interdiff please? It's hard to see the changes you made otherwise.

In stripping out the other code, we lose some of the backend instances of "approved" language that we earlier decided to change to "published". New patch here that adds some of those back.

Here's my thinking on the language:

* comments have a status of "published" or "unpublished"
* the human process of reviewing unpublished comments is "approval"
* The action taken to a comment during approval is "publishing". Comments which do not pass approval are left "unpublished".

I wanted to make it even more consistent, but there are some cases where the language just has to describe what the human is doing (approval). ie the message "Your comment has been queued for review by site administrators and will be published after approval." perfectly describes what is going on.

The most visible remaining use of "approval" language is in the permission "Skip comment approval". This still seems appropriate to me. The only alternative I can think of is "Publish comments on submission", which is more awkward IMO. Otherwise it's just in test comments, where we could actually use either term and accurately describe what's going on.

Is anyone aware of any contrib module adding such a functionallity for D8/D9?

Needs reroll of patch for 10.1.x version.

As seen that latest patch is not compatible with the current version 10.1.x so i reroll the patch for the current version and made it compatible with, also attaching the reroll_diff along.

I applied patch but it throws following error, I have attached screenshot for the same.
Fatal error: Class Drupal\comment\CommentStorage contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Drupal\comment\CommentStorageInterface::updateEntityStatistics) in /var/www/html/core/modules/comment/src/CommentStorage.php on line 26

Attached patch against Drupal 10.1.x and Address #121 Comment.
also Some deprecated functions have changed and been removed..

Removed whitespace errors and Re rolled patch with 11.x.