Hi and thanks for the great module!

One feature requested by one of our clients is to manually reopen comments on some nodes. Right now, it is perfectly possible to reopen commenting by switching the node's comment settings to "Read/Write". However, whenever Comment Closer is activated by cron again, the comments on these nodes are reclosed. It would be great if there was some mechanism to keep these nodes' comments open.

Would this feature be a useful addition, do you think, and any ideas on how best to implement it?

Thanks, David

CommentFileSizeAuthor
#5 reopen-comments-4574740-3.patch7.23 KBdbassendine
#2 reopen-comments-4574740-2.patch6.85 KBrjmackay

Comments

rjmackay’s picture

rjmackay commented

I'm just looking at doing this now.

My quick thoughts on an approach are:
Add a table with columns: nid, status (maybe vid too?)
This would track each node that has been reopened, or marked do not close
i.e. statuses
0 - done nothing (not sure why this would happen but just in case)
1 - autoclosed
2 - reopened
3 - do not close
Populate this table when comments are auto closed.
Add a check box for 'do not auto-close comments' to the node edit form.. save this value to this table.
If comments are reopened after being autoclosed then save that into this table. Do not reclose this comments on cron runs.

Not sure I need all those statuses, but that's just my initial thoughts.
Does this sound reasonable?
Would you accept a patch to add this functionality?
Torchbox (sponsoring this development) would be keen to have this added back to contrib.

Thanks

rjmackay’s picture

rjmackay commented
StatusFileSize
new reopen-comments-4574740-2.patch6.85 KB

Attaching a patch that adds an option on each node to stop closing comments.

Designed so this could be extended to include custom closing settings per node too. But currently just allow you to reopen comments, set comment closer to donotclose so they stay open.

rjmackay’s picture

rjmackay commented
Status: Active » Needs review
dbassendine’s picture

dbassendine commented

Works well for me. A note: unlike the above, the status column seems to reflect:

0 - Use site default settings
1 - Do not close

Is that correct and the intended behaviour? I don't think myself that the other statuses are needed, so I would agree, it's best to go for simplicity.

Thanks, David

dbassendine’s picture

dbassendine commented
StatusFileSize
new reopen-comments-4574740-3.patch7.23 KB

On further testing, it emerged that the above approach (#2) does not work if the comment_closer_node table is not populated (ie. each node would have to have an entry), and it doesn't have a mechanism for populating the table when new content types are enabled, or new nodes are created.

I've reviewed the approach: in order to avoid building up a big unwieldy database table with entries for all nodes for all enabled content types, I suggest rewriting the queries to assume that a node follows the site default if it is not in the table. Only if there is an entry, and that entry = 1, would the node preference override the site default. This is more expensive in terms of the number of queries, but each query should be faster in a smaller table. In addition, the table will only ever be queried for nodes that fit the sitewide configuration criteria, and already have comments enabled.

I also fixed a bug that prevented the "Do not close" dropdown from appearing when configuring to close comments based on the number of comments.

Patch attached below.

Thanks, David

rjmackay’s picture

rjmackay commented

Thanks for the fix David.
And yes you are correct about the status column values.
The idea is that it could be extended if more granular behaviour is needed per node. But those 2 values were all that's needed to reopen comments

dbassendine’s picture

dbassendine commented

Thanks for the work on this, Robbie. I think this could still be extended to use multiple status values if the need arises. The main change here is that if a node isn't in the table a value of 0 is assumed, to avoid having to generate massive tables of zeroes.

One possible addition may be to inform the user if they save a node with "do not close comments" but forget to actually open the comments to "Read/write" on the node. I'm not sure what the correct behaviour would be here, though. Suggestions welcome.

Cheers, David

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented

Set the default value to 0 and use a LEFT JOIN, then a missing value becomes 0 (or at least NULL).

Status: Needs review » Needs work

The last submitted patch, reopen-comments-4574740-3.patch, failed testing.