I would expect last_comment_timestamp to be NULL is no comment has been left on a node. This is not the case, $node->last_comment_timestamp is set to $node->created upon node insert. The logic is valid for a value such as last_comment_or_change, but last_comment_timestamp as it is currently implemented is wrong.
We could either rename that variable to last_comment_or_change or last_activity, or fix last_comment_timestamp. or both.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | rename-last_comment_timestamp-to-last_activity_timestamp-1112044-1.patch | 16.15 KB | pillarsdotnet |
Comments
Comment #1
pillarsdotnet commentedPatch
Comment #2
scor commentedThe tracker module adds the
last_activityto nodes, so the last_activity_timestamp would be redundant. Maybe tracker module could be fixed to used that new value instead of its own custom last_activity?Comment #3
pillarsdotnet commented@#2 -- Out of scope. Tracker is not part of comment.module. Please file a separate issue.
Comment #4
scor commentedI would like to keep/fix last_comment_timestamp, this is can be useful along with last_comment_uid for printing the name and date of the last comment on a teaser for example. last_comment_timestamp should be 0 if there are no comment.
re tracker, this could be part of a follow up, so please ignore for now.
Comment #5
pillarsdotnet commented$last_activity_timestamp = max($node->changed, $node->last_comment_timestamp);Why do you need a separate field for that?
Comment #6
scor commentedI don't necessarily want a separate field for last_activity_timestamp, as you are showing, it's quite easy to compute.
The point of this issue is really that last_comment_timestamp contains the wrong value. so the current value of last_comment_timestamp should go into a new attribute last_activity_timestamp (or discarded), and most importantly, the attribute last_comment_timestamp should be kept to contain the actual last comment timestamp or 0 if there has been no comment, but not the node last node changed timestamp.
Note that your patch is good, it renames last_comment_timestamp to last_activity_timestamp. It just lacks the old last_comment_timestamp attribute which should contain the last comment timestamp.
Comment #7
scor commentedAlso the description of the last_comment_timestamp in the node_comment_statistics table is wrong:
last_comment_timestamp The Unix timestamp of the last comment that was posted within this node, from comment.timestamp.it can be the node create stamp if no comments exist for the node
Comment #8
jhedstromTentatively closing out due to inactivity, feel free to re-open if there's interest in pursuing this change (it would most likely need to wait for 9.x unless the field could be changed in a backward compatible way).