Direct linked comment to correct page on multipage threads [#378356]

Hi,

Love this module!

This would be a nice to have:

For nodes that have more than one page of comments, it would be great if the comment link in the notification email also contained the page the comment is on (in addition to the comment anchor link).

Thanks,
Mike

Comment	File	Size	Author
#3	watcher-comment-page.patch	1.1 KB	mpaler

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

solipsist CreditAttribution: solipsist commented 27 February 2009 at 00:37

Good idea! I'm keeping it for version 2!

Comment #2

solipsist CreditAttribution: solipsist commented 27 February 2009 at 00:37

Status:

Active

» Postponed

Comment #3

mpaler CreditAttribution: mpaler commented 30 March 2009 at 00:55

Status:

Postponed

» Patch (to be ported)

File	Size
watcher-comment-page.patch	1.1 KB

Hi,

I've managed to get this going on my 5.x installation. Attached is a patch with my mods...

Mike

Comment #4

solipsist CreditAttribution: solipsist commented 30 March 2009 at 04:55

Nice, but this will always point to the last page of comments. Someone may have replied to an earlier comment and the comment in reply may be on the first page of comments. Because of that you need to recursively back track up the tree of comments to get the ordered sequence number of the comment the notification is about and then derive its page number. Also, we need to be able to take care of reverse date order for comments (latest first).

Comment #5

mpaler CreditAttribution: mpaler commented 30 March 2009 at 22:09

Oh jeez, you're right. How selfish of me: I didn't even think of that because I'm forcing my forum to be Flat list - expanded & oldest first so new comments are always on the bottom...

I'm afraid I'm super swamped right now -- Hopefully later this week I can can give this some more thought. However, this may prove to be a much bigger problem than I bargained for.

Mike

Comment #6

solipsist CreditAttribution: solipsist commented 3 April 2009 at 05:47

I had a sense it was a tricky one, why I've delayed implementing it. If you can come up with a patch that implements this in a way that works with what I wrote above, I could add it straight away. When patching, please use the latest version from CVS rather than the releases on the project page.