I found two bugs in comment_publish_action() and comment_unpublish_action() function in the comment.module as you can see below.

function comment_publish_action($comment, $context = array()) {

    // $subject = db_query('SELECT subject FROM {comment} WHERE cid = :cid', array(':cid', $cid))->fetchField(); // NG
    $subject = db_query('SELECT subject FROM {comment} WHERE cid = :cid', array(':cid' => $cid))->fetchField();

function comment_unpublish_action($comment, $context = array()) {

    // $subject = db_query('SELECT subject FROM {comment} WHERE cid = :cid', array(':cid', $cid))->fetchField(); // NG
    $subject = db_query('SELECT subject FROM {comment} WHERE cid = :cid', array(':cid' => $cid))->fetchField();
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pixture’s picture

Version: 7.0-beta3 » 7.0-rc1

Still not fixed with RC1.

Damien Tournoud’s picture

Title: Bugs in comment_publish_action() and comment_unpublish_action() » Comment publish / unpublish actions are broken
Version: 7.0-rc1 » 7.x-dev
Priority: Normal » Critical
Status: Active » Needs review
FileSize
4.26 KB

Actually, both publish and unpublish actions are completely broken. Raising to critical.

Damien Tournoud’s picture

Added proper documentation blocks.

carlos8f’s picture

carlos8f’s picture

Looks like a simple DBTNG muckup, except:

+++ modules/comment/comment.module
@@ -2500,13 +2500,13 @@ function comment_action_info() {
-  if (isset($comment->comment)) {
+  if (isset($comment->subject)) {

Can you explain the reasoning behind this?

BTW, another test that could use #652394: Aggressive watchdog message assertion, which is unfortunately D8 now.

catch’s picture

$comment->comment can never exist - it's now a field. Subject should be set even if it's empty iirc. Looks good to me.

moshe weitzman’s picture

Priority: Critical » Major
Status: Needs review » Reviewed & tested by the community

Oh come on. It is just a bug. But I'll meet you half way. and RTBC.

Damien Tournoud’s picture

Those two actions don't work. At all. Period. That's the definition of a critical bug.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

grugnir’s picture

Nice patch. So just what does one DO with it?? I've been searching for hours and have found NOTHING in plain English (or any other human language) that is of any practical use to an average person, in any of the literally dozens of threads I've waded through. I'm not dumb as a box of rocks, but neither am I a complete geek.

Surely there must be SOME simple (or somewhat simple, at least) method for applying patches?????? Even WordPress has that one pegged, for cryin' out loud...

carlos8f’s picture

@grugnir, the patch is already part of D7 (issue status "fixed" back in December), so you don't have to do anything with it :) But if you're running 7.0 stable (or newer) and are still noticing this bug, please speak up.

grugnir’s picture

Okay, here's my pickle:

I created an action, "Unpublish comment containing keyword(s)," because I was getting spammed up the pooper. I assigned the action to the "When either saving a new comment or updating an existing comment" trigger. So when I try to submit a comment, it barfs up "Fatal error: Cannot use object of type stdClass as array in blahblahblah/includes/common.inc on line 5564" at me. Logged in or not makes no difference.

I upgraded from 7.0 to 7.2, thinking that would unbugger it, but no dice. Unassign the action from the trigger, and everything's jolly. I'll leave the trigger enabled for now.

The crunchy stuff:
Site: http://www.gunownersresource.com
Shared hosting (cpanel)
Linux CentOS
PHP: 5.2.17
MySQL: 5.1.52
Keywords in the action, exactly as they appear (in case that means anything):

amoxicillin, cialis, clonazepam, dapoxetine, hydrocodone, klonopin, levitra, lorazepam, misoprostol, oxycodone, oxycontin, percocet, phentermine, priligy, tramadol, viagra, xanax, zithromax, zolpidem, airmaxwinkel.nl, bluetoothheadphonesstore.info, brand-sneakers.net, febdian.net, fivefingershoesgo.com, lambofgodlutheran.com, mbtcrown.com, tibetanjewelry1.com, topmbtstore.com, trendy-jewelry1.com

... and it'll not doubt get bigger. Also running CKEditor 7.x-1.1 (what diff that could make, I have no idea, but there it is anyway).

I ran my site for years on Joomla with no real issues and, long story, decided to give Drupal a go. Got everything almost ready to go live with 6.whatever (can't even remember now), when a friend told me "uh, you know they're up to 7 now, right?" So I figured, screw it, rather than have to play catch-up later...

And now here I am. I'm gonna go get drunk now.

HansVdb’s picture

Version: 7.x-dev » 7.8
Status: Closed (fixed) » Active

I confirm grugnir's post #13 above. I have exactly the same error message in D7.8 in similar circumstances.
This issue should be re-opened.

catch’s picture

Status: Active » Closed (fixed)