Closed (duplicate)
Project:
Drupal core
Version:
7.x-dev
Component:
comment.module
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
15 Feb 2010 at 16:47 UTC
Updated:
31 Dec 2010 at 10:56 UTC
Jump to comment: Most recent file
Comments
Comment #1
mfbComment #2
mfbChanged the logic around because
$comment->createddoesn't always exist.Comment #4
mfbFailed because the entered date string is now getting processed and translated to the local time zone. Tweaked test to expect the date to be formatted in the local time zone.
Comment #5
retester2010 commented#4: comment-date.patch queued for re-testing.
Comment #7
salvisThis bothers me, too. As administrator I want to be able to edit a comment without changing the timestamp that appears in the GUI, just like editing nodes. As the OP explained, D7 initializes the Authored on textfield with the
changedtimestamp rather than thecreatedone (!) and then stores it back into thecreatedfield.The attached patch fixes this: it initializes the Authored on textfield with the
createdtimestamp, so that the administrator can save the comment without changing the timestamp that appears on the comment view.From the OP:
This is correct behavior, IMO. My patch does not change this.
Comment #9
salvisJust get the right value...
Comment #11
mfbWhat makes this correct? Doesn't
$comment->changedexist so that$comment->createdcan store the time the comment was created while$comment->changedcan store the time the comment was last updated?I don't think I agree, but setting to needs review so test bot will give it a whirl.
Comment #12
salvisNon-admins should not be able to change their comments without leaving a (visible-for-all) trace that they did so. This is how d.o here works: if you go back and edit your #4 above (for the sake of the example), then it'll get today's timestamp and you cannot claim that "you've said it all back in February"...
This, to me, is more important than knowing when you wrote the original version of #4.
Thanks for setting the status!
Comment #13
mfbfine :p although that sounds like a theme issue to me -- the site should display "Updated:
$comment->changed" for non-admin comments..I couldn't help but notice a bunch of micro-optimizations: 1) get rid of extra parentheses, 2) use isset(), and 3) use date() rather than format_date() because this is a numeric date format that can't be translated (and there's no need to apply a time zone because Drupal now sets the PHP default time zone to the user's time zone during bootstrap).
Comment #15
salvis1. It's a good habit to always enclose the ternary operator in parentheses, because its precedence is not well-established. Different programming languages handle it differently — those parentheses are free and they ensure that there are no surprises.
2. !empty() is not exactly the same as isset(). I wouldn't trade one for the other without having really really studied the implications. I haven't found out where $comment->date could come from — the safe thing to do is to preserve what's there and tested.
3. The testbot disagrees.
Comment #16
mfbhmm couldn't reproduce the test failure so not sure what the problem was. Let's roll this back to your patch from #9
Comment #17
mfbAh I believe the test failure for #13 was caused by #808560: Node comment statistics are only partially exposed in node_load() and are missing last_comment_uid
Comment #18
salvisShoot, we've been there seven weeks ago, but we should have marked this 'critical'...
Fixed in #1005004: Editing a comment destroys its creation date...