[D7] Unpublish own comment

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Your git command is not in the correct format.

You cannot use a command in the format
username@git.drupal.org:sandbox/username/repo

It must be in the format
http://git.drupal.org/sandbox/username/repo

Please put pareview.sh link into this issue description: http://pareview.sh/pareview/httpgitdrupalorgsandboxalvar0hurtad02303353git

You've a message: You need to set a default branch 7.x-1.x http://drupal.org/node/1659588

Please chech 3 other project manual reviews.

Check review bonus

In unpublish_own_comment.module:
On line 17, permissions should generally be entirely lowercase, declarative and related to their title. 'unpublish own comment' would make sense.

The title and description could afford to be more verbose, such as:

(Unpublish Own Comment) and (Allows the user to unpublish comments they have made.)

I'm not sure what the purpose of this is:

/**
 * Implements hook_menu_alter().
 */
function unpublish_own_comment_menu_alter(&$items) {
  $items['node/%/unpublish_own_comment']['access callback'] = 'unpublish_own_comment_access';
  $items['node/%/unpublish_own_comment']['access arguments'] = array(1);
}

See @gwprod review https://www.drupal.org/node/2303363#comment-8971085

The words 'publish' and 'unpublish' should be wrapped as translatable here (lines 22 and 26):

  if ($comment->status) {
    $action = 'unpublish';
    $form['action']['#value'] = 0;
  }
  else {
    $action = 'publish';
    $form['action']['#value'] = 1;
  }

Similarly, you should probably make "available" and "unavailable" translatable as well on line 41:

  t(
      'Submitting this form will @action this comment, making it @visibility to users on the site.',
      array('@action' => $action, '@visibility' => ($comment->status ? 'unavailable' : 'available'))
    ),

Finally, this translation call on line 43 does not do anything as you are just passing a variable

t('@action', array('@action' => ucwords($action)))

Hi @alvar0hurtad0

If user is authenticated and this user has a "Unpublish Own Content" permission, this user don't see link to unpublish

When push unpublish, using user 1, take this error:

Notice: Undefined index: action in unpublish_own_comment_form_submit() (line 40 of /Proyectos/drupal7/sites/all/modules/custom/unpublish_own_comment/unpublish_own_comment.pages.inc).

I'm using clear drupal 7.29 installation

Please review it.

Hi,

I do a patch for this error:

If user is authenticated and this user has a "Unpublish Own Content" permission, this user don't see link to unpublish

Review second error ;)

Hi Alvar0Hurtad0.

You need add manually tag "PAReview: review bonus" into the main issue ;)

Good work!

Manual review

Application contains a repository and project page link.

Project is not a duplication.

I did find a patch from two years ago to add this functionality to Comment goodness. Without the patch, Comment goodness has functionality to allow user to delete user's own comment. But it also makes sense to have this module separate from anything else. The module name is very indicative of what it does and will certainly be easy to find.

Repository contains code.

Repository has a single version-specific branch.

No security issues.

Licensing is good.

No third-party code.

Readme file contains typos.

You have a README.txt, and it's detailed enough, but I saw a few typos and bad wordings:
You'll se the button if -> You'll see the button if
You can admin comments -> You can administer comments
This module is very useful if you need some role must be able to unpublish own
comments but not edit or delete then ->
This module is very useful if you need some role that must be able to unpublish own
comments but not edit or delete them

PAReview shows no problems.

Using Drupal's API correctly.

Doesn't actually work

This looks to be a mistake in unpublish_own_comment_form_submit()

$comment->status = $form_state['values']['action'];

Notice: Undefined index: action in unpublish_own_comment_form_submit() (line 40 of ../unpublish_own_comment.pages.inc).
At this point all must be validated, and access checks should have been done, and you can just set status directly:

$comment->status = COMMENT_NOT_PUBLISHED;

Confusing code in access callback.

unpublish_own_comment_access() has 5 returns. I would restructure it
To initialize $access on top: $access = FALSE
To return access at the bottom: return $access;
To have nested if statements in between:

  $access = FALSE;

  if ($node && $comment && $comment->status... ) {

    if (user_access('... ) {
      $access = TRUE;
    }

  }

  return $access;

Also, it looks to me like you don't sufficiently check for comment existing and being published. I think comment must absolutely exist and be published in order for the access check to return TRUE, regardless of permissions.

Additional suggestion

With this module, you allow the user to unpublish the comment, even if comments are threaded and there is already an answer to that comment. This would make the thread incoherent, and you should maybe consider this. Comment goodness allows to delete his own comment, but only if there are no answers to that comment already.

@alvar0hurtad0
Your module looks good to me as well. As I reviewed your code and also installed on my local machine.
Additional suggestion
Please add hook_help function to your .module file.

Works fine!

Assigning to myself for next review.

Automated Review

Review of the 7.x-1.x branch (commit 298135f):

• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity (but barely).

Secure code

No. If "no", list security issues identified.

You have a node access bypass problem. If a user has commented on a node, and then for some reason loses read access to the node afterwards, they can
still directly browse to the url defined by your hook_menu and unpublish the comment. You also need to add a
node_access('view', $node) to unpublish_own_comment_access() to make sure that the user has read access to the node on which
they wish to unpublish the comment. Note that the autoloader that gives you the $node doesn't do the check; it is the router
item's responsibility to do this. I also think that you need to check whether comments on the node are hidden or not; users
shouldn't be able to unpublish comments on a node where they are hidden.

Coding style & Drupal API usage

The actual permission name in unpublish_own_comment_permission() is a little misleading. It should be more tied to the action, and not what you can see. I would just use 'unpublish own comment'.

In unpublish_own_comment_comment_view(), use theCOMMENT_PUBLISHED instead of the values. See https://api.drupal.org/api/drupal/modules!comment!comment.module/7

In unpublish_own_comment_comment_view(), do you need an additional class to identify this link? Check the other links to see what they have.

(+) unpublish_own_comment_access() needs to check whether the comment is valid or not. The normal checks are isset($node->nid) and isset($comment->cid).

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

@alvar0hurtad0

The unpublish_own_comment_access() access callback should check $comment->status === COMMENT_PUBLISHED. It is the only problem from my comment #20 that you didn't address. Everything else looks good. I will go ahead and set this one as RTBC.

Review of the 7.x-1.x branch (commit 5e24b42):

• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. project page misses a screenshot and is a bit short, see https://www.drupal.org/node/997024
2. unpublish_own_comment_access(): why is the $comment parameter optional with a NULL default value? You will always need it, otherwise the function does not make sense?
3. unpublish_own_comment_access(): I don't think you need the $access variable. Just immediately return TRUE when you check the permission and return FALSE as default in the end. I think this has been suggested by another reviewer, but having multiple return statements is perfectly fine in a function.
4. We usually don't use @author tags in Drupal since over time many people will contribute to a project.
5. unpublish_own_comment_comment_view(): the second if() is not necessary, just add to the comment content in the first if() body?

But that are not critical application blockers, so ...

Thanks for your contribution, alvar0hurtad0!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.