1. run D7 update.php on a Drupal 6 site containing comments and with the "access comments" enabled for anonymous users (you can use the database dump in #1). the update runs smoothly.
2. go to your brand new D7 site and note how the comments do not appear for the anonymous user, despite the fact that the "view comments" permission is checked for anonymous user.
This behavior is not specific to the anonymous user, but to any role which has the "access/view comments" and is intended to view the comments. Granting the permission "Administer comments and comment settings" shows the comments.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 891250_8_fix_comment_update_7001.patch | 912 bytes | scor |
| #4 | 891250_4_fix_comment_update_7001.patch | 666 bytes | scor |
| #1 | 891250_d619_view_comments_dump.sql_.tar_.gz | 72.91 KB | scor |
Comments
Comment #1
scor commentedattaching a Drupal 6.19 db dump containing a few comments and "access comments" for anonymous user, which you can use to reproduce this bug (login as uid 1 with admin/admin).
Comment #2
aspilicious commentedThere is a patch changing the names of the permissions in drupal 7 cause they are confusing.
This is not a critical bug (if I find the issue)
EDIT: I was wrong, issue was about posting comments.
Comment #3
scor commentedlooking into this.
Comment #4
scor commentedThe bug was in comment_update_7001() which sets comment.status of all records to 3. The k/v pairs of $changes need to be swapped.
Comment #5
jody lynnI see that comment status codes were swapped between 0 and 1 for unpublished, but I don't understand what a comment status of 3 represents.
Comment #6
sunWeird. Also no idea what 3 could be. I only know of switched COMMENT_PUBLISHED and COMMENT_NOT_PUBLISHED constants, and therefore $comment->status (column values).
So 0 <=> 1 would be sufficient.
Comment #7
jody lynnFound the back story in CVS.
In #237636: Comment status field should match node status field the 3 was introduced, presumably to avoid a race condition during the update. Then in #570900: Destroy remnants of update_sql() Crell refactored the update and the keys ended up reversed.
So I think scor's patch is exactly right, except I'm not sure if we still are at risk of race conditions here. If not, we should remove the use of '3', as we never know if someone has a custom module that uses a status of 3 for their own purpose, so not doing that would be preferable.
Comment #8
scor commentedUsing anything different from 0 and 1 as status means you lose the core comment functionality (core will not know what to do with such status value and will ignore these comments). Anyways, I've changed the patch to account for that by ensuring we choose a temporary status which is different from the existing status values.
Comment #9
bjaspan commentedWith this plus #890210: Cannot enable Comment module after a Drupal 6 to Drupal 7 upgrade, I can upgrade the test database from D6 to D7, enable the comment module, and see the comments in all the right places. RTBC.
I can't mark the other issue RTBC since it is my patch but hopefully it will go in soon. :-)
Comment #10
bjaspan commentedBy RTBC, I meant "RTBC". :-)
Comment #11
webchickCommitted to HEAD! Thanks!