Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
I have a forum topic ( namely http://treehouse/go/en/node/74 ) which shows up if you go to it directly.
If you go to the "Next" forum topic, it shows up as the Previous link.
If you go to the Previous forum topic, it shows up at the Next link.
However, if you go up to the correct page of the containing forum, both its siblings appear, but it does not.
Comment | File | Size | Author |
---|---|---|---|
#8 | forum_deleted_user.patch | 1.66 KB | greggles |
Comments
Comment #1
Wesley Tanaka CreditAttribution: Wesley Tanaka commentedSorry. If any of you are "following along from home," I meant
http://treehouse.ofb.net/go/en/node/74
Comment #2
Wesley Tanaka CreditAttribution: Wesley Tanaka commentedIt's being excluded with this clause:
l.last_comment_uid = cu.uid
Comment #3
Wesley Tanaka CreditAttribution: Wesley Tanaka commentedChanged the title to reflect the problem.
I assume it's possible to reproduce this by:
0. creating a test user
1. creating a comment on a forum topic (while logged in as that user)
2. deleting the user
3. going to the forum which contains that topic.
I'd suggest removing the inner join on
{users} cu
and replacing it with a left join on{users} cu
. I'm already maintaining a patch on that (really long) line of code, else I'd make a patch right now.Comment #4
Wesley Tanaka CreditAttribution: Wesley Tanaka commentedThe line in question being line 694 of forum.module (v1.312)
Comment #5
beginner CreditAttribution: beginner commentedHello wtanaka ,
Did you change anything or update to latest HEAD recently?
I just checked your forum, and the said thread DOES appear in the forum index
(currently at the very bottom of http://treehouse.ofb.net/go/en/forum/main/travel?page=1 ).
Can we consider this fixed?
b.
Comment #6
Wesley Tanaka CreditAttribution: Wesley Tanaka commentedSince originally posting this issue, I manually updated the value of the last_comment_uid column for the affected rows in my database. The fix suggested in #3 still, to my knowledge, has not been applied, so I assume this issue (also described by #3) still exists.
Thus, it would be incorrect to consider this fixed.
Comment #7
greggleswtanaka's solution takes this from being a problem where forum posts mysteriously disappear to a state where forum posts mysterial report more replies than actually exist. This seems better, but not complete.
I've attached his proposal as a patch. I think that the approach in the attached patch lacks in that it doesn't reset the comment count. However, there is a PHP snippet which takes care of that situation:
http://drupal.org/node/43528
Ideally, I think that the user delete process could be improved, but that is a whole different issue (literally, there are many issues on that topic).
Comment #8
gregglesForgot the patch...
Comment #9
dopry CreditAttribution: dopry commentedThis patch doesn't seem to get the job done...
I enabled forums, created a forum container, and a forum..
I made a two posts as uid=0, and a post as 'test user'. I commented on 1 of the forums topics posted by uid=0 as test user. I deleted 'test user'.
What I observed before the patch. From the forum/$tid page I could not see the post made by the test user, nor the post the test user had commented on. From the forum topic page... I saw the link to the forum topic which the test user had posted, but got an access denied error when trying to open it. I could not see a link to the post the 'test user' had commented on.
After patching....
From the forum page.. forum/tid
I can see the forum topic that the test user commented
I cannot see the forum topic the 'test user' posted
From the forum topic page.. node/nid
I can see the link to the forum topic the 'test user' created but get a drupal_access_denied.
I cannot see the link to the forum topic the 'test user' commented on.
Comment #10
gregglesDopry, I think you are expect something that this patch is not intended to do.
If a deleted user has a forum topic that they created, that node is deleted along with all of their other nodes.
The intention of this patch is just to show the forum topics where a deleted user has posted and by your comment:
I can see the forum topic that the test user commented the patch "works as designed", right?
Thanks for testing it.
Comment #11
dopry CreditAttribution: dopry commentedYes the patch works as designed... The othe part I noted is another forum bug I should post I guess.
+1 for the patch..
-fixes the deleted user/comments issue.
-I think it brings the JOINS in line with what is recommended after the new db_rewrite_sql patch..
.darrel.
Comment #12
Jose Reyero CreditAttribution: Jose Reyero commentedPlease see http://drupal.org/node/53348
Comment #13
dopry CreditAttribution: dopry commentedWe can save the user_delete handling for a later 4.8 conversation. Right now lets close this forum topics with comments owned by deleted users not showing up issue.
Comment #14
merlinofchaos CreditAttribution: merlinofchaos commentedI can +1 this patch; the query change is relatively simple, and we get the bonus of removing a construct that breaks under MySQL 5.0.12+ and db_rewrite_sql.
Comment #15
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedDid anybody test the performance impact this patch has? /forum is already quite slow and I'd hate to make it slower. Also I think that the issue should be resolved by either deleting the user's comments when deleting the user or assign them to anonymous. this would fix this automatically.
Comment #16
moshe weitzman CreditAttribution: moshe weitzman commentedgood catch killes. someone has to verify that this doesn't slow down large forums.
Comment #17
Jose Reyero CreditAttribution: Jose Reyero commentedSorry for insisting on this but,
This is not a specific forum problem. This happens in a lot of places in Drupal when you delete an user account associated with a node/comment/watchdog entry.
I've posted a patch -which also includes this one- here: http://drupal.org/node/53348
Comment #18
gregglesThe outer join will degrade performance, so I see the solutions as
1. if the performance impact is small enough, use the outer join anyway
2. Periodically (i.e. in cron) update some of these denormalized bits of information like the last comment UID, last comment time, etc. This has the drawback of potentially having incorrect data for the time between when a user is deleted and when cron is run, but has the benefit of not getting into the controversial user delete issue and being decent performance. This could also include some fixes like fixing comment count ( http://drupal.org/node/43528 )
3. fix the actual data inconsistency at the time the user is deleted by using the patch in http://drupal.org/node/53348 if that is acceptable (but I know that is controversial)
I'd be happy to test the performance of number 1 if people want me to.
Comment #19
chx CreditAttribution: chx commentedmoshe have said it in other issues that we should not delete users just mark them deleted. i agree
Comment #20
killes@www.drop.org CreditAttribution: killes@www.drop.org commented"mark as deleted" is more or less the same as "blocked". No need to duplicate.
Comment #21
chx CreditAttribution: chx commentedHow this is critical? User deletion is a rare thing and even rare that it matches the last_comment_uid.
Comment #22
catchin 5.x comments by deleted users are now assigned to anonymous
Comment #23
(not verified) CreditAttribution: commented