Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I have two problems with my upgrade at http://www.pennywit.com. First is that a number of comments have the body turn to the number 3. It looks like this happens when one of my users tries to edit his comments. The second is that in any nodes submitted before I upgraded, I don't see a count of the comments already submitted. I'm echoing this to the support forum ...
Comment | File | Size | Author |
---|---|---|---|
#61 | node_comment_stats_head2.patch | 11.92 KB | Junyor |
#47 | node_comment_stats_head.patch | 12.08 KB | Junyor |
#46 | node_comment_stats_4-5_0.patch | 11.64 KB | Anonymous (not verified) |
#42 | node_comment_stats_4-5.patch | 11.45 KB | Junyor |
#32 | node_comment_patch2-3.patch | 4.2 KB | Junyor |
Comments
Comment #1
pennywit CreditAttribution: pennywit commentedIt says this has been fixed ... what do I need to do on my side?
--|PW|--
Comment #2
Bèr Kessels CreditAttribution: Bèr Kessels commentedhttp://drupal.org/node/11316`was the bug. Update commment.module will fix this problem.
Comment #3
kps CreditAttribution: kps commentedThis problem remains (in 4.5.0-rc 1/2 hour before this post):
The {node_comment_statistics} table is evidently not initialized correctly.
Justification for "critical": Since most people who install 4.5.0 will be users of older releases, upgrading really ought to work before the release.
Comment #4
kps CreditAttribution: kps commentedAttached, raw and as-is, the script I used to initialize the {node_comment_statistics} table.
Comment #5
JonBob CreditAttribution: JonBob commentedComment #6
kps CreditAttribution: kps commentedMy above attachment (updc.php) is broken. This one is... better. Visiting this page should at least populate the node_comment_statistics table with something not entirely unreasonable.
Comment #7
Junyor CreditAttribution: Junyor commentedkps is right, the update won't correctly populate the table. Here's (part of) update_105:
"INSERT INTO {node_comment_statistics} (nid, cid, last_comment_timestamp, last_comment_name, last_comment_uid, comment_count) SELECT n.nid, 0, n.created, NULL, n.uid, 0 FROM {node} n"
So, it's setting the comment_timestamp = node creation time, forgetting the comment_creator, using the userid that created the node as the last_comment_uid, and setting the comment count to 0.
update_105 needs to be redone and I'd recommend a new update be created for 4.5.1 that will correctly initialize the node_comment_statistics table.
Comment #8
Junyor CreditAttribution: Junyor commentedOK, here's two patches to solve the problem.
Issues fixed:
- Fixes node_comment_statistics table prefixing for PostGreSQL
- Drops CID column from node_comment_statistics and supporting code in comment.module
- Initializes comments table with usernames (needed by node_comment_statistics table)
- Correctly initializing node_comments_statistics table
- Removed duplicate query for last_comment_name in node_comment_statistics query in comment.module
Needs checking:
- I couldn't test this with PostGreSQL
- I'm no database expert, so the updates.inc changes need to be gone over with a fine-toothed comb
- Do we need any special handling for comments with no 'name', i.e. truly anonymous comments
These patches are for the DRUPAL-4-5-0 branch.
Comment #9
Junyor CreditAttribution: Junyor commentedComment #10
Junyor CreditAttribution: Junyor commentedComment #11
Junyor CreditAttribution: Junyor commentedActually attaching patches. :)
Comment #12
Dries CreditAttribution: Dries commentedOtherwise these patches look fine. Good job.
Comment #13
Junyor CreditAttribution: Junyor commentedI don't know if it's necessary, but I'm just making sure it's consistent. Maybe the author of the node_comment_statistics patch could comment? I'm also concerned about having the name information for registered users outside of the users table.
I've tested this on a test site and I'll try testing on my production site once we have this worked out.
Comment #14
Dries CreditAttribution: Dries commentedI don't know whether the original author (ccourtne) is still around but it should be easy enough to test whether it is necessary or not. From what I've seen, it isn't necessary. In fact, your current patch might break things: like, what happens when somone changes his or her username? AFAIK, the old username would be shown in the comments.
Comment #15
Junyor CreditAttribution: Junyor commentedOK, I'll edit that bit and resubmit. You'd like this for HEAD only?
Comment #16
Junyor CreditAttribution: Junyor commentedNew patch for database files.
Comment #17
Junyor CreditAttribution: Junyor commentedNew patch for comment.module.
Comment #18
Junyor CreditAttribution: Junyor commentedPatches for head.
Comment #19
Junyor CreditAttribution: Junyor commentedComment #20
Dries CreditAttribution: Dries commentedI'd like to move forward with this patch and include it in Drupal 4.5.1. I can't reproduce this problem (it seems) so it would be much appreciated if those who can, can test it.
Comment #21
Junyor CreditAttribution: Junyor commentedIt applied and works well on my 4.5.0 site. A lot of nodes were listed as not having comments before, but they're working now. Dries alerted me to a drupal-support message regarding the patch and I'll look into that tomorrow.
Comment #22
crw CreditAttribution: crw commentedAfter applying the patch, approving new comments still does not update the node_comment_statistics table, therefore the comment_count field is off and the number of new comments does not show up on the main page of my site.
I'm currently troubleshooting this and will report my findings here.
Comment #23
crw CreditAttribution: crw commentedI should point out that I applied the following patch:
node-comment-statistics_head2.patch
And the problem described above still exists.
Comment #24
crw CreditAttribution: crw commentedOk, headway.
Looks like submitting a comment triggers an updating of node_comment_statistics, but editing a comment to publish/unpublish does not. Both should trigger the update, so I just need to find out why it isn't working for the 'update' case.
Comment #25
crw CreditAttribution: crw commentedOk, I found the problem and came up with a solution. I've been manually approving comments from anonymous users via the admin interface. comment_admin_edit() calls comment_save(), but neither calls _comment_update_node_statistics. I inserted the following two lines around line 952 of comment.module after the call to comment_save():
$nid = db_result(db_query('SELECT nid FROM {comments} WHERE cid = %d',$edit['cid']));
_comment_update_node_statistics($nid);
Ideally, there'd be tests for all these. comment_save should produce a return value, and comment_admin_edit() shouldn't go forward unless comment_save returns true.
Please excuse my lack of diff/patch-fu. :)
Comment #26
Junyor CreditAttribution: Junyor commentedBah, that function should call comment_save(). All comment changes should go through comment_save(). It would make life so much easier.
I'll try to come up with an updated patch soon, but probably not before this weekend.
Comment #27
Dries CreditAttribution: Dries commentedI agree that all comment editing should go through comment_save(), yet that will require a bit of refactoring. Also, there are two 'edit comment' forms (one for users, one for administrators) that should be merged, much like we merged 'node edit' forms. Anyone?
Comment #28
Junyor CreditAttribution: Junyor commentedDries: I see that you made a change to the updating of node_comment_statistics in CVS HEAD. Do I still need to add an updated patch? Are there plans for a 4.5.2 that could use this patch?
Comment #29
ax CreditAttribution: ax commentedi tried upgrading a 4.4 site to 4.5 yesterday and fell into the same trap as pennywit, kps, junyor, and others: the update for the node_comment_statistics table only updates forum comments and inserts num_comments 0 for all other node types. quite cheaty, this.
junyors 4.5 patches (node_comment_stats-2.patch, node_comment_stats2-2.patch) seem to solve the problem for me. at least, i see proper "replies" counts in tracker and node views now. i didn't try approving / publishing / unpublishing comments, though, nor did i check other issues mentioned in the thread (user names, postgresql, ...).
i applied the 2 patches to a 4.4 database / 4.5 code both before running update.php and afterwards. in the first case, there is a small glitch in that update.php throws an error:
user error: Unknown table 'node_comment_statistics'
DROP TABLE {node_comment_statistics}
because the patch removes "CREATE TABLE {node_comment_statistics}" from update_105. this could be catched by changing to "DROP TABLE IF EXISTS {node_comment_statistics}".
this is definitely a critical bug, and these patches should be applied to 4.5 as soon as possible, regardless of the "all comment editing should go through comment_save()" issue.
Comment #30
Jeremy CreditAttribution: Jeremy commentedI've just run into this same bug, trying to upgrade from 4.4 to 4.5. It's a show stopper for me. :( Ugh, so close.
Comment #31
Junyor CreditAttribution: Junyor commentedChanged patch for updates.inc based on feedback from ax. For 4.5.
Comment #32
Junyor CreditAttribution: Junyor commentedAnd an update for comment.module with additions to comment_save to update the node_comment_statistics table. This should cover all edit cases. For 4.5. Untested.
Comment #33
Jeremy CreditAttribution: Jeremy commentedI tested the upgrade process on KernelTrap -- worked perfectly nearest I could tell. I'm also running with your comment.module patch.
Comment #34
Junyor CreditAttribution: Junyor commentedWhat needs to be done for this to be included in core?
Comment #35
Jeremy CreditAttribution: Jeremy commentedFWIW: In the days following my upgrade, I've found numerous issues with comment statistics. Specifically, many old postings list "x new of x", but clicking "x new" shows that in fact none of them were new.
I have not figured out what's different about the nodes for which this is a problem compared to the nodes for which this isn't a problem. ie, it doesn't seem to be a problem with a specific node type, etc.
In other words: I retract my earlier comment that the upgrade went perfectly with this patch applied. Sorry I don't have more specifics to offer, perhaps I'll have more time at a future date to help dig into this more. In any case, the patch did improve the process noticeably.
Comment #36
Junyor CreditAttribution: Junyor commentedSo, none of the comments said they were new or you had read them previously despite them being marked new?
Comment #37
Jeremy CreditAttribution: Jeremy commented> or you had read them previously despite them being marked new?
Right. Comments I had already read when running 4.4 were suddenly marked as new after the upgrade to 4.5.
Comment #38
Junyor CreditAttribution: Junyor commentedThat sounds like a problem with history more than node_comment_statistics. I'm pretty sure this code doesn't do anything related to marking comments as new or old.
Comment #39
Dries CreditAttribution: Dries commentedIf possible provide a single patch against DRUPAL-4-5 and a second patch against HEAD. Looks like the patches are no longer in sync.
Comment #40
Junyor CreditAttribution: Junyor commentedI will do so soon, probably this weekend.
Comment #41
Jonathan Furness CreditAttribution: Jonathan Furness commentedAh.... that's why I am struggling with my upgrade from 4.4.2 to 4.5.1..... thank goodness I found this page.
Is there a case for building a script which looks for all the records in the comments table and updates the node_comment_statistics table... for those of us who are now working in 4.5.1 with inaccurate records in node_comment_statistics table?
Looking forward to the fix....
Jonathan
==========
Jonathan Furness
www.jonathansblog.net
Comment #42
Junyor CreditAttribution: Junyor commentedUpdating patch to current 4.5.1.
Jonathan: Just apply this patch to your installation (from the main Drupal directory, type 'patch -p0 -u < ../node_comment_stats_4-5.patch') and run update.php.
Comment #43
Junyor CreditAttribution: Junyor commentedDries: The comment.module in HEAD makes use of the cid column in node_comment_statistics. My patches were removing that column, as it wasn't being used. Should I leave it alone afterall?
Comment #44
Dries CreditAttribution: Dries commentedThe column seems to get populated though (it's not empty). If it the data is not used, you can remove it.
Comment #45
(not verified) CreditAttribution: commentedWell, cid wasn't used until the change you made in revision 1.314 of comment.module. It was never set correctly or used anywhere in the code, so I simply removed it.
Comment #46
(not verified) CreditAttribution: commentedSlightly updated patch for 4.5.1.
Comment #47
Junyor CreditAttribution: Junyor commentedAnd patch for HEAD (with all CID stuff removed).
Comment #48
Junyor CreditAttribution: Junyor commentedDries: Is there anything else you need here before its commitable?
Comment #49
ax CreditAttribution: ax commentedi too would much appreciate if this *release* *critical* bug could be fixed soon. thanks a lot!
Comment #50
Dries CreditAttribution: Dries commentedI look into this patch tomorrow (err, later today).
Comment #51
Dries CreditAttribution: Dries commentedI committed the patch to DRUPAL-4-5. I'm putting the HEAD version on hold until the revisions patch landed.
Please upgrade your Drupal 4.5 sites to DRUPAL-4-5 in preparation of Drupal 4.5.2. Thanks.
Comment #52
rooey CreditAttribution: rooey commentedI hate to be a total baboon - but i'm still seeing a problem after upgrading to 4.5.2 - the "poll" block has no comment options available (stats or otherwise).
Have i missed something? (or are these patches not included in the 4.5.2 release?)
Thanks!
Comment #53
Junyor CreditAttribution: Junyor commentedThey are included in 4.5.2. Did you run the update script (it has to be run despite what the Drupal 4.5.2 announcement says)?
Comment #54
rooey CreditAttribution: rooey commentedYeah... I did :s
(assuming you're talkin' about update.php)
Comment #55
Junyor CreditAttribution: Junyor commentedI'm not entirely sure what this fix has to do with the poll block, as I don't use that. This fix is to display the correct number of comments for nodes.
Could you go into some more details about the problem you're having?
Comment #56
Jeremy CreditAttribution: Jeremy commentedI'm having a similar problem. Note that if you look at the poll in a node view, the comments show up:
http://kerneltrap.org/node/4536
But if you look at the poll on the front page, it doesn't list any comments:
http://kerneltrap.org/
I've not taken any time to try and track this down yet.
Comment #57
rooey CreditAttribution: rooey commentedSure... Since the upgrade from 4.4 to 4.5 the poll block lost all ability to play with comments.
For example, there used to be the standard "add new comment" links etc... Now there is not :(
The poll module itself *seems* to be calling the various bits of code it's supposed to, but alas, nothing is happening.
Take a look at the site if it helps: http://www.lineageone.com
I have jury-rigged the "quote" module to at least get people able to comment on stuff for now.
Comment #58
Morbus IffRegarding the poll/comment issue, see also: http://drupal.org/node/14936. Not sure if that bug should be closed and wrapped into this Issue, or if we should move over to there as a different issue.
Comment #59
rooey CreditAttribution: rooey commentedThanks :)
Dang - and they described it so much better than me too!
How do you want to proceed?
Comment #60
Junyor CreditAttribution: Junyor commentedLet's take discussions to that bug report. I'll take a quick look in a couple hours to see if there's anything glaringly wrong.
Comment #61
Junyor CreditAttribution: Junyor commentedSynched patch with HEAD.
Comment #62
Dries CreditAttribution: Dries commentedCommitted to HEAD.
Comment #63
(not verified) CreditAttribution: commentedComment #64
Dries CreditAttribution: Dries commentedThe upgrade did not work on drupal.org! Marking this both 'active' and 'critical' again. The code does too many queries, and th comment_count is always 1.
Comment #65
Junyor CreditAttribution: Junyor commentedDries: It looks like you already made the needed changes. Is there anything else to update?
Comment #66
ax CreditAttribution: ax commentedthis was a *4.5* bug originally. so if we don't want to have a broken 4.5, dries' latest patch should be applied to the 4.5 branch, too. looking at the cvs log, this hasn't happened yet.
Comment #67
ax CreditAttribution: ax commenteddoes anyone care to fix this *critical* bug (== apply Dries' HEAD fix) in 4.5?
maybe setting status to patch will help.
Comment #68
Junyor CreditAttribution: Junyor commentedThere's a further problem that needs to be addressed, too. If the comment name has a single quote, the update will throw SQL errors.
Comment #69
Junyor CreditAttribution: Junyor commentedThe problem I mentioned in my last comment is http://drupal.org/node/19432.
I've looked at the change that Dries did and I honestly can't figure out why the change was made. I'd be happy to provide a patch for 4.5.x if I could figure out the problem. If anything, I think the query Dries is doing is returning too many matches, not the query I was doing.
Comment #70
Junyor CreditAttribution: Junyor commentedLooks like the old query was getting the oldest comment, not the newest comment. I'll try to look at this a bit more this weekend.
Comment #71
ax CreditAttribution: ax commentedany updates on this (critical bug)?
Comment #72
Junyor CreditAttribution: Junyor commentedI haven't been able to work on it.
Comment #73
Richard Archer CreditAttribution: Richard Archer commentedThis was committed to HEAD in February and seems to be working fine still.
I'm closing this issue because ports of patches to 4.5.0 seem pretty pointless.