We allow nodes to receive their 'created' date passed into node_save(). The nearest analog to a comment's "created" date, however, is 'timestamp'. Timestamp isn't settable from outside comment_save(); some modules (such as mailhandler) just /assume/ that it is. This patch is necessary for imports from other applications, but also matches the patterns already established in node_save().

CommentFileSizeAuthor
#11 comment_save.patch580 bytesnevergone
#6 253577_comment_timestamp.patch711 bytesmorbus iff
#3 comment_timestamp.patch732 bytesmorbus iff
comment_timestamp.patch399 bytesmorbus iff

Comments

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work

A couple of things

patch is missing -u option (I am deeply shocked)
I think we should have 'created' and 'updated' timestamps for comments, this may be out of scope though.

morbus iff’s picture

morbus iff
they/them
commented
Status: Needs work » Needs review

If the way a patch was generated is a code quality issue, please set this back. Otherwise, pithy.

(And yeah, I agree: created/updated timestamps are out of scope here.)

morbus iff’s picture

morbus iff
they/them
commented
StatusFileSize
new comment_timestamp.patch732 bytes

Reapplied. This was the first time I had used cvs since I reinstalled the OS, so didn't have a .cvsrc file with the proper diff settings.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

Works as advertised ... fapi protects us from any unauthorized user sending a timestamp.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work

I'd love to commit this patch but it no longer applies. Needs a quick re-roll.

morbus iff’s picture

morbus iff
they/them
commented
StatusFileSize
new 253577_comment_timestamp.patch711 bytes

Attached. Sorry for the delay. Set to RTBC after re-roll.

morbus iff’s picture

morbus iff
they/them
commented
Status: Needs work » Reviewed & tested by the community
dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS. Thanks Morbus.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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

pasqualle’s picture

pasqualle
Location 🇭🇺 Budapest
commented
Version: 7.x-dev » 6.x-dev
Assigned: morbus iff » Unassigned
Status: Closed (fixed) » Patch (to be ported)
nevergone’s picture

nevergone
Primary language Hungarian
commented
Status: Patch (to be ported) » Reviewed & tested by the community
StatusFileSize
new comment_save.patch580 bytes

This patch only works with Drupal 6.3.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

6.x patch contained tabs, so fixed those before committing. Will be in next Drupal 6 release, thanks!

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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

cor3huis’s picture

cor3huis commented
Assigned: Unassigned » cor3huis
Status: Closed (won't fix) » Active

The original issue was invalid to begin with, "node_presave" should have been used I bet.

damien tournoud’s picture

damien tournoud commented
Component: comment.module » node.module

This is *not* the same issue. This issue was about *comments*.

I not sure I understand what you believe needs fixing in node_save(): by design, you can set $node->created, but not $node->changed, which will always be the timestamp of when node_save() was called.

Anyway, really not the same issue. Please open a different issue (targeted at Drupal 7, we always fix bug in the highest version first) and describe your use case there.

cor3huis’s picture

cor3huis commented
Assigned: cor3huis » Unassigned
Status: Active » Needs review

THX

cor3huis’s picture

cor3huis commented
Status: Closed (fixed) » Closed (won't fix)

Status: Needs review » Closed (won't fix)

Correcting issue status, see #2125263: Issue status not taken from last comment during D7 migration.