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.

CommentFileSizeAuthor
#8 forum_deleted_user.patch1.66 KBgreggles
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wesley Tanaka’s picture

Priority: Normal » Critical

Sorry. If any of you are "following along from home," I meant
http://treehouse.ofb.net/go/en/node/74

Wesley Tanaka’s picture

It's being excluded with this clause:
l.last_comment_uid = cu.uid

Wesley Tanaka’s picture

Title: forum topic not showing up in forum index » forum topic does not show up in forum index if associated last_comment_uid user no longer exists

Changed 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.

Wesley Tanaka’s picture

The line in question being line 694 of forum.module (v1.312)

beginner’s picture

Hello 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.

Wesley Tanaka’s picture

Since 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.

greggles’s picture

Status: Active » Needs review

wtanaka'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).

greggles’s picture

FileSize
1.66 KB

Forgot the patch...

dopry’s picture

This 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.

greggles’s picture

Dopry, 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.

dopry’s picture

Yes 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.

Jose Reyero’s picture

dopry’s picture

We 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.

merlinofchaos’s picture

Status: Needs review » Reviewed & tested by the community

I 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.

killes@www.drop.org’s picture

Did 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.

moshe weitzman’s picture

Status: Reviewed & tested by the community » Needs work

good catch killes. someone has to verify that this doesn't slow down large forums.

Jose Reyero’s picture

Sorry 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

greggles’s picture

The 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.

chx’s picture

moshe have said it in other issues that we should not delete users just mark them deleted. i agree

killes@www.drop.org’s picture

"mark as deleted" is more or less the same as "blocked". No need to duplicate.

chx’s picture

Priority: Critical » Normal

How this is critical? User deletion is a rare thing and even rare that it matches the last_comment_uid.

catch’s picture

Version: x.y.z » 5.x-dev
Status: Needs work » Fixed

in 5.x comments by deleted users are now assigned to anonymous

Anonymous’s picture

Status: Fixed » Closed (fixed)