471523 - cloning a bug copies the newest visible comment for Newest to Oldest comment order

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[timeless]

1. load https://bugzilla.mozilla.org/userprefs.cgi
2. set:
When viewing a bug, show comments in this order [Newest to Oldest]
3. load a bug, e.g. bug 468617
4. click "clone"
5. click "Bugzilla"
6. examine the comment that was copied

expected results:
(presumably) copy the initial comment (unless it's hidden!)

actual results:
it copied the newest comment.

Comment 1

[Frédéric Buclin]

In enter_bug.cgi, $vars->{'comment'} = $cloned_bug->longdescs->[0]->{'body'}; will return the first comment based on your preferences. We should probably call GetComments() directly so that we can pass the sort order, and eventually limit the search to the first comment. Else if you clone a bug with 200 comments, they are all returned, with 200 commenter objects, despite you absolutely don't care about the other 199 comments.

Comment 2

[Frédéric Buclin]

Attachment: patch for tip, v1

This patch is for tip only as it depends on bug 472556, which permits to call GetComments() with an end time only.

The way I modified the code in enter_bug.cgi makes it only load the first comment instead of all comments of the original bug, which is a small perf win. To do this, I had to change the format of the creation_ts field to also return seconds, else GetComments() would see creation_ts (without seconds) as always being older than the real creation date of the bug (with seconds).

For 3.2, I will attach a much less invasive patch.

Comment 3

[Frédéric Buclin]

Attachment: patch for 3.2, v1

This backport is safe as it doesn't alter Bug.pm at all.

Comment 4

[Frédéric Buclin]

Comment on attachment 358718 [details] [diff] [review]
patch for tip, v1

errr... this one is not obsolete.

Comment 5

[Max Kanat-Alexander]

Comment on attachment 358718 [details] [diff] [review]
patch for tip, v1

>+    $dbh->sql_date_format('creation_ts', '%Y.%m.%d %H:%i:%s') . ' AS creation_ts',

  That puts seconds into the bugmail comment header. :-(

Comment 6

[Frédéric Buclin]

(In reply to comment #5)
>   That puts seconds into the bugmail comment header. :-(

Seconds are already displayed in bugmail comment headers. That's unrelated to my patch.

Comment 7

[Frédéric Buclin]

OK, I'm going to land the "patch for 3.2" on both tip and the 3.2 branch, as mkanat seems to hesitate about the "patch for tip". But I still think it's the right way to go to avoid loading all comments when only the first one is in use.

Comment 8

[Frédéric Buclin]

tip:

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.168; previous revision: 1.167
done

3.2:

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.160.2.5; previous revision: 1.160.2.4
done