API page: http://api.drupal.org/api/drupal/modules--comment--comment.module/functi...

Describe the problem you have found:

It appears this API has broken code. Is this function working as intended?

The db_update is encapsulated inside the ELSE statement, and won't fire if the IF conditional is true. Shouldn't the code for comment_unpublish_action be:

function comment_unpublish_action($comment, $context = array()) {
  if (isset($comment->subject)) {
    $subject = $comment->subject;
    $comment->status = COMMENT_NOT_PUBLISHED;
  }
  else {
    $cid = $context['cid'];
    $subject = db_query('SELECT subject FROM {comment} WHERE cid = :cid', array(':cid' => $cid))->fetchField();
  }
  db_update('comment')
    ->fields(array('status' => COMMENT_NOT_PUBLISHED))
    ->condition('cid', $cid)
    ->execute();
  watchdog('action', 'Unpublished comment %subject.', array('%subject' => $subject));
}

(moving the dp_update outside the IF-ELSE-END block).

This fix would appear to replicate D6 behavior.

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Active » Closed (duplicate)

I believe this is a duplicate of #244093: Node and comment actions are (still) completely broken and have broken tests too. Some help there with the tests would be awesome!

jmoyles’s picture

jmoyles commented

Thinking this is a different issue - the failure, patch, and test conditions in #244093 appear to be pointed at a different issue and area entirely - a more formative issue about function actions_function_lookup(), presave/save states, and what should be returned in those situations.

This one is about comment_unpublish_action(). Here the issue is a much simpler one - the end "}" of the IF-ELSE-END block appears to be in the wrong place. The D6 flavor of the API has it right, the D7 does not appear to.

I'm new to the community, and don't want to step on any toes, so I'll leave this one set as "closed (duplicate)" for a reasonable time until more experienced members let me know what is up.

jmoyles’s picture

jmoyles commented
Status: Closed (duplicate) » Active

No reply, moving back to active. See comment #2 for why I believe this is NOT a duplicate issue (at least to the one mentioned).

Again, I'm new to the community, and don't want to step on any toes, so if this indeed a duplicate; or if I'm violating some other protocol, please let me know specifically what I did wrong so I don't repeat the mistake.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Closed (duplicate)

Sorry this really is a duplicate, we should fix these actions in that issue along with better tests. It's be good if you could copy your thoughts over there and like webchick says help with that bug report is welcome. Just a note once an issue is marked duplicate it won't be found in default listings, hence no response from others when in that state.