Because of a UPDATE-Query missing in advanced_forum_markasread(), markasread did not work propperly (Nodes allready present in {history} were not updated to current timestamp).

Instead of setting an additional UPDATE-Query, I added a DELETE before the INSERT. Because UPDATE seems to bring performance to its knees. Besides so only nodes are saved, that are within the NODE_NEW_LIMIT-tolerance, so {history} stays smaller, i guess.

Patch attached (on alpha12).

CommentFileSizeAuthor
advanced_forum_markasread_alpha12.patch3.24 KBderhasi

Comments

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

Could you define "not work properly"? I use it all the time on my site to mark everything read for UID 1 since I read them with my normal account. I've never had a problem with it. Is this a postgres problem?

Michelle

derhasi’s picture

derhasi commented

It's not a postgres poblem. I'm using MySQL.

The problem is, that users and me could not mark content as read. So I looked at your queries and recognized, that existing nids in history were not updated to the current timestamp, but that has to be done, for marking them as read. They were not updated, because there is only an INSERT for nodes that do not exist in {history} for the given user.

So the problem is, that once a node (of a given user) is in {history} it can't be set as read by markasread. So it would only work, for threads that were never visited by the user.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Version: 5.x-1.0-alpha12 » 5.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

Ahhhh, I see. Since I never actually read them with that account, that's why it works. I didn't write this code and I guess I never looked that close at it. Now that I look at it, you're right, of course. I'll get this into the next alpha. Setting RTBC just taking your word for it. I'll test it myself before I commit it.

Thanks!

Michelle

swill’s picture

swill commented

I have tested this patch. It seems to work, but it is not without some minor issues...

When I test it for a specific forum

if ($current_forum_id) {

it works fine without any issues (that i have found yet).

However if you do "Mark All Read" for all of the forum (aka on the front page of the forum)

} else { // else from if ($current_forum_id)

I get the following user warnings (with different numbers) every time I do it. I have done a bit of testing and it seems to happen all the time in that case. Here is the warning...

user warning: Duplicate entry '1-82658' for key 1 query: INSERT INTO history (uid,nid,timestamp) SELECT 1, n.nid, 1224177392 FROM node AS n INNER JOIN term_node AS tn ON n.nid=tn.nid INNER JOIN node_comment_statistics AS ncs ON ncs.nid=n.nid INNER JOIN term_data AS td ON tn.tid=td.tid WHERE (n.created > 1221585392 OR ncs.last_comment_timestamp > 1221585392) AND td.vid = 1 in /var/www/includes/database.mysql.inc on line 172.

Let me know if you have suggestions...

derhasi’s picture

derhasi commented

maybe we have to add a DISTINCT to the SELECT ...? That could be because of nodes that have associated two or more terms of the forums' vocabulary.

swill’s picture

swill commented

that seemed to fix the problem. thanks for the quick response derhasi...

this is the query that is working for me.

$sql = 'INSERT INTO {history} (uid,nid,timestamp)'
           .' SELECT DISTINCT %d, n.nid, %d FROM {node} AS n'
           .' INNER JOIN {term_node} AS tn ON n.nid=tn.nid' 
           .' INNER JOIN {node_comment_statistics} AS ncs ON ncs.nid=n.nid'
           .' INNER JOIN {term_data} AS td ON tn.tid=td.tid' 
		   		 .' WHERE (n.created > %d OR ncs.last_comment_timestamp > %d)'
           .' AND td.vid = %d';

cheer...

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Version: 5.x-1.x-dev » 6.x-1.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Thanks, guys, for getting this fixed up. I fixed the formatting and a some missed "AS"s and committed it to the D5 branch. I still need to port it to D6 but my dev site is hosed so it will have to wait a bit.

Michelle

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Status: Patch (to be ported) » Fixed

Committed to 6.x.

Michelle

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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