Fix $node->last_comment_timestamp and introduce $node->last_activity_timestamp [#1112044]

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
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

pillarsdotnet CreditAttribution: pillarsdotnet commented 1 June 2011 at 21:51

Title:	$node->last_comment_timestamp is misleading	» Rename last_comment_timestamp to last_activity_timestamp
Status:	Active	» Needs review

File	Size
rename-last_comment_timestamp-to-last_activity_timestamp-1112044-1.patch	16.15 KB

Patch

Comment #2

scor CreditAttribution: scor commented 2 June 2011 at 16:08

Status:

Needs review

» Needs work

The tracker module adds the last_activity to 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 CreditAttribution: pillarsdotnet commented 2 June 2011 at 16:10

Status:

Needs work

» Needs review

@#2 -- Out of scope. Tracker is not part of comment.module. Please file a separate issue.

Comment #4

scor CreditAttribution: scor commented 2 June 2011 at 16:17

Title:	Rename last_comment_timestamp to last_activity_timestamp	» Fix $node->last_comment_timestamp and introduce $node->last_activity_timestamp
Status:	Needs review	» Needs work

I 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 CreditAttribution: pillarsdotnet commented 2 June 2011 at 16:21

$last_activity_timestamp = max($node->changed, $node->last_comment_timestamp);

Why do you need a separate field for that?

Comment #6

scor CreditAttribution: scor commented 2 June 2011 at 16:32

I 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 CreditAttribution: scor commented 2 June 2011 at 16:54

Also 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

jhedstrom

English

Portland, OR

CreditAttribution: jhedstrom at Phase2 commented 7 March 2016 at 17:08

Issue summary:	View changes
Status:	Needs work	» Closed (outdated)

Tentatively 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).