Whenever a comment is edited, the "created" timestamp still gets changed in two ways:

a) admin edit
While #1005004: Editing a comment destroys its creation date makes sure it isn't completely overwritten, it still gets rounded down to the full minute.
This is because the "Authored on" field in the Admin section only takes H:i (hh:mm), and this value gets saved to the record, whether it has been changed or not. This won't be a big thing in most cases, but we never know - it's simply not correct.
Probably the "Authored on" field should take H:i:s (hh:mm:ss), which seems the cleanest way and is consistent to the node edit form.

b) user edit
The second part of #714958: Comment timestamp lost when edited by administrator wasn't fixed either:

When a non-admin edits a comment, the "created" date is always set to now.

For non-admins the "Administration" section stays hidden. However, the "Authored on" field is fed with an empty string, which upon saving is automatically replaced by now().
Instead, the created date should be given, if existing (it obviously doesn't exist for new, unsaved comments).

Files: 
CommentFileSizeAuthor
#50 d7-comment_creation_date-1374090-50.patch12 KBLynxas
PASSED: [[SimpleTest]]: [MySQL] 40,717 pass(es).
[ View ]
#45 d8-comment_creation_date-1374090-45.patch3.1 KBdroplet
PASSED: [[SimpleTest]]: [MySQL] 58,996 pass(es).
[ View ]
#43 tests.patch1.7 KBdroplet
FAILED: [[SimpleTest]]: [MySQL] 58,412 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#43 d8-comment_creation_date-1374090-43.patch3.06 KBdroplet
PASSED: [[SimpleTest]]: [MySQL] 58,767 pass(es).
[ View ]
#41 d8-comment_creation_date-1374090-41.patch1.37 KBdroplet
PASSED: [[SimpleTest]]: [MySQL] 58,735 pass(es).
[ View ]
#36 d8-comment_creation_date-1374090-36.patch1.51 KBG.I.Joe
PASSED: [[SimpleTest]]: [MySQL] 58,915 pass(es).
[ View ]
#35 d8-comment-creation_date-1374090-35.patch1.68 KBG.I.Joe
PASSED: [[SimpleTest]]: [MySQL] 59,396 pass(es).
[ View ]
#33 comment-creation_date-1374090-33.patch1.64 KBG.I.Joe
PASSED: [[SimpleTest]]: [MySQL] 58,582 pass(es).
[ View ]
#28 comment-creation_date-1374090-28.patch2.94 KBbrenda003
FAILED: [[SimpleTest]]: [MySQL] 52,592 pass(es), 7 fail(s), and 493 exception(s).
[ View ]
#26 comment-creation_date-1374090-26.patch2.94 KBbrenda003
FAILED: [[SimpleTest]]: [MySQL] 52,213 pass(es), 7 fail(s), and 494 exception(s).
[ View ]
#23 d8-comment_date-1374090-23.patch2.8 KBmarthinal
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch d8-comment_date-1374090-23.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#21 d8-comment_date_new_comment-1374090-21.patch2.98 KBmarthinal
PASSED: [[SimpleTest]]: [MySQL] 42,084 pass(es).
[ View ]
#14 comment-creation_date-1374090-14.patch2.14 KBjfhovinne
PASSED: [[SimpleTest]]: [MySQL] 41,251 pass(es).
[ View ]
#7 comment_date.patch1.85 KBdroplet
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch comment_date.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#5 1374090-5.patch1.13 KBPancho
FAILED: [[SimpleTest]]: [MySQL] 37,326 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#3 1374090-3.patch1.15 KBPancho
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1374090-3.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#1 1374090.patch1.13 KBPancho
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1374090.patch. See the log in the details link for more information.
[ View ]

Comments

Issue summary:View changes

Expanded issue to cover both cases

Status:Needs work» Needs review
StatusFileSize
new1.13 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1374090.patch. See the log in the details link for more information.
[ View ]

Here's a patch that should fix both parts of the bug.

Status:Active» Needs work

The last submitted patch, 1374090.patch, failed testing.

StatusFileSize
new1.15 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1374090-3.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Sorry, malformatted for the testbot. This one should work, I hope.

Status:Needs review» Needs work

The last submitted patch, 1374090-3.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.13 KB
FAILED: [[SimpleTest]]: [MySQL] 37,326 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

Again malformed. Will it work this time?

Status:Needs review» Needs work

The last submitted patch, 1374090-5.patch, failed testing.

Version:7.x-dev» 8.x-dev
Status:Needs work» Needs review
Issue tags:+needs backport to D7
StatusFileSize
new1.85 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch comment_date.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

try this, bump up to D8 as well.

THX for fixing the patch and sorry for all the clutter. I'm going to test it as soon as I find some time.

I tried this patch on drupal 7.15 which still has the bug and it seems to solve the problem.

Issue tags:-needs backport to D7

#7: comment_date.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+needs backport to D7

The last submitted patch, comment_date.patch, failed testing.

Issue tags:+Novice

tagging Novice, anyone willing to reroll a patch, thanks.

Status:Needs work» Needs review
StatusFileSize
new2.14 KB
PASSED: [[SimpleTest]]: [MySQL] 41,251 pass(es).
[ View ]

Patch for d8.

The following test was performed:

1. Downloaded the latest copy of D8 and built a site
2. Added content and initial comments
3. Added a comment then updated the comment - Saw the error of the created date changed
Comment update before the patch
4. Applied the patch
5. Updated a previous comment - Saw that the created date did not change
comment after the patch
6. It appears that the user's comment update does not change the create date of the comments.

Note: I was not able to recreate the Admin error to verify that the patch corrected that issue.

Status:Reviewed & tested by the community» Needs review

Ok, patch works!

Status:Needs review» Reviewed & tested by the community

Works without patch for me too

Status:Needs review» Reviewed & tested by the community

#17, with admin user always works, the patch applied to the non-admin user.

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests

+++ b/core/modules/comment/lib/Drupal/comment/CommentFormController.php
@@ -64,7 +64,6 @@ class CommentFormController extends EntityFormController {
-      $date = (!empty($comment->date) ? $comment->date : format_date($comment->created, 'custom', 'Y-m-d H:i O'));
@@ -74,7 +73,11 @@ class CommentFormController extends EntityFormController {
+      $date = (!empty($comment->date) ? $comment->date : format_date($comment->created, 'custom', 'Y-m-d H:i:s O'));
+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentPreviewTest.php
@@ -85,7 +85,7 @@ class CommentPreviewTest extends CommentTestBase {
-    $expected_form_date = format_date($raw_date, 'custom', 'Y-m-d H:i O');
+    $expected_form_date = format_date($raw_date, 'custom', 'Y-m-d H:i:s O');

Hm. Can you explain why you're changing the date format? I think seconds granularity seems a little over the top to me. :) I'd rather retain the old version, unless there's a compelling reason not to do that.

+++ b/core/modules/comment/lib/Drupal/comment/CommentFormController.php
@@ -74,7 +73,11 @@ class CommentFormController extends EntityFormController {
+    if(!empty($comment->cid)) {

(nitpick) There should be a space between "if" and the "(" - See Coding standards

Also, let's make sure we add an automated test for this behaviour so we don't accidentally break it again.

@#19,

DB saving a UNIX timestamp and front UI using human-readable format:

Authored on: 2012-10-07 00:57 +0800

Every SECOND will cause a timestamp changes.

To reviewer:
Please wait 2 or 3 mins between each comment editing, or check the DB directly. It's a bug for all user-levels

Status:Needs work» Needs review
StatusFileSize
new2.98 KB
PASSED: [[SimpleTest]]: [MySQL] 42,084 pass(es).
[ View ]

Reviewing the patch I verify that it works as expected. So when a user edits his comment for example, the date and time is the same as in the date of comment creation. At the same time I see that when we edit the comment, we have a new comment.

If a comment is edited, we never know that there's another new comment or one comment modified.

I add a patch with the previous line(patch) and also some code to verify that we have a new comment when a new one is edited. If we have a new one ... and we edit this comment ... only 1 new comment is shown, not 2.

If you think this is an interesting feature, I could open a new issue or maybe change the title and fix this in the same issue...

Also I could add the test.

Assigned:Unassigned» marthinal
Status:Needs review» Needs work

About this issue test I think the problem was that we test using admin user and we need to try using web user.

So:

1) I will check if there's another feature request for d8 like added in the previous patch.

2) If not I'll create another issue with the feature.

3) I'll check the actual test and modify to test correctly with admin and web user.

Status:Needs work» Needs review
StatusFileSize
new2.8 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch d8-comment_date-1374090-23.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Patch + test

Status:Needs review» Needs work
Issue tags:+Needs tests, +Novice, +needs backport to D7

The last submitted patch, d8-comment_date-1374090-23.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.94 KB
FAILED: [[SimpleTest]]: [MySQL] 52,213 pass(es), 7 fail(s), and 494 exception(s).
[ View ]

Patch rerolled.

Status:Needs review» Needs work

The last submitted patch, comment-creation_date-1374090-26.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.94 KB
FAILED: [[SimpleTest]]: [MySQL] 52,592 pass(es), 7 fail(s), and 493 exception(s).
[ View ]

Patch rerolled.

Status:Needs review» Needs work

The last submitted patch, comment-creation_date-1374090-28.patch, failed testing.

Test case on #7. doesn't it enough ?

Assigned:marthinal» Unassigned

@marthinal, are you still work on it ? :)

Assigned:Unassigned» G.I.Joe

If you don't mind I will take over this issue.

Status:Needs work» Needs review
StatusFileSize
new1.64 KB
PASSED: [[SimpleTest]]: [MySQL] 58,582 pass(es).
[ View ]

It's was not possible to reroll the previous patch.
So, I created a new patch.

Status:Needs review» Needs work

The patch contains some weird characters and I'm unable to apply it:

$ patch -p1 < comment-creation_date-1374090-33.patch
patch: **** Only garbage was found in the patch input.
$ git apply comment-creation_date-1374090-33.patch
fatal: unrecognized input

Strange that the test bot didn't complain. Can you perhaps reroll the patch?

Status:Needs work» Needs review
StatusFileSize
new1.68 KB
PASSED: [[SimpleTest]]: [MySQL] 59,396 pass(es).
[ View ]

It's was not possible to reroll the previous patch
So, I created a new patch.

StatusFileSize
new1.51 KB
PASSED: [[SimpleTest]]: [MySQL] 58,915 pass(es).
[ View ]

The patch contained color characters.

Please, try following patch.

Status:Needs review» Needs work

All few reroll, all tests have been gone.

Indeed, there were some tests in the patch from #28 and these have been lost in the reroll.

+++ b/core/modules/comment/lib/Drupal/comment/CommentFormController.phpundefined
@@ -110,7 +110,7 @@ public function form(array $form, array &$form_state) {
-      $date = (!empty($comment->date) ? $comment->date : DrupalDateTime::createFromTimestamp($comment->created->value));
+      $date = (!empty($comment->date) ? $comment->date : (isset($comment->created->value) ? DrupalDateTime::createFromTimestamp($comment->created->value) : ''));

Nested ternaries are hard to read, maybe you can split it in two lines?
Also, as a minor remark, it is not needed to wrap the entire line in parentheses. This was present in the original code but it can be cleaned up now.

+++ b/core/modules/comment/lib/Drupal/comment/CommentFormController.phpundefined
@@ -125,7 +125,7 @@ public function form(array $form, array &$form_state) {
-      $date = '';
+      $date = (!empty($comment->date) ? $comment->date : (isset($comment->created->value) ? DrupalDateTime::createFromTimestamp($comment->created->value) : ''));
     }

Same remark as above: split nested ternaries, and remove unneeded parentheses.

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests

I was looking into the missing tests from patch #28 and comparing it to the current drupal code. I may be mistaken but it looks like some tests were already added to make sure the created date is the same as before it was edited.

// Check that the saved comment is still correct.
$comment_loaded = comment_load($comment->id(), TRUE);
$this->assertEqual($comment_loaded->subject->value, $edit['subject'], 'Subject loaded.');
$this->assertEqual($comment_loaded->comment_body->value, $edit['comment_body[0][value]'], 'Comment body loaded.');
$this->assertEqual($comment_loaded->name->value, $edit['name'], 'Name loaded.');
$this->assertEqual($comment_loaded->created->value, $raw_date, 'Date loaded.');

I'd say #7 test / first part of #28 test is good enough for this change.

Status:Needs work» Needs review
Issue tags:-Needs tests
StatusFileSize
new1.37 KB
PASSED: [[SimpleTest]]: [MySQL] 58,735 pass(es).
[ View ]

OK. D8 changed the tests, now it has already caught seconds:

    $expected_form_date = $date->format('Y-m-d');
    $expected_form_time = $date->format('H:i:s');

Status:Needs review» Reviewed & tested by the community

Well done, Paljas.
I reviewed the code and fix solved issue.

StatusFileSize
new3.06 KB
PASSED: [[SimpleTest]]: [MySQL] 58,767 pass(es).
[ View ]
new1.7 KB
FAILED: [[SimpleTest]]: [MySQL] 58,412 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Sorry. I just read this thread again. We need a test case for non-admin users.

Issue tags:-Needs tests
  1. +++ b/core/modules/comment/lib/Drupal/comment/CommentFormController.php
    @@ -125,7 +124,11 @@ public function form(array $form, array &$form_state) {
    +      $date = (!empty($comment->date) ? $comment->date : DrupalDateTime::createFromTimestamp($comment->created->value));

    It is not needed to wrap a ternary operator in parentheses.

  2. +++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentPreviewTest.php
    @@ -164,6 +164,15 @@ function testCommentEditPreviewSave() {
    +    // Check that the date and time of the comment are correct when edited by non-admin users.

    Comment exceeds 80 characters.

  3. +++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentPreviewTest.php
    @@ -164,6 +164,15 @@ function testCommentEditPreviewSave() {
    +    $this->drupalLogout();
    +    $user_edit = array();
    +    $expected_created_time = $comment_loaded->created->value;
    +    $this->drupalLogin($web_user);
    +    $this->drupalPostForm('comment/' . $comment->id() . '/edit', $user_edit, t('Save'));

    I would put the $this->drupalLogout() and $this->drupalLogin() next to eachother.

    The $user_edit array is only used in one line so it can be replaced with an inline array() in $this->drupalPostForm().

  4. +++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentPreviewTest.php
    @@ -164,6 +164,15 @@ function testCommentEditPreviewSave() {
    +    $comment_loaded = comment_load($comment->id(), TRUE);
    +    $this->assertEqual($comment_loaded->created->value, $expected_created_time, 'Expected date and time for comment edited.');

    I would use $comment rather than $comment_loaded.

Status:Needs work» Needs review
StatusFileSize
new3.1 KB
PASSED: [[SimpleTest]]: [MySQL] 58,996 pass(es).
[ View ]

Although I think only #2 need to fix, just make extra more changes in this patch to fix everyone needs. :)

#4,
$comment is the original comment object.
$comment_loaded is a new comment object.

We also keep it same to previous usage:

    // Check that the saved comment is still correct.
    $comment_loaded = comment_load($comment->id(), TRUE);

Thanks.

Status:Needs review» Reviewed & tested by the community

Makes sense. It's looking good now, thanks!

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Awesome work, on both the find and the fix!

Committed and pushed to 8.x. Thanks!

Moving to 7.x backport.

Issue summary:View changes

minor stylefix

Status:Patch (to be ported)» Needs review
StatusFileSize
new12 KB
PASSED: [[SimpleTest]]: [MySQL] 40,717 pass(es).
[ View ]

Back port from Drupal 8 to Drupal 7.

I had to change the original code that was into the Drupal 8 patch.

Status:Needs review» Needs work

wrong patch and still working on the issue

Assigned:G.I.Joe» Unassigned