While testing the pgsql patch I discovered that the current MAR functionality misses some posts on my dev site. Need to investigate.

Michelle

CommentFileSizeAuthor
#3 advanced_forum-470202.patch1.44 KBtom_o_t

Comments

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Version: 6.x-1.x-dev » 6.x-2.x-dev

Bumping to development branch. Will backport when it's fixed.

Michelle

michelle’s picture

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

The likely culprit:

[12:51] I will note that mark as read compares node.created and Views, at least, compares node.changed
[12:51] So with this query I think a node with a very old created and a more recent changed might get marked unread because its history record would get deleted and no one one might get added.

tom_o_t’s picture

tom_o_t commented
Status: Active » Needs review
StatusFileSize
new advanced_forum-470202.patch1.44 KB

Here's the patch for testing - thanks to merlinofchaos for getting to the root of this one

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Status: Needs review » Fixed

I reviewed this enough to test that it didn't break MAR but I didn't set up the edge case the fix targetted. Going to call this fixed. If I (or anyone) run into any more weirdness, I'll re-open.

Thanks,

Michelle

neclimdul’s picture

neclimdul
He/Him
commented
Status: Fixed » Needs review

seems to be working for me.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs review » Fixed
dparrott’s picture

dparrott commented

hi,
is this fix in the current dev version of advanced forums? my users are running into this issue.

Thanks!
dave

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Version: 6.x-2.x-dev » 6.x-1.x-dev
Status: Fixed » Patch (to be ported)

It's fixed in 2.x. I haven't backported it to 1.x. I suppose I could do that. It's a small enough thing.

Michelle

dparrott’s picture

dparrott commented

well, what do you suggest? I could either upgrade to 2.x-dev or use the 1.x-dev (if you want to port it), or finally, if you give me the fix I could just put it into my current code base (if it isn't too complicated).

michelle’s picture

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

The fix is in #3. No, I don't recommend upgrading to 2.x. I'll get it into the 1.x dev eventually. This is a weird edge case thing that rarely happens so it hasn't been a super high priority for me.

Michelle

dparrott’s picture

dparrott commented

thanks, michelle. guess i didn't look close enough to the thread. sorry about that!

dparrott’s picture

dparrott commented

well, i noticed this works when i am at a forum page (viewing just topics for a forum). But, when i try to mark all read from a container page (containing multiple forums), it doesn't seem to work. am i missing something?

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Title: Mark all read doesn't always work » Mark all read needs to use changed date, not created date of node

Containers can't have posts in them so there would be nothing to mark read. Being able to mark all forums in a container all read at once would be a new feature request. The code isn't designed to handle that, and that's unrelated to this issue.

Michelle

dparrott’s picture

dparrott commented
Title: Mark all read needs to use changed date, not created date of node » Mark all read doesn't always work

oh ok, i guess i'll just make it so the button doesn't show up in containers.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented
Title: Mark all read doesn't always work » Mark all read needs to use changed date, not created date of node

Fixing title so I remember what this is.

Michelle

michelle’s picture

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

Ok, backported to both 1.x and D5.

Michelle

Status: Fixed » Closed (fixed)

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

maulwuff’s picture

maulwuff commented

please create a new D5 release, as the backport is still in the dev branch. It's missing in 5.x-1.1