When comment_save() is used to create a comment, it always sets the status to match the user's "post comments without approval" permission:

          $status = user_access('post comments without approval') ? COMMENT_PUBLISHED : COMMENT_NOT_PUBLISHED;

However, if you want to use a module like mailhandler to override this setting so that the status is always moderated in a given situation (such as, I want moderates to clean up the garbage sending a comment by email adds, etc.), this line of code won't let you. The work around is to perform your save twice:

// Setup $edit...
$edit['status'] = COMMENT_NOT_PUBLISHED;
$cid = comment_save($edit);

if ($cid) {
    $edit['cid'] = $cid;
    comment_save($edit);
}
else {
    // Bad stuff...
}

The second save will set the status as desired. However, this is kind of gross.

Rather, I propose allowing the initial $edit to explicitly request moderation. Thus, you could do the above with only a single call to comment_save(), but you're still not permitted to request publishing when you don't have such permission. I've attached the patch for doing this.

I think it would be really nice if this made it into 6.x.

CommentFileSizeAuthor
#8 comment-status-in-form_0.patch3.44 KBzostay
#5 comment-status-in-form.patch3.42 KBzostay
comment-moderation-request.patch897 byteszostay

Comments

zostay’s picture

zostay commented
Status: Active » Needs review
moshe weitzman’s picture

moshe weitzman commented

this does not belong in comment save at all. the moderation should be a #value field on the comment form and modules can then form_alter it as needed.

zostay’s picture

zostay commented

Can you elaborate? I'm not sure how hook_form_alter() helps since comment_save() is directly invoked here. I could also just drop the data straight into the comment table without using comment_save() at all.

It seems that if there's an API for doing it, I'd be better off doing that in case the process of entering a comment should change in the future (e.g., an additional hook call is added or something). I think creating a implementation for hook_comment() would be more applicable in this case.

I still don't see why forcing moderation on a comment during the INSERT is a bad thing. Any other solution I can think of means there's a chance that someone could hit the page and see the published comment during the few milliseconds between the initial insert and update. Whereas having the ability to force moderation means this is never the possibility and peforms better to boot.

zostay’s picture

zostay commented

Nevermind, I misunderstood. I now see what saying. I'm dumb. :)

I'll submit a patch using your idea as well.

zostay’s picture

zostay commented
StatusFileSize
new comment-status-in-form.patch3.42 KB

Here's a patch that moves the decision into comment_form(), which allows another module to modify the default value via form_alter() and allows the decision to made explicitly without interference in comment_save().

This should perform actions as per moshe weitzman's comment.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Needs work

yes, this is wat i was describing. thanks.

i think the form could be simplified further. we can tead admins same as everyone else. so, how about this:


  if ($edit['cid']) {
    $status = (bool)$edit['status'];
  }
  else {
    $status = user_access('post comments without approval') ? COMMENT_PUBLISHED : COMMENT_NOT_PUBLISHED;
  }
  $form['status'] = array('#type' => 'value', '#value' =>  $status);
zostay’s picture

zostay commented

Two comments:

  1. $comment->status is not a boolean. I know that it is effectively one now, but COMMENT_PUBLISHED and COMMENT_UNPUBLISHED are definitely integers and opposite the normal values one would expect from $node->status. Therefore, I'm not sure if casting to (bool) is appropriate.
  2. If a user already has "administer comments", the status is provided in $edit['status'] via the $form['admin']['status'] radio button. Thus, I don't know that adding status a second time as a "#value" in the same form is the correct treatment.
zostay’s picture

zostay commented
StatusFileSize
new comment-status-in-form_0.patch3.44 KB

Regarding (2) in my previous comment. The test does miss the case where !$edit['cid'], so I added the opposite of the admin guard from earlier:

if ($edit['cid'] && user_access('administer comments')) {

into the patch:

if (!$edit['cid'] || !user_access('administer comments')) {
zostay’s picture

zostay commented
Status: Needs work » Needs review
zostay’s picture

zostay commented

This patch still works on Drupal 5.2, though hunks #4 and #5 are offset.

catch’s picture

catch
he/him
Primary language English
commented
Version: 6.x-dev » 7.x-dev
Status: Needs review » Needs work

No longer applies, bumping to D7.

moshe weitzman’s picture

moshe weitzman commented

Still a worthwhile patch.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Closed (duplicate)

http://api.drupal.org/api/function/comment_form/7 was already streamlined and 'status' is now always contained in the form.