When a user with appropriate rights, tries to edit an anonymous comment, the following PDOexception is thrown:
SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'uid' cannot be null in comment_save() (line 739 of /var/www/home/d7/modules/comment/comment.module).

Problem lies in the fact that $edit['uid'] is NULL when an anonymous comment is edited, which is not allowed by the current database scheme scheme. This can be fixed by forcing it to 0 when it is NULL.

Patch attached does that, and also adds a test for editing anonymous comments, because apparently that was missing.

CommentFileSizeAuthor
#10 334826-comment-form-uid-D6-rev2.patch1.58 KBbrianV
#7 334826-comment-form-uid-D6.patch2.16 KBDave Reid
#2 334826_broken_anonymous_comment_editing.patch2.32 KBmaartenvg
edit_anonymous_comments.patch1.64 KBmaartenvg
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented
Status: Needs review » Needs work

Please see the similar problem in taxonomy: #332145: UNSTABLE-3 blocker: taxonomy_form_term_submit passes empty string as parent [dbtng conversion regression]

Looking at your patch, I'm wondering if it is possible to make the same improvement as in the other one, namely making sure the uid is 0 from the start rather than filtering it through empty() later on. If it isn't, I apologize (just set back to CNR).

maartenvg’s picture

maartenvg CreditAttribution: maartenvg commented
Status: Needs work » Needs review
FileSize
334826_broken_anonymous_comment_editing.patch2.32 KB

You mean something like this?

This changes the default value of the FAPI variable to 0 instead of NULL. Unrelated to this bug but a consequence of the way this is solved, the empty()-check in comment_save() on inserting a comment is dropped.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs review » Reviewed & tested by the community

Patch looks good, and the test runs well. I wonder how many more of these are lurking.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Long live tests! :)

Status: Fixed » Closed (fixed)

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

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Version: 7.x-dev » 6.x-dev
Assigned: Unassigned » Dave Reid
Status: Closed (fixed) » Patch (to be ported)

I ran into this bug today with 6.x. Will post a patch shortly.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Patch (to be ported) » Needs review
FileSize
334826-comment-form-uid-D6.patch2.16 KB
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

I found this in 6.x while using an override of preprocess_user_picture, which is called during preview in editing a comment. Before the patch, the $variables['account'] that is passed to template_preprocess_user_picture is the following:

stdClass::__set_state(array(
   'name' => 'Anonymous',
   'mail' => 'anonymous@example.com',
   'homepage' => 'http://example.com/',
   'date' => '2008-11-14 18:03 -0500',
   'status' => '1',
   'is_anonymous' => true,
   'subject' => 'Test subject',
   'comment' => '

Test comment

',
   'format' => '1',
   'cid' => '3',
   'pid' => NULL,
   'nid' => '3531',
   'uid' => NULL,
   'timestamp' => 1226703780,
   'new' => 0,
   'signature' => '',
))

And after the patch:

stdClass::__set_state(array(
   'name' => 'Anonymous',
   'mail' => 'anonymous@example.com',
   'homepage' => 'http://example.com/',
   'date' => '2008-11-14 18:03 -0500',
   'status' => '1',
   'is_anonymous' => true,
   'subject' => 'Test subject',
   'comment' => '

Test comment

',
   'format' => '1',
   'cid' => '3',
   'pid' => NULL,
   'nid' => '3531',
   'uid' => 0,
   'timestamp' => 1226703780,
   'new' => 0,
   'signature' => '',
))

...which the above matches the same output in current D7 HEAD:

stdClass::__set_state(array(
   'name' => 'Anonymous',
   'mail' => 'anonymous@example.com',
   'homepage' => 'http://example.com/',
   'date' => '2008-12-20 23:07 -0600',
   'status' => '1',
   'is_anonymous' => true,
   'subject' => 'Test subject',
   'comment' => '

Test comment

',
   'comment_format' => '1',
   'cid' => '1',
   'pid' => NULL,
   'nid' => '41',
   'uid' => 0,
   'timestamp' => 1229836020,
   'format' => '1',
   'new' => 0,
   'signature' => '',
))
brianV’s picture

brianV CreditAttribution: brianV commented
Status: Needs review » Reviewed & tested by the community

Looks good, RTBC.

brianV’s picture

brianV CreditAttribution: brianV commented
FileSize
334826-comment-form-uid-D6-rev2.patch1.58 KB

Err, somehow overlooked that the final hunk (whitespace adjustment) didn't apply. Rerolled for D6, and tested appropriately.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Fixed

Committed to Drupal 6, thank you!

Status: Fixed » Closed (fixed)

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